travelagent-core 1.10.2

//! Application config — rendering, keybinds, theme, forge-host
//! mapping, auto-collapse rules, session-gc policy, mental-model
//! bounds. The file is **shared and committable**: the global
//! `~/.config/travelagent/config.toml` and the per-repo
//! `<repo_root>/.travelagent/config.toml` are both parsed by the
//! machinery in this module, and per-repo values override global
//! ones per the layering rules in [`merge_overrides`].
//!
//! ## Split with [`crate::review_config`]
//!
//! `review_config` is a separate file because it carries the
//! reviewer's *review flow* — what the reviewer sees in the diff
//! and when they see it. Reviewer preferences are not generally
//! something other people on the project want to inherit, so
//! splitting them into `.travelagent/review.toml` (often
//! gitignored) keeps this file (`config.toml`, typically
//! committed) free of them.
//!
//! **Rule of thumb for new keys.** If the behaviour is a per-repo
//! team default (risk patterns, auto-collapse globs, forge
//! host-to-type mapping), land it in `config.toml`. If the
//! behaviour is reviewer-local (what the reviewer can see,
//! sparring-mode state, the reviewer's blind-tests filter), land
//! it in `review.toml`. The boundary is "is this safe to commit
//! to a shared repo" — not file size.

use std::fs;
use std::io::ErrorKind;
use std::path::{Path, PathBuf};

use anyhow::{Result, anyhow};
use serde::{Deserialize, Serialize};
use toml::Value;

#[derive(Debug, Clone, Default, Deserialize, Serialize, PartialEq, Eq)]
#[serde(default)]
pub struct CommentTypeConfig {
    pub id: String,
    pub label: Option<String>,
    pub definition: Option<String>,
    pub color: Option<String>,
}

/// `[session_gc]` section controlling session-directory rotation. Each bound
/// uses `0` to mean "unbounded" so an empty or partial section still round-trips
/// with the historical 7-day-age-only behaviour.
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)]
#[serde(default)]
pub struct SessionGcConfig {
    pub max_age_days: u64,
    pub max_size_mb: u64,
    pub max_count: u64,
}

/// Today's behavior: purge after 7 days, no size/count caps.
pub const DEFAULT_SESSION_MAX_AGE_DAYS: u64 = 7;

impl Default for SessionGcConfig {
    fn default() -> Self {
        Self {
            max_age_days: DEFAULT_SESSION_MAX_AGE_DAYS,
            max_size_mb: 0,
            max_count: 0,
        }
    }
}

/// `[mental_model]` section controlling the pre-diff reflection panel
/// (Phase I2). Only one knob today; structured as a section so we can
/// add follow-up fields (e.g. custom prompts, auto-open behavior)
/// without another config breakage.
///
/// The field is `byte_limit` because the cap is enforced on `String`
/// byte length, not Unicode scalars. The legacy TOML key `char_limit`
/// (shipped in v1.5.0) was removed in v1.6.0 — users upgrading from
/// v1.5.x see an "unknown config key" warning and fall back to the
/// default.
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)]
#[serde(default)]
pub struct MentalModelConfig {
    /// Maximum bytes per field. Bounded at input time in the TUI modal
    /// and on the MCP boundary; the core model type accepts any
    /// `String` so sessions written at a different cap still
    /// round-trip.
    pub byte_limit: usize,
}

/// 2 KiB — comfortable for a paragraph of reflection per prompt.
/// Four fields × default → 8 KiB session-size overhead ceiling, which
/// is negligible against the 1 MB sessions the round-trip test
/// exercises.
pub const DEFAULT_MENTAL_MODEL_BYTE_LIMIT: usize = 2048;

/// Upper bound on the configurable `[mental_model].byte_limit`. Caps
/// session-size blow-up to 4 × 64 KiB = 256 KiB of reflection per
/// session, well below the 1 MB round-trip test ceiling. Values above
/// this get clamped down with a warning at parse time.
pub const MAX_MENTAL_MODEL_BYTE_LIMIT: usize = 65_536;

impl Default for MentalModelConfig {
    fn default() -> Self {
        Self {
            byte_limit: DEFAULT_MENTAL_MODEL_BYTE_LIMIT,
        }
    }
}

#[derive(Debug, Clone, Default, Deserialize, Serialize, PartialEq, Eq)]
#[serde(default)]
pub struct AppConfig {
    pub theme: Option<String>,
    pub theme_dark: Option<String>,
    pub theme_light: Option<String>,
    pub appearance: Option<String>,
    pub comment_types: Option<Vec<CommentTypeConfig>>,
    pub show_file_list: Option<bool>,
    /// File list pane width as a percentage of the main content area (10..=60).
    pub file_list_width: Option<u16>,
    pub diff_view: Option<String>,
    pub wrap: Option<bool>,
    pub export_legend: Option<bool>,
    /// Map custom hostnames to forge types. Example: { "gitlab.acme.com" = "gitlab" }
    pub forge_hosts: Option<std::collections::HashMap<String, String>>,
    /// Automatically `git add` files when marked as reviewed (local mode only)
    pub auto_stage: Option<bool>,
    /// Ask for confirmation before quitting with unsaved changes (default true)
    pub confirm_on_quit: Option<bool>,
    /// Show command palette popup when pressing : (default true)
    pub command_palette: Option<bool>,
    /// Highlight changed words within modified diff lines (default true).
    /// When false, added/deleted lines are painted as a single solid color.
    pub word_diff: Option<bool>,
    /// Render PR descriptions and comment bodies as styled markdown (default true).
    /// When false, bodies are rendered as plain text.
    pub markdown_rendering: Option<bool>,
    /// Risk-scoring configuration (`[risk]` section). Missing fields inside
    /// the section fall back to the per-field defaults on
    /// [`crate::risk::RiskConfig`].
    pub risk: crate::risk::RiskConfig,
    /// Auto-collapse configuration (`[auto_collapse]` section). Controls
    /// which files start collapsed when a review is opened — lockfiles and
    /// other "boring" paths plus anything over the line threshold.
    pub auto_collapse: crate::auto_collapse::AutoCollapseConfig,
    /// Reusable comment-body templates (`[comment_templates]` section).
    /// Keys are short names shown in the picker (`nit`, `q`, …); values
    /// are the text pasted into the comment buffer when a template is
    /// selected via `Ctrl+T` in comment mode.
    pub comment_templates: Option<std::collections::HashMap<String, String>>,
    /// Session rotation policy (`[session_gc]` section). Controls the
    /// age / size / count bounds enforced on the persisted-session
    /// directory at startup and from `trv --session-gc`.
    pub session_gc: SessionGcConfig,
    /// Mental-model capture configuration (`[mental_model]` section).
    /// Controls the per-field character limit for the Sparring Review
    /// pre-diff reflection panel (Phase I2).
    pub mental_model: MentalModelConfig,
}

/// Known top-level config keys. Used to warn about typos.
const KNOWN_KEYS: &[&str] = &[
    "theme",
    "theme_dark",
    "theme_light",
    "appearance",
    "comment_types",
    "show_file_list",
    "file_list_width",
    "diff_view",
    "wrap",
    "export_legend",
    "forge_hosts",
    "auto_stage",
    "confirm_on_quit",
    "command_palette",
    "word_diff",
    "markdown_rendering",
    "risk",
    "auto_collapse",
    "comment_templates",
    "session_gc",
    "mental_model",
];

/// Keys accepted under `[mental_model]`. Typos produce a precise warning
/// rather than silently reverting to the default byte limit.
/// The v1.5.0 `char_limit` alias was removed in v1.6.0 — users still
/// running v1.5.x configs see an "unknown config key" warning and
/// fall back to the default `byte_limit = 2048`.
const KNOWN_MENTAL_MODEL_KEYS: &[&str] = &["byte_limit"];

/// Known keys inside the `[auto_collapse]` section. Used to warn about typos.
const KNOWN_AUTO_COLLAPSE_KEYS: &[&str] = &["enabled", "line_threshold", "patterns"];

/// Keys accepted under `[session_gc]`. Typos produce a precise warning
/// rather than silently reverting to the default cap.
const KNOWN_SESSION_GC_KEYS: &[&str] = &["max_age_days", "max_size_mb", "max_count"];

/// Keys accepted under `[risk]`. Anything else produces a warning so a
/// typo like `default_cod = 3` doesn't silently drop to the default.
const KNOWN_RISK_KEYS: &[&str] = &[
    "default_code",
    "default_config",
    "default_documentation",
    "default_formatting",
    "extensions",
    "rules",
];

#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct ConfigLoadOutcome {
    pub config: Option<AppConfig>,
    pub warnings: Vec<String>,
}

pub fn config_path() -> Result<PathBuf> {
    let xdg_config_home = std::env::var_os("XDG_CONFIG_HOME").map(PathBuf::from);
    let home = std::env::var_os("HOME").map(PathBuf::from);
    let appdata = std::env::var_os("APPDATA").map(PathBuf::from);

    config_path_from_parts(xdg_config_home, home, appdata)
}

fn config_path_from_parts(
    xdg_config_home: Option<PathBuf>,
    home: Option<PathBuf>,
    _appdata: Option<PathBuf>,
) -> Result<PathBuf> {
    #[cfg(windows)]
    {
        let base = _appdata
            .filter(|p| !p.as_os_str().is_empty())
            .ok_or_else(|| anyhow!("Could not determine APPDATA for config directory"))?;
        return Ok(base.join("travelagent").join("config.toml"));
    }

    #[cfg(not(windows))]
    {
        if let Some(base) = xdg_config_home.filter(|p| !p.as_os_str().is_empty()) {
            return Ok(base.join("travelagent").join("config.toml"));
        }

        let home = home
            .filter(|p| !p.as_os_str().is_empty())
            .ok_or_else(|| anyhow!("Could not determine HOME for config directory"))?;
        Ok(home.join(".config").join("travelagent").join("config.toml"))
    }
}

pub fn load_config() -> Result<ConfigLoadOutcome> {
    let path = config_path()?;
    load_config_from_path(&path)
}

/// Per-repo config path: `<repo_root>/.travelagent/config.toml`. Kept as a
/// thin helper so call sites stay declarative and tests don't have to
/// redo the path construction.
#[must_use]
pub fn repo_config_path(repo_root: &Path) -> PathBuf {
    repo_root.join(".travelagent").join("config.toml")
}

/// Load a per-repo override config. Returns [`RepoConfigLoadOutcome`],
/// which carries the parsed config, its warnings (already prefixed with
/// `[repo]` so aggregation with global warnings is unambiguous), and
/// the set of top-level section names that were present in the raw TOML.
/// The presence set drives wholesale-replacement semantics for
/// struct-valued sections like `[risk]` / `[auto_collapse]` in
/// [`merge_overrides`].
///
/// A missing `<repo>/.travelagent/config.toml` is a normal case and
/// returns a default outcome with no warnings.
pub fn load_repo_config(repo_root: &Path) -> Result<RepoConfigLoadOutcome> {
    let path = repo_config_path(repo_root);
    load_repo_config_from_path(&path)
}

/// Outcome of loading a per-repo config, carrying enough state for the
/// merge step to distinguish "field absent" from "field set to default".
#[derive(Debug, Clone, Default)]
pub struct RepoConfigLoadOutcome {
    pub config: Option<AppConfig>,
    pub warnings: Vec<String>,
    /// Which top-level section / key names were actually present in
    /// the raw TOML. Used by [`merge_overrides`] to tell "user wrote
    /// `[risk]` with one rule → replace global wholesale" apart from
    /// "user didn't touch risk → keep global".
    pub sections_present: std::collections::HashSet<String>,
    pub path: PathBuf,
}

fn load_repo_config_from_path(path: &Path) -> Result<RepoConfigLoadOutcome> {
    let contents = match fs::read_to_string(path) {
        Ok(contents) => contents,
        Err(err) if err.kind() == ErrorKind::NotFound => {
            return Ok(RepoConfigLoadOutcome {
                path: path.to_path_buf(),
                ..Default::default()
            });
        }
        Err(err) => return Err(err.into()),
    };

    let value: Value = toml::from_str(&contents)?;
    let table = value
        .as_table()
        .ok_or_else(|| anyhow!("Repo config root must be a TOML table"))?;

    // Record which top-level keys were present in the raw TOML before
    // parsing — `try_into`-backed sections like `[risk]` default their
    // fields, so once parsed we can't tell "field absent" from "field
    // explicitly set to default". The merge step consults this set.
    let sections_present: std::collections::HashSet<String> =
        table.keys().map(String::clone).collect();

    // Reuse the same parser as global config so warnings, validation,
    // and field-by-field semantics match exactly. We feed the
    // already-read `contents` into `parse_config_toml_str` instead of
    // calling `load_config_from_path(path)` here — the latter would
    // re-read the file from disk, wasting a syscall and opening a tiny
    // TOCTOU window where `sections_present` (computed above) could
    // diverge from what the second read actually parses. Post-process
    // the warnings with a `[repo]` prefix so downstream aggregators can
    // tell at a glance which file a given warning originated from.
    let mut inner = parse_config_toml_str(&contents)?;
    let warnings = inner
        .warnings
        .drain(..)
        .map(|w| format!("[repo] {w}"))
        .collect();

    Ok(RepoConfigLoadOutcome {
        config: inner.config,
        warnings,
        sections_present,
        path: path.to_path_buf(),
    })
}

/// Merge a per-repo `AppConfig` on top of a global one. Repo values win
/// for most fields; theme-family fields are global-only and produce a
/// warning if set per-repo. Map-valued sections (`forge_hosts`,
/// `comment_templates`) are unioned with repo winning on collision.
/// Struct-valued sections (`risk`, `auto_collapse`) are replaced
/// wholesale when the repo TOML has the section present.
///
/// `repo_sections_present` is the set of top-level TOML keys the repo
/// file actually contained (as recorded by [`load_repo_config`]). This
/// is the only way to distinguish "user wrote `[risk]` with one rule,
/// wants global's rules gone" from "user didn't touch risk, wants
/// global kept" — a parsed `RiskConfig` can't answer that.
#[must_use]
pub fn merge_overrides(
    mut global: AppConfig,
    repo: AppConfig,
    repo_sections_present: &std::collections::HashSet<String>,
    warnings: &mut Vec<String>,
) -> AppConfig {
    // Theme family is global-only. Any per-repo attempt gets rejected
    // with a warning — silently changing a user's color scheme based on
    // cwd would be a nasty surprise.
    for key in ["theme", "theme_dark", "theme_light", "appearance"] {
        if repo_sections_present.contains(key) {
            warnings.push(format!(
                "[repo] Warning: Config key '{key}' is not supported in per-repo config; ignored (theme is global-only)"
            ));
        }
    }

    // Simple Option<T>: repo wins if Some; keep global otherwise.
    if repo.show_file_list.is_some() {
        global.show_file_list = repo.show_file_list;
    }
    if repo.file_list_width.is_some() {
        global.file_list_width = repo.file_list_width;
    }
    if repo.diff_view.is_some() {
        global.diff_view = repo.diff_view;
    }
    if repo.wrap.is_some() {
        global.wrap = repo.wrap;
    }
    if repo.export_legend.is_some() {
        global.export_legend = repo.export_legend;
    }
    if repo.auto_stage.is_some() {
        global.auto_stage = repo.auto_stage;
    }
    if repo.confirm_on_quit.is_some() {
        global.confirm_on_quit = repo.confirm_on_quit;
    }
    if repo.command_palette.is_some() {
        global.command_palette = repo.command_palette;
    }
    if repo.word_diff.is_some() {
        global.word_diff = repo.word_diff;
    }
    if repo.markdown_rendering.is_some() {
        global.markdown_rendering = repo.markdown_rendering;
    }
    if repo.comment_types.is_some() {
        global.comment_types = repo.comment_types;
    }

    // Map-valued sections: union, repo wins on collision. This lets a
    // repo add a `[comment_templates]` entry without wiping globals.
    if let Some(repo_templates) = repo.comment_templates {
        let mut merged = global.comment_templates.unwrap_or_default();
        merged.extend(repo_templates);
        global.comment_templates = Some(merged);
    }
    if let Some(repo_hosts) = repo.forge_hosts {
        let mut merged = global.forge_hosts.unwrap_or_default();
        merged.extend(repo_hosts);
        global.forge_hosts = Some(merged);
    }

    // Struct-valued sections: wholesale replacement when present. Fields
    // inside `RiskConfig` / `AutoCollapseConfig` have meaningful defaults
    // so the parsed struct can't signal "user didn't set this field";
    // `repo_sections_present` is the presence oracle.
    if repo_sections_present.contains("risk") {
        global.risk = repo.risk;
    }
    if repo_sections_present.contains("auto_collapse") {
        global.auto_collapse = repo.auto_collapse;
    }
    if repo_sections_present.contains("session_gc") {
        global.session_gc = repo.session_gc;
    }
    if repo_sections_present.contains("mental_model") {
        global.mental_model = repo.mental_model;
    }

    global
}

/// Read a string value from the table, pushing a warning if the type is wrong.
fn read_string(table: &toml::Table, key: &str, warnings: &mut Vec<String>) -> Option<String> {
    let val = table.get(key)?;
    if let Some(s) = val.as_str() {
        Some(s.to_string())
    } else {
        warnings.push(format!(
            "Warning: Config key '{key}' must be a string; ignoring value"
        ));
        None
    }
}

/// Read a boolean value from the table, pushing a warning if the type is wrong.
fn read_bool(table: &toml::Table, key: &str, warnings: &mut Vec<String>) -> Option<bool> {
    let val = table.get(key)?;
    if let Some(b) = val.as_bool() {
        Some(b)
    } else {
        warnings.push(format!(
            "Warning: Config key '{key}' must be a boolean; ignoring value"
        ));
        None
    }
}

/// Read a u16 value constrained to an inclusive range. Out-of-range values
/// are clamped with a warning so a mistyped config doesn't silently drop.
fn read_u16_in_range(
    table: &toml::Table,
    key: &str,
    min: u16,
    max: u16,
    warnings: &mut Vec<String>,
) -> Option<u16> {
    let val = table.get(key)?;
    let Some(n) = val.as_integer() else {
        warnings.push(format!(
            "Warning: Config key '{key}' must be an integer; ignoring value"
        ));
        return None;
    };
    if n < i64::from(min) || n > i64::from(max) {
        warnings.push(format!(
            "Warning: Config key '{key}' must be between {min} and {max}; got {n}, clamping"
        ));
        return Some((n.clamp(i64::from(min), i64::from(max))) as u16);
    }
    Some(n as u16)
}

/// Read a string value constrained to a set of allowed values.
fn read_enum(
    table: &toml::Table,
    key: &str,
    allowed: &[&str],
    warnings: &mut Vec<String>,
) -> Option<String> {
    let raw = read_string(table, key, warnings)?;
    if allowed.contains(&raw.as_str()) {
        Some(raw)
    } else {
        let choices = allowed
            .iter()
            .map(|s| format!("\"{s}\""))
            .collect::<Vec<_>>()
            .join(" or ");
        warnings.push(format!(
            "Warning: Config key '{key}' must be {choices}; got \"{raw}\", ignoring"
        ));
        None
    }
}

fn load_config_from_path(path: &Path) -> Result<ConfigLoadOutcome> {
    let contents = match fs::read_to_string(path) {
        Ok(contents) => contents,
        Err(err) if err.kind() == ErrorKind::NotFound => return Ok(ConfigLoadOutcome::default()),
        Err(err) => return Err(err.into()),
    };

    parse_config_toml_str(&contents)
}

/// Parse a TOML config body into a [`ConfigLoadOutcome`] without touching
/// the filesystem. Exists so property/fuzz tests can drive the parser
/// with arbitrary inputs.
///
/// Invariant under test: the parser either returns `Err` (malformed TOML
/// / non-table root) or `Ok(ConfigLoadOutcome)` with zero panics for any
/// input. Unknown top-level keys surface as warnings, not errors.
pub fn parse_config_toml_str(contents: &str) -> Result<ConfigLoadOutcome> {
    let value: Value = toml::from_str(contents)?;
    let table = value
        .as_table()
        .ok_or_else(|| anyhow!("Config root must be a TOML table"))?;

    let mut warnings = Vec::new();

    let config = AppConfig {
        theme: read_string(table, "theme", &mut warnings),
        theme_dark: read_string(table, "theme_dark", &mut warnings),
        theme_light: read_string(table, "theme_light", &mut warnings),
        appearance: read_string(table, "appearance", &mut warnings),
        comment_types: table
            .get("comment_types")
            .and_then(|v| parse_comment_types(v, &mut warnings)),
        show_file_list: read_bool(table, "show_file_list", &mut warnings),
        file_list_width: read_u16_in_range(table, "file_list_width", 10, 60, &mut warnings),
        diff_view: read_enum(
            table,
            "diff_view",
            &["unified", "side-by-side"],
            &mut warnings,
        ),
        wrap: read_bool(table, "wrap", &mut warnings),
        export_legend: read_bool(table, "export_legend", &mut warnings),
        forge_hosts: table
            .get("forge_hosts")
            .and_then(|v| parse_forge_hosts(v, &mut warnings)),
        auto_stage: read_bool(table, "auto_stage", &mut warnings),
        confirm_on_quit: read_bool(table, "confirm_on_quit", &mut warnings),
        command_palette: read_bool(table, "command_palette", &mut warnings),
        word_diff: read_bool(table, "word_diff", &mut warnings),
        markdown_rendering: read_bool(table, "markdown_rendering", &mut warnings),
        risk: parse_risk_section(table, &mut warnings),
        auto_collapse: parse_auto_collapse_section(table, &mut warnings),
        comment_templates: table
            .get("comment_templates")
            .and_then(|v| parse_comment_templates(v, &mut warnings)),
        session_gc: parse_session_gc_section(table, &mut warnings),
        mental_model: parse_mental_model_section(table, &mut warnings),
    };

    for key in table.keys() {
        if !KNOWN_KEYS.contains(&key.as_str()) {
            warnings.push(format!("Warning: Unknown config key '{key}', ignoring"));
        }
    }

    Ok(ConfigLoadOutcome {
        config: Some(config),
        warnings,
    })
}

fn parse_comment_types(
    value: &Value,
    warnings: &mut Vec<String>,
) -> Option<Vec<CommentTypeConfig>> {
    let Some(items) = value.as_array() else {
        warnings.push(
            "Warning: Config key 'comment_types' must be an array of objects; ignoring value"
                .to_string(),
        );
        return None;
    };

    let mut parsed = Vec::new();
    let mut seen_ids = std::collections::HashSet::new();

    for (index, item) in items.iter().enumerate() {
        let Some(entry) = item.as_table() else {
            warnings.push(format!(
                "Warning: Config key 'comment_types[{index}]' must be an object; ignoring entry"
            ));
            continue;
        };

        for key in entry.keys() {
            if key != "id" && key != "label" && key != "definition" && key != "color" {
                warnings.push(format!(
                    "Warning: Unknown key 'comment_types[{index}].{key}', ignoring"
                ));
            }
        }

        let Some(id_raw) = entry.get("id").and_then(Value::as_str) else {
            warnings.push(format!(
                "Warning: Config key 'comment_types[{index}].id' must be a string; ignoring entry"
            ));
            continue;
        };

        let id = id_raw.trim().to_ascii_lowercase();
        if id.is_empty() {
            warnings.push(format!(
                "Warning: Config key 'comment_types[{index}].id' cannot be empty; ignoring entry"
            ));
            continue;
        }

        if seen_ids.contains(&id) {
            warnings.push(format!(
                "Warning: Duplicate comment type id '{id}' in config; ignoring duplicate entry"
            ));
            continue;
        }

        let label = parse_optional_nonempty_string(entry, "label", index, warnings);
        let definition = parse_optional_nonempty_string(entry, "definition", index, warnings);

        let color = match entry.get("color") {
            None => None,
            Some(raw) => {
                if let Some(text) = raw.as_str() {
                    let trimmed = text.trim();
                    if trimmed.is_empty() {
                        warnings.push(format!(
                        "Warning: Config key 'comment_types[{index}].color' cannot be empty; ignoring value"
                    ));
                        None
                    } else if !is_supported_color_value(trimmed) {
                        warnings.push(format!(
                        "Warning: Config key 'comment_types[{index}].color' must be a named color or #RRGGBB; ignoring value"
                    ));
                        None
                    } else {
                        Some(trimmed.to_string())
                    }
                } else {
                    warnings.push(format!(
                    "Warning: Config key 'comment_types[{index}].color' must be a string; ignoring value"
                ));
                    None
                }
            }
        };

        seen_ids.insert(id.clone());
        parsed.push(CommentTypeConfig {
            id,
            label,
            definition,
            color,
        });
    }

    if parsed.is_empty() {
        warnings.push(
            "Warning: Config key 'comment_types' contains no valid entries; using defaults"
                .to_string(),
        );
        None
    } else {
        Some(parsed)
    }
}

/// Parse an optional non-empty string field from a comment_types entry.
fn parse_optional_nonempty_string(
    entry: &toml::Table,
    field: &str,
    index: usize,
    warnings: &mut Vec<String>,
) -> Option<String> {
    let raw = entry.get(field)?;
    if let Some(text) = raw.as_str() {
        let trimmed = text.trim();
        if trimmed.is_empty() {
            warnings.push(format!(
                "Warning: Config key 'comment_types[{index}].{field}' cannot be empty; ignoring value"
            ));
            None
        } else {
            Some(trimmed.to_string())
        }
    } else {
        warnings.push(format!(
            "Warning: Config key 'comment_types[{index}].{field}' must be a string; ignoring value"
        ));
        None
    }
}

fn is_supported_color_value(value: &str) -> bool {
    let normalized = value.trim().to_ascii_lowercase();
    if normalized.is_empty() {
        return false;
    }

    if let Some(hex) = normalized.strip_prefix('#') {
        return hex.len() == 6 && hex.chars().all(|ch| ch.is_ascii_hexdigit());
    }

    matches!(
        normalized.as_str(),
        "black"
            | "red"
            | "green"
            | "yellow"
            | "blue"
            | "magenta"
            | "cyan"
            | "gray"
            | "grey"
            | "darkgray"
            | "dark_gray"
            | "darkgrey"
            | "dark_grey"
            | "lightred"
            | "light_red"
            | "lightgreen"
            | "light_green"
            | "lightyellow"
            | "light_yellow"
            | "lightblue"
            | "light_blue"
            | "lightmagenta"
            | "light_magenta"
            | "lightcyan"
            | "light_cyan"
            | "white"
    )
}

/// Parse the `[risk]` section into a [`crate::risk::RiskConfig`]. Missing
/// section or any parse failure falls back to `RiskConfig::default()` with a
/// warning so a mistyped rule doesn't silently drop the whole section.
///
/// Unknown top-level keys inside `[risk]` are reported individually before
/// `try_into` drops them, so `default_cod = 3` (typo of `default_code`)
/// produces a precise warning instead of the generic TOML parse-failure
/// message. Matches the pattern used by `parse_auto_collapse_section` /
/// `parse_comment_types` — grok-4-20-thinking flagged the inconsistency
/// in the 2026-05-02 crew review.
fn parse_risk_section(table: &toml::Table, warnings: &mut Vec<String>) -> crate::risk::RiskConfig {
    let Some(value) = table.get("risk") else {
        return crate::risk::RiskConfig::default();
    };

    // Warn on unknown keys before try_into silently drops them.
    if let Some(section) = value.as_table() {
        for key in section.keys() {
            if !KNOWN_RISK_KEYS.contains(&key.as_str()) {
                warnings.push(format!(
                    "Warning: Unknown config key 'risk.{key}', ignoring"
                ));
            }
        }
    }

    match value.clone().try_into::<crate::risk::RiskConfig>() {
        Ok(cfg) => cfg,
        Err(e) => {
            // `toml::de::Error`'s Display already includes the field path
            // and value when the error originates from a specific key, so
            // surfacing it verbatim is the closest we get to "precise per
            // field" without reimplementing the derive.
            warnings.push(format!(
                "Warning: Config section '[risk]' could not be parsed ({e}); using defaults"
            ));
            crate::risk::RiskConfig::default()
        }
    }
}

/// Parse the `[auto_collapse]` section into an
/// [`crate::auto_collapse::AutoCollapseConfig`]. Unknown keys inside the
/// section produce a warning (same pattern as top-level KNOWN_KEYS). A
/// non-positive `line_threshold` is also warned about and clamped to the
/// default.
fn parse_auto_collapse_section(
    table: &toml::Table,
    warnings: &mut Vec<String>,
) -> crate::auto_collapse::AutoCollapseConfig {
    let Some(value) = table.get("auto_collapse") else {
        return crate::auto_collapse::AutoCollapseConfig::default();
    };

    // Warn on unknown keys inside the section before try_into drops them.
    if let Some(section) = value.as_table() {
        for key in section.keys() {
            if !KNOWN_AUTO_COLLAPSE_KEYS.contains(&key.as_str()) {
                warnings.push(format!(
                    "Warning: Unknown config key 'auto_collapse.{key}', ignoring"
                ));
            }
        }
    }

    let mut cfg = match value
        .clone()
        .try_into::<crate::auto_collapse::AutoCollapseConfig>()
    {
        Ok(cfg) => cfg,
        Err(e) => {
            warnings.push(format!(
                "Warning: Config section '[auto_collapse]' could not be parsed ({e}); using defaults"
            ));
            return crate::auto_collapse::AutoCollapseConfig::default();
        }
    };

    // Nonsense thresholds: below 1 would collapse everything on any edit. Warn
    // and fall back to the default rather than silently breaking.
    if cfg.line_threshold < 1 {
        warnings.push(format!(
            "Warning: Config key 'auto_collapse.line_threshold' must be >= 1; got {}, using default",
            cfg.line_threshold
        ));
        cfg.line_threshold = crate::auto_collapse::DEFAULT_LINE_THRESHOLD;
    }

    cfg
}

/// Parse the `[session_gc]` section into a [`SessionGcConfig`]. Missing
/// section falls back to `SessionGcConfig::default()` (7-day age, no size /
/// count cap). Unknown keys produce per-key warnings; a parse failure falls
/// back to defaults with a single section-level warning.
fn parse_session_gc_section(table: &toml::Table, warnings: &mut Vec<String>) -> SessionGcConfig {
    let Some(value) = table.get("session_gc") else {
        return SessionGcConfig::default();
    };

    if let Some(section) = value.as_table() {
        for key in section.keys() {
            if !KNOWN_SESSION_GC_KEYS.contains(&key.as_str()) {
                warnings.push(format!(
                    "Warning: Unknown config key 'session_gc.{key}', ignoring"
                ));
            }
        }
    }

    match value.clone().try_into::<SessionGcConfig>() {
        Ok(cfg) => cfg,
        Err(e) => {
            warnings.push(format!(
                "Warning: Config section '[session_gc]' could not be parsed ({e}); using defaults"
            ));
            SessionGcConfig::default()
        }
    }
}

fn parse_mental_model_section(
    table: &toml::Table,
    warnings: &mut Vec<String>,
) -> MentalModelConfig {
    let Some(value) = table.get("mental_model") else {
        return MentalModelConfig::default();
    };

    // Typo-check known keys. The v1.5.x `char_limit` alias was
    // removed in v1.6.0; configs still using it fall through the
    // unknown-key warning below and land on the default byte_limit.
    if let Some(section) = value.as_table() {
        for key in section.keys() {
            if !KNOWN_MENTAL_MODEL_KEYS.contains(&key.as_str()) {
                warnings.push(format!(
                    "Warning: Unknown config key 'mental_model.{key}', ignoring"
                ));
            }
        }
    }
    let migrated_value = value.clone();

    match migrated_value.try_into::<MentalModelConfig>() {
        Ok(mut cfg) => {
            // Clamp pathological values to avoid unbounded per-field
            // buffers. Matches the validation pattern used by
            // `auto_collapse.line_threshold`.
            if cfg.byte_limit == 0 {
                warnings.push(
                    "Warning: Config key 'mental_model.byte_limit' must be >= 1; got 0, using default".to_string(),
                );
                cfg.byte_limit = DEFAULT_MENTAL_MODEL_BYTE_LIMIT;
            } else if cfg.byte_limit > MAX_MENTAL_MODEL_BYTE_LIMIT {
                warnings.push(format!(
                    "Warning: Config key 'mental_model.byte_limit' exceeds maximum ({MAX_MENTAL_MODEL_BYTE_LIMIT}); got {}, clamping",
                    cfg.byte_limit
                ));
                cfg.byte_limit = MAX_MENTAL_MODEL_BYTE_LIMIT;
            }
            cfg
        }
        Err(e) => {
            warnings.push(format!(
                "Warning: Config section '[mental_model]' could not be parsed ({e}); using defaults"
            ));
            MentalModelConfig::default()
        }
    }
}

/// Parse the `[comment_templates]` section into a `HashMap<String, String>`.
/// Each key is a short template name; each value is the expansion text.
/// Non-string values produce a warning and are dropped; an empty or missing
/// section returns `None`.
fn parse_comment_templates(
    value: &Value,
    warnings: &mut Vec<String>,
) -> Option<std::collections::HashMap<String, String>> {
    let Some(table) = value.as_table() else {
        warnings.push(
            "Warning: Config key 'comment_templates' must be a table of { name = \"expansion\" }; ignoring value"
                .to_string(),
        );
        return None;
    };

    let mut map = std::collections::HashMap::new();
    for (name, val) in table {
        if let Some(text) = val.as_str() {
            map.insert(name.clone(), text.to_string());
        } else {
            warnings.push(format!(
                "Warning: Config key 'comment_templates.{name}' must be a string; ignoring value"
            ));
        }
    }

    if map.is_empty() { None } else { Some(map) }
}

fn parse_forge_hosts(
    value: &Value,
    warnings: &mut Vec<String>,
) -> Option<std::collections::HashMap<String, String>> {
    let Some(table) = value.as_table() else {
        warnings.push(
            "Warning: Config key 'forge_hosts' must be a table of { host = \"github\" | \"gitlab\" }; ignoring value"
                .to_string(),
        );
        return None;
    };

    let mut map = std::collections::HashMap::new();
    for (host, val) in table {
        if let Some(forge_type) = val.as_str() {
            let normalized = forge_type.trim().to_ascii_lowercase();
            if normalized == "github" || normalized == "gitlab" {
                map.insert(host.clone(), normalized);
            } else {
                warnings.push(format!(
                    "Warning: Config key 'forge_hosts.{host}' must be \"github\" or \"gitlab\"; got \"{forge_type}\", ignoring"
                ));
            }
        } else {
            warnings.push(format!(
                "Warning: Config key 'forge_hosts.{host}' must be a string; ignoring value"
            ));
        }
    }

    if map.is_empty() { None } else { Some(map) }
}

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

    /// Helper: write a config file, parse it, and return the outcome.
    fn parse_config(toml_content: &str) -> ConfigLoadOutcome {
        let dir = tempdir().expect("failed to create temp dir");
        let path = dir.path().join("config.toml");
        fs::write(&path, toml_content).expect("failed to write config");
        load_config_from_path(&path).expect("config should parse")
    }

    #[test]
    fn should_return_none_when_config_file_missing() {
        let dir = tempdir().expect("failed to create temp dir");
        let path = dir.path().join("config.toml");
        let outcome = load_config_from_path(&path).expect("missing config should not fail");
        assert_eq!(outcome.config, None);
        assert!(outcome.warnings.is_empty());
    }

    #[test]
    fn should_load_theme_from_valid_toml() {
        let outcome = parse_config("theme = \"light\"\n");
        assert_eq!(
            outcome.config.as_ref().and_then(|cfg| cfg.theme.as_deref()),
            Some("light")
        );
        assert!(outcome.warnings.is_empty());
    }

    #[test]
    fn should_load_theme_variants_and_appearance_from_valid_toml() {
        let outcome = parse_config(
            "theme_dark = \"gruvbox-dark\"\ntheme_light = \"gruvbox-light\"\nappearance = \"system\"\n",
        );
        let cfg = outcome.config.as_ref().unwrap();
        assert_eq!(cfg.theme_dark.as_deref(), Some("gruvbox-dark"));
        assert_eq!(cfg.theme_light.as_deref(), Some("gruvbox-light"));
        assert_eq!(cfg.appearance.as_deref(), Some("system"));
        assert!(outcome.warnings.is_empty());
    }

    #[test]
    fn should_parse_empty_config_as_defaults() {
        let outcome = parse_config("");
        assert_eq!(outcome.config, Some(AppConfig::default()));
        assert!(outcome.warnings.is_empty());
    }

    #[test]
    fn should_error_on_invalid_toml() {
        let dir = tempdir().expect("failed to create temp dir");
        let path = dir.path().join("config.toml");
        fs::write(&path, "theme =\n").expect("failed to write config");
        let result = load_config_from_path(&path);
        assert!(result.is_err(), "invalid TOML should return error");
    }

    #[test]
    fn should_warn_on_unknown_keys_and_keep_known_values() {
        let outcome = parse_config("theme = \"light\"\nthemes = \"typo\"\n");
        assert_eq!(
            outcome.config.as_ref().and_then(|cfg| cfg.theme.as_deref()),
            Some("light")
        );
        assert_eq!(outcome.warnings.len(), 1);
        assert_eq!(
            outcome.warnings[0],
            "Warning: Unknown config key 'themes', ignoring"
        );
    }

    #[test]
    fn should_warn_on_unknown_keys_only_and_use_defaults() {
        let outcome = parse_config("themes = \"typo\"\n");
        assert_eq!(outcome.config, Some(AppConfig::default()));
        assert_eq!(outcome.warnings.len(), 1);
        assert_eq!(
            outcome.warnings[0],
            "Warning: Unknown config key 'themes', ignoring"
        );
    }

    #[test]
    fn should_warn_and_ignore_theme_with_invalid_type() {
        let outcome = parse_config("theme = 123\n");
        assert_eq!(outcome.config, Some(AppConfig::default()));
        assert_eq!(outcome.warnings.len(), 1);
        assert_eq!(
            outcome.warnings[0],
            "Warning: Config key 'theme' must be a string; ignoring value"
        );
    }

    #[test]
    fn should_warn_and_ignore_theme_dark_with_invalid_type() {
        let outcome = parse_config("theme_dark = 123\n");
        assert_eq!(outcome.config, Some(AppConfig::default()));
        assert_eq!(outcome.warnings.len(), 1);
        assert_eq!(
            outcome.warnings[0],
            "Warning: Config key 'theme_dark' must be a string; ignoring value"
        );
    }

    // show_file_list

    #[test]
    fn should_parse_show_file_list_false() {
        let outcome = parse_config("show_file_list = false\n");
        assert_eq!(
            outcome.config.as_ref().and_then(|cfg| cfg.show_file_list),
            Some(false)
        );
        assert!(outcome.warnings.is_empty());
    }

    #[test]
    fn should_warn_and_ignore_show_file_list_with_invalid_type() {
        let outcome = parse_config("show_file_list = \"no\"\n");
        assert_eq!(
            outcome.config.as_ref().and_then(|cfg| cfg.show_file_list),
            None
        );
        assert_eq!(outcome.warnings.len(), 1);
    }

    // diff_view

    #[test]
    fn should_parse_diff_view_side_by_side() {
        let outcome = parse_config("diff_view = \"side-by-side\"\n");
        assert_eq!(
            outcome
                .config
                .as_ref()
                .and_then(|cfg| cfg.diff_view.as_deref()),
            Some("side-by-side")
        );
        assert!(outcome.warnings.is_empty());
    }

    #[test]
    fn should_parse_diff_view_unified() {
        let outcome = parse_config("diff_view = \"unified\"\n");
        assert_eq!(
            outcome
                .config
                .as_ref()
                .and_then(|cfg| cfg.diff_view.as_deref()),
            Some("unified")
        );
        assert!(outcome.warnings.is_empty());
    }

    #[test]
    fn should_warn_and_ignore_diff_view_with_invalid_value() {
        let outcome = parse_config("diff_view = \"split\"\n");
        assert_eq!(
            outcome
                .config
                .as_ref()
                .and_then(|cfg| cfg.diff_view.as_deref()),
            None
        );
        assert_eq!(outcome.warnings.len(), 1);
        assert!(outcome.warnings[0].contains("\"unified\" or \"side-by-side\""));
    }

    #[test]
    fn should_warn_and_ignore_diff_view_with_invalid_type() {
        let outcome = parse_config("diff_view = true\n");
        assert_eq!(
            outcome
                .config
                .as_ref()
                .and_then(|cfg| cfg.diff_view.as_deref()),
            None
        );
        assert_eq!(outcome.warnings.len(), 1);
        assert_eq!(
            outcome.warnings[0],
            "Warning: Config key 'diff_view' must be a string; ignoring value"
        );
    }

    // wrap

    #[test]
    fn should_parse_wrap_true() {
        let outcome = parse_config("wrap = true\n");
        assert_eq!(outcome.config.as_ref().and_then(|cfg| cfg.wrap), Some(true));
        assert!(outcome.warnings.is_empty());
    }

    #[test]
    fn should_parse_wrap_false() {
        let outcome = parse_config("wrap = false\n");
        assert_eq!(
            outcome.config.as_ref().and_then(|cfg| cfg.wrap),
            Some(false)
        );
        assert!(outcome.warnings.is_empty());
    }

    #[test]
    fn should_warn_and_ignore_wrap_with_invalid_type() {
        let outcome = parse_config("wrap = \"yes\"\n");
        assert_eq!(outcome.config.as_ref().and_then(|cfg| cfg.wrap), None);
        assert_eq!(outcome.warnings.len(), 1);
        assert_eq!(
            outcome.warnings[0],
            "Warning: Config key 'wrap' must be a boolean; ignoring value"
        );
    }

    // export_legend

    #[test]
    fn should_parse_export_legend_false() {
        let outcome = parse_config("export_legend = false\n");
        assert_eq!(
            outcome.config.as_ref().and_then(|cfg| cfg.export_legend),
            Some(false)
        );
        assert!(outcome.warnings.is_empty());
    }

    #[test]
    fn should_default_export_legend_to_none() {
        let outcome = parse_config("\n");
        assert_eq!(
            outcome.config.as_ref().and_then(|cfg| cfg.export_legend),
            None
        );
    }

    // comment_types

    #[test]
    fn should_parse_comment_types_from_array_of_objects() {
        let outcome = parse_config(
            r#"comment_types = [
  { id = "note", label = "question", definition = "ask for clarification", color = "yellow" },
  { id = "issue" }
]"#,
        );
        let comment_types = outcome
            .config
            .as_ref()
            .and_then(|cfg| cfg.comment_types.as_ref())
            .expect("comment types should be set");

        assert_eq!(comment_types.len(), 2);
        assert_eq!(comment_types[0].id, "note");
        assert_eq!(comment_types[0].label.as_deref(), Some("question"));
        assert_eq!(
            comment_types[0].definition.as_deref(),
            Some("ask for clarification")
        );
        assert_eq!(comment_types[0].color.as_deref(), Some("yellow"));
        assert_eq!(comment_types[1].id, "issue");
        assert!(outcome.warnings.is_empty());
    }

    #[test]
    fn should_warn_and_ignore_invalid_comment_type_entries() {
        let outcome = parse_config(
            r#"comment_types = [
  { id = "" },
  { id = "note" },
  { id = "NOTE" },
  42
]"#,
        );
        let comment_types = outcome
            .config
            .as_ref()
            .and_then(|cfg| cfg.comment_types.as_ref())
            .expect("comment types should be set");

        assert_eq!(comment_types.len(), 1);
        assert_eq!(comment_types[0].id, "note");
        assert_eq!(outcome.warnings.len(), 3);
    }

    #[test]
    fn should_warn_and_ignore_invalid_comment_type_color() {
        let outcome = parse_config(
            r#"comment_types = [
  { id = "note", color = "not-a-color" }
]"#,
        );
        let comment_types = outcome
            .config
            .as_ref()
            .and_then(|cfg| cfg.comment_types.as_ref())
            .expect("comment types should be set");

        assert_eq!(comment_types.len(), 1);
        assert_eq!(comment_types[0].id, "note");
        assert_eq!(comment_types[0].color, None);
        assert_eq!(outcome.warnings.len(), 1);
    }

    // [risk] section

    #[test]
    fn risk_section_parses_from_toml() {
        let outcome = parse_config(
            r"
[risk]
default_code = 5
",
        );
        let cfg = outcome
            .config
            .as_ref()
            .expect("config should parse with [risk] section");
        assert_eq!(
            cfg.risk.default_code, 5,
            "risk.default_code override should stick"
        );
        // All other risk defaults should still match the RiskConfig defaults.
        assert_eq!(cfg.risk.default_config, 2);
        assert_eq!(cfg.risk.default_documentation, 1);
        assert_eq!(cfg.risk.default_formatting, 0);
        assert!(
            cfg.risk.extensions.config.iter().any(|g| g == "*.toml"),
            "default config extensions should still be present"
        );
        assert!(outcome.warnings.is_empty(), "{:?}", outcome.warnings);
    }

    #[test]
    fn risk_section_defaults_when_missing() {
        let outcome = parse_config("theme = \"light\"\n");
        let cfg = outcome.config.as_ref().expect("config should parse");
        let defaults = crate::risk::RiskConfig::default();
        assert_eq!(cfg.risk, defaults, "missing [risk] should fall back");
    }

    #[test]
    fn risk_section_warns_and_falls_back_on_invalid_value() {
        // A non-u8 value for a u8 field should not kill the whole config; it
        // should fall back to RiskConfig::default() with a warning.
        let outcome = parse_config(
            r#"
[risk]
default_code = "high"
"#,
        );
        let cfg = outcome.config.as_ref().expect("config should still parse");
        assert_eq!(cfg.risk, crate::risk::RiskConfig::default());
        assert_eq!(outcome.warnings.len(), 1);
        assert!(outcome.warnings[0].contains("[risk]"));
    }

    #[test]
    fn risk_section_warns_on_unknown_key_inside_section() {
        // Regression (post-crew review 2026-05-02): typos like `default_cod`
        // should produce a precise per-key warning, not just a generic
        // `[risk]` parse-failure message. Matches auto_collapse's pattern.
        let outcome = parse_config(
            r#"
[risk]
default_code = 3
banana = 42
"#,
        );
        let cfg = outcome.config.as_ref().expect("config should still parse");
        // Known field retained.
        assert_eq!(cfg.risk.default_code, 3);
        // Unknown key produces exactly one targeted warning.
        assert_eq!(outcome.warnings.len(), 1, "{:?}", outcome.warnings);
        assert!(
            outcome.warnings[0].contains("risk.banana"),
            "{:?}",
            outcome.warnings
        );
    }

    // [auto_collapse] section

    #[test]
    fn auto_collapse_section_parses_defaults_when_missing() {
        let outcome = parse_config("theme = \"light\"\n");
        let cfg = outcome.config.as_ref().expect("config should parse");
        let defaults = crate::auto_collapse::AutoCollapseConfig::default();
        assert_eq!(cfg.auto_collapse, defaults);
    }

    #[test]
    fn auto_collapse_section_parses_empty_section_as_defaults() {
        let outcome = parse_config("[auto_collapse]\n");
        let cfg = outcome.config.as_ref().expect("config should parse");
        assert_eq!(
            cfg.auto_collapse,
            crate::auto_collapse::AutoCollapseConfig::default()
        );
        assert!(outcome.warnings.is_empty(), "{:?}", outcome.warnings);
    }

    #[test]
    fn auto_collapse_section_parses_custom_threshold_and_patterns() {
        let outcome = parse_config(
            r#"
[auto_collapse]
line_threshold = 1000
patterns = ["**/*.foo"]
"#,
        );
        let cfg = outcome.config.as_ref().expect("config should parse");
        assert!(cfg.auto_collapse.enabled);
        assert_eq!(cfg.auto_collapse.line_threshold, 1000);
        assert_eq!(cfg.auto_collapse.patterns, vec!["**/*.foo".to_string()]);
        assert!(outcome.warnings.is_empty(), "{:?}", outcome.warnings);
    }

    #[test]
    fn auto_collapse_section_warns_on_unknown_key_inside_section() {
        let outcome = parse_config(
            r#"
[auto_collapse]
enabled = true
banana = 42
"#,
        );
        let cfg = outcome.config.as_ref().expect("config should still parse");
        // Known fields retain their parsed values.
        assert!(cfg.auto_collapse.enabled);
        // Exactly one warning about the unknown key.
        assert_eq!(outcome.warnings.len(), 1, "{:?}", outcome.warnings);
        assert!(
            outcome.warnings[0].contains("auto_collapse.banana"),
            "{:?}",
            outcome.warnings
        );
    }

    #[test]
    fn auto_collapse_section_warns_and_clamps_zero_line_threshold() {
        let outcome = parse_config(
            r#"
[auto_collapse]
line_threshold = 0
"#,
        );
        let cfg = outcome.config.as_ref().expect("config should parse");
        // Falls back to the default rather than keeping 0.
        assert_eq!(
            cfg.auto_collapse.line_threshold,
            crate::auto_collapse::DEFAULT_LINE_THRESHOLD
        );
        assert_eq!(outcome.warnings.len(), 1, "{:?}", outcome.warnings);
        assert!(
            outcome.warnings[0].contains("line_threshold"),
            "{:?}",
            outcome.warnings
        );
    }

    // [comment_templates] section

    #[test]
    fn comment_templates_parse_from_config_toml() {
        let outcome = parse_config(
            r#"
[comment_templates]
nit = "nit: "
q = "Question: "
style = "Nit (style): "
"#,
        );
        let cfg = outcome.config.as_ref().expect("config should parse");
        let templates = cfg
            .comment_templates
            .as_ref()
            .expect("comment_templates should be set");
        assert_eq!(templates.len(), 3);
        assert_eq!(templates.get("nit").map(String::as_str), Some("nit: "));
        assert_eq!(templates.get("q").map(String::as_str), Some("Question: "));
        assert_eq!(
            templates.get("style").map(String::as_str),
            Some("Nit (style): ")
        );
        assert!(outcome.warnings.is_empty(), "{:?}", outcome.warnings);
    }

    #[test]
    fn comment_templates_missing_section_is_empty_map() {
        // Absent [comment_templates] yields None (treated as empty at callsite),
        // not an error, and no warnings.
        let outcome = parse_config("theme = \"light\"\n");
        let cfg = outcome.config.as_ref().expect("config should parse");
        assert!(cfg.comment_templates.is_none());
        assert!(outcome.warnings.is_empty(), "{:?}", outcome.warnings);
    }

    #[test]
    fn comment_templates_warn_on_non_string_value() {
        let outcome = parse_config(
            r#"
[comment_templates]
good = "nit: "
bad = 42
"#,
        );
        let cfg = outcome.config.as_ref().expect("config should parse");
        let templates = cfg
            .comment_templates
            .as_ref()
            .expect("comment_templates should be set");
        assert_eq!(templates.len(), 1);
        assert_eq!(templates.get("good").map(String::as_str), Some("nit: "));
        assert_eq!(outcome.warnings.len(), 1);
        assert!(
            outcome.warnings[0].contains("comment_templates.bad"),
            "{:?}",
            outcome.warnings
        );
    }

    // config path resolution

    #[cfg(not(windows))]
    #[test]
    fn should_use_xdg_config_home_when_set() {
        let path = config_path_from_parts(
            Some(PathBuf::from("/tmp/xdg-config")),
            Some(PathBuf::from("/tmp/home")),
            None,
        )
        .expect("config path should resolve");

        assert_eq!(
            path,
            PathBuf::from("/tmp/xdg-config/travelagent/config.toml")
        );
    }

    #[cfg(not(windows))]
    #[test]
    fn should_fallback_to_home_dot_config_when_xdg_unset() {
        let path = config_path_from_parts(None, Some(PathBuf::from("/home/tester")), None)
            .expect("config path should resolve");

        assert_eq!(
            path,
            PathBuf::from("/home/tester/.config/travelagent/config.toml")
        );
    }

    #[cfg(not(windows))]
    #[test]
    fn should_ignore_empty_xdg_config_home() {
        let path = config_path_from_parts(
            Some(PathBuf::from("")),
            Some(PathBuf::from("/home/tester")),
            None,
        )
        .expect("config path should resolve");

        assert_eq!(
            path,
            PathBuf::from("/home/tester/.config/travelagent/config.toml")
        );
    }

    #[cfg(not(windows))]
    #[test]
    fn should_append_travelagent_config_toml_suffix() {
        let path = config_path_from_parts(
            Some(PathBuf::from("/tmp/xdg-config")),
            Some(PathBuf::from("/tmp/home")),
            None,
        )
        .expect("config path should resolve");

        assert!(path.ends_with(Path::new("travelagent").join("config.toml")));
    }

    #[cfg(windows)]
    #[test]
    fn should_use_windows_appdata_base_dir() {
        let path = config_path_from_parts(
            Some(PathBuf::from(r"C:\xdg\ignored")),
            Some(PathBuf::from(r"C:\Users\tester")),
            Some(PathBuf::from(r"C:\Users\tester\AppData\Roaming")),
        )
        .expect("config path should resolve");

        assert_eq!(
            path,
            PathBuf::from(r"C:\Users\tester\AppData\Roaming\travelagent\config.toml")
        );
    }

    // ---- Phase C: per-repo config overrides ----

    /// Helper: materialise a `<repo>/.travelagent/config.toml` under
    /// `tempdir()` and return the temp dir + its `load_repo_config` result.
    fn load_repo_with(toml_content: &str) -> (tempfile::TempDir, RepoConfigLoadOutcome) {
        let dir = tempdir().expect("temp dir");
        let cfg_dir = dir.path().join(".travelagent");
        fs::create_dir_all(&cfg_dir).expect("mkdir .travelagent");
        fs::write(cfg_dir.join("config.toml"), toml_content).expect("write repo config");
        let outcome = load_repo_config(dir.path()).expect("repo config loads");
        (dir, outcome)
    }

    #[test]
    fn per_repo_file_missing_returns_default_outcome() {
        let dir = tempdir().expect("temp dir");
        let outcome = load_repo_config(dir.path()).expect("should not fail on missing");
        assert!(outcome.config.is_none());
        assert!(outcome.warnings.is_empty());
        assert!(outcome.sections_present.is_empty());
        assert_eq!(outcome.path, repo_config_path(dir.path()));
    }

    #[test]
    fn per_repo_overrides_global_simple_fields() {
        // Global sets word_diff = true; repo flips it to false.
        let global = AppConfig {
            word_diff: Some(true),
            markdown_rendering: Some(true),
            ..AppConfig::default()
        };
        let (_dir, repo_outcome) = load_repo_with("word_diff = false\n");
        let repo_cfg = repo_outcome.config.expect("repo parses");
        let mut warnings = Vec::new();
        let merged = merge_overrides(
            global.clone(),
            repo_cfg,
            &repo_outcome.sections_present,
            &mut warnings,
        );
        assert_eq!(merged.word_diff, Some(false), "repo wins");
        assert_eq!(
            merged.markdown_rendering,
            Some(true),
            "global kept when repo didn't set"
        );
        assert!(warnings.is_empty());
    }

    #[test]
    fn per_repo_theme_is_rejected_with_warning() {
        let global = AppConfig {
            theme: Some("dark".to_string()),
            ..AppConfig::default()
        };
        let (_dir, repo_outcome) = load_repo_with("theme = \"light\"\n");
        let repo_cfg = repo_outcome.config.expect("repo parses");
        let mut warnings = Vec::new();
        let merged = merge_overrides(
            global.clone(),
            repo_cfg,
            &repo_outcome.sections_present,
            &mut warnings,
        );
        assert_eq!(
            merged.theme.as_deref(),
            Some("dark"),
            "global theme preserved; repo theme ignored"
        );
        assert_eq!(warnings.len(), 1);
        assert!(
            warnings[0].contains("theme"),
            "warning should name the rejected key: {:?}",
            warnings
        );
        assert!(
            warnings[0].contains("[repo]"),
            "warning should be labelled as repo-origin: {:?}",
            warnings
        );
    }

    #[test]
    fn per_repo_comment_templates_union_repo_wins_on_collision() {
        let mut global_templates = std::collections::HashMap::new();
        global_templates.insert("nit".to_string(), "nit: ".to_string());
        global_templates.insert("q".to_string(), "Q: ".to_string());
        let global = AppConfig {
            comment_templates: Some(global_templates),
            ..AppConfig::default()
        };

        let (_dir, repo_outcome) = load_repo_with(
            r#"
[comment_templates]
nit = "nit (repo): "
style = "S: "
"#,
        );
        let repo_cfg = repo_outcome.config.expect("repo parses");
        let mut warnings = Vec::new();
        let merged = merge_overrides(
            global,
            repo_cfg,
            &repo_outcome.sections_present,
            &mut warnings,
        );
        let templates = merged.comment_templates.expect("merged map present");
        assert_eq!(templates.len(), 3, "union of global + repo keys");
        assert_eq!(
            templates.get("nit").map(String::as_str),
            Some("nit (repo): "),
            "repo wins on collision"
        );
        assert_eq!(
            templates.get("q").map(String::as_str),
            Some("Q: "),
            "global-only key kept"
        );
        assert_eq!(
            templates.get("style").map(String::as_str),
            Some("S: "),
            "repo-only key added"
        );
    }

    #[test]
    fn per_repo_forge_hosts_union_repo_wins() {
        let mut global_hosts = std::collections::HashMap::new();
        global_hosts.insert("gitlab.a.com".to_string(), "gitlab".to_string());
        global_hosts.insert("ghe.a.com".to_string(), "github".to_string());
        let global = AppConfig {
            forge_hosts: Some(global_hosts),
            ..AppConfig::default()
        };

        let (_dir, repo_outcome) = load_repo_with(
            r#"
[forge_hosts]
"gitlab.a.com" = "github"
"gitlab.b.com" = "gitlab"
"#,
        );
        let repo_cfg = repo_outcome.config.expect("repo parses");
        let mut warnings = Vec::new();
        let merged = merge_overrides(
            global,
            repo_cfg,
            &repo_outcome.sections_present,
            &mut warnings,
        );
        let hosts = merged.forge_hosts.expect("merged map present");
        assert_eq!(hosts.len(), 3);
        assert_eq!(
            hosts.get("gitlab.a.com").map(String::as_str),
            Some("github"),
            "repo overrides the forge mapping"
        );
        assert_eq!(hosts.get("ghe.a.com").map(String::as_str), Some("github"));
        assert_eq!(
            hosts.get("gitlab.b.com").map(String::as_str),
            Some("gitlab")
        );
    }

    #[test]
    fn per_repo_risk_section_fully_replaces_global() {
        // Global has two rules and a non-default default_code; repo sets
        // a single-rule `[risk]` and omits default_code. Wholesale
        // replacement means the merged config has just the repo's one
        // rule, and default_code falls back to RiskConfig::default — NOT
        // to the global's 4.
        let global_risk = crate::risk::RiskConfig {
            default_code: 4,
            rules: vec![
                crate::risk::RiskRule {
                    glob: "a/*".into(),
                    level: 5,
                },
                crate::risk::RiskRule {
                    glob: "b/*".into(),
                    level: 1,
                },
            ],
            ..crate::risk::RiskConfig::default()
        };
        let global = AppConfig {
            risk: global_risk,
            ..AppConfig::default()
        };

        let (_dir, repo_outcome) = load_repo_with(
            r#"
[risk]
rules = [
  { glob = "secrets/*", level = 5 },
]
"#,
        );
        let repo_cfg = repo_outcome.config.expect("repo parses");
        let mut warnings = Vec::new();
        let merged = merge_overrides(
            global,
            repo_cfg,
            &repo_outcome.sections_present,
            &mut warnings,
        );
        assert_eq!(
            merged.risk.rules.len(),
            1,
            "repo rules replace global rules wholesale"
        );
        assert_eq!(merged.risk.rules[0].glob, "secrets/*");
        assert_eq!(
            merged.risk.default_code,
            crate::risk::RiskConfig::default().default_code,
            "fields not set in repo fall back to RiskConfig::default(), not global's value"
        );
    }

    #[test]
    fn per_repo_session_gc_section_fully_replaces_global() {
        // Global sets a non-default session_gc; repo sets `[session_gc]` with
        // only max_count. Per the wholesale-replace rule, merged.session_gc
        // should be the repo value — NOT a field-by-field merge — so
        // max_age_days falls back to SessionGcConfig::default().
        let global = AppConfig {
            session_gc: SessionGcConfig {
                max_age_days: 30,
                max_size_mb: 500,
                max_count: 100,
            },
            ..AppConfig::default()
        };

        let (_dir, repo_outcome) = load_repo_with(
            r"
[session_gc]
max_count = 5
",
        );
        let repo_cfg = repo_outcome.config.expect("repo parses");
        let mut warnings = Vec::new();
        let merged = merge_overrides(
            global,
            repo_cfg,
            &repo_outcome.sections_present,
            &mut warnings,
        );
        assert_eq!(merged.session_gc.max_count, 5, "repo's max_count wins");
        assert_eq!(
            merged.session_gc.max_age_days,
            SessionGcConfig::default().max_age_days,
            "fields not set in repo fall back to SessionGcConfig::default(), not global's value"
        );
        assert_eq!(
            merged.session_gc.max_size_mb,
            SessionGcConfig::default().max_size_mb
        );
    }

    #[test]
    fn session_gc_section_warns_on_unknown_key_inside_section() {
        let outcome = parse_config(
            r#"
[session_gc]
max_age_days = 14
banana = 42
"#,
        );
        let cfg = outcome.config.as_ref().expect("config should still parse");
        assert_eq!(cfg.session_gc.max_age_days, 14);
        assert_eq!(outcome.warnings.len(), 1, "{:?}", outcome.warnings);
        assert!(
            outcome.warnings[0].contains("session_gc.banana"),
            "{:?}",
            outcome.warnings
        );
    }

    #[test]
    fn session_gc_section_defaults_when_missing() {
        let outcome = parse_config("theme = \"light\"\n");
        let cfg = outcome.config.as_ref().expect("config should parse");
        assert_eq!(cfg.session_gc, SessionGcConfig::default());
    }

    #[test]
    fn per_repo_warnings_carry_repo_prefix() {
        // Typos inside the repo file should produce warnings that start
        // with `[repo]` so aggregated warning output disambiguates
        // global vs. repo origins.
        let (_dir, repo_outcome) = load_repo_with(
            r#"
themes = "typo"

[auto_collapse]
banana = 42
"#,
        );
        assert!(
            repo_outcome
                .warnings
                .iter()
                .all(|w| w.starts_with("[repo]")),
            "every repo warning should carry the [repo] prefix: {:?}",
            repo_outcome.warnings
        );
        assert!(
            repo_outcome.warnings.iter().any(|w| w.contains("themes")),
            "warning for top-level typo should mention the key"
        );
        assert!(
            repo_outcome
                .warnings
                .iter()
                .any(|w| w.contains("auto_collapse.banana")),
            "warning for section-internal typo should qualify with the section"
        );
    }

    // ── Phase I2: [mental_model] section ──

    #[test]
    fn mental_model_section_defaults_when_missing() {
        let outcome = parse_config("theme = \"light\"\n");
        let cfg = outcome.config.as_ref().expect("config should parse");
        assert_eq!(cfg.mental_model, MentalModelConfig::default());
        assert_eq!(cfg.mental_model.byte_limit, DEFAULT_MENTAL_MODEL_BYTE_LIMIT);
    }

    #[test]
    fn mental_model_section_honors_explicit_byte_limit() {
        let outcome = parse_config(
            r#"
[mental_model]
byte_limit = 4096
"#,
        );
        let cfg = outcome.config.as_ref().expect("config should parse");
        assert_eq!(cfg.mental_model.byte_limit, 4096);
        assert!(
            outcome.warnings.is_empty(),
            "valid config must produce no warnings: {:?}",
            outcome.warnings
        );
    }

    #[test]
    fn mental_model_legacy_char_limit_key_is_unknown_in_v16() {
        // The v1.5.0 `char_limit` alias was removed in v1.6.0. A config
        // still using it surfaces an unknown-key warning and falls back
        // to the default byte_limit — same behavior as any other typo.
        let outcome = parse_config(
            r#"
[mental_model]
char_limit = 4096
"#,
        );
        let cfg = outcome.config.as_ref().expect("config should parse");
        assert_eq!(
            cfg.mental_model.byte_limit, DEFAULT_MENTAL_MODEL_BYTE_LIMIT,
            "legacy key must not silently become byte_limit in v1.6+"
        );
        assert!(
            outcome
                .warnings
                .iter()
                .any(|w| w.contains("char_limit") && w.contains("Unknown config key")),
            "legacy key must surface as an unknown-key warning: {:?}",
            outcome.warnings
        );
    }

    #[test]
    fn mental_model_section_warns_on_unknown_key() {
        let outcome = parse_config(
            r#"
[mental_model]
byte_limit = 2048
banana = 42
"#,
        );
        let cfg = outcome.config.as_ref().expect("config should still parse");
        assert_eq!(cfg.mental_model.byte_limit, 2048);
        assert_eq!(outcome.warnings.len(), 1, "{:?}", outcome.warnings);
        assert!(
            outcome.warnings[0].contains("mental_model.banana"),
            "{:?}",
            outcome.warnings
        );
    }

    #[test]
    fn mental_model_section_warns_and_defaults_on_type_error() {
        let outcome = parse_config(
            r#"
[mental_model]
byte_limit = "big"
"#,
        );
        let cfg = outcome.config.as_ref().expect("config should still parse");
        assert_eq!(
            cfg.mental_model,
            MentalModelConfig::default(),
            "type-error fallback restores defaults"
        );
        assert!(
            outcome
                .warnings
                .iter()
                .any(|w| w.contains("[mental_model]")),
            "warning must name the section: {:?}",
            outcome.warnings
        );
    }

    #[test]
    fn mental_model_section_replaced_wholesale_by_per_repo_override() {
        let mut global = AppConfig::default();
        global.mental_model.byte_limit = 2048;

        let mut repo = AppConfig::default();
        repo.mental_model.byte_limit = 8192;

        let mut repo_sections_present = std::collections::HashSet::new();
        repo_sections_present.insert("mental_model".to_string());

        let mut warnings = Vec::new();
        let merged = merge_overrides(global, repo, &repo_sections_present, &mut warnings);
        assert_eq!(
            merged.mental_model.byte_limit, 8192,
            "per-repo override must replace the global value wholesale"
        );
    }

    #[test]
    fn mental_model_section_untouched_when_absent_from_repo() {
        let mut global = AppConfig::default();
        global.mental_model.byte_limit = 2048;

        let repo = AppConfig::default();
        // `mental_model` NOT in the presence set → global value survives.
        let repo_sections_present = std::collections::HashSet::new();

        let mut warnings = Vec::new();
        let merged = merge_overrides(global, repo, &repo_sections_present, &mut warnings);
        assert_eq!(
            merged.mental_model.byte_limit, 2048,
            "absent repo section must leave the global value alone"
        );
    }

    #[test]
    fn mental_model_byte_limit_zero_clamps_to_default_with_warning() {
        let outcome = parse_config(
            r#"
[mental_model]
byte_limit = 0
"#,
        );
        let cfg = outcome.config.as_ref().expect("config should still parse");
        assert_eq!(cfg.mental_model.byte_limit, DEFAULT_MENTAL_MODEL_BYTE_LIMIT);
        assert!(
            outcome
                .warnings
                .iter()
                .any(|w| w.contains("mental_model.byte_limit") && w.contains(">= 1")),
            "warning expected for zero: {:?}",
            outcome.warnings
        );
    }

    #[test]
    fn mental_model_byte_limit_exceeding_max_clamps_with_warning() {
        let outcome = parse_config(
            r#"
[mental_model]
byte_limit = 9999999
"#,
        );
        let cfg = outcome.config.as_ref().expect("config should still parse");
        assert_eq!(cfg.mental_model.byte_limit, MAX_MENTAL_MODEL_BYTE_LIMIT);
        assert!(
            outcome
                .warnings
                .iter()
                .any(|w| w.contains("mental_model.byte_limit") && w.contains("maximum")),
            "warning expected for exceeding max: {:?}",
            outcome.warnings
        );
    }
}