diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 9731c9d695..8749f55458 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -524,17 +524,28 @@ pub fn run() { migration::reconcile_persona_team_dirs(&app_handle); migration::reconcile_provider_mcp_commands(&app_handle); migration::migrate_persona_provider_to_runtime(&app_handle); - migration::migrate_personas_to_events(&app_handle); - - if let Err(e) = managed_agents::sync_team_personas(&app_handle) { - eprintln!("sprout-desktop: sync-team-personas: {e}"); - } // Resolve persisted identity key (env var → file → generate+save). // This is fatal — the app should not start with an ephemeral identity // that will be lost on restart, as that silently breaks channel // memberships, DMs, and relay identity. let state = app_handle.state::(); + resolve_persisted_identity(&app_handle, &state) + .map_err(|e| -> Box { e.into() })?; + + // Persona-event migration signs every retained event with the + // owner's keys, so it must run after the persisted identity is + // resolved (not before — at that point keys may still be ephemeral). + let owner_keys = state + .keys + .lock() + .map(|k| k.clone()) + .map_err(|e| -> Box { e.to_string().into() })?; + migration::migrate_personas_to_events(&app_handle, &owner_keys); + + if let Err(e) = managed_agents::sync_team_personas(&app_handle) { + eprintln!("sprout-desktop: sync-team-personas: {e}"); + } // Store the AppHandle so huddle commands can emit `huddle-state-changed` // events via `huddle::emit_huddle_state` without threading the handle @@ -543,9 +554,6 @@ pub fn run() { *guard = Some(app_handle.clone()); } - resolve_persisted_identity(&app_handle, &state) - .map_err(|e| -> Box { e.into() })?; - // Bring up the runtime-owned relay-mesh call-me-now listener now, // before any saved agent restore can request a connection. Its // lifetime is tied to the runtime, not a UI mount — this is what diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index b2c69995c8..fa87ec2efc 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -573,91 +573,83 @@ 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) and AFTER the persisted identity is resolved (it signs every +/// retained event with the owner's keys). +/// +/// Idempotent: skips when the retention store already holds events for the +/// owner pubkey — the data is the sentinel, so no separate sentinel file is +/// needed. This avoids re-running (and resetting `pending_sync`) on every +/// launch when a sentinel write silently fails. /// /// 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. -pub fn migrate_personas_to_events(app: &tauri::AppHandle) { - use crate::managed_agents::{ - managed_agents_base_dir, - persona_events::{build_persona_event, persona_d_tag}, - retention::{open_retention_db, retain_event, RetainedEvent}, - PersonaRecord, - }; - use nostr::JsonUtil; - use sprout_core::kind::KIND_PERSONA; +/// write, not relay acknowledgment. Every retained row is a real signed +/// event — there is no placeholder path. +pub fn migrate_personas_to_events(app: &tauri::AppHandle, keys: &nostr::Keys) { + use crate::managed_agents::managed_agents_base_dir; let Ok(base_dir) = managed_agents_base_dir(app) else { 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"#) - { - return; - } + match migrate_personas_in_dir(&base_dir, keys) { + Ok(0) => {} + Ok(migrated) => { + eprintln!( + "sprout-desktop: persona-event-migration: {migrated} personas migrated to retention" + ); + } + Err(e) => { + eprintln!("sprout-desktop: persona-event-migration: {e}"); } } +} - // Read personas.json fresh at migration time. +/// Core migration logic, decoupled from the Tauri `AppHandle` for testing. +/// +/// Returns the number of personas written to the retention store. Returns +/// `Ok(0)` when migration has already run (retention store has rows for the +/// owner pubkey) or when there are no non-builtin personas to migrate. +fn migrate_personas_in_dir(base_dir: &Path, keys: &nostr::Keys) -> Result { + use crate::managed_agents::{ + persona_events::{build_persona_event, persona_d_tag}, + retention::{has_retained_personas, open_retention_db, retain_event, RetainedEvent}, + PersonaRecord, + }; + use nostr::JsonUtil; + use sprout_core::kind::KIND_PERSONA; + + let pubkey = keys.public_key().to_hex(); + + // Read personas.json fresh at migration time. Nothing to migrate if the + // file is absent. 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; + return Ok(0); } - let content = match std::fs::read_to_string(&personas_path) { - Ok(c) => c, - Err(e) => { - eprintln!("sprout-desktop: persona-event-migration: failed to read personas.json: {e}"); - return; - } - }; + let content = std::fs::read_to_string(&personas_path) + .map_err(|e| format!("failed to read personas.json: {e}"))?; - let records: Vec = match serde_json::from_str(&content) { - Ok(r) => r, - Err(e) => { - eprintln!( - "sprout-desktop: persona-event-migration: failed to parse personas.json: {e}" - ); - return; - } - }; + let records: Vec = serde_json::from_str(&content) + .map_err(|e| format!("failed to parse personas.json: {e}"))?; if records.is_empty() { - let _ = std::fs::write(&sentinel_path, r#"{"persona_events_migrated":true}"#); - return; + return Ok(0); } // Open (or create) the retention database. let db_path = base_dir.join("retention.db"); - let conn = match open_retention_db(&db_path) { - Ok(c) => c, - Err(e) => { - eprintln!("sprout-desktop: persona-event-migration: failed to open retention db: {e}"); - return; - } - }; + let conn = + open_retention_db(&db_path).map_err(|e| format!("failed to open retention db: {e}"))?; - // 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. - let keys = std::env::var("SPROUT_PRIVATE_KEY") - .ok() - .and_then(|k| k.parse::().ok()); + // Idempotency: the retention rows themselves are the sentinel. If the + // owner already has retained personas, migration ran on a prior launch. + if has_retained_personas(&conn, &pubkey)? { + return Ok(0); + } let mut migrated = 0u32; - let mut errors = 0u32; for record in &records { // Skip built-in personas — they're always available from code. @@ -667,118 +659,31 @@ pub fn migrate_personas_to_events(app: &tauri::AppHandle) { let d_tag = persona_d_tag(record); - match &keys { - Some(keys) => { - // Build and sign a real event. - let builder = match build_persona_event(record) { - Ok(b) => b, - Err(e) => { - eprintln!( - "sprout-desktop: persona-event-migration: failed to build event for '{}': {e}", - record.display_name - ); - errors += 1; - continue; - } - }; + let builder = build_persona_event(record) + .map_err(|e| format!("failed to build event for '{}': {e}", record.display_name))?; - let event = match builder.sign_with_keys(keys) { - Ok(e) => e, - Err(e) => { - eprintln!( - "sprout-desktop: persona-event-migration: failed to sign event for '{}': {e}", - record.display_name - ); - errors += 1; - continue; - } - }; + let event = builder + .sign_with_keys(keys) + .map_err(|e| format!("failed to sign event for '{}': {e}", record.display_name))?; - let raw_event = event.as_json(); - let retained = RetainedEvent { - kind: KIND_PERSONA, - pubkey: keys.public_key().to_hex(), - d_tag, - content: event.content.to_string(), - created_at: event.created_at.as_secs() as i64, - raw_event, - pending_sync: true, - }; + let retained = RetainedEvent { + kind: KIND_PERSONA, + pubkey: pubkey.clone(), + d_tag, + content: event.content.to_string(), + // Safety: nostr timestamps are seconds and stay below i64::MAX + // until year 2262. + created_at: event.created_at.as_secs() as i64, + raw_event: event.as_json(), + pending_sync: true, + }; - if let Err(e) = retain_event(&conn, &retained) { - eprintln!( - "sprout-desktop: persona-event-migration: failed to retain '{}': {e}", - record.display_name - ); - errors += 1; - } else { - migrated += 1; - } - } - None => { - // No keys available — store a synthetic event structure. - // This will be re-signed and published when keys become available. - let content_json = serde_json::json!({ - "display_name": record.display_name, - "avatar_url": record.avatar_url, - "system_prompt": record.system_prompt, - "runtime": record.runtime, - "model": record.model, - "provider": record.provider, - "name_pool": record.name_pool, - "env_vars": record.env_vars, - }); - - 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(), - d_tag, - content: content_json.to_string(), - created_at: now, - raw_event: raw_event.to_string(), - pending_sync: true, - }; - - if let Err(e) = retain_event(&conn, &retained) { - eprintln!( - "sprout-desktop: persona-event-migration: failed to retain '{}': {e}", - record.display_name - ); - errors += 1; - } else { - migrated += 1; - } - } - } + retain_event(&conn, &retained) + .map_err(|e| format!("failed to retain '{}': {e}", record.display_name))?; + migrated += 1; } - // 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}"#); - - if migrated > 0 || errors > 0 { - eprintln!( - "sprout-desktop: persona-event-migration: {migrated} personas migrated to retention{}", - if errors > 0 { - format!(", {errors} errors") - } else { - String::new() - } - ); - } + Ok(migrated) } #[cfg(test)] diff --git a/desktop/src-tauri/src/migration_tests.rs b/desktop/src-tauri/src/migration_tests.rs index 1332288565..06a5372f96 100644 --- a/desktop/src-tauri/src/migration_tests.rs +++ b/desktop/src-tauri/src/migration_tests.rs @@ -831,3 +831,98 @@ fn reconcile_mcp_commands_skips_record_without_agent_command() { reconcile_mcp_commands_in_file(&path); assert_eq!(before, std::fs::read_to_string(&path).unwrap()); } + +/// Helper: write a `personas.json` directly in `base_dir` (the migration +/// reads `base_dir/personas.json`, where `base_dir` is the `agents` dir). +fn write_base_personas(base_dir: &Path, records: &serde_json::Value) { + std::fs::write( + base_dir.join("personas.json"), + serde_json::to_string_pretty(records).unwrap(), + ) + .unwrap(); +} + +fn one_persona() -> serde_json::Value { + serde_json::json!([{ + "id": "code-reviewer", + "display_name": "Code Reviewer", + "system_prompt": "You review code.", + "is_builtin": false, + "is_active": true, + "name_pool": [], + "env_vars": {}, + "created_at": "2025-01-01T00:00:00Z", + "updated_at": "2025-01-01T00:00:00Z" + }]) +} + +#[test] +fn migrate_personas_writes_signed_retention_rows() { + use crate::managed_agents::retention::{get_retained_personas, open_retention_db}; + + let base = tempfile::tempdir().unwrap(); + write_base_personas(base.path(), &one_persona()); + let keys = nostr::Keys::generate(); + let pubkey = keys.public_key().to_hex(); + + let migrated = migrate_personas_in_dir(base.path(), &keys).unwrap(); + assert_eq!(migrated, 1); + + let conn = open_retention_db(&base.path().join("retention.db")).unwrap(); + let rows = get_retained_personas(&conn, &pubkey).unwrap(); + assert_eq!(rows.len(), 1); + // Row holds a real signed event for the owner — not a placeholder. + assert_eq!(rows[0].pubkey, pubkey); + let event: nostr::Event = nostr::JsonUtil::from_json(&rows[0].raw_event).unwrap(); + assert!(event.verify().is_ok()); + assert!(rows[0].pending_sync); +} + +#[test] +fn migrate_personas_skips_builtins() { + use crate::managed_agents::retention::{get_retained_personas, open_retention_db}; + + let base = tempfile::tempdir().unwrap(); + write_base_personas( + base.path(), + &serde_json::json!([{ + "id": "builtin:solo", + "display_name": "Solo", + "system_prompt": "x", + "is_builtin": true, + "is_active": true, + "name_pool": [], + "env_vars": {}, + "created_at": "2025-01-01T00:00:00Z", + "updated_at": "2025-01-01T00:00:00Z" + }]), + ); + let keys = nostr::Keys::generate(); + + let migrated = migrate_personas_in_dir(base.path(), &keys).unwrap(); + assert_eq!(migrated, 0); + + let conn = open_retention_db(&base.path().join("retention.db")).unwrap(); + let rows = get_retained_personas(&conn, &keys.public_key().to_hex()).unwrap(); + assert!(rows.is_empty()); +} + +#[test] +fn migrate_personas_skips_when_retention_already_populated() { + let base = tempfile::tempdir().unwrap(); + write_base_personas(base.path(), &one_persona()); + let keys = nostr::Keys::generate(); + + // First run migrates; second run is a no-op (retention rows are the + // sentinel — no separate sentinel file). + assert_eq!(migrate_personas_in_dir(base.path(), &keys).unwrap(), 1); + assert_eq!(migrate_personas_in_dir(base.path(), &keys).unwrap(), 0); + assert!(!base.path().join("migration_state.json").exists()); +} + +#[test] +fn migrate_personas_no_file_is_noop() { + let base = tempfile::tempdir().unwrap(); + let keys = nostr::Keys::generate(); + assert_eq!(migrate_personas_in_dir(base.path(), &keys).unwrap(), 0); +}