fix(storage): don't tombstone Frozen descendants on subtree delete

crates/storage/src/index.rs

@@ -1003,6 +1003,14 @@ impl<S: StorageAdaptor> Index<S> {
     /// single-level concurrent add/update-vs-delete semantics applied
     /// transitively.
     ///
+    /// `Frozen` descendants are the one exception: they are immutable and never
+    /// deleted, so the walk skips a `Frozen` node and its whole subtree rather
+    /// than tombstoning it. A local delete never reaches this state — it is
+    /// rejected up front by `remove_child_from`'s `find_frozen_descendant` scan.
+    /// The skip here is the replay-side fallback: on `apply_delete_ref_action`
+    /// this replica can't reject a peer's `DeleteRef` without diverging, so it
+    /// preserves the frozen data (leaving it a detached orphan) and converges.
+    ///
     /// Internal subtree parent-lists and hashes are intentionally NOT
     /// recomputed: the whole subtree is detached at the root, so none of it
     /// feeds a live hash. Only the root's removal from its parent (done by the
@@ -1029,6 +1037,21 @@ impl<S: StorageAdaptor> Index<S> {
             let Some(index) = Self::get_index(id)? else {
                 continue;
             };
+            // Frozen data is immutable and never deleted: skip a Frozen node AND
+            // its subtree (don't recurse) so deleting a non-frozen parent leaves
+            // the frozen descendant as a surviving orphan. Mirrors the
+            // `RemoveMode::Delete` Frozen guard in `Interface`. Scoped to
+            // descendants (`id != root_id`): both callers (`remove_child_from`,
+            // `apply_delete_ref_action`) reject deleting Frozen data upstream, so
+            // the root is never Frozen here; the caller tombstones it regardless.
+            if id != root_id
+                && matches!(
+                    index.metadata.storage_type,
+                    crate::entities::StorageType::Frozen
+                )
+            {
+                continue;
+            }
             if let Some(children) = &index.children {
                 for child in children {
                     stack.push(child.id());
@@ -1051,6 +1074,41 @@ impl<S: StorageAdaptor> Index<S> {
         Ok(())
     }
 
+    /// Returns the id of the first `Frozen` entity in `root_id`'s subtree
+    /// (excluding `root_id` itself), or `None` if the subtree holds no frozen
+    /// data.
+    ///
+    /// Read-only pre-check for the local delete guard in
+    /// [`remove_child_from`](crate::interface::Interface::remove_child_from): a
+    /// genuine subtree delete must refuse to strand `Frozen` data, so the
+    /// caller rejects the delete and asks the operator to relocate the frozen
+    /// entity out of the subtree first. Kept separate from
+    /// [`tombstone_descendants_of`](Self::tombstone_descendants_of) because the
+    /// check must complete and reject BEFORE any state is mutated.
+    pub(crate) fn find_frozen_descendant(root_id: Id) -> Result<Option<Id>, StorageError> {
+        let _mutation_guard = index_mutation_guard();
+        let mut stack = vec![root_id];
+        while let Some(id) = stack.pop() {
+            let Some(index) = Self::get_index(id)? else {
+                continue;
+            };
+            if id != root_id
+                && matches!(
+                    index.metadata.storage_type,
+                    crate::entities::StorageType::Frozen
+                )
+            {
+                return Ok(Some(id));
+            }
+            if let Some(children) = &index.children {
+                for child in children {
+                    stack.push(child.id());
+                }
+            }
+        }
+        Ok(None)
+    }
+
     /// Removes a child reference from a parent without creating a tombstone.
     ///
     /// Used when reassigning collection IDs - we need to remove the old child

crates/storage/src/interface.rs

@@ -2500,6 +2500,21 @@ impl<S: StorageAdaptor> Interface<S> {
             ));
         }
 
+        // A genuine subtree delete must not strand Frozen data buried deeper in
+        // the tree either. Scan descendants BEFORE mutating any state; if any
+        // Frozen entity exists, reject so the operator relocates it out of the
+        // subtree first. Same split-brain avoidance as the direct-child guard
+        // above: we never tombstone the subtree or broadcast a `DeleteRef` that
+        // would leave the frozen data detached on every peer.
+        if mode == RemoveMode::Delete {
+            if let Some(frozen_id) = <Index<S>>::find_frozen_descendant(child_id)? {
+                return Err(StorageError::ActionNotAllowed(format!(
+                    "cannot delete subtree {child_id}: it contains Frozen data at {frozen_id}; \
+                     relocate the frozen entity out of the subtree before deleting"
+                )));
+            }
+        }
+
         // If this is a local user action, set the nonce
         if let StorageType::User { owner, .. } = metadata.storage_type {
             if *owner == crate::env::executor_id() {

crates/storage/src/tests/index.rs

@@ -793,6 +793,171 @@ mod subtree_tombstoning {
         }
         assert!(!<Index<TestStorage>>::is_deleted(root).unwrap());
     }
+
+    /// A non-frozen parent may contain a `Frozen` descendant (e.g. a
+    /// `FrozenStorage` field). Frozen data is immutable and never deleted, so
+    /// deleting the parent must SKIP the frozen node AND its subtree (no
+    /// recursion) while still tombstoning the non-frozen rows. Regression for
+    /// the #286 cascade, which tombstoned frozen leaves too.
+    ///
+    /// This exercises the `tombstone_descendants_of` skip layer directly
+    /// (`Index`-level). The `Interface`-level behavior — a local delete of such
+    /// a subtree is REJECTED before any mutation — is covered separately by
+    /// `remove_child_from_rejects_subtree_with_frozen_descendant` in
+    /// `tests/interface.rs`. The skip here is the replay-side fallback.
+    #[test]
+    fn subtree_delete_skips_frozen_descendant_and_its_subtree() {
+        use crate::entities::StorageType;
+        type S = MockedStorage<2104>;
+
+        let frozen_md = Metadata {
+            storage_type: StorageType::Frozen,
+            ..Metadata::default()
+        };
+
+        let root = Id::random();
+        let a = Id::random(); // non-frozen parent being deleted
+        let b = Id::random(); // non-frozen descendant -> tombstoned
+        let f = Id::random(); // Frozen descendant -> survives
+        let fc = Id::random(); // element under the frozen node -> survives (no recursion)
+
+        <Index<S>>::add_root(ChildInfo::new(root, [1; 32], Metadata::default())).unwrap();
+        <Index<S>>::add_child_to(root, ChildInfo::new(a, [2; 32], Metadata::default())).unwrap();
+        <Index<S>>::add_child_to(a, ChildInfo::new(b, [3; 32], Metadata::default())).unwrap();
+        <Index<S>>::add_child_to(a, ChildInfo::new(f, [4; 32], frozen_md.clone())).unwrap();
+        <Index<S>>::add_child_to(f, ChildInfo::new(fc, [5; 32], frozen_md)).unwrap();
+
+        <Index<S>>::remove_child_from(root, a, time_now()).unwrap();
+
+        assert!(
+            <Index<S>>::is_deleted(a).unwrap(),
+            "deleted parent tombstoned"
+        );
+        assert!(
+            <Index<S>>::is_deleted(b).unwrap(),
+            "non-frozen descendant tombstoned"
+        );
+        assert!(
+            !<Index<S>>::is_deleted(f).unwrap(),
+            "Frozen descendant must survive the subtree delete"
+        );
+        assert!(
+            !<Index<S>>::is_deleted(fc).unwrap(),
+            "walk must not recurse into a Frozen node: its subtree survives too"
+        );
+        assert!(
+            <Index<S>>::get_children_of(root)
+                .unwrap()
+                .iter()
+                .all(|c| c.id() != a),
+            "deleted node must be removed from parent's children list"
+        );
+        // The skipped frozen node must NOT be advertised as deleted on the sync
+        // wire: it lives on, so listing it in its parent's `deleted_children`
+        // would make a syncing peer apply a spurious delete-wins against it.
+        assert!(
+            !<Index<S>>::get_index(a)
+                .unwrap()
+                .unwrap()
+                .deleted_children()
+                .contains(&f),
+            "surviving frozen node must not appear in parent's deleted_children advert"
+        );
+    }
+
+    /// The skip is by-subtree, not by-node-type: a Frozen node is skipped
+    /// together with EVERYTHING under it, including a *non-frozen* child. Also
+    /// covers a Frozen leaf (no children of its own). Complements
+    /// `subtree_delete_skips_frozen_descendant_and_its_subtree`.
+    #[test]
+    fn subtree_delete_skips_whole_frozen_subtree_and_frozen_leaf() {
+        use crate::entities::StorageType;
+        type S = MockedStorage<2105>;
+
+        let frozen_md = Metadata {
+            storage_type: StorageType::Frozen,
+            ..Metadata::default()
+        };
+
+        let root = Id::random();
+        let a = Id::random(); // non-frozen parent being deleted
+        let f = Id::random(); // Frozen node with a non-frozen child below it
+        let nf_under_f = Id::random(); // NON-frozen, under Frozen -> still survives (no recursion)
+        let fleaf = Id::random(); // Frozen leaf (no children) -> survives
+
+        <Index<S>>::add_root(ChildInfo::new(root, [1; 32], Metadata::default())).unwrap();
+        <Index<S>>::add_child_to(root, ChildInfo::new(a, [2; 32], Metadata::default())).unwrap();
+        <Index<S>>::add_child_to(a, ChildInfo::new(f, [3; 32], frozen_md.clone())).unwrap();
+        <Index<S>>::add_child_to(f, ChildInfo::new(nf_under_f, [4; 32], Metadata::default()))
+            .unwrap();
+        <Index<S>>::add_child_to(a, ChildInfo::new(fleaf, [5; 32], frozen_md)).unwrap();
+
+        <Index<S>>::remove_child_from(root, a, time_now()).unwrap();
+
+        assert!(
+            <Index<S>>::is_deleted(a).unwrap(),
+            "deleted parent tombstoned"
+        );
+        assert!(!<Index<S>>::is_deleted(f).unwrap(), "Frozen node survives");
+        assert!(
+            !<Index<S>>::is_deleted(nf_under_f).unwrap(),
+            "non-frozen child under a Frozen node survives: the walk skips the \
+             whole frozen subtree, not just frozen nodes"
+        );
+        assert!(
+            !<Index<S>>::is_deleted(fleaf).unwrap(),
+            "Frozen leaf (no children) survives"
+        );
+    }
+
+    /// `find_frozen_descendant` powers the local delete guard: it locates a
+    /// Frozen entity buried anywhere below the delete root (excluding the root
+    /// itself) and returns `None` for a frozen-free subtree.
+    #[test]
+    fn find_frozen_descendant_locates_deep_frozen_and_ignores_root() {
+        use crate::entities::StorageType;
+        type S = MockedStorage<2106>;
+
+        let frozen_md = Metadata {
+            storage_type: StorageType::Frozen,
+            ..Metadata::default()
+        };
+
+        let root = Id::random();
+        let a = Id::random(); // delete root (non-frozen)
+        let b = Id::random(); // non-frozen descendant
+        let f = Id::random(); // Frozen descendant, two levels down
+
+        <Index<S>>::add_root(ChildInfo::new(root, [1; 32], Metadata::default())).unwrap();
+        <Index<S>>::add_child_to(root, ChildInfo::new(a, [2; 32], Metadata::default())).unwrap();
+        <Index<S>>::add_child_to(a, ChildInfo::new(b, [3; 32], Metadata::default())).unwrap();
+        <Index<S>>::add_child_to(b, ChildInfo::new(f, [4; 32], frozen_md.clone())).unwrap();
+
+        assert_eq!(
+            <Index<S>>::find_frozen_descendant(a).unwrap(),
+            Some(f),
+            "must find the frozen entity nested below the delete root"
+        );
+
+        // A frozen-free subtree returns None.
+        let c = Id::random();
+        <Index<S>>::add_child_to(root, ChildInfo::new(c, [5; 32], Metadata::default())).unwrap();
+        assert_eq!(
+            <Index<S>>::find_frozen_descendant(c).unwrap(),
+            None,
+            "a subtree with no frozen data must not be flagged"
+        );
+
+        // The root's own StorageType is ignored — only descendants count.
+        let froot = Id::random();
+        <Index<S>>::add_child_to(root, ChildInfo::new(froot, [6; 32], frozen_md)).unwrap();
+        assert_eq!(
+            <Index<S>>::find_frozen_descendant(froot).unwrap(),
+            None,
+            "the scan excludes the root itself: a Frozen root with no frozen \
+             descendants returns None"
+        );
+    }
 }
 
 #[cfg(test)]

crates/storage/src/tests/interface.rs

@@ -2018,6 +2018,76 @@ mod frozen_storage_verification {
         }
     }
 
+    /// Deleting a non-frozen subtree that has a Frozen entity buried below it
+    /// must also be rejected — the operator has to relocate the frozen data
+    /// out first. The direct child is NOT frozen here, so this exercises the
+    /// descendant scan (`find_frozen_descendant`), not the direct-child guard.
+    #[test]
+    fn remove_child_from_rejects_subtree_with_frozen_descendant() {
+        use crate::delta::{commit_causal_delta, reset_delta_context};
+        use crate::entities::{ChildInfo, Metadata};
+        use crate::index::Index;
+        use crate::store::MainStorage;
+
+        env::reset_for_testing();
+
+        // Non-frozen parent under root, with a non-frozen paragraph beneath it.
+        let mut page = Page::new_from_element("Parent", Element::root());
+        assert!(MainInterface::save(&mut page).unwrap());
+        let mut para = Paragraph::new_from_element("Middle", Element::new(None));
+        assert!(MainInterface::add_child_to(page.id(), &mut para).unwrap());
+
+        // Attach a Frozen entity two levels down, under the paragraph.
+        let frozen_id = Id::new([9; 32]);
+        <Index<MainStorage>>::add_child_to(
+            para.id(),
+            ChildInfo::new(
+                frozen_id,
+                [7; 32],
+                Metadata {
+                    storage_type: StorageType::Frozen,
+                    ..Metadata::default()
+                },
+            ),
+        )
+        .unwrap();
+
+        reset_delta_context();
+
+        // Deleting the paragraph must be refused: its subtree holds frozen data.
+        match MainInterface::remove_child_from(page.id(), para.id()) {
+            Err(StorageError::ActionNotAllowed(msg)) => {
+                assert!(
+                    msg.contains("Frozen") && msg.contains("subtree"),
+                    "error should name the frozen subtree: {msg}"
+                );
+            }
+            other => panic!("Expected ActionNotAllowed error, got {other:?}"),
+        }
+
+        // Nothing tombstoned: the paragraph and the frozen node both survive.
+        assert!(
+            !<Index<MainStorage>>::is_deleted(para.id()).unwrap(),
+            "paragraph must not be tombstoned by a rejected delete"
+        );
+        assert!(
+            !<Index<MainStorage>>::is_deleted(frozen_id).unwrap(),
+            "frozen descendant must survive"
+        );
+
+        // And no `DeleteRef` was broadcast — the guard fires before mutation.
+        if let Some(delta) = commit_causal_delta(&[0; 32]).unwrap() {
+            assert!(
+                !delta
+                    .actions
+                    .iter()
+                    .any(|a| matches!(a, Action::DeleteRef { id, .. } if *id == para.id())),
+                "rejected delete must not emit a DeleteRef, got: {:?}",
+                delta.actions
+            );
+        }
+    }
+
     #[test]
     fn frozen_blob_too_small_fails() {
         env::reset_for_testing();