descriptor: make add_file_descriptor_set transactional

.changes/unreleased/added-20260628-210901.yaml

@@ -0,0 +1,3 @@
+kind: Added
+body: '`DescriptorPool` now implements `Clone`. (#253)'
+time: 2026-06-28T21:09:01.000000000Z

.changes/unreleased/fixed-20260628-210900.yaml

@@ -0,0 +1,3 @@
+kind: Fixed
+body: '`DescriptorPool::add_file_descriptor_set` is now transactional: a failed add (e.g. an unresolvable type name) leaves the pool unchanged instead of retaining placeholder descriptors and name entries that made a corrected retry fail with `DuplicateName`. (#253)'
+time: 2026-06-28T21:09:00.000000000Z

buffa-descriptor/src/pool.rs

@@ -154,7 +154,7 @@ enum Definition {
 /// or accumulated via [`DescriptorPool::add_file_descriptor_set`]. Once built,
 /// the pool is immutable — descriptor handles are pool indices and all data
 /// is stored in flat `Vec`s.
-#[derive(Debug, Default)]
+#[derive(Clone, Debug, Default)]
 pub struct DescriptorPool {
     /// Original file descriptors, retained for raw access.
     files: Vec<FileDescriptorProto>,
@@ -224,11 +224,37 @@ impl DescriptorPool {
 
     /// Add the files in a `FileDescriptorSet` to the pool, registering and
     /// linking new types. Files already in the pool (by filename) are skipped.
+    /// If linking fails, the pool is left unchanged.
+    ///
+    /// Failure atomicity is implemented by staging the add against a clone of
+    /// the pool, so each call that introduces new files costs a deep copy of
+    /// the existing pool. Callers loading many files should batch them into a
+    /// single `FileDescriptorSet` rather than adding files one set at a time.
     ///
     /// # Errors
     ///
     /// Returns a [`PoolError`] on resolution failure.
     pub fn add_file_descriptor_set(&mut self, set: FileDescriptorSet) -> Result<(), PoolError> {
+        // Fast path for no-op re-adds without cloning the existing pool.
+        let has_new_files = set.file.iter().any(|f| {
+            f.name
+                .as_deref()
+                // MSRV: `Option::is_none_or` requires 1.82.
+                .map_or(true, |n| !self.file_by_name.contains_key(n))
+        });
+        if !has_new_files {
+            return Ok(());
+        }
+
+        // Build against a staged clone so fallible later passes cannot leave
+        // the live pool partially mutated.
+        let mut staged = self.clone();
+        staged.add_file_descriptor_set_staged(set)?;
+        *self = staged;
+        Ok(())
+    }
+
+    fn add_file_descriptor_set_staged(&mut self, set: FileDescriptorSet) -> Result<(), PoolError> {
         // Filter out files already present (idempotent re-add).
         let new_files: Vec<FileDescriptorProto> = set
             .file

buffa-descriptor/tests/pool_e2e.rs

@@ -206,6 +206,87 @@ fn idempotent_re_add() {
     );
 }
 
+#[test]
+fn failed_add_does_not_mutate_pool_and_retry_succeeds() {
+    use buffa_descriptor::generated::descriptor::field_descriptor_proto::{Label, Type};
+    use buffa_descriptor::generated::descriptor::{
+        DescriptorProto, FieldDescriptorProto, FileDescriptorProto, FileDescriptorSet,
+    };
+
+    let mut p = DescriptorPool::decode(FDS_BYTES).unwrap();
+    let baseline_message_count = p.messages().len();
+    let baseline_file_count = p.files().len();
+
+    let broken = FileDescriptorSet {
+        file: vec![FileDescriptorProto {
+            name: Some("poison.proto".into()),
+            package: Some("poison.test".into()),
+            syntax: Some("proto3".into()),
+            message_type: vec![DescriptorProto {
+                name: Some("RetryMe".into()),
+                field: vec![FieldDescriptorProto {
+                    name: Some("broken".into()),
+                    number: Some(1),
+                    label: Some(Label::LABEL_OPTIONAL),
+                    r#type: Some(Type::TYPE_MESSAGE),
+                    type_name: Some(".poison.test.Missing".into()),
+                    ..Default::default()
+                }],
+                ..Default::default()
+            }],
+            ..Default::default()
+        }],
+        ..Default::default()
+    };
+
+    let err = p.add_file_descriptor_set(broken).unwrap_err();
+    assert!(
+        err.to_string().contains("unresolved type name"),
+        "unexpected error: {err}"
+    );
+    assert!(
+        p.message_by_name("poison.test.RetryMe").is_none(),
+        "failed add must not register placeholder descriptors"
+    );
+    assert!(
+        p.file_by_name("poison.proto").is_none(),
+        "failed add must not record the file as loaded"
+    );
+    assert_eq!(p.messages().len(), baseline_message_count);
+    assert_eq!(p.files().len(), baseline_file_count);
+
+    let fixed = FileDescriptorSet {
+        file: vec![FileDescriptorProto {
+            name: Some("poison.proto".into()),
+            package: Some("poison.test".into()),
+            syntax: Some("proto3".into()),
+            message_type: vec![DescriptorProto {
+                name: Some("RetryMe".into()),
+                field: vec![FieldDescriptorProto {
+                    name: Some("ok".into()),
+                    number: Some(1),
+                    label: Some(Label::LABEL_OPTIONAL),
+                    r#type: Some(Type::TYPE_STRING),
+                    ..Default::default()
+                }],
+                ..Default::default()
+            }],
+            ..Default::default()
+        }],
+        ..Default::default()
+    };
+
+    p.add_file_descriptor_set(fixed)
+        .expect("retry with a valid descriptor set succeeds");
+
+    let retry = p
+        .message_by_name("poison.test.RetryMe")
+        .expect("message loads after the corrected retry");
+    assert_eq!(retry.field(1).unwrap().name(), "ok");
+    assert_eq!(p.messages().len(), baseline_message_count + 1);
+    assert_eq!(p.files().len(), baseline_file_count + 1);
+}
+
 #[test]
 fn service_descriptor_links() {
     let p = pool();