mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): handle invalid retention rows and remove sentinel file
The migration wrote synthetic events with zero pubkey/sig when signing keys were unavailable at startup. These rows break load_from_retention because nostr::Event deserialization rejects the placeholder values. Additionally, the sentinel file (migration_state.json) caused a filesystem write on every launch regardless of whether migration ran. Fix both criticals: 1. Migration now resolves keys from the identity.key file (which exists from prior launches), making the no-keys path unreachable in practice. When it is reached, stores content-only rows (empty raw_event, "unkeyed" placeholder pubkey) instead of invalid synthetic events. 2. load_from_retention branches on raw_event validity: tries nostr::Event parse first, falls back to parsing the content column directly as PersonaEventContent. Also queries for unkeyed migration rows. 3. Replaces the sentinel file with a DB-row-exists idempotency check, eliminating write amplification on every launch. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
co-authored by
Will Pfleger
parent
3c6cd9c3f0
commit
b9f0dd2319
@@ -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<PathBuf, String> {
|
||||
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<PersonaRecord, String> {
|
||||
// Try parsing raw_event as a valid nostr::Event first.
|
||||
if !row.raw_event.is_empty() {
|
||||
if let Ok(event) = serde_json::from_str::<nostr::Event>(&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<Vec<PersonaRecord>, String> {
|
||||
let path = personas_store_path(app)?;
|
||||
let now = now_iso();
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
|
||||
@@ -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::<nostr::Keys>().ok());
|
||||
.and_then(|k| k.parse::<nostr::Keys>().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::<nostr::Keys>().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!(
|
||||
|
||||
Reference in New Issue
Block a user