fresh-editor 0.4.2

//! Runtime configuration I/O operations.
//!
//! This module contains system directory detection and config loading utilities
//! that require runtime dependencies (dirs, tracing).
//! These are separated from config.rs to allow schema-only builds.

use crate::config::{Config, ConfigError};
use crate::partial_config::{Merge, PartialConfig, SessionConfig};
use serde_json::Value;
use std::path::{Path, PathBuf};

// ============================================================================
// JSON Utilities
// ============================================================================

/// Recursively strip null values and empty objects from a JSON value.
/// This ensures that config layer files only contain the actual overridden values,
/// not null placeholders for inherited fields.
fn strip_nulls(value: Value) -> Option<Value> {
    match value {
        Value::Null => None,
        Value::Object(map) => {
            let filtered: serde_json::Map<String, Value> = map
                .into_iter()
                .filter_map(|(k, v)| strip_nulls(v).map(|v| (k, v)))
                .collect();
            if filtered.is_empty() {
                None
            } else {
                Some(Value::Object(filtered))
            }
        }
        Value::Array(arr) => {
            let filtered: Vec<Value> = arr.into_iter().filter_map(strip_nulls).collect();
            Some(Value::Array(filtered))
        }
        other => Some(other),
    }
}

/// Recursively strip default values (empty strings, empty arrays) from a JSON value.
/// This ensures that fields with default serde values don't get saved to config files.
fn strip_empty_defaults(value: Value) -> Option<Value> {
    match value {
        Value::Null => None,
        Value::String(s) if s.is_empty() => None,
        Value::Array(arr) if arr.is_empty() => None,
        Value::Object(map) => {
            let filtered: serde_json::Map<String, Value> = map
                .into_iter()
                .filter_map(|(k, v)| strip_empty_defaults(v).map(|v| (k, v)))
                .collect();
            if filtered.is_empty() {
                None
            } else {
                Some(Value::Object(filtered))
            }
        }
        Value::Array(arr) => {
            let filtered: Vec<Value> = arr.into_iter().filter_map(strip_empty_defaults).collect();
            if filtered.is_empty() {
                None
            } else {
                Some(Value::Array(filtered))
            }
        }
        other => Some(other),
    }
}

/// Set a value at a JSON pointer path, creating intermediate objects as needed.
/// The pointer should be in JSON Pointer format (e.g., "/editor/tab_size").
fn set_json_pointer(root: &mut Value, pointer: &str, value: Value) {
    if pointer.is_empty() || pointer == "/" {
        *root = value;
        return;
    }

    let parts: Vec<&str> = pointer.trim_start_matches('/').split('/').collect();

    let mut current = root;
    for (i, part) in parts.iter().enumerate() {
        if i == parts.len() - 1 {
            // Last part - set the value
            if let Value::Object(map) = current {
                map.insert(part.to_string(), value);
            }
            return;
        }

        // Intermediate part - ensure it exists as an object
        if let Value::Object(map) = current {
            if !map.contains_key(*part) {
                map.insert(part.to_string(), Value::Object(Default::default()));
            }
            current = map.get_mut(*part).unwrap();
        } else {
            return; // Can't traverse non-object
        }
    }
}

/// Remove a value at a JSON pointer path.
pub(crate) fn remove_json_pointer(root: &mut Value, pointer: &str) {
    if pointer.is_empty() || pointer == "/" {
        return;
    }

    let parts: Vec<&str> = pointer.trim_start_matches('/').split('/').collect();

    let mut current = root;
    for (i, part) in parts.iter().enumerate() {
        if i == parts.len() - 1 {
            // Last part - remove the key
            if let Value::Object(map) = current {
                map.remove(*part);
            }
            return;
        }

        // Intermediate part - traverse
        if let Value::Object(map) = current {
            if let Some(next) = map.get_mut(*part) {
                current = next;
            } else {
                return; // Path doesn't exist
            }
        } else {
            return; // Can't traverse non-object
        }
    }
}

/// Find all JSON pointer paths where two values differ.
/// Returns leaf paths that have different values between old and new.
fn find_changed_paths(old: &Value, new: &Value) -> std::collections::HashSet<String> {
    let mut changed = std::collections::HashSet::new();
    find_changed_paths_recursive(old, new, String::new(), &mut changed);
    changed
}

fn find_changed_paths_recursive(
    old: &Value,
    new: &Value,
    prefix: String,
    changed: &mut std::collections::HashSet<String>,
) {
    match (old, new) {
        (Value::Object(old_map), Value::Object(new_map)) => {
            // Check all keys in both objects
            let all_keys: std::collections::HashSet<_> =
                old_map.keys().chain(new_map.keys()).collect();
            for key in all_keys {
                let path = if prefix.is_empty() {
                    format!("/{}", key)
                } else {
                    format!("{}/{}", prefix, key)
                };
                let old_val = old_map.get(key).unwrap_or(&Value::Null);
                let new_val = new_map.get(key).unwrap_or(&Value::Null);
                find_changed_paths_recursive(old_val, new_val, path, changed);
            }
        }
        (old_val, new_val) if old_val != new_val => {
            // Leaf values differ - mark as changed
            if !prefix.is_empty() {
                changed.insert(prefix);
            }
        }
        _ => {} // Values are equal, no change
    }
}

/// Strip defaults/nulls from `value`, ensure the parent directory exists, and
/// write to `path`.
///
/// When `path` already holds a JSON object, the new values are applied onto the
/// existing file *text* through the JSONC CST so the user's comments,
/// formatting, and untouched inline annotations survive the write. Only when
/// there is no usable existing file (new file, unparseable, or a non-object
/// root) do we fall back to pretty-printing from scratch.
fn write_clean_value_to_path(path: &Path, value: Value) -> Result<(), ConfigError> {
    if let Some(parent_dir) = path.parent() {
        std::fs::create_dir_all(parent_dir)
            .map_err(|e| ConfigError::IoError(format!("{}: {}", parent_dir.display(), e)))?;
    }
    let stripped = strip_nulls(value).unwrap_or(Value::Object(Default::default()));
    let clean = strip_empty_defaults(stripped).unwrap_or(Value::Object(Default::default()));

    let output = render_config_text(path, &clean)?;
    std::fs::write(path, output)
        .map_err(|e| ConfigError::IoError(format!("{}: {}", path.display(), e)))?;
    Ok(())
}

/// Produce the on-disk text for `clean`, preserving comments from any existing
/// file at `path` when possible (see `write_clean_value_to_path`).
fn render_config_text(path: &Path, clean: &Value) -> Result<String, ConfigError> {
    if let (Value::Object(_), Ok(existing)) = (clean, std::fs::read_to_string(path)) {
        if let Some(text) = reconcile_preserving_comments(&existing, clean) {
            return Ok(text);
        }
    }
    serde_json::to_string_pretty(clean).map_err(|e| ConfigError::SerializeError(e.to_string()))
}

/// Apply `clean` (a JSON object) onto the existing JSONC `existing` text via the
/// CST, returning the edited text. Returns `None` when `existing` can't be
/// parsed or its root isn't an object, so the caller can fall back to a fresh
/// pretty-print.
fn reconcile_preserving_comments(existing: &str, clean: &Value) -> Option<String> {
    use jsonc_parser::cst::CstRootNode;

    let Value::Object(target) = clean else {
        return None;
    };
    let root = CstRootNode::parse(existing, &Default::default()).ok()?;
    // Bail out (pretty-print fresh) if the document root isn't a JSON object —
    // editing a scalar/array root in place isn't meaningful for our configs.
    root.value()?.as_object()?;
    let obj = root.object_value_or_set();
    reconcile_cst_object(&obj, target);
    Some(root.to_string())
}

/// Recursively reconcile a CST object node so it matches `target`, touching as
/// little as possible: unchanged properties are left exactly as written (commas,
/// whitespace, and inline comments intact), nested objects recurse, removed keys
/// are deleted, and new keys are appended.
fn reconcile_cst_object(
    obj: &jsonc_parser::cst::CstObject,
    target: &serde_json::Map<String, Value>,
) {
    use jsonc_parser::cst::CstObjectProp;

    let prop_name = |prop: &CstObjectProp| -> Option<String> {
        prop.name().and_then(|n| n.decoded_value().ok())
    };

    // Remove properties that are no longer present in the target.
    for prop in obj.properties() {
        match prop_name(&prop) {
            Some(name) if target.contains_key(&name) => {}
            _ => prop.remove(),
        }
    }

    // Insert or update properties from the target.
    for (key, new_value) in target {
        match obj.get(key) {
            Some(prop) => {
                // Skip writes when the value is already equal so existing
                // formatting/comments on that value are preserved verbatim.
                let current = prop.value().and_then(|n| n.to_serde_value());
                if current.as_ref() == Some(new_value) {
                    continue;
                }
                match (new_value, prop.value().and_then(|n| n.as_object())) {
                    // Both sides are objects: recurse to keep inner comments.
                    (Value::Object(child_target), Some(child_obj)) => {
                        reconcile_cst_object(&child_obj, child_target);
                    }
                    // Otherwise replace the value wholesale.
                    _ => prop.set_value(json_value_to_cst_input(new_value)),
                }
            }
            None => {
                obj.append(key, json_value_to_cst_input(new_value));
            }
        }
    }
}

/// Convert a `serde_json::Value` into the CST's input-value representation used
/// for inserts and replacements.
fn json_value_to_cst_input(value: &Value) -> jsonc_parser::cst::CstInputValue {
    use jsonc_parser::cst::CstInputValue;
    match value {
        Value::Null => CstInputValue::Null,
        Value::Bool(b) => CstInputValue::Bool(*b),
        Value::Number(n) => CstInputValue::Number(n.to_string()),
        Value::String(s) => CstInputValue::String(s.clone()),
        Value::Array(arr) => {
            CstInputValue::Array(arr.iter().map(json_value_to_cst_input).collect())
        }
        Value::Object(map) => CstInputValue::Object(
            map.iter()
                .map(|(k, v)| (k.clone(), json_value_to_cst_input(v)))
                .collect(),
        ),
    }
}

/// Read an existing config file as raw JSON, returning an empty object when the file
/// is absent or empty.
///
/// **Errors** (rather than returning an empty object) when the file exists with
/// content that can't be parsed. This is used on the read-modify-write save
/// path: silently treating an unparseable file as empty would make the
/// subsequent write overwrite the user's entire config with just the new
/// edits. Failing here instead leaves the file untouched and lets the caller
/// surface the parse error.
fn read_existing_json(path: &Path) -> Result<Value, ConfigError> {
    if !path.exists() {
        return Ok(Value::Object(Default::default()));
    }
    let content = std::fs::read_to_string(path)
        .map_err(|e| ConfigError::IoError(format!("{}: {}", path.display(), e)))?;
    if content.trim().is_empty() {
        return Ok(Value::Object(Default::default()));
    }
    crate::config::parse_config_jsonc(&content)
        .map_err(|e| ConfigError::ParseError(format!("{}: {}", path.display(), e)))
}

// ============================================================================
// Configuration Migration System
// ============================================================================

/// Current config schema version.
/// Increment this when making breaking changes to config structure.
pub const CURRENT_CONFIG_VERSION: u32 = 2;

/// Apply all necessary migrations to bring a config JSON to the current version.
pub fn migrate_config(mut value: Value) -> Result<Value, ConfigError> {
    let version = value.get("version").and_then(|v| v.as_u64()).unwrap_or(0) as u32;

    // Apply migrations sequentially
    if version < 1 {
        value = migrate_v0_to_v1(value)?;
    }
    if version < 2 {
        value = migrate_v1_to_v2(value)?;
    }

    Ok(value)
}

/// Migration from v0 (implicit/missing version) to v1.
/// This is the initial migration that establishes the version field.
fn migrate_v0_to_v1(mut value: Value) -> Result<Value, ConfigError> {
    if let Value::Object(ref mut map) = value {
        // Set version to 1
        map.insert("version".to_string(), Value::Number(1.into()));

        // Example: rename camelCase keys to snake_case if they exist
        if let Some(Value::Object(ref mut editor_map)) = map.get_mut("editor") {
            // tabSize -> tab_size (hypothetical legacy format)
            if let Some(val) = editor_map.remove("tabSize") {
                editor_map.entry("tab_size").or_insert(val);
            }
            // lineNumbers -> line_numbers
            if let Some(val) = editor_map.remove("lineNumbers") {
                editor_map.entry("line_numbers").or_insert(val);
            }
        }
    }
    Ok(value)
}

/// Migration from v1 to v2.
///
/// Injects `"{remote}"` at the front of `editor.status_bar.left` when
/// the user has customized the list and the element is not already
/// present. Users who never overrode the default get the element via
/// `default_status_bar_left` at resolve time — we intentionally skip
/// inserting a `status_bar` object here so those users stay on the
/// rolling default if future versions reorder or rename elements.
fn migrate_v1_to_v2(mut value: Value) -> Result<Value, ConfigError> {
    if let Value::Object(ref mut map) = value {
        map.insert("version".to_string(), Value::Number(2.into()));

        let left = map
            .get_mut("editor")
            .and_then(|editor| editor.as_object_mut())
            .and_then(|editor| editor.get_mut("status_bar"))
            .and_then(|status_bar| status_bar.as_object_mut())
            .and_then(|status_bar| status_bar.get_mut("left"))
            .and_then(|left| left.as_array_mut());

        if let Some(left) = left {
            let already_present = left.iter().any(|v| v.as_str() == Some("{remote}"));
            if !already_present {
                left.insert(0, Value::String("{remote}".to_string()));
            }
        }
    }
    Ok(value)
}

/// Represents a configuration layer in the 4-level hierarchy.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ConfigLayer {
    /// Hardcoded defaults embedded in binary (lowest precedence)
    System,
    /// User-global settings (~/.config/fresh/config.json)
    User,
    /// Project-local settings ($PROJECT_ROOT/.fresh/config.json)
    Project,
    /// Runtime/volatile session state (highest precedence)
    Session,
}

impl ConfigLayer {
    /// Get the precedence level (higher = takes priority)
    pub fn precedence(self) -> u8 {
        match self {
            Self::System => 0,
            Self::User => 1,
            Self::Project => 2,
            Self::Session => 3,
        }
    }
}

/// Manages loading and merging of all configuration layers.
///
/// Resolution order: System → User → Project → Session
/// Higher precedence layers override lower precedence layers.
pub struct ConfigResolver {
    dir_context: DirectoryContext,
    working_dir: PathBuf,
}

impl ConfigResolver {
    /// Create a new ConfigResolver for a working directory.
    pub fn new(dir_context: DirectoryContext, working_dir: PathBuf) -> Self {
        Self {
            dir_context,
            working_dir,
        }
    }

    /// Load all layers and merge them into a resolved Config.
    ///
    /// Layers are merged from highest to lowest precedence:
    /// Session > Project > UserPlatform > User > System
    ///
    /// Each layer fills in values missing from higher precedence layers.
    pub fn resolve(&self) -> Result<Config, ConfigError> {
        // Start with highest precedence layer (Session)
        let mut merged = self.load_session_layer()?.unwrap_or_default();

        // Merge in Project layer (fills missing values)
        if let Some(project_partial) = self.load_project_layer()? {
            tracing::debug!("Loaded project config layer");
            merged.merge_from(&project_partial);
        }

        // Merge in User Platform layer (e.g., config_linux.json)
        if let Some(platform_partial) = self.load_user_platform_layer()? {
            tracing::debug!("Loaded user platform config layer");
            merged.merge_from(&platform_partial);
        }

        // Merge in User layer (fills remaining missing values)
        if let Some(user_partial) = self.load_user_layer()? {
            tracing::debug!("Loaded user config layer");
            merged.merge_from(&user_partial);
        }

        // Resolve to concrete Config (applies system defaults for any remaining None values)
        Ok(merged.resolve())
    }

    /// Get the path to user config file.
    pub fn user_config_path(&self) -> PathBuf {
        self.dir_context.config_path()
    }

    /// Get the path to project config file.
    /// Checks new location first (.fresh/config.json), falls back to legacy (config.json).
    pub fn project_config_path(&self) -> PathBuf {
        let new_path = self.working_dir.join(".fresh").join("config.json");
        if new_path.exists() {
            return new_path;
        }
        // Fall back to legacy location for backward compatibility
        let legacy_path = self.working_dir.join("config.json");
        if legacy_path.exists() {
            return legacy_path;
        }
        // Return new path as default for new projects
        new_path
    }

    /// Get the preferred path for writing project config (new location).
    pub fn project_config_write_path(&self) -> PathBuf {
        self.working_dir.join(".fresh").join("config.json")
    }

    /// Get the path to session config file.
    pub fn session_config_path(&self) -> PathBuf {
        self.working_dir.join(".fresh").join("session.json")
    }

    /// Get the platform-specific config filename.
    fn platform_config_filename() -> Option<&'static str> {
        if cfg!(target_os = "linux") {
            Some("config_linux.json")
        } else if cfg!(target_os = "macos") {
            Some("config_macos.json")
        } else if cfg!(target_os = "windows") {
            Some("config_windows.json")
        } else {
            None
        }
    }

    /// Get the path to platform-specific user config file.
    pub fn user_platform_config_path(&self) -> Option<PathBuf> {
        Self::platform_config_filename().map(|filename| self.dir_context.config_dir.join(filename))
    }

    /// Load the user layer from disk.
    pub fn load_user_layer(&self) -> Result<Option<PartialConfig>, ConfigError> {
        self.load_layer_from_path(&self.user_config_path())
    }

    /// Load the platform-specific user layer from disk.
    pub fn load_user_platform_layer(&self) -> Result<Option<PartialConfig>, ConfigError> {
        if let Some(path) = self.user_platform_config_path() {
            self.load_layer_from_path(&path)
        } else {
            Ok(None)
        }
    }

    /// Load the project layer from disk.
    pub fn load_project_layer(&self) -> Result<Option<PartialConfig>, ConfigError> {
        self.load_layer_from_path(&self.project_config_path())
    }

    /// Load the session layer from disk.
    pub fn load_session_layer(&self) -> Result<Option<PartialConfig>, ConfigError> {
        self.load_layer_from_path(&self.session_config_path())
    }

    /// Load a layer from a specific path, applying migrations if needed.
    fn load_layer_from_path(&self, path: &Path) -> Result<Option<PartialConfig>, ConfigError> {
        if !path.exists() {
            return Ok(None);
        }

        let content = std::fs::read_to_string(path)
            .map_err(|e| ConfigError::IoError(format!("{}: {}", path.display(), e)))?;

        // Parse as raw JSONC first (comments/trailing commas tolerated)
        let value: Value = crate::config::parse_config_jsonc(&content)
            .map_err(|e| ConfigError::ParseError(format!("{}: {}", path.display(), e)))?;

        // Apply migrations
        let migrated = migrate_config(value)?;

        // Now deserialize to PartialConfig
        let partial: PartialConfig = serde_json::from_value(migrated)
            .map_err(|e| ConfigError::ParseError(format!("{}: {}", path.display(), e)))?;

        Ok(Some(partial))
    }

    /// Resolve the writable path for `layer`, returning an error for the read-only
    /// System layer.
    fn layer_write_path(&self, layer: ConfigLayer) -> Result<PathBuf, ConfigError> {
        match layer {
            ConfigLayer::User => Ok(self.user_config_path()),
            ConfigLayer::Project => Ok(self.project_config_write_path()),
            ConfigLayer::Session => Ok(self.session_config_path()),
            ConfigLayer::System => Err(ConfigError::ValidationError(
                "Cannot write to System layer".to_string(),
            )),
        }
    }

    /// Save a config to a specific layer, writing only the delta from parent layers.
    pub fn save_to_layer(&self, config: &Config, layer: ConfigLayer) -> Result<(), ConfigError> {
        let path = self.layer_write_path(layer)?;

        let parent_partial = self.resolve_up_to_layer(layer)?;
        let parent = PartialConfig::from(&parent_partial.resolve());
        let current = PartialConfig::from(config);
        let delta = diff_partial_config(&current, &parent);

        // Preserve any manual edits made externally; delta takes precedence.
        // Fail loudly on an unparseable existing file rather than discarding it
        // (which would clobber the user's whole config on write).
        let existing: PartialConfig = if path.exists() {
            let content = std::fs::read_to_string(&path)
                .map_err(|e| ConfigError::IoError(format!("{}: {}", path.display(), e)))?;
            if content.trim().is_empty() {
                PartialConfig::default()
            } else {
                let value = crate::config::parse_config_jsonc(&content)
                    .map_err(|e| ConfigError::ParseError(format!("{}: {}", path.display(), e)))?;
                serde_json::from_value(value)
                    .map_err(|e| ConfigError::ParseError(format!("{}: {}", path.display(), e)))?
            }
        } else {
            PartialConfig::default()
        };
        let mut merged = delta;
        merged.merge_from(&existing);

        let merged_value = serde_json::to_value(&merged)
            .map_err(|e| ConfigError::SerializeError(e.to_string()))?;
        write_clean_value_to_path(&path, merged_value)
    }

    /// Save a config to a specific layer, using a baseline to track changes.
    ///
    /// This solves the problem where `save_to_layer` can't distinguish between:
    /// - "User didn't change this field" (should preserve external edits)
    /// - "User changed this field to the default" (should update the file)
    ///
    /// By comparing `current` against `baseline` (what was loaded), we know exactly
    /// which fields the user modified. Those fields are updated even if they match
    /// defaults; untouched fields preserve any external edits to the file.
    pub fn save_to_layer_with_baseline(
        &self,
        current: &Config,
        baseline: &Config,
        layer: ConfigLayer,
    ) -> Result<(), ConfigError> {
        let path = self.layer_write_path(layer)?;

        let parent_partial = self.resolve_up_to_layer(layer)?;
        let parent = PartialConfig::from(&parent_partial.resolve());

        let current_json = serde_json::to_value(current)
            .map_err(|e| ConfigError::SerializeError(e.to_string()))?;
        let baseline_json = serde_json::to_value(baseline)
            .map_err(|e| ConfigError::SerializeError(e.to_string()))?;
        let parent_json = serde_json::to_value(&parent)
            .map_err(|e| ConfigError::SerializeError(e.to_string()))?;

        let changed_paths = find_changed_paths(&baseline_json, &current_json);

        let mut result = read_existing_json(&path)?;

        // For each changed path: remove if value reverted to default, otherwise set it.
        for pointer in &changed_paths {
            let current_val = current_json.pointer(pointer);
            let parent_val = parent_json.pointer(pointer);
            if current_val == parent_val {
                remove_json_pointer(&mut result, pointer);
            } else if let Some(val) = current_val {
                set_json_pointer(&mut result, pointer, val.clone());
            }
        }

        write_clean_value_to_path(&path, result)
    }

    /// Save specific changes to a layer file using JSON pointer paths.
    ///
    /// This reads the existing file, applies only the specified changes,
    /// and writes back. This preserves any manual edits not touched by the changes.
    pub fn save_changes_to_layer(
        &self,
        changes: &std::collections::HashMap<String, serde_json::Value>,
        deletions: &std::collections::HashSet<String>,
        layer: ConfigLayer,
    ) -> Result<(), ConfigError> {
        let path = self.layer_write_path(layer)?;

        let mut config_value = read_existing_json(&path)?;

        for pointer in deletions {
            remove_json_pointer(&mut config_value, pointer);
        }
        for (pointer, value) in changes {
            set_json_pointer(&mut config_value, pointer, value.clone());
        }

        // Validate before writing.
        let _: PartialConfig = serde_json::from_value(config_value.clone()).map_err(|e| {
            ConfigError::ValidationError(format!("Result config would be invalid: {}", e))
        })?;

        write_clean_value_to_path(&path, config_value)
    }

    /// Save a SessionConfig to the session layer file.
    pub fn save_session(&self, session: &SessionConfig) -> Result<(), ConfigError> {
        let path = self.session_config_path();

        // Ensure .fresh directory exists
        if let Some(parent_dir) = path.parent() {
            std::fs::create_dir_all(parent_dir)
                .map_err(|e| ConfigError::IoError(format!("{}: {}", parent_dir.display(), e)))?;
        }

        let json = serde_json::to_string_pretty(session)
            .map_err(|e| ConfigError::SerializeError(e.to_string()))?;
        std::fs::write(&path, json)
            .map_err(|e| ConfigError::IoError(format!("{}: {}", path.display(), e)))?;

        tracing::debug!("Saved session config to {}", path.display());
        Ok(())
    }

    /// Load the session config from disk, or return an empty one if it doesn't exist.
    pub fn load_session(&self) -> Result<SessionConfig, ConfigError> {
        match self.load_session_layer()? {
            Some(partial) => Ok(SessionConfig::from(partial)),
            None => Ok(SessionConfig::new()),
        }
    }

    /// Clear the session config file on editor exit.
    pub fn clear_session(&self) -> Result<(), ConfigError> {
        let path = self.session_config_path();
        if path.exists() {
            std::fs::remove_file(&path)
                .map_err(|e| ConfigError::IoError(format!("{}: {}", path.display(), e)))?;
            tracing::debug!("Cleared session config at {}", path.display());
        }
        Ok(())
    }

    /// Resolve config by merging layers below the target layer.
    /// Used to calculate the "parent" config for delta serialization.
    fn resolve_up_to_layer(&self, layer: ConfigLayer) -> Result<PartialConfig, ConfigError> {
        let mut merged = PartialConfig::default();

        // Merge from highest precedence (just below target) to lowest
        // Session layer: parent includes Project + UserPlatform + User
        // Project layer: parent includes UserPlatform + User
        // User layer: parent is empty (system defaults applied during resolve)

        if layer == ConfigLayer::Session {
            // Session's parent is Project + UserPlatform + User
            if let Some(project) = self.load_project_layer()? {
                merged = project;
            }
            if let Some(platform) = self.load_user_platform_layer()? {
                merged.merge_from(&platform);
            }
            if let Some(user) = self.load_user_layer()? {
                merged.merge_from(&user);
            }
        } else if layer == ConfigLayer::Project {
            // Project's parent is UserPlatform + User
            if let Some(platform) = self.load_user_platform_layer()? {
                merged = platform;
            }
            if let Some(user) = self.load_user_layer()? {
                merged.merge_from(&user);
            }
        }
        // User layer's parent is empty (defaults handled during resolve)

        Ok(merged)
    }

    /// Determine which layer each setting value comes from.
    /// Returns a map of JSON pointer paths to their source layer.
    pub fn get_layer_sources(
        &self,
    ) -> Result<std::collections::HashMap<String, ConfigLayer>, ConfigError> {
        use std::collections::HashMap;

        let mut sources: HashMap<String, ConfigLayer> = HashMap::new();

        // Load each layer and mark which paths come from it
        // Check layers in precedence order (highest first)
        // Session layer takes priority, then Project, then User, then System defaults

        if let Some(session) = self.load_session_layer()? {
            let json = serde_json::to_value(&session).unwrap_or_default();
            collect_paths(&json, "", &mut |path| {
                sources.insert(path, ConfigLayer::Session);
            });
        }

        if let Some(project) = self.load_project_layer()? {
            let json = serde_json::to_value(&project).unwrap_or_default();
            collect_paths(&json, "", &mut |path| {
                sources.entry(path).or_insert(ConfigLayer::Project);
            });
        }

        if let Some(user) = self.load_user_layer()? {
            let json = serde_json::to_value(&user).unwrap_or_default();
            collect_paths(&json, "", &mut |path| {
                sources.entry(path).or_insert(ConfigLayer::User);
            });
        }

        // Any path not in the map comes from System defaults (implicitly)

        Ok(sources)
    }
}

/// Recursively collect all non-null leaf paths in a JSON value.
fn collect_paths<F>(value: &Value, prefix: &str, collector: &mut F)
where
    F: FnMut(String),
{
    match value {
        Value::Object(map) => {
            for (key, val) in map {
                let path = if prefix.is_empty() {
                    format!("/{}", key)
                } else {
                    format!("{}/{}", prefix, key)
                };
                collect_paths(val, &path, collector);
            }
        }
        Value::Null => {} // Skip nulls (unset in partial config)
        _ => {
            // Leaf value - collect this path
            collector(prefix.to_string());
        }
    }
}

/// Calculate the delta between a partial config and its parent.
/// Returns a PartialConfig containing only values that differ from parent.
fn diff_partial_config(current: &PartialConfig, parent: &PartialConfig) -> PartialConfig {
    // Convert both to JSON values and diff them
    let current_json = serde_json::to_value(current).unwrap_or_default();
    let parent_json = serde_json::to_value(parent).unwrap_or_default();

    let diff = json_diff(&parent_json, &current_json);

    // Convert diff back to PartialConfig
    serde_json::from_value(diff).unwrap_or_default()
}

impl Config {
    /// Get the system config file paths (without local/working directory).
    ///
    /// On macOS, prioritizes `~/.config/fresh/config.json` if it exists.
    /// Then checks the standard system config directory.
    fn system_config_paths() -> Vec<PathBuf> {
        let mut paths = Vec::with_capacity(2);

        // macOS: Prioritize ~/.config/fresh/config.json
        #[cfg(target_os = "macos")]
        if let Some(home) = dirs::home_dir() {
            let path = home.join(".config").join("fresh").join(Config::FILENAME);
            if path.exists() {
                paths.push(path);
            }
        }

        // Standard system paths (XDG on Linux, AppSupport on macOS, Roaming on Windows)
        if let Some(config_dir) = dirs::config_dir() {
            let path = config_dir.join("fresh").join(Config::FILENAME);
            if !paths.contains(&path) && path.exists() {
                paths.push(path);
            }
        }

        paths
    }

    /// Get all config search paths, checking local (working directory) first.
    ///
    /// Search order:
    /// 1. `{working_dir}/config.json` (project-local config)
    /// 2. System config paths (see `system_config_paths()`)
    ///
    /// Only returns paths that exist on disk.
    fn config_search_paths(working_dir: &Path) -> Vec<PathBuf> {
        let local = Self::local_config_path(working_dir);
        let mut paths = Vec::with_capacity(3);

        if local.exists() {
            paths.push(local);
        }

        paths.extend(Self::system_config_paths());
        paths
    }

    /// Find the first existing config file, checking local directory first.
    ///
    /// Returns `None` if no config file exists anywhere.
    pub fn find_config_path(working_dir: &Path) -> Option<PathBuf> {
        Self::config_search_paths(working_dir).into_iter().next()
    }

    /// Load configuration using the 4-level layer system.
    ///
    /// Merges layers in precedence order: Session > Project > User > System
    /// Falls back to defaults for any unspecified values.
    pub fn load_with_layers(dir_context: &DirectoryContext, working_dir: &Path) -> Self {
        let resolver = ConfigResolver::new(dir_context.clone(), working_dir.to_path_buf());
        match resolver.resolve() {
            Ok(config) => {
                tracing::info!("Loaded layered config for {}", working_dir.display());
                config
            }
            Err(e) => {
                tracing::warn!("Failed to load layered config: {}, using defaults", e);
                Self::default()
            }
        }
    }

    /// Read the raw user config file content as JSON.
    ///
    /// This returns the sparse user config (only what's in the file, not merged
    /// with defaults). Useful for plugins that need to distinguish between
    /// user-set values and defaults.
    ///
    /// Checks working directory first, then system paths.
    pub fn read_user_config_raw(working_dir: &Path) -> serde_json::Value {
        for path in Self::config_search_paths(working_dir) {
            if let Ok(contents) = std::fs::read_to_string(&path) {
                match crate::config::parse_config_jsonc(&contents) {
                    Ok(value) => return value,
                    Err(e) => {
                        tracing::warn!("Failed to parse config from {}: {}", path.display(), e);
                    }
                }
            }
        }
        serde_json::Value::Object(serde_json::Map::new())
    }
}

/// Compute the difference between two JSON values.
/// Returns only the parts of `current` that differ from `defaults`.
fn json_diff(defaults: &serde_json::Value, current: &serde_json::Value) -> serde_json::Value {
    use serde_json::Value;

    match (defaults, current) {
        // Both are objects - recursively diff
        (Value::Object(def_map), Value::Object(cur_map)) => {
            let mut result = serde_json::Map::new();

            for (key, cur_val) in cur_map {
                if let Some(def_val) = def_map.get(key) {
                    // Key exists in both - recurse
                    let diff = json_diff(def_val, cur_val);
                    // Only include if there's an actual difference
                    if !is_empty_diff(&diff) {
                        result.insert(key.clone(), diff);
                    }
                } else {
                    // Key only in current - include it, but strip empty defaults
                    if let Some(stripped) = strip_empty_defaults(cur_val.clone()) {
                        result.insert(key.clone(), stripped);
                    }
                }
            }

            Value::Object(result)
        }
        // For arrays and primitives, include if different
        _ => {
            // Treat empty string as "not set" - don't include in diff
            if let Value::String(s) = current {
                if s.is_empty() {
                    return Value::Object(serde_json::Map::new()); // No diff
                }
            }
            if defaults == current {
                Value::Object(serde_json::Map::new()) // Empty object signals "no diff"
            } else {
                current.clone()
            }
        }
    }
}

/// Check if a diff result represents "no changes"
fn is_empty_diff(value: &serde_json::Value) -> bool {
    match value {
        serde_json::Value::Object(map) => map.is_empty(),
        _ => false,
    }
}

/// Directory paths for editor state and configuration
///
/// This struct holds all directory paths that the editor needs.
/// Only the top-level `main` function should use `dirs::*` to construct this;
/// all other code should receive it by construction/parameter passing.
///
/// This design ensures:
/// - Tests can use isolated temp directories
/// - Parallel tests don't interfere with each other
/// - No hidden global state dependencies
#[derive(Debug, Clone)]
pub struct DirectoryContext {
    /// Data directory for persistent state (recovery, workspaces, history)
    /// e.g., ~/.local/share/fresh on Linux, ~/Library/Application Support/fresh on macOS
    pub data_dir: std::path::PathBuf,

    /// Config directory for user configuration
    /// e.g., ~/.config/fresh on Linux, ~/Library/Application Support/fresh on macOS
    pub config_dir: std::path::PathBuf,

    /// User's home directory (for file open dialog shortcuts)
    pub home_dir: Option<std::path::PathBuf>,

    /// User's documents directory (for file open dialog shortcuts)
    pub documents_dir: Option<std::path::PathBuf>,

    /// User's downloads directory (for file open dialog shortcuts)
    pub downloads_dir: Option<std::path::PathBuf>,
}

impl DirectoryContext {
    /// Create a DirectoryContext from the system directories
    /// This should ONLY be called from main()
    pub fn from_system() -> std::io::Result<Self> {
        let data_dir = dirs::data_dir()
            .ok_or_else(|| {
                std::io::Error::new(
                    std::io::ErrorKind::NotFound,
                    "Could not determine data directory",
                )
            })?
            .join("fresh");

        let config_dir = Self::default_config_dir().ok_or_else(|| {
            std::io::Error::new(
                std::io::ErrorKind::NotFound,
                "Could not determine config directory",
            )
        })?;

        Ok(Self {
            data_dir,
            config_dir,
            home_dir: dirs::home_dir(),
            documents_dir: dirs::document_dir(),
            downloads_dir: dirs::download_dir(),
        })
    }

    /// Create a DirectoryContext for testing with a temp directory
    /// All paths point to subdirectories within the provided temp_dir
    pub fn for_testing(temp_dir: &std::path::Path) -> Self {
        Self {
            data_dir: temp_dir.join("data"),
            config_dir: temp_dir.join("config"),
            home_dir: Some(temp_dir.join("home")),
            documents_dir: Some(temp_dir.join("documents")),
            downloads_dir: Some(temp_dir.join("downloads")),
        }
    }

    /// Get the recovery directory path
    pub fn recovery_dir(&self) -> std::path::PathBuf {
        self.data_dir.join("recovery")
    }

    /// Get the workspaces directory path
    pub fn workspaces_dir(&self) -> std::path::PathBuf {
        self.data_dir.join("workspaces")
    }

    /// Per-project state directory: all persistent state for one workspace
    /// lives under here (workspace trust today; room for more). Keyed by the
    /// canonicalized working directory (so symlinked spellings of the same
    /// project share state) under the shared `workspaces/` location. Using a
    /// directory per project — rather than one shared file — keeps concurrent
    /// `fresh` processes on different projects from contending over a single
    /// file.
    ///
    // TODO(workspace-state): consolidate the rest of a project's persistent
    // state into this directory. Today the workspace snapshot still lives
    // beside it as `workspaces/<encoded>.json`, and recovery/history live
    // under their own `data_dir` subtrees. Migrate them to
    // `project_state_dir(working_dir)/{workspace,recovery,history}.json` (with
    // a one-time move of the legacy paths, falling back to read-old/write-new)
    // so a project's full state is self-contained in one directory.
    pub fn project_state_dir(&self, working_dir: &std::path::Path) -> std::path::PathBuf {
        let canonical = working_dir
            .canonicalize()
            .unwrap_or_else(|_| working_dir.to_path_buf());
        self.workspaces_dir()
            .join(crate::workspace::encode_path_for_filename(&canonical))
    }

    /// Get the history file path for a specific prompt type
    /// This is the generic method used by prompt_histories HashMap.
    /// history_name can be: "search", "replace", "goto_line", "plugin:custom_name", etc.
    pub fn prompt_history_path(&self, history_name: &str) -> std::path::PathBuf {
        // Sanitize the name for filesystem safety (replace : with _)
        let safe_name = history_name.replace(':', "_");
        self.data_dir.join(format!("{}_history.json", safe_name))
    }

    /// Get the search history file path (legacy, calls generic method)
    pub fn search_history_path(&self) -> std::path::PathBuf {
        self.prompt_history_path("search")
    }

    /// Get the replace history file path (legacy, calls generic method)
    pub fn replace_history_path(&self) -> std::path::PathBuf {
        self.prompt_history_path("replace")
    }

    /// Get the goto line history file path (legacy, calls generic method)
    pub fn goto_line_history_path(&self) -> std::path::PathBuf {
        self.prompt_history_path("goto_line")
    }

    /// Get the terminals root directory
    pub fn terminals_dir(&self) -> std::path::PathBuf {
        self.data_dir.join("terminals")
    }

    /// Get the terminal directory for a specific working directory
    pub fn terminal_dir_for(&self, working_dir: &std::path::Path) -> std::path::PathBuf {
        let encoded = crate::workspace::encode_path_for_filename(working_dir);
        self.terminals_dir().join(encoded)
    }

    /// Per-working-directory data root (`<data_dir>/workdirs/<encoded-cwd>/`).
    /// The canonical home for plugin state that should be scoped to a single
    /// project root / worktree rather than shared across all of them — each
    /// worktree gets its own subtree. Plugins reach this via
    /// `editor.getWorkingDataDir()`.
    pub fn working_data_dir_for(&self, working_dir: &std::path::Path) -> std::path::PathBuf {
        let encoded = crate::workspace::encode_path_for_filename(working_dir);
        self.data_dir.join("workdirs").join(encoded)
    }

    /// Get the config file path
    pub fn config_path(&self) -> std::path::PathBuf {
        self.config_dir.join(Config::FILENAME)
    }

    /// Get the themes directory path
    pub fn themes_dir(&self) -> std::path::PathBuf {
        self.config_dir.join("themes")
    }

    /// Get the grammars directory path
    pub fn grammars_dir(&self) -> std::path::PathBuf {
        self.config_dir.join("grammars")
    }

    /// Get the plugins directory path
    pub fn plugins_dir(&self) -> std::path::PathBuf {
        self.config_dir.join("plugins")
    }

    /// Get the default config directory path (static/internal version).
    ///
    /// This is used internally by `from_system()` to determine the config directory.
    ///
    /// On macOS, this prioritizes `~/.config/fresh` over `~/Library/Application Support/fresh`
    /// to match the documented configuration location.
    fn default_config_dir() -> Option<std::path::PathBuf> {
        #[cfg(target_os = "macos")]
        {
            dirs::home_dir().map(|p| p.join(".config").join("fresh"))
        }

        #[cfg(not(target_os = "macos"))]
        {
            dirs::config_dir().map(|p| p.join("fresh"))
        }
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use tempfile::TempDir;

    fn create_test_resolver() -> (TempDir, ConfigResolver) {
        let temp_dir = TempDir::new().unwrap();
        let dir_context = DirectoryContext::for_testing(temp_dir.path());
        let working_dir = temp_dir.path().join("project");
        std::fs::create_dir_all(&working_dir).unwrap();
        let resolver = ConfigResolver::new(dir_context, working_dir);
        (temp_dir, resolver)
    }

    #[test]
    fn resolver_returns_defaults_when_no_config_files() {
        let (_temp, resolver) = create_test_resolver();
        let config = resolver.resolve().unwrap();

        // Should have system defaults
        assert_eq!(config.editor.tab_size, 4);
        assert!(config.editor.line_numbers);
    }

    #[test]
    fn resolver_loads_user_layer() {
        let (temp, resolver) = create_test_resolver();

        // Create user config
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        std::fs::write(&user_config_path, r#"{"editor": {"tab_size": 2}}"#).unwrap();

        let config = resolver.resolve().unwrap();
        assert_eq!(config.editor.tab_size, 2);
        assert!(config.editor.line_numbers); // Still default
        drop(temp);
    }

    #[test]
    fn resolver_loads_user_layer_with_comments() {
        // A user config annotated with JSONC comments and a trailing comma
        // must be honored, not silently dropped to defaults.
        let (temp, resolver) = create_test_resolver();

        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        std::fs::write(
            &user_config_path,
            r#"{
                // I like a 7-space tab in this project
                "editor": {
                    "tab_size": 7, /* trailing comma below is allowed too */
                    "line_numbers": false,
                }
            }"#,
        )
        .unwrap();

        let config = resolver.resolve().unwrap();
        assert_eq!(
            config.editor.tab_size, 7,
            "commented user config should still apply tab_size"
        );
        assert!(
            !config.editor.line_numbers,
            "commented user config should still apply line_numbers"
        );
        drop(temp);
    }

    #[test]
    fn resolver_loads_project_layer_with_comments() {
        // The project `.fresh/config.json` layer must also tolerate comments.
        let (temp, resolver) = create_test_resolver();

        let project_config_path = resolver.project_config_write_path();
        std::fs::create_dir_all(project_config_path.parent().unwrap()).unwrap();
        std::fs::write(
            &project_config_path,
            "{\n  // project override\n  \"editor\": { \"tab_size\": 3 }\n}\n",
        )
        .unwrap();

        let config = resolver.resolve().unwrap();
        assert_eq!(config.editor.tab_size, 3);
        drop(temp);
    }

    #[test]
    fn save_preserves_external_commented_values() {
        // Saving a delta to a layer whose file already contains comments must
        // not wipe the user's other (commented) settings. The comment is not
        // required to survive, but the sibling values must.
        let (temp, resolver) = create_test_resolver();

        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        std::fs::write(
            &user_config_path,
            r#"{
                // hand-edited by the user
                "editor": {
                    "tab_size": 7
                }
            }"#,
        )
        .unwrap();

        // Change a *different* field and save back to the user layer.
        let mut config = resolver.resolve().unwrap();
        assert_eq!(config.editor.tab_size, 7);
        config.editor.line_numbers = false;
        resolver.save_to_layer(&config, ConfigLayer::User).unwrap();

        // The externally-set tab_size must survive the round-trip.
        let reloaded = resolver.resolve().unwrap();
        assert_eq!(
            reloaded.editor.tab_size, 7,
            "saving an unrelated field must not drop the commented tab_size"
        );
        assert!(!reloaded.editor.line_numbers);
        drop(temp);
    }

    #[test]
    fn load_from_file_accepts_comments() {
        // The explicit `--config <file>` path (Config::load_from_file) must
        // accept JSONC instead of refusing to start.
        let temp = TempDir::new().unwrap();
        let path = temp.path().join("config-with-comments.json");
        std::fs::write(
            &path,
            r#"{
                // 2-space tabs, no gutter
                "editor": {
                    "tab_size": 2,
                    "line_numbers": false /* turn off the gutter */
                }
            }"#,
        )
        .unwrap();

        let config = Config::load_from_file(&path).expect("commented --config file should load");
        assert_eq!(config.editor.tab_size, 2);
        assert!(!config.editor.line_numbers);
    }

    #[test]
    fn reconcile_preserves_comments_and_unchanged_inline_annotations() {
        // Direct unit test of the comment-preserving writer: changing one
        // field must keep every comment and leave the untouched field's inline
        // annotation byte-for-byte.
        let existing = "{\n  \
            // top-of-file note\n  \
            \"editor\": {\n    \
                \"tab_size\": 7, // my preferred width\n    \
                \"line_numbers\": true\n  \
            }\n\
        }\n";

        let target = serde_json::json!({
            "editor": { "tab_size": 7, "line_numbers": false }
        });

        let out =
            reconcile_preserving_comments(existing, &target).expect("object root should reconcile");

        assert!(
            out.contains("// top-of-file note"),
            "file comment lost:\n{out}"
        );
        assert!(
            out.contains("\"tab_size\": 7, // my preferred width"),
            "inline comment on the unchanged field should be untouched:\n{out}"
        );
        // The changed field was actually updated.
        let reparsed = crate::config::parse_config_jsonc(&out).unwrap();
        assert_eq!(
            reparsed.pointer("/editor/line_numbers"),
            Some(&serde_json::json!(false))
        );
    }

    #[test]
    fn reconcile_appends_new_key_without_disturbing_comments() {
        let existing = "{\n  // keep me\n  \"editor\": { \"tab_size\": 2 }\n}\n";
        let target = serde_json::json!({
            "editor": { "tab_size": 2 },
            "theme": "dark"
        });

        let out = reconcile_preserving_comments(existing, &target).unwrap();
        assert!(out.contains("// keep me"), "comment lost:\n{out}");
        let reparsed = crate::config::parse_config_jsonc(&out).unwrap();
        assert_eq!(reparsed.pointer("/theme"), Some(&serde_json::json!("dark")));
        assert_eq!(
            reparsed.pointer("/editor/tab_size"),
            Some(&serde_json::json!(2))
        );
    }

    #[test]
    fn save_changes_to_layer_preserves_user_comments() {
        // End-to-end through the Settings-UI save path: a commented user
        // config keeps its comments after a single field is changed.
        let (temp, resolver) = create_test_resolver();

        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        std::fs::write(
            &user_config_path,
            "{\n  \
                // I like a 7-space tab in this project\n  \
                \"editor\": {\n    \
                    \"tab_size\": 7 /* keep this */\n  \
                }\n\
            }\n",
        )
        .unwrap();

        let mut changes: std::collections::HashMap<String, serde_json::Value> =
            std::collections::HashMap::new();
        changes.insert("/editor/line_numbers".to_string(), serde_json::json!(false));
        resolver
            .save_changes_to_layer(
                &changes,
                &std::collections::HashSet::new(),
                ConfigLayer::User,
            )
            .unwrap();

        let saved = std::fs::read_to_string(&user_config_path).unwrap();
        assert!(
            saved.contains("// I like a 7-space tab in this project"),
            "line comment must survive a settings save:\n{saved}"
        );
        assert!(
            saved.contains("/* keep this */"),
            "inline comment on the untouched field must survive:\n{saved}"
        );

        // Both the preserved and the newly written values resolve correctly.
        let config = resolver.resolve().unwrap();
        assert_eq!(config.editor.tab_size, 7);
        assert!(!config.editor.line_numbers);
        drop(temp);
    }

    #[test]
    fn reconcile_falls_back_for_non_object_root() {
        // A non-object existing document can't be edited in place; the writer
        // signals a fall-back so the caller pretty-prints fresh.
        assert!(reconcile_preserving_comments("[1, 2, 3]", &serde_json::json!({"a": 1})).is_none());
    }

    /// A realistic user config: comments in two places, a `keybindings` array,
    /// and a `languages` map. Mirrors the file from the data-loss report.
    const REALISTIC_USER_CONFIG: &str = r#"{
  "version": 2,
  "theme": "builtin://dracula",
  "editor": {
    // stuff that's really thingy
    "hide_current_line_on_selection": true,
    "auto_read_only": false,
    "indentation_guide": "all"
  },
  // file explorer hooray:
  "file_explorer": {
    "show_hidden": true,
    "custom_ignore_patterns": ["*.log"]
  },
  "keybindings": [
    {
      "key": "=",
      "modifiers": ["alt"],
      "action": "next_window"
    }
  ],
  "languages": {
    "go": {
      "extensions": ["go"],
      "grammar": "go",
      "use_tabs": true,
      "tab_size": 8,
      "formatter": {
        "command": "gofmt",
        "stdin": true,
        "timeout_ms": 10000
      },
      "format_on_save": true
    }
  },
  "check_for_updates": false
}
"#;

    /// Reproduces the original parse failure: the reported config is perfectly
    /// legitimate JSON-with-comments, but the strict `serde_json` parser the
    /// loader used to rely on rejects it (the `key must be a string` error from
    /// the bug report, raised at the first `//`). Our config loader must accept
    /// it and read every value correctly.
    #[test]
    fn realistic_commented_config_is_rejected_by_strict_json_but_accepted_by_loader() {
        // Original behavior: strict JSON cannot parse comments — this is the bug.
        assert!(
            serde_json::from_str::<serde_json::Value>(REALISTIC_USER_CONFIG).is_err(),
            "sanity: the sample must actually contain JSONC that strict JSON rejects"
        );

        // Fixed behavior: the loader parses it and the values are intact.
        let v = crate::config::parse_config_jsonc(REALISTIC_USER_CONFIG)
            .expect("loader must accept legitimate JSON-with-comments");
        assert_eq!(
            v.pointer("/theme"),
            Some(&serde_json::json!("builtin://dracula"))
        );
        assert_eq!(
            v.pointer("/languages/go/tab_size"),
            Some(&serde_json::json!(8))
        );
        assert_eq!(
            v.pointer("/keybindings/0/action"),
            Some(&serde_json::json!("next_window"))
        );
        assert_eq!(
            v.pointer("/file_explorer/custom_ignore_patterns/0"),
            Some(&serde_json::json!("*.log"))
        );
    }

    /// Reproduces the reported data-loss bug. Saving a single settings change
    /// onto an existing config file that fails to parse must NOT overwrite the
    /// whole file with just that change. The save must error (so the UI can
    /// alert) and leave the file byte-for-byte intact.
    ///
    /// Without the guard, `read_existing_json` returns an empty object for an
    /// unparseable file, the one change is applied on top, and the user's
    /// entire config is overwritten — exactly the failure that was reported.
    #[test]
    fn save_changes_does_not_clobber_unparseable_config() {
        let (temp, resolver) = create_test_resolver();

        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        // Truncated / malformed JSON: missing closing braces.
        let original = "{\n  \"editor\": {\n    \"tab_size\": 7\n";
        std::fs::write(&user_config_path, original).unwrap();

        let mut changes = std::collections::HashMap::new();
        changes.insert("/editor/line_numbers".to_string(), serde_json::json!(false));
        let result = resolver.save_changes_to_layer(
            &changes,
            &std::collections::HashSet::new(),
            ConfigLayer::User,
        );

        assert!(
            result.is_err(),
            "saving onto an unparseable config must error, not silently succeed and clobber it"
        );
        let after = std::fs::read_to_string(&user_config_path).unwrap();
        assert_eq!(
            after, original,
            "a failed save must leave the unparseable config file untouched"
        );
        drop(temp);
    }

    /// Same guarantee for the baseline-based save path used by the Settings UI.
    #[test]
    fn save_with_baseline_does_not_clobber_unparseable_config() {
        let (temp, resolver) = create_test_resolver();

        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        let original = "{ \"editor\": { \"tab_size\": 7  oops not json";
        std::fs::write(&user_config_path, original).unwrap();

        let baseline = Config::default();
        let mut current = Config::default();
        current.editor.tab_size = 3;
        let result = resolver.save_to_layer_with_baseline(&current, &baseline, ConfigLayer::User);

        assert!(result.is_err(), "must error on unparseable existing file");
        assert_eq!(
            std::fs::read_to_string(&user_config_path).unwrap(),
            original,
            "a failed save must leave the file untouched"
        );
        drop(temp);
    }

    /// Same guarantee for `save_to_layer`.
    #[test]
    fn save_to_layer_does_not_clobber_unparseable_config() {
        let (temp, resolver) = create_test_resolver();

        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        let original = "{ broken";
        std::fs::write(&user_config_path, original).unwrap();

        let mut config = Config::default();
        config.editor.tab_size = 3;
        let result = resolver.save_to_layer(&config, ConfigLayer::User);

        assert!(result.is_err(), "must error on unparseable existing file");
        assert_eq!(
            std::fs::read_to_string(&user_config_path).unwrap(),
            original,
            "a failed save must leave the file untouched"
        );
        drop(temp);
    }

    /// End-to-end: a Settings-UI save of one unrelated field onto the realistic
    /// commented config preserves every comment, the keybindings array, and the
    /// languages map, while applying the change. This is the regression that the
    /// original (strict-parse) code silently failed by wiping the file.
    #[test]
    fn settings_save_preserves_full_realistic_config() {
        let (temp, resolver) = create_test_resolver();

        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        std::fs::write(&user_config_path, REALISTIC_USER_CONFIG).unwrap();

        let mut changes = std::collections::HashMap::new();
        changes.insert(
            "/editor/auto_read_only".to_string(),
            serde_json::json!(true),
        );
        resolver
            .save_changes_to_layer(
                &changes,
                &std::collections::HashSet::new(),
                ConfigLayer::User,
            )
            .unwrap();

        let saved = std::fs::read_to_string(&user_config_path).unwrap();
        // Comments survive.
        assert!(
            saved.contains("// stuff that's really thingy"),
            "editor comment lost:\n{saved}"
        );
        assert!(
            saved.contains("// file explorer hooray:"),
            "file_explorer comment lost:\n{saved}"
        );
        // Complex structures survive.
        let reparsed = crate::config::parse_config_jsonc(&saved).unwrap();
        assert_eq!(
            reparsed.pointer("/keybindings/0/action"),
            Some(&serde_json::json!("next_window")),
            "keybindings array lost:\n{saved}"
        );
        assert_eq!(
            reparsed.pointer("/languages/go/formatter/command"),
            Some(&serde_json::json!("gofmt")),
            "languages map lost:\n{saved}"
        );
        assert_eq!(
            reparsed.pointer("/theme"),
            Some(&serde_json::json!("builtin://dracula"))
        );
        // The change was applied.
        assert_eq!(
            reparsed.pointer("/editor/auto_read_only"),
            Some(&serde_json::json!(true))
        );
        drop(temp);
    }

    #[test]
    fn resolver_project_overrides_user() {
        let (temp, resolver) = create_test_resolver();

        // Create user config with tab_size=2
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        std::fs::write(
            &user_config_path,
            r#"{"editor": {"tab_size": 2, "line_numbers": false}}"#,
        )
        .unwrap();

        // Create project config with tab_size=8
        let project_config_path = resolver.project_config_path();
        std::fs::create_dir_all(project_config_path.parent().unwrap()).unwrap();
        std::fs::write(&project_config_path, r#"{"editor": {"tab_size": 8}}"#).unwrap();

        let config = resolver.resolve().unwrap();
        assert_eq!(config.editor.tab_size, 8); // Project wins
        assert!(!config.editor.line_numbers); // User value preserved
        drop(temp);
    }

    #[test]
    fn resolver_session_overrides_all() {
        let (temp, resolver) = create_test_resolver();

        // Create user config
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        std::fs::write(&user_config_path, r#"{"editor": {"tab_size": 2}}"#).unwrap();

        // Create project config
        let project_config_path = resolver.project_config_path();
        std::fs::create_dir_all(project_config_path.parent().unwrap()).unwrap();
        std::fs::write(&project_config_path, r#"{"editor": {"tab_size": 4}}"#).unwrap();

        // Create session config
        let session_config_path = resolver.session_config_path();
        std::fs::write(&session_config_path, r#"{"editor": {"tab_size": 16}}"#).unwrap();

        let config = resolver.resolve().unwrap();
        assert_eq!(config.editor.tab_size, 16); // Session wins
        drop(temp);
    }

    #[test]
    fn layer_precedence_ordering() {
        assert!(ConfigLayer::Session.precedence() > ConfigLayer::Project.precedence());
        assert!(ConfigLayer::Project.precedence() > ConfigLayer::User.precedence());
        assert!(ConfigLayer::User.precedence() > ConfigLayer::System.precedence());
    }

    #[test]
    fn save_to_system_layer_fails() {
        let (_temp, resolver) = create_test_resolver();
        let config = Config::default();
        let result = resolver.save_to_layer(&config, ConfigLayer::System);
        assert!(result.is_err());
    }

    #[test]
    fn resolver_loads_legacy_project_config() {
        let (temp, resolver) = create_test_resolver();

        // Create legacy project config at {working_dir}/config.json
        let working_dir = temp.path().join("project");
        let legacy_path = working_dir.join("config.json");
        std::fs::write(&legacy_path, r#"{"editor": {"tab_size": 3}}"#).unwrap();

        let config = resolver.resolve().unwrap();
        assert_eq!(config.editor.tab_size, 3);
        drop(temp);
    }

    #[test]
    fn resolver_prefers_new_config_over_legacy() {
        let (temp, resolver) = create_test_resolver();

        // Create both legacy and new project configs
        let working_dir = temp.path().join("project");

        // Legacy: tab_size=3
        let legacy_path = working_dir.join("config.json");
        std::fs::write(&legacy_path, r#"{"editor": {"tab_size": 3}}"#).unwrap();

        // New: tab_size=5
        let new_path = working_dir.join(".fresh").join("config.json");
        std::fs::create_dir_all(new_path.parent().unwrap()).unwrap();
        std::fs::write(&new_path, r#"{"editor": {"tab_size": 5}}"#).unwrap();

        let config = resolver.resolve().unwrap();
        assert_eq!(config.editor.tab_size, 5); // New path wins
        drop(temp);
    }

    #[test]
    fn load_with_layers_works() {
        let temp = TempDir::new().unwrap();
        let dir_context = DirectoryContext::for_testing(temp.path());
        let working_dir = temp.path().join("project");
        std::fs::create_dir_all(&working_dir).unwrap();

        // Create user config
        std::fs::create_dir_all(&dir_context.config_dir).unwrap();
        std::fs::write(dir_context.config_path(), r#"{"editor": {"tab_size": 2}}"#).unwrap();

        let config = Config::load_with_layers(&dir_context, &working_dir);
        assert_eq!(config.editor.tab_size, 2);
    }

    #[test]
    fn platform_config_overrides_user() {
        let (temp, resolver) = create_test_resolver();

        // Create user config with tab_size=2
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        std::fs::write(&user_config_path, r#"{"editor": {"tab_size": 2}}"#).unwrap();

        // Create platform config with tab_size=6
        if let Some(platform_path) = resolver.user_platform_config_path() {
            std::fs::write(&platform_path, r#"{"editor": {"tab_size": 6}}"#).unwrap();

            let config = resolver.resolve().unwrap();
            assert_eq!(config.editor.tab_size, 6); // Platform overrides user
        }
        drop(temp);
    }

    #[test]
    fn project_overrides_platform() {
        let (temp, resolver) = create_test_resolver();

        // Create user config
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        std::fs::write(&user_config_path, r#"{"editor": {"tab_size": 2}}"#).unwrap();

        // Create platform config
        if let Some(platform_path) = resolver.user_platform_config_path() {
            std::fs::write(&platform_path, r#"{"editor": {"tab_size": 6}}"#).unwrap();
        }

        // Create project config with tab_size=10
        let project_config_path = resolver.project_config_path();
        std::fs::create_dir_all(project_config_path.parent().unwrap()).unwrap();
        std::fs::write(&project_config_path, r#"{"editor": {"tab_size": 10}}"#).unwrap();

        let config = resolver.resolve().unwrap();
        assert_eq!(config.editor.tab_size, 10); // Project overrides platform
        drop(temp);
    }

    #[test]
    fn migration_adds_version() {
        let input = serde_json::json!({
            "editor": {"tab_size": 2}
        });

        let migrated = migrate_config(input).unwrap();

        assert_eq!(
            migrated.get("version"),
            Some(&serde_json::json!(CURRENT_CONFIG_VERSION))
        );
    }

    #[test]
    fn migration_v1_to_v2_injects_remote_element() {
        // User who customized status_bar.left on v1 without the
        // indicator: v2 migration prepends `"{remote}"`.
        let input = serde_json::json!({
            "version": 1,
            "editor": {
                "status_bar": {
                    "left": ["{filename}", "{cursor}"]
                }
            }
        });

        let migrated = migrate_config(input).unwrap();

        assert_eq!(migrated.get("version"), Some(&serde_json::json!(2)));
        let left = migrated
            .pointer("/editor/status_bar/left")
            .and_then(|v| v.as_array())
            .expect("status_bar.left should remain an array");
        assert_eq!(left[0], serde_json::json!("{remote}"));
        assert_eq!(left[1], serde_json::json!("{filename}"));
        assert_eq!(left[2], serde_json::json!("{cursor}"));
    }

    #[test]
    fn migration_v1_to_v2_is_idempotent() {
        // User already has `"{remote}"` somewhere in left (e.g. they
        // opted in manually before v2). Don't double-insert.
        let input = serde_json::json!({
            "version": 1,
            "editor": {
                "status_bar": {
                    "left": ["{filename}", "{remote}", "{cursor}"]
                }
            }
        });

        let migrated = migrate_config(input).unwrap();

        let left = migrated
            .pointer("/editor/status_bar/left")
            .and_then(|v| v.as_array())
            .unwrap();
        let remote_count = left
            .iter()
            .filter(|v| v.as_str() == Some("{remote}"))
            .count();
        assert_eq!(
            remote_count, 1,
            "migration should never duplicate an existing {{remote}} entry; left = {:?}",
            left
        );
    }

    #[test]
    fn migration_v1_to_v2_leaves_default_users_alone() {
        // User with no `status_bar` override stays on the rolling
        // default — migration bumps the version but doesn't
        // materialize a status_bar object.
        let input = serde_json::json!({
            "version": 1,
            "editor": {"tab_size": 4}
        });

        let migrated = migrate_config(input).unwrap();

        assert_eq!(migrated.get("version"), Some(&serde_json::json!(2)));
        assert!(
            migrated.pointer("/editor/status_bar").is_none(),
            "migration must not fabricate a status_bar object for users \
             who never overrode the default; migrated = {:?}",
            migrated
        );
    }

    #[test]
    fn migration_renames_camelcase_keys() {
        let input = serde_json::json!({
            "editor": {
                "tabSize": 8,
                "lineNumbers": false
            }
        });

        let migrated = migrate_config(input).unwrap();

        let editor = migrated.get("editor").unwrap();
        assert_eq!(editor.get("tab_size"), Some(&serde_json::json!(8)));
        assert_eq!(editor.get("line_numbers"), Some(&serde_json::json!(false)));
        assert!(editor.get("tabSize").is_none());
        assert!(editor.get("lineNumbers").is_none());
    }

    #[test]
    fn migration_preserves_existing_snake_case() {
        let input = serde_json::json!({
            "version": 1,
            "editor": {"tab_size": 4}
        });

        let migrated = migrate_config(input).unwrap();

        let editor = migrated.get("editor").unwrap();
        assert_eq!(editor.get("tab_size"), Some(&serde_json::json!(4)));
    }

    #[test]
    fn resolver_loads_legacy_camelcase_config() {
        let (temp, resolver) = create_test_resolver();

        // Create config with legacy camelCase keys
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        std::fs::write(
            &user_config_path,
            r#"{"editor": {"tabSize": 3, "lineNumbers": false}}"#,
        )
        .unwrap();

        let config = resolver.resolve().unwrap();
        assert_eq!(config.editor.tab_size, 3);
        assert!(!config.editor.line_numbers);
        drop(temp);
    }

    #[test]
    fn resolver_migrates_v1_status_bar_left_on_load() {
        // A user with a v1 config that customized status_bar.left
        // without {remote} loads the editor for the first time on
        // v2. The resolver runs the migration in-memory; the
        // resolved Config has {remote} at index 0.
        let (temp, resolver) = create_test_resolver();

        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        std::fs::write(
            &user_config_path,
            r#"{
                "version": 1,
                "editor": {
                    "status_bar": {
                        "left": ["{filename}", "{cursor}"],
                        "right": []
                    }
                }
            }"#,
        )
        .unwrap();

        let config = resolver.resolve().unwrap();
        let left = &config.editor.status_bar.left;
        assert_eq!(
            left.first().cloned(),
            Some(crate::config::StatusBarElement::RemoteIndicator),
            "resolver should inject RemoteIndicator at index 0 during v1→v2 \
             migration; left = {:?}",
            left
        );
        drop(temp);
    }

    #[test]
    fn save_and_load_session() {
        let (_temp, resolver) = create_test_resolver();

        let mut session = SessionConfig::new();
        session.set_theme(crate::config::ThemeName::from("dark"));
        session.set_editor_option(|e| e.tab_size = Some(2));

        // Save session
        resolver.save_session(&session).unwrap();

        // Load session
        let loaded = resolver.load_session().unwrap();
        assert_eq!(loaded.theme, Some(crate::config::ThemeName::from("dark")));
        assert_eq!(loaded.editor.as_ref().unwrap().tab_size, Some(2));
    }

    #[test]
    fn clear_session_removes_file() {
        let (_temp, resolver) = create_test_resolver();

        let mut session = SessionConfig::new();
        session.set_theme(crate::config::ThemeName::from("dark"));

        // Save then clear
        resolver.save_session(&session).unwrap();
        assert!(resolver.session_config_path().exists());

        resolver.clear_session().unwrap();
        assert!(!resolver.session_config_path().exists());
    }

    #[test]
    fn load_session_returns_empty_when_no_file() {
        let (_temp, resolver) = create_test_resolver();

        let session = resolver.load_session().unwrap();
        assert!(session.is_empty());
    }

    #[test]
    fn session_affects_resolved_config() {
        let (_temp, resolver) = create_test_resolver();

        // Save a session with tab_size=16
        let mut session = SessionConfig::new();
        session.set_editor_option(|e| e.tab_size = Some(16));
        resolver.save_session(&session).unwrap();

        // Resolve should pick up session value
        let config = resolver.resolve().unwrap();
        assert_eq!(config.editor.tab_size, 16);
    }

    #[test]
    fn save_to_layer_writes_minimal_delta() {
        let (temp, resolver) = create_test_resolver();

        // Create user config with tab_size=2
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        std::fs::write(
            &user_config_path,
            r#"{"editor": {"tab_size": 2, "line_numbers": false}}"#,
        )
        .unwrap();

        // Resolve the full config (inherits user values)
        let mut config = resolver.resolve().unwrap();
        assert_eq!(config.editor.tab_size, 2);
        assert!(!config.editor.line_numbers);

        // Change only tab_size in the project layer
        config.editor.tab_size = 8;

        // Save to project layer
        resolver
            .save_to_layer(&config, ConfigLayer::Project)
            .unwrap();

        // Read the project config file and verify it contains ONLY the delta
        let project_config_path = resolver.project_config_write_path();
        let content = std::fs::read_to_string(&project_config_path).unwrap();
        let json: serde_json::Value = serde_json::from_str(&content).unwrap();

        // Should only have editor.tab_size = 8, nothing else
        assert_eq!(
            json.get("editor").and_then(|e| e.get("tab_size")),
            Some(&serde_json::json!(8)),
            "Project config should contain tab_size override"
        );

        // Should NOT have line_numbers (inherited from user, not changed)
        assert!(
            json.get("editor")
                .and_then(|e| e.get("line_numbers"))
                .is_none(),
            "Project config should NOT contain line_numbers (it's inherited from user layer)"
        );

        // Should NOT have other editor fields like scroll_offset (system default)
        assert!(
            json.get("editor")
                .and_then(|e| e.get("scroll_offset"))
                .is_none(),
            "Project config should NOT contain scroll_offset (it's a system default)"
        );

        drop(temp);
    }

    /// Known limitation of save_to_layer: when a value is set to match the parent layer,
    /// save_to_layer cannot distinguish this from "value unchanged" and may preserve
    /// the old file value due to the merge behavior.
    ///
    /// Use save_changes_to_layer with explicit deletions for workflows that need this.
    #[test]
    #[ignore = "Known limitation: save_to_layer cannot remove values that match parent layer"]
    fn save_to_layer_removes_inherited_values() {
        let (temp, resolver) = create_test_resolver();

        // Create user config with tab_size=2
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        std::fs::write(&user_config_path, r#"{"editor": {"tab_size": 2}}"#).unwrap();

        // Create project config with tab_size=8
        let project_config_path = resolver.project_config_write_path();
        std::fs::create_dir_all(project_config_path.parent().unwrap()).unwrap();
        std::fs::write(&project_config_path, r#"{"editor": {"tab_size": 8}}"#).unwrap();

        // Resolve config
        let mut config = resolver.resolve().unwrap();
        assert_eq!(config.editor.tab_size, 8);

        // Set tab_size back to the user value (2)
        config.editor.tab_size = 2;

        // Save to project layer
        resolver
            .save_to_layer(&config, ConfigLayer::Project)
            .unwrap();

        // Read the project config - tab_size should be removed (same as parent)
        let content = std::fs::read_to_string(&project_config_path).unwrap();
        let json: serde_json::Value = serde_json::from_str(&content).unwrap();

        // Should not have editor.tab_size since it matches the user value
        assert!(
            json.get("editor").and_then(|e| e.get("tab_size")).is_none(),
            "Project config should NOT contain tab_size when it matches user layer"
        );

        drop(temp);
    }

    /// Issue #630 FIX: save_to_layer saves only the delta, defaults are inherited.
    ///
    /// The save_to_layer method correctly:
    /// 1. Saves only settings that differ from defaults
    /// 2. Loads correctly because defaults are applied during resolve()
    ///
    /// This test verifies that modifying a config and saving works correctly.
    #[test]
    fn issue_630_save_to_file_strips_settings_matching_defaults() {
        let (_temp, resolver) = create_test_resolver();

        // Create a config with some non-default settings
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        std::fs::write(
            &user_config_path,
            r#"{
                "theme": "dracula",
                "editor": {
                    "tab_size": 2
                }
            }"#,
        )
        .unwrap();

        // Load the config
        let mut config = resolver.resolve().unwrap();
        assert_eq!(config.theme.0, "dracula");
        assert_eq!(config.editor.tab_size, 2);

        // User disables LSP via UI
        if let Some(lsp_configs) = config.lsp.get_mut("python") {
            for c in lsp_configs.as_mut_slice().iter_mut() {
                c.enabled = false;
            }
        }

        // Save using save_to_layer
        resolver.save_to_layer(&config, ConfigLayer::User).unwrap();

        // Read back the saved config file
        let content = std::fs::read_to_string(&user_config_path).unwrap();
        let json: serde_json::Value = serde_json::from_str(&content).unwrap();

        eprintln!(
            "Saved config:\n{}",
            serde_json::to_string_pretty(&json).unwrap()
        );

        // Verify the delta contains what we changed
        assert_eq!(
            json.get("theme").and_then(|v| v.as_str()),
            Some("dracula"),
            "Theme should be saved (differs from default)"
        );
        assert_eq!(
            json.get("editor")
                .and_then(|e| e.get("tab_size"))
                .and_then(|v| v.as_u64()),
            Some(2),
            "tab_size should be saved (differs from default)"
        );
        assert_eq!(
            json.get("lsp")
                .and_then(|l| l.get("python"))
                .and_then(|p| p.get("enabled"))
                .and_then(|v| v.as_bool()),
            Some(false),
            "lsp.python.enabled should be saved (differs from default)"
        );

        // Reload and verify the full config is correct
        let reloaded = resolver.resolve().unwrap();
        assert_eq!(reloaded.theme.0, "dracula");
        assert_eq!(reloaded.editor.tab_size, 2);
        assert!(!reloaded.lsp["python"].as_slice()[0].enabled);
        // Command should come from defaults
        assert_eq!(reloaded.lsp["python"].as_slice()[0].command, "pylsp");
    }

    /// Test that toggling LSP enabled/disabled preserves the command field.
    ///
    /// 1. Start with empty config (defaults apply, python has command "pylsp")
    /// 2. Disable python LSP, save
    /// 3. Load, enable python LSP, save
    /// 4. Load and verify command is still the default
    #[test]
    fn toggle_lsp_preserves_command() {
        let (_temp, resolver) = create_test_resolver();
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();

        // Step 1: Empty config - defaults apply (python has command "pylsp")
        std::fs::write(&user_config_path, r#"{}"#).unwrap();

        // Load and verify default command
        let config = resolver.resolve().unwrap();
        let original_command = config.lsp["python"].as_slice()[0].command.clone();
        assert!(
            !original_command.is_empty(),
            "Default python LSP should have a command"
        );

        // Step 2: Disable python LSP, save
        let mut config = resolver.resolve().unwrap();
        config.lsp.get_mut("python").unwrap().as_mut_slice()[0].enabled = false;
        resolver.save_to_layer(&config, ConfigLayer::User).unwrap();

        // Verify saved file only has enabled:false, not empty command/args
        let saved_content = std::fs::read_to_string(&user_config_path).unwrap();
        assert!(
            !saved_content.contains(r#""command""#),
            "Saved config should not contain 'command' field. File content: {}",
            saved_content
        );
        assert!(
            !saved_content.contains(r#""args""#),
            "Saved config should not contain 'args' field. File content: {}",
            saved_content
        );

        // Step 3: Load again, enable python LSP, save
        let mut config = resolver.resolve().unwrap();
        assert!(!config.lsp["python"].as_slice()[0].enabled);
        config.lsp.get_mut("python").unwrap().as_mut_slice()[0].enabled = true;
        resolver.save_to_layer(&config, ConfigLayer::User).unwrap();

        // Step 4: Load and verify command is still the same
        let config = resolver.resolve().unwrap();
        assert_eq!(
            config.lsp["python"].as_slice()[0].command,
            original_command,
            "Command should be preserved after toggling enabled. Got: '{}'",
            config.lsp["python"].as_slice()[0].command
        );
    }

    /// Issue #631 REPRODUCTION: Config with disabled LSP (no command) should be valid.
    ///
    /// Users write configs like:
    /// ```json
    /// { "lsp": { "python": { "enabled": false } } }
    /// ```
    /// This SHOULD be valid - a disabled LSP doesn't need a command.
    /// But currently it FAILS because `command` is required.
    ///
    /// THIS TEST WILL FAIL until the bug is fixed.
    #[test]
    fn issue_631_disabled_lsp_without_command_should_be_valid() {
        let (_temp, resolver) = create_test_resolver();

        // Create the exact config from issue #631 - disabled LSP without command field
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();
        std::fs::write(
            &user_config_path,
            r#"{
                "lsp": {
                    "json": { "enabled": false },
                    "python": { "enabled": false },
                    "toml": { "enabled": false }
                },
                "theme": "dracula"
            }"#,
        )
        .unwrap();

        // Try to load this config - it SHOULD succeed
        let result = resolver.resolve();

        // THIS ASSERTION FAILS - demonstrating bug #631
        // A disabled LSP config should NOT require a command field
        assert!(
            result.is_ok(),
            "BUG #631: Config with disabled LSP should be valid even without 'command' field. \
             Got parse error: {:?}",
            result.err()
        );

        // Verify the theme was loaded (config parsed correctly)
        let config = result.unwrap();
        assert_eq!(
            config.theme.0, "dracula",
            "Theme should be 'dracula' from config file"
        );
    }

    /// Test that loading a config without command field uses the default command.
    #[test]
    fn loading_lsp_without_command_uses_default() {
        let (_temp, resolver) = create_test_resolver();
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();

        // Write config with rust LSP but no command field
        std::fs::write(
            &user_config_path,
            r#"{ "lsp": { "rust": { "enabled": false } } }"#,
        )
        .unwrap();

        // Load and check that command comes from defaults
        let config = resolver.resolve().unwrap();
        assert_eq!(
            config.lsp["rust"].as_slice()[0].command,
            "rust-analyzer",
            "Command should come from defaults when not in file. Got: '{}'",
            config.lsp["rust"].as_slice()[0].command
        );
        assert!(
            !config.lsp["rust"].as_slice()[0].enabled,
            "enabled should be false from file"
        );
    }

    /// Test simulating the Settings UI flow using save_changes_to_layer:
    /// 1. Load config with defaults
    /// 2. Apply change (toggle enabled) via JSON pointer (like Settings UI does)
    /// 3. Save via save_changes_to_layer with explicit changes
    /// 4. Reload and verify command is preserved
    #[test]
    fn settings_ui_toggle_lsp_preserves_command() {
        let (_temp, resolver) = create_test_resolver();
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();

        // Step 1: Start with empty config
        std::fs::write(&user_config_path, r#"{}"#).unwrap();

        // Load resolved config - should have rust with command="rust-analyzer"
        let config = resolver.resolve().unwrap();
        assert_eq!(
            config.lsp["rust"].as_slice()[0].command,
            "rust-analyzer",
            "Default rust command should be rust-analyzer"
        );
        assert!(
            config.lsp["rust"].as_slice()[0].enabled,
            "Default rust enabled should be true"
        );

        // Step 2: Simulate Settings UI applying a change to disable rust LSP
        // Using save_changes_to_layer with explicit change tracking
        let mut changes = std::collections::HashMap::new();
        changes.insert("/lsp/rust/enabled".to_string(), serde_json::json!(false));
        let deletions = std::collections::HashSet::new();

        // Step 3: Save via save_changes_to_layer
        resolver
            .save_changes_to_layer(&changes, &deletions, ConfigLayer::User)
            .unwrap();

        // Check what was saved to file
        let saved_content = std::fs::read_to_string(&user_config_path).unwrap();
        eprintln!("After disable, file contains:\n{}", saved_content);

        // Step 4: Reload and verify command is preserved
        let reloaded = resolver.resolve().unwrap();
        assert_eq!(
            reloaded.lsp["rust"].as_slice()[0].command,
            "rust-analyzer",
            "Command should be preserved after save/reload (disabled). Got: '{}'",
            reloaded.lsp["rust"].as_slice()[0].command
        );
        assert!(
            !reloaded.lsp["rust"].as_slice()[0].enabled,
            "rust should be disabled"
        );

        // Step 5: Re-enable rust LSP (simulating Settings UI)
        let mut changes = std::collections::HashMap::new();
        changes.insert("/lsp/rust/enabled".to_string(), serde_json::json!(true));
        let deletions = std::collections::HashSet::new();

        // Step 6: Save via save_changes_to_layer
        resolver
            .save_changes_to_layer(&changes, &deletions, ConfigLayer::User)
            .unwrap();

        // Check what was saved to file
        let saved_content = std::fs::read_to_string(&user_config_path).unwrap();
        eprintln!("After re-enable, file contains:\n{}", saved_content);

        // Step 7: Reload and verify command is STILL preserved
        let final_config = resolver.resolve().unwrap();
        assert_eq!(
            final_config.lsp["rust"].as_slice()[0].command,
            "rust-analyzer",
            "Command should be preserved after toggle cycle. Got: '{}'",
            final_config.lsp["rust"].as_slice()[0].command
        );
        assert!(
            final_config.lsp["rust"].as_slice()[0].enabled,
            "rust should be enabled"
        );
    }

    /// Issue #806 REPRODUCTION: Manual config.json edits are lost when saving from Settings UI.
    ///
    /// Scenario:
    /// 1. User manually edits config.json to add custom LSP settings (e.g., rust-analyzer with custom args)
    /// 2. User opens Settings UI and changes a simple setting (e.g., tab_size)
    /// 3. User saves the settings
    /// 4. Result: The manually-added LSP settings are GONE
    ///
    /// Expected behavior: Only the changed setting (tab_size) should be modified;
    /// the manually-added LSP settings should be preserved.
    #[test]
    fn issue_806_manual_config_edits_lost_when_saving_from_ui() {
        let (_temp, resolver) = create_test_resolver();
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();

        // Step 1: User manually creates config.json with custom LSP settings
        // This is the EXACT example from issue #806
        std::fs::write(
            &user_config_path,
            r#"{
                "lsp": {
                    "rust-analyzer": {
                        "enabled": true,
                        "command": "rust-analyzer",
                        "args": ["--log-file", "/tmp/rust-analyzer-{pid}.log"],
                        "languages": ["rust"]
                    }
                }
            }"#,
        )
        .unwrap();

        // Step 2: Load the config (simulating Fresh startup)
        let config = resolver.resolve().unwrap();

        // Verify the custom LSP settings were loaded
        assert!(
            config.lsp.contains_key("rust-analyzer"),
            "Config should contain manually-added 'rust-analyzer' LSP entry"
        );
        let rust_analyzer = &config.lsp["rust-analyzer"].as_slice()[0];
        assert!(rust_analyzer.enabled, "rust-analyzer should be enabled");
        assert_eq!(
            rust_analyzer.command, "rust-analyzer",
            "rust-analyzer command should be preserved"
        );
        assert_eq!(
            rust_analyzer.args,
            vec!["--log-file", "/tmp/rust-analyzer-{pid}.log"],
            "rust-analyzer args should be preserved"
        );

        // Step 3: User opens Settings UI and changes tab_size
        // This simulates what SettingsState::apply_changes does
        let mut config_json = serde_json::to_value(&config).unwrap();
        *config_json
            .pointer_mut("/editor/tab_size")
            .expect("path should exist") = serde_json::json!(2);
        let modified_config: crate::config::Config =
            serde_json::from_value(config_json).expect("should deserialize");

        // Step 4: Save via save_to_layer (what save_settings() does)
        resolver
            .save_to_layer(&modified_config, ConfigLayer::User)
            .unwrap();

        // Step 5: Check what was saved to file
        let saved_content = std::fs::read_to_string(&user_config_path).unwrap();
        let saved_json: serde_json::Value = serde_json::from_str(&saved_content).unwrap();

        eprintln!(
            "Issue #806 - Saved config after changing tab_size:\n{}",
            serde_json::to_string_pretty(&saved_json).unwrap()
        );

        // CRITICAL ASSERTION: The "lsp" section with "rust-analyzer" MUST still be present
        assert!(
            saved_json.get("lsp").is_some(),
            "BUG #806: 'lsp' section should NOT be deleted when saving unrelated changes. \
             File content: {}",
            saved_content
        );

        assert!(
            saved_json
                .get("lsp")
                .and_then(|l| l.get("rust-analyzer"))
                .is_some(),
            "BUG #806: 'lsp.rust-analyzer' should NOT be deleted when saving unrelated changes. \
             File content: {}",
            saved_content
        );

        // Verify the custom args are preserved
        let saved_args = saved_json
            .get("lsp")
            .and_then(|l| l.get("rust-analyzer"))
            .and_then(|r| r.get("args"));
        assert!(
            saved_args.is_some(),
            "BUG #806: 'lsp.rust-analyzer.args' should be preserved. File content: {}",
            saved_content
        );
        assert_eq!(
            saved_args.unwrap(),
            &serde_json::json!(["--log-file", "/tmp/rust-analyzer-{pid}.log"]),
            "BUG #806: Custom args should be preserved exactly"
        );

        // Verify the tab_size change was saved
        assert_eq!(
            saved_json
                .get("editor")
                .and_then(|e| e.get("tab_size"))
                .and_then(|v| v.as_u64()),
            Some(2),
            "tab_size should be saved"
        );

        // Step 6: Reload and verify everything is intact
        let reloaded = resolver.resolve().unwrap();
        assert_eq!(
            reloaded.editor.tab_size, 2,
            "tab_size change should be persisted"
        );
        assert!(
            reloaded.lsp.contains_key("rust-analyzer"),
            "BUG #806: rust-analyzer should still exist after reload"
        );
        let reloaded_ra = &reloaded.lsp["rust-analyzer"].as_slice()[0];
        assert_eq!(
            reloaded_ra.args,
            vec!["--log-file", "/tmp/rust-analyzer-{pid}.log"],
            "BUG #806: Custom args should survive save/reload cycle"
        );
    }

    /// Issue #806 - Variant: Test with multiple custom settings that don't exist in defaults.
    ///
    /// This tests a broader scenario where the user has added multiple custom
    /// configurations that are not part of the default config structure.
    #[test]
    fn issue_806_custom_lsp_entries_preserved_across_unrelated_changes() {
        let (_temp, resolver) = create_test_resolver();
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();

        // User creates config with a completely custom LSP server not in defaults
        std::fs::write(
            &user_config_path,
            r#"{
                "theme": "dracula",
                "lsp": {
                    "my-custom-lsp": {
                        "enabled": true,
                        "command": "/usr/local/bin/my-custom-lsp",
                        "args": ["--verbose", "--config", "/etc/my-lsp.json"],
                        "languages": ["mycustomlang"]
                    }
                },
                "languages": {
                    "mycustomlang": {
                        "extensions": [".mcl"],
                        "grammar": "mycustomlang"
                    }
                }
            }"#,
        )
        .unwrap();

        // Load and verify custom settings exist
        let config = resolver.resolve().unwrap();
        assert!(
            config.lsp.contains_key("my-custom-lsp"),
            "Custom LSP entry should be loaded"
        );
        assert!(
            config.languages.contains_key("mycustomlang"),
            "Custom language should be loaded"
        );

        // User changes only line_numbers in Settings UI
        let mut config_json = serde_json::to_value(&config).unwrap();
        *config_json
            .pointer_mut("/editor/line_numbers")
            .expect("path should exist") = serde_json::json!(false);
        let modified_config: crate::config::Config =
            serde_json::from_value(config_json).expect("should deserialize");

        // Save
        resolver
            .save_to_layer(&modified_config, ConfigLayer::User)
            .unwrap();

        // Verify file still contains custom LSP
        let saved_content = std::fs::read_to_string(&user_config_path).unwrap();
        let saved_json: serde_json::Value = serde_json::from_str(&saved_content).unwrap();

        eprintln!(
            "Saved config:\n{}",
            serde_json::to_string_pretty(&saved_json).unwrap()
        );

        // Custom LSP must be preserved
        assert!(
            saved_json
                .get("lsp")
                .and_then(|l| l.get("my-custom-lsp"))
                .is_some(),
            "BUG #806: Custom LSP 'my-custom-lsp' should be preserved. Got: {}",
            saved_content
        );

        // Custom language must be preserved
        assert!(
            saved_json
                .get("languages")
                .and_then(|l| l.get("mycustomlang"))
                .is_some(),
            "BUG #806: Custom language 'mycustomlang' should be preserved. Got: {}",
            saved_content
        );

        // Reload and verify
        let reloaded = resolver.resolve().unwrap();
        assert!(
            reloaded.lsp.contains_key("my-custom-lsp"),
            "Custom LSP should survive save/reload"
        );
        assert!(
            reloaded.languages.contains_key("mycustomlang"),
            "Custom language should survive save/reload"
        );
        assert!(
            !reloaded.editor.line_numbers,
            "line_numbers change should be applied"
        );
    }

    /// Issue #806 - Scenario 2: External file modification after Fresh is running.
    ///
    /// This is the most likely real-world scenario:
    /// 1. User starts Fresh with default/existing config (loaded into memory)
    /// 2. User manually edits config.json WHILE Fresh is running (external edit)
    /// 3. User opens Settings UI in Fresh and changes a simple setting
    /// 4. User saves from Settings UI
    /// 5. BUG: The external edits are LOST because Fresh's in-memory config
    ///    doesn't have them
    ///
    /// This test verifies that even if the file was modified externally,
    /// the save operation should preserve those external changes.
    #[test]
    fn issue_806_external_file_modification_lost_on_ui_save() {
        let (_temp, resolver) = create_test_resolver();
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();

        // Step 1: User starts Fresh with a simple config
        std::fs::write(&user_config_path, r#"{"theme": "monokai"}"#).unwrap();

        // Step 2: Fresh loads the config (simulating startup)
        let config_at_startup = resolver.resolve().unwrap();
        assert_eq!(config_at_startup.theme.0, "monokai");
        assert!(
            !config_at_startup.lsp.contains_key("rust-analyzer"),
            "No custom LSP at startup"
        );

        // Step 3: User externally edits config.json (e.g., with another editor)
        // to add custom LSP settings. Fresh doesn't see this change yet.
        std::fs::write(
            &user_config_path,
            r#"{
                "theme": "monokai",
                "lsp": {
                    "rust-analyzer": {
                        "enabled": true,
                        "command": "rust-analyzer",
                        "args": ["--log-file", "/tmp/ra.log"]
                    }
                }
            }"#,
        )
        .unwrap();

        // Step 4: User opens Settings UI and changes tab_size
        // The Settings UI works with the IN-MEMORY config (config_at_startup)
        // which does NOT have the external LSP changes
        let mut config_json = serde_json::to_value(&config_at_startup).unwrap();
        *config_json
            .pointer_mut("/editor/tab_size")
            .expect("path should exist") = serde_json::json!(2);
        let modified_config: crate::config::Config =
            serde_json::from_value(config_json).expect("should deserialize");

        // Step 5: User saves from Settings UI
        // This is where the bug occurs - the in-memory config (without LSP)
        // is saved, overwriting the external changes
        resolver
            .save_to_layer(&modified_config, ConfigLayer::User)
            .unwrap();

        // Step 6: Check what was saved
        let saved_content = std::fs::read_to_string(&user_config_path).unwrap();
        let saved_json: serde_json::Value = serde_json::from_str(&saved_content).unwrap();

        eprintln!(
            "Issue #806 scenario 2 - After UI save (external edits should be preserved):\n{}",
            serde_json::to_string_pretty(&saved_json).unwrap()
        );

        // This assertion will FAIL if the bug exists
        // The LSP section added externally should be preserved
        // BUT with current implementation, it will be LOST because
        // save_to_layer computes delta from in-memory config (which has no LSP)
        // vs system defaults, NOT from the current file contents
        assert!(
            saved_json.get("lsp").is_some(),
            "BUG #806: External edits to config.json were lost! \
             The 'lsp' section added while Fresh was running should be preserved. \
             Saved content: {}",
            saved_content
        );

        assert!(
            saved_json
                .get("lsp")
                .and_then(|l| l.get("rust-analyzer"))
                .is_some(),
            "BUG #806: rust-analyzer config should be preserved"
        );
    }

    /// Issue #806 - Scenario 3: Multiple users/processes editing config
    ///
    /// Even more edge case: Config is modified by another process right before save.
    /// This demonstrates that save_to_layer() should ideally do a read-modify-write
    /// operation, not just a write.
    #[test]
    fn issue_806_concurrent_modification_scenario() {
        let (_temp, resolver) = create_test_resolver();
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();

        // Start with empty config
        std::fs::write(&user_config_path, r#"{}"#).unwrap();

        // Load config
        let mut config = resolver.resolve().unwrap();

        // Modify in memory: change tab_size
        config.editor.tab_size = 8;

        // Meanwhile, another process adds LSP config to the file
        std::fs::write(
            &user_config_path,
            r#"{
                "lsp": {
                    "custom-lsp": {
                        "enabled": true,
                        "command": "/usr/bin/custom-lsp"
                    }
                }
            }"#,
        )
        .unwrap();

        // Now save our in-memory config
        // With current implementation, this will OVERWRITE the concurrent changes
        resolver.save_to_layer(&config, ConfigLayer::User).unwrap();

        // Check result
        let saved_content = std::fs::read_to_string(&user_config_path).unwrap();
        let saved_json: serde_json::Value = serde_json::from_str(&saved_content).unwrap();

        eprintln!(
            "Concurrent modification scenario result:\n{}",
            serde_json::to_string_pretty(&saved_json).unwrap()
        );

        // Verify our change was saved
        assert_eq!(
            saved_json
                .get("editor")
                .and_then(|e| e.get("tab_size"))
                .and_then(|v| v.as_u64()),
            Some(8),
            "Our tab_size change should be saved"
        );

        // The concurrent LSP change will be lost with current implementation
        // This is a known limitation - documenting it here
        // A proper fix would involve read-modify-write with conflict detection
        //
        // For now, we just document that this scenario loses concurrent changes:
        let lsp_preserved = saved_json.get("lsp").is_some();
        if !lsp_preserved {
            eprintln!(
                "NOTE: Concurrent file modifications are lost with current implementation. \
                 This is expected behavior but could be improved with read-modify-write pattern."
            );
        }
    }

    /// Bug reproduction: changing a config value to match the default should persist.
    ///
    /// When a user changes a setting FROM a non-default value TO the default value,
    /// the change should be saved (either by writing the default value explicitly,
    /// or by removing the field so the default propagates).
    ///
    /// The bug: save_to_layer computes delta vs defaults, so changing TO default
    /// results in no delta for that field. The merge with existing file then
    /// preserves the OLD value from the file instead of the new default.
    #[test]
    fn save_to_layer_changing_to_default_value_should_persist() {
        let (_temp, resolver) = create_test_resolver();
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();

        // Step 1: Create user config with non-default theme
        std::fs::write(&user_config_path, r#"{"theme": "dracula"}"#).unwrap();

        // Step 2: Load config - theme should be "dracula" from file
        let baseline = resolver.resolve().unwrap();
        assert_eq!(
            baseline.theme.0, "dracula",
            "Theme should be 'dracula' from file"
        );

        // Step 3: User changes theme to the DEFAULT value ("high-contrast")
        let mut config = baseline.clone();
        config.theme = crate::config::ThemeName::from("high-contrast");

        // Step 4: Save the change using baseline tracking
        resolver
            .save_to_layer_with_baseline(&config, &baseline, ConfigLayer::User)
            .unwrap();

        // Step 5: Check what was saved to file
        let saved_content = std::fs::read_to_string(&user_config_path).unwrap();
        eprintln!(
            "Saved config after changing to default theme:\n{}",
            saved_content
        );

        // Step 6: Reload config
        let reloaded = resolver.resolve().unwrap();

        // The theme should be "high-contrast" (either explicitly in file, or absent so default applies)
        assert_eq!(
            reloaded.theme.0, "high-contrast",
            "Theme should be 'high-contrast' after changing to default and saving. \
             With save_to_layer_with_baseline, the theme field should be removed from file \
             so the default applies. File content: {}",
            saved_content
        );
    }

    /// Test that universal_lsp config round-trips through save/load correctly.
    /// This exercises the PartialConfig From/resolve_with_defaults paths.
    #[test]
    fn universal_lsp_round_trip_via_config_resolver() {
        let (_temp, resolver) = create_test_resolver();
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();

        // Write a config that enables quicklsp
        std::fs::write(
            &user_config_path,
            r#"{
                "universal_lsp": {
                    "quicklsp": { "enabled": true, "auto_start": true }
                }
            }"#,
        )
        .unwrap();

        let config = resolver.resolve().unwrap();

        // quicklsp should be enabled (user override merged with defaults)
        assert!(config.universal_lsp.contains_key("quicklsp"));
        let server = &config.universal_lsp["quicklsp"].as_slice()[0];
        assert!(server.enabled, "User override should enable quicklsp");
        assert!(server.auto_start, "User override should enable auto_start");
        assert_eq!(
            server.command, "quicklsp",
            "Command should come from defaults"
        );
    }

    /// Test that universal_lsp supports adding custom servers alongside defaults.
    #[test]
    fn universal_lsp_custom_server_merges_with_defaults() {
        let (_temp, resolver) = create_test_resolver();
        let user_config_path = resolver.user_config_path();
        std::fs::create_dir_all(user_config_path.parent().unwrap()).unwrap();

        std::fs::write(
            &user_config_path,
            r#"{
                "universal_lsp": {
                    "my-universal-server": {
                        "command": "my-server-bin",
                        "enabled": true
                    }
                }
            }"#,
        )
        .unwrap();

        let config = resolver.resolve().unwrap();

        // Custom server should be present
        assert!(
            config.universal_lsp.contains_key("my-universal-server"),
            "Custom universal server should be loaded"
        );
        assert_eq!(
            config.universal_lsp["my-universal-server"].as_slice()[0].command,
            "my-server-bin"
        );

        // Default quicklsp should still be present (merged from defaults)
        assert!(
            config.universal_lsp.contains_key("quicklsp"),
            "Default quicklsp should be preserved when adding custom servers"
        );
    }

    /// Test that the PartialConfig conversion (Config -> PartialConfig -> Config)
    /// preserves universal_lsp entries. This catches bugs where universal_lsp
    /// is missing from the PartialConfig struct or its From/resolve impls.
    #[test]
    fn universal_lsp_partial_config_round_trip() {
        use crate::partial_config::PartialConfig;

        let mut config = Config::default();
        // Enable quicklsp in the original config
        if let Some(quicklsp) = config.universal_lsp.get_mut("quicklsp") {
            quicklsp.as_mut_slice()[0].enabled = true;
        }

        // Convert to partial and back
        let partial = PartialConfig::from(&config);
        let resolved = partial.resolve();

        // Verify universal_lsp survived the round trip
        assert!(
            resolved.universal_lsp.contains_key("quicklsp"),
            "quicklsp should survive Config -> PartialConfig -> Config round trip"
        );
        assert!(
            resolved.universal_lsp["quicklsp"].as_slice()[0].enabled,
            "quicklsp enabled state should be preserved through round trip"
        );
    }
}