From 8e12e5eeda3c431b0d62b2f67a280728913067a1 Mon Sep 17 00:00:00 2001
From: Justin John <34035011+justinjohn0306@users.noreply.github.com>
Date: Wed, 14 Oct 2026 09:01:30 +0530
Subject: [PATCH] fix(server): never persist a snapshot the reader refuses
 (#188) (#236)

Snapshots hold more than the document (recent batches, deletions), so near
the cap compaction could write one over max_file_bytes, which both readers
then refused: the file vanished on restart. Reading now allows twice the
cap, and compaction refuses a snapshot beyond that, keeping the old
snapshot and log.

Co-authored-by: Justin John <babyjjbabyjj549@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
---
 apps/figcraft-server/src/hardening_tests.rs | 15 +++++++++++++++
 apps/figcraft-server/src/store.rs           | 15 +++++++++++++--
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/apps/figcraft-server/src/hardening_tests.rs b/apps/figcraft-server/src/hardening_tests.rs
index 8b20510..6a74e61 100644
--- a/apps/figcraft-server/src/hardening_tests.rs
+++ b/apps/figcraft-server/src/hardening_tests.rs
@@ -235,6 +235,21 @@ fn a_file_cant_grow_past_its_cap_through_batches() {
     // Its snapshot holds the document and the recent batches kept for reconnecting clients (at
     // most a quarter of the cap), and the last batch let in may finish what the cap allows.
     assert!(on_disk <= max + max / 4 + 210_000, "{on_disk} bytes on disk");
+    drop(raw);
+    server.stop();
+    // Everything confirmed opens again with the same settings, after compacting on the way out.
+    let kept = client.document().nodes().count();
+    let mut server = crate::start(&cfg).unwrap();
+    let mut raw = Raw::connect(server.addr());
+    raw.send(&Client::hello(&file.id, &edit.token, "Grower", ""));
+    let welcome = loop {
+        match raw.next() {
+            w @ ServerMsg::Welcome { .. } => break w,
+            ServerMsg::Refused { reason } => panic!("the file doesn't open again: {reason}"),
+            _ => {}
+        }
+    };
+    assert_eq!(Client::join(welcome).unwrap().document().nodes().count(), kept);
     server.stop();
 }
 
diff --git a/apps/figcraft-server/src/store.rs b/apps/figcraft-server/src/store.rs
index 0bb8537..e5e56b8 100644
--- a/apps/figcraft-server/src/store.rs
+++ b/apps/figcraft-server/src/store.rs
@@ -426,7 +426,7 @@ impl Store {
         let meta: Meta = serde_json::from_slice(&disk::read_capped(&path.join(META), 1 << 20)?).map_err(|e| format!("{META} is damaged ({e})"))?;
         let links: Vec<Link> =
             serde_json::from_slice(&disk::read_capped(&path.join(LINKS), 16 << 20)?).map_err(|e| format!("{LINKS} is damaged ({e})"))?;
-        let generation = disk::check_snapshot(&disk::read_capped(&path.join(SNAPSHOT), self.cfg.max_file_bytes)?)?;
+        let generation = disk::check_snapshot(&disk::read_capped(&path.join(SNAPSHOT), self.snapshot_cap())?)?;
         // A damaged log only loses its end; it is reported when the file opens.
         if let Ok(bytes) = disk::read_capped(&path.join(LOG), self.cfg.max_file_bytes)
             && let Ok(log) = disk::read_log(&bytes)
@@ -1086,7 +1086,7 @@ impl Store {
     fn load(&mut self, file: &str) -> Result<Server, String> {
         self.touch(file);
         let dir = self.dir(file);
-        let (generation, state) = disk::decode_snapshot(&disk::read_capped(&dir.join(SNAPSHOT), self.cfg.max_file_bytes)?)?;
+        let (generation, state) = disk::decode_snapshot(&disk::read_capped(&dir.join(SNAPSHOT), self.snapshot_cap())?)?;
         let mut server = Server::restore(state, "", self.cfg.core).map_err(|e| format!("the snapshot can't be used: {e}"))?;
         let mut issues = Vec::new();
         let mut fresh_log = true;
@@ -1130,11 +1130,22 @@ impl Store {
         Ok(server)
     }
 
+    /// The largest snapshot read back: a file's cap plus room for what its state keeps beside the
+    /// document (recent batches for reconnecting clients, deletions), which may take it over.
+    fn snapshot_cap(&self) -> u64 {
+        self.cfg.max_file_bytes.saturating_mul(2)
+    }
+
     /// Write `server`'s state as snapshot `generation` and start an empty log of it.
     fn rewrite(&mut self, file: &str, generation: u64, server: &Server) -> Result<Log, String> {
         let dir = self.dir(file);
         let state = server.state().map_err(|e| e.to_string())?;
         let bytes = disk::encode_snapshot(generation, &state)?;
+        // Never replace a readable snapshot with one that can't be opened again: the old snapshot
+        // and log still hold everything.
+        if bytes.len() as u64 > self.snapshot_cap() {
+            return Err(format!("the snapshot would be {} MiB, more than can be opened again", bytes.len() >> 20));
+        }
         if bytes.len() as u64 > self.cfg.max_file_bytes {
             log::warn!("file {file} is {} MiB, over the {} MiB cap (kept anyway)", bytes.len() >> 20, self.cfg.max_file_bytes >> 20);
         }