authorclover caruso <git@paperclover.net> 2026-10-10 21:04:35-07:00
committerclover caruso <git@paperclover.net> 2026-10-10 22:56:06-07:00
log4ea1421ffb1f6c887df24f9c459c933ddcc4df68
tree656223862ebf064500faf08e35b0cbcc821d0e71
parent7baef319fd7df019dca65653490704ee440341cd
signature Signed by SSH key SHA256:52mNGHRsVFBDED9IAX5pe+LRWUefqTbxEReunq21QvU (~clover)

fix: await folder receipts when recycling pages

Publish recycle-bin and TOC append transactions through their durable host receipts, then confirm the exact result stamp before source retirement. Keep native synchronous entry points and share existing TOC/bin construction. Fixes #119 Assisted-by: gpt-6.1-sol

4 files changed, 328 insertions(+), 46 deletions(-)

crates/notebook/src/session.rs+129-38
......@@ -114,6 +114,31 @@ pub trait Storage: Send + Sync {
114114 fn supersede(&self, path: &str, base: &Stamp, with: &str) -> Result<()>;
115115}
116116
117struct Publication {
118 path: String,
119 transaction: Transaction,
120}
121
122impl Publication {
123 async fn commit(self, storage: &dyn Storage) -> Result<()> {
124 match storage.commit(&self.path, &self.transaction) {
125 Err(Error::Remote(error))
126 if error.state == onestore::CommitState::Unknown
127 && error.error.kind() == io::ErrorKind::WouldBlock =>
128 {
129 let expected = self.transaction.stamp();
130 fs::durable().await.map_err(|error| CommitError {
131 state: onestore::CommitState::Unknown,
132 error,
133 })?;
134 storage.confirm(&self.path, &expected)?;
135 Ok(())
136 }
137 result => result,
138 }
139 }
140}
141
117142/// A mounted notebook directory.
118143struct Directory(PathBuf);
119144
......@@ -687,6 +712,17 @@ impl Notebook {
687712 /// Commits `edits` to the TOC at `toc`. A name it still lists for a file gone from its
688713 /// folder passes to the file an edit gives that name; the stale entry goes.
689714 fn commit_toc(&self, toc: &str, edits: &[onestore::TocEdit]) -> Result<()> {
715 match self.toc_publication(toc, edits)? {
716 Some(commit) => self.storage.commit(&commit.path, &commit.transaction),
717 None => Ok(()),
718 }
719 }
720
721 fn toc_publication(
722 &self,
723 toc: &str,
724 edits: &[onestore::TocEdit],
725 ) -> Result<Option<Publication>> {
690726 let unresolved = self
691727 .folder(split(toc).0)
692728 .ok()
......@@ -714,10 +750,14 @@ impl Notebook {
714750 superseded.push(edit.clone());
715751 }
716752 let source = self.storage.read(toc)?;
717 match onestore::edit_table_of_contents(&source, &superseded)? {
718 Some(transaction) => self.storage.commit(toc, &transaction),
719 None => Ok(()),
720 }
753 Ok(
754 onestore::edit_table_of_contents(&source, &superseded)?.map(|transaction| {
755 Publication {
756 path: toc.into(),
757 transaction,
758 }
759 }),
760 )
721761 }
722762
723763 /// Creates `name.one` in `folder` holding the page `page` creates, in the next of
......@@ -1016,6 +1056,14 @@ impl Notebook {
10161056 /// The recycle bin's TOC path and file identity, creating the bin as OneNote does: a
10171057 /// section group the root TOC lists. A bin the root TOC misses is listed again.
10181058 fn bin(&self) -> Result<(String, [u8; 16])> {
1059 let (toc, identity, publication) = self.prepare_bin()?;
1060 if let Some(commit) = publication {
1061 self.storage.commit(&commit.path, &commit.transaction)?;
1062 }
1063 Ok((toc, identity))
1064 }
1065
1066 fn prepare_bin(&self) -> Result<(String, [u8; 16], Option<Publication>)> {
10191067 if let Some(entry) = self
10201068 .catalog
10211069 .unavailable
......@@ -1046,18 +1094,20 @@ impl Notebook {
10461094 .file_id;
10471095 let (root, root_identity) = self.toc("")?;
10481096 // OneNote takes a TOC placed under another parent for a new one.
1049 if !lists(&self.storage.read(&root)?, identity)? {
1097 let publication = if !lists(&self.storage.read(&root)?, identity)? {
10501098 self.storage.place(&bin_toc, root_identity, RECYCLE_BIN)?;
1051 self.commit_toc(
1099 self.toc_publication(
10521100 &root,
10531101 &[onestore::TocEdit::Add {
10541102 filename: RECYCLE_BIN.into(),
10551103 identity,
10561104 group: true,
10571105 }],
1058 )?;
1059 }
1060 Ok((bin_toc, identity))
1106 )?
1107 } else {
1108 None
1109 };
1110 Ok((bin_toc, identity, publication))
10611111 }
10621112
10631113 /// Moves the section file at `path`, its folder's `entry`, into the recycle bin, under a
......@@ -1093,8 +1143,18 @@ impl Notebook {
10931143 /// level with its identity, title, date and creation time. The caller then deletes the
10941144 /// pages from their section.
10951145 pub fn recycle_pages(&mut self, pages: &[Page], author: &str) -> Result<()> {
1146 crate::task::blocking(self.recycle_pages_async(pages, author))?
1147 }
1148
1149 /// Recycles pages after the browser host acknowledges each destination append.
1150 pub async fn recycle_pages_async(&mut self, pages: &[Page], author: &str) -> Result<()> {
10961151 const DELETED: &str = "OneNote_DeletedPages.one";
1097 let (bin_toc, bin_identity) = self.bin()?;
1152 // A rejected host receipt leaves its append queued ahead of this retry.
1153 fs::durable().await?;
1154 let (bin_toc, bin_identity, publication) = self.prepare_bin()?;
1155 if let Some(commit) = publication {
1156 commit.commit(self.storage.as_ref()).await?;
1157 }
10981158 let path = catalog_path(RECYCLE_BIN, DELETED);
10991159 if !self.storage.exists(&path) {
11001160 // OneNote's "Deleted Pages" section is grey.
......@@ -1106,32 +1166,51 @@ impl Notebook {
11061166 let identity = onestore::Store::parse(&bytes)?.header.file_id;
11071167 if !lists(&self.storage.read(&bin_toc)?, identity)? {
11081168 self.storage.place(&path, bin_identity, DELETED)?;
1109 self.commit_toc(
1169 if let Some(commit) = self.toc_publication(
11101170 &bin_toc,
11111171 &[onestore::TocEdit::Add {
11121172 filename: DELETED.into(),
11131173 identity,
11141174 group: false,
11151175 }],
1116 )?;
1176 )? {
1177 commit.commit(self.storage.as_ref()).await?;
1178 }
11171179 bytes = self.storage.read(&path)?;
11181180 }
1119 let arena = onestore::Arena::default();
1120 let mut section = onestore::Section::open(&arena, bytes)?;
1121 let mut ops = Vec::new();
1122 for page in pages {
1123 ops.push(moved(page, author)?);
1124 }
1125 section.apply(
1126 author,
1127 &Edit {
1128 at: crate::now(),
1129 ops,
1130 },
1131 )?;
1132 if let Some(transaction) = section.seal()? {
1133 self.storage.commit(&path, &transaction)?;
1181 let transaction = {
1182 let arena = onestore::Arena::default();
1183 let mut section = onestore::Section::open(&arena, bytes)?;
1184 let mut replaced = Vec::new();
1185 for (space, ..) in section.pages()? {
1186 if section.page(space)?.identity.is_some_and(|identity| {
1187 pages.iter().any(|page| page.identity == Some(identity))
1188 }) {
1189 replaced.push(space);
1190 }
1191 }
1192 let mut ops = Vec::new();
1193 if !replaced.is_empty() {
1194 ops.push(Op::Section(SectionOp::Delete(replaced)));
1195 }
1196 for page in pages {
1197 ops.push(moved(page, author)?);
1198 }
1199 section.apply(
1200 author,
1201 &Edit {
1202 at: crate::now(),
1203 ops,
1204 },
1205 )?;
1206 section.seal()?
1207 };
1208 if let Some(transaction) = transaction {
1209 Publication { path, transaction }
1210 .commit(self.storage.as_ref())
1211 .await?;
11341212 }
1213 fs::durable().await?;
11351214 self.refresh().map(drop)
11361215 }
11371216
......@@ -1139,6 +1218,12 @@ impl Notebook {
11391218 /// revision, as OneNote 2010 does when Undo brings deleted pages back. Pages not there
11401219 /// are passed over.
11411220 pub fn unrecycle_pages(&mut self, identities: &[[u8; 16]]) -> Result<()> {
1221 crate::task::blocking(self.unrecycle_pages_async(identities))?
1222 }
1223
1224 /// Removes recycled pages after the browser host acknowledges their deletion append.
1225 pub async fn unrecycle_pages_async(&mut self, identities: &[[u8; 16]]) -> Result<()> {
1226 fs::durable().await?;
11421227 let path = catalog_path(RECYCLE_BIN, "OneNote_DeletedPages.one");
11431228 if !self.storage.exists(&path) {
11441229 return Ok(());
......@@ -1157,18 +1242,24 @@ impl Notebook {
11571242 if binned.is_empty() {
11581243 return Ok(());
11591244 }
1160 let arena = onestore::Arena::default();
1161 let mut section = onestore::Section::open(&arena, bytes)?;
1162 section.apply(
1163 "",
1164 &Edit {
1165 at: crate::now(),
1166 ops: vec![Op::Section(SectionOp::Delete(binned))],
1167 },
1168 )?;
1169 if let Some(transaction) = section.seal()? {
1170 self.storage.commit(&path, &transaction)?;
1245 let transaction = {
1246 let arena = onestore::Arena::default();
1247 let mut section = onestore::Section::open(&arena, bytes)?;
1248 section.apply(
1249 "",
1250 &Edit {
1251 at: crate::now(),
1252 ops: vec![Op::Section(SectionOp::Delete(binned))],
1253 },
1254 )?;
1255 section.seal()?
1256 };
1257 if let Some(transaction) = transaction {
1258 Publication { path, transaction }
1259 .commit(self.storage.as_ref())
1260 .await?;
11711261 }
1262 fs::durable().await?;
11721263 self.refresh().map(drop)
11731264 }
11741265
crates/notebook/src/session/tests.rs+182-6
......@@ -73,14 +73,20 @@ fn renaming_an_unlisted_section_creates_its_toc_entry_and_preserves_its_neighbor
7373 }
7474}
7575
76/// A mounted notebook where another writer commits `revision` to the section file just
77/// before Snowbound's rewritten file takes its place.
78struct Racing {
76struct FaultStorage {
7977 inner: Directory,
8078 revision: Mutex<Option<(String, Transaction)>>,
79 receipt: Option<Arc<Mutex<Receipt>>>,
8180}
8281
83impl Racing {
82#[derive(Default)]
83struct Receipt {
84 reject: Option<String>,
85 reject_confirmation: Option<String>,
86 confirmed: Vec<String>,
87}
88
89impl FaultStorage {
8490 fn race(&self, path: &str) {
8591 let revision = self.revision.lock().unwrap().take();
8692 if let Some((section, transaction)) = revision {
......@@ -90,7 +96,7 @@ impl Racing {
9096 }
9197}
9298
93impl Storage for Racing {
99impl Storage for FaultStorage {
94100 fn discover(
95101 &self,
96102 cache: &mut discover::Cache,
......@@ -141,9 +147,29 @@ impl Storage for Racing {
141147 self.inner.place(path, ancestor, name)
142148 }
143149 fn commit(&self, path: &str, transaction: &Transaction) -> Result<()> {
150 if let Some(receipt) = &self.receipt {
151 if receipt.lock().unwrap().reject.as_deref() != Some(path) {
152 self.inner.commit(path, transaction)?;
153 }
154 return Err(CommitError {
155 state: onestore::CommitState::Unknown,
156 error: io::ErrorKind::WouldBlock.into(),
157 }
158 .into());
159 }
144160 self.inner.commit(path, transaction)
145161 }
146162 fn confirm(&self, path: &str, base: &Stamp) -> std::result::Result<(), CommitError> {
163 if let Some(receipt) = &self.receipt {
164 let mut receipt = receipt.lock().unwrap();
165 receipt.confirmed.push(path.into());
166 if receipt.reject_confirmation.as_deref() == Some(path) {
167 return Err(CommitError {
168 state: onestore::CommitState::Unknown,
169 error: io::ErrorKind::PermissionDenied.into(),
170 });
171 }
172 }
147173 self.inner.confirm(path, base)
148174 }
149175 fn supersede(&self, path: &str, base: &Stamp, with: &str) -> Result<()> {
......@@ -183,9 +209,10 @@ fn a_password_never_overwrites_a_concurrent_revision() {
183209 let revision = section.seal().unwrap().unwrap();
184210
185211 let root = folder.canonicalize().unwrap();
186 let storage = Racing {
212 let storage = FaultStorage {
187213 inner: Directory(root.clone()),
188214 revision: Mutex::new(Some((path.clone(), revision))),
215 receipt: None,
189216 };
190217 let mut notebook = Notebook::with(Box::new(storage), Some(root), &cache).unwrap();
191218 let set = notebook.set_password(&path, None, Some("secret"));
......@@ -206,3 +233,152 @@ fn a_password_never_overwrites_a_concurrent_revision() {
206233 Err(_) => assert_eq!(titles(&stored), ["Mine", "Theirs"]),
207234 }
208235}
236
237#[test]
238fn recycling_confirms_uncertain_toc_and_page_appends_and_undo() {
239 let temporary = tempfile::tempdir().unwrap();
240 let folder = temporary.path().join("notebook");
241 let cache = temporary.path().join("cache");
242 let creation = PageCreation::new(None, Some("Keep this page"), "Fixture").unwrap();
243 let notebook = Notebook::create(&folder, &cache, Notebook::NEW_COLOR, &creation).unwrap();
244 let path = notebook.catalog().sections[0].path.clone();
245 let source = notebook.read_section(&path).unwrap();
246 let page = stored_pages(&source).unwrap().remove(0).page;
247 drop(notebook);
248 let root = folder.canonicalize().unwrap();
249 let receipt = Arc::new(Mutex::new(Receipt::default()));
250 let storage = FaultStorage {
251 inner: Directory(root.clone()),
252 revision: Mutex::new(None),
253 receipt: Some(receipt.clone()),
254 };
255 let mut notebook = Notebook::with(Box::new(storage), Some(root), &cache).unwrap();
256 notebook
257 .recycle_pages(std::slice::from_ref(&page), "Fixture")
258 .unwrap();
259 let deleted = catalog_path(RECYCLE_BIN, "OneNote_DeletedPages.one");
260 assert_eq!(
261 receipt.lock().unwrap().confirmed,
262 [
263 TOC.to_string(),
264 catalog_path(RECYCLE_BIN, TOC),
265 deleted.clone(),
266 ]
267 );
268 let binned = fs::read(folder.join(&deleted)).unwrap();
269 let pages = stored_pages(&binned).unwrap();
270 assert_eq!(pages.len(), 1);
271 assert_eq!(pages[0].page.title, page.title);
272 assert_eq!(pages[0].page.identity, page.identity);
273 assert_eq!(pages[0].page.created, page.created);
274 assert_eq!(pages[0].page.date_text(), page.date_text());
275 let cold = Notebook::open(&folder, temporary.path().join("cold-cache")).unwrap();
276 assert_eq!(cold.catalog().groups.len(), 1);
277 assert_eq!(cold.folder(RECYCLE_BIN).unwrap().sections.len(), 1);
278 assert_eq!(
279 stored_pages(&cold.read_section(&deleted).unwrap())
280 .unwrap()
281 .len(),
282 1
283 );
284 drop(cold);
285 notebook.unrecycle_pages(&[page.identity.unwrap()]).unwrap();
286 let restored = fs::read(folder.join(&deleted)).unwrap();
287 assert!(
288 restored.len() > binned.len(),
289 "Undo must append its deletion revision"
290 );
291 assert!(stored_pages(&restored).unwrap().is_empty());
292 assert_eq!(receipt.lock().unwrap().confirmed.last(), Some(&deleted));
293 assert_eq!(receipt.lock().unwrap().confirmed.len(), 4);
294 assert_eq!(fs::read(folder.join(&path)).unwrap(), source);
295}
296
297#[test]
298fn recycling_rejects_unconfirmed_appends_and_can_retry_each_stage() {
299 let deleted = catalog_path(RECYCLE_BIN, "OneNote_DeletedPages.one");
300 for (rejected, retained) in [
301 (TOC.to_string(), false),
302 (catalog_path(RECYCLE_BIN, TOC), false),
303 (deleted.clone(), false),
304 (deleted.clone(), true),
305 ] {
306 let temporary = tempfile::tempdir().unwrap();
307 let folder = temporary.path().join("notebook");
308 let cache = temporary.path().join("cache");
309 let creation = PageCreation::new(None, Some("Keep this page"), "Fixture").unwrap();
310 let notebook = Notebook::create(&folder, &cache, Notebook::NEW_COLOR, &creation).unwrap();
311 let path = notebook.catalog().sections[0].path.clone();
312 let mut source = notebook.read_section(&path).unwrap();
313 let mut page = stored_pages(&source).unwrap().remove(0).page;
314 drop(notebook);
315 let root = folder.canonicalize().unwrap();
316 let receipt = Arc::new(Mutex::new(Receipt {
317 reject: (!retained).then(|| rejected.clone()),
318 reject_confirmation: retained.then(|| rejected.clone()),
319 confirmed: Vec::new(),
320 }));
321 let storage = FaultStorage {
322 inner: Directory(root.clone()),
323 revision: Mutex::new(None),
324 receipt: Some(receipt.clone()),
325 };
326 let mut notebook = Notebook::with(Box::new(storage), Some(root), &cache).unwrap();
327 assert!(
328 matches!(
329 notebook.recycle_pages(std::slice::from_ref(&page), "Fixture"),
330 Err(Error::Remote(_))
331 ),
332 "{rejected}"
333 );
334 assert_eq!(receipt.lock().unwrap().confirmed.last(), Some(&rejected));
335 assert_eq!(fs::read(folder.join(&path)).unwrap(), source);
336 if folder.join(&deleted).exists() {
337 assert_eq!(
338 stored_pages(&fs::read(folder.join(&deleted)).unwrap())
339 .unwrap()
340 .len(),
341 usize::from(retained)
342 );
343 }
344 if retained {
345 let arena = onestore::Arena::default();
346 let mut section = onestore::Section::open(&arena, source).unwrap();
347 let space = section.pages().unwrap()[0].0;
348 section
349 .apply(
350 "Fixture",
351 &Edit {
352 at: crate::now(),
353 ops: vec![Op::Page {
354 space,
355 op: onestore::op::PageOp::Color(Some(0xabcdef)),
356 }],
357 },
358 )
359 .unwrap();
360 section
361 .seal()
362 .unwrap()
363 .unwrap()
364 .commit_file(folder.join(&path))
365 .unwrap();
366 source = fs::read(folder.join(&path)).unwrap();
367 page = stored_pages(&source).unwrap().remove(0).page;
368 }
369 {
370 let mut receipt = receipt.lock().unwrap();
371 receipt.reject = None;
372 receipt.reject_confirmation = None;
373 }
374 notebook
375 .recycle_pages(std::slice::from_ref(&page), "Fixture")
376 .unwrap();
377 let binned = stored_pages(&fs::read(folder.join(&deleted)).unwrap()).unwrap();
378 assert_eq!(binned.len(), 1, "{rejected}");
379 assert_eq!(binned[0].page.title, page.title);
380 assert_eq!(binned[0].page.identity, page.identity);
381 assert_eq!(binned[0].page.color, page.color);
382 assert_eq!(fs::read(folder.join(&path)).unwrap(), source);
383 }
384}
crates/onestore/src/commit.rs+9
......@@ -529,6 +529,14 @@ impl Transaction {
529529 &self.base
530530 }
531531
532 /// The committed image's header and length, for confirmation after an uncertain answer.
533 pub fn stamp(&self) -> Stamp {
534 Stamp {
535 header: self.header,
536 length: self.base.length + self.append.len() as u64,
537 }
538 }
539
532540 /// The transaction as bytes `from_bytes` reads back, to carry it to another machine to
533541 /// commit: the base header and length, the new header, the appended bytes' length and
534542 /// the bytes, then each patch's offset, length and bytes. Integers are little-endian.
......@@ -715,6 +723,7 @@ mod tests {
715723 let mut together = original;
716724 combined.apply(&mut together).unwrap();
717725 assert_eq!(together, separate);
726 assert_eq!(combined.stamp(), Stamp::of(&together).unwrap());
718727 }
719728
720729 #[test]
crates/snowbound/src/undo.rs+8-2
......@@ -454,7 +454,10 @@ impl Site {
454454 let (show, added) = self.after(&listed, &pages, show, &mut ops);
455455 if !created && binned {
456456 let pages: Vec<Page> = kept.iter().map(|kept| kept.page.clone()).collect();
457 self.library.reopen()?.recycle_pages(&pages, &self.author)?;
457 self.library
458 .reopen()?
459 .recycle_pages_async(&pages, &self.author)
460 .await?;
458461 self.replica.written_async().await?;
459462 }
460463 self.apply(ops).await?;
......@@ -505,7 +508,10 @@ impl Site {
505508 if !created && binned {
506509 let identities: Vec<[u8; 16]> =
507510 back.iter().filter_map(|kept| kept.page.identity).collect();
508 self.library.reopen()?.unrecycle_pages(&identities)?;
511 self.library
512 .reopen()?
513 .unrecycle_pages_async(&identities)
514 .await?;
509515 }
510516 Applied {
511517 undo: Change::Delete {