Keep session joins working when a document has out-of-range ids

Fixes #1067

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/collab/src/entry_ids_tests.rs

@@ -92,6 +92,37 @@ fn client_ids_skip_spaces_used_by_styles_and_comments() {
     assert_eq!(join(&mut server).id(), 10);
 }
 
+#[test]
+fn an_id_in_the_top_space_does_not_block_joins() {
+    let mut d = Document::new("a");
+    d.styles.push(style(format!("{STYLE_ID_PREFIX}{}", u64::MAX), "Out of range"));
+    let c = comment(u64::from(u32::MAX) << 32, &d);
+    d.comments.push(c);
+    let mut server = Server::new(d, "", Limits::default());
+    let a = join(&mut server);
+    assert!(a.id() > 0 && a.id() < u32::MAX, "{}", a.id());
+    // After a reload the session still takes newcomers and keeps the document as it was.
+    let state = server.state().unwrap();
+    let mut restored = Server::restore(state.clone(), "", Limits::default()).unwrap();
+    let b = join(&mut restored);
+    assert!(b.id() > a.id() && b.id() < u32::MAX, "{}", b.id());
+    assert_eq!(restored.document().styles.len(), 1);
+    assert_eq!(restored.document().comments.len(), 1);
+    // A state saved while the counter was stuck at the top recovers too.
+    let stuck = crate::SessionState { next_client: u32::MAX, ..state };
+    let mut restored = Server::restore(stuck, "", Limits::default()).unwrap();
+    let c = join(&mut restored);
+    assert!(c.id() > a.id() && c.id() < u32::MAX, "{}", c.id());
+    // A new entry may not take an id from a space nobody was given yet.
+    for (k, space) in [c.id() + 1, c.id() + 1000, u32::MAX].into_iter().enumerate() {
+        let id = format!("{STYLE_ID_PREFIX}{}", (u64::from(space) << 32) | 1);
+        let ServerMsg::Reject { reason, .. } = send(&mut restored, c.id(), 1 + k as u64, vec![put_style(id.clone())]) else {
+            panic!("accepted {id}")
+        };
+        assert!(reason.contains("id space"), "{reason}");
+    }
+}
+
 /// Each client makes `rounds` styles, collections (with an extra mode), variables and comments
 /// before seeing anyone else's.
 fn create_everything(doc: &mut Document, who: usize, round: usize) {

crates/collab/src/server.rs

@@ -153,7 +153,13 @@ impl Server {
         server.state = replica;
         server.session = state.session;
         server.seq = state.seq;
-        server.next_client = server.next_client.max(state.next_client);
+        // A counter stuck at the top (saved while an out-of-range id in the document blocked
+        // every join) carries on above the ids actually handed out.
+        let saved = match state.next_client {
+            u32::MAX => state.issued.keys().chain(state.log.iter().map(|a| &a.author)).max().map_or(0, |c| c.saturating_add(1)),
+            n => n,
+        };
+        server.next_client = server.next_client.max(saved);
         server.issued = state.issued.into_iter().filter(|(id, _)| *id < server.next_client).collect();
         server.deleted = state.deleted;
         server.removed = state.removed;
@@ -481,9 +487,9 @@ impl Server {
 
     /// The document-level entries (styles, variables, collections, comments) a batch adds or
     /// removes, each with whether it is there afterwards. A new one is refused when its id lies in
-    /// the space of another client this session gave an id to (unless it was removed before: undo
-    /// puts entries back); ids from before the session (space 0, earlier sessions, other files)
-    /// are fine.
+    /// the space of another client this session gave an id to, or of one it hasn't given out yet
+    /// (unless it was removed before: undo puts entries back); ids from before the session (space
+    /// 0, earlier sessions, other files) are fine.
     fn check_entries(&self, from: u32, changes: &[Change], after: &Document) -> Result<Vec<(Entry, bool)>, Error> {
         let before = self.state.document();
         let mut touched: BTreeSet<Entry> = BTreeSet::new();
@@ -510,7 +516,7 @@ impl Server {
             }
             if now
                 && !self.removed.contains(&entry)
-                && let Some(space) = entry.id_space().filter(|s| *s != from && self.issued.contains_key(s))
+                && let Some(space) = entry.id_space().filter(|s| *s != from && (self.issued.contains_key(s) || *s >= self.next_client))
             {
                 return Err(Error::Invalid(format!("new {} isn't in client {from}'s id space (it is client {space}'s)", entry.describe())));
             }

crates/doc/src/ids.rs

@@ -128,7 +128,8 @@ impl Document {
 
     /// The highest client id space any id in the document lies in (layers, comments, styles,
     /// variables, collections, modes): a session hands out client ids above it, so ids made in
-    /// earlier sessions are never made again.
+    /// earlier sessions are never made again. Space `u32::MAX` doesn't count: numbers too big for
+    /// any client's space land there ([`space_of`]) and no session hands that client id out.
     pub fn highest_id_space(&self) -> u32 {
         let nodes = self.ids().map(|id| id.0);
         let comments = self.comments.iter().map(|c| c.id);
@@ -136,6 +137,6 @@ impl Document {
         let vars = self.variables.variables.iter().filter_map(|v| id_number(&v.id, VARIABLE_ID_PREFIX));
         let colls = self.variables.collections.iter().filter_map(|c| id_number(&c.id, COLLECTION_ID_PREFIX));
         let modes = self.variables.collections.iter().flat_map(|c| c.modes.iter().filter_map(|m| mode_number(&m.id)));
-        nodes.chain(comments).chain(styles).chain(vars).chain(colls).chain(modes).map(space_of).max().unwrap_or(0)
+        nodes.chain(comments).chain(styles).chain(vars).chain(colls).chain(modes).map(space_of).filter(|s| *s != u32::MAX).max().unwrap_or(0)
     }
 }

crates/doc/src/ids_tests.rs

@@ -103,6 +103,10 @@ fn the_highest_space_counts_every_kind_of_id() {
     assert_eq!(d.highest_id_space(), 9);
     d.comments.push(comment(11u64 << 32, d.first_page().unwrap()));
     assert_eq!(d.highest_id_space(), 11);
+    // Numbers past every client's space (they saturate to `u32::MAX`) are no session's.
+    d.styles.push(style(&format!("S:{}", u64::MAX)));
+    d.comments.push(comment(u64::from(u32::MAX) << 32, d.first_page().unwrap()));
+    assert_eq!(d.highest_id_space(), 11);
     // Other forms are space 0.
     assert_eq!(id_space("lib:S:99999999999", "S:"), 0);
     assert_eq!(id_space("S:+5", "S:"), 0);