review: an undo concludes again only what it had deleted

Undoing a consolidate concluded its moved entries a second time, and a later
removal deleted only the newer conclusion. An undo's added entry is now
concluded only when the ledger decided a Conclude for it and then an
Unconclude: an entry still concluded, or never concluded (a secret session's,
one from before sync), gets nothing. Ledger::concluded and unconcluded answer
from the decisions, in a module of their own to keep ledger.rs under 500 lines.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

crates/memoryd/src/honcho/concluded.rs

@@ -0,0 +1,40 @@
+//! What the ledger has decided about one conclusion (M5c spec, section 4): whether it is made, or
+//! was made and then deleted, by the `Conclude` and `Unconclude` decisions alone, sent or not.
+//! The owner's undo asks this, so that it concludes again only what it had deleted.
+
+use crate::honcho::ledger::{Action, Ledger, Workspace};
+
+impl Ledger {
+    /// Whether the latest decided `Conclude` or `Unconclude` of `text` about `peer` in
+    /// `workspace`, in the ledger's order, is a `Conclude`: its conclusion is made, or will be.
+    pub fn concluded(&self, workspace: Workspace, peer: &str, text: &str) -> bool {
+        self.last_of(workspace, peer, text) == Some(true)
+    }
+
+    /// Whether a `Conclude` of the same was decided, and the latest was followed by an
+    /// `Unconclude`: its conclusion was made, then deleted.
+    pub fn unconcluded(&self, workspace: Workspace, peer: &str, text: &str) -> bool {
+        self.last_of(workspace, peer, text) == Some(false)
+    }
+
+    /// The latest of `Conclude` (true) and, after a `Conclude`, `Unconclude` (false) for the key;
+    /// `None` when no `Conclude` was ever decided.
+    fn last_of(&self, workspace: Workspace, peer: &str, text: &str) -> Option<bool> {
+        let key = |w: &Workspace, p: &str, t: &str| *w == workspace && p == peer && t == text;
+        self.decisions()
+            .fold(None, |last, (_, action)| match action {
+                Action::Conclude {
+                    workspace: w,
+                    peer: p,
+                    text: t,
+                    ..
+                } if key(w, p, t) => Some(true),
+                Action::Unconclude {
+                    workspace: w,
+                    peer: p,
+                    text: t,
+                } if key(w, p, t) => last.map(|_| false),
+                _ => last,
+            })
+    }
+}

crates/memoryd/src/honcho/mirror.rs

@@ -185,6 +185,22 @@ pub fn actions(
     kept
 }
 
+/// Whether an undo's `action` is kept: an entry it brings back is concluded again only when its
+/// conclusion was made and then deleted (M5c spec, section 4: "any it had deleted, written
+/// again"). One never concluded (a secret session's, from before sync, skipped) or still
+/// concluded (moved out by `consolidate`) gets nothing. Every other action is kept.
+fn brought_back(ledger: &Ledger, action: &Action) -> bool {
+    match action {
+        Action::Conclude {
+            workspace,
+            peer,
+            text,
+            ..
+        } => ledger.unconcluded(*workspace, peer, text),
+        _ => true,
+    }
+}
+
 /// With `[honcho]`: decides each journalled edit into the ledger.
 pub struct Mirror {
     ledger: Arc<Mutex<Ledger>>,
@@ -207,8 +223,9 @@ impl Mirror {
     }
 
     /// Decide the journalled edit `id` (the latest when `None`), called under the index lock
-    /// right after it was journalled, so the ledger has the journal's order. Its parts are
-    /// `Edit { id, part }` from 0. An edit already decided, or asking nothing, adds no line. A
+    /// right after it was journalled, so the ledger has the journal's order; the ledger's lock is
+    /// taken inside the index lock, and before an undo is decided against it. Its parts are
+    /// `Edit { id, part }` from 0, numbering what is decided. An edit already decided, or asking nothing, adds no line. A
     /// failure is logged; the edit itself stands.
     pub fn journalled(&self, store: &Store, id: Option<&str>, now: Timestamp, log: &dyn Fn(&str)) {
         let records = match journal::records(store) {
@@ -229,8 +246,11 @@ impl Mirror {
             return;
         };
         let session = session_of(record, &records);
-        let actions = actions(record, session.as_ref(), &self.agent, &self.owner);
         let mut book = self.ledger.lock().unwrap_or_else(|p| p.into_inner());
+        let mut actions = actions(record, session.as_ref(), &self.agent, &self.owner);
+        if record.undoes.is_some() {
+            actions.retain(|a| brought_back(&book, a));
+        }
         for (part, action) in actions.into_iter().enumerate() {
             let Ok(part) = u32::try_from(part) else {
                 return;

crates/memoryd/src/honcho/mod.rs

@@ -1,6 +1,7 @@
 //! Honcho, the memory this agent shares with the fleet, reached through `roncho` (M5c spec).
 
 pub mod client;
+pub mod concluded;
 pub mod config;
 pub mod decide;
 pub mod edits;

crates/memoryd/tests/honcho_mirror_undo.rs

@@ -0,0 +1,219 @@
+//! The owner's undo, mirrored by what the ledger has decided (M5c spec, section 4; review of
+//! M5c-1, finding 2): an entry an undo brings back is concluded again only when its conclusion
+//! was made and then deleted; one never concluded (a secret session's) or still concluded (moved
+//! out by `consolidate`) gets nothing.
+
+#[path = "support/tmp.rs"]
+mod tmp;
+
+use std::sync::{Arc, Mutex};
+
+use bxcore::{DataClass, MemoryArgs, Timestamp};
+use memoryd::edit::{Context, apply};
+use memoryd::files::Store;
+use memoryd::honcho::ledger::{Action, Item, Ledger, Record, Workspace};
+use memoryd::honcho::mirror::Mirror;
+use memoryd::journal;
+use tmp::TempDir;
+
+const AGENT: &str = "boxmaker0";
+
+fn now() -> Timestamp {
+    Timestamp::parse("2026-09-28T10:00:00.000Z").unwrap()
+}
+
+struct Fixture {
+    store: Store,
+    ledger: Arc<Mutex<Ledger>>,
+    mirror: Mirror,
+    sent: usize,
+    _dir: TempDir,
+}
+
+impl Fixture {
+    fn new(tag: &str) -> Fixture {
+        let dir = TempDir::new(tag);
+        let starter = dir.write("etc/soul.md", "Soul.\n");
+        let store = Store::new(&dir.path().join("memory"));
+        store.init(&starter).unwrap();
+        let mut ledger = Ledger::open(&dir.path().join("memory/honcho.jsonl")).unwrap();
+        ledger.append(Record::Began { time: now() }).unwrap();
+        let ledger = Arc::new(Mutex::new(ledger));
+        let mirror = Mirror::new(Arc::clone(&ledger), AGENT, "kyle");
+        Fixture {
+            store,
+            ledger,
+            mirror,
+            sent: 0,
+            _dir: dir,
+        }
+    }
+
+    /// A memory call from `session` at `taint`, applied, journalled and mirrored; its edit id.
+    fn write(&self, session: &str, taint: DataClass, json: &str) -> String {
+        let args: MemoryArgs = serde_json::from_str(json).unwrap();
+        let ctx = Context {
+            session: session.to_string(),
+            call: 1,
+            taint,
+            untrusted: false,
+            now: now(),
+        };
+        let outcome = apply(&self.store, &args, &ctx).unwrap();
+        let id = journal::record(&self.store, &ctx, &outcome).unwrap();
+        self.mirror
+            .journalled(&self.store, Some(&id), now(), &|_| {});
+        id
+    }
+
+    fn undo(&self, id: &str) {
+        journal::undo(&self.store, id, now()).unwrap();
+        self.mirror.journalled(&self.store, None, now(), &|_| {});
+    }
+
+    /// The worker's part, done by hand: every waiting item marked sent, a conclusion with an id
+    /// of its own.
+    fn send_all(&mut self) {
+        let mut ledger = self.ledger.lock().unwrap();
+        for (item, _) in ledger.outbox() {
+            self.sent += 1;
+            let ids = vec![format!("c{}", self.sent)];
+            ledger
+                .append(Record::Sent {
+                    time: now(),
+                    item,
+                    ids,
+                })
+                .unwrap();
+        }
+    }
+
+    /// The edit parts decided, in order, with their actions.
+    fn decisions(&self) -> Vec<(Item, Action)> {
+        self.ledger
+            .lock()
+            .unwrap()
+            .decisions()
+            .map(|(i, a)| (i.clone(), a.clone()))
+            .collect()
+    }
+
+    fn concludes_of(&self, text: &str) -> usize {
+        self.decisions()
+            .iter()
+            .filter(|(_, a)| matches!(a, Action::Conclude { text: t, .. } if t == text))
+            .count()
+    }
+
+    fn actions_of(&self, id: &str) -> Vec<Action> {
+        self.decisions()
+            .into_iter()
+            .filter(|(i, _)| matches!(i, Item::Edit { id: e, .. } if e == id))
+            .map(|(_, a)| a)
+            .collect()
+    }
+}
+
+fn add(text: &str) -> String {
+    format!(r#"{{"action":"add","file":"memory","text":"{text}"}}"#)
+}
+
+fn remove(text: &str) -> String {
+    format!(r#"{{"action":"remove","file":"memory","old":"{text}"}}"#)
+}
+
+fn last_edit(store: &Store) -> String {
+    journal::records(store).unwrap().pop().unwrap().id
+}
+
+#[test]
+fn undoing_a_consolidate_does_not_conclude_a_moved_entry_twice() {
+    let mut f = Fixture::new("undo-consolidate");
+    f.write("mm-a", DataClass::Private, &add("Alpha."));
+    f.write("mm-a", DataClass::Private, &add("Bravo."));
+    f.send_all();
+    let consolidate = f.write(
+        "mm-a",
+        DataClass::Private,
+        r#"{"action":"consolidate","file":"memory","entries":[{"old":"Alpha.","keep":true},{"old":"Bravo.","move":true}]}"#,
+    );
+    assert!(f.actions_of(&consolidate).is_empty(), "a move asks nothing");
+    f.undo(&consolidate);
+    assert!(
+        f.actions_of(&last_edit(&f.store)).is_empty(),
+        "Bravo.'s conclusion is still there"
+    );
+    {
+        let ledger = f.ledger.lock().unwrap();
+        assert!(ledger.concluded(Workspace::Trusted, AGENT, "Bravo."));
+    }
+    f.write("mm-a", DataClass::Private, &remove("Bravo."));
+    f.send_all();
+    assert_eq!(f.concludes_of("Bravo."), 1, "one conclusion was ever made");
+    let ledger = f.ledger.lock().unwrap();
+    assert!(!ledger.concluded(Workspace::Trusted, AGENT, "Bravo."));
+    assert_eq!(
+        ledger.conclusion(Workspace::Trusted, AGENT, "Bravo."),
+        None,
+        "and it is deleted"
+    );
+    assert!(
+        ledger
+            .conclusion(Workspace::Trusted, AGENT, "Alpha.")
+            .is_some()
+    );
+}
+
+#[test]
+fn undoing_a_remove_concludes_the_entry_again() {
+    let mut f = Fixture::new("undo-remove");
+    f.write("mm-a", DataClass::Private, &add("Charlie."));
+    f.send_all();
+    let removed = f.write("mm-a", DataClass::Private, &remove("Charlie."));
+    f.send_all();
+    f.undo(&removed);
+    assert!(matches!(
+        &f.actions_of(&last_edit(&f.store))[..],
+        [Action::Conclude { text, .. }] if text == "Charlie."
+    ));
+    f.send_all();
+    let ledger = f.ledger.lock().unwrap();
+    assert!(ledger.concluded(Workspace::Trusted, AGENT, "Charlie."));
+    assert_eq!(
+        ledger
+            .conclusion(Workspace::Trusted, AGENT, "Charlie.")
+            .map(|(_, id)| id),
+        Some("c3".to_string())
+    );
+}
+
+#[test]
+fn undoing_an_edit_of_secret_entries_concludes_nothing() {
+    let mut f = Fixture::new("undo-secret");
+    f.write("mm-s", DataClass::Secret, &add("Delta."));
+    f.send_all();
+    let removed = f.write("mm-a", DataClass::Private, &remove("Delta."));
+    f.send_all();
+    f.undo(&removed);
+    assert!(
+        f.actions_of(&last_edit(&f.store)).is_empty(),
+        "{:?}",
+        f.decisions()
+    );
+    assert_eq!(f.concludes_of("Delta."), 0);
+    // A secret replace of a clean entry, undone: the clean entry comes back, the secret one goes.
+    f.write("mm-a", DataClass::Private, &add("Echo."));
+    f.send_all();
+    let replaced = f.write(
+        "mm-s",
+        DataClass::Secret,
+        r#"{"action":"replace","file":"memory","old":"Echo.","text":"Echo, secretly."}"#,
+    );
+    f.send_all();
+    f.undo(&replaced);
+    let undo = f.actions_of(&last_edit(&f.store));
+    assert_eq!(undo.len(), 2, "{undo:?}");
+    assert!(matches!(&undo[0], Action::Unconclude { text, .. } if text == "Echo, secretly."));
+    assert!(matches!(&undo[1], Action::Conclude { text, .. } if text == "Echo."));
+    assert_eq!(f.concludes_of("Echo, secretly."), 0);
+}