feat(managed-agents): add §2.5 recovery reap for unreferenced orphans

The mint orchestrator leaves a durable orphan row whenever a crash lands
before the atomic step-4 commit; nothing reclaimed the dangling keyring
secret. Add reap_unreferenced_orphans: for each orphan with no live
binding, delete its keyring entry THEN drop its journal row, persisting
the trimmed document once. Delete-before-drop and drop-only-on-success
make the sweep idempotent across recovery points, and a backend delete
failure keeps that row for a later retry.

store_all only merges, so a new KeyStore::delete seam (delegating to
SecretStore::delete, absent-entry = Ok) is required; the fakes mirror
that contract. Also folds in Paul's read-back-miss mint crash point:
load returning None after a verified write yields Err, no binding, and a
surviving orphan row for the reap.

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

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

@@ -818,8 +818,8 @@ impl LibraryDocument {
     /// once reaped, the caller drops the row via [`remove_orphan_pubkey`]. Scans
     /// RAW entries for a live binding to the pubkey (binding only — a
     /// `deferred_archives` marker is not a live binding and must not keep an
-    /// uncommitted orphan alive). Pure selection; the keyring delete is the IO
-    /// half performed by the recovery point (Phase 4b).
+    /// uncommitted orphan alive). Pure selection; [`reap_unreferenced_orphans`]
+    /// performs the keyring delete + journal drop at the recovery point.
     pub fn unreferenced_orphans(&self) -> Vec<&str> {
         self.orphan_keys
             .iter()
@@ -829,6 +829,59 @@ impl LibraryDocument {
     }
 }
 
+/// Reap every unreferenced orphan at a §4 recovery point (§2.5 step 3): for each
+/// [`LibraryDocument::unreferenced_orphans`] pubkey, delete its keyring entry
+/// THEN drop its journal row, and durably `persist` the trimmed document once.
+///
+/// Ordering is the crash-safety guarantee: the keyring `delete` precedes the
+/// journal-row drop, and the row is only dropped for a pubkey whose delete
+/// SUCCEEDED. A crash after a delete but before `persist` leaves the row intact,
+/// so the next recovery point re-deletes (a no-op on the already-absent entry —
+/// [`KeyStore::delete`] treats absent as success) and drops the row then; the
+/// reap is idempotent. A backend `delete` failure keeps that pubkey's row for a
+/// later retry and surfaces as `Err` AFTER the successful drops are persisted,
+/// so partial progress is never lost. Referenced (committed) orphans are never
+/// touched — their key backs a live binding.
+///
+/// `persist` and [`KeyStore`] are injected so the ordering is unit-testable at
+/// each crash point without real IO; production passes the live secret store and
+/// `|doc| save_library_document(base_dir, doc)`.
+pub(crate) fn reap_unreferenced_orphans(
+    document: &mut LibraryDocument,
+    store: &impl KeyStore,
+    persist: impl FnOnce(&LibraryDocument) -> Result<(), String>,
+) -> Result<(), String> {
+    let targets: Vec<String> = document
+        .unreferenced_orphans()
+        .into_iter()
+        .map(str::to_string)
+        .collect();
+
+    let mut failures = Vec::new();
+    for pubkey in &targets {
+        match store.delete(&agent_keyring_name(pubkey)) {
+            // Key gone (or already absent) — safe to drop the coordinate.
+            Ok(()) => document.remove_orphan_pubkey(pubkey),
+            // Backend failure: keep the row so a later recovery point retries.
+            Err(e) => failures.push(format!("{pubkey}: {e}")),
+        }
+    }
+
+    // Durably record the successful drops before reporting any failure, so a
+    // partial reap never repeats work it already completed.
+    persist(document)?;
+
+    if failures.is_empty() {
+        Ok(())
+    } else {
+        Err(format!(
+            "reap left {} orphan(s) for retry: {}",
+            failures.len(),
+            failures.join("; ")
+        ))
+    }
+}
+
 #[cfg(test)]
 mod tests;
 

desktop/src-tauri/src/managed_agents/library/binding_tests.rs

@@ -277,7 +277,13 @@ fn test_unreferenced_orphans_ignores_deferred_only_reference() {
 struct FakeKeyStore {
     stored: RefCell<HashMap<String, String>>,
     fail_write: bool,
+    fail_delete: bool,
+    /// `load` returns `Ok(None)` even after a verified write — models a keyring
+    /// where the minted key vanishes between write-back and read-back (§2.5
+    /// step 4's "minted key absent immediately after verified write" arm).
+    load_misses: bool,
     writes: RefCell<usize>,
+    deletes: RefCell<usize>,
 }
 
 impl FakeKeyStore {
@@ -285,16 +291,36 @@ impl FakeKeyStore {
         Self {
             stored: RefCell::new(HashMap::new()),
             fail_write: false,
+            fail_delete: false,
+            load_misses: false,
             writes: RefCell::new(0),
+            deletes: RefCell::new(0),
         }
     }
     fn write_fails() -> Self {
         Self {
-            stored: RefCell::new(HashMap::new()),
             fail_write: true,
-            writes: RefCell::new(0),
+            ..Self::ok()
         }
     }
+    fn delete_fails() -> Self {
+        Self {
+            fail_delete: true,
+            ..Self::ok()
+        }
+    }
+    fn load_misses() -> Self {
+        Self {
+            load_misses: true,
+            ..Self::ok()
+        }
+    }
+    fn with_key(self, name: &str, value: &str) -> Self {
+        self.stored
+            .borrow_mut()
+            .insert(name.to_string(), value.to_string());
+        self
+    }
 }
 
 impl crate::managed_agents::storage::KeyStore for FakeKeyStore {
@@ -302,6 +328,9 @@ impl crate::managed_agents::storage::KeyStore for FakeKeyStore {
         KeyringProbe::ReachableButEmpty
     }
     fn load(&self, name: &str) -> Result<Option<String>, String> {
+        if self.load_misses {
+            return Ok(None);
+        }
         Ok(self.stored.borrow().get(name).cloned())
     }
     fn load_all_readonly(&self) -> Result<Option<HashMap<String, String>>, String> {
@@ -324,6 +353,14 @@ impl crate::managed_agents::storage::KeyStore for FakeKeyStore {
             .extend(entries.iter().map(|(k, v)| (k.clone(), v.clone())));
         Ok(())
     }
+    fn delete(&self, name: &str) -> Result<(), String> {
+        *self.deletes.borrow_mut() += 1;
+        if self.fail_delete {
+            return Err("keyring backend unreachable".to_string());
+        }
+        self.stored.borrow_mut().remove(name);
+        Ok(())
+    }
 }
 
 #[test]
@@ -453,3 +490,153 @@ fn test_mint_crash_at_keyring_write_leaves_durable_orphan_for_reap() {
             .collect::<Vec<_>>()
     );
 }
+
+#[test]
+fn test_mint_crash_at_read_back_leaves_durable_orphan_no_binding() {
+    // Crash point in step 4: the keyring write verified, but the immediate
+    // read-back misses (key vanished between write and load). No binding is
+    // produced, the error surfaces, and the durable orphan row (step 2) survives
+    // so the dangling key is reaped at the next recovery point.
+    let owner = Keys::generate();
+    let store = FakeKeyStore::load_misses();
+    let mut doc = library(vec![]);
+    let mut persisted: Option<LibraryDocument> = None;
+
+    let err = mint_bound_identity(&mut doc, &owner, &store, |d| {
+        persisted = Some(d.clone());
+        Ok(())
+    })
+    .expect_err("read-back miss propagates");
+
+    assert!(err.contains("absent from keyring"));
+    assert_eq!(
+        *store.writes.borrow(),
+        1,
+        "keyring write verified before load"
+    );
+    // The durable snapshot carries the orphan coordinate, unreferenced — the reap
+    // selects it exactly as at the keyring-write crash point.
+    let durable = persisted.expect("journal was persisted before the keyring write");
+    assert_eq!(durable.orphan_keys.len(), 1);
+    assert_eq!(
+        durable.unreferenced_orphans(),
+        durable
+            .orphan_keys
+            .iter()
+            .map(String::as_str)
+            .collect::<Vec<_>>()
+    );
+}
+
+// ── reap_unreferenced_orphans: §4 recovery sweep (§2.5 step 3) ───────────────────
+
+#[test]
+fn test_reap_deletes_uncommitted_keys_and_drops_rows_leaving_committed() {
+    // Two orphans journaled: one committed (a live binding references it), one
+    // dangling. The sweep deletes only the dangling key and drops only its row;
+    // the committed key and row survive untouched.
+    let committed = "aa".repeat(32);
+    let dangling = "bb".repeat(32);
+    let store = FakeKeyStore::ok()
+        .with_key(&format!("agent:{committed}"), "nsec-committed")
+        .with_key(&format!("agent:{dangling}"), "nsec-dangling");
+    let mut doc = library(vec![bound_entry(&committed, false)]);
+    doc.journal_orphan_pubkey(&committed);
+    doc.journal_orphan_pubkey(&dangling);
+    let saves: RefCell<usize> = RefCell::new(0);
+
+    reap_unreferenced_orphans(&mut doc, &store, |_| {
+        *saves.borrow_mut() += 1;
+        Ok(())
+    })
+    .expect("reap");
+
+    assert_eq!(
+        *saves.borrow(),
+        1,
+        "one durable write for the trimmed journal"
+    );
+    assert_eq!(*store.deletes.borrow(), 1, "only the dangling key deleted");
+    // Committed row + key survive; dangling row + key gone.
+    assert_eq!(doc.orphan_keys, vec![committed.clone()]);
+    assert!(store
+        .stored
+        .borrow()
+        .contains_key(&format!("agent:{committed}")));
+    assert!(!store
+        .stored
+        .borrow()
+        .contains_key(&format!("agent:{dangling}")));
+}
+
+#[test]
+fn test_reap_is_idempotent_across_recovery_points() {
+    // A second sweep after the first has nothing to reap: no deletes, the
+    // journal is already clean. Proves re-running a recovery point is safe.
+    let dangling = "cc".repeat(32);
+    let store = FakeKeyStore::ok().with_key(&format!("agent:{dangling}"), "nsec");
+    let mut doc = library(vec![]);
+    doc.journal_orphan_pubkey(&dangling);
+
+    reap_unreferenced_orphans(&mut doc, &store, |_| Ok(())).expect("first reap");
+    assert!(doc.orphan_keys.is_empty());
+    assert_eq!(*store.deletes.borrow(), 1);
+
+    reap_unreferenced_orphans(&mut doc, &store, |_| Ok(())).expect("second reap");
+    assert_eq!(
+        *store.deletes.borrow(),
+        1,
+        "nothing left to delete on re-sweep"
+    );
+}
+
+#[test]
+fn test_reap_delete_failure_keeps_rows_persists_and_errors() {
+    // Every keyring delete backend-fails: no row is dropped, yet persist still
+    // runs once (recording zero progress durably), and the sweep returns Err
+    // naming the retained orphans so a later recovery point retries them.
+    let ok_pubkey = "dd".repeat(32);
+    let fail_pubkey = "ee".repeat(32);
+    let store = FakeKeyStore::delete_fails();
+    let mut doc = library(vec![]);
+    doc.journal_orphan_pubkey(&ok_pubkey);
+    doc.journal_orphan_pubkey(&fail_pubkey);
+    let mut persisted: Option<LibraryDocument> = None;
+
+    let err = reap_unreferenced_orphans(&mut doc, &store, |d| {
+        persisted = Some(d.clone());
+        Ok(())
+    })
+    .expect_err("backend delete failure surfaces");
+
+    assert!(err.contains("retry"));
+    assert_eq!(*store.deletes.borrow(), 2, "both deletes attempted");
+    // Persist ran once even though every delete failed — no row was dropped.
+    let durable = persisted.expect("persist ran");
+    assert_eq!(
+        durable.orphan_keys.len(),
+        2,
+        "failed deletes keep their rows"
+    );
+}
+
+#[test]
+fn test_reap_persist_failure_propagates() {
+    // A durable-write failure after the keyring deletes propagates — the caller
+    // must know the trimmed journal did not reach disk (the next recovery point
+    // re-deletes harmlessly and retries the drop).
+    let dangling = "ff".repeat(32);
+    let store = FakeKeyStore::ok().with_key(&format!("agent:{dangling}"), "nsec");
+    let mut doc = library(vec![]);
+    doc.journal_orphan_pubkey(&dangling);
+
+    let err = reap_unreferenced_orphans(&mut doc, &store, |_| Err("disk full".to_string()))
+        .expect_err("persist failure propagates");
+
+    assert!(err.contains("disk full"));
+    assert_eq!(
+        *store.deletes.borrow(),
+        1,
+        "delete ran before the failed persist"
+    );
+}

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

@@ -164,6 +164,11 @@ pub(crate) trait KeyStore {
     fn write_and_verify(&self, name: &str, value: &str) -> Result<(), String>;
     /// Insert all entries from `entries` in a single blob mutation.
     fn store_all(&self, entries: &HashMap<String, String>) -> Result<(), String>;
+    /// Delete the entry for `name`. Deleting an absent entry is `Ok(())` (not a
+    /// backend failure) — so the §2.5 recovery reap is idempotent across repeated
+    /// recovery points until the journal row is also dropped. `Err` only on a
+    /// backend failure that leaves the entry possibly still present.
+    fn delete(&self, name: &str) -> Result<(), String>;
 }
 
 impl KeyStore for SecretStore {
@@ -186,6 +191,9 @@ impl KeyStore for SecretStore {
     fn store_all(&self, entries: &HashMap<String, String>) -> Result<(), String> {
         SecretStore::store_all(self, entries)
     }
+    fn delete(&self, name: &str) -> Result<(), String> {
+        SecretStore::delete(self, name)
+    }
 }
 
 /// Outcome of attempting to lift a record's inline key into the keyring.

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

@@ -118,6 +118,14 @@ impl KeyStore for FakeKeyStore {
         }
         Ok(())
     }
+    fn delete(&self, name: &str) -> Result<(), String> {
+        if !self.reachable {
+            return Err("keyring backend unreachable".to_string());
+        }
+        // Deleting an absent entry is a no-op success, matching SecretStore.
+        self.stored.borrow_mut().remove(name);
+        Ok(())
+    }
 }
 
 fn record_with_key(nsec: &str) -> ManagedAgentRecord {