Commit b28c7cbe0b6bd7420e717e2005265ecbea3a37de | snowbound

author	clover caruso <git@paperclover.net> 2026-10-10 21:29:14-07:00
committer	clover caruso <git@paperclover.net> 2026-10-10 22:56:06-07:00
log	b28c7cbe0b6bd7420e717e2005265ecbea3a37de
tree	322d97775b50afaf2dd6283bac6019c71b0659bf
parent	1cec85d1bb8078889dab2f72485b2407ee2d2de3
signature	Signed by SSH key SHA256:52mNGHRsVFBDED9IAX5pe+LRWUefqTbxEReunq21QvU (~clover)

fix: deliver the refreshed recycle-bin catalog to the open notebook

Keep the completed recycle/unrecycle Notebook in Site.library and reconcile its catalog independently of page loading. Delivery follows the current section in the original open notebook lineage, leaving newer navigation and closed/reopened notebooks alone. Acquire structural ownership before retaining the replica, and enqueue catalog completion before releasing it, to avoid folder-rename lock inversion and older catalog delivery after a newer structure mutation.

Extend the existing delete/Undo/Redo regression to check refreshed bin tabs without a watcher and unchanged replica ownership.

Fixes #119
Assisted-by: gpt-6.1-sol

1 files changed, 77 insertions(+), 16 deletions(-)

crates
- snowbound
  - src
    undo.rs

crates/snowbound/src/undo.rs+77-16

 @@ -411,7 +411,7 @@ impl Site {
     /// Does `change` to the section's pages, `shown` the page open; `None` where what it
     /// changes is gone or holds what it did not make, which it leaves alone.
     async fn change(
-        &self,
+        &mut self,
         change: Change,
         shown: ExGuid,
     ) -> Result<Option<Applied>, Box<dyn Error>> {
 @@ -454,10 +454,9 @@ impl Site {
                 let (show, added) = self.after(&listed, &pages, show, &mut ops);
                 if !created && binned {
                     let pages: Vec<Page> = kept.iter().map(|kept| kept.page.clone()).collect();
-                    self.library
+                    let mut notebook = self.library.reopen()?;
-                        .reopen()?
+                    notebook.recycle_pages_async(&pages, &self.author).await?;
-                        .recycle_pages_async(&pages, &self.author)
+                    self.library = Arc::new(self.library.with(notebook));
-                        .await?;
                     self.replica.written_async().await?;
                 }
                 self.apply(ops).await?;
 @@ -508,10 +507,9 @@ impl Site {
                 if !created && binned {
                     let identities: Vec<[u8; 16]> =
                         back.iter().filter_map(|kept| kept.page.identity).collect();
-                    self.library
+                    let mut notebook = self.library.reopen()?;
-                        .reopen()?
+                    notebook.unrecycle_pages_async(&identities).await?;
-                        .unrecycle_pages_async(&identities)
+                    self.library = Arc::new(self.library.with(notebook));
-                        .await?;
                 }
                 Applied {
                     undo: Change::Delete {
 @@ -1062,13 +1060,29 @@ impl State {
         };
         self.persist()?;
         let session = self.session.as_ref().ok_or("No section is open")?;
-        let site = Site {
+        let changing = if matches!(
+            &change,
+            Change::Delete { created: false, .. } | Change::Restore { created: false, .. }
+        ) && matches!(session.library.notebook, Ok(Some(_)))
+        {
+            Some(
+                session
+                    .library
+                    .structure
+                    .try_lock_arc()
+                    .ok_or_else(|| std::io::Error::from(std::io::ErrorKind::WouldBlock))?,
+            )
+        } else {
+            None
+        };
+        let mut site = Site {
             replica: Arc::clone(session.section.replica()),
             library: Arc::clone(&session.library),
             author: self.author.clone(),
             fresh: self.dated_page(None, "")?,
         };
         let shown = session.space;
+        let original = Arc::clone(&site.library);
         let proxy = self.proxy.clone();
         self.undo.busy += 1;
         self.switching = Some((None, web_time::Instant::now()));
 @@ -1080,8 +1094,34 @@ impl State {
             };
             let failed = applied.as_ref().err().map(ToString::to_string);
             let (stale, applied) = (failed.is_none(), applied.ok().flatten());
+            let library = Arc::clone(&site.library);
             let _ = proxy.send_event(UserEvent::Then(Box::new(move |state: &mut State| {
                 state.undo.busy -= 1;
+                let lineage = |listed: &Library| {
+                    listed.location == original.location
+                        && Arc::ptr_eq(&listed.structure, &original.structure)
+                };
+                if !Arc::ptr_eq(&original, &library)
+                    && state.notebooks.iter().any(|listed| lineage(listed))
+                {
+                    let path = state
+                        .session
+                        .as_ref()
+                        .filter(|session| lineage(&session.library))
+                        .map(|session| session.tabs[session.tab].path.clone());
+                    if let Some(path) = path {
+                        state.adopt(Arc::clone(&library), Some(&path))?;
+                    } else {
+                        state.list(&library);
+                        if state
+                            .sectionless
+                            .as_ref()
+                            .is_some_and(|listed| lineage(listed))
+                        {
+                            state.sectionless = Some(library);
+                        }
+                    }
+                }
                 let Some(applied) = applied else {
                     // What it would change is gone: Undo goes on to the step before.
                     return match taken {
 @@ -1102,6 +1142,7 @@ impl State {
                 state.edited(Vec::new());
                 Ok(())
             })));
+            drop(changing);
             if let Some(failed) = failed {
                 return Err(failed.into());
             }
 @@ -1426,21 +1467,41 @@ mod tests {
     fn deleted_pages_come_back_where_they_were_and_leave_the_bin() {
         let fixture = Fixture::new("delete", &["First", "Second", "Third"]);
         let second = fixture.space("Second");
-        let deleted = fixture
+        let mut site = fixture.site();
-            .change(Change::Delete {
+        assert!(site.library.tabs(crate::recycle::BIN).is_empty());
+        let deleted = pollster::block_on(site.change(
+            Change::Delete {
                 pages: vec![second],
                 show: None,
                 created: false,
-            })
+            },
-            .unwrap();
+            fixture.space("First"),
+        ))
+        .unwrap()
+        .unwrap();
+        assert!(!site.library.tabs(crate::recycle::BIN).is_empty());
+        assert!(!Arc::ptr_eq(&site.library, &fixture.library));
+        assert!(Arc::ptr_eq(
+            &site.library.structure,
+            &fixture.library.structure
+        ));
+        assert!(Arc::ptr_eq(&site.replica, fixture.section().replica()));
+        assert!(fixture.library.tabs(crate::recycle::BIN).is_empty());
+        let deleted_library = Arc::clone(&site.library);
         assert_eq!(fixture.titles(), titled(&["First", "Third"]));
         assert_eq!(deleted.show, fixture.space("Third"));
         assert_eq!(fixture.binned(), ["Second"]);
-        let restored = fixture.change(deleted.undo).unwrap();
+        let restored = pollster::block_on(site.change(deleted.undo, deleted.show))
+            .unwrap()
+            .unwrap();
+        assert!(!Arc::ptr_eq(&site.library, &deleted_library));
+        assert!(!site.library.tabs(crate::recycle::BIN).is_empty());
         assert_eq!(fixture.titles(), titled(&["First", "Second", "Third"]));
         assert_eq!(restored.show, fixture.space("Second"));
         assert!(fixture.binned().is_empty());
-        let again = fixture.change(restored.undo).unwrap();
+        let again = pollster::block_on(site.change(restored.undo, restored.show))
+            .unwrap()
+            .unwrap();
         assert_eq!(fixture.titles(), titled(&["First", "Third"]));
         assert_eq!(fixture.binned(), ["Second"]);