feat(plugin): persist XDATA to DWG and let plugins modify/delete entities (#249, #250)

#249 — plugin XDATA written via write_record survived only in memory: the
acadrust DWG writer dropped ExtendedData::records on save. Bump acadrust to
e88a9a6 (records now encode to EED and decode back on read) and fix the host
side that fed it:
  - ensure_app_id allocates a real APPID handle; a null handle serializes as
    0 and the EED reference can't resolve, so the XDATA vanished on reopen.
  - write_record / remove_record drop stale raw_dwg_eed for the target app so
    an edit made after a save/reopen wins over the pre-edit bytes.

#250 — out-of-process plugins got a throwaway document_mut() snapshot, so
edits to existing entities were silently discarded and deletion wasn't
expressible at all. Add the missing mutation surface:
  - UpdateEntity / RemoveEntity IPC requests + HostApi::update_entity /
    remove_entity (default in-process impls, RPC overrides on the client that
    invalidate the stale document cache).
  - Scene::update_entity replaces the entity in place, preserving its handle
    and owning block, and reseeds only its derived caches; remove reuses the
    cache-coherent erase_entities (which also honours layer locks).
  - document_mut() is documented as a local-only snapshot out-of-process.

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

Cargo.lock

@@ -71,7 +71,7 @@ checksum = "366ffbaa4442f4684d91e2cd7c5ea7c4ed8add41959a31447066e279e432b618"
 [[package]]
 name = "acadrust"
 version = "0.4.0"
-source = "git+https://github.com/HakanSeven12/acadrust?branch=main#4ae2ceb72f00b69e983b20111c809aecf138814c"
+source = "git+https://github.com/HakanSeven12/acadrust?branch=main#e88a9a6afdcf20db4c1f3140088b2f4b8b851e2b"
 dependencies = [
  "ahash 0.8.12",
  "anyhow",

crates/ocs_plugin_api/src/host.rs

@@ -120,10 +120,38 @@ pub trait HostApi {
 
     // ── Document ────────────────────────────────────────────────────────────
     fn document(&self) -> &CadDocument;
+    /// Mutable access to the active document.
+    ///
+    /// For an **out-of-process** plugin this borrows a *local snapshot*: edits
+    /// to existing entities made through it are NOT sent back to the host and
+    /// are silently discarded. To modify or delete entities from any plugin,
+    /// use [`add_entity`](Self::add_entity), [`update_entity`](Self::update_entity)
+    /// and [`remove_entity`](Self::remove_entity), which are committed to the
+    /// host document over IPC.
     fn document_mut(&mut self) -> &mut CadDocument;
 
     /// Add an entity to the active document, returning its handle.
     fn add_entity(&mut self, entity: EntityType) -> Handle;
+    /// Replace the existing entity that carries `entity`'s handle, preserving
+    /// its identity (handle and owning block). Returns `false` when no entity
+    /// has that handle. This is the sanctioned way to commit in-place edits
+    /// from an out-of-process plugin — mutating `document_mut()` does not work
+    /// across the process boundary.
+    fn update_entity(&mut self, entity: EntityType) -> bool {
+        let handle = entity.common().handle;
+        match self.document_mut().get_entity_mut(handle) {
+            Some(slot) => {
+                *slot = entity;
+                true
+            }
+            None => false,
+        }
+    }
+    /// Delete the entity with `handle` (and any derived render caches). Returns
+    /// `true` when an entity was removed.
+    fn remove_entity(&mut self, handle: Handle) -> bool {
+        self.document_mut().remove_entity(handle).is_some()
+    }
     /// Mark the scene geometry dirty so it is re-tessellated next frame.
     fn bump_geometry(&mut self);
 

crates/ocs_plugin_api/src/ipc/client.rs

@@ -151,6 +151,46 @@ impl HostApi for PluginHostApi {
         }
     }
 
+    fn update_entity(&mut self, entity: EntityType) -> bool {
+        match self.client.request(PluginRequest::UpdateEntity(entity)) {
+            Ok(PluginResponse::Bool(b)) => {
+                if b {
+                    // The cached snapshot is now stale; drop it so a later
+                    // document() re-fetches the host's post-edit truth.
+                    self.document_cache = OnceCell::new();
+                }
+                b
+            }
+            Ok(other) => {
+                eprintln!("[plugin] unexpected UpdateEntity response: {other:?}");
+                false
+            }
+            Err(e) => {
+                eprintln!("[plugin] UpdateEntity failed: {e}");
+                false
+            }
+        }
+    }
+
+    fn remove_entity(&mut self, handle: Handle) -> bool {
+        match self.client.request(PluginRequest::RemoveEntity { handle }) {
+            Ok(PluginResponse::Bool(b)) => {
+                if b {
+                    self.document_cache = OnceCell::new();
+                }
+                b
+            }
+            Ok(other) => {
+                eprintln!("[plugin] unexpected RemoveEntity response: {other:?}");
+                false
+            }
+            Err(e) => {
+                eprintln!("[plugin] RemoveEntity failed: {e}");
+                false
+            }
+        }
+    }
+
     fn bump_geometry(&mut self) {
         let _ = self.client.request(PluginRequest::BumpGeometry);
     }
@@ -438,4 +478,36 @@ mod tests {
         peer_handle.join().unwrap();
         assert_eq!(handle, Handle::new(42));
     }
+
+    #[test]
+    fn update_entity_awaits_bool_response() {
+        let (mut api, mut peer) = make_client();
+        let peer_handle = thread::spawn(move || {
+            let msg = recv::<PluginToHost>(&mut peer).unwrap();
+            match msg {
+                PluginToHost::Request(PluginRequest::UpdateEntity(_)) => {}
+                other => panic!("unexpected: {other:?}"),
+            }
+            send(&mut peer, &HostToPlugin::Response(PluginResponse::Bool(true))).unwrap();
+        });
+        assert!(api.update_entity(EntityType::Point(Point::new())));
+        peer_handle.join().unwrap();
+    }
+
+    #[test]
+    fn remove_entity_awaits_bool_response() {
+        let (mut api, mut peer) = make_client();
+        let peer_handle = thread::spawn(move || {
+            let msg = recv::<PluginToHost>(&mut peer).unwrap();
+            match msg {
+                PluginToHost::Request(PluginRequest::RemoveEntity { handle }) => {
+                    assert_eq!(handle, Handle::new(7));
+                }
+                other => panic!("unexpected: {other:?}"),
+            }
+            send(&mut peer, &HostToPlugin::Response(PluginResponse::Bool(true))).unwrap();
+        });
+        assert!(api.remove_entity(Handle::new(7)));
+        peer_handle.join().unwrap();
+    }
 }

crates/ocs_plugin_api/src/ipc/protocol.rs

@@ -75,6 +75,12 @@ pub enum PluginRequest {
     PushOutput(String),
     PushError(String),
     AddEntity(EntityType),
+    /// Replace the existing entity carrying this entity's handle in place.
+    UpdateEntity(EntityType),
+    /// Delete the entity with `handle`.
+    RemoveEntity {
+        handle: Handle,
+    },
     BumpGeometry,
     ReadRecord {
         handle: Handle,

crates/ocs_plugin_api/src/ipc/server.rs

@@ -28,6 +28,8 @@ pub fn handle_plugin_request(
             PluginResponse::Ok
         }
         AddEntity(entity) => PluginResponse::Handle(host.add_entity(entity)),
+        UpdateEntity(entity) => PluginResponse::Bool(host.update_entity(entity)),
+        RemoveEntity { handle } => PluginResponse::Bool(host.remove_entity(handle)),
         BumpGeometry => {
             host.bump_geometry();
             PluginResponse::Ok

src/app/plugin_host.rs

@@ -73,6 +73,31 @@ impl<'a> HostSession<'a> {
         self.app.tabs[self.tab].scene.bump_geometry();
     }
 
+    /// Replace the entity carrying `entity`'s handle in place, refreshing the
+    /// scene's derived caches. Returns `false` when no entity has that handle.
+    pub fn update_entity(&mut self, entity: EntityType) -> bool {
+        let ok = self.app.tabs[self.tab].scene.update_entity(entity);
+        if ok {
+            self.publish_document_view();
+        }
+        ok
+    }
+
+    /// Delete the entity with `handle`, keeping the scene's render caches in
+    /// sync. Returns `false` when the entity is absent or on a locked layer
+    /// (which `erase_entities` refuses to remove).
+    pub fn remove_entity(&mut self, handle: Handle) -> bool {
+        if self.document().get_entity(handle).is_none() {
+            return false;
+        }
+        self.app.tabs[self.tab].scene.erase_entities(&[handle]);
+        let removed = self.document().get_entity(handle).is_none();
+        if removed {
+            self.publish_document_view();
+        }
+        removed
+    }
+
     // ── XDATA convenience ──────────────────────────────────────────────────
     // Plugins persist domain data as XDATA on plain entities so it round-trips
     // through DWG/DXF. These wrap the `acadrust::xdata` API keyed by entity
@@ -94,6 +119,7 @@ impl<'a> HostSession<'a> {
     pub fn write_record(&mut self, handle: Handle, record: ExtendedDataRecord) -> bool {
         let app = record.application_name.clone();
         self.ensure_app_id(&app);
+        let app_handle = self.document().app_ids.get(&app).map(|a| a.handle.value());
         let Some(entity) = self.document_mut().get_entity_mut(handle) else {
             return false;
         };
@@ -110,6 +136,13 @@ impl<'a> HostSession<'a> {
             xd.add_record(r);
         }
         xd.add_record(record);
+        // Drop stale verbatim EED for this app so the fresh record — not the
+        // pre-edit bytes captured on a prior read — wins on the next save.
+        // Otherwise a plugin's edit made after a save/reopen (which registered
+        // the app in `raw_dwg_eed`) would not persist.
+        if let Some(ah) = app_handle {
+            xd.raw_dwg_eed.retain(|(a, _)| *a != ah);
+        }
         self.publish_document_view();
         true
     }
@@ -117,6 +150,7 @@ impl<'a> HostSession<'a> {
     /// Remove the XDATA record for `app_name` from entity `handle`. Returns
     /// `true` when a record was actually removed.
     pub fn remove_record(&mut self, handle: Handle, app_name: &str) -> bool {
+        let app_handle = self.document().app_ids.get(app_name).map(|a| a.handle.value());
         let Some(entity) = self.document_mut().get_entity_mut(handle) else {
             return false;
         };
@@ -127,23 +161,36 @@ impl<'a> HostSession<'a> {
             .filter(|r| r.application_name != app_name)
             .cloned()
             .collect();
-        if kept.len() == xd.records().len() {
+        let removed_record = kept.len() != xd.records().len();
+        // Also drop verbatim EED for this app so the removal persists across a
+        // save (a record read back from DWG lives in `raw_dwg_eed`).
+        let removed_raw = app_handle
+            .map(|ah| xd.raw_dwg_eed.iter().any(|(a, _)| *a == ah))
+            .unwrap_or(false);
+        if !removed_record && !removed_raw {
             return false;
         }
         xd.clear();
         for r in kept {
             xd.add_record(r);
         }
+        if let Some(ah) = app_handle {
+            xd.raw_dwg_eed.retain(|(a, _)| *a != ah);
+        }
         self.publish_document_view();
         true
     }
 
     /// Register `name` in the APPID table if it is not already present, so XDATA
-    /// written under it survives a DWG/DXF round-trip.
+    /// written under it survives a DWG/DXF round-trip. The entry is given a real
+    /// handle — a null-handle APPID is written as handle 0, which the DWG EED
+    /// reference then can't resolve, so the XDATA would be dropped on reopen.
     fn ensure_app_id(&mut self, name: &str) {
         let doc = self.document_mut();
         if !doc.app_ids.contains(name) {
-            let _ = doc.app_ids.add(AppId::new(name));
+            let mut app = AppId::new(name);
+            app.handle = doc.allocate_handle();
+            let _ = doc.app_ids.add(app);
         }
     }
 
@@ -186,6 +233,12 @@ impl HostApi for HostSession<'_> {
     fn add_entity(&mut self, entity: EntityType) -> Handle {
         self.add_entity(entity)
     }
+    fn update_entity(&mut self, entity: EntityType) -> bool {
+        self.update_entity(entity)
+    }
+    fn remove_entity(&mut self, handle: Handle) -> bool {
+        self.remove_entity(handle)
+    }
     fn bump_geometry(&mut self) {
         self.bump_geometry()
     }
@@ -447,6 +500,54 @@ mod tests {
         );
     }
 
+    #[test]
+    fn update_entity_replaces_in_place_preserving_handle() {
+        let mut app = OpenCADStudio::new_for_test();
+        app.tabs[0].is_start = false;
+        let mut host = HostSession::new(&mut app, 0);
+        let h = host.add_entity(EntityType::Point(Point::at(acadrust::types::Vector3::new(
+            1.0, 1.0, 0.0,
+        ))));
+        let epoch_before = host.app.tabs[0].scene.geometry_epoch;
+
+        // Edit a snapshot copy (as a plugin would) and commit it.
+        let mut edited = host.document().get_entity(h).unwrap().clone();
+        edited.common_mut().layer = "PLUGIN_EDIT".to_string();
+        assert!(host.update_entity(edited));
+
+        // Same handle, edit applied, geometry re-tessellated.
+        let got = host.document().get_entity(h).expect("entity kept its handle");
+        assert_eq!(got.common().layer, "PLUGIN_EDIT");
+        assert_ne!(
+            host.app.tabs[0].scene.geometry_epoch, epoch_before,
+            "update should bump geometry"
+        );
+
+        // Updating an unknown handle fails and changes nothing.
+        let mut ghost = Point::new();
+        ghost.common.handle = Handle::new(999_999);
+        assert!(!host.update_entity(EntityType::Point(ghost)));
+    }
+
+    #[test]
+    fn remove_entity_deletes_and_clears_caches() {
+        let mut app = OpenCADStudio::new_for_test();
+        app.tabs[0].is_start = false;
+        let mut host = HostSession::new(&mut app, 0);
+        let h = host.add_entity(EntityType::Point(Point::at(acadrust::types::Vector3::new(
+            2.0, 2.0, 0.0,
+        ))));
+        assert!(host.document().get_entity(h).is_some());
+
+        assert!(host.remove_entity(h));
+        assert!(host.document().get_entity(h).is_none());
+        assert!(!host.app.tabs[0].scene.hatches.contains_key(&h));
+        assert!(!host.app.tabs[0].scene.meshes.contains_key(&h));
+
+        // Removing an already-gone handle reports false.
+        assert!(!host.remove_entity(h));
+    }
+
     /// A plugin command: second point commits a Point and ends.
     struct PlacePoint {
         got_first: bool,

src/scene/entity.rs

@@ -90,6 +90,88 @@ impl Scene {
         handle
     }
 
+    /// Replace the entity stored under `entity`'s handle with `entity`, keeping
+    /// its identity (handle + owning block), and refresh the derived
+    /// hatch/image/mesh caches so the edit is visible. Returns `false` when no
+    /// entity has that handle. This is the in-place counterpart to
+    /// [`add_entity`](Self::add_entity) used to commit a plugin's edit of an
+    /// existing entity.
+    pub fn update_entity(&mut self, mut entity: EntityType) -> bool {
+        let handle = entity.common().handle;
+        let Some(existing) = self.document.get_entity(handle) else {
+            return false;
+        };
+        // The caller edited a snapshot copy; keep the live entity in its block.
+        entity.common_mut().owner_handle = existing.common().owner_handle;
+
+        // Replacing (or becoming) a block entity forces a full block-cache
+        // rebuild; a plain entity only needs its own wires re-tessellated.
+        let affects_blocks = matches!(
+            existing,
+            EntityType::Insert(_) | EntityType::Block(_) | EntityType::BlockEnd(_)
+        ) || matches!(
+            &entity,
+            EntityType::Insert(_) | EntityType::Block(_) | EntityType::BlockEnd(_)
+        );
+
+        // Rebuild the derived-model seeds from the new entity (as add_entity).
+        let hatch_seed = if let EntityType::Hatch(dxf) = &entity {
+            let color = self.render_style(&entity).0;
+            Self::hatch_model_from_dxf(dxf, color)
+        } else if let EntityType::Solid(solid) = &entity {
+            let color = self.render_style(&entity).0;
+            Some(Self::solid_hatch_model(solid, color))
+        } else {
+            None
+        };
+        let image_seed = if let EntityType::RasterImage(img) = &entity {
+            ImageModel::from_raster_image(img)
+        } else {
+            None
+        };
+        let facet_res = self.document.header.facet_resolution;
+        let mesh_seed = if matches!(
+            &entity,
+            EntityType::Solid3D(_) | EntityType::Region(_) | EntityType::Body(_) | EntityType::Surface(_)
+        ) {
+            let color = self.render_style(&entity).0;
+            crate::entities::solid3d::tessellate_volume(&entity, color, facet_res)
+                .map(|m| offset_mesh_lod_set(m))
+        } else {
+            None
+        };
+
+        // Write the new entity into the live slot.
+        let Some(slot) = self.document.get_entity_mut(handle) else {
+            return false;
+        };
+        *slot = entity;
+
+        // Drop stale derived caches for this handle, then reseed for the new
+        // entity's type (which may differ from the old one).
+        self.hatches.remove(&handle);
+        self.images.remove(&handle);
+        self.meshes.remove(&handle);
+        self.solid_models.remove(&handle);
+        if let Some(model) = hatch_seed {
+            self.hatches.insert(handle, model);
+        }
+        if let Some(model) = image_seed {
+            self.images.insert(handle, model);
+        }
+        if let Some(model) = mesh_seed {
+            self.meshes.insert(handle, model);
+        }
+
+        self.mark_entity_dirty(handle);
+        if affects_blocks {
+            self.bump_geometry();
+        } else {
+            self.bump_geometry_no_blocks();
+        }
+        true
+    }
+
     /// Returns the RGBA color for the given layer name.
     pub fn layer_color(&self, layer: &str) -> [f32; 4] {
         let layer_entry = self.document.layers.get(layer);