supercode-harness 0.4.8

//! §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
}

/// P4 (design §5.2 "P4", §1.8: "env substitution in values"): expand
/// `${VAR}` references in `s` against the process environment. Applied at
/// [`HarnessConfig::to_config_profile`] to the string-valued `[core]`
/// fields that plausibly vary per deployment — `base_url`, `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` are deliberately EXCLUDED — the
/// former is already an env var NAME not a value, the latter is a shell
/// command the shell itself expands when it runs, see the call site's
/// comment).
///
/// An unset variable is left LITERAL (`${VAR}` stays in the output) rather
/// than silently substituted with an empty string — a config author sees
/// immediately that something didn't resolve instead of silently getting a
/// blank `base_url`/header/etc. Only the braced `${NAME}` form is
/// recognized — no bare `$NAME`, no shell-style `:-default` operators —
/// the smallest form that satisfies the obligation without inventing a
/// shell-expansion dialect.
pub fn expand_env_vars(s: &str) -> String {
    let mut out = String::with_capacity(s.len());
    let mut rest = s;
    while let Some(start) = rest.find("${") {
        out.push_str(&rest[..start]);
        let after = &rest[start + 2..];
        match after.find('}') {
            Some(end) => {
                let var_name = &after[..end];
                match std::env::var(var_name) {
                    Ok(v) => out.push_str(&v),
                    // Unset (or an invalid var name, e.g. one containing
                    // `=`): keep the placeholder literal rather than
                    // silently blanking it.
                    Err(_) => out.push_str(&rest[start..start + 2 + end + 1]),
                }
                rest = &after[end + 1..];
            }
            None => {
                // Unterminated `${` — emit the rest literally and stop.
                out.push_str(&rest[start..]);
                rest = "";
                break;
            }
        }
    }
    out.push_str(rest);
    out
}

/// 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 {
    /// 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_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>,
    /// `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>,
    /// 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>,
    /// 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>,
    /// `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>,
}

/// `[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>,
}

/// `[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>>,
}

/// `[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>,
}

/// `[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(),
            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,
            // §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,
            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(),
            project_doc_max_bytes: c.project_doc_max_bytes,
            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,
            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,
            // 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,
            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(),
        }
    }

    /// 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_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),
        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_tool_output_bytes: merge_opt!(base, over, max_tool_output_bytes),
        parallel_tool_calls: merge_opt!(base, over, parallel_tool_calls),
        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),
        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),
        },
        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),
            },
            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),
        },
        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),
        },
        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.
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`).
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`).
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.
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 {
    if d.contains("${") {
        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());
    }
    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());
    }
    // 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());
        }
    }

    // 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(),
            );
        }
    }

    // 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(),
            );
        }
    }

    // ---- 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>,
}

/// 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] = &[
    "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",
    "effort",
    "temperature",
    "max_tokens",
    "max_iterations",
    "max_total_output_tokens",
    "max_tool_output_bytes",
    "parallel_tool_calls",
    "shell_env_snapshot",
    "system_prompt",
    "append_system_prompt",
    "project_context",
    "env_context",
    "context_injections",
    "nested_instructions",
    "instruction_imports",
    "project_root_markers",
    "hot_reload",
    "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);
        }
    }
    let mut config = ConfigBuilder::default().apply_profile(&profile).build();

    // P3 (design §5.2): the resolved module-activation set + the risk-2
    // `[experimental] module_registry` gate, both carried on `Config` itself
    // so `ToolRegistry::from_config` (and prompt assembly) can consult them
    // without re-walking `HarnessConfig` — "pure config → set, testable
    // without the loop" (§5.3 risk 2).
    config.module_registry = experimental_flag(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);

    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);
        // 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.
    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();
        }
    }

    // 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);
    }

    // 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`.
    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;

    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);
        out.insert(
            name.clone(),
            crate::subagents::NamedAgentDefinition {
                name: name.clone(),
                system_prompt,
                tools,
                model,
            },
        );
    }
    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"),
    })
}

/// 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)
}

/// `[experimental].<key>` as a bool (§3.1 obligation 8: "feature flags,
/// staged gates not yet promoted to `[core]`"). Absent or non-bool → `false`
/// — an experimental gate defaults OFF, never silently on.
fn experimental_flag(hc: &HarnessConfig, key: &str) -> bool {
    hc.experimental
        .get(key)
        .and_then(|v| v.as_bool())
        .unwrap_or(false)
}

/// §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>,
}

/// 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),
    /// 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,
    },
    /// 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::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::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 {}

/// §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.
pub fn resolve(
    top_toml: &str,
    project_toml: Option<&str>,
    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, project_toml, 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, None, opts, Vec::new())
}

/// Shared tail of [`resolve`]/[`resolve_harness`]: steps 1-7 over an
/// already-parsed top layer.
fn resolve_top(
    top: HarnessConfig,
    project_toml: Option<&str>,
    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.
    let final_hc = match project_toml {
        Some(proj_text) => {
            let proj_unknown = unknown_keys(proj_text).map_err(ResolveError::Parse)?;
            if opts.strict {
                if let Some(first) = proj_unknown.first() {
                    return Err(ResolveError::UnknownKey(first.clone()));
                }
            } else {
                for k in &proj_unknown {
                    warnings.push(format!(
                        "unknown key `{k}` in project config (lenient mode, §3.5 step 5)"
                    ));
                }
            }
            let proj = HarnessConfig::from_toml_str(proj_text).map_err(ResolveError::Parse)?;
            let (sanitized, dropped) = sanitize_for_project(&proj);
            for d in &dropped {
                warnings.push(format!(
                    "project config: dropped untrusted key `{d}` (§3.3 monotonic tightening)"
                ));
            }
            let mut merged = user_layer.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(
                user_layer.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(
                user_layer.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(&user_layer, &sanitized, &mut merged);
            for c in &clamped {
                warnings.push(format!(
                    "project config: clamped `{c}` to the stricter base-layer value \
                     (§3.3 monotonic tightening)"
                ));
            }
            merged
        }
        None => user_layer,
    };

    // `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)"
                ));
            }
        }
    }

    // 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);
    }
}