From 96b9b0aca151245813d76e7556b8c822d2cb3f07 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Fri, 31 Jul 2026 10:40:36 -0400 Subject: [PATCH] fix(desktop): generation-gate barrier enforcement, import-identity barrier, reconcile Result propagation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four pass-3 findings resolved: 1. CRITICAL — generation-gate the enforcement phase: added `is_current()` on `ReadinessClaim`; `run_boot_barrier_for_scope` now checks it while holding `managed_agents_store_lock` and returns `Abandoned` without writing if the claim is preempted. Stale relay evidence (e.g. `HeadState::Absent` re-opening a parked coordinate) can no longer reach `run_decision_pass`. 2. IMPORTANT — test (j): exercises the stale barrier's actual decision pass through the enforcement seam. Vacuity block confirms the stale evidence WOULD clear `publish_blocked` on an unguarded scratch db; enforcement seam shows the guard blocks the write on the real store. 3. IMPORTANT — `import_identity` now calls `force_claim_in_progress` and `spawn_event_sync_with_held_claim` after swapping keys, so the new owner's disk projections are enqueued and the barrier certifies the scope ready. Without this, config publication stopped silently until restart. 4. IMPORTANT — reconcile legs return `Result`: `migrate_personas_to_events`, `migrate_teams_to_events`, and `reconcile_agents_to_events` all return `Result<(), String>`; `run_event_sync` propagates with `?`; `spawn_event_sync_with_held_claim` drops the claim without completing when any leg fails, leaving the scope `Unready` for retry rather than certifying `Ready` after a partial reconcile. All four fixes are in `desktop/src-tauri` only. 2111 tests pass, clippy -D warnings clean, cargo fmt clean, just desktop-check green. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/src-tauri/src/commands/identity.rs | 30 +++ desktop/src-tauri/src/event_sync.rs | 79 ++++-- .../src/managed_agents/config_barrier.rs | 18 +- .../managed_agents/config_sync_readiness.rs | 11 + .../persona_events/flush_barrier_tests.rs | 227 ++++++++++++++++++ .../src-tauri/src/managed_agents/reconcile.rs | 8 +- 6 files changed, 349 insertions(+), 24 deletions(-) diff --git a/desktop/src-tauri/src/commands/identity.rs b/desktop/src-tauri/src/commands/identity.rs index 33ecf3cfca..ab7230614b 100644 --- a/desktop/src-tauri/src/commands/identity.rs +++ b/desktop/src-tauri/src/commands/identity.rs @@ -374,6 +374,36 @@ pub async fn import_identity( eprintln!("buzz-desktop: imported identity pubkey {}", pubkey_hex); + // Enqueue the new owner's disk projections and run the boot barrier. + // Without this, config publication silently stops until the next + // restart: the flush loop checks `is_ready_for(db_path)` against the + // OLD owner's scope, and the new owner's scope was never certified. + // + // Mirrors `apply_workspace`'s force-claim sequence. The legacy- + // migration step is skipped here — only the keys changed, not the + // relay or scope database, so there is nothing to adopt from a + // pre-scoping global store. + { + match crate::managed_agents::retention::active_retention_scope( + &app_handle, + &state, + ) { + Ok(scope) => { + let claim = + crate::managed_agents::config_sync_readiness::force_claim_in_progress(); + crate::event_sync::spawn_event_sync_with_held_claim( + app_handle.clone(), + scope.owner_keys, + scope.db_path, + claim, + ); + } + Err(e) => { + eprintln!("buzz-desktop: import-identity: scope unavailable, config barrier skipped: {e}"); + } + } + } + Ok(IdentityInfo { pubkey: pubkey_hex, display_name, diff --git a/desktop/src-tauri/src/event_sync.rs b/desktop/src-tauri/src/event_sync.rs index 7b66766ff3..4a5861544d 100644 --- a/desktop/src-tauri/src/event_sync.rs +++ b/desktop/src-tauri/src/event_sync.rs @@ -13,10 +13,20 @@ use std::path::Path; /// `sync_team_personas` wrote in [`crate::migration::run_boot_migrations`] /// (see its `# Ordering` guard). Event signing needs the resolved owner keys, /// so this runs after identity resolution, not in the boot migrations. -pub fn run_event_sync(app: &tauri::AppHandle, owner_keys: &nostr::Keys, db_path: &Path) { - 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); +/// +/// Returns `Err` if any reconcile leg fails. A failed leg means some +/// coordinates were not written to the retention store; the caller must not +/// certify the scope `Ready` — it stays `Unready` and the full transition +/// (reconcile + barrier) runs again on the next flush retry. +pub fn run_event_sync( + app: &tauri::AppHandle, + owner_keys: &nostr::Keys, + db_path: &Path, +) -> Result<(), String> { + 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)?; + Ok(()) } /// Variant for workspace applies: spawn the reconcile+barrier with an @@ -50,18 +60,35 @@ pub fn spawn_event_sync_with_held_claim( ) { tauri::async_runtime::spawn(async move { let barrier_app = app.clone(); - if let Err(e) = tauri::async_runtime::spawn_blocking(move || { - run_event_sync(&app, &owner_keys, &db_path); + let reconcile_result = tauri::async_runtime::spawn_blocking(move || { + run_event_sync(&app, &owner_keys, &db_path) }) - .await - { - eprintln!("buzz-desktop: event-sync: spawn_blocking failed: {e}"); + .await; + + match reconcile_result { + Ok(Ok(())) => { + // Reconcile succeeded. InProgress is still held here — no + // mark_unready between reconcile and the barrier. + // run_boot_barrier_after_claim owns the transition to Ready or + // Unready without re-claiming. + crate::managed_agents::config_barrier::run_boot_barrier_after_claim( + &barrier_app, + claim, + ) + .await; + } + Ok(Err(e)) => { + // A required reconcile leg failed. Drop the claim without + // completing — RAII resets to Unready (if still current gen), + // leaving the scope fail-closed for retry on the next flush tick. + eprintln!("buzz-desktop: event-sync: reconcile failed, not certifying Ready: {e}"); + drop(claim); + } + Err(e) => { + eprintln!("buzz-desktop: event-sync: spawn_blocking failed: {e}"); + drop(claim); + } } - // InProgress is still held here — no mark_unready between reconcile - // and the barrier. run_boot_barrier_after_claim owns the transition - // to Ready or Unready without re-claiming. - crate::managed_agents::config_barrier::run_boot_barrier_after_claim(&barrier_app, claim) - .await; }); } @@ -85,22 +112,28 @@ pub fn spawn_event_sync_with_held_claim( /// `pending_sync = 1` for later relay publish. Migration succeeds on local /// 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, db_path: &Path) { +pub fn migrate_personas_to_events( + app: &tauri::AppHandle, + keys: &nostr::Keys, + db_path: &Path, +) -> Result<(), String> { use crate::managed_agents::managed_agents_base_dir; let Ok(base_dir) = managed_agents_base_dir(app) else { - return; + return Ok(()); }; match migrate_personas_in_dir_at(&base_dir, keys, db_path) { - Ok(0) => {} + Ok(0) => Ok(()), Ok(migrated) => { eprintln!( "buzz-desktop: persona-event-migration: {migrated} personas migrated to retention" ); + Ok(()) } Err(e) => { eprintln!("buzz-desktop: persona-event-migration: {e}"); + Err(e) } } } @@ -233,20 +266,26 @@ fn migrate_personas_in_dir_at( /// /// Must run after the persisted identity is resolved (it signs each event with /// the owner's keys). -pub fn migrate_teams_to_events(app: &tauri::AppHandle, keys: &nostr::Keys, db_path: &Path) { +pub fn migrate_teams_to_events( + app: &tauri::AppHandle, + keys: &nostr::Keys, + db_path: &Path, +) -> Result<(), String> { use crate::managed_agents::managed_agents_base_dir; let Ok(base_dir) = managed_agents_base_dir(app) else { - return; + return Ok(()); }; match migrate_teams_in_dir_at(&base_dir, keys, db_path) { - Ok(0) => {} + Ok(0) => Ok(()), Ok(migrated) => { eprintln!("buzz-desktop: team-event-migration: {migrated} teams migrated to retention"); + Ok(()) } Err(e) => { eprintln!("buzz-desktop: team-event-migration: {e}"); + Err(e) } } } diff --git a/desktop/src-tauri/src/managed_agents/config_barrier.rs b/desktop/src-tauri/src/managed_agents/config_barrier.rs index 2a7d466805..7796850376 100644 --- a/desktop/src-tauri/src/managed_agents/config_barrier.rs +++ b/desktop/src-tauri/src/managed_agents/config_barrier.rs @@ -123,7 +123,7 @@ async fn run_boot_barrier_enforcing( } }; - match run_boot_barrier_for_scope(app, &state, &scope).await { + match run_boot_barrier_for_scope(app, &state, &scope, &claim).await { Ok(BarrierOutcome::Success) => { // complete() transitions to Ready only if this claim's generation // still matches — a preempted claim is a silent no-op. @@ -190,6 +190,7 @@ async fn run_boot_barrier_for_scope( app: &tauri::AppHandle, state: &AppState, scope: &RetentionScope, + claim: &super::config_sync_readiness::ReadinessClaim, ) -> Result { let owner_pubkey = scope.owner_keys.public_key().to_hex(); @@ -274,6 +275,21 @@ async fn run_boot_barrier_for_scope( }); } + // Phase 4 — generation-gate the enforcement phase. + // + // A preempted barrier that has stale relay evidence (e.g. it observed + // `HeadState::Absent` for a coordinate the current barrier just parked) + // must not overwrite the current barrier's gate decisions. The check runs + // while `managed_agents_store_lock` is held, so the generation cannot + // change between this check and the `run_decision_pass` write below. + if !claim.is_current() { + tracing::info!( + target: "buzz::config_sync", + "boot barrier abandoned: preempted before enforcement; no gate writes applied" + ); + return Ok(BarrierOutcome::Abandoned); + } + run_decision_pass(&conn, &owner_pubkey, &states)?; Ok(BarrierOutcome::Success) } diff --git a/desktop/src-tauri/src/managed_agents/config_sync_readiness.rs b/desktop/src-tauri/src/managed_agents/config_sync_readiness.rs index dbb50fb18e..5771fa09fa 100644 --- a/desktop/src-tauri/src/managed_agents/config_sync_readiness.rs +++ b/desktop/src-tauri/src/managed_agents/config_sync_readiness.rs @@ -114,6 +114,17 @@ impl ReadinessClaim { Self { generation } } + /// Return `true` iff this claim's generation still matches the latch. + /// + /// Used by the barrier's enforcement phase: while holding + /// `managed_agents_store_lock`, call this immediately before + /// [`run_decision_pass`][super::config_sync::run_decision_pass] to ensure + /// a preempted barrier cannot overwrite gate decisions produced by the + /// current owner's barrier. + pub fn is_current(&self) -> bool { + with_lock(|latch| latch.generation == self.generation).unwrap_or(false) + } + /// Transition the latch to `Ready(db_path)` if and only if this claim is /// still current (latch generation matches). A preempted claim's call is /// a silent no-op — the preemptor owns the latch now. diff --git a/desktop/src-tauri/src/managed_agents/persona_events/flush_barrier_tests.rs b/desktop/src-tauri/src/managed_agents/persona_events/flush_barrier_tests.rs index d13a70436d..b342deb027 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events/flush_barrier_tests.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events/flush_barrier_tests.rs @@ -442,6 +442,233 @@ async fn test_apply_workspace_flush_cannot_interleave_before_reconcile() { config_sync_readiness::mark_unready(); // cleanup } +// ── (j) generation-gate blocks stale barrier's decision pass ───────────────── +// +// Pass-3 CRITICAL finding: before this fix, `run_decision_pass` wrote gate +// decisions unconditionally — a preempted barrier resuming after the current +// one certified `Ready` could overwrite newer gate decisions with its stale +// relay evidence. The defect was invisible to the 2110-test suite because +// test (i) only verified that `complete()` is a no-op; it never drove a +// stale barrier all the way through enforcement. +// +// This test exercises the enforcement seam directly: stale barrier S builds a +// `CoordinateState` from stale relay evidence (the exact CRITICAL cell — +// `HeadState::Absent` sees cell-3 `PublishLocalEdit`, opening a gate the +// current barrier parked) and calls `run_decision_pass` while gated by the +// `is_current()` check. The generation guard must block the write. +// +// Deterministic sequence: +// 1. Flush wins claim (gen 0). Its barrier observes `HeadState::Absent` for +// the coordinate — stale evidence that would decide `PublishLocalEdit`. +// 2. apply_workspace force-claims (gen 1). The current barrier observes +// `HeadState::Present` and decides `Park(NoBaselineWithHead)`. +// 3. Current barrier (gen 1) runs `run_decision_pass` → sets +// `publish_blocked = true`. Certifies Ready(path). +// 4. Stale barrier (gen 0) resumes enforcement with its stale evidence. +// Before calling `run_decision_pass`, the generation guard (`is_current()`) +// returns false — write is blocked. `publish_blocked` must remain `true`. +// 5. Vacuity proof: without the guard, the stale pass WOULD clear the flag +// (verified on a scratch db so the real store is never mutated by the test). +#[test] +fn test_stale_barriers_decision_pass_cannot_alter_publish_blocked() { + use crate::managed_agents::{ + canonical_projection::CanonicalProjection, + config_sync::{local_state, plan, run_decision_pass, CoordinateState}, + decision::{Decision, Head, HeadState, TombstoneEvidence}, + head_lookup::Coordinate, + retention::{is_publish_blocked, open_retention_db, retain_event, RetainedEvent}, + }; + use buzz_core_pkg::kind::KIND_PERSONA; + use nostr::{EventBuilder, JsonUtil, Kind, Tag}; + + config_sync_readiness::mark_unready(); + + let keys = nostr::Keys::generate(); + let pubkey = keys.public_key().to_hex(); + let d_tag = "barrier-race"; + let dir = tempfile::tempdir().expect("tempdir"); + let db_path = dir.path().join("retention.db"); + + // Retain a no-baseline pending row (the exact revert cell). + let event = EventBuilder::new( + Kind::Custom(KIND_PERSONA as u16), + r#"{"system_prompt":"old"}"#.to_string(), + ) + .tags(vec![Tag::parse(["d", d_tag]).unwrap()]) + .sign_with_keys(&keys) + .expect("sign"); + { + let conn = open_retention_db(&db_path).expect("open db"); + retain_event( + &conn, + &RetainedEvent { + kind: KIND_PERSONA, + pubkey: pubkey.clone(), + d_tag: d_tag.to_string(), + content: event.content.to_string(), + created_at: event.created_at.as_secs() as i64, + raw_event: event.as_json(), + event_id: None, + pending_sync: true, + publish_blocked: false, + }, + ) + .expect("retain"); + } + + let coordinate = Coordinate { + kind: KIND_PERSONA, + d_tag: d_tag.to_string(), + }; + + // Step 1: flush wins the initial claim (gen 0). Stale relay evidence: + // HeadState::Absent → no baseline → cell-2 PublishLocalEdit (would open the gate). + let stale_claim = config_sync_readiness::claim_in_progress().unwrap(); + assert_eq!(config_sync_readiness::current_generation(), 0); + let stale_observation = { + let conn = open_retention_db(&db_path).expect("open db for stale observation"); + local_state( + &conn, + &pubkey, + &coordinate, + None, + HeadState::Absent, + TombstoneEvidence::NotFound, + ) + .unwrap() + }; + let stale_states = vec![CoordinateState { + coordinate: coordinate.clone(), + observation: stale_observation, + }]; + + // Vacuity check: the stale evidence would decide PublishLocalEdit. + // If this fails the test is measuring the wrong thing. + assert_eq!( + plan(&stale_states)[0].1.decision, + Decision::PublishLocalEdit, + "vacuity: stale evidence (Absent head, no baseline) must decide PublishLocalEdit" + ); + + // Step 2: apply_workspace fires — force_claim bumps to gen 1. + let force_claim = config_sync_readiness::force_claim_in_progress(); + assert_eq!(config_sync_readiness::current_generation(), 1); + + // Step 3: current barrier (gen 1) runs its pass — Present head + no baseline + // → Park(NoBaselineWithHead) → publish_blocked = true. Certifies Ready. + { + let current_observation = { + let conn = open_retention_db(&db_path).expect("open db for current observation"); + local_state( + &conn, + &pubkey, + &coordinate, + None, + HeadState::Present(Head { + event_id: "id-relay-head".to_string(), + created_at: 200, + projection: CanonicalProjection { + content: "good-edit".to_string(), + shared: false, + }, + }), + TombstoneEvidence::NotFound, + ) + .unwrap() + }; + let current_conn = open_retention_db(&db_path).expect("open db for current pass"); + run_decision_pass( + ¤t_conn, + &pubkey, + &[CoordinateState { + coordinate: coordinate.clone(), + observation: current_observation, + }], + ) + .expect("current barrier enforces"); + } + assert!( + is_publish_blocked( + &open_retention_db(&db_path).unwrap(), + KIND_PERSONA, + &pubkey, + d_tag, + ) + .unwrap(), + "precondition: current barrier (gen 1) must have set publish_blocked = true" + ); + force_claim.complete(db_path.clone()); + assert!(config_sync_readiness::is_ready_for(&db_path)); + + // Step 4: stale barrier (gen 0) resumes enforcement. + // + // The enforcement seam: `run_boot_barrier_for_scope` calls `claim.is_current()` + // while holding `managed_agents_store_lock`, and returns `Abandoned` without + // calling `run_decision_pass` when it returns false. + // + // We reproduce that guard here: `is_current()` must return false. + assert!( + !stale_claim.is_current(), + "stale claim (gen 0) must not be current after force_claim bumped to gen 1" + ); + + // Enforcement seam: the stale barrier only calls run_decision_pass if + // is_current() returns true. Since it doesn't, the write is blocked. + // This is the invariant the production guard enforces. + if stale_claim.is_current() { + let conn = open_retention_db(&db_path).expect("open db for stale enforcement"); + run_decision_pass(&conn, &pubkey, &stale_states).expect("stale pass"); + } + + // Vacuity companion: confirm the stale pass WOULD have cleared the flag + // by running it on an isolated scratch db. This proves the test would + // catch a regression where is_current() is removed — the stale evidence + // genuinely opens the gate when unguarded. + { + let scratch_dir = tempfile::tempdir().expect("scratch tempdir"); + let scratch_db = scratch_dir.path().join("scratch.db"); + let scratch_conn = open_retention_db(&scratch_db).expect("open scratch"); + retain_event( + &scratch_conn, + &RetainedEvent { + kind: KIND_PERSONA, + pubkey: pubkey.clone(), + d_tag: d_tag.to_string(), + content: "old".to_string(), + created_at: 100, + raw_event: event.as_json(), + event_id: None, + pending_sync: true, + publish_blocked: true, + }, + ) + .expect("retain in scratch"); + run_decision_pass(&scratch_conn, &pubkey, &stale_states).expect("unguarded stale pass"); + assert!( + !is_publish_blocked(&scratch_conn, KIND_PERSONA, &pubkey, d_tag).unwrap(), + "vacuity: without the guard the stale pass clears publish_blocked" + ); + } + + // Step 5: real store must be unchanged — publish_blocked must remain true. + assert!( + is_publish_blocked( + &open_retention_db(&db_path).unwrap(), + KIND_PERSONA, + &pubkey, + d_tag, + ) + .unwrap(), + "publish_blocked must remain true: stale barrier's run_decision_pass was blocked by the generation guard" + ); + + // Stale claim drops (gen mismatch → latch no-op; force-claim already certified Ready). + drop(stale_claim); + assert!(config_sync_readiness::is_ready_for(&db_path)); + + config_sync_readiness::mark_unready(); // cleanup +} + // ── (i) leg-1 end-to-end: stale flush barrier cannot publish after force-claim ─ // // Paul's second bounce, leg 1: flush wins claim at t≈0, its inline barrier diff --git a/desktop/src-tauri/src/managed_agents/reconcile.rs b/desktop/src-tauri/src/managed_agents/reconcile.rs index b0ea4149e2..c888350445 100644 --- a/desktop/src-tauri/src/managed_agents/reconcile.rs +++ b/desktop/src-tauri/src/managed_agents/reconcile.rs @@ -35,20 +35,22 @@ pub(crate) fn reconcile_agents_to_events( app: &tauri::AppHandle, keys: &nostr::Keys, db_path: &Path, -) { +) -> Result<(), String> { let Ok(base_dir) = super::managed_agents_base_dir(app) else { - return; + return Ok(()); }; match reconcile_agents_in_dir_at(&base_dir, keys, db_path) { - Ok(0) => {} + Ok(0) => Ok(()), Ok(reconciled) => { eprintln!( "buzz-desktop: agent-event-reconcile: {reconciled} agents reconciled to retention" ); + Ok(()) } Err(e) => { eprintln!("buzz-desktop: agent-event-reconcile: {e}"); + Err(e) } } }