//! CLI-side overrides for the suite's `servers:` map.
//!
//! Preview-deploy and CI scenarios often do not know the target URL or
//! subprocess command at the moment a YAML test file is authored. The
//! global `--server-url`, `--server-command`, `--server-auth-bearer-env`,
//! and `--server-config` flags exist so the same YAML can target a
//! freshly-deployed preview without editing the file. This module owns the
//! collection and application of those flags. The overrides retarget the
//! sole entry in the suite's `servers:` map (the only place the suite
//! parser reads server specs from); a multi-server config is rejected
//! because the flags cannot guess which server to redirect.

use std::fs;
use std::path::PathBuf;
use std::time::Duration;

use anyhow::{anyhow, Context, Result};
use mcptest_core::transport::HeaderValue;
use serde_yaml::Value;

use crate::cli::Cli;

/// Aggregated CLI-side server-target overrides.
///
/// Built from the global flags. The runner consumes this when loading a
/// YAML config to replace or augment the sole entry in the suite's
/// `servers:` map.
#[derive(Debug, Default, Clone)]
pub struct ServerOverride {
    /// Replacement for the target server's `url`. Set by `--server-url`.
    pub url: Option<String>,
    /// Replacement for the target server's `command`. Already split into
    /// argv via `shell-words`.
    pub command: Option<Vec<String>>,
    /// Replacement for the target server's `auth.bearer_token_env`.
    pub auth_bearer_env: Option<String>,
    /// Path to a YAML file holding a full `server:` block.
    pub config_path: Option<PathBuf>,
    /// Custom HTTP headers added to the target server's `headers`. Each
    /// entry is a `(name, HeaderValue)` pair; later entries win on
    /// collision. CLI `--header` and `--header-env` flow in here in
    /// source order.
    pub headers: Vec<(String, HeaderValue)>,
    /// True when `--insecure-skip-verify` is set. Surfaces in the banner so
    /// the operator can spot a misconfigured CI job at a glance.
    pub insecure_skip_verify: bool,
    /// Path to a PEM-encoded CA bundle. Applied to the target server's
    /// `http.tls.ca_bundle_path`.
    pub ca_bundle: Option<PathBuf>,
    /// Override for the target server's `http.timeout`.
    pub http_timeout: Option<Duration>,
    /// Override for the target server's `http.connect_timeout`.
    pub connect_timeout: Option<Duration>,
}

impl ServerOverride {
    /// Build from the parsed [`Cli`].
    ///
    /// `--server-command` is split with `shell-words` so the runner sees a
    /// proper argv. A parse failure (mismatched quotes, etc.) surfaces as an
    /// `Err` here rather than later when the runner spawns the child.
    pub fn from_cli(cli: &Cli) -> Result<Self> {
        let command = match &cli.global.server_command {
            Some(raw) => Some(
                shell_words::split(raw)
                    .with_context(|| format!("failed to parse --server-command: {raw}"))?,
            ),
            None => None,
        };

        let mut headers: Vec<(String, HeaderValue)> = Vec::new();
        for (name, value) in &cli.global.headers_literal {
            headers.push((name.clone(), HeaderValue::literal(value.clone())));
        }
        for (name, var) in &cli.global.headers_env {
            headers.push((name.clone(), HeaderValue::from_env(var.clone())));
        }

        Ok(Self {
            url: cli.global.server_url.clone(),
            command,
            auth_bearer_env: cli.global.server_auth_bearer_env.clone(),
            config_path: cli.global.server_config.clone(),
            headers,
            insecure_skip_verify: cli.global.insecure_skip_verify,
            ca_bundle: cli.global.ca_bundle.clone(),
            http_timeout: cli.global.http_timeout,
            connect_timeout: cli.global.connect_timeout,
        })
    }

    /// True if any override flag is set.
    pub fn is_any(&self) -> bool {
        self.url.is_some()
            || self.command.is_some()
            || self.auth_bearer_env.is_some()
            || self.config_path.is_some()
            || !self.headers.is_empty()
            || self.insecure_skip_verify
            || self.ca_bundle.is_some()
            || self.http_timeout.is_some()
            || self.connect_timeout.is_some()
    }

    /// Validate the combination of override flags.
    ///
    /// Rejects `--server-url` together with `--server-command` (the two
    /// pick conflicting target shapes). Confirms `--server-config` points
    /// at an existing readable file. Catches header-name mistakes
    /// (Authorization in `--header`, bogus characters) before the runner
    /// would have to surface them.
    pub fn validate(&self) -> Result<()> {
        if self.url.is_some() && self.command.is_some() {
            return Err(anyhow!(
                "--server-url and --server-command are mutually exclusive (pick one target shape)"
            ));
        }
        if let Some(path) = &self.config_path {
            if !path.exists() {
                return Err(anyhow!(
                    "--server-config path does not exist: {}",
                    path.display()
                ));
            }
        }
        // Header names are validated by the shared `HeaderValidator` so the
        // CLI and the YAML loader reject the same set of names with the same
        // error message.
        let mut by_name: std::collections::HashMap<String, HeaderValue> =
            std::collections::HashMap::new();
        for (name, value) in &self.headers {
            by_name.insert(name.clone(), value.clone());
        }
        mcptest_core::transport::HeaderValidator::validate(&by_name)
            .map_err(|e| anyhow!("--header / --header-env: {e}"))?;
        Ok(())
    }

    /// Human-readable list of which flags are set.
    ///
    /// Used to render the runner banner so the operator sees which flag
    /// changed the target before any test runs.
    pub fn flag_names(&self) -> Vec<&'static str> {
        let mut names = Vec::new();
        if self.url.is_some() {
            names.push("--server-url");
        }
        if self.command.is_some() {
            names.push("--server-command");
        }
        if self.auth_bearer_env.is_some() {
            names.push("--server-auth-bearer-env");
        }
        if self.config_path.is_some() {
            names.push("--server-config");
        }
        if !self.headers.is_empty() {
            // Distinguish literal vs env in the banner so the operator does
            // not have to grep the YAML when something looks off.
            let mut had_literal = false;
            let mut had_env = false;
            for (_, value) in &self.headers {
                match value {
                    HeaderValue::Literal(_) => had_literal = true,
                    HeaderValue::Env(_) => had_env = true,
                }
            }
            if had_literal {
                names.push("--header");
            }
            if had_env {
                names.push("--header-env");
            }
        }
        if self.insecure_skip_verify {
            names.push("--insecure-skip-verify");
        }
        if self.ca_bundle.is_some() {
            names.push("--ca-bundle");
        }
        if self.http_timeout.is_some() {
            names.push("--http-timeout");
        }
        if self.connect_timeout.is_some() {
            names.push("--connect-timeout");
        }
        names
    }

    /// True if `--insecure-skip-verify` is set.
    ///
    /// Pulled out so the run handler can emit the warning banner without
    /// re-checking the same boolean.
    pub fn warns_about_insecure_skip_verify(&self) -> bool {
        self.insecure_skip_verify
    }

    /// Apply this override to a parsed YAML config.
    ///
    /// The override always targets the suite's `servers:` map, which is
    /// the only place [`mcptest_core::suite::parse`] reads server specs
    /// from. Single-server semantics keep the retarget unambiguous:
    /// - Exactly one entry in `servers:`: the overrides land on it.
    /// - `servers:` absent or empty: a single `default` entry is created.
    /// - More than one entry: an error, because the flags cannot guess
    ///   which server to redirect.
    ///
    /// Precedence (lowest to highest):
    /// 1. The YAML file's own `servers:` entry.
    /// 2. `--server-config` (full-block replacement of the target entry,
    ///    sourced from the external file's `server:` block).
    /// 3. `--server-url`, `--server-command`, `--server-auth-bearer-env`,
    ///    and the transport flags (single-field overrides). A single-field
    ///    flag wins over the same field in `--server-config`.
    ///
    /// Returns the human-readable notes the run handler can print.
    pub fn apply(&self, yaml: &mut Value) -> Result<Vec<String>> {
        let mut notes = Vec::new();
        let mapping = yaml
            .as_mapping_mut()
            .ok_or_else(|| anyhow!("top-level YAML must be a mapping"))?;

        // Resolve which entry in `servers:` the overrides target. The
        // override is single-server by design, so a config with two or
        // more servers is an honest error rather than a silent guess.
        let target_name = resolve_target_server(mapping)?;

        // Step 1: --server-config replaces the entire target entry.
        self.apply_server_config(mapping, &target_name, &mut notes)?;

        // Step 2: single-field flags rewrite the target entry in place.
        let server_map = target_entry_mut(mapping, &target_name)?;
        self.apply_url(server_map, &target_name, &mut notes);
        self.apply_command(server_map, &target_name, &mut notes);
        self.apply_auth_bearer(server_map, &target_name, &mut notes)?;
        self.apply_headers(server_map, &target_name, &mut notes)?;
        self.apply_http_options(server_map, &target_name, &mut notes)?;

        Ok(notes)
    }

    /// Step 1: `--server-config` replaces the entire target entry. The
    /// external file still uses a `server:` block; we lift that block into the
    /// `servers:` map under the target name.
    fn apply_server_config(
        &self,
        mapping: &mut serde_yaml::Mapping,
        target_name: &str,
        notes: &mut Vec<String>,
    ) -> Result<()> {
        let Some(path) = &self.config_path else {
            return Ok(());
        };
        let text = fs::read_to_string(path)
            .with_context(|| format!("reading --server-config {}", path.display()))?;
        let loaded: Value = serde_yaml::from_str(&text)
            .with_context(|| format!("parsing --server-config {}", path.display()))?;
        let loaded_map = loaded
            .as_mapping()
            .ok_or_else(|| anyhow!("--server-config must be a YAML mapping"))?;
        let server_block = loaded_map
            .get(Value::String("server".to_owned()))
            .ok_or_else(|| anyhow!("--server-config file must contain a `server:` block"))?
            .clone();
        servers_map(mapping)?.insert(Value::String(target_name.to_owned()), server_block);
        notes.push(format!(
            "servers.{target_name} replaced from --server-config {}",
            path.display()
        ));
        Ok(())
    }

    /// `--server-url` override: set `url:` and drop a conflicting `command:`.
    fn apply_url(
        &self,
        server_map: &mut serde_yaml::Mapping,
        target_name: &str,
        notes: &mut Vec<String>,
    ) {
        let Some(url) = &self.url else {
            return;
        };
        let had_command = server_map
            .remove(Value::String("command".to_owned()))
            .is_some();
        server_map.insert(Value::String("url".to_owned()), Value::String(url.clone()));
        if had_command {
            notes.push(format!(
                "servers.{target_name} target overridden from command to URL"
            ));
        } else {
            notes.push(format!("servers.{target_name}.url overridden to {url}"));
        }
    }

    /// `--server-command` override: set `command:` and drop a conflicting `url:`.
    fn apply_command(
        &self,
        server_map: &mut serde_yaml::Mapping,
        target_name: &str,
        notes: &mut Vec<String>,
    ) {
        let Some(cmd) = &self.command else {
            return;
        };
        let had_url = server_map.remove(Value::String("url".to_owned())).is_some();
        let argv: Vec<Value> = cmd.iter().map(|s| Value::String(s.clone())).collect();
        server_map.insert(Value::String("command".to_owned()), Value::Sequence(argv));
        if had_url {
            notes.push(format!(
                "servers.{target_name} target overridden from URL to command"
            ));
        } else {
            notes.push(format!("servers.{target_name}.command overridden"));
        }
    }

    /// `--server-auth-bearer-env` override: set `auth.bearer_token_env`.
    fn apply_auth_bearer(
        &self,
        server_map: &mut serde_yaml::Mapping,
        target_name: &str,
        notes: &mut Vec<String>,
    ) -> Result<()> {
        let Some(env_name) = &self.auth_bearer_env else {
            return Ok(());
        };
        let auth_value = server_map
            .entry(Value::String("auth".to_owned()))
            .or_insert_with(|| Value::Mapping(serde_yaml::Mapping::new()));
        let auth_map = auth_value
            .as_mapping_mut()
            .ok_or_else(|| anyhow!("`servers.{target_name}.auth:` block must be a mapping"))?;
        auth_map.insert(
            Value::String("bearer_token_env".to_owned()),
            Value::String(env_name.clone()),
        );
        notes.push(format!(
            "servers.{target_name}.auth.bearer_token_env overridden to {env_name}"
        ));
        Ok(())
    }

    /// Merge custom headers from `--header` / `--header-env` into the target
    /// entry. The merge keeps the last-wins semantics documented for the
    /// header merger.
    fn apply_headers(
        &self,
        server_map: &mut serde_yaml::Mapping,
        target_name: &str,
        notes: &mut Vec<String>,
    ) -> Result<()> {
        if self.headers.is_empty() {
            return Ok(());
        }
        let headers_value = server_map
            .entry(Value::String("headers".to_owned()))
            .or_insert_with(|| Value::Mapping(serde_yaml::Mapping::new()));
        let headers_map = headers_value
            .as_mapping_mut()
            .ok_or_else(|| anyhow!("`servers.{target_name}.headers:` block must be a mapping"))?;
        for (name, value) in &self.headers {
            let yaml_value = match value {
                HeaderValue::Literal(s) => Value::String(s.clone()),
                HeaderValue::Env(env) => {
                    let mut m = serde_yaml::Mapping::new();
                    m.insert(
                        Value::String("env".to_owned()),
                        Value::String(env.env.clone()),
                    );
                    Value::Mapping(m)
                }
            };
            headers_map.insert(Value::String(name.clone()), yaml_value);
        }
        notes.push(format!(
            "servers.{target_name}.headers merged ({} CLI header(s))",
            self.headers.len()
        ));
        Ok(())
    }

    /// Merge http transport options. We touch only the fields the user
    /// actually set, so a CLI `--http-timeout` does not silently rewrite a
    /// YAML `connect_timeout`.
    fn apply_http_options(
        &self,
        server_map: &mut serde_yaml::Mapping,
        target_name: &str,
        notes: &mut Vec<String>,
    ) -> Result<()> {
        let needs_http = self.insecure_skip_verify
            || self.ca_bundle.is_some()
            || self.http_timeout.is_some()
            || self.connect_timeout.is_some();
        if !needs_http {
            return Ok(());
        }
        let http_value = server_map
            .entry(Value::String("http".to_owned()))
            .or_insert_with(|| Value::Mapping(serde_yaml::Mapping::new()));
        let http_map = http_value
            .as_mapping_mut()
            .ok_or_else(|| anyhow!("`servers.{target_name}.http:` block must be a mapping"))?;

        if let Some(t) = self.http_timeout {
            http_map.insert(
                Value::String("timeout".to_owned()),
                Value::String(format!("{}s", t.as_secs())),
            );
            notes.push(format!(
                "servers.{target_name}.http.timeout overridden to {}s",
                t.as_secs()
            ));
        }
        if let Some(t) = self.connect_timeout {
            http_map.insert(
                Value::String("connect_timeout".to_owned()),
                Value::String(format!("{}s", t.as_secs())),
            );
            notes.push(format!(
                "servers.{target_name}.http.connect_timeout overridden to {}s",
                t.as_secs()
            ));
        }
        self.apply_tls_options(http_map, target_name, notes)
    }

    /// Merge the TLS sub-block of the http transport options.
    fn apply_tls_options(
        &self,
        http_map: &mut serde_yaml::Mapping,
        target_name: &str,
        notes: &mut Vec<String>,
    ) -> Result<()> {
        if !(self.insecure_skip_verify || self.ca_bundle.is_some()) {
            return Ok(());
        }
        let tls_value = http_map
            .entry(Value::String("tls".to_owned()))
            .or_insert_with(|| Value::Mapping(serde_yaml::Mapping::new()));
        let tls_map = tls_value
            .as_mapping_mut()
            .ok_or_else(|| anyhow!("`servers.{target_name}.http.tls:` block must be a mapping"))?;
        if self.insecure_skip_verify {
            tls_map.insert(
                Value::String("insecure_skip_verify".to_owned()),
                Value::Bool(true),
            );
            notes.push(format!(
                "servers.{target_name}.http.tls.insecure_skip_verify enabled (DANGEROUS)"
            ));
        }
        if let Some(path) = &self.ca_bundle {
            tls_map.insert(
                Value::String("ca_bundle_path".to_owned()),
                Value::String(path.display().to_string()),
            );
            notes.push(format!(
                "servers.{target_name}.http.tls.ca_bundle_path overridden to {}",
                path.display()
            ));
        }
        Ok(())
    }
}

/// Default name for the synthesized `servers:` entry when the config
/// declares none. Single-server overrides need a name to key the entry
/// in the map; `default` reads clearly in `--print-config` output.
const DEFAULT_SERVER_NAME: &str = "default";

/// Decide which `servers:` entry the override targets, enforcing
/// single-server semantics.
///
/// Returns `default` when `servers:` is absent or empty, the sole key
/// when exactly one entry exists, and an error naming the count when two
/// or more entries exist (the flags cannot guess which one to redirect).
fn resolve_target_server(mapping: &serde_yaml::Mapping) -> Result<String> {
    let Some(servers) = mapping
        .get(Value::String("servers".to_owned()))
        .and_then(Value::as_mapping)
    else {
        return Ok(DEFAULT_SERVER_NAME.to_owned());
    };
    match servers.len() {
        0 => Ok(DEFAULT_SERVER_NAME.to_owned()),
        1 => {
            let (key, _) = servers
                .iter()
                .next()
                .ok_or_else(|| anyhow!("`servers:` map reported one entry but yielded none"))?;
            let name = key
                .as_str()
                .ok_or_else(|| anyhow!("`servers:` keys must be strings"))?;
            Ok(name.to_owned())
        }
        n => Err(anyhow!(
            "server-target overrides need a single-server config, but `servers:` declares {n} \
             servers; name the one to redirect in the YAML or split the suite"
        )),
    }
}

/// Borrow the top-level `servers:` mapping, creating an empty one if the
/// config did not declare it.
fn servers_map(mapping: &mut serde_yaml::Mapping) -> Result<&mut serde_yaml::Mapping> {
    mapping
        .entry(Value::String("servers".to_owned()))
        .or_insert_with(|| Value::Mapping(serde_yaml::Mapping::new()))
        .as_mapping_mut()
        .ok_or_else(|| anyhow!("`servers:` block must be a mapping"))
}

/// Borrow the target entry inside `servers:`, creating an empty mapping
/// when the entry does not exist yet (the synthesized `default` case).
fn target_entry_mut<'a>(
    mapping: &'a mut serde_yaml::Mapping,
    target_name: &str,
) -> Result<&'a mut serde_yaml::Mapping> {
    servers_map(mapping)?
        .entry(Value::String(target_name.to_owned()))
        .or_insert_with(|| Value::Mapping(serde_yaml::Mapping::new()))
        .as_mapping_mut()
        .ok_or_else(|| anyhow!("`servers.{target_name}:` entry must be a mapping"))
}

#[cfg(test)]
mod tests {
    use super::*;
    use clap::Parser;
    use std::io::Write;
    use tempfile::NamedTempFile;

    fn parse(args: &[&str]) -> Cli {
        Cli::try_parse_from(args).expect("parse")
    }

    fn yaml(text: &str) -> Value {
        serde_yaml::from_str(text).expect("yaml parse")
    }

    /// Borrow the named entry from the resolved `servers:` map.
    fn server_entry<'a>(value: &'a Value, name: &str) -> &'a serde_yaml::Mapping {
        value
            .get("servers")
            .and_then(Value::as_mapping)
            .expect("servers map")
            .get(Value::String(name.to_owned()))
            .and_then(Value::as_mapping)
            .unwrap_or_else(|| panic!("servers.{name} entry"))
    }

    #[test]
    fn from_cli_reads_each_flag() {
        let cli = parse(&[
            "mcptest",
            "--server-url",
            "https://example.com",
            "--server-auth-bearer-env",
            "MY_TOKEN",
            "run",
        ]);
        let ov = ServerOverride::from_cli(&cli).expect("build");
        assert_eq!(ov.url.as_deref(), Some("https://example.com"));
        assert_eq!(ov.auth_bearer_env.as_deref(), Some("MY_TOKEN"));
        assert!(ov.command.is_none());
        assert!(ov.config_path.is_none());
        assert!(ov.is_any());

        let cli = parse(&["mcptest", "--server-command", "./dev --flag a b", "run"]);
        let ov = ServerOverride::from_cli(&cli).expect("build");
        assert_eq!(
            ov.command,
            Some(vec![
                "./dev".to_owned(),
                "--flag".to_owned(),
                "a".to_owned(),
                "b".to_owned(),
            ])
        );
    }

    #[test]
    fn validate_rejects_url_and_command_together() {
        let cli = parse(&[
            "mcptest",
            "--server-url",
            "https://example.com",
            "--server-command",
            "./dev",
            "run",
        ]);
        let ov = ServerOverride::from_cli(&cli).expect("build");
        let err = ov.validate().expect_err("should reject");
        assert!(err.to_string().contains("mutually exclusive"));
    }

    #[test]
    fn validate_accepts_each_supported_combination() {
        let mut tmp = NamedTempFile::new().unwrap();
        writeln!(tmp, "server:\n  url: https://staging.example.com").unwrap();

        let cli = parse(&[
            "mcptest",
            "--server-config",
            tmp.path().to_str().unwrap(),
            "run",
        ]);
        ServerOverride::from_cli(&cli).unwrap().validate().unwrap();

        let cli = parse(&[
            "mcptest",
            "--server-config",
            tmp.path().to_str().unwrap(),
            "--server-url",
            "https://example.com",
            "--server-auth-bearer-env",
            "TOKEN",
            "run",
        ]);
        ServerOverride::from_cli(&cli).unwrap().validate().unwrap();

        let cli = parse(&["mcptest", "run"]);
        let ov = ServerOverride::from_cli(&cli).unwrap();
        assert!(!ov.is_any());
        ov.validate().unwrap();
    }

    #[test]
    fn apply_url_overwrites_sole_servers_entry_command() {
        let mut value = yaml(
            r#"
servers:
  api:
    command: ["./dev-server", "--debug"]
tools: []
"#,
        );
        let ov = ServerOverride {
            url: Some("https://preview.example.com".to_owned()),
            ..Default::default()
        };
        let notes = ov.apply(&mut value).unwrap();
        let server = server_entry(&value, "api");
        assert!(server.get(Value::String("command".to_owned())).is_none());
        assert_eq!(
            server
                .get(Value::String("url".to_owned()))
                .and_then(Value::as_str),
            Some("https://preview.example.com")
        );
        assert!(
            notes
                .iter()
                .any(|n| n.contains("overridden from command to URL")),
            "notes missing target-type conversion: {notes:?}"
        );
    }

    #[test]
    fn apply_command_overwrites_sole_servers_entry_url() {
        let mut value = yaml(
            r#"
servers:
  api:
    url: https://prod.example.com
    auth:
      bearer_token_env: PROD_TOKEN
tools: []
"#,
        );
        let ov = ServerOverride {
            command: Some(vec!["./dev".to_owned(), "--flag".to_owned()]),
            ..Default::default()
        };
        let notes = ov.apply(&mut value).unwrap();
        let server = server_entry(&value, "api");
        assert!(server.get(Value::String("url".to_owned())).is_none());
        let cmd = server
            .get(Value::String("command".to_owned()))
            .and_then(Value::as_sequence)
            .expect("command seq");
        let argv: Vec<&str> = cmd.iter().filter_map(Value::as_str).collect();
        assert_eq!(argv, vec!["./dev", "--flag"]);
        assert!(
            notes
                .iter()
                .any(|n| n.contains("overridden from URL to command")),
            "notes missing target-type conversion: {notes:?}"
        );
    }

    #[test]
    fn apply_creates_default_entry_when_servers_absent() {
        let mut value = yaml(
            r#"
tools: []
"#,
        );
        let ov = ServerOverride {
            url: Some("https://preview.example.com".to_owned()),
            ..Default::default()
        };
        ov.apply(&mut value).unwrap();
        let server = server_entry(&value, "default");
        assert_eq!(
            server
                .get(Value::String("url".to_owned()))
                .and_then(Value::as_str),
            Some("https://preview.example.com")
        );
    }

    #[test]
    fn apply_rejects_multi_server_config() {
        let mut value = yaml(
            r#"
servers:
  api:
    url: https://api.example.com
  worker:
    url: https://worker.example.com
tools: []
"#,
        );
        let ov = ServerOverride {
            url: Some("https://preview.example.com".to_owned()),
            ..Default::default()
        };
        let err = ov.apply(&mut value).expect_err("multi-server must reject");
        let msg = err.to_string();
        assert!(msg.contains("single-server"), "msg: {msg}");
        assert!(msg.contains('2'), "error should name the count: {msg}");
    }

    #[test]
    fn apply_server_config_replaces_sole_entry() {
        let mut tmp = NamedTempFile::new().unwrap();
        writeln!(
            tmp,
            "server:\n  url: https://from-file.example.com\n  auth:\n    bearer_token_env: FILE_TOKEN"
        )
        .unwrap();

        let mut value = yaml(
            r#"
servers:
  api:
    command: ["./old"]
tools: []
"#,
        );
        let ov = ServerOverride {
            config_path: Some(tmp.path().to_path_buf()),
            ..Default::default()
        };
        let notes = ov.apply(&mut value).unwrap();
        let server = server_entry(&value, "api");
        assert!(server.get(Value::String("command".to_owned())).is_none());
        assert_eq!(
            server
                .get(Value::String("url".to_owned()))
                .and_then(Value::as_str),
            Some("https://from-file.example.com")
        );
        assert!(
            notes.iter().any(|n| n.contains("replaced from")),
            "notes missing block-replacement: {notes:?}"
        );
    }

    #[test]
    fn apply_url_flag_wins_over_server_config_url() {
        let mut tmp = NamedTempFile::new().unwrap();
        writeln!(
            tmp,
            "server:\n  url: https://from-file.example.com\n  auth:\n    bearer_token_env: FILE_TOKEN"
        )
        .unwrap();

        let mut value = yaml(
            r#"
servers:
  api:
    command: ["./old"]
tools: []
"#,
        );
        let ov = ServerOverride {
            url: Some("https://flag-wins.example.com".to_owned()),
            config_path: Some(tmp.path().to_path_buf()),
            ..Default::default()
        };
        ov.apply(&mut value).unwrap();
        let server = server_entry(&value, "api");
        assert_eq!(
            server
                .get(Value::String("url".to_owned()))
                .and_then(Value::as_str),
            Some("https://flag-wins.example.com")
        );
        // bearer_token_env came from --server-config (no flag overrode it).
        let auth = server
            .get(Value::String("auth".to_owned()))
            .and_then(Value::as_mapping)
            .expect("auth map");
        assert_eq!(
            auth.get(Value::String("bearer_token_env".to_owned()))
                .and_then(Value::as_str),
            Some("FILE_TOKEN")
        );
    }

    #[test]
    fn apply_sets_bearer_token_env() {
        let mut value = yaml(
            r#"
servers:
  api:
    url: https://example.com
tools: []
"#,
        );
        let ov = ServerOverride {
            auth_bearer_env: Some("MY_PREVIEW_TOKEN".to_owned()),
            ..Default::default()
        };
        let notes = ov.apply(&mut value).unwrap();
        let server = server_entry(&value, "api");
        let auth = server
            .get(Value::String("auth".to_owned()))
            .and_then(Value::as_mapping)
            .expect("auth map");
        assert_eq!(
            auth.get(Value::String("bearer_token_env".to_owned()))
                .and_then(Value::as_str),
            Some("MY_PREVIEW_TOKEN")
        );
        assert!(
            notes
                .iter()
                .any(|n| n.contains("bearer_token_env overridden")),
            "notes missing bearer override: {notes:?}"
        );
    }

    #[test]
    fn validate_rejects_missing_server_config_path() {
        let cli = parse(&[
            "mcptest",
            "--server-config",
            "/this/path/does/not/exist.yaml",
            "run",
        ]);
        let ov = ServerOverride::from_cli(&cli).unwrap();
        let err = ov.validate().expect_err("missing path should fail");
        assert!(err.to_string().contains("does not exist"));
    }

    #[test]
    fn flag_names_reports_each_set_flag() {
        let ov = ServerOverride {
            url: Some("u".to_owned()),
            auth_bearer_env: Some("E".to_owned()),
            ..Default::default()
        };
        assert_eq!(
            ov.flag_names(),
            vec!["--server-url", "--server-auth-bearer-env"]
        );
    }

    #[test]
    fn from_cli_collects_literal_and_env_headers() {
        let cli = parse(&[
            "mcptest",
            "--header",
            "X-Tenant=acme",
            "--header-env",
            "X-API=MY_TOKEN",
            "--header",
            "X-Trace-Id=abc-123",
            "run",
        ]);
        let ov = ServerOverride::from_cli(&cli).unwrap();
        assert_eq!(ov.headers.len(), 3);
        // Literals come before env-backed values; downstream merging is
        // case-insensitive so the order across the two groups does not
        // matter for correctness.
        let by_name: std::collections::HashMap<_, _> = ov
            .headers
            .iter()
            .map(|(n, v)| (n.clone(), v.clone()))
            .collect();
        assert!(matches!(by_name.get("X-Tenant"), Some(HeaderValue::Literal(s)) if s == "acme"));
        assert!(
            matches!(by_name.get("X-Trace-Id"), Some(HeaderValue::Literal(s)) if s == "abc-123")
        );
        assert!(matches!(by_name.get("X-API"), Some(HeaderValue::Env(e)) if e.env == "MY_TOKEN"));
        assert!(ov.is_any());
    }

    #[test]
    fn validate_rejects_authorization_in_header_flag() {
        let cli = parse(&["mcptest", "--header", "Authorization=Bearer x", "run"]);
        let ov = ServerOverride::from_cli(&cli).unwrap();
        let err = ov.validate().expect_err("must reject");
        assert!(err.to_string().contains("auth:"));
    }

    #[test]
    fn validate_rejects_invalid_header_name() {
        let cli = parse(&["mcptest", "--header", "X Header With Spaces=foo", "run"]);
        let ov = ServerOverride::from_cli(&cli).unwrap();
        let err = ov.validate().expect_err("must reject");
        assert!(err.to_string().contains("invalid character"));
    }

    #[test]
    fn apply_merges_headers_into_server_block() {
        let mut value = yaml(
            r#"
servers:
  api:
    url: https://example.com
tools: []
"#,
        );
        let ov = ServerOverride {
            url: None,
            headers: vec![
                ("X-Tenant".to_owned(), HeaderValue::literal("acme")),
                (
                    "X-API".to_owned(),
                    HeaderValue::from_env("MY_TOKEN".to_owned()),
                ),
            ],
            ..Default::default()
        };
        let notes = ov.apply(&mut value).unwrap();
        let server = server_entry(&value, "api");
        let headers = server
            .get(Value::String("headers".to_owned()))
            .and_then(Value::as_mapping)
            .expect("headers map");
        assert_eq!(
            headers
                .get(Value::String("X-Tenant".to_owned()))
                .and_then(Value::as_str),
            Some("acme")
        );
        let api = headers
            .get(Value::String("X-API".to_owned()))
            .and_then(Value::as_mapping)
            .expect("env mapping");
        assert_eq!(
            api.get(Value::String("env".to_owned()))
                .and_then(Value::as_str),
            Some("MY_TOKEN")
        );
        assert!(notes.iter().any(|n| n.contains(".headers merged")));
    }

    #[test]
    fn apply_writes_http_timeouts_and_tls_overrides() {
        let mut value = yaml(
            r#"
servers:
  api:
    url: https://example.com
tools: []
"#,
        );
        let ov = ServerOverride {
            url: None,
            insecure_skip_verify: true,
            ca_bundle: Some(PathBuf::from("/tmp/ca.pem")),
            http_timeout: Some(Duration::from_secs(45)),
            connect_timeout: Some(Duration::from_secs(3)),
            ..Default::default()
        };
        let _ = ov.apply(&mut value).unwrap();
        let server = server_entry(&value, "api");
        let http = server
            .get(Value::String("http".to_owned()))
            .and_then(Value::as_mapping)
            .expect("http map");
        assert_eq!(
            http.get(Value::String("timeout".to_owned()))
                .and_then(Value::as_str),
            Some("45s")
        );
        assert_eq!(
            http.get(Value::String("connect_timeout".to_owned()))
                .and_then(Value::as_str),
            Some("3s")
        );
        let tls = http
            .get(Value::String("tls".to_owned()))
            .and_then(Value::as_mapping)
            .expect("tls map");
        assert_eq!(
            tls.get(Value::String("insecure_skip_verify".to_owned()))
                .and_then(Value::as_bool),
            Some(true)
        );
        assert_eq!(
            tls.get(Value::String("ca_bundle_path".to_owned()))
                .and_then(Value::as_str),
            Some("/tmp/ca.pem")
        );
    }

    #[test]
    fn flag_names_includes_transport_flags_when_set() {
        let ov = ServerOverride {
            headers: vec![("X-Tenant".to_owned(), HeaderValue::literal("acme"))],
            insecure_skip_verify: true,
            ca_bundle: Some(PathBuf::from("/tmp/ca.pem")),
            http_timeout: Some(Duration::from_secs(10)),
            connect_timeout: Some(Duration::from_secs(2)),
            ..Default::default()
        };
        let names = ov.flag_names();
        assert!(names.contains(&"--header"));
        assert!(names.contains(&"--insecure-skip-verify"));
        assert!(names.contains(&"--ca-bundle"));
        assert!(names.contains(&"--http-timeout"));
        assert!(names.contains(&"--connect-timeout"));
    }
}
