fix(config): Apply shared extends files only once

crates/contrib/schematic_macros/src/common/field.rs

@@ -7,7 +7,7 @@ use syn::{Attribute, Expr, ExprPath, Field as NativeField, Type};
 
 use crate::{
     common::{FieldValue, PartialAttr, TypeInfo, extract_inner_type, macros::ContainerSerdeArgs},
-    utils::{extract_common_attrs, format_case, preserve_str_literal},
+    utils::{extract_common_attrs, format_case, parse_default},
 };
 
 // #[serde()]
@@ -42,7 +42,7 @@ pub struct FieldArgs {
     pub exclude: bool,
 
     // config
-    #[darling(with = preserve_str_literal, map = "Some")]
+    #[darling(with = parse_default)]
     pub default: Option<Expr>,
     #[cfg(feature = "env")]
     pub env: Option<String>,
@@ -58,6 +58,7 @@ pub struct FieldArgs {
 
     // serde
     pub alias: Option<String>,
+    pub deserialize_with: Option<String>,
     pub flatten: bool,
     pub rename: Option<String>,
     pub skip: bool,
@@ -82,7 +83,21 @@ pub struct Field<'l> {
 
 impl Field<'_> {
     pub fn from(field: &NativeField) -> Field<'_> {
-        let args = FieldArgs::from_attributes(&field.attrs).unwrap_or_default();
+        // Errors are fatal rather than defaulted: a mistyped or unknown key in
+        // schematic's own `#[setting]` / `#[schema]` attribute would otherwise
+        // silently discard every other key on the same field.
+        //
+        // `#[serde]` stays lenient. It is serde's namespace, not schematic's,
+        // and carries keys (`with`, `bound`, `default = "path"`) that
+        // `FieldSerdeArgs` deliberately does not model.
+        let args = FieldArgs::from_attributes(&field.attrs).unwrap_or_else(|error| {
+            let name = field
+                .ident
+                .as_ref()
+                .map_or_else(|| "<unnamed>".to_owned(), ToString::to_string);
+
+            panic!("Invalid `#[setting]` or `#[schema]` attribute on field `{name}`: {error}");
+        });
         let serde_args = FieldSerdeArgs::from_attributes(&field.attrs).unwrap_or_default();
 
         let partial_via_ty = args.partial_via.as_ref().map(|ep| {
@@ -305,6 +320,12 @@ impl Field<'_> {
 
             if self.args.skip_deserializing || self.serde_args.skip_deserializing {
                 meta.push(quote! { skip_deserializing });
+            } else if let Some(deserialize_with) = &self.args.deserialize_with {
+                // `deserialize_with` disables serde's implicit "missing
+                // `Option` field is `None`" handling, so the partial field
+                // needs an explicit default to stay optional.
+                meta.push(quote! { default });
+                meta.push(quote! { deserialize_with = #deserialize_with });
             }
         }
 

crates/contrib/schematic_macros/src/utils.rs

@@ -39,6 +39,20 @@ pub fn preserve_str_literal(meta: &Meta) -> darling::Result<Expr> {
     }
 }
 
+/// Parse a `default` argument, which accepts both `default` and `default =
+/// <expr>`.
+///
+/// The bare form states that the field falls back to its type's `Default` impl,
+/// which is what the generated code emits when no default expression is
+/// present, so it parses to `None`.
+pub fn parse_default(meta: &Meta) -> darling::Result<Option<Expr>> {
+    match meta {
+        Meta::Path(_) => Ok(None),
+        Meta::List(_) => Err(darling::Error::unsupported_format("list").with_span(meta)),
+        Meta::NameValue(nv) => Ok(Some(nv.value.clone())),
+    }
+}
+
 pub fn get_meta_path(meta: &Meta) -> &Path {
     match meta {
         Meta::Path(path) => path,

crates/jp_config/src/assistant.rs

@@ -21,7 +21,7 @@ use crate::{
         tool_choice::ToolChoice,
     },
     delta::{PartialConfigDelta, delta_opt, delta_opt_partial},
-    fill::FillDefaults,
+    fill::{FillDefaults, fill_opt},
     internal::merge::{string_with_strategy, vec_with_strategy},
     model::{ModelConfig, PartialModelConfig},
     partial::{ToPartial, partial_opt, partial_opts},
@@ -98,6 +98,16 @@ impl AssignKeyValue for PartialAssistantConfig {
             "" => kv.try_merge_object(self)?,
             "name" => self.name = kv.try_some_string()?,
             "system_prompt" => self.system_prompt = kv.try_some_object_or_from_str()?,
+            // Nested keys (`system_prompt.value`, `.strategy`, `.separator`,
+            // `.discard_when_merged`, `.dedup`) address the merge metadata, so
+            // an absent value starts as `Merged` rather than the plain-string
+            // default, which has nowhere to put them.
+            _ if kv.p("system_prompt") => self
+                .system_prompt
+                .get_or_insert_with(|| {
+                    PartialMergeableString::Merged(PartialMergedString::default())
+                })
+                .assign(kv)?,
             _ if kv.p("instructions") => kv.try_vec_of_nested(self.instructions.as_mut())?,
             _ if kv.p("system_prompt_sections") => {
                 kv.try_vec_of_nested(self.system_prompt_sections.as_mut())?;
@@ -139,7 +149,10 @@ impl FillDefaults for PartialAssistantConfig {
     fn fill_from(self, defaults: Self) -> Self {
         Self {
             name: self.name.or(defaults.name),
-            system_prompt: self.system_prompt.or(defaults.system_prompt),
+            // `fill_opt` rather than `or`: a metadata-only prompt (from e.g.
+            // `--cfg assistant.system_prompt.dedup=false`) still needs the
+            // default's value filled in.
+            system_prompt: fill_opt(self.system_prompt, defaults.system_prompt),
             system_prompt_sections: self
                 .system_prompt_sections
                 .fill_from(defaults.system_prompt_sections),
@@ -210,6 +223,7 @@ fn default_system_prompt(_: &()) -> TransformResult<Option<PartialMergeableStrin
         strategy: None,
         separator: None,
         discard_when_merged: Some(true),
+        dedup: None,
     })))
 }
 

crates/jp_config/src/assistant_tests.rs

@@ -180,6 +180,7 @@ fn test_assistant_config_instructions() {
             strategy: None,
             separator: None,
             discard_when_merged: None,
+            dedup: None,
         }))
     );
 
@@ -194,6 +195,7 @@ fn test_assistant_config_instructions() {
             strategy: Some(MergedStringStrategy::Append),
             separator: None,
             discard_when_merged: None,
+            dedup: None,
         }))
     );
 
@@ -210,6 +212,7 @@ fn test_assistant_config_instructions() {
             strategy: Some(MergedStringStrategy::Append),
             separator: Some(MergedStringSeparator::Space),
             discard_when_merged: None,
+            dedup: None,
         }))
     );
 }
@@ -424,6 +427,7 @@ fn test_assistant_config_deserialize() {
                     strategy: Some(MergedStringStrategy::Append),
                     separator: Some(MergedStringSeparator::Paragraph),
                     discard_when_merged: None,
+                    dedup: None,
                 })),
                 instructions: MergedVec {
                     value: vec![

crates/jp_config/src/internal/merge.rs

@@ -1,9 +1,11 @@
 //! Internal merge strategies.
 
 mod map;
+mod plain_vec;
 mod string;
 mod vec;
 
 pub use map::map_with_strategy;
+pub use plain_vec::append_vec_dedup;
 pub use string::string_with_strategy;
 pub use vec::vec_with_strategy;

crates/jp_config/src/internal/merge/plain_vec.rs

@@ -0,0 +1,47 @@
+//! Merge strategies for plain `Vec` fields.
+//!
+//! These operate on `Vec<T>` directly, unlike [`vec_with_strategy`], which
+//! reads its strategy from a [`MergeableVec`] wrapper.
+//!
+//! [`MergeableVec`]: crate::types::vec::MergeableVec
+//! [`vec_with_strategy`]: super::vec_with_strategy
+
+use schematic::MergeResult;
+
+/// Append `next` to `prev`, dropping items already present.
+///
+/// Comparison uses `PartialEq` and the first occurrence wins, so the result
+/// keeps `prev`'s order with `next`'s new items appended.
+///
+/// Only combining merges reach this function: schematic's `merge_setting`
+/// invokes a merge strategy only when both layers supply a value, so a list
+/// supplied by a single layer is stored as written, duplicates included.
+/// That is the same rule `replace` follows on [`MergeableVec`] — repeated
+/// items within one source are the author's own data, not something a merge of
+/// two sources should rewrite.
+///
+/// Deduplicating here rather than through a `transform` is deliberate:
+/// transforms run in [`PartialConfig::finalize`], which JP's config pipeline
+/// never calls — it merges layers with `load_partial` and resolves them with
+/// `AppConfig::from_partial_with_defaults`.
+///
+/// [`MergeableVec`]: crate::types::vec::MergeableVec
+/// [`PartialConfig::finalize`]: schematic::PartialConfig::finalize
+#[expect(clippy::unnecessary_wraps)]
+pub fn append_vec_dedup<T: PartialEq, C>(
+    mut prev: Vec<T>,
+    next: Vec<T>,
+    _: &C,
+) -> MergeResult<Vec<T>> {
+    for item in next {
+        if !prev.contains(&item) {
+            prev.push(item);
+        }
+    }
+
+    Ok(Some(prev))
+}
+
+#[cfg(test)]
+#[path = "plain_vec_tests.rs"]
+mod tests;

crates/jp_config/src/internal/merge/plain_vec_tests.rs

@@ -0,0 +1,43 @@
+use test_log::test;
+
+use super::*;
+
+#[test]
+fn appends_new_items() {
+    let result = append_vec_dedup(vec![1, 2], vec![3, 4], &())
+        .unwrap()
+        .unwrap();
+
+    assert_eq!(result, vec![1, 2, 3, 4]);
+}
+
+#[test]
+fn drops_items_already_present() {
+    // Two config layers naming the same directory contribute it once, which is
+    // what `config_load_paths` and `beta_headers` need: the resolved list is
+    // searched (respectively sent) in order, and a repeat is pure noise.
+    let result = append_vec_dedup(vec!["a", "b"], vec!["b", "c"], &())
+        .unwrap()
+        .unwrap();
+
+    assert_eq!(result, vec!["a", "b", "c"]);
+}
+
+#[test]
+fn keeps_first_occurrence_order() {
+    let result = append_vec_dedup(vec![3, 1], vec![2, 1, 3], &())
+        .unwrap()
+        .unwrap();
+
+    assert_eq!(result, vec![3, 1, 2]);
+}
+
+#[test]
+fn collapses_repeats_inside_the_incoming_layer() {
+    // Only reachable when two layers combine — a list supplied by a single
+    // layer never reaches this function, so its own repeats are kept. See
+    // `test_load_partial_at_path_keeps_repeats_from_a_single_file`.
+    let result = append_vec_dedup(vec![1], vec![2, 2], &()).unwrap().unwrap();
+
+    assert_eq!(result, vec![1, 2]);
+}

crates/jp_config/src/internal/merge/string.rs

@@ -12,9 +12,16 @@ pub fn string_with_strategy(
     next: PartialMergeableString,
     _context: &(),
 ) -> MergeResult<PartialMergeableString> {
+    // Resolve the explicit dedup opinion: next's choice wins, then inherit from
+    // prev. `None` means neither side expressed one.
+    //
+    // A discarded prev still contributes dedup when next has no opinion, but
+    // NOT when next explicitly sets it.
+    let dedup = dedup_flag(&next).or_else(|| dedup_flag(&prev));
+
     // If prev is default, replace regardless of strategy.
     if prev.discard_when_merged() {
-        return Ok(Some(next));
+        return Ok(Some(with_dedup_flag(next, dedup)));
     }
 
     let prev_value = match prev {
@@ -32,13 +39,25 @@ pub fn string_with_strategy(
         }
     };
 
+    // Skip an append or prepend whose value is already present, unless a config
+    // explicitly opts out. Applying the same config source twice — through an
+    // `extends` diamond, or by re-supplying `--cfg` on a conversation that
+    // already merged it — must not duplicate its contribution.
+    let dedup_active = dedup.unwrap_or(true);
+
+    // An unstated strategy means `append`, matching `MergedStringStrategy`'s
+    // default. Only the computation resolves it; the stored `strategy` keeps
+    // the unstated form so later merges can still express their own.
+    let resolved_strategy = strategy.unwrap_or_default();
+
     let sep = separator.as_ref().map_or("", |sep| sep.as_str());
     let value = match (prev_value, next_value) {
-        (_, n) if strategy == Some(MergedStringStrategy::Replace) => n,
-        (Some(p), Some(n)) if strategy == Some(MergedStringStrategy::Append) => {
+        (_, n) if resolved_strategy == MergedStringStrategy::Replace => n,
+        (Some(p), Some(n)) if dedup_active && contains_block(&p, &n, sep) => Some(p),
+        (Some(p), Some(n)) if resolved_strategy == MergedStringStrategy::Append => {
             Some(format!("{p}{sep}{n}"))
         }
-        (Some(p), Some(n)) if strategy == Some(MergedStringStrategy::Prepend) => {
+        (Some(p), Some(n)) if resolved_strategy == MergedStringStrategy::Prepend => {
             Some(format!("{n}{sep}{p}"))
         }
         (Some(p), None) => Some(p),
@@ -47,17 +66,83 @@ pub fn string_with_strategy(
     };
 
     Ok(Some(if next_is_replace {
-        PartialMergeableString::String(value.unwrap_or_default())
+        with_dedup_flag(
+            PartialMergeableString::String(value.unwrap_or_default()),
+            dedup,
+        )
     } else {
         PartialMergeableString::Merged(PartialMergedString {
             value,
             strategy,
             separator,
             discard_when_merged,
+            dedup,
         })
     }))
 }
 
+/// The explicit `dedup` opinion carried by a value, if any.
+const fn dedup_flag(v: &PartialMergeableString) -> Option<bool> {
+    match v {
+        PartialMergeableString::String(_) => None,
+        PartialMergeableString::Merged(m) => m.dedup,
+    }
+}
+
+/// Attach an explicit `dedup` opinion, wrapping a plain string in `Merged` with
+/// a `replace` strategy so the flag survives the next merge.
+///
+/// A `None` opinion is left implicit and the value's shape is unchanged.
+fn with_dedup_flag(v: PartialMergeableString, dedup: Option<bool>) -> PartialMergeableString {
+    if dedup.is_none() {
+        return v;
+    }
+
+    match v {
+        PartialMergeableString::String(value) => {
+            PartialMergeableString::Merged(PartialMergedString {
+                value: Some(value),
+                strategy: Some(MergedStringStrategy::Replace),
+                separator: None,
+                discard_when_merged: None,
+                dedup,
+            })
+        }
+        PartialMergeableString::Merged(mut m) => {
+            m.dedup = dedup;
+            PartialMergeableString::Merged(m)
+        }
+    }
+}
+
+/// Whether `needle` already appears in `haystack` as a whole
+/// `separator`-delimited block.
+///
+/// A match must start at the beginning of `haystack` or right after a
+/// separator, and end at the end of `haystack` or right before one.
+/// Anchoring on both sides keeps a value that merely occurs inside a larger
+/// block from counting as present.
+///
+/// An empty separator leaves no boundaries to anchor on, so only an exact match
+/// of the whole string counts.
+fn contains_block(haystack: &str, needle: &str, separator: &str) -> bool {
+    if needle.is_empty() || haystack == needle {
+        return true;
+    }
+
+    if separator.is_empty() {
+        return false;
+    }
+
+    haystack.match_indices(needle).any(|(index, matched)| {
+        let end = index + matched.len();
+        let starts_block = index == 0 || haystack[..index].ends_with(separator);
+        let ends_block = end == haystack.len() || haystack[end..].starts_with(separator);
+
+        starts_block && ends_block
+    })
+}
+
 #[cfg(test)]
 #[path = "string_tests.rs"]
 mod tests;