cflx 0.6.327

//! Configuration module for Conflux.
//!
//! Supports JSONC format (JSON with Comments) for configuration files.
//! Configuration is loaded with the following priority:
//! 1. Custom config path (if provided)
//! 2. Project config: `.cflx.jsonc`
//! 3. Global config (XDG): `$XDG_CONFIG_HOME/cflx/config.jsonc` (if XDG_CONFIG_HOME is set)
//! 4. Global config (XDG default): `~/.config/cflx/config.jsonc`
//! 5. Global config (platform default): `dirs::config_dir()/cflx/config.jsonc`
//! 6. Default values (OpenCode-based commands)
//!
//! # Module Structure
//!
//! - `defaults` - Default values and path constants
//! - `expand` - Placeholder expansion utilities
//! - `jsonc` - JSONC parser (reusable by other modules)
//! - `types`  - All configuration struct/enum definitions and business-logic impls
//! - `load`   - File I/O: loading and parsing configuration files

pub mod defaults;
pub mod expand;
pub mod jsonc;
mod load;
mod types;

pub use types::*;

use std::path::PathBuf;

/// Get the XDG config path from environment variable ($XDG_CONFIG_HOME)
///
/// Returns `$XDG_CONFIG_HOME/cflx/config.jsonc` if XDG_CONFIG_HOME is set and non-empty.
fn get_xdg_env_config_path() -> Option<PathBuf> {
    if let Ok(xdg_config_home) = std::env::var("XDG_CONFIG_HOME") {
        if !xdg_config_home.is_empty() {
            return Some(
                PathBuf::from(xdg_config_home)
                    .join(GLOBAL_CONFIG_DIR)
                    .join(GLOBAL_CONFIG_FILE),
            );
        }
    }
    None
}

/// Get the XDG config path from default location (~/.config)
///
/// Returns `~/.config/cflx/config.jsonc` if home directory is available.
fn get_xdg_default_config_path() -> Option<PathBuf> {
    dirs::home_dir().map(|home| {
        home.join(".config")
            .join(GLOBAL_CONFIG_DIR)
            .join(GLOBAL_CONFIG_FILE)
    })
}

/// Get the XDG config path, checking $XDG_CONFIG_HOME first, then falling back to ~/.config
///
/// Deprecated: Use get_xdg_env_config_path() and get_xdg_default_config_path() for explicit priority.
/// Returns:
/// - `$XDG_CONFIG_HOME/cflx/config.jsonc` if XDG_CONFIG_HOME is set
/// - `~/.config/cflx/config.jsonc` otherwise
#[deprecated(
    since = "0.1.0",
    note = "Use get_xdg_env_config_path() and get_xdg_default_config_path() for explicit priority"
)]
#[allow(dead_code)]
fn get_xdg_config_path() -> Option<PathBuf> {
    get_xdg_env_config_path().or_else(get_xdg_default_config_path)
}

/// Get the path to the global configuration file (platform default)
///
/// Returns platform-specific config directory + `cflx/config.jsonc`
/// - macOS: `~/Library/Application Support/cflx/config.jsonc`
/// - Linux: `~/.config/cflx/config.jsonc`
/// - Windows: `%APPDATA%/cflx/config.jsonc`
fn get_platform_config_path() -> Option<PathBuf> {
    dirs::config_dir().map(|config_dir| config_dir.join(GLOBAL_CONFIG_DIR).join(GLOBAL_CONFIG_FILE))
}

/// Collect global config candidate paths in merge priority order (low → high).
///
/// Order:
/// 1. Platform default: `dirs::config_dir()/cflx/config.jsonc`
/// 2. XDG default: `~/.config/cflx/config.jsonc`
/// 3. XDG env: `$XDG_CONFIG_HOME/cflx/config.jsonc`
fn get_global_config_paths() -> Vec<PathBuf> {
    let mut paths = Vec::new();

    if let Some(platform_path) = get_platform_config_path() {
        paths.push(platform_path);
    }

    if let Some(xdg_default_path) = get_xdg_default_config_path() {
        paths.push(xdg_default_path);
    }

    if let Some(xdg_env_path) = get_xdg_env_config_path() {
        paths.push(xdg_env_path);
    }

    paths
}

/// Get the path to the global configuration file
///
/// Deprecated: Use get_xdg_env_config_path() and get_xdg_default_config_path() for explicit priority.
/// This function now delegates to get_xdg_env_config_path() for backwards compatibility.
#[deprecated(
    since = "0.1.0",
    note = "Use get_xdg_env_config_path() and get_xdg_default_config_path() for explicit priority"
)]
#[allow(dead_code)]
#[allow(deprecated)]
pub fn get_global_config_path() -> Option<PathBuf> {
    get_xdg_env_config_path().or_else(get_xdg_default_config_path)
}

// Re-export commonly used items for convenience; also brings them into scope for path helpers.
#[allow(unused_imports)]
pub use defaults::{
    DEFAULT_ACCEPTANCE_MAX_CONTINUES, DEFAULT_APPLY_PROMPT, DEFAULT_ARCHIVE_PROMPT,
    DEFAULT_MAX_CONCURRENT_WORKSPACES, DEFAULT_MAX_ITERATIONS, GLOBAL_CONFIG_DIR,
    GLOBAL_CONFIG_FILE, PROJECT_CONFIG_FILE,
};

// Re-export external types referenced in tests and public API
#[allow(unused_imports)]
pub use crate::vcs::VcsBackend;

#[cfg(test)]
mod tests {
    use super::*;
    use std::collections::HashMap;
    use std::sync::{Mutex, MutexGuard, OnceLock};

    fn env_test_lock() -> MutexGuard<'static, ()> {
        static ENV_TEST_MUTEX: OnceLock<Mutex<()>> = OnceLock::new();
        ENV_TEST_MUTEX
            .get_or_init(|| Mutex::new(()))
            .lock()
            .unwrap()
    }

    fn set_env_var<K: AsRef<std::ffi::OsStr>, V: AsRef<std::ffi::OsStr>>(key: K, value: V) {
        // SAFETY: All tests that mutate process-wide environment variables in this module
        // take `env_test_lock()` first, ensuring serialized access.
        unsafe { std::env::set_var(key, value) }
    }

    fn remove_env_var<K: AsRef<std::ffi::OsStr>>(key: K) {
        // SAFETY: All tests that mutate process-wide environment variables in this module
        // take `env_test_lock()` first, ensuring serialized access.
        unsafe { std::env::remove_var(key) }
    }

    #[test]
    fn test_default_config() {
        let config = OrchestratorConfig::default();
        assert!(config.apply_command.is_none());
        assert!(config.archive_command.is_none());
        assert!(config.analyze_command.is_none());
    }

    #[test]
    fn test_default_logging_config() {
        let config = OrchestratorConfig::default();
        assert_eq!(config.get_logging(), LoggingConfig::default());
    }

    #[test]
    fn test_parse_command_envs_config() {
        let _guard = env_test_lock();
        set_env_var("CFLX_ENV_PARENT", "parent-value");
        let config = OrchestratorConfig::parse_jsonc(
            r#"{
                "envs": {
                    "CFLX_LITERAL": "literal-value",
                    "CFLX_EXPANDED": "$CFLX_ENV_PARENT/${CFLX_ENV_PARENT}"
                }
            }"#,
        )
        .unwrap();

        let envs = config.get_command_envs();
        assert_eq!(envs.get("CFLX_LITERAL").unwrap(), "literal-value");
        assert_eq!(
            envs.get("CFLX_EXPANDED").unwrap(),
            "parent-value/parent-value"
        );
    }

    #[test]
    fn test_merge_command_envs_key_wise() {
        let mut lower = OrchestratorConfig::parse_jsonc(
            r#"{
                "envs": {
                    "KEEP": "lower",
                    "OVERRIDE": "lower"
                }
            }"#,
        )
        .unwrap();
        let higher = OrchestratorConfig::parse_jsonc(
            r#"{
                "envs": {
                    "OVERRIDE": "higher",
                    "HIGH_ONLY": "higher"
                }
            }"#,
        )
        .unwrap();

        lower.merge(higher);
        let envs = lower.get_command_envs();
        assert_eq!(envs.get("KEEP").unwrap(), "lower");
        assert_eq!(envs.get("OVERRIDE").unwrap(), "higher");
        assert_eq!(envs.get("HIGH_ONLY").unwrap(), "higher");
    }

    #[test]
    fn test_parse_logging_config() {
        let jsonc = r#"{
            "logging": {
                "suppress_repetitive_debug": false,
                "summary_interval_secs": 15
            }
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        let logging = config.get_logging();
        assert!(!logging.suppress_repetitive_debug);
        assert_eq!(logging.summary_interval_secs, 15);
    }

    #[test]
    fn test_stall_detection_defaults() {
        let config = OrchestratorConfig::default();
        assert_eq!(
            config.get_stall_detection(),
            StallDetectionConfig::default()
        );
        assert!(config.get_stall_detection().enabled);
        assert_eq!(config.get_stall_detection().threshold, 5);
    }

    #[test]
    fn test_parse_stall_detection_config() {
        let jsonc = r#"{
            "stall_detection": {
                "enabled": false,
                "threshold": 5
            }
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        let stall = config.get_stall_detection();
        assert!(!stall.enabled);
        assert_eq!(stall.threshold, 5);
        assert_eq!(stall.apply_escalation_after_empty_wip, None);
        assert_eq!(stall.apply_escalation_max_uses_per_stall, None);
    }

    #[test]
    fn test_parse_apply_escalation_and_diagnose_config() {
        let jsonc = r#"{
            "apply_command": "apply {change_id}",
            "apply_escalation_command": "apply-deep {change_id} {prompt}",
            "apply_stall_diagnose_command": "diagnose {change_id} {prompt}",
            "archive_command": "archive {change_id}",
            "analyze_command": "analyze {prompt}",
            "acceptance_command": "accept {change_id}",
            "resolve_command": "resolve {prompt}",
            "stall_detection": {
                "enabled": true,
                "threshold": 5,
                "apply_escalation_after_empty_wip": 3,
                "apply_escalation_max_uses_per_stall": 2
            }
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(
            config.get_apply_escalation_command(),
            Some("apply-deep {change_id} {prompt}")
        );
        assert_eq!(
            config.get_apply_stall_diagnose_command(),
            Some("diagnose {change_id} {prompt}")
        );
        let stall = config.get_stall_detection();
        assert_eq!(stall.apply_escalation_after_empty_wip, Some(3));
        assert_eq!(stall.apply_escalation_max_uses_per_stall, Some(2));
        assert!(stall.apply_escalation_policy_enabled());
        assert!(config.validate_required_commands().is_ok());
    }

    #[test]
    fn test_apply_escalation_boundary_validation_rejects_start_at_threshold() {
        let mut config = complete_required_command_config();
        config.stall_detection = Some(StallDetectionConfig {
            enabled: true,
            threshold: 5,
            apply_escalation_after_empty_wip: Some(5),
            apply_escalation_max_uses_per_stall: Some(1),
        });

        let result = config.validate_required_commands();
        assert!(result.is_err());
        assert!(result
            .unwrap_err()
            .to_string()
            .contains("apply_escalation_after_empty_wip"));
    }

    #[test]
    fn test_apply_escalation_max_uses_validation_rejects_zero() {
        let mut config = complete_required_command_config();
        config.stall_detection = Some(StallDetectionConfig {
            enabled: true,
            threshold: 5,
            apply_escalation_after_empty_wip: Some(3),
            apply_escalation_max_uses_per_stall: Some(0),
        });

        let result = config.validate_required_commands();
        assert!(result.is_err());
        assert!(result
            .unwrap_err()
            .to_string()
            .contains("apply_escalation_max_uses_per_stall"));
    }

    #[test]
    fn test_missing_optional_apply_escalation_commands_are_silent_noop_config() {
        let mut config = complete_required_command_config();
        config.stall_detection = Some(StallDetectionConfig {
            enabled: true,
            threshold: 5,
            apply_escalation_after_empty_wip: Some(3),
            apply_escalation_max_uses_per_stall: Some(2),
        });

        assert!(config.apply_escalation_command.is_none());
        assert!(config.apply_stall_diagnose_command.is_none());
        assert_eq!(config.get_apply_escalation_command(), None);
        assert_eq!(config.get_apply_stall_diagnose_command(), None);
        assert!(config.validate_required_commands().is_ok());
    }

    #[test]
    fn acceptance_escalation_command_and_policy_are_optional_and_parsed() {
        let jsonc = r#"{
            "apply_command": "apply {change_id}",
            "archive_command": "archive {change_id}",
            "analyze_command": "analyze {prompt}",
            "acceptance_command": "accept {change_id} {prompt}",
            "acceptance_escalation_command": "deep-accept {change_id} {prompt}",
            "resolve_command": "resolve {prompt}",
            "acceptance_escalation": {
                "after_invalid_results": 2,
                "max_uses_per_sequence": 3
            }
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();

        assert_eq!(
            config.get_acceptance_command().unwrap(),
            "accept {change_id} {prompt}"
        );
        assert_eq!(
            config.get_acceptance_escalation_command(),
            Some("deep-accept {change_id} {prompt}")
        );
        let policy = config.get_acceptance_escalation();
        assert_eq!(policy.after_invalid_results(), 2);
        assert_eq!(policy.max_uses_per_sequence(), 3);
        assert!(config.validate_required_commands().is_ok());
    }

    /// Omitting the optional command must not warn or fail validation, and the
    /// policy still resolves to its bounded defaults.
    #[test]
    fn missing_acceptance_escalation_config_is_a_silent_noop() {
        let config = complete_required_command_config();

        assert!(config.acceptance_escalation_command.is_none());
        assert!(config.acceptance_escalation.is_none());
        assert_eq!(config.get_acceptance_escalation_command(), None);
        let policy = config.get_acceptance_escalation();
        assert_eq!(policy.after_invalid_results(), 1);
        assert_eq!(policy.max_uses_per_sequence(), 1);
        assert!(config.validate_required_commands().is_ok());
    }

    /// The two knobs answer different questions, so a higher-priority layer that
    /// overrides one must keep the lower-priority value of the other.
    #[test]
    fn acceptance_escalation_policy_values_merge_item_wise() {
        let mut base = complete_required_command_config();
        base.acceptance_escalation = Some(AcceptanceEscalationConfig {
            after_invalid_results: Some(2),
            max_uses_per_sequence: Some(4),
        });

        base.merge(OrchestratorConfig {
            acceptance_escalation: Some(AcceptanceEscalationConfig {
                after_invalid_results: Some(3),
                max_uses_per_sequence: None,
            }),
            ..Default::default()
        });

        let policy = base.get_acceptance_escalation();
        assert_eq!(policy.after_invalid_results(), 3);
        assert_eq!(
            policy.max_uses_per_sequence(),
            4,
            "overriding the threshold must not reset the maximum-use value"
        );
    }

    /// A layer that configures no escalation at all preserves whatever the
    /// lower-priority layer set, command and policy alike.
    #[test]
    fn acceptance_escalation_merge_preserves_unset_layers() {
        let mut base = complete_required_command_config();
        base.acceptance_escalation_command = Some("deep-accept {prompt}".to_string());
        base.acceptance_escalation = Some(AcceptanceEscalationConfig {
            after_invalid_results: Some(2),
            max_uses_per_sequence: Some(4),
        });

        base.merge(OrchestratorConfig::default());

        assert_eq!(
            base.get_acceptance_escalation_command(),
            Some("deep-accept {prompt}")
        );
        let policy = base.get_acceptance_escalation();
        assert_eq!(policy.after_invalid_results(), 2);
        assert_eq!(policy.max_uses_per_sequence(), 4);
    }

    /// Zero has no coherent meaning: disabling escalation is done by omitting
    /// the command, so a zero is reported as the operator mistake it is.
    #[test]
    fn zero_acceptance_escalation_policy_value_is_rejected() {
        for (field, policy) in [
            (
                "after_invalid_results",
                AcceptanceEscalationConfig {
                    after_invalid_results: Some(0),
                    max_uses_per_sequence: Some(1),
                },
            ),
            (
                "max_uses_per_sequence",
                AcceptanceEscalationConfig {
                    after_invalid_results: Some(1),
                    max_uses_per_sequence: Some(0),
                },
            ),
        ] {
            let mut config = complete_required_command_config();
            config.acceptance_escalation = Some(policy);

            let message = config
                .validate_required_commands()
                .expect_err("a zero policy value must fail validation")
                .to_string();
            assert!(
                message.contains(&format!("acceptance_escalation.{field}")),
                "diagnostic must name the offending field: {message}"
            );
            assert!(
                message.contains("positive integer"),
                "diagnostic must state the positive-value requirement: {message}"
            );
        }
    }

    fn complete_required_command_config() -> OrchestratorConfig {
        OrchestratorConfig {
            apply_command: Some("apply {change_id}".to_string()),
            archive_command: Some("archive {change_id}".to_string()),
            analyze_command: Some("analyze {prompt}".to_string()),
            acceptance_command: Some("accept {change_id}".to_string()),
            resolve_command: Some("resolve {prompt}".to_string()),
            ..Default::default()
        }
    }

    #[test]
    fn test_get_commands_missing_returns_error() {
        let config = OrchestratorConfig::default();
        assert!(config.get_apply_command().is_err());
        assert!(config.get_archive_command().is_err());
        assert!(config.get_analyze_command().is_err());
        assert!(config.get_acceptance_command().is_err());
        assert!(config.get_resolve_command().is_err());
    }

    #[test]
    fn test_get_commands_with_custom_values() {
        let config = OrchestratorConfig {
            apply_command: Some("custom apply {change_id}".to_string()),
            archive_command: Some("custom archive {change_id}".to_string()),
            analyze_command: Some("custom analyze '{prompt}'".to_string()),
            acceptance_command: Some("custom acceptance {change_id}".to_string()),
            resolve_command: Some("custom resolve".to_string()),
            ..Default::default()
        };
        assert_eq!(
            config.get_apply_command().unwrap(),
            "custom apply {change_id}"
        );
        assert_eq!(
            config.get_archive_command().unwrap(),
            "custom archive {change_id}"
        );
        assert_eq!(
            config.get_analyze_command().unwrap(),
            "custom analyze '{prompt}'"
        );
        assert_eq!(
            config.get_acceptance_command().unwrap(),
            "custom acceptance {change_id}"
        );
        assert_eq!(config.get_resolve_command().unwrap(), "custom resolve");
    }

    #[test]
    fn test_expand_change_id() {
        let template = "agent run --apply {change_id}";
        let result = OrchestratorConfig::expand_change_id(template, "update-auth");
        assert_eq!(result, "agent run --apply update-auth");
    }

    #[test]
    fn test_expand_change_id_multiple() {
        let template = "agent --id {change_id} --name {change_id}";
        let result = OrchestratorConfig::expand_change_id(template, "fix-bug");
        assert_eq!(result, "agent --id fix-bug --name fix-bug");
    }

    #[test]
    fn test_expand_prompt() {
        let template = "claude '{prompt}'";
        let result = OrchestratorConfig::expand_prompt(template, "Select the next change");
        assert_eq!(result, "claude 'Select the next change'");
    }

    #[test]
    fn test_parse_simple_json() {
        let json = r#"{
            "apply_command": "test apply {change_id}"
        }"#;
        let config = OrchestratorConfig::parse_jsonc(json).unwrap();
        assert_eq!(
            config.apply_command,
            Some("test apply {change_id}".to_string())
        );
    }

    #[test]
    fn test_parse_jsonc_with_single_line_comments() {
        let jsonc = r#"{
            // This is a comment
            "apply_command": "test apply {change_id}"
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(
            config.apply_command,
            Some("test apply {change_id}".to_string())
        );
    }

    #[test]
    fn test_parse_jsonc_with_multi_line_comments() {
        let jsonc = r#"{
            /* This is a
               multi-line comment */
            "apply_command": "test apply {change_id}"
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(
            config.apply_command,
            Some("test apply {change_id}".to_string())
        );
    }

    #[test]
    fn test_parse_jsonc_with_trailing_comma() {
        let jsonc = r#"{
            "apply_command": "test apply {change_id}",
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(
            config.apply_command,
            Some("test apply {change_id}".to_string())
        );
    }

    #[test]
    fn test_parse_jsonc_full_example() {
        let jsonc = r#"{
            // Apply command configuration
            "apply_command": "codex run 'openspec-apply {change_id}'",

            /* Archive command - used after change completion */
            "archive_command": "codex run 'conflux:archive {change_id}'",

            // Dependency analysis command
            "analyze_command": "claude '{prompt}'",
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(
            config.apply_command,
            Some("codex run 'openspec-apply {change_id}'".to_string())
        );
        assert_eq!(
            config.archive_command,
            Some("codex run 'conflux:archive {change_id}'".to_string())
        );
        assert_eq!(
            config.analyze_command,
            Some("claude '{prompt}'".to_string())
        );
    }

    #[test]
    fn test_parse_jsonc_preserves_strings_with_slashes() {
        let jsonc = r#"{
            "apply_command": "opencode run '/openspec-apply {change_id}'"
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(
            config.apply_command,
            Some("opencode run '/openspec-apply {change_id}'".to_string())
        );
    }

    #[test]
    fn test_partial_config_requires_all_commands() {
        let jsonc = r#"{
            "apply_command": "custom apply {change_id}"
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();

        // Custom value should be used
        assert_eq!(
            config.get_apply_command().unwrap(),
            "custom apply {change_id}"
        );

        // Missing commands should return errors (no fallback to defaults)
        assert!(config.get_archive_command().is_err());
        assert!(config.get_analyze_command().is_err());
    }

    #[test]
    fn test_empty_config_requires_commands() {
        let jsonc = "{}";
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();

        // All commands should return errors when missing
        assert!(config.get_apply_command().is_err());
        assert!(config.get_archive_command().is_err());
        assert!(config.get_analyze_command().is_err());
    }

    #[test]
    fn test_load_from_custom_path() {
        use std::io::Write;
        use tempfile::NamedTempFile;

        // Create a temporary config file with all required commands
        let mut temp_file = NamedTempFile::new().unwrap();
        writeln!(
            temp_file,
            r#"{{
                "apply_command": "custom-agent apply {{change_id}}",
                "archive_command": "custom-agent archive {{change_id}}",
                "analyze_command": "custom-agent analyze",
                "acceptance_command": "custom-agent acceptance",
                "resolve_command": "custom-agent resolve"
            }}"#
        )
        .unwrap();

        // Load from custom path
        let config = OrchestratorConfig::load(Some(temp_file.path())).unwrap();

        assert_eq!(
            config.get_apply_command().unwrap(),
            "custom-agent apply {change_id}"
        );
    }

    #[test]
    fn test_load_from_custom_path_returns_parse_error_with_path_context() {
        use std::io::Write;
        use tempfile::NamedTempFile;

        let mut temp_file = NamedTempFile::new().unwrap();
        writeln!(temp_file, r#"{{"apply_command": invalid-json}}"#).unwrap();

        let result = OrchestratorConfig::load_from_file(temp_file.path());

        assert!(result.is_err());
        let err_msg = result.unwrap_err().to_string();
        assert!(err_msg.contains("Failed to parse config file"));
        assert!(err_msg.contains(&temp_file.path().display().to_string()));
    }

    #[test]
    #[ignore] // Requires isolated environment (may load global config)
    fn test_load_returns_error_when_no_config_exists() {
        use std::env;
        use tempfile::TempDir;

        let _env_lock = env_test_lock();

        // Create a temporary directory with no config files
        let temp_dir = TempDir::new().unwrap();

        // Save current directory and environment
        let original_dir = env::current_dir().unwrap();
        let original_xdg = env::var("XDG_CONFIG_HOME").ok();
        let original_home = env::var("HOME").ok();

        // Point XDG_CONFIG_HOME and HOME to temp directory (where no configs exist)
        set_env_var("XDG_CONFIG_HOME", temp_dir.path().join("config"));
        set_env_var("HOME", temp_dir.path());
        env::set_current_dir(temp_dir.path()).unwrap();

        // Load config - should fail validation due to missing required commands
        let result = OrchestratorConfig::load(None);

        // Restore original directory and environment
        env::set_current_dir(original_dir).unwrap();
        match original_xdg {
            Some(val) => set_env_var("XDG_CONFIG_HOME", val),
            None => remove_env_var("XDG_CONFIG_HOME"),
        }
        match original_home {
            Some(val) => set_env_var("HOME", val),
            None => remove_env_var("HOME"),
        }

        // Should return error when no config exists (no fallback to defaults)
        assert!(result.is_err());
        let err_msg = result.unwrap_err().to_string();
        assert!(err_msg.contains("Missing required config"));
    }

    #[test]
    fn test_load_project_config_takes_priority() {
        use std::env;
        use std::fs;
        use tempfile::TempDir;

        let _env_lock = env_test_lock();

        // Create a temporary directory
        let temp_dir = TempDir::new().unwrap();

        // Create project config file with all required commands
        let project_config_path = temp_dir.path().join(PROJECT_CONFIG_FILE);
        fs::write(
            &project_config_path,
            r#"{
                "apply_command": "project-agent apply {change_id}",
                "archive_command": "project-agent archive {change_id}",
                "analyze_command": "project-agent analyze",
                "acceptance_command": "project-agent acceptance",
                "resolve_command": "project-agent resolve"
            }"#,
        )
        .unwrap();

        // Save current directory and environment
        let original_dir = env::current_dir().unwrap();
        let original_xdg = env::var("XDG_CONFIG_HOME").ok();
        let original_home = env::var("HOME").ok();

        // Point XDG_CONFIG_HOME and HOME to temp directory (where no global configs exist)
        set_env_var("XDG_CONFIG_HOME", temp_dir.path().join("config"));
        set_env_var("HOME", temp_dir.path());
        env::set_current_dir(temp_dir.path()).unwrap();

        // Load config (should use project config)
        let config = OrchestratorConfig::load(None).unwrap();

        // Restore original directory and environment
        env::set_current_dir(original_dir).unwrap();
        match original_xdg {
            Some(val) => set_env_var("XDG_CONFIG_HOME", val),
            None => remove_env_var("XDG_CONFIG_HOME"),
        }
        match original_home {
            Some(val) => set_env_var("HOME", val),
            None => remove_env_var("HOME"),
        }

        // Project config should be used
        assert_eq!(
            config.get_apply_command().unwrap(),
            "project-agent apply {change_id}"
        );
    }

    #[test]
    fn test_get_apply_prompt_default() {
        let config = OrchestratorConfig::default();
        assert_eq!(config.get_apply_prompt(), DEFAULT_APPLY_PROMPT);
    }

    #[test]
    fn test_get_archive_prompt_default() {
        let config = OrchestratorConfig::default();
        assert_eq!(config.get_archive_prompt(), DEFAULT_ARCHIVE_PROMPT);
    }

    #[test]
    fn test_get_prompts_with_custom_values() {
        let config = OrchestratorConfig {
            apply_prompt: Some("Custom apply prompt".to_string()),
            archive_prompt: Some("Custom archive prompt".to_string()),
            ..Default::default()
        };
        assert_eq!(config.get_apply_prompt(), "Custom apply prompt");
        assert_eq!(config.get_archive_prompt(), "Custom archive prompt");
    }

    #[test]
    fn test_parse_jsonc_with_prompts() {
        let jsonc = r#"{
            "apply_command": "claude -p '/openspec:apply {change_id} {prompt}'",
            "archive_command": "claude -p '/openspec:archive {change_id} {prompt}'",
            "apply_prompt": "Test apply prompt",
            "archive_prompt": "Test archive prompt"
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(config.apply_prompt, Some("Test apply prompt".to_string()));
        assert_eq!(
            config.archive_prompt,
            Some("Test archive prompt".to_string())
        );
        assert_eq!(config.get_apply_prompt(), "Test apply prompt");
        assert_eq!(config.get_archive_prompt(), "Test archive prompt");
    }

    #[test]
    fn test_parse_jsonc_with_append_prompts() {
        let jsonc = r#"{
            "apply_append_prompt": "apply tail",
            "acceptance_append_prompt": "acceptance tail",
            "archive_append_prompt": "archive tail",
            "analyze_append_prompt": "analyze tail",
            "resolve_append_prompt": "resolve tail"
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(config.get_apply_append_prompt(), Some("apply tail"));
        assert_eq!(
            config.get_acceptance_append_prompt(),
            Some("acceptance tail")
        );
        assert_eq!(config.get_archive_append_prompt(), Some("archive tail"));
        assert_eq!(config.get_analyze_append_prompt(), Some("analyze tail"));
        assert_eq!(config.get_resolve_append_prompt(), Some("resolve tail"));
    }

    #[test]
    fn test_merge_append_prompt_precedence() {
        let mut base = OrchestratorConfig {
            apply_append_prompt: Some("base apply".to_string()),
            acceptance_append_prompt: Some("base acceptance".to_string()),
            archive_append_prompt: Some("base archive".to_string()),
            analyze_append_prompt: Some("base analyze".to_string()),
            resolve_append_prompt: Some("base resolve".to_string()),
            ..Default::default()
        };
        base.merge(OrchestratorConfig {
            apply_append_prompt: Some("override apply".to_string()),
            acceptance_append_prompt: None,
            archive_append_prompt: Some("override archive".to_string()),
            analyze_append_prompt: Some("override analyze".to_string()),
            resolve_append_prompt: Some("override resolve".to_string()),
            ..Default::default()
        });
        assert_eq!(base.get_apply_append_prompt(), Some("override apply"));
        assert_eq!(base.get_acceptance_append_prompt(), Some("base acceptance"));
        assert_eq!(base.get_archive_append_prompt(), Some("override archive"));
        assert_eq!(base.get_analyze_append_prompt(), Some("override analyze"));
        assert_eq!(base.get_resolve_append_prompt(), Some("override resolve"));
    }

    #[test]
    fn test_expand_prompt_in_apply_command() {
        let template = "claude -p '/openspec:apply {change_id} {prompt}'";
        let command = OrchestratorConfig::expand_change_id(template, "fix-bug");
        let command = OrchestratorConfig::expand_prompt(&command, "Custom instructions");
        assert_eq!(
            command,
            "claude -p '/openspec:apply fix-bug Custom instructions'"
        );
    }

    #[test]
    fn test_expand_prompt_with_empty_string() {
        let template = "claude -p '/openspec:archive {change_id} {prompt}'";
        let command = OrchestratorConfig::expand_change_id(template, "add-feature");
        let command = OrchestratorConfig::expand_prompt(&command, "");
        assert_eq!(command, "claude -p '/openspec:archive add-feature '");
    }

    #[test]
    fn test_backward_compatible_no_prompt_placeholder() {
        // Commands without {prompt} placeholder should continue to work
        let template = "claude -p '/openspec:apply {change_id}'";
        let command = OrchestratorConfig::expand_change_id(template, "fix-bug");
        let command = OrchestratorConfig::expand_prompt(&command, "Ignored prompt");
        // The {prompt} replacement does nothing since placeholder doesn't exist
        assert_eq!(command, "claude -p '/openspec:apply fix-bug'");
    }

    #[test]
    fn test_get_max_iterations_default() {
        let config = OrchestratorConfig::default();
        assert_eq!(config.get_max_iterations(), DEFAULT_MAX_ITERATIONS);
        assert_eq!(config.get_max_iterations(), 50);
    }

    #[test]
    fn test_get_max_iterations_custom() {
        let config = OrchestratorConfig {
            max_iterations: Some(100),
            ..Default::default()
        };
        assert_eq!(config.get_max_iterations(), 100);
    }

    #[test]
    fn test_get_max_iterations_zero_disables_limit() {
        let config = OrchestratorConfig {
            max_iterations: Some(0),
            ..Default::default()
        };
        assert_eq!(config.get_max_iterations(), 0);
    }

    #[test]
    fn test_parse_jsonc_with_max_iterations() {
        let jsonc = r#"{
            "max_iterations": 75
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(config.max_iterations, Some(75));
        assert_eq!(config.get_max_iterations(), 75);
    }

    #[test]
    fn test_parse_jsonc_max_iterations_zero() {
        let jsonc = r#"{
            "max_iterations": 0
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(config.max_iterations, Some(0));
        assert_eq!(config.get_max_iterations(), 0);
    }

    #[test]
    fn test_get_propose_command_default() {
        let config = OrchestratorConfig::default();
        assert!(config.get_propose_command().is_none());
    }

    #[test]
    fn test_get_propose_command_configured() {
        let config = OrchestratorConfig {
            propose_command: Some("claude -p '/openspec:proposal {proposal}'".to_string()),
            ..Default::default()
        };
        assert_eq!(
            config.get_propose_command(),
            Some("claude -p '/openspec:proposal {proposal}'")
        );
    }

    #[test]
    fn test_parse_jsonc_with_propose_command() {
        let jsonc = r#"{
            "propose_command": "opencode run '/openspec:proposal {proposal}'"
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(
            config.propose_command,
            Some("opencode run '/openspec:proposal {proposal}'".to_string())
        );
    }

    #[test]
    fn test_get_worktree_command_default() {
        let config = OrchestratorConfig::default();
        assert!(config.get_worktree_command().is_none());
    }

    #[test]
    fn test_get_worktree_command_configured() {
        let config = OrchestratorConfig {
            worktree_command: Some("cmd --repo {repo_root}".to_string()),
            ..Default::default()
        };
        assert_eq!(
            config.get_worktree_command(),
            Some("cmd --repo {repo_root}")
        );
    }

    #[test]
    fn test_parse_jsonc_with_worktree_command() {
        let jsonc = r#"{
            "worktree_command": "cmd --cwd {workspace_dir}"
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(
            config.worktree_command,
            Some("cmd --cwd {workspace_dir}".to_string())
        );
    }

    #[test]
    fn test_expand_worktree_command() {
        let template = "cmd {workspace_dir} {repo_root}";
        let result =
            OrchestratorConfig::expand_worktree_command(template, "/tmp/worktree", "/repo/root");
        assert_eq!(result, "cmd /tmp/worktree /repo/root");
    }

    #[test]
    fn test_expand_proposal_simple() {
        let template = "claude -p '{proposal}'";
        let result = OrchestratorConfig::expand_proposal(template, "Add login feature");
        assert_eq!(result, "claude -p 'Add login feature'");
    }

    #[test]
    fn test_expand_proposal_multiline() {
        let template = "claude -p '{proposal}'";
        let proposal = "Add login feature\n- Username\n- Password";
        let result = OrchestratorConfig::expand_proposal(template, proposal);
        assert_eq!(
            result,
            "claude -p 'Add login feature\n- Username\n- Password'"
        );
    }

    // === Tests for hooks config in OrchestratorConfig (hooks spec 3.1) ===

    #[test]
    fn test_hooks_config_can_be_parsed_from_jsonc() {
        let jsonc = r#"{
            "hooks": {
                "on_queue_add": "echo 'Added {change_id}'",
                "on_queue_remove": "echo 'Removed {change_id}'"
            }
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        let hooks = config.get_hooks();

        use crate::hooks::HookType;
        assert!(hooks.get(HookType::OnQueueAdd).is_some());
        assert!(hooks.get(HookType::OnQueueRemove).is_some());
    }

    #[test]
    fn test_hooks_config_with_all_hook_types() {
        let jsonc = r#"{
            "hooks": {
                "on_start": "echo start",
                "on_finish": "echo finish",
                "on_error": "echo error",
                "on_change_start": "echo change_start",
                "pre_apply": "echo pre_apply",
                "post_apply": "echo post_apply",
                "on_change_complete": "echo change_complete",
                "pre_archive": "echo pre_archive",
                "post_archive": "echo post_archive",
                "on_change_end": "echo change_end",
                "on_queue_add": "echo queue_add",
                "on_queue_remove": "echo queue_remove"
            }
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        let hooks = config.get_hooks();

        use crate::hooks::HookType;
        assert!(hooks.get(HookType::OnStart).is_some());
        assert!(hooks.get(HookType::OnFinish).is_some());
        assert!(hooks.get(HookType::OnError).is_some());
        assert!(hooks.get(HookType::OnChangeStart).is_some());
        assert!(hooks.get(HookType::PreApply).is_some());
        assert!(hooks.get(HookType::PostApply).is_some());
        assert!(hooks.get(HookType::OnChangeComplete).is_some());
        assert!(hooks.get(HookType::PreArchive).is_some());
        assert!(hooks.get(HookType::PostArchive).is_some());
        assert!(hooks.get(HookType::OnChangeEnd).is_some());
        assert!(hooks.get(HookType::OnQueueAdd).is_some());
        assert!(hooks.get(HookType::OnQueueRemove).is_some());
    }

    #[test]
    fn test_get_hooks_returns_default_when_not_configured() {
        let config = OrchestratorConfig::default();
        let hooks = config.get_hooks();

        // Default HooksConfig should have no hooks configured
        use crate::hooks::HookType;
        assert!(hooks.get(HookType::OnStart).is_none());
        assert!(hooks.get(HookType::OnQueueAdd).is_none());
    }

    // === Tests for parallel execution config (parallel-execution spec) ===

    /// The retired execution-mode key must stop configuration loading with an
    /// actionable removal message rather than being silently ignored.
    #[test]
    fn test_retired_parallel_mode_key_is_rejected_with_migration_guidance() {
        for document in [
            r#"{"parallel_mode": true}"#,
            r#"{"parallel_mode": false}"#,
            r#"{
                // still selecting a mode that no longer exists
                "parallel_mode": true,
                "max_concurrent_workspaces": 6,
            }"#,
        ] {
            let error = OrchestratorConfig::parse_jsonc(document)
                .expect_err("a retired key must fail configuration loading");
            let message = error.to_string();
            assert!(
                message.contains("parallel_mode"),
                "the error must name the retired key, got: {message}"
            );
            assert!(
                message.contains("remove"),
                "the error must tell the operator to remove it, got: {message}"
            );
        }
    }

    /// Removal is limited to the known retired key: unrelated configuration is
    /// unaffected, and a string that merely mentions the key is not a match.
    #[test]
    fn test_unrelated_configuration_is_not_rejected_as_retired() {
        let config = OrchestratorConfig::parse_jsonc(
            r#"{
                "apply_command": "echo parallel_mode",
                "max_concurrent_workspaces": 6
            }"#,
        )
        .expect("unrelated configuration must still load");
        assert_eq!(config.get_max_concurrent_workspaces(), 6);
    }

    /// A retired key inside a nested object is not the top-level retired key.
    #[test]
    fn test_nested_parallel_mode_key_is_not_treated_as_retired() {
        let config = OrchestratorConfig::parse_jsonc(
            r#"{
                "envs": { "parallel_mode": "1" }
            }"#,
        )
        .expect("a nested key must not be mistaken for the retired top-level key");
        assert_eq!(
            config.envs.as_ref().and_then(|e| e.get("parallel_mode")),
            Some(&"1".to_string())
        );
    }

    #[test]
    fn test_max_concurrent_workspaces_default() {
        let config = OrchestratorConfig::default();
        assert_eq!(
            config.get_max_concurrent_workspaces(),
            DEFAULT_MAX_CONCURRENT_WORKSPACES
        );
        // Default is 3 according to defaults.rs
        assert_eq!(config.get_max_concurrent_workspaces(), 3);
    }

    #[test]
    fn test_max_concurrent_workspaces_can_be_configured() {
        let config = OrchestratorConfig {
            max_concurrent_workspaces: Some(8),
            ..Default::default()
        };
        assert_eq!(config.get_max_concurrent_workspaces(), 8);
    }

    #[test]
    fn test_workspace_base_dir_default_is_none() {
        let config = OrchestratorConfig::default();
        assert!(config.get_workspace_base_dir().is_none());
    }

    #[test]
    fn test_workspace_base_dir_can_be_configured() {
        let config = OrchestratorConfig {
            workspace_base_dir: Some("/tmp/ws".to_string()),
            ..Default::default()
        };
        assert_eq!(config.get_workspace_base_dir(), Some("/tmp/ws"));
    }

    #[test]
    fn test_workspace_base_dir_empty_string_treated_as_none() {
        let config = OrchestratorConfig {
            workspace_base_dir: Some("".to_string()),
            ..Default::default()
        };
        assert!(config.get_workspace_base_dir().is_none());
    }

    // === Tests for state_base_dir config ===

    #[test]
    fn test_state_base_dir_default_is_none() {
        let config = OrchestratorConfig::default();
        assert!(config.get_state_base_dir().is_none());
    }

    #[test]
    fn test_state_base_dir_can_be_configured() {
        let config = OrchestratorConfig {
            state_base_dir: Some("/Volumes/BigDisk/cflx/state".to_string()),
            ..Default::default()
        };
        assert_eq!(
            config.get_state_base_dir(),
            Some("/Volumes/BigDisk/cflx/state")
        );
    }

    #[test]
    fn test_state_base_dir_empty_string_treated_as_none() {
        let config = OrchestratorConfig {
            state_base_dir: Some("".to_string()),
            ..Default::default()
        };
        assert!(config.get_state_base_dir().is_none());
    }

    #[test]
    fn test_parse_jsonc_storage_roots() {
        let jsonc = r#"{
            // Both storage roots are independent of each other.
            "workspace_base_dir": "/Volumes/BigDisk/cflx/worktrees",
            "state_base_dir": "/Volumes/BigDisk/cflx/state"
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();

        assert_eq!(
            config.get_workspace_base_dir(),
            Some("/Volumes/BigDisk/cflx/worktrees")
        );
        assert_eq!(
            config.get_state_base_dir(),
            Some("/Volumes/BigDisk/cflx/state")
        );
    }

    #[test]
    fn test_state_base_dir_merge_follows_standard_precedence() {
        let mut lower = OrchestratorConfig {
            state_base_dir: Some("/global/state".to_string()),
            ..Default::default()
        };

        // A higher-priority config without the key preserves the lower value.
        lower.merge(OrchestratorConfig::default());
        assert_eq!(lower.get_state_base_dir(), Some("/global/state"));

        // A higher-priority value overwrites it.
        lower.merge(OrchestratorConfig {
            state_base_dir: Some("/project/state".to_string()),
            ..Default::default()
        });
        assert_eq!(lower.get_state_base_dir(), Some("/project/state"));

        // An explicit empty value is set, and normalizes back to unset.
        lower.merge(OrchestratorConfig {
            state_base_dir: Some(String::new()),
            ..Default::default()
        });
        assert!(lower.get_state_base_dir().is_none());
    }

    /// The configured state root scopes Conflux-owned paths only. The env
    /// overlay Conflux applies to the commands it starts is the one place a
    /// storage setting could leak into a child, so it must stay untouched:
    /// children keep the `XDG_STATE_HOME` they inherited.
    #[test]
    fn test_state_base_dir_never_enters_the_child_command_environment() {
        let config = OrchestratorConfig {
            state_base_dir: Some("/Volumes/BigDisk/cflx/state".to_string()),
            workspace_base_dir: Some("/Volumes/BigDisk/cflx/worktrees".to_string()),
            ..Default::default()
        };

        let envs = config.get_command_envs();
        assert!(
            envs.is_empty(),
            "storage roots must not add child environment entries, got {envs:?}"
        );
        assert!(!envs.contains_key("XDG_STATE_HOME"));
        assert!(!envs.contains_key("XDG_DATA_HOME"));

        // An explicitly configured child environment is still passed through
        // untouched, including an XDG value the operator chose themselves.
        let config = OrchestratorConfig {
            state_base_dir: Some("/Volumes/BigDisk/cflx/state".to_string()),
            envs: Some(HashMap::from([(
                "XDG_STATE_HOME".to_string(),
                "/operator/choice".to_string(),
            )])),
            ..Default::default()
        };
        assert_eq!(
            config
                .get_command_envs()
                .get("XDG_STATE_HOME")
                .map(String::as_str),
            Some("/operator/choice"),
            "the configured state root must not rewrite an operator's own env"
        );
    }

    #[test]
    fn test_vcs_backend_defaults_to_auto() {
        let config = OrchestratorConfig::default();
        assert_eq!(config.get_vcs_backend(), VcsBackend::Auto);
    }

    #[test]
    fn test_vcs_backend_can_be_set_to_git() {
        let config = OrchestratorConfig {
            vcs_backend: Some(VcsBackend::Git),
            ..Default::default()
        };
        assert_eq!(config.get_vcs_backend(), VcsBackend::Git);
    }

    #[test]
    fn test_use_llm_analysis_defaults_to_true() {
        let config = OrchestratorConfig::default();
        assert!(config.use_llm_analysis());
    }

    #[test]
    fn test_use_llm_analysis_can_be_disabled() {
        let config = OrchestratorConfig {
            use_llm_analysis: Some(false),
            ..Default::default()
        };
        assert!(!config.use_llm_analysis());
    }

    #[test]
    fn test_parse_jsonc_parallel_config() {
        let jsonc = r#"{
            "max_concurrent_workspaces": 6,
            "workspace_base_dir": "/custom/path",
            "vcs_backend": "git",
            "use_llm_analysis": false
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();

        assert_eq!(config.get_max_concurrent_workspaces(), 6);
        assert_eq!(config.get_workspace_base_dir(), Some("/custom/path"));
        assert_eq!(config.get_vcs_backend(), VcsBackend::Git);
        assert!(!config.use_llm_analysis());
    }

    // === Tests for resolve_command config ===

    #[test]
    fn test_resolve_command_missing_returns_error() {
        let config = OrchestratorConfig::default();
        // Should return error when resolve command is missing
        assert!(config.get_resolve_command().is_err());
    }

    #[test]
    fn test_resolve_command_can_be_configured() {
        let config = OrchestratorConfig {
            resolve_command: Some("custom-resolver {conflict_files}".to_string()),
            ..Default::default()
        };
        assert_eq!(
            config.get_resolve_command().unwrap(),
            "custom-resolver {conflict_files}"
        );
    }

    #[test]
    fn test_expand_conflict_files_placeholder() {
        let template = "claude resolve {conflict_files}";
        let conflict_files = "src/main.rs src/lib.rs";
        let result = OrchestratorConfig::expand_conflict_files(template, conflict_files);
        let expected = format!(
            "claude resolve {}",
            shlex::try_quote(conflict_files).unwrap()
        );
        assert_eq!(result, expected);
    }

    // === Tests for command queue config ===

    #[test]
    fn test_command_queue_config_defaults() {
        let config = OrchestratorConfig::default();
        // Default values should be None (will use defaults from defaults.rs)
        assert!(config.command_queue_stagger_delay_ms.is_none());
        assert!(config.command_queue_max_retries.is_none());
        assert!(config.command_queue_retry_delay_ms.is_none());
        assert!(config.command_queue_retry_patterns.is_none());
        assert!(config.command_queue_retry_if_duration_under_secs.is_none());
    }

    #[test]
    fn test_command_queue_config_custom() {
        let jsonc = r#"{
            "command_queue_stagger_delay_ms": 3000,
            "command_queue_max_retries": 3,
            "command_queue_retry_delay_ms": 10000,
            "command_queue_retry_patterns": [
                "Custom error pattern",
                "Another pattern"
            ],
            "command_queue_retry_if_duration_under_secs": 10
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();

        assert_eq!(config.command_queue_stagger_delay_ms, Some(3000));
        assert_eq!(config.command_queue_max_retries, Some(3));
        assert_eq!(config.command_queue_retry_delay_ms, Some(10000));
        assert_eq!(
            config.command_queue_retry_patterns,
            Some(vec![
                "Custom error pattern".to_string(),
                "Another pattern".to_string()
            ])
        );
        assert_eq!(config.command_queue_retry_if_duration_under_secs, Some(10));
    }

    #[test]
    fn test_parse_jsonc_with_command_queue() {
        let jsonc = r#"{
            // Command queue configuration
            "command_queue_stagger_delay_ms": 1500,
            "command_queue_max_retries": 5
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();

        assert_eq!(config.command_queue_stagger_delay_ms, Some(1500));
        assert_eq!(config.command_queue_max_retries, Some(5));
    }

    #[test]
    fn test_acceptance_max_continues_default() {
        let config = OrchestratorConfig::default();
        assert_eq!(
            config.get_acceptance_max_continues(),
            DEFAULT_ACCEPTANCE_MAX_CONTINUES
        );
        assert_eq!(config.get_acceptance_max_continues(), 10);
    }

    #[test]
    fn test_acceptance_max_continues_custom() {
        let config = OrchestratorConfig {
            acceptance_max_continues: Some(4),
            ..Default::default()
        };
        assert_eq!(config.get_acceptance_max_continues(), 4);
    }

    #[test]
    fn test_parse_jsonc_with_acceptance_max_continues() {
        let jsonc = r#"{
            "acceptance_max_continues": 5
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(config.acceptance_max_continues, Some(5));
        assert_eq!(config.get_acceptance_max_continues(), 5);
    }

    #[test]
    fn test_inactivity_timeout_max_retries_default_is_three() {
        let config = OrchestratorConfig::default();
        assert!(config.command_inactivity_timeout_max_retries.is_none());
        assert_eq!(
            config.get_command_inactivity_timeout_max_retries(),
            3,
            "Default inactivity timeout max retries must be 3"
        );
    }

    #[test]
    fn test_inactivity_timeout_max_retries_can_be_disabled() {
        let jsonc = r#"{
            "command_inactivity_timeout_max_retries": 0
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(config.command_inactivity_timeout_max_retries, Some(0));
        assert_eq!(config.get_command_inactivity_timeout_max_retries(), 0);
    }

    #[test]
    fn test_inactivity_timeout_max_retries_can_be_configured() {
        let jsonc = r#"{
            "command_inactivity_timeout_max_retries": 3
        }"#;
        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(config.command_inactivity_timeout_max_retries, Some(3));
        assert_eq!(config.get_command_inactivity_timeout_max_retries(), 3);
    }

    // === Absolute command runtime limit ===

    #[test]
    fn command_max_runtime_defaults_to_three_hours() {
        let config = OrchestratorConfig::default();
        assert!(config.command_max_runtime_secs.is_none());
        assert_eq!(
            config.get_command_max_runtime_secs(),
            10800,
            "the absolute command runtime limit defaults to 10800 seconds"
        );
    }

    #[test]
    fn command_max_runtime_zero_disables_the_deadline() {
        let config = OrchestratorConfig::parse_jsonc(
            r#"{
            "command_max_runtime_secs": 0
        }"#,
        )
        .unwrap();
        assert_eq!(config.command_max_runtime_secs, Some(0));
        assert_eq!(
            config.get_command_max_runtime_secs(),
            0,
            "`0` is an explicit disable, never a fallback to the default"
        );
    }

    /// The knob follows the same precedence every other command setting does:
    /// a higher-priority `Some` wins, and `None` preserves what is already set.
    #[test]
    fn command_max_runtime_follows_normal_config_precedence() {
        let mut base = OrchestratorConfig {
            command_max_runtime_secs: Some(120),
            ..OrchestratorConfig::default()
        };
        base.merge(OrchestratorConfig::default());
        assert_eq!(
            base.get_command_max_runtime_secs(),
            120,
            "an unset higher-priority layer must not erase the configured value"
        );

        base.merge(OrchestratorConfig {
            command_max_runtime_secs: Some(0),
            ..OrchestratorConfig::default()
        });
        assert_eq!(
            base.get_command_max_runtime_secs(),
            0,
            "a higher-priority layer may disable the deadline"
        );
    }

    /// The previous default (3,600) is now an ordinary explicit value, so it must
    /// still win over the 10,800-second default at every layer of the merge.
    #[test]
    fn command_max_runtime_explicit_value_overrides_the_default() {
        let explicit = OrchestratorConfig::parse_jsonc(
            r#"{
            "command_max_runtime_secs": 3600
        }"#,
        )
        .unwrap();
        assert_eq!(
            explicit.get_command_max_runtime_secs(),
            3600,
            "an explicitly configured limit is never replaced by the default"
        );

        // global -> project -> custom, each layer merged over the previous one.
        let mut merged = OrchestratorConfig {
            command_max_runtime_secs: Some(3600),
            ..OrchestratorConfig::default()
        };
        merged.merge(OrchestratorConfig {
            command_max_runtime_secs: Some(1800),
            ..OrchestratorConfig::default()
        });
        merged.merge(OrchestratorConfig {
            command_max_runtime_secs: Some(900),
            ..OrchestratorConfig::default()
        });
        assert_eq!(
            merged.get_command_max_runtime_secs(),
            900,
            "custom overrides project, which overrides global"
        );
    }

    /// The absolute deadline and the inactivity timeout are independent knobs:
    /// configuring one must never move the other.
    #[test]
    fn command_max_runtime_is_independent_of_inactivity_timeout() {
        let config = OrchestratorConfig::parse_jsonc(
            r#"{
            "command_max_runtime_secs": 30,
            "command_inactivity_timeout_secs": 900
        }"#,
        )
        .unwrap();
        assert_eq!(config.get_command_max_runtime_secs(), 30);
        assert_eq!(config.get_command_inactivity_timeout_secs(), 900);

        let inactivity_only = OrchestratorConfig::parse_jsonc(
            r#"{
            "command_inactivity_timeout_secs": 0
        }"#,
        )
        .unwrap();
        assert_eq!(inactivity_only.get_command_inactivity_timeout_secs(), 0);
        assert_eq!(
            inactivity_only.get_command_max_runtime_secs(),
            10800,
            "disabling the inactivity timeout must not disable the absolute deadline"
        );
    }

    /// Every generated configuration example is a distributed contract: an operator
    /// who copies one must read the same default and the same `0`-disable meaning
    /// the code enforces.
    #[test]
    fn generated_config_examples_document_the_absolute_runtime_limit() {
        for template in [
            crate::cli::Template::Claude,
            crate::cli::Template::Opencode,
            crate::cli::Template::Codex,
        ] {
            let example = crate::templates::get_template_content(template);
            assert!(
                example.contains("\"command_max_runtime_secs\": 10800"),
                "{template:?} example must show the {}-second default",
                defaults::DEFAULT_COMMAND_MAX_RUNTIME_SECS
            );
            assert!(
                example.contains("Default: 10800 (3 hours)"),
                "{template:?} example must name the default explicitly"
            );
            assert!(
                example.contains("0 disables it"),
                "{template:?} example must keep the `0`-disable semantics"
            );
            assert!(
                !example.contains("command_max_runtime_secs\": 3600"),
                "{template:?} example must not advertise the retired 3600-second default"
            );
        }
    }

    // === Acceptance absolute runtime limit ===

    /// A fully-populated config, so `validate_required_commands` reaches the
    /// range check instead of stopping at a missing command.
    fn config_with_required_commands(
        acceptance_max_runtime_secs: Option<u64>,
    ) -> OrchestratorConfig {
        OrchestratorConfig {
            apply_command: Some("apply".to_string()),
            archive_command: Some("archive".to_string()),
            analyze_command: Some("analyze".to_string()),
            acceptance_command: Some("accept".to_string()),
            resolve_command: Some("resolve".to_string()),
            acceptance_max_runtime_secs,
            ..OrchestratorConfig::default()
        }
    }

    #[test]
    fn acceptance_max_runtime_defaults_to_thirty_minutes() {
        let config = OrchestratorConfig::default();
        assert!(config.acceptance_max_runtime_secs.is_none());
        assert_eq!(
            config.get_acceptance_max_runtime_secs(),
            1800,
            "Acceptance defaults to a 30-minute absolute runtime limit"
        );
        assert_eq!(
            config.get_acceptance_max_runtime_secs(),
            defaults::DEFAULT_ACCEPTANCE_MAX_RUNTIME_SECS
        );
    }

    /// The dedicated limit follows the same precedence every other command
    /// setting does: a higher-priority `Some` wins, `None` preserves.
    #[test]
    fn acceptance_max_runtime_follows_normal_config_precedence() {
        let mut base = OrchestratorConfig {
            acceptance_max_runtime_secs: Some(600),
            ..OrchestratorConfig::default()
        };
        base.merge(OrchestratorConfig::default());
        assert_eq!(
            base.get_acceptance_max_runtime_secs(),
            600,
            "an unset higher-priority layer must not erase the configured value"
        );

        // global -> project -> custom, each layer merged over the previous one.
        let mut merged = OrchestratorConfig {
            acceptance_max_runtime_secs: Some(3600),
            ..OrchestratorConfig::default()
        };
        merged.merge(OrchestratorConfig {
            acceptance_max_runtime_secs: Some(1200),
            ..OrchestratorConfig::default()
        });
        merged.merge(OrchestratorConfig {
            acceptance_max_runtime_secs: Some(900),
            ..OrchestratorConfig::default()
        });
        assert_eq!(
            merged.get_acceptance_max_runtime_secs(),
            900,
            "custom overrides project, which overrides global"
        );

        let parsed = OrchestratorConfig::parse_jsonc(
            r#"{
            "acceptance_max_runtime_secs": 2400
        }"#,
        )
        .unwrap();
        assert_eq!(parsed.acceptance_max_runtime_secs, Some(2400));
        assert_eq!(parsed.get_acceptance_max_runtime_secs(), 2400);
    }

    /// `0` disables the *common* budget; here it is simply out of range, because
    /// Acceptance may not run without a wall-clock guard at all.
    #[test]
    fn acceptance_max_runtime_rejects_zero_with_an_actionable_diagnostic() {
        let config = config_with_required_commands(Some(0));
        let error = config
            .validate_acceptance_max_runtime_secs()
            .expect_err("zero must not load");
        let message = error.to_string();
        assert!(
            message.contains("acceptance_max_runtime_secs"),
            "the diagnostic must name the field: {message}"
        );
        assert!(
            message.contains("between 300 and 10800"),
            "the diagnostic must state the accepted range: {message}"
        );
        assert!(
            message.contains("`0` is not accepted"),
            "the diagnostic must say why zero is refused: {message}"
        );

        assert!(
            config.validate_required_commands().is_err(),
            "the range check is reached on the configuration load path"
        );
    }

    /// The floor is 300, not 60: a minute-scale interval reads as a generally
    /// safe production value while being closer to skill-loading overhead than
    /// to a review. 299 and 10,801 are the two values that prove the inclusive
    /// bounds are the bounds.
    #[test]
    fn acceptance_max_runtime_enforces_its_range_bounds() {
        assert_eq!(defaults::MIN_ACCEPTANCE_MAX_RUNTIME_SECS, 300);
        assert_eq!(defaults::MAX_ACCEPTANCE_MAX_RUNTIME_SECS, 10_800);

        for rejected in [1_u64, 60, 299, 10_801, 86_400] {
            assert!(
                config_with_required_commands(Some(rejected))
                    .validate_acceptance_max_runtime_secs()
                    .is_err(),
                "{rejected}s is outside 300..=10800 and must be refused"
            );
        }

        for accepted in [
            defaults::MIN_ACCEPTANCE_MAX_RUNTIME_SECS,
            300,
            1800,
            10_800,
            defaults::MAX_ACCEPTANCE_MAX_RUNTIME_SECS,
        ] {
            let config = config_with_required_commands(Some(accepted));
            assert!(
                config.validate_acceptance_max_runtime_secs().is_ok(),
                "{accepted}s is inside the accepted range"
            );
            assert!(
                config.validate_required_commands().is_ok(),
                "{accepted}s must also load"
            );
        }

        assert!(
            config_with_required_commands(None)
                .validate_acceptance_max_runtime_secs()
                .is_ok(),
            "an unconfigured field falls back to the in-range default"
        );
    }

    /// The effective Acceptance bound, read the way the runner reads it: the
    /// configured layers produce a `CommandQueueConfig`, and that resolves the
    /// class deadline. There is no second accessor computing the same rule.
    fn effective_acceptance_limit_secs(config: &OrchestratorConfig) -> u64 {
        crate::command_queue::CommandQueueConfig::from(config)
            .effective_max_runtime_secs(Some(crate::command_queue::ACCEPTANCE_OPERATION_TYPE))
    }

    /// The two knobs are independent: the Acceptance guard survives a disabled
    /// common budget, and a shorter common budget still caps Acceptance.
    #[test]
    fn acceptance_runtime_limit_composes_with_the_common_limit() {
        let disabled_common = OrchestratorConfig::parse_jsonc(
            r#"{
            "command_max_runtime_secs": 0
        }"#,
        )
        .unwrap();
        assert_eq!(disabled_common.get_command_max_runtime_secs(), 0);
        assert_eq!(
            effective_acceptance_limit_secs(&disabled_common),
            1800,
            "disabling the common command budget must not unbound Acceptance"
        );

        let shorter_common = OrchestratorConfig::parse_jsonc(
            r#"{
            "command_max_runtime_secs": 300,
            "acceptance_max_runtime_secs": 1800
        }"#,
        )
        .unwrap();
        assert_eq!(
            effective_acceptance_limit_secs(&shorter_common),
            300,
            "a shorter common safety limit still applies to Acceptance"
        );
        assert_eq!(
            shorter_common.get_command_max_runtime_secs(),
            300,
            "other command classes are untouched by the Acceptance knob"
        );

        let acceptance_only = OrchestratorConfig::parse_jsonc(
            r#"{
            "acceptance_max_runtime_secs": 600
        }"#,
        )
        .unwrap();
        assert_eq!(effective_acceptance_limit_secs(&acceptance_only), 600);
        assert_eq!(
            acceptance_only.get_command_max_runtime_secs(),
            10800,
            "configuring the Acceptance limit must never move the common one"
        );
    }

    /// A common limit far below the dedicated key's own configuration floor is
    /// still an overriding safety bound. The floor says what may be *configured*
    /// for `acceptance_max_runtime_secs`; it is not a clamp on the effective
    /// bound, so 30 seconds wins over 1,800.
    #[test]
    fn a_shorter_common_limit_overrides_the_dedicated_floor() {
        let config = OrchestratorConfig::parse_jsonc(
            r#"{
            "command_max_runtime_secs": 30,
            "acceptance_max_runtime_secs": 1800
        }"#,
        )
        .unwrap();

        assert_eq!(
            effective_acceptance_limit_secs(&config),
            30,
            "a 30-second common safety limit binds Acceptance below its own floor"
        );
        assert!(
            effective_acceptance_limit_secs(&config) < defaults::MIN_ACCEPTANCE_MAX_RUNTIME_SECS,
            "the dedicated floor must not be treated as a clamp on the effective bound"
        );
        assert!(
            config.validate_acceptance_max_runtime_secs().is_ok(),
            "the dedicated key itself is in range; only the common limit is shorter"
        );
    }

    /// The effective bound is selected inside the common runner from the
    /// operation type the invocation already declares. Acceptance is the only
    /// class that gets the dedicated deadline; cleanup review runs the *same
    /// configured agent command* under its own operation type and keeps the
    /// common limit, `0`-disable semantics included.
    #[test]
    fn command_queue_config_selects_the_limit_by_operation_type() {
        use crate::command_queue::{CommandQueueConfig, ACCEPTANCE_OPERATION_TYPE};

        const OTHER_CLASSES: [Option<&str>; 6] = [
            Some("cleanup-review"),
            Some("apply"),
            Some("archive"),
            Some("analyze"),
            Some("resolve"),
            None,
        ];

        // Defaults: 1,800 for Acceptance, 10,800 for everyone else.
        let defaults_queue = CommandQueueConfig::from(&OrchestratorConfig::default());
        assert_eq!(
            defaults_queue.effective_max_runtime_secs(Some(ACCEPTANCE_OPERATION_TYPE)),
            1800
        );
        for class in OTHER_CLASSES {
            assert_eq!(
                defaults_queue.effective_max_runtime_secs(class),
                10800,
                "{class:?} keeps `command_max_runtime_secs`"
            );
        }

        // A 30-second common limit binds every class, Acceptance included.
        let shorter_common = CommandQueueConfig::from(&OrchestratorConfig {
            command_max_runtime_secs: Some(30),
            acceptance_max_runtime_secs: Some(1800),
            ..OrchestratorConfig::default()
        });
        assert_eq!(
            shorter_common.effective_max_runtime_secs(Some(ACCEPTANCE_OPERATION_TYPE)),
            30
        );
        for class in OTHER_CLASSES {
            assert_eq!(
                shorter_common.effective_max_runtime_secs(class),
                30,
                "{class:?} keeps the common limit"
            );
        }

        // A disabled common limit unbounds every other class but not Acceptance.
        let disabled_common = CommandQueueConfig::from(&OrchestratorConfig {
            command_max_runtime_secs: Some(0),
            acceptance_max_runtime_secs: Some(1800),
            ..OrchestratorConfig::default()
        });
        assert_eq!(
            disabled_common.effective_max_runtime_secs(Some(ACCEPTANCE_OPERATION_TYPE)),
            1800,
            "`0` disables the common budget only"
        );
        for class in OTHER_CLASSES {
            assert_eq!(
                disabled_common.effective_max_runtime_secs(class),
                0,
                "{class:?} keeps the `0`-disable semantics of the common limit"
            );
        }

        // Sharing the Acceptance *agent command* is not sharing its deadline.
        let cleanup_review_shares_the_command = CommandQueueConfig::from(&OrchestratorConfig {
            acceptance_command: Some("agent accept".to_string()),
            cleanup_review_skill: Some("cflx-cleanup-review".to_string()),
            command_max_runtime_secs: Some(10800),
            acceptance_max_runtime_secs: Some(600),
            ..OrchestratorConfig::default()
        });
        assert_eq!(
            cleanup_review_shares_the_command.effective_max_runtime_secs(Some("cleanup-review")),
            10800,
            "cleanup review must not inherit the dedicated limit from a shared command"
        );
        assert_eq!(
            cleanup_review_shares_the_command
                .effective_max_runtime_secs(Some(ACCEPTANCE_OPERATION_TYPE)),
            600
        );

        // Only the exact operation-type token selects the dedicated limit.
        for near_miss in ["Acceptance", "acceptance-repair", "accept", " acceptance"] {
            assert_eq!(
                defaults_queue.effective_max_runtime_secs(Some(near_miss)),
                10800,
                "{near_miss:?} is not the Acceptance operation type"
            );
        }
    }

    /// The generated template is where most operators first meet a key, so both
    /// storage roots must appear there with the rule that makes them safe.
    #[test]
    fn generated_config_examples_document_both_storage_roots() {
        for template in [
            crate::cli::Template::Claude,
            crate::cli::Template::Opencode,
            crate::cli::Template::Codex,
        ] {
            let example = crate::templates::get_template_content(template);
            assert!(
                example.contains("\"workspace_base_dir\""),
                "{template:?} example must document the managed-worktree root"
            );
            assert!(
                example.contains("\"state_base_dir\""),
                "{template:?} example must document the Conflux state root"
            );
            assert!(
                example.contains("XDG_STATE_HOME"),
                "{template:?} example must state the precedence over XDG_STATE_HOME"
            );
            assert!(
                example.contains("never migrated"),
                "{template:?} example must state that migration is the operator's job"
            );
        }
    }

    /// Every generated configuration example is a distributed contract: an
    /// operator who copies one must read the same default, the same range, and
    /// the same "no disable" rule the code enforces.
    #[test]
    fn generated_config_examples_document_the_acceptance_runtime_limit() {
        for template in [
            crate::cli::Template::Claude,
            crate::cli::Template::Opencode,
            crate::cli::Template::Codex,
        ] {
            let example = crate::templates::get_template_content(template);
            assert!(
                example.contains("\"acceptance_max_runtime_secs\": 1800"),
                "{template:?} example must show the {}-second default",
                defaults::DEFAULT_ACCEPTANCE_MAX_RUNTIME_SECS
            );
            assert!(
                example.contains("Default: 1800 (30 minutes)"),
                "{template:?} example must name the default explicitly"
            );
            assert!(
                example.contains("Accepts 300 through 10800"),
                "{template:?} example must state the accepted range"
            );
            assert!(
                example.contains("0 is rejected"),
                "{template:?} example must keep the no-disable rule"
            );
        }
    }

    // === Tests for XDG config path precedence ===

    #[test]
    #[allow(deprecated)]
    fn test_get_xdg_config_path_returns_path() {
        // Test that get_xdg_config_path returns a valid path
        let result = super::get_xdg_config_path();
        assert!(result.is_some());

        let path = result.unwrap();
        let path_str = path.to_string_lossy();

        // Should end with cflx/config.jsonc
        assert!(
            path_str.ends_with("cflx/config.jsonc"),
            "Expected path to end with cflx/config.jsonc, got: {:?}",
            path
        );

        // Should contain either .config (XDG default) or custom XDG_CONFIG_HOME
        assert!(
            path_str.contains(".config") || std::env::var("XDG_CONFIG_HOME").is_ok(),
            "Expected path to contain .config or use XDG_CONFIG_HOME, got: {:?}",
            path
        );
    }

    #[test]
    fn test_get_xdg_env_config_path() {
        let _env_lock = env_test_lock();

        // Test without XDG_CONFIG_HOME
        remove_env_var("XDG_CONFIG_HOME");
        assert!(super::get_xdg_env_config_path().is_none());

        // Test with XDG_CONFIG_HOME set
        set_env_var("XDG_CONFIG_HOME", "/custom/config");
        let result = super::get_xdg_env_config_path();
        assert!(result.is_some());
        let path = result.unwrap();
        assert_eq!(path.to_str().unwrap(), "/custom/config/cflx/config.jsonc");

        // Clean up
        remove_env_var("XDG_CONFIG_HOME");
    }

    #[test]
    fn test_get_xdg_default_config_path() {
        // Should always return a path if home directory is available
        let result = super::get_xdg_default_config_path();
        if let Some(path) = result {
            let path_str = path.to_string_lossy();
            assert!(path_str.contains(".config"));
            assert!(path_str.ends_with("cflx/config.jsonc"));
        }
    }

    #[test]
    #[allow(deprecated)]
    fn test_deprecated_global_config_path_prefers_xdg_env_over_default() {
        let _env_lock = env_test_lock();

        let original_xdg = std::env::var("XDG_CONFIG_HOME").ok();

        set_env_var("XDG_CONFIG_HOME", "/tmp/cflx-xdg-env");

        let resolved =
            super::get_global_config_path().expect("deprecated helper should resolve path");
        assert_eq!(
            resolved.to_string_lossy(),
            "/tmp/cflx-xdg-env/cflx/config.jsonc",
            "deprecated global helper must keep preferring XDG_CONFIG_HOME"
        );

        match original_xdg {
            Some(val) => set_env_var("XDG_CONFIG_HOME", val),
            None => remove_env_var("XDG_CONFIG_HOME"),
        }
    }

    #[test]
    #[allow(deprecated)]
    fn test_deprecated_global_config_path_falls_back_to_xdg_default() {
        let _env_lock = env_test_lock();

        let original_xdg = std::env::var("XDG_CONFIG_HOME").ok();
        remove_env_var("XDG_CONFIG_HOME");

        let from_deprecated = super::get_global_config_path();
        let from_explicit = super::get_xdg_default_config_path();
        assert_eq!(
            from_deprecated, from_explicit,
            "deprecated global helper must preserve XDG default fallback behavior"
        );

        match original_xdg {
            Some(val) => set_env_var("XDG_CONFIG_HOME", val),
            None => remove_env_var("XDG_CONFIG_HOME"),
        }
    }

    #[test]
    fn test_get_global_config_paths_returns_low_to_high_priority_order() {
        let _env_lock = env_test_lock();

        let original_xdg = std::env::var("XDG_CONFIG_HOME").ok();
        set_env_var("XDG_CONFIG_HOME", "/tmp/cflx-priority");

        let paths = super::get_global_config_paths();
        assert!(
            paths.len() >= 2,
            "expected at least XDG default and XDG env paths"
        );

        let last = paths
            .last()
            .expect("global path list should not be empty")
            .to_string_lossy()
            .to_string();
        assert_eq!(
            last, "/tmp/cflx-priority/cflx/config.jsonc",
            "XDG env path must be highest priority and appear last"
        );

        match original_xdg {
            Some(val) => set_env_var("XDG_CONFIG_HOME", val),
            None => remove_env_var("XDG_CONFIG_HOME"),
        }
    }

    #[test]
    fn test_get_platform_config_path_returns_path() {
        // Test that get_platform_config_path returns a valid path
        let result = super::get_platform_config_path();

        // Platform path may be None if dirs::config_dir() is None (rare)
        if let Some(path) = result {
            let path_str = path.to_string_lossy();
            assert!(
                path_str.ends_with("cflx/config.jsonc"),
                "Expected path to end with cflx/config.jsonc, got: {:?}",
                path
            );
        }
    }

    #[test]
    #[ignore] // Requires sequential execution due to env::current_dir() manipulation
    fn test_load_xdg_config_precedence() {
        use std::env;
        use std::fs;
        use tempfile::TempDir;

        let _env_lock = env_test_lock();

        // Create temporary directories for XDG and platform configs
        let xdg_dir = TempDir::new().unwrap();
        let platform_dir = TempDir::new().unwrap();
        let work_dir = TempDir::new().unwrap();

        // Create XDG config with all required commands
        let xdg_config_dir = xdg_dir.path().join("cflx");
        fs::create_dir_all(&xdg_config_dir).unwrap();
        fs::write(
            xdg_config_dir.join("config.jsonc"),
            r#"{
                "apply_command": "xdg-agent apply {change_id}",
                "archive_command": "xdg-agent archive {change_id}",
                "analyze_command": "xdg-agent analyze",
                "acceptance_command": "xdg-agent acceptance",
                "resolve_command": "xdg-agent resolve"
            }"#,
        )
        .unwrap();

        // Create platform config with different content
        let platform_config_dir = platform_dir.path().join("cflx");
        fs::create_dir_all(&platform_config_dir).unwrap();
        fs::write(
            platform_config_dir.join("config.jsonc"),
            r#"{
                "apply_command": "platform-agent apply {change_id}",
                "archive_command": "platform-agent archive {change_id}",
                "analyze_command": "platform-agent analyze",
                "acceptance_command": "platform-agent acceptance",
                "resolve_command": "platform-agent resolve"
            }"#,
        )
        .unwrap();

        // Save original environment and directory
        let original_xdg = env::var("XDG_CONFIG_HOME").ok();
        let original_dir = env::current_dir().unwrap();

        // Set XDG_CONFIG_HOME and change to work directory (no project config)
        set_env_var("XDG_CONFIG_HOME", xdg_dir.path());
        env::set_current_dir(work_dir.path()).unwrap();

        // Load config - should prefer XDG path
        let config = OrchestratorConfig::load(None).unwrap();

        // Restore environment
        env::set_current_dir(original_dir).unwrap();
        match original_xdg {
            Some(val) => set_env_var("XDG_CONFIG_HOME", val),
            None => remove_env_var("XDG_CONFIG_HOME"),
        }

        // XDG config should be loaded
        assert_eq!(
            config.get_apply_command().unwrap(),
            "xdg-agent apply {change_id}",
            "Expected XDG config to be loaded"
        );
    }

    #[test]
    #[ignore] // Requires isolated environment (may load global config)
    fn test_load_platform_fallback_when_xdg_missing() {
        use std::env;
        use tempfile::TempDir;

        let _env_lock = env_test_lock();

        // Create temporary work directory with no config files
        let work_dir = TempDir::new().unwrap();

        // Save original environment and directory
        let original_xdg = env::var("XDG_CONFIG_HOME").ok();
        let original_dir = env::current_dir().unwrap();

        // Set XDG_CONFIG_HOME to non-existent path
        let nonexistent = work_dir.path().join("nonexistent");
        set_env_var("XDG_CONFIG_HOME", &nonexistent);
        env::set_current_dir(work_dir.path()).unwrap();

        // Load config - should fail due to missing required commands
        let result = OrchestratorConfig::load(None);

        // Restore environment
        env::set_current_dir(original_dir).unwrap();
        match original_xdg {
            Some(val) => set_env_var("XDG_CONFIG_HOME", val),
            None => remove_env_var("XDG_CONFIG_HOME"),
        }

        // Should return error since no config files exist with required commands
        assert!(result.is_err());
        let err_msg = result.unwrap_err().to_string();
        assert!(err_msg.contains("Missing required config"));
    }

    #[test]
    #[ignore] // Requires sequential execution due to env::current_dir() manipulation
    fn test_project_config_takes_priority_over_xdg() {
        use std::env;
        use std::fs;
        use tempfile::TempDir;

        let _env_lock = env_test_lock();

        // Create temporary directories
        let xdg_dir = TempDir::new().unwrap();
        let work_dir = TempDir::new().unwrap();

        // Create XDG config with all required commands
        let xdg_config_dir = xdg_dir.path().join("cflx");
        fs::create_dir_all(&xdg_config_dir).unwrap();
        fs::write(
            xdg_config_dir.join("config.jsonc"),
            r#"{
                "apply_command": "xdg-agent apply {change_id}",
                "archive_command": "xdg-agent archive {change_id}",
                "analyze_command": "xdg-agent analyze",
                "acceptance_command": "xdg-agent acceptance",
                "resolve_command": "xdg-agent resolve"
            }"#,
        )
        .unwrap();

        // Create project config with different apply_command (other commands inherited from XDG)
        fs::write(
            work_dir.path().join(PROJECT_CONFIG_FILE),
            r#"{"apply_command": "project-agent apply {change_id}"}"#,
        )
        .unwrap();

        // Save original environment and directory
        let original_xdg = env::var("XDG_CONFIG_HOME").ok();
        let original_dir = env::current_dir().unwrap();

        // Set XDG_CONFIG_HOME and change to work directory
        set_env_var("XDG_CONFIG_HOME", xdg_dir.path());
        env::set_current_dir(work_dir.path()).unwrap();

        // Load config - should prefer project config for apply_command
        let config = OrchestratorConfig::load(None).unwrap();

        // Restore environment
        env::set_current_dir(original_dir).unwrap();
        match original_xdg {
            Some(val) => set_env_var("XDG_CONFIG_HOME", val),
            None => remove_env_var("XDG_CONFIG_HOME"),
        }

        // Project config should take priority for apply_command
        assert_eq!(
            config.get_apply_command().unwrap(),
            "project-agent apply {change_id}",
            "Expected project config to take priority over XDG config"
        );
        // Other commands should be inherited from XDG
        assert_eq!(
            config.get_archive_command().unwrap(),
            "xdg-agent archive {change_id}"
        );
    }

    // Test for merge-based config loading
    #[test]
    fn test_config_merge_partial_project_inherits_global() {
        use std::env;
        use std::fs;
        use tempfile::TempDir;

        let _env_lock = env_test_lock();

        // Create temporary directories
        let xdg_dir = TempDir::new().unwrap();
        let work_dir = TempDir::new().unwrap();

        // Create XDG config with all commands
        let xdg_config_dir = xdg_dir.path().join("cflx");
        fs::create_dir_all(&xdg_config_dir).unwrap();
        fs::write(
            xdg_config_dir.join("config.jsonc"),
            r#"{
                "apply_command": "global-agent apply {change_id}",
                "archive_command": "global-agent archive {change_id}",
                "analyze_command": "global-agent analyze '{prompt}'"
            }"#,
        )
        .unwrap();

        // Create project config with only apply_command (partial config)
        let project_config_path = work_dir.path().join(PROJECT_CONFIG_FILE);
        fs::write(
            &project_config_path,
            r#"{"apply_command": "project-agent apply {change_id}"}"#,
        )
        .unwrap();

        // Save original environment
        let original_xdg = env::var("XDG_CONFIG_HOME").ok();
        let original_dir = env::current_dir().unwrap();

        // Set XDG_CONFIG_HOME and work directory
        set_env_var("XDG_CONFIG_HOME", xdg_dir.path());
        env::set_current_dir(work_dir.path()).unwrap();

        // Load config (should merge project and global)
        let config = OrchestratorConfig::load(None).unwrap();

        // Restore environment
        env::set_current_dir(original_dir).unwrap();
        match original_xdg {
            Some(val) => set_env_var("XDG_CONFIG_HOME", val),
            None => remove_env_var("XDG_CONFIG_HOME"),
        }

        // Project config should override apply_command
        assert_eq!(
            config.get_apply_command().unwrap(),
            "project-agent apply {change_id}",
            "Project config should override apply_command"
        );

        // Global config should provide archive_command and analyze_command
        assert_eq!(
            config.get_archive_command().unwrap(),
            "global-agent archive {change_id}",
            "Global config should provide archive_command when missing in project config"
        );
        assert_eq!(
            config.get_analyze_command().unwrap(),
            "global-agent analyze '{prompt}'",
            "Global config should provide analyze_command when missing in project config"
        );
    }

    #[test]
    fn test_hooks_deep_merge() {
        use std::env;
        use std::fs;
        use tempfile::TempDir;

        let _env_lock = env_test_lock();

        // Create temporary directories
        let xdg_dir = TempDir::new().unwrap();
        let work_dir = TempDir::new().unwrap();

        // Create XDG config with on_start and pre_apply hooks
        let xdg_config_dir = xdg_dir.path().join("cflx");
        fs::create_dir_all(&xdg_config_dir).unwrap();
        fs::write(
            xdg_config_dir.join("config.jsonc"),
            r#"{
                "apply_command": "test apply",
                "hooks": {
                    "on_start": "echo global start",
                    "pre_apply": "echo global pre_apply"
                }
            }"#,
        )
        .unwrap();

        // Create project config with on_finish and post_apply hooks
        let project_config_path = work_dir.path().join(PROJECT_CONFIG_FILE);
        fs::write(
            &project_config_path,
            r#"{
                "hooks": {
                    "on_finish": "echo project finish",
                    "post_apply": "echo project post_apply"
                }
            }"#,
        )
        .unwrap();

        // Save original environment
        let original_xdg = env::var("XDG_CONFIG_HOME").ok();
        let original_dir = env::current_dir().unwrap();

        // Set XDG_CONFIG_HOME and work directory
        set_env_var("XDG_CONFIG_HOME", xdg_dir.path());
        env::set_current_dir(work_dir.path()).unwrap();

        // Load config (should deep merge hooks)
        let config = OrchestratorConfig::load(None).unwrap();

        // Restore environment
        env::set_current_dir(original_dir).unwrap();
        match original_xdg {
            Some(val) => set_env_var("XDG_CONFIG_HOME", val),
            None => remove_env_var("XDG_CONFIG_HOME"),
        }

        let hooks = config.get_hooks();

        // All hooks should be present (deep merge)
        use crate::hooks::HookType;
        assert!(hooks.get(HookType::OnStart).is_some());
        assert!(hooks.get(HookType::PreApply).is_some());
        assert!(hooks.get(HookType::OnFinish).is_some());
        assert!(hooks.get(HookType::PostApply).is_some());

        // Verify hook commands
        assert_eq!(
            hooks.get(HookType::OnStart).unwrap().command,
            "echo global start"
        );
        assert_eq!(
            hooks.get(HookType::PreApply).unwrap().command,
            "echo global pre_apply"
        );
        assert_eq!(
            hooks.get(HookType::OnFinish).unwrap().command,
            "echo project finish"
        );
        assert_eq!(
            hooks.get(HookType::PostApply).unwrap().command,
            "echo project post_apply"
        );
    }

    // === Tests for required command validation ===

    #[test]
    fn test_validate_required_commands_all_present() {
        let config = OrchestratorConfig {
            apply_command: Some("apply".to_string()),
            archive_command: Some("archive".to_string()),
            analyze_command: Some("analyze".to_string()),
            acceptance_command: Some("acceptance".to_string()),
            resolve_command: Some("resolve".to_string()),
            ..Default::default()
        };

        // Should not error when all required commands are present
        assert!(config.validate_required_commands().is_ok());
    }

    #[test]
    fn test_validate_required_commands_missing_apply() {
        let config = OrchestratorConfig {
            archive_command: Some("archive".to_string()),
            analyze_command: Some("analyze".to_string()),
            acceptance_command: Some("acceptance".to_string()),
            resolve_command: Some("resolve".to_string()),
            ..Default::default()
        };

        let result = config.validate_required_commands();
        assert!(result.is_err());
        let err = result.unwrap_err();
        assert!(err.to_string().contains("apply_command"));
    }

    #[test]
    fn test_validate_required_commands_missing_multiple() {
        let config = OrchestratorConfig {
            apply_command: Some("apply".to_string()),
            // Missing: archive, analyze, acceptance, resolve
            ..Default::default()
        };

        let result = config.validate_required_commands();
        assert!(result.is_err());
        let err_msg = result.unwrap_err().to_string();
        assert!(err_msg.contains("archive_command"));
        assert!(err_msg.contains("analyze_command"));
        assert!(err_msg.contains("acceptance_command"));
        assert!(err_msg.contains("resolve_command"));
    }

    #[test]
    #[ignore] // Requires isolated environment (may load global config)
    fn test_load_validation_fails_on_missing_commands() {
        use std::env;
        use std::fs;
        use tempfile::TempDir;

        let temp_dir = TempDir::new().unwrap();
        let config_path = temp_dir.path().join(".cflx.jsonc");

        // Write a config with missing required commands
        fs::write(
            &config_path,
            r#"{
                "apply_command": "test apply"
            }"#,
        )
        .unwrap();

        // Set current dir to temp dir
        let original_dir = env::current_dir().unwrap();
        env::set_current_dir(temp_dir.path()).unwrap();

        // Load should fail due to missing commands
        let result = OrchestratorConfig::load(None);
        assert!(result.is_err());

        let err_msg = result.unwrap_err().to_string();
        assert!(err_msg.contains("archive_command"));
        assert!(err_msg.contains("analyze_command"));
        assert!(err_msg.contains("acceptance_command"));
        assert!(err_msg.contains("resolve_command"));

        // Restore original directory
        env::set_current_dir(original_dir).unwrap();
    }

    #[test]
    #[ignore] // Requires sequential execution due to env manipulation
    fn test_xdg_env_takes_priority_over_xdg_default() {
        use std::env;
        use std::fs;
        use tempfile::TempDir;

        let _env_lock = env_test_lock();

        // Create temporary directories for XDG env and default paths
        let xdg_env_dir = TempDir::new().unwrap();
        let xdg_default_dir = TempDir::new().unwrap();
        let work_dir = TempDir::new().unwrap();

        // Create XDG env config (higher priority)
        let xdg_env_config_dir = xdg_env_dir.path().join("cflx");
        fs::create_dir_all(&xdg_env_config_dir).unwrap();
        fs::write(
            xdg_env_config_dir.join("config.jsonc"),
            r#"{
                "apply_command": "xdg-env-agent apply {change_id}",
                "archive_command": "xdg-env-agent archive {change_id}",
                "analyze_command": "xdg-env-agent analyze",
                "acceptance_command": "xdg-env-agent acceptance",
                "resolve_command": "xdg-env-agent resolve"
            }"#,
        )
        .unwrap();

        // Create XDG default config (lower priority) - this should be at ~/.config location
        // We'll simulate this by setting HOME to xdg_default_dir
        let home_config_dir = xdg_default_dir.path().join(".config").join("cflx");
        fs::create_dir_all(&home_config_dir).unwrap();
        fs::write(
            home_config_dir.join("config.jsonc"),
            r#"{
                "apply_command": "xdg-default-agent apply {change_id}",
                "archive_command": "xdg-default-agent archive {change_id}",
                "analyze_command": "xdg-default-agent analyze",
                "acceptance_command": "xdg-default-agent acceptance",
                "resolve_command": "xdg-default-agent resolve"
            }"#,
        )
        .unwrap();

        // Save original environment and directory
        let original_xdg = env::var("XDG_CONFIG_HOME").ok();
        let original_home = env::var("HOME").ok();
        let original_dir = env::current_dir().unwrap();

        // Set XDG_CONFIG_HOME (env) and HOME (for default path)
        set_env_var("XDG_CONFIG_HOME", xdg_env_dir.path());
        set_env_var("HOME", xdg_default_dir.path());
        env::set_current_dir(work_dir.path()).unwrap();

        // Load config - XDG env should take priority over XDG default
        let config = OrchestratorConfig::load(None).unwrap();

        // Restore environment
        env::set_current_dir(original_dir).unwrap();
        match original_xdg {
            Some(val) => set_env_var("XDG_CONFIG_HOME", val),
            None => remove_env_var("XDG_CONFIG_HOME"),
        }
        match original_home {
            Some(val) => set_env_var("HOME", val),
            None => remove_env_var("HOME"),
        }

        // XDG env config should be loaded (higher priority)
        assert_eq!(
            config.get_apply_command().unwrap(),
            "xdg-env-agent apply {change_id}",
            "Expected XDG env config to take priority over XDG default config"
        );
    }

    // Server-only configuration is no longer a supported section.
    //
    // Unknown top-level keys stay tolerated (forward/backward compatibility), so
    // the contract is that a legacy `server` block parses without error and
    // contributes nothing — not that it is rejected.
    #[test]
    fn legacy_server_section_is_ignored_and_unrelated_config_still_parses() {
        let jsonc = r#"{
            "server": {
                "bind": "0.0.0.0",
                "port": 41234,
                "auth": { "mode": "bearer_token", "token": "secret" }
            },
            "proposal_session": {
                "transport_command": "custom-opencode"
            },
            "apply_command": "agent apply {change_id}",
            "max_iterations": 7
        }"#;

        let config = OrchestratorConfig::parse_jsonc(jsonc)
            .expect("a legacy server section must not break configuration parsing");

        assert_eq!(
            config.apply_command.as_deref(),
            Some("agent apply {change_id}")
        );
        assert_eq!(config.get_max_iterations(), 7);
    }

    // === Characterization tests: config loading priority (task 1.1) ===

    /// Characterizes the full merge priority order used by OrchestratorConfig::load().
    /// Order (lowest to highest): platform default → XDG default → XDG env → project → custom.
    /// A later merge() call wins when it supplies a Some value; None never overrides Some.
    #[test]
    fn test_characterize_merge_priority_full_order() {
        // Simulate each config layer using merge() in the same order as load()
        let platform = OrchestratorConfig {
            apply_command: Some("platform-apply".to_string()),
            archive_command: Some("platform-archive".to_string()),
            analyze_command: Some("platform-analyze".to_string()),
            acceptance_command: Some("platform-acceptance".to_string()),
            resolve_command: Some("platform-resolve".to_string()),
            ..Default::default()
        };
        let xdg_default = OrchestratorConfig {
            apply_command: Some("xdg-default-apply".to_string()),
            ..Default::default()
        };
        let xdg_env = OrchestratorConfig {
            apply_command: Some("xdg-env-apply".to_string()),
            archive_command: Some("xdg-env-archive".to_string()),
            ..Default::default()
        };
        let project = OrchestratorConfig {
            apply_command: Some("project-apply".to_string()),
            ..Default::default()
        };
        let custom = OrchestratorConfig {
            apply_command: Some("custom-apply".to_string()),
            ..Default::default()
        };

        let mut merged = OrchestratorConfig::default();
        merged.merge(platform);
        merged.merge(xdg_default);
        merged.merge(xdg_env);
        merged.merge(project);
        merged.merge(custom);

        // custom wins for apply_command (set at every layer; last write wins)
        assert_eq!(merged.apply_command, Some("custom-apply".to_string()));
        // xdg_env wins for archive_command (project and custom did not set it)
        assert_eq!(merged.archive_command, Some("xdg-env-archive".to_string()));
        // only platform set analyze/acceptance/resolve
        assert_eq!(merged.analyze_command, Some("platform-analyze".to_string()));
        assert_eq!(
            merged.acceptance_command,
            Some("platform-acceptance".to_string())
        );
        assert_eq!(merged.resolve_command, Some("platform-resolve".to_string()));
    }

    /// Characterizes that a None value in a higher-priority config does NOT overwrite a Some
    /// value from a lower-priority config.
    #[test]
    fn test_characterize_none_does_not_override_some() {
        let mut base = OrchestratorConfig {
            apply_command: Some("base-apply".to_string()),
            max_iterations: Some(42),
            ..Default::default()
        };
        // Higher-priority config that leaves these fields as None
        base.merge(OrchestratorConfig::default());

        assert_eq!(base.apply_command, Some("base-apply".to_string()));
        assert_eq!(base.max_iterations, Some(42));
    }

    /// Characterizes that a custom config (highest priority) overrides a project config while
    /// project config overrides a global config for the same field.
    #[test]
    fn test_characterize_custom_beats_project_beats_global() {
        let global = OrchestratorConfig {
            apply_command: Some("global-apply".to_string()),
            archive_command: Some("global-archive".to_string()),
            ..Default::default()
        };
        let project = OrchestratorConfig {
            apply_command: Some("project-apply".to_string()),
            // archive_command deliberately absent
            ..Default::default()
        };
        let custom = OrchestratorConfig {
            apply_command: Some("custom-apply".to_string()),
            // archive_command deliberately absent
            ..Default::default()
        };

        let mut merged = OrchestratorConfig::default();
        merged.merge(global);
        merged.merge(project);
        merged.merge(custom);

        // custom > project > global for apply_command
        assert_eq!(merged.apply_command, Some("custom-apply".to_string()));
        // project and custom did not set archive_command → global value is preserved
        assert_eq!(merged.archive_command, Some("global-archive".to_string()));
    }

    // === Characterization tests: config merge semantics ===

    #[test]
    fn test_characterize_some_values_override_across_option_field_types() {
        let mut base = OrchestratorConfig {
            apply_command: Some("base-apply".to_string()),
            max_iterations: Some(10),
            max_concurrent_workspaces: Some(2),
            vcs_backend: Some(VcsBackend::Auto),
            ..Default::default()
        };

        base.merge(OrchestratorConfig {
            apply_command: Some("override-apply".to_string()),
            max_iterations: Some(20),
            max_concurrent_workspaces: Some(4),
            vcs_backend: Some(VcsBackend::Git),
            ..Default::default()
        });

        assert_eq!(base.apply_command.as_deref(), Some("override-apply"));
        assert_eq!(base.max_iterations, Some(20));
        assert_eq!(base.max_concurrent_workspaces, Some(4));
        assert_eq!(base.vcs_backend, Some(VcsBackend::Git));
    }

    #[test]
    fn test_characterize_hooks_deep_merge_preserves_lower_priority_fields() {
        use crate::hooks::HookType;

        let mut base = OrchestratorConfig::parse_jsonc(
            r#"{
                "hooks": {
                    "on_start": "echo base start",
                    "pre_apply": "echo base pre_apply"
                }
            }"#,
        )
        .unwrap();
        let higher = OrchestratorConfig::parse_jsonc(
            r#"{
                "hooks": {
                    "on_finish": "echo higher finish",
                    "pre_apply": "echo higher pre_apply"
                }
            }"#,
        )
        .unwrap();

        base.merge(higher);
        let hooks = base.get_hooks();

        assert_eq!(
            hooks.get(HookType::OnStart).unwrap().command,
            "echo base start",
            "hooks merge must preserve lower-priority hook fields absent from higher-priority config"
        );
        assert_eq!(
            hooks.get(HookType::OnFinish).unwrap().command,
            "echo higher finish",
            "hooks merge must add higher-priority hook fields"
        );
        assert_eq!(
            hooks.get(HookType::PreApply).unwrap().command,
            "echo higher pre_apply",
            "hooks merge must let higher-priority hook fields override matching lower-priority fields"
        );
    }

    // === Characterization tests: JSONC deserialization and defaults (task 1.2) ===

    /// Characterizes default numeric/bool values exposed through getter methods.
    #[test]
    fn test_characterize_default_getter_values() {
        let config = OrchestratorConfig::default();

        assert_eq!(config.get_max_iterations(), 50);
        assert_eq!(config.get_max_concurrent_workspaces(), 3);
        assert_eq!(config.get_acceptance_max_continues(), 10);
        assert_eq!(config.get_command_inactivity_timeout_secs(), 900);
        assert_eq!(config.get_command_inactivity_kill_grace_secs(), 5);
        assert_eq!(config.get_command_inactivity_timeout_max_retries(), 3);
        assert_eq!(config.get_command_max_runtime_secs(), 10800);
        assert!(config.use_llm_analysis());
        assert!(config.get_stream_json_textify());
        assert!(config.get_command_strict_process_cleanup());
        assert_eq!(config.get_vcs_backend(), VcsBackend::Auto);
    }

    /// Characterizes that LoggingConfig defaults are applied when the field is absent from JSONC.
    #[test]
    fn test_characterize_logging_defaults_when_absent() {
        let config = OrchestratorConfig::parse_jsonc("{}").unwrap();
        // logging is None in the raw struct …
        assert!(config.logging.is_none());
        // … but get_logging() materialises defaults
        let logging = config.get_logging();
        assert!(logging.suppress_repetitive_debug);
        assert_eq!(logging.summary_interval_secs, 60);
    }

    /// Characterizes that JSONC comments (// and /* */) and trailing commas are accepted.
    #[test]
    fn test_characterize_jsonc_comment_styles_and_trailing_comma() {
        let jsonc = r#"{
            // single-line comment
            "apply_command": "cmd-apply",  // inline comment
            /* multi-line
               comment */
            "archive_command": "cmd-archive",
            "analyze_command": "cmd-analyze", // trailing comma on next line:
            "acceptance_command": "cmd-acceptance",
            "resolve_command": "cmd-resolve",
        }"#;

        let config = OrchestratorConfig::parse_jsonc(jsonc).unwrap();
        assert_eq!(config.apply_command, Some("cmd-apply".to_string()));
        assert_eq!(config.archive_command, Some("cmd-archive".to_string()));
        assert_eq!(config.analyze_command, Some("cmd-analyze".to_string()));
        assert_eq!(
            config.acceptance_command,
            Some("cmd-acceptance".to_string())
        );
        assert_eq!(config.resolve_command, Some("cmd-resolve".to_string()));
    }

    /// Characterizes that absent optional fields stay None after parsing, while getters
    /// return the hardcoded defaults.
    #[test]
    fn test_characterize_absent_optional_fields_stay_none() {
        let config = OrchestratorConfig::parse_jsonc(r#"{"apply_command": "x"}"#).unwrap();

        // Raw optional fields are None
        assert!(config.archive_command.is_none());
        assert!(config.logging.is_none());
        assert!(config.stall_detection.is_none());
        assert!(config.max_iterations.is_none());

        // But getters resolve to their defaults
        assert_eq!(config.get_max_iterations(), 50);
    }
}