From cdf52df270dc5337bdc15faab17cee0d46bbdd4a Mon Sep 17 00:00:00 2001 From: Duncan Date: Sat, 29 Aug 2026 20:19:10 -0400 Subject: [PATCH] fix(personas): dual-store loader, post-fold regression, outcome logging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IMPORTANT: refresh_for_persona_at read only personas.json, which is retired (renamed .bak) by the Phase 1A.2 fold migration. On a post-fold install this resolved zero definitions, causing resolve_team_members to fail and resolve_and_refresh_or_retract_at to retract (purge+tombstone) every affected shared 30178 head. A successful persona retry/share-toggle would silently destroy the team catalog and report success. Fix: replace the single-store load in refresh_for_persona_at with the same dual-store rule as boot reconciliation: try personas.json first; if empty, read keyless definition records from managed-agents.json. Reuses crate::event_sync::read_persona_definitions (promoted to pub(crate)). Test: split the regression into pre-fold and post-fold variants. Post-fold variant: teams.json + keyless managed-agents.json record, personas.json explicitly absent. Verified mutations: - Mutation A (single-store revert): pre-fold GREEN, post-fold RED 0/1. - Mutation B (delete refresh call): both RED 0/2; restored: both GREEN 2/2. MINOR 1: remove stale coordinator comment claiming the flush loop retries and reopen/save forces retry — both proven false by Thufir's trace. MINOR 2: add per-team eprintln logging in refresh_for_persona_at for RemovalQueued and Err outcomes, matching the logging in the AppHandle path (refresh_shared_team_catalog_heads_for_persona). Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../src-tauri/src/commands/teams/pending.rs | 32 ++- .../src/commands/teams/pending/tests.rs | 217 +++++++++++++----- desktop/src-tauri/src/event_sync.rs | 2 +- .../agents/ui/agentSaveCoordinator.ts | 7 +- 4 files changed, 191 insertions(+), 67 deletions(-) diff --git a/desktop/src-tauri/src/commands/teams/pending.rs b/desktop/src-tauri/src/commands/teams/pending.rs index cc1b8c6573..9c449fc218 100644 --- a/desktop/src-tauri/src/commands/teams/pending.rs +++ b/desktop/src-tauri/src/commands/teams/pending.rs @@ -439,11 +439,12 @@ pub(super) fn refresh_shared_team_catalog_heads_for_persona( /// File-system seam for [`refresh_shared_team_catalog_heads_for_persona`]. /// -/// Reads teams and personas from flat JSON files in `base_dir` rather than -/// through the Tauri store. Calls the SAME `resolve_and_refresh_or_retract_at` -/// that production uses — the seam is a thin file-loading shim with no -/// independent logic. Tests therefore exercise the exact production code path, -/// and production can call this seam directly when the base directory is known. +/// Reads teams and personas from flat JSON files in `base_dir` using the same +/// dual-store rule as boot reconciliation: legacy `personas.json` when +/// non-empty, otherwise keyless definition records from `managed-agents.json` +/// (see [`crate::event_sync::read_persona_definitions`]). This makes the seam +/// correct on both pre-fold and post-fold installs. Calls the SAME +/// `resolve_and_refresh_or_retract_at` that the AppHandle path uses. pub(crate) fn refresh_for_persona_at( base_dir: &std::path::Path, keys: &nostr::Keys, @@ -454,15 +455,28 @@ pub(crate) fn refresh_for_persona_at( let teams: Vec = read_json_store(&base_dir.join("teams.json"))?; - let personas: Vec = - read_json_store(&base_dir.join("personas.json"))?; + let personas = crate::event_sync::read_persona_definitions(base_dir)?; for team in &teams { if team.is_builtin || !team.persona_ids.iter().any(|id| id == persona_id) { continue; } - // Identical call to production — no parallel implementation. - let _ = resolve_and_refresh_or_retract_at(db_path, keys, team, &personas); + let outcome = resolve_and_refresh_or_retract_at(db_path, keys, team, &personas); + match outcome { + Ok(RefreshOrRetractOutcome::RemovalQueued { ref reason }) => { + eprintln!( + "buzz-desktop: team-catalog-refresh: retracting '{}' after persona edit — {reason}", + team.name + ); + } + Err(ref e) => { + eprintln!( + "buzz-desktop: team-catalog-refresh: '{}' after persona edit — {e}", + team.name + ); + } + _ => {} + } } Ok(()) } diff --git a/desktop/src-tauri/src/commands/teams/pending/tests.rs b/desktop/src-tauri/src/commands/teams/pending/tests.rs index bcb1a90066..70a2bb1aea 100644 --- a/desktop/src-tauri/src/commands/teams/pending/tests.rs +++ b/desktop/src-tauri/src/commands/teams/pending/tests.rs @@ -827,19 +827,84 @@ fn test_all_catalog_call_paths_produce_a_dominating_tombstone() { mod cross_device; mod gate; -/// Command-path regression: `set_persona_shared` (the publish-retry seam) must -/// refresh affected shared 30178 heads after it publishes the persona head. -/// -/// This test calls `publish_and_refresh_teams_at` — the extracted command core -/// that production `set_persona_shared` delegates to. Deleting the -/// `refresh_for_persona_at` call from that function turns this test RED; -/// restoring it returns GREEN. The previous stand-in called `refresh_for_persona_at` -/// independently, so it passed even when the production wiring was removed. +// ── Command-path regression: publish-retry team-refresh ─────────────────────── +// +// Both variants call `publish_and_refresh_teams_at` — the extracted command +// core that production `set_persona_shared` delegates to. They cover: +// - Pre-fold: persona definitions in `personas.json` (legacy path). +// - Post-fold: definitions as keyless records in `managed-agents.json`; no +// `personas.json` present (the normal state after Phase 1A.2 fold). +// +// Deletion mutation for both: removing the `refresh_for_persona_at` call from +// `publish_and_refresh_teams_at` turns both tests RED; restoring returns GREEN. +// Additionally, reverting the loader to the single-store path (hard-coding +// `personas.json`) turns only the post-fold test RED, proving the dual-store +// fix is load-bearing. + +fn post_fold_write_stores( + base_dir: &std::path::Path, + teams: &[TeamRecord], + personas: &[AgentDefinition], +) { + std::fs::write( + base_dir.join("teams.json"), + serde_json::to_string(teams).unwrap(), + ) + .unwrap(); + // Post-fold: definitions live as keyless records in managed-agents.json. + // personas.json is absent (retired by the fold migration). + let records: Vec = personas + .iter() + .cloned() + .map(|p| p.into_agent_record()) + .collect(); + std::fs::write( + base_dir.join("managed-agents.json"), + serde_json::to_string(&records).unwrap(), + ) + .unwrap(); + // Assert personas.json absent so the test correctly models the post-fold state. + assert!( + !base_dir.join("personas.json").exists(), + "post-fold fixture must not have personas.json" + ); +} + +async fn run_publish_and_refresh_for_team( + dir: &std::path::Path, + db_path: &std::path::Path, + keys: &nostr::Keys, + persona_def: &AgentDefinition, + team_id: &str, +) { + let (event, retained, persona) = crate::commands::personas::prepare_persona_publication_at( + db_path, + keys, + persona_def, + Some(true), + ) + .unwrap(); + let prepared = crate::commands::personas::PreparedPersonaPublication { + scope: crate::managed_agents::retention::RetentionScope { + db_path: db_path.to_path_buf(), + relay_url: "http://127.0.0.1:1".to_string(), // unreachable → queued, not error + owner_keys: keys.clone(), + }, + event, + retained, + persona, + }; + let state = crate::app_state::build_app_state(); + crate::commands::personas::publish_and_refresh_teams_at( + &state, prepared, dir, keys, db_path, team_id, + ) + .await + .unwrap(); +} + +/// Pre-fold variant: definitions in `personas.json`. #[tokio::test] -async fn test_set_persona_shared_publish_retry_refreshes_shared_team_head() { - // Scenario: persona "m1" belongs to a shared team. After a publish-retry - // (persona updated on disk, catalog not yet refreshed), calling the command - // core must update the 30178 team head to reflect the new persona content. +async fn test_set_persona_shared_publish_retry_refreshes_shared_team_head_pre_fold() { let dir = tempfile::tempdir().unwrap(); let keys = nostr::Keys::generate(); let owner = keys.public_key().to_hex(); @@ -848,8 +913,6 @@ async fn test_set_persona_shared_publish_retry_refreshes_shared_team_head() { let m1_before = member("m1", "Original prompt."); let t = team_with_members("team-retry", "Retry Team", vec!["m1".to_string()]); - // Pre-share the team head with the original member prompt so the refresh - // has a stale head to replace. prepare_team_publication_at( &db_path, &keys, @@ -869,48 +932,23 @@ async fn test_set_persona_shared_publish_retry_refreshes_shared_team_head() { "pre-retry head must reflect the original persona prompt" ); - // Simulate the persona being updated on disk (prompt rewritten) — this is - // the state `set_persona_shared` sees after the persona save lands. + // Pre-fold store: definitions in personas.json. let m1_after = member("m1", "Updated prompt after publish-retry."); - write_stores(dir.path(), std::slice::from_ref(&t), &[m1_after]); + write_stores( + dir.path(), + std::slice::from_ref(&t), + std::slice::from_ref(&m1_after), + ); - // Build a PreparedPersonaPublication for m1 at the updated-prompt state. - // This mirrors what `set_persona_shared` does before calling - // `publish_and_refresh_teams_at`. - let persona_def = member("m1", "Updated prompt after publish-retry."); - let (event, retained, persona) = crate::commands::personas::prepare_persona_publication_at( + run_publish_and_refresh_for_team( + dir.path(), &db_path, &keys, - &persona_def, - Some(true), - ) - .unwrap(); - let prepared = crate::commands::personas::PreparedPersonaPublication { - scope: crate::managed_agents::retention::RetentionScope { - db_path: db_path.clone(), - relay_url: "http://127.0.0.1:1".to_string(), // unreachable → queued, not error - owner_keys: keys.clone(), - }, - event, - retained, - persona, - }; - - // Call the extracted command core — the function `set_persona_shared` delegates to. - // An unreachable relay returns Ok(Queued), so the call succeeds. - let state = crate::app_state::build_app_state(); - crate::commands::personas::publish_and_refresh_teams_at( - &state, - prepared, - dir.path(), // base_dir: teams.json + personas.json live here - &keys, - &db_path, + &member("m1", "Updated prompt after publish-retry."), "m1", ) - .await - .unwrap(); + .await; - // The 30178 team head must now reflect the updated persona content. let head_after = { let conn = open_retention_db(&db_path).unwrap(); get_retained_event(&conn, KIND_TEAM_CATALOG, &owner, "team-retry") @@ -921,12 +959,83 @@ async fn test_set_persona_shared_publish_retry_refreshes_shared_team_head() { head_after .content .contains("Updated prompt after publish-retry."), - "the team 30178 must reflect the updated persona content after the command core fires" - ); - assert!( - head_after.pending_sync, - "the refreshed 30178 head must be queued for relay sync" + "the team 30178 must reflect the updated persona content (pre-fold)" ); + assert!(head_after.pending_sync, "refreshed head must be queued"); + assert!( + head_after.created_at >= head_before.created_at, + "refreshed head must not predate the original" + ); +} + +/// Post-fold variant: definitions as keyless records in `managed-agents.json`; +/// `personas.json` absent. This is the normal state after the Phase 1A.2 fold. +/// Reverting `refresh_for_persona_at` to read only `personas.json` turns this +/// test RED (zero definitions → retraction instead of refresh). +#[tokio::test] +async fn test_set_persona_shared_publish_retry_refreshes_shared_team_head_post_fold() { + let dir = tempfile::tempdir().unwrap(); + let keys = nostr::Keys::generate(); + let owner = keys.public_key().to_hex(); + let db_path = scoped_db(dir.path(), "wss://a.example", &owner); + + let m1_before = member("m1", "Original prompt."); + let t = team_with_members("team-retry", "Retry Team", vec!["m1".to_string()]); + + // Pre-share the team head using the pre-fold writer (personas.json exists here). + prepare_team_publication_at( + &db_path, + &keys, + &t, + std::slice::from_ref(&m1_before), + Some(true), + ) + .unwrap(); + let head_before = { + let conn = open_retention_db(&db_path).unwrap(); + get_retained_event(&conn, KIND_TEAM_CATALOG, &owner, "team-retry") + .unwrap() + .expect("shared head must exist before retry") + }; + assert!( + head_before.content.contains("Original prompt."), + "pre-retry head must reflect the original persona prompt" + ); + + // Write the post-fold store: teams.json + managed-agents.json with a + // keyless record, no personas.json. + let m1_after = member("m1", "Updated prompt after publish-retry."); + post_fold_write_stores( + dir.path(), + std::slice::from_ref(&t), + std::slice::from_ref(&m1_after), + ); + + run_publish_and_refresh_for_team( + dir.path(), + &db_path, + &keys, + &member("m1", "Updated prompt after publish-retry."), + "m1", + ) + .await; + + // The 30178 team head must reflect the updated prompt — not be retracted. + let head_after = { + let conn = open_retention_db(&db_path).unwrap(); + get_retained_event(&conn, KIND_TEAM_CATALOG, &owner, "team-retry") + .unwrap() + .expect( + "shared head must still exist after post-fold retry-path refresh — not retracted", + ) + }; + assert!( + head_after + .content + .contains("Updated prompt after publish-retry."), + "the team 30178 must reflect the updated persona content (post-fold, keyless managed-agents.json)" + ); + assert!(head_after.pending_sync, "refreshed head must be queued"); assert!( head_after.created_at >= head_before.created_at, "refreshed head must not predate the original" diff --git a/desktop/src-tauri/src/event_sync.rs b/desktop/src-tauri/src/event_sync.rs index 2745ce5917..27ff2feb1b 100644 --- a/desktop/src-tauri/src/event_sync.rs +++ b/desktop/src-tauri/src/event_sync.rs @@ -766,7 +766,7 @@ pub(crate) fn read_json_store_pub( /// agent store; `personas.json` survives only on a boot where the fold /// errored. Both callers must read the same set — a reconcile that saw an /// empty persona list would conclude every team's members were deleted. -fn read_persona_definitions( +pub(crate) fn read_persona_definitions( base_dir: &Path, ) -> Result, String> { let personas: Vec = diff --git a/desktop/src/features/agents/ui/agentSaveCoordinator.ts b/desktop/src/features/agents/ui/agentSaveCoordinator.ts index 304dc27c5c..c023fb223c 100644 --- a/desktop/src/features/agents/ui/agentSaveCoordinator.ts +++ b/desktop/src/features/agents/ui/agentSaveCoordinator.ts @@ -432,9 +432,10 @@ export async function runAgentSaveCoordinator( // status — the relay was never reached. Attempt a publish-only retry via // set_persona_shared (reads the current on-disk definition; no field write // needed). If the retry succeeds, fall through to full success. If it - // fails (or no retry seam is available), report a terminal "saved locally, - // not published" state — the flush loop will re-attempt on reconnect, and - // the user can re-open and save again to force a retry. + // fails (or no retry seam is available), report only that the profile saved + // locally and could not be published — no automatic-retry promise and no + // reopen instruction (a fresh reopen seeds persisted values, emits null + // personaInput, and the coordinator never re-attempts publication). const personaName = observedPersona?.displayName ?? personaInput?.displayName ??