repotoire 0.8.0

//! Phase 1d: feature-flag scaffolding for the dual-branch findings system.
//!
//! This is the fourth and final infrastructure step (Phase 1d in the
//! architecture note at
//! `docs/superpowers/specs/2026-05-09-dual-branch-phase1-architecture.md`).
//!
//! # What this does
//!
//! Adds a `[dual_branch]` section to `repotoire.toml` with:
//!
//! - `enabled` — master switch (default `false`); when `false`, no
//!   detector emits dual-branch shape regardless of per-detector opt-in.
//! - `detectors` — map from detector ID (kebab-case, normalized via
//!   `normalize_detector_name`) to bool. A detector emits dual-branch
//!   shape only when **both** `enabled = true` *and*
//!   `detectors[its_id] = true`.
//!
//! # What this does NOT do
//!
//! - **Phase 1d itself is a no-op at runtime.** No detector reads this
//!   config in Phase 1. The flag exists so Phase 2 detector migrations
//!   can plumb it in without schema churn. Until detectors migrate,
//!   they emit single-branch findings as today regardless of this
//!   config.
//! - **Does not gate the Phase 1c `enrich_graph_evidence` pass.** That
//!   pass runs unconditionally because its outputs (`prediction_reasons`
//!   on every finding) are useful evidence regardless of whether any
//!   detector has migrated to dual-branch. If a real user reports
//!   JSON size as a problem before Phase 2 lands, that's the time to
//!   add a separate `enrich_graph_evidence_when` flag — not now.
//!
//! # Deviation from the architecture note's spec
//!
//! The architecture note specified a hardcoded list of 5 detector
//! bool fields:
//!
//! ```toml
//! [dual_branch.detectors]
//! insecure_crypto = false
//! path_traversal = false
//! insecure_tls = false
//! command_injection = false
//! xxe = false
//! ```
//!
//! That shape is replaced here with a `HashMap<String, bool>`. Reasons:
//!
//! 1. **The hardcoded list pre-commits to which detectors will
//!    migrate.** If Phase 2 ends up starting with just one detector
//!    (to validate the approach in production before broad rollout),
//!    or if priorities shift, the spec'd list is wrong from day 1 and
//!    requires both a code change and a config schema change to fix.
//! 2. **A map is extensible to plugins.** A user with a custom
//!    detector can opt it in without modifying repotoire's source.
//! 3. **Adding/removing a detector becomes a one-line TOML change**,
//!    not a schema migration.
//! 4. **Matches the existing `per_file_ignores` convention.** That
//!    field is also `HashMap<String, Vec<String>>` keyed by string
//!    detector IDs, normalized via `normalize_detector_name`. Using
//!    the same convention here means users learn one pattern.
//!
//! # API contract
//!
//! Phase 2 detectors must use **only** `is_enabled_for(detector_id)`,
//! not the raw fields. The method handles the two-level check
//! (master switch AND per-detector opt-in) and detector ID
//! normalization in one call. Raw-field access is technically
//! possible (the fields are `pub` because TOML deserialization
//! requires it) but using it directly is a bug — the master switch
//! is easy to forget.
//!
//! # Conflict resolution
//!
//! If a user writes the same detector under multiple key spellings
//! (e.g. both `insecure-crypto = false` and
//! `InsecureCryptoDetector = true`), `is_enabled_for` resolves the
//! conflict deterministically: **`true` wins**. Rationale: the
//! semantics are opt-in, and we should honor an explicit opt-in
//! anywhere in the map rather than letting the result depend on
//! `HashMap` iteration order. Pinned by
//! `conflicting_keys_resolve_to_enabled_for_opt_in_semantics`.
//!
//! # Example
//!
//! ```toml
//! [dual_branch]
//! enabled = true
//!
//! [dual_branch.detectors]
//! insecure-crypto = true       # opt this one detector in
//! "InsecureCryptoDetector" = true  # equivalent: normalized to insecure-crypto
//! path-traversal = false       # explicitly opt out (same as omitting)
//! ```

use crate::config::project_config::normalize_detector_name;
use serde::{Deserialize, Serialize};
use std::collections::HashMap;

/// Configuration for the dual-branch findings system.
///
/// Loaded from the `[dual_branch]` section of `repotoire.toml`.
/// All fields are optional and default to "feature off".
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
pub struct DualBranchConfig {
    /// Master switch. When `false` (default), `is_enabled_for` returns
    /// `false` for every detector regardless of per-detector opt-in.
    /// Acts as a single-toggle kill switch for emergency rollback.
    #[serde(default)]
    pub enabled: bool,

    /// Per-detector opt-in. Keys are detector IDs in any of the
    /// supported forms (kebab-case `insecure-crypto`, snake_case
    /// `insecure_crypto`, PascalCase `InsecureCryptoDetector`); they
    /// are normalized via `normalize_detector_name` at lookup time.
    ///
    /// Absent keys are treated as `false`. Explicit `false` is
    /// equivalent to omitting the key (kept here only so users can
    /// document their opt-out decisions in TOML).
    #[serde(default)]
    pub detectors: HashMap<String, bool>,
}

impl DualBranchConfig {
    /// The single API Phase 2 detectors should use to check whether
    /// they may emit dual-branch shape.
    ///
    /// Returns `true` only when **both**:
    /// 1. `enabled` is `true` (master switch on), and
    /// 2. `detectors[normalize(detector_id)]` is `Some(true)`.
    ///
    /// `detector_id` may be in any form accepted by
    /// `normalize_detector_name` (kebab-case, snake_case,
    /// PascalCase with optional `Detector` suffix). The map keys
    /// are normalized at lookup time so users can write whichever
    /// form is most readable in their TOML.
    pub fn is_enabled_for(&self, detector_id: &str) -> bool {
        if !self.enabled {
            return false;
        }
        let normalized = normalize_detector_name(detector_id);

        // CONFLICT POLICY: opt-in wins. If any key in the map
        // normalizes to the requested detector ID and is `true`,
        // return `true`. Only return `false` if every matching
        // key (or no keys at all) is `false`.
        //
        // Why scan instead of direct `get(&normalized)`: a direct
        // lookup creates a silent inconsistency — it would honor
        // the normalized-form key's value, but the fallback path
        // (for unnormalized keys) would honor opt-in-wins. Two
        // policies on the same conceptual operation is exactly
        // the silent-failure pattern we avoid.
        //
        // Cost: O(n) where n is the number of opted-in detectors
        // (~5-20). Lookups happen at detector-init time, not
        // per-finding, so the cost is negligible.
        self.detectors
            .iter()
            .filter(|(k, _)| normalize_detector_name(k) == normalized)
            .any(|(_, &v)| v)
    }
}

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

    fn make_config(enabled: bool, pairs: &[(&str, bool)]) -> DualBranchConfig {
        DualBranchConfig {
            enabled,
            detectors: pairs.iter().map(|(k, v)| (k.to_string(), *v)).collect(),
        }
    }

    #[test]
    fn default_config_disables_everything() {
        // The empty config (which is what every existing user has,
        // because the field is new and additive) must report every
        // detector as disabled. This is the "Phase 1 ships zero
        // user-visible behavior change" guarantee from the RFC.
        let config = DualBranchConfig::default();
        assert!(!config.is_enabled_for("insecure-crypto"));
        assert!(!config.is_enabled_for("any-other-detector"));
        assert!(!config.is_enabled_for(""));
    }

    #[test]
    fn master_switch_off_disables_even_opted_in_detectors() {
        // Pinned policy: `enabled = false` overrides everything.
        // Acts as the documented kill switch. If this test fails,
        // a future user toggling the master switch off will get a
        // surprise.
        let config = make_config(false, &[("insecure-crypto", true)]);
        assert!(
            !config.is_enabled_for("insecure-crypto"),
            "master switch off must override per-detector opt-in",
        );
    }

    #[test]
    fn detector_off_with_master_on_is_disabled() {
        // The other half of the AND: master switch on alone is not
        // enough. The detector ID must also appear with `true`.
        let config = make_config(true, &[("path-traversal", false)]);
        assert!(!config.is_enabled_for("path-traversal"));
        // Unmentioned detector defaults to false.
        assert!(!config.is_enabled_for("not-in-map"));
    }

    #[test]
    fn both_master_and_detector_on_enables() {
        // Happy path: master on + detector key present with true.
        let config = make_config(true, &[("insecure-crypto", true)]);
        assert!(config.is_enabled_for("insecure-crypto"));
    }

    #[test]
    fn lookup_normalizes_detector_id() {
        // The lookup uses `normalize_detector_name` so callers can
        // pass the detector class name and the TOML can use the
        // kebab-case form (or vice versa). Verifies all four forms
        // from `normalize_detector_name`'s docstring round-trip.
        let config = make_config(true, &[("insecure-crypto", true)]);
        for form in [
            "insecure-crypto",
            "insecure_crypto",
            "InsecureCrypto",
            "InsecureCryptoDetector",
        ] {
            assert!(
                config.is_enabled_for(form),
                "detector ID form {:?} should match normalized key 'insecure-crypto'",
                form,
            );
        }
    }

    #[test]
    fn lookup_normalizes_toml_keys_too() {
        // The TOML key may be written in any form; the lookup must
        // match it after normalization. This means a user who writes
        // `InsecureCryptoDetector = true` in their TOML gets the
        // same behavior as `insecure-crypto = true`.
        let config = make_config(true, &[("InsecureCryptoDetector", true)]);
        assert!(config.is_enabled_for("insecure-crypto"));
        assert!(config.is_enabled_for("InsecureCrypto"));
    }

    #[test]
    fn toml_deserialization_populates_config() {
        // Defensive: deserialize a hand-written TOML snippet and
        // verify behavior is what the docs promise. Catches any
        // future serde attribute mistake (e.g. accidentally
        // `#[serde(skip)]` on the detectors map) that would silently
        // produce an empty config. We use `basic_toml` because that's
        // the deserializer the rest of the project uses (see
        // `load_project_config`), so this is the actual user-visible
        // path.
        let toml_text = r#"
            enabled = true
            [detectors]
            insecure-crypto = true
            path-traversal = false
        "#;
        let parsed: DualBranchConfig =
            basic_toml::from_str(toml_text).expect("deserialize dual_branch config");

        assert!(parsed.enabled);
        assert!(parsed.is_enabled_for("insecure-crypto"));
        assert!(!parsed.is_enabled_for("path-traversal"));
        assert!(!parsed.is_enabled_for("not-mentioned"));
    }

    #[test]
    fn missing_dual_branch_section_in_toml_is_disabled() {
        // The whole point of "additive" is that an existing user with
        // no `[dual_branch]` section in their TOML gets a fully
        // disabled config — no surprises, no behavior change.
        let toml_text = "";
        let project: crate::config::project_config::ProjectConfig =
            basic_toml::from_str(toml_text).expect("deserialize");
        assert!(!project.dual_branch.enabled);
        assert!(!project.dual_branch.is_enabled_for("anything"));
    }

    #[test]
    fn explicit_false_in_map_is_treated_as_disabled() {
        // An explicit `false` in the TOML is semantically identical
        // to the key being absent. We document this in module docs;
        // this test pins the behavior.
        let config = make_config(true, &[("insecure-crypto", false)]);
        assert!(!config.is_enabled_for("insecure-crypto"));
    }

    #[test]
    fn conflicting_keys_resolve_to_enabled_for_opt_in_semantics() {
        // POLICY: if the same detector appears under multiple key
        // spellings with conflicting values, opt-in wins. This is
        // uniform regardless of whether the normalized form, the
        // unnormalized form, or both are present.
        //
        // Without this rule, the result would depend on `HashMap`
        // iteration order (non-deterministic) or on which key the
        // implementation happens to look up first (silently
        // inconsistent across spellings).
        //
        // DO NOT change these assertions to expect `false` if a
        // future refactor breaks them — the policy is documented in
        // module docs and the right fix is to restore opt-in-wins,
        // not flip the test.

        // Case 1: normalized form `false`, unnormalized form `true`.
        // Opt-in wins.
        let config1 = make_config(
            true,
            &[("insecure-crypto", false), ("InsecureCryptoDetector", true)],
        );
        assert!(
            config1.is_enabled_for("insecure-crypto"),
            "opt-in must win even when normalized-form key is false",
        );

        // Case 2: only unnormalized forms, conflicting. Opt-in wins.
        let config2 = make_config(
            true,
            &[("InsecureCrypto", false), ("InsecureCryptoDetector", true)],
        );
        assert!(
            config2.is_enabled_for("insecure-crypto"),
            "opt-in must win across unnormalized-key conflicts \
             regardless of HashMap iteration order",
        );

        // Case 3: all matching keys `false`. Disabled.
        let config3 = make_config(
            true,
            &[
                ("insecure-crypto", false),
                ("InsecureCryptoDetector", false),
            ],
        );
        assert!(
            !config3.is_enabled_for("insecure-crypto"),
            "all-false must disable (no opt-in to honor)",
        );
    }
}