fix(agent-config): make regression tests mutation-sensitive via production seams

IMPORTANT#1: extract force_claim_and_resolve() helper in config_sync_readiness.rs
- Both apply_workspace and import_identity route through force_claim_and_resolve()
  rather than calling force_claim_in_progress() + active_retention_scope() inline.
- On resolver failure the helper drops the claim (RAII -> Unready); no stale
  Ready(old_path) survives.
- Test drives force_claim_and_resolve() with a failing resolver; reordering the
  helper (resolve before claim) leaves Ready(A) intact and fails the assertion.

IMPORTANT#2: factor run_event_sync through run_event_sync_impl with injectable resolver
- run_event_sync_impl<F: FnOnce() -> Result<PathBuf, String>>(...) is the shared
  non-test core; the single base_dir_fn()? is the mutation-sensitive propagation
  point that replaced the three per-wrapper let-Ok-else-return-Ok swallows.
- run_event_sync() passes managed_agents_base_dir(app) as the resolver.
- reconcile_agents_to_events() removed (dead; reconcile_agents_in_dir_at promoted
  to pub(crate) for direct use by the impl core).
- Test injects Err via run_event_sync_impl(|| Err(...)); swapping ? to swallow
  returns Ok(()) and fails result.is_err().

MINOR: rewrite enforce_decision_pass() function-level contract doc
- Replaces false store-lock-prevents-generation-change claim with the real
  invariant: store lock serializes observation/writes; post-check force-claim sets
  InProgress on the new generation and its own barrier supplies authoritative writes.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

desktop/src-tauri/src/commands/identity.rs

@@ -394,14 +394,15 @@ pub async fn import_identity(
         // change on key swap — `scoped_retention_db_path` hashes the owner
         // pubkey — so the reconcile below is necessary to queue the new
         // owner's disk projections for the new scope.
+        //
+        // Both swap paths route through `force_claim_and_resolve` so the
+        // claim-before-resolve ordering is a named production seam rather than
+        // an inline convention at each call site.
         {
-            let claim =
-                crate::managed_agents::config_sync_readiness::force_claim_in_progress();
-            match crate::managed_agents::retention::active_retention_scope(
-                &app_handle,
-                &state,
-            ) {
-                Ok(scope) => {
+            match crate::managed_agents::config_sync_readiness::force_claim_and_resolve(|| {
+                crate::managed_agents::retention::active_retention_scope(&app_handle, &state)
+            }) {
+                Ok((claim, scope)) => {
                     crate::event_sync::spawn_event_sync_with_held_claim(
                         app_handle.clone(),
                         scope.owner_keys,
@@ -411,10 +412,9 @@ pub async fn import_identity(
                 }
                 Err(e) => {
                     eprintln!("buzz-desktop: import-identity: scope unavailable, config barrier skipped: {e}");
-                    // Claim drops here via RAII → latch becomes Unready; the
-                    // flush loop will retry with the full transition once scope
-                    // resolves.
-                    drop(claim);
+                    // force_claim_and_resolve already dropped the claim →
+                    // latch is Unready; the flush loop will retry with the
+                    // full transition once scope resolves.
                 }
             }
         }

desktop/src-tauri/src/commands/workspace.rs

@@ -221,9 +221,14 @@ pub async fn apply_workspace(
     // `flush_active_pending_events` to refuse every retry claim for the session.
     // Dropping the claim on failure leaves the latch Unready so the flush loop
     // can claim and run the full transition once scope resolution recovers.
-    let claim = crate::managed_agents::config_sync_readiness::force_claim_in_progress();
-    match crate::managed_agents::retention::active_retention_scope(&restore_app, &state) {
-        Ok(scope) => {
+    //
+    // Both swap paths (apply_workspace and import_identity) route through
+    // `force_claim_and_resolve` so the claim-before-resolve ordering is a named
+    // production seam rather than an inline convention at each call site.
+    match crate::managed_agents::config_sync_readiness::force_claim_and_resolve(|| {
+        crate::managed_agents::retention::active_retention_scope(&restore_app, &state)
+    }) {
+        Ok((claim, scope)) => {
             // Adopt whatever the pre-scoping release left queued in the global
             // retention database BEFORE the scoped reconcile and flush run, so
             // stranded tombstones and archive requests publish on this boot
@@ -238,9 +243,9 @@ pub async fn apply_workspace(
         }
         Err(error) => {
             eprintln!("buzz-desktop: scoped event-sync unavailable after workspace apply: {error}");
-            // Claim drops here via RAII → latch becomes Unready; the flush
-            // loop will retry with the full transition once scope resolves.
-            drop(claim);
+            // force_claim_and_resolve already dropped the claim → latch is
+            // Unready; the flush loop will retry with the full transition once
+            // scope resolves.
         }
     }
 

desktop/src-tauri/src/event_sync.rs

@@ -23,9 +23,35 @@ pub fn run_event_sync(
     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)?;
+    run_event_sync_impl(
+        || crate::managed_agents::managed_agents_base_dir(app),
+        owner_keys,
+        db_path,
+    )
+}
+
+/// Shared implementation entered through an injectable base-directory resolver.
+///
+/// Both the production entry ([`run_event_sync`]) and the test seam route
+/// through this function. The resolver closure is called ONCE; on `Err` the
+/// error propagates without running any reconcile leg — this is the single
+/// mutation point that protects against a swallowed base-directory failure.
+///
+/// Mutation-sensitive: replacing the `?` on the resolver call with a
+/// `let Ok(base_dir) = ... else { return Ok(()) }` swallow leaves any
+/// test that injects a failing resolver incorrectly green.
+pub(crate) fn run_event_sync_impl<F>(
+    base_dir_fn: F,
+    owner_keys: &nostr::Keys,
+    db_path: &Path,
+) -> Result<(), String>
+where
+    F: FnOnce() -> Result<std::path::PathBuf, String>,
+{
+    let base_dir = base_dir_fn()?;
+    migrate_personas_to_events_in_dir(&base_dir, owner_keys, db_path)?;
+    migrate_teams_to_events_in_dir(&base_dir, owner_keys, db_path)?;
+    crate::managed_agents::reconcile::reconcile_agents_in_dir_at(&base_dir, owner_keys, db_path)?;
     Ok(())
 }
 
@@ -112,16 +138,12 @@ 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,
+fn migrate_personas_to_events_in_dir(
+    base_dir: &Path,
     keys: &nostr::Keys,
     db_path: &Path,
 ) -> Result<(), String> {
-    use crate::managed_agents::managed_agents_base_dir;
-
-    let base_dir = managed_agents_base_dir(app)?;
-
-    match migrate_personas_in_dir_at(&base_dir, keys, db_path) {
+    match migrate_personas_in_dir_at(base_dir, keys, db_path) {
         Ok(0) => Ok(()),
         Ok(migrated) => {
             eprintln!(
@@ -256,7 +278,7 @@ fn migrate_personas_in_dir_at(
 
 /// Reconcile `teams.json` into kind:30176 team events in the retention store.
 ///
-/// Mirrors [`migrate_personas_to_events`] for teams: it picks up team metadata
+/// Mirrors [`migrate_personas_to_events_in_dir`] for teams: it picks up team metadata
 /// edits (name/description/persona_ids) made on disk between launches and
 /// queues them for relay publish. Managed agents (kind:30177) are deliberately
 /// NOT reconciled here — they have no pack/dir source and are backfilled from
@@ -264,16 +286,12 @@ 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,
+fn migrate_teams_to_events_in_dir(
+    base_dir: &Path,
     keys: &nostr::Keys,
     db_path: &Path,
 ) -> Result<(), String> {
-    use crate::managed_agents::managed_agents_base_dir;
-
-    let base_dir = managed_agents_base_dir(app)?;
-
-    match migrate_teams_in_dir_at(&base_dir, keys, db_path) {
+    match migrate_teams_in_dir_at(base_dir, keys, db_path) {
         Ok(0) => Ok(()),
         Ok(migrated) => {
             eprintln!("buzz-desktop: team-event-migration: {migrated} teams migrated to retention");
@@ -372,27 +390,6 @@ fn migrate_teams_in_dir_at(
     Ok(migrated)
 }
 
-/// Test seam: `run_event_sync` with an explicit `base_dir` instead of an
-/// `AppHandle`. Lets tests drive all three reconcile legs and the propagation
-/// of their `Result`s without a running Tauri application.
-///
-/// Used by `test_base_dir_failure_propagates_through_run_event_sync` to prove
-/// that a failing leg returns `Err`, the claim does NOT certify `Ready`, and
-/// a subsequent full retry enqueues the row.
-#[cfg(test)]
-pub(crate) fn run_event_sync_in_dir(
-    base_dir: &Path,
-    owner_keys: &nostr::Keys,
-    db_path: &Path,
-) -> Result<(), String> {
-    migrate_personas_in_dir_at(base_dir, owner_keys, db_path)?;
-    migrate_teams_in_dir_at(base_dir, owner_keys, db_path)?;
-    crate::managed_agents::reconcile::reconcile_agents_in_dir_at_test(
-        base_dir, owner_keys, db_path,
-    )?;
-    Ok(())
-}
-
 #[cfg(test)]
 #[path = "event_sync_tests.rs"]
 mod tests;

desktop/src-tauri/src/event_sync_tests.rs

@@ -384,23 +384,32 @@ fn test_reconcile_failure_retry_with_full_transition_certifies_and_enqueues() {
 
 // ── Base-dir failure propagates through run_event_sync ────────────────────────
 //
-// Thufir IMPORTANT#2 coverage: all three reconcile wrappers formerly returned
-// `Ok(())` when `managed_agents_base_dir` failed (via `let Ok(...) else { return Ok(()) }`).
-// The fix replaces that with `let base_dir = ...?`, making the failure propagate.
+// Thufir IMPORTANT#2 coverage: `run_event_sync` routes through the shared
+// `run_event_sync_impl` core, which propagates the base-dir resolver's `Err`
+// before any reconcile leg runs. The test injects a failing resolver directly
+// into `run_event_sync_impl` — the same function the production
+// `run_event_sync` delegates to — proving that a base-directory failure
+// returns `Err` and prevents certification.
 //
-// This test drives `run_event_sync_in_dir` (the AppHandle-free seam) with a
-// `db_path` pointing inside a non-existent directory so SQLite cannot open it,
-// forcing the first leg to return `Err`. Proves:
-//   1. `run_event_sync_in_dir` returns `Err` — a real reconcile failure, not a
-//      manual `drop(claim)`.
-//   2. The claim does NOT call `complete` when the leg fails — scope stays
-//      `Unready` (claim drops via RAII when the caller observes `Err`).
-//   3. A subsequent full retry with a valid `db_path` enqueues the row.
+// Mutation-sensitive: if `run_event_sync_impl`'s `base_dir_fn()?` is replaced
+// with `let Ok(base_dir) = base_dir_fn() else { return Ok(()) }`, the
+// injected `Err` is swallowed, `result.is_err()` below fails, and the test
+// catches the regression.
+//
+// Proves:
+//   1. `run_event_sync_impl` with a failing resolver returns `Err` — a real
+//      propagated failure, not a manual `drop(claim)`.
+//   2. The claim does NOT call `complete` — scope stays `Unready` via RAII.
+//   3. A subsequent full retry with a valid resolver enqueues the row and
+//      certifies `Ready`.
 #[test]
 fn test_base_dir_failure_propagates_through_run_event_sync() {
-    use crate::managed_agents::{
-        config_sync_readiness::{self, ReadinessState},
-        retention::{get_retained_event, open_retention_db},
+    use crate::{
+        event_sync::run_event_sync_impl,
+        managed_agents::{
+            config_sync_readiness::{self, ReadinessState},
+            retention::{get_retained_event, open_retention_db},
+        },
     };
     use buzz_core_pkg::kind::KIND_PERSONA;
 
@@ -411,48 +420,50 @@ fn test_base_dir_failure_propagates_through_run_event_sync() {
     let base = tempfile::tempdir().unwrap();
     write_base_personas(base.path(), &one_persona());
 
-    // A db_path inside a non-existent directory — SQLite cannot open it.
-    let bad_db_path = base.path().join("nonexistent_subdir").join("retention.db");
-    // A valid db_path for the successful retry.
-    let good_db_path = base.path().join("retention.db");
+    let db_path = base.path().join("retention.db");
 
-    // Step 1: claim InProgress, run reconcile with bad db_path — leg fails.
+    // Step 1: claim InProgress, run reconcile with a failing base-dir resolver.
+    // The failing resolver simulates `managed_agents_base_dir(app)` returning Err.
     // The claim must NOT be completed (scope stays Unready via RAII drop).
     {
         let claim = config_sync_readiness::claim_in_progress().expect("must claim from Unready");
 
-        let result = run_event_sync_in_dir(base.path(), &keys, &bad_db_path);
+        let result = run_event_sync_impl(
+            || Err::<std::path::PathBuf, String>("test-injected base-dir failure".to_string()),
+            &keys,
+            &db_path,
+        );
         assert!(
             result.is_err(),
-            "run_event_sync_in_dir must return Err when a reconcile leg fails"
+            "run_event_sync_impl with a failing base-dir resolver must return Err"
         );
 
         // Observe the error: claim drops here without complete() → Unready.
-        let _ = result;
         drop(claim);
     }
     assert_eq!(
         config_sync_readiness::readiness_state(),
         Some(ReadinessState::Unready),
-        "a failed reconcile leg must leave scope Unready (not Ready)"
+        "a base-dir failure must leave scope Unready (not Ready)"
     );
 
-    // Step 2: retry — full transition with valid db_path. The row must be queued.
+    // Step 2: retry — full transition with a valid resolver. The row must be queued.
     {
         let claim = config_sync_readiness::claim_in_progress()
             .expect("Unready scope must allow retry claim");
 
-        run_event_sync_in_dir(base.path(), &keys, &good_db_path)
-            .expect("full retry with valid db_path must succeed");
+        let base_path = base.path().to_path_buf();
+        run_event_sync_impl(|| Ok(base_path), &keys, &db_path)
+            .expect("full retry with valid base-dir resolver must succeed");
 
-        claim.complete(good_db_path.clone());
+        claim.complete(db_path.clone());
     }
     assert!(
-        config_sync_readiness::is_ready_for(&good_db_path),
+        config_sync_readiness::is_ready_for(&db_path),
         "full retry must certify Ready"
     );
 
-    let conn = open_retention_db(&good_db_path).expect("open db");
+    let conn = open_retention_db(&db_path).expect("open db");
     let row = get_retained_event(&conn, KIND_PERSONA, &pubkey, "code-reviewer")
         .expect("db query")
         .expect("row must exist after successful retry");

desktop/src-tauri/src/managed_agents/config_barrier.rs

@@ -271,8 +271,14 @@ async fn run_boot_barrier_for_scope(
 /// the guard can only mirror the `if stale_claim.is_current()` conditional
 /// manually — which stays green even when the production guard is removed.
 ///
-/// Caller must hold `managed_agents_store_lock` around this call so the
-/// generation cannot change between the currency check and the write.
+/// Caller holds `managed_agents_store_lock`, which serializes store
+/// observation and writes within this call. It does NOT prevent the readiness
+/// generation from changing: `force_claim_in_progress` takes only the
+/// separate readiness mutex, so a concurrent force-claim can advance the
+/// generation at any point. Post-check preemption is safe because the new
+/// generation is set to `InProgress` and its own barrier — or a full flush
+/// retry — supplies the authoritative gate writes; this barrier returns
+/// `Abandoned` without writing.
 pub(crate) fn enforce_decision_pass(
     claim: &super::config_sync_readiness::ReadinessClaim,
     conn: &rusqlite::Connection,

desktop/src-tauri/src/managed_agents/config_sync_readiness.rs

@@ -202,6 +202,33 @@ pub fn force_claim_in_progress() -> ReadinessClaim {
     ReadinessClaim::new(gen)
 }
 
+/// Force-claim then resolve: acquires a preempting [`ReadinessClaim`] BEFORE
+/// invoking `resolver`. If `resolver` succeeds, returns `Ok((claim, value))`.
+/// If `resolver` fails, the claim is dropped (RAII → `Unready`) and the error
+/// is returned.
+///
+/// Both `apply_workspace` and `import_identity` route through this helper to
+/// guarantee that a transient scope-resolution failure always leaves the latch
+/// `Unready` (retryable by the flush loop) rather than preserving a stale
+/// `Ready(old_path)` that would reject every flush retry claim for the session.
+///
+/// The production callers supply `active_retention_scope` as `resolver`; tests
+/// can supply `|| Err(...)` to prove that a resolver failure drops the claim
+/// and leaves the latch `Unready`.
+pub fn force_claim_and_resolve<T, E, F>(resolver: F) -> Result<(ReadinessClaim, T), E>
+where
+    F: FnOnce() -> Result<T, E>,
+{
+    let claim = force_claim_in_progress();
+    match resolver() {
+        Ok(value) => Ok((claim, value)),
+        Err(e) => {
+            drop(claim); // RAII → Unready
+            Err(e)
+        }
+    }
+}
+
 /// Reset to `Unready` unconditionally.
 ///
 /// Use for test cleanup and exceptional reset paths only. Normal transitions
@@ -465,15 +492,18 @@ mod tests {
         assert_eq!(readiness_state(), Some(ReadinessState::Unready));
     }
 
-    // ── (i) force-claim before scope resolution: scope failure → Unready ─────
+    // ── (i) force_claim_and_resolve: resolver failure leaves latch Unready ───
     //
-    // Covers the Thufir IMPORTANT#1 fix: `force_claim_in_progress` runs BEFORE
-    // `active_retention_scope`. If scope resolution fails, the claim drops via
-    // RAII and the latch lands `Unready`. Without this fix, the stale
-    // `Ready(old_path)` would survive and `claim_in_progress` would reject every
-    // flush retry for the session (it only accepts from `Unready`).
+    // Covers the Thufir IMPORTANT#1 fix: both swap paths (`apply_workspace`
+    // and `import_identity`) route through `force_claim_and_resolve`, which
+    // acquires `force_claim_in_progress` BEFORE invoking the resolver. If the
+    // resolver fails, the claim drops via RAII and the latch lands `Unready`.
     //
-    // Scenario: scope A Ready → swap → scope resolution fails →
+    // Mutation-sensitive: if `force_claim_and_resolve` is rewritten to invoke
+    // the resolver BEFORE acquiring the claim, the stale `Ready(A)` survives
+    // (nothing preempts it) and the assert below fails.
+    //
+    // Scenario: scope A Ready → force_claim_and_resolve with failing resolver →
     //   Unready (via RAII drop) → retry from flush loop wins claim →
     //   full transition runs for scope B → Ready(B).
     #[test]
@@ -488,20 +518,18 @@ mod tests {
         claim_a.complete(path_a.clone());
         assert!(is_ready_for(&path_a), "scope A must be Ready before swap");
 
-        // Force-claim BEFORE scope resolution (the fix). Simulated scope
-        // resolution failure: drop the claim without calling complete().
-        {
-            let _claim = force_claim_in_progress();
-            // Scope resolution fails here — _claim drops via RAII.
-        }
+        // Both swap commands call force_claim_and_resolve; supply a failing
+        // resolver to simulate a transient scope-resolution error.
+        let result = force_claim_and_resolve::<(), &str, _>(|| Err("transient scope failure"));
+        assert!(result.is_err(), "failing resolver must return Err");
         assert_eq!(
             readiness_state(),
             Some(ReadinessState::Unready),
-            "scope failure after force_claim must leave latch Unready, not Ready(old_path)"
+            "resolver failure must leave latch Unready, not Ready(old_path)"
         );
         assert!(
             !is_ready_for(&path_a),
-            "stale Ready(A) must not survive after force-claim + drop"
+            "stale Ready(A) must not survive after force_claim_and_resolve + resolver failure"
         );
 
         // The flush loop's Unready arm can now claim and run the full

desktop/src-tauri/src/managed_agents/reconcile.rs

@@ -28,31 +28,6 @@ use super::{
 };
 use buzz_core_pkg::kind::KIND_MANAGED_AGENT;
 
-/// Reconcile `managed-agents.json` into kind:30177 events in the retention
-/// store. Boot-time entry point, called from `event_sync::run_event_sync`
-/// after the persona and team legs.
-pub(crate) fn reconcile_agents_to_events(
-    app: &tauri::AppHandle,
-    keys: &nostr::Keys,
-    db_path: &Path,
-) -> Result<(), String> {
-    let base_dir = super::managed_agents_base_dir(app)?;
-
-    match reconcile_agents_in_dir_at(&base_dir, keys, db_path) {
-        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)
-        }
-    }
-}
-
 /// Core reconcile logic, decoupled from the Tauri `AppHandle` for testing.
 ///
 /// Reads `managed-agents.json` raw — no keyring hydration: the published
@@ -64,23 +39,9 @@ pub(crate) fn reconcile_agents_to_events(
 /// never churns `pending_sync`.
 ///
 /// Returns the number of agents (re)written to the retention store.
-#[cfg(test)]
-pub(crate) fn reconcile_agents_in_dir(base_dir: &Path, keys: &nostr::Keys) -> Result<u32, String> {
-    reconcile_agents_in_dir_at(base_dir, keys, &base_dir.join("retention.db"))
-}
-
-/// Test seam with an explicit `db_path`. Used by `event_sync::run_event_sync_in_dir`
-/// so tests can pass a controlled (including failing) db_path to all three reconcile legs.
-#[cfg(test)]
-pub(crate) fn reconcile_agents_in_dir_at_test(
-    base_dir: &Path,
-    keys: &nostr::Keys,
-    db_path: &Path,
-) -> Result<u32, String> {
-    reconcile_agents_in_dir_at(base_dir, keys, db_path)
-}
-
-fn reconcile_agents_in_dir_at(
+/// Called from [`crate::event_sync::run_event_sync_impl`] (production) and
+/// from tests via [`reconcile_agents_in_dir`].
+pub(crate) fn reconcile_agents_in_dir_at(
     base_dir: &Path,
     keys: &nostr::Keys,
     db_path: &Path,
@@ -172,5 +133,12 @@ pub(crate) fn retain_agent_record(
     Ok(true)
 }
 
+/// Convenience test wrapper: `reconcile_agents_in_dir_at` with the default
+/// retention db path (`base_dir/retention.db`).
+#[cfg(test)]
+pub(crate) fn reconcile_agents_in_dir(base_dir: &Path, keys: &nostr::Keys) -> Result<u32, String> {
+    reconcile_agents_in_dir_at(base_dir, keys, &base_dir.join("retention.db"))
+}
+
 #[cfg(test)]
 mod tests;