cratefield_cli/

doctor.rs

//! `fz doctor` (issue #8): harness validity, lockfile consistency and the
//! portable-SQL lint. It re-asserts the contract-version rule (issue #17)
//! and the production abuse-port rules (issues #13/#102/#133) — the same
//! checks `Harness::build` enforces at initialization, kept visible here
//! for operator tooling.
//!
//! Output comes in two disciplines (harness #140). Without `--json` the
//! doctor is prose: warnings on stderr, every failure joined into one
//! message, byte-for-byte as it has always been. With `--json` it prints
//! exactly one JSON object to stdout — `schema`, `ok` and the failures,
//! each with its stable code from [`crate::codes`] — and nothing else,
//! so an agent can parse the verdict without screen-scraping.

use crate::EnvVars;
use crate::codes::{CODES, DoctorCodeDef};
use crate::collect::{LockedMigrationError, verify_locked};
use crate::lint::banned_tokens;
use crate::lock::{Lock, read_lock};
use cratefield_core::lint_card_data;
use cratefield_core::{
    HARNESS_API, HARNESS_SIDECARS, Harness, MIN_SECRET_BYTES, Port, SIDECAR_GATEWAY_SECRET,
    VentureEnv, deployed_env, env_disagreement, harness_api_mismatch,
};
use cratefield_push_wiring::PushWiring;
use std::path::{Path, PathBuf};
use std::process::ExitCode;

/// The `schema` field of the `fz doctor --json` object (harness #140):
/// consumers branch on it, so it only ever grows.
pub const REPORT_SCHEMA: u32 = 1;

/// One failed doctor check: its stable code from the catalogue
/// ([`crate::codes`]) plus the human message, which is byte-identical to
/// the prose path's.
#[derive(Debug)]
pub struct DoctorFailure {
    /// The catalogue definition this failure is classified under.
    pub code: &'static DoctorCodeDef,
    /// The failure message, exactly as the prose path prints it.
    pub message: String,
}

/// The outcome of a doctor run: every failed check, in check order.
#[derive(Debug)]
pub struct DoctorReport {
    /// Empty when every check passed.
    pub failures: Vec<DoctorFailure>,
}

impl DoctorReport {
    /// `true` when every check passed.
    #[must_use]
    pub fn ok(&self) -> bool {
        self.failures.is_empty()
    }

    /// The exact stdout payload of `fz doctor --json`: one object, one
    /// line, no trailing newline.
    ///
    /// # Errors
    ///
    /// Only if `serde_json` cannot serialize the report — a struct of
    /// strings and numbers cannot hit that, but the Result keeps every
    /// caller total instead of panicking.
    pub fn render_json(&self) -> Result<String, serde_json::Error> {
        #[derive(serde::Serialize)]
        struct Report<'a> {
            schema: u32,
            ok: bool,
            failures: Vec<Failure<'a>>,
        }
        #[derive(serde::Serialize)]
        struct Failure<'a> {
            code: &'a str,
            message: &'a str,
        }
        serde_json::to_string(&Report {
            schema: REPORT_SCHEMA,
            ok: self.ok(),
            failures: self
                .failures
                .iter()
                .map(|failure| Failure {
                    code: failure.code.code,
                    message: &failure.message,
                })
                .collect(),
        })
    }
}

/// Which output discipline the doctor runs under.
#[derive(Clone, Copy, PartialEq, Eq)]
enum Output {
    /// Prose: operator warnings on stderr, failures joined into one
    /// message by [`doctor`].
    Human,
    /// `--json`: exactly one object on stdout, operator warnings
    /// suppressed so nothing but the JSON reaches a consumer.
    Json,
}

/// Runs every doctor check. `allow_no_captcha` downgrades the
/// production-captcha failure to a printed warning for the stated
/// reason (issue #13).
///
/// # Errors
///
/// One message listing every failed check — the same bytes `fz doctor`
/// has always produced.
pub fn doctor(
    harness: &Harness,
    migrations_dir: &Path,
    allow_no_captcha: Option<&str>,
    sidecars: Option<&str>,
) -> Result<(), String> {
    let report = run_checks(
        harness,
        migrations_dir,
        allow_no_captcha,
        sidecars,
        Output::Human,
        Configured::from_env(),
    );
    if report.ok() {
        return Ok(());
    }
    let messages: Vec<String> = report
        .failures
        .into_iter()
        .map(|failure| failure.message)
        .collect();
    Err(messages.join("\n  - "))
}

/// Runs every doctor check under `--json` discipline (harness #140):
/// prints exactly one JSON object to stdout — `schema`, `ok`, `failures`
/// with their stable codes — and nothing else, then reports the verdict
/// as the exit code.
#[must_use = "call process::exit with the returned ExitCode"]
pub fn doctor_json(
    harness: &Harness,
    migrations_dir: &Path,
    allow_no_captcha: Option<&str>,
    sidecars: Option<&str>,
) -> ExitCode {
    let report = run_checks(
        harness,
        migrations_dir,
        allow_no_captcha,
        sidecars,
        Output::Json,
        Configured::from_env(),
    );
    match report.render_json() {
        Ok(payload) => println!("{payload}"),
        Err(err) => {
            eprintln!("fz: cannot serialize the doctor report: {err}");
            return ExitCode::FAILURE;
        }
    }
    if report.ok() {
        ExitCode::SUCCESS
    } else {
        ExitCode::FAILURE
    }
}

/// The doctor's checks under `--json` discipline, without printing —
/// the exact report [`doctor_json`] serializes, for tests and future
/// consumers. Operator warnings are suppressed, as `--json` promises.
#[must_use]
pub fn doctor_report_json(
    harness: &Harness,
    migrations_dir: &Path,
    allow_no_captcha: Option<&str>,
    sidecars: Option<&str>,
) -> DoctorReport {
    run_checks(
        harness,
        migrations_dir,
        allow_no_captcha,
        sidecars,
        Output::Json,
        Configured::from_env(),
    )
}

fn run_checks(
    harness: &Harness,
    migrations_dir: &Path,
    allow_no_captcha: Option<&str>,
    sidecars: Option<&str>,
    output: Output,
    configured: Configured,
) -> DoctorReport {
    let mut failures: Vec<DoctorFailure> = Vec::new();

    contract_failures(harness, &mut failures);
    self_check_failures(harness, &mut failures);

    // Harness::build already succeeded by construction; the production-only
    // port rules (captcha, payments webhook) are checked against the runtime's
    // provided ports.
    //
    // The environment is the deployment's, not only the compiled one
    // (issue #143): the doctor runs where `ENV` is set, and a venture that
    // never called `.env()` still ships to production.
    let env = deployed_env(harness.venture().env, &EnvVars);
    if let Some(note) = env_disagreement(harness.venture().env, env)
        && output == Output::Human
    {
        eprintln!("fz: warning: {note}");
    }
    if env == VentureEnv::Production {
        production_port_checks(
            harness,
            configured,
            &EnvVars,
            allow_no_captcha,
            output,
            &mut failures,
        );
    }

    // The admin-token floor (issue #437) is deliberately *not*
    // production-only: `HarnessConfig::from_config` refuses a set-but-short
    // `ADMIN_TOKEN` in every environment (crates/core/src/config.rs), so a
    // production gate here would leave the doctor more permissive than the
    // boot it predicts. Absent stays legal here exactly as it does there —
    // removing the token is how the admin plane stays disabled.
    if let Some(message) = admin_token_failure(configured.admin_token_bytes) {
        failures.push(DoctorFailure {
            code: &CODES.admin_token_too_short,
            message,
        });
    }

    push_checks(harness, env, output, &mut failures);
    lockfile_failures(harness, migrations_dir, &mut failures);
    undeclared_migration_failures(harness, migrations_dir, &mut failures);
    sidecar_failures(harness, sidecars, output, &mut failures);
    lint_failures(harness, &mut failures);

    DoctorReport { failures }
}

/// The contract-version re-check (issue #17). `Harness::build` already
/// refuses a mismatched module, so a harness that reaches the doctor is
/// coherent; the re-check keeps the rule visible here as well, and reports
/// the module, its version and the core crate if it ever fires.
fn contract_failures(harness: &Harness, failures: &mut Vec<DoctorFailure>) {
    for module in harness.modules() {
        if module.harness_api() != HARNESS_API {
            failures.push(DoctorFailure {
                code: &CODES.harness_api_mismatch,
                message: harness_api_mismatch(module.as_ref()),
            });
        }
    }
}

/// What each module says about its own embedded data
/// ([`Module::self_check`](cratefield_core::Module::self_check)).
///
/// The doctor has no deploy config, so it cannot run a module's
/// `validate_config` — those values live on the runtime `Env` and exist
/// only per request. It *can* ask whether what the module compiled in is
/// coherent, and that question has the same answer everywhere: the
/// notifications module answers it with every translation its catalog is
/// missing for a message the venture declared (issue #190).
///
/// Unconditional, not production-only. A missing translation is a person
/// reading `booking-confirmed.title` on a lock screen in every
/// environment, and finding it on staging is the point.
fn self_check_failures(harness: &Harness, failures: &mut Vec<DoctorFailure>) {
    for module in harness.modules() {
        for message in module.self_check() {
            failures.push(DoctorFailure {
                code: &CODES.module_self_check,
                message,
            });
        }
    }
}

/// Lockfile consistency: every module migration locked, every locked
/// file present and byte-identical.
fn lockfile_failures(harness: &Harness, migrations_dir: &Path, failures: &mut Vec<DoctorFailure>) {
    let lock = match read_lock(migrations_dir) {
        Ok(lock) => lock,
        Err(err) => {
            failures.push(DoctorFailure {
                code: &CODES.lockfile_unreadable,
                message: err,
            });
            Lock::default()
        }
    };
    for module in harness.modules() {
        for migration in module.migrations().sqlite {
            let key = format!("{}/{}", module.name(), migration.id);
            match lock.get(&key) {
                Some(entry) => {
                    if let Err(err) = verify_locked(migrations_dir, &key, entry) {
                        let code = match &err {
                            LockedMigrationError::Missing { .. } => &CODES.locked_migration_missing,
                            LockedMigrationError::Edited { .. } => &CODES.locked_migration_edited,
                        };
                        failures.push(DoctorFailure {
                            code,
                            message: err.to_string(),
                        });
                    }
                }
                None => failures.push(DoctorFailure {
                    code: &CODES.migration_not_collected,
                    message: format!(
                        "migration {key} is not collected yet — run `fz migrations collect`"
                    ),
                }),
            }
        }
    }
}

/// Undeclared dependency migrations (issue #870). `fz migrations collect`
/// writes exactly what the mounted modules return from
/// `Module::migrations()` — so SQL a module *crate* ships on disk that no
/// mounted module declares is never collected, and its tables never exist
/// on D1. Flags every sqlite file the venture's local dependencies ship
/// ([`shipped_sql_files`]) whose body no module declares. The match is on
/// the body, not the id: a module may re-wrap another crate's SQL under
/// new ids (the control-plane console re-id-s six domain crates' schemas
/// under one module).
fn undeclared_migration_failures(
    harness: &Harness,
    migrations_dir: &Path,
    failures: &mut Vec<DoctorFailure>,
) {
    let declared: std::collections::BTreeSet<&str> = harness
        .modules()
        .iter()
        .flat_map(|module| module.migrations().sqlite)
        .map(|migration| migration.sql.trim())
        .collect();
    for dir in venture_dependency_dirs(migrations_dir) {
        let dep = dir.file_name().unwrap_or(dir.as_os_str()).to_string_lossy();
        for file in shipped_sql_files(&dir) {
            let message = match std::fs::read_to_string(&file) {
                // Declared — the same bytes some module hands the runner.
                Ok(body) if declared.contains(body.trim()) => continue,
                Ok(_) => format!(
                    "dependency `{dep}` ships `{}` but no module declares its SQL — it is \
                     never collected and its tables never exist on D1. Return it from the \
                     module's `Module::migrations()` (e.g. `include_str!`) so \
                     `fz migrations collect` ships it, or drop the file",
                    file.display()
                ),
                // Like the lockfile check above: an unreadable file is a
                // reported failure, never a panic — and never a clean bill.
                Err(err) => format!(
                    "dependency `{dep}` ships `{}` but it cannot be read ({err}), so it \
                     cannot be checked against any module's declared SQL",
                    file.display()
                ),
            };
            failures.push(DoctorFailure {
                code: &CODES.migration_not_declared,
                message,
            });
        }
    }
}

/// The local crate directories a venture's dependencies resolve to, for
/// the venture whose migrations dir this is — the directory the
/// migrations dir sits in, canonicalized so a `--out` reached through
/// `..` or a symlink still walks up to the real workspace root. A `path`
/// dependency resolves relative to the venture; a `workspace = true` one
/// through the nearest ancestor manifest with a `[workspace]` table and
/// its `[workspace.dependencies]` entry (by the member's dependency key,
/// as Cargo itself looks it up). Dev and build dependencies are not
/// mounted in the harness, crates.io and git dependencies ship no
/// directory to read, and a workspace dependency whose table never names
/// a path is skipped with them. Sorted and deduped, because the failures
/// reported over them are.
///
/// Empty when there is no manifest beside the migrations dir, or it does
/// not parse — cargo itself refuses to build such a venture.
#[must_use]
pub fn venture_dependency_dirs(migrations_dir: &Path) -> Vec<PathBuf> {
    // A bare `migrations` names the relative `""` parent: the working
    // directory. The lexical path is the fallback when the directory does
    // not exist to canonicalize.
    let venture = match migrations_dir.parent() {
        Some(parent) if !parent.as_os_str().is_empty() => parent.to_path_buf(),
        _ => PathBuf::from("."),
    };
    let venture = std::fs::canonicalize(&venture).unwrap_or(venture);
    let manifest = std::fs::read_to_string(venture.join("Cargo.toml"))
        .ok()
        .and_then(|text| text.parse::<toml::Table>().ok());
    let Some(manifest) = manifest else {
        return Vec::new();
    };
    // Walked once for the first `workspace = true` dependency and kept.
    let mut workspace: Option<PathBuf> = None;
    let mut dirs = std::collections::BTreeSet::new();
    for deps in dependency_tables(&manifest) {
        for (name, dep) in deps {
            if let Some(path) = dep.get("path").and_then(toml::Value::as_str) {
                dirs.insert(venture.join(path));
            } else if dep.get("workspace").and_then(toml::Value::as_bool) == Some(true) {
                if workspace.is_none() {
                    workspace = workspace_root(&venture);
                }
                if let Some(path) = workspace
                    .as_deref()
                    .and_then(|root| workspace_dep_path(root, name))
                {
                    dirs.insert(path);
                }
            }
        }
    }
    dirs.into_iter().collect()
}

/// The dependency tables a venture manifest declares: `[dependencies]`
/// and every `[target.*.dependencies]`.
fn dependency_tables(manifest: &toml::Table) -> Vec<&toml::Table> {
    let mut tables = Vec::new();
    if let Some(deps) = manifest.get("dependencies").and_then(toml::Value::as_table) {
        tables.push(deps);
    }
    if let Some(targets) = manifest.get("target").and_then(toml::Value::as_table) {
        for target in targets.values() {
            if let Some(deps) = target.get("dependencies").and_then(toml::Value::as_table) {
                tables.push(deps);
            }
        }
    }
    tables
}

/// The nearest manifest at or above the venture that carries a
/// `[workspace]` table — the table a `workspace = true` dependency is
/// resolved against.
fn workspace_root(venture: &Path) -> Option<PathBuf> {
    let mut dir = venture.to_path_buf();
    loop {
        let is_workspace = std::fs::read_to_string(dir.join("Cargo.toml"))
            .ok()
            .and_then(|text| text.parse::<toml::Table>().ok())
            .is_some_and(|table| table.contains_key("workspace"));
        if is_workspace {
            return Some(dir);
        }
        if !dir.pop() || dir.as_os_str().is_empty() {
            return None;
        }
    }
}

/// The path a workspace dependency is pinned to in the root manifest's
/// `[workspace.dependencies]`, relative to that root.
fn workspace_dep_path(root: &Path, name: &str) -> Option<PathBuf> {
    let table = std::fs::read_to_string(root.join("Cargo.toml"))
        .ok()
        .and_then(|text| text.parse::<toml::Table>().ok())?;
    table
        .get("workspace")?
        .get("dependencies")?
        .get(name)?
        .get("path")?
        .as_str()
        .map(|path| root.join(path))
}

/// The sqlite migration files a crate directory ships, sorted:
/// `migrations/sqlite/*.sql` when that set exists, else the flat
/// `migrations/*.sql` — but only when it carries no `.harness-lock.json`
/// (the lock file `fz migrations collect` writes), which marks another
/// venture's collected output rather than a module's own SQL.
/// `migrations/postgres/` is never read: a crate that ships a postgres
/// set has declared where its SQL differs, the same rule the in-crate
/// lint check applies to a module. Public because the `cli-acceptance`
/// migration test reads module crates and resolved dependencies with the
/// same rule, so the two cannot drift.
#[must_use]
pub fn shipped_sql_files(dir: &Path) -> Vec<PathBuf> {
    let migrations = dir.join("migrations");
    let set = if migrations.join("sqlite").is_dir() {
        migrations.join("sqlite")
    } else if migrations.is_dir() && !migrations.join(crate::lock::LOCK_FILE).exists() {
        migrations
    } else {
        return Vec::new();
    };
    let mut files: Vec<PathBuf> = std::fs::read_dir(&set)
        .map(|entries| {
            entries
                .flatten()
                .map(|entry| entry.path())
                .filter(|path| {
                    path.is_file()
                        && path
                            .extension()
                            .is_some_and(|ext| ext.eq_ignore_ascii_case("sql"))
                })
                .collect()
        })
        .unwrap_or_default();
    files.sort();
    files
}

/// Sidecars (issue #66). The doctor sees only compiled-in modules, so
/// it says plainly what it cannot check rather than reporting a clean
/// bill for half the venture. A mount that shadows a compiled-in
/// module is a real misconfiguration: the runtime ignores it and
/// keeps the compiled-in module, so someone believes a sidecar is
/// serving a prefix that it is not.
fn sidecar_failures(
    harness: &Harness,
    sidecars: Option<&str>,
    output: Output,
    failures: &mut Vec<DoctorFailure>,
) {
    match crate::sidecars::parse(sidecars) {
        Ok(mounts) if !mounts.is_empty() => {
            for mount in crate::sidecars::shadowing(harness, &mounts) {
                failures.push(DoctorFailure {
                    code: &CODES.sidecar_shadows_module,
                    message: format!(
                        "sidecar mount `{}` names a module compiled into this venture; the runtime \
                         ignores the mount and serves the compiled-in module. Remove it from \
                         {HARNESS_SIDECARS} or remove the module from the composition",
                        mount.name
                    ),
                });
            }
            if output == Output::Human {
                eprintln!(
                    "fz: warning: {} is sidecar-mounted, so its tables, migrations and SQL are not \
                     checked here — they live in its own repository (docs/MOUNTING.md)",
                    crate::sidecars::listed(&mounts)
                );
                // Gateway advisory (issue #131): without the shared secret the
                // host stamps no token, so a sidecar that requires one refuses
                // every forwarded request and a sidecar that does not cannot
                // tell the host from the open internet.
                if std::env::var(SIDECAR_GATEWAY_SECRET)
                    .ok()
                    .is_none_or(|value| value.trim().is_empty())
                {
                    eprintln!(
                        "fz: warning: {SIDECAR_GATEWAY_SECRET} is unset — the host stamps no \
                         gateway token, so a sidecar that requires one (SIDECAR_REQUIRE_GATEWAY) \
                         refuses every forwarded request (issue #131)"
                    );
                }
            }
        }
        Ok(_) => {}
        Err(errors) => failures.extend(errors.into_iter().map(|message| DoctorFailure {
            code: &CODES.sidecar_mount_invalid,
            message,
        })),
    }
}

/// The SQL lints: the portable-SQL check and the card-data check (#44).
/// The portable lint skips a module that ships a `postgres` set — it has
/// declared where its SQL differs, the same rule
/// `cratefield_adapter_postgres::select_set` applies when it chooses a set,
/// and the doctor should not disagree with the runner. The card-data lint
/// never skips: no dialect stores a card number, verification code or full
/// expiry, unlike the portable lint above.
fn lint_failures(harness: &Harness, failures: &mut Vec<DoctorFailure>) {
    for module in harness.modules() {
        if !module.migrations().postgres.is_empty() {
            continue;
        }
        for migration in module.migrations().sqlite {
            for (token, why) in banned_tokens(migration.sql) {
                failures.push(DoctorFailure {
                    code: &CODES.non_portable_sql,
                    message: format!(
                        "module `{}` migration {}: banned token `{token}` — {why}",
                        module.name(),
                        migration.id
                    ),
                });
            }
        }
    }

    for module in harness.modules() {
        let migrations = module.migrations();
        for migration in migrations.sqlite.iter().chain(migrations.postgres.iter()) {
            for (fragment, why) in lint_card_data(migration.sql) {
                failures.push(DoctorFailure {
                    code: &CODES.card_data_in_migration,
                    message: format!(
                        "module `{}` migration {}: `{fragment}` {why}",
                        module.name(),
                        migration.id
                    ),
                });
            }
        }
    }
}

/// The push-wiring section (issue #191). The doctor does not read the push
/// environment itself: it calls the same `build_push` `serve()` and `fz push`
/// call, through `inspect_push`, so the three cannot check different variable
/// names.
///
/// Gated on what the harness declares, like every other production-only rule
/// here: the process environment is shared, and a venture that mounts no push
/// module at all must not fail its deploy because the env file it inherits
/// exports someone else's `VAPID_SUBJECT`.
fn push_checks(
    harness: &Harness,
    env: VentureEnv,
    output: Output,
    failures: &mut Vec<DoctorFailure>,
) {
    let declaration = PushDeclaration::of(harness.modules());
    if !declaration.declared {
        return;
    }
    push_wiring_checks(
        &cratefield_push_wiring::inspect_push(&EnvVars),
        env,
        declaration,
        output,
        failures,
    );
}

/// What this venture says about push, so the doctor's push rules key on the
/// harness rather than on whatever the process environment happens to
/// export (issue #191).
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
struct PushDeclaration {
    /// A module lists [`Port::Push`] in `requires()` or `optional()`. Only
    /// then is the push environment this venture's business.
    declared: bool,
    /// A module lists it in `requires()`: this venture does not work
    /// without a transport that can actually send.
    required: bool,
}

impl PushDeclaration {
    fn of(modules: &[std::sync::Arc<dyn cratefield_core::Module>]) -> Self {
        Self::from_ports(
            modules
                .iter()
                .map(|module| (module.requires(), module.optional())),
        )
    }

    /// The rule itself, over nothing but the port lists — so it is unit
    /// tested without composing a harness, which for a module that
    /// *requires* a port would mean a runtime that provides it too.
    fn from_ports<'a>(modules: impl Iterator<Item = (&'a [Port], &'a [Port])>) -> Self {
        let mut declaration = Self {
            declared: false,
            required: false,
        };
        for (requires, optional) in modules {
            if requires.contains(&Port::Push) {
                declaration.declared = true;
                declaration.required = true;
            } else if optional.contains(&Port::Push) {
                declaration.declared = true;
            }
        }
        declaration
    }
}

/// Turns a push wiring report into doctor output: failures in production,
/// warnings below it (issue #191). Split out so the rules are unit tested
/// without a process environment.
///
/// The severity rule itself is [`PushWiring::check`]'s, called rather than
/// restated — the same rule encoded twice is the drift this crate exists to
/// prevent, one copy short of a test.
///
/// The second rule is the build-time gate `push_from_env()` cannot keep. A
/// runtime that assembles the port from the environment provides `Port::Push`
/// whatever that environment holds — on Workers the `Env` does not exist
/// until the first fetch, so `Harness::build` cannot know — and a module that
/// requires push then compiles, deploys, and answers `NotConfigured` for
/// every send. The doctor is where the deployment's environment *is* known,
/// so the refusal lives here: in production, a venture whose modules require
/// push and whose environment routes no transport at all fails.
fn push_wiring_checks(
    wiring: &PushWiring,
    env: VentureEnv,
    declaration: PushDeclaration,
    output: Output,
    failures: &mut Vec<DoctorFailure>,
) {
    match wiring.check(env) {
        // Half-wired and refused-credentials are one code, not two: the
        // message already names which transport and which variables, and
        // `check` reports every misconfigured transport in one string, so a
        // venture with one of each has a single failure to classify. The
        // operator action is the same either way — finish or correct that
        // transport's variables, or unset them to say it is deliberately off.
        Err(problems) => failures.push(DoctorFailure {
            code: &CODES.push_transport_misconfigured,
            message: problems,
        }),
        // Below production the same problems are a warning: wiring a
        // transport one variable at a time is what development looks like.
        Ok(()) => {
            if output == Output::Human {
                for problem in wiring.problems() {
                    eprintln!("fz: warning: {problem}");
                }
            }
        }
    }

    if declaration.required && env == VentureEnv::Production && !wiring.any_routed() {
        failures.push(DoctorFailure {
            code: &CODES.push_required_but_unrouted,
            message: "production venture has modules that require the Push port but no push \
                      transport is configured — every send would answer NotConfigured. Set one \
                      transport's variables (docs/PUSH-ENV.md), or drop the modules that require \
                      push. If this venture provides Push from an adapter of its own rather than \
                      from the environment, that adapter is not visible here (issue #191)"
                .to_owned(),
        });
    }
}

/// The production-only port rules, gathered so `doctor` stays a flat list of
/// checks: the captcha rule (with its override), the payments webhook rule,
/// and the rate-limiter rule (with its override).
///
/// Read once and passed in, rather than each check reaching for
/// `std::env` itself. Environment variables are process-global and tests
/// run in one process, so a check that reads them directly can be tested
/// as a pure function and never as a *called* one — falsifying the auth
/// check showed exactly that: deleting the call left every test green.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) struct Configured {
    pub(crate) auth_issuer: bool,
    pub(crate) auth_client_id: bool,
    pub(crate) stripe_webhook_secret: bool,
    /// The `ADMIN_TOKEN`'s length, not its presence: the boot's floor is a
    /// length (`MIN_SECRET_BYTES`, issue #437), so the doctor has to
    /// measure the same thing the boot measures. `None` is unset — legal
    /// everywhere, here as in `HarnessConfig::from_config`.
    pub(crate) admin_token_bytes: Option<usize>,
}

impl Configured {
    fn from_env() -> Self {
        Self {
            auth_issuer: set("AUTH_ISSUER"),
            auth_client_id: set("AUTH_CLIENT_ID"),
            stripe_webhook_secret: set("STRIPE_WEBHOOK_SECRET"),
            // Read the way `EnvVars::get` reads — the read the boot's
            // config makes — so a set-but-empty variable is unset here
            // too, and the length is the raw value's, untrimmed: the boot
            // measures the raw value, and the doctor may not be stricter.
            admin_token_bytes: std::env::var("ADMIN_TOKEN")
                .ok()
                .filter(|value| !value.is_empty())
                .map(|value| value.len()),
        }
    }
}

fn production_port_checks(
    harness: &Harness,
    configured: Configured,
    config: &dyn cratefield_core::Config,
    allow_no_captcha: Option<&str>,
    output: Output,
    failures: &mut Vec<DoctorFailure>,
) {
    let guards = cratefield_core::WriteGuards::collect(harness.modules());
    if let Some(message) = captcha_production_failure(
        &guards,
        cratefield_core::captcha_effective(harness.runtime()),
    ) {
        // The same rule the gate in `route_policy` applies, from the
        // same function: `--allow-no-captcha ""` is not an acceptance,
        // so it does not downgrade the failure and does not get logged
        // as though somebody had answered for it.
        match cratefield_core::stated_reason(allow_no_captcha) {
            Some(reason) => {
                // An override of a production abuse control is a decision
                // someone has to answer for later, so it is recorded, not
                // just printed (issue #143). `tracing` is the audited sink
                // — scrubbed by #135 and shipped wherever the operator
                // sends logs — and fires in both disciplines, while the
                // stderr line keeps the operator running the command
                // informed.
                tracing::warn!(
                    control = "captcha",
                    reason,
                    modules = guards.captcha_modules.join(","),
                    "production abuse control overridden by an operator"
                );
                if output == Output::Human {
                    eprintln!("fz: warning: captcha override accepted ({reason}): {message}");
                    eprintln!(
                        "fz: note: this override is recorded; it is for previews only and must \
                         not stand in for a Captcha port in production"
                    );
                }
            }
            None => failures.push(DoctorFailure {
                code: &CODES.captcha_not_effective,
                message,
            }),
        }
    }

    if let Some(message) = auth_production_failure(
        harness
            .modules()
            .iter()
            .any(|module| module.requires().contains(&Port::Auth)),
        configured.auth_issuer,
        configured.auth_client_id,
    ) {
        failures.push(DoctorFailure {
            code: &CODES.auth_not_configured,
            message,
        });
    }

    // Name each Payments-verifying module with the routes it guards
    // (issue #595): a module may verify one webhook through Payments and
    // another with an HMAC secret, so the operator should see which route
    // the missing `STRIPE_WEBHOOK_SECRET` blocks.
    let payments: Vec<String> = guards
        .payments_signature_modules
        .iter()
        .map(|module| {
            guards.signature_module_named(module, cratefield_core::SignatureVerification::Payments)
        })
        .collect();
    if let Some(message) = payments_webhook_failure(&payments, configured.stripe_webhook_secret) {
        failures.push(DoctorFailure {
            code: &CODES.payments_webhook_secret_missing,
            message,
        });
    }

    // The other half of the webhook rule (issues #533, #595): the key is
    // per module, so this check takes composed keys and routes and a
    // config rather than a `Configured` bool — still pure, the caller
    // supplies the deployment config.
    if let Some(message) = hmac_webhook_failure(&guards.hmac_signature_routes(), config) {
        failures.push(DoctorFailure {
            code: &CODES.hmac_webhook_secret_missing,
            message,
        });
    }

    // The limiter leg mirrors the captcha rule above, hatch and all: the
    // same two answers the boot gate (`production_readiness`) reads —
    // what the composition declares and whether the runtime's limiter is
    // effective — with an operator's recorded reason downgrading the
    // failure to a logged warning. `unlimited_public_routes_override`
    // applies the blank-reason rule itself, so an empty acceptance never
    // counts, exactly as `stated_reason` does for the captcha flag.
    if let Some(message) = rate_limiter_production_failure(
        guards.needs_rate_limiter(),
        cratefield_core::rate_limiter_effective(harness.runtime()),
        &guards
            .rate_limiter_modules
            .iter()
            .map(|(module, routes)| format!("{module} ({})", routes.join(", ")))
            .collect::<Vec<_>>(),
    ) {
        match cratefield_core::unlimited_public_routes_override(config) {
            Some(reason) => {
                tracing::warn!(
                    control = "rate-limiter",
                    reason,
                    "production abuse control overridden by an operator"
                );
                if output == Output::Human {
                    eprintln!("fz: warning: rate-limiter override accepted ({reason}): {message}");
                    eprintln!(
                        "fz: note: this override is recorded; resolve the RateLimiter binding \
                         and remove HARNESS_ALLOW_UNLIMITED_PUBLIC_ROUTES before serving \
                         production traffic"
                    );
                }
            }
            None => failures.push(DoctorFailure {
                code: &CODES.rate_limiter_not_effective,
                message,
            }),
        }
    }
}

/// Whether an environment variable is set to something non-blank.
fn set(key: &str) -> bool {
    std::env::var(key).is_ok_and(|value| !value.trim().is_empty())
}

/// The production auth rule (issue #153): a module that declares
/// `Port::Auth` is a module that must know who is calling, and a
/// deployment with no issuer configured cannot tell it.
///
/// The runtime provides the port either way — on Workers the `Env` exists
/// per request, so a runtime cannot know at compose time whether the
/// issuer is set, and refusing to provide it would stop every such
/// deployment booting, including the ones whose tables are all public.
/// The cost of that choice is that the failure moves to request time: an
/// unconfigured deployment serves its public routes and answers `503` on
/// every route that needs a caller.
///
/// `fz doctor` runs where the operator's environment is, which is why the
/// check belongs here — the same place `STRIPE_WEBHOOK_SECRET` is read,
/// and for the same reason.
fn auth_production_failure(
    needs_auth: bool,
    issuer_set: bool,
    client_id_set: bool,
) -> Option<String> {
    if !needs_auth || (issuer_set && client_id_set) {
        return None;
    }
    let mut missing = Vec::new();
    if !issuer_set {
        missing.push("AUTH_ISSUER");
    }
    if !client_id_set {
        missing.push("AUTH_CLIENT_ID");
    }
    Some(format!(
        "a module requires the Auth port and {} is not set; every route that needs a signed-in \
         caller will answer 503",
        missing.join(" and ")
    ))
}

/// The production captcha rule (issue #133): captcha-guarded writes need a
/// Captcha port that is *effectively* configured — provided **and** bound per
/// the adapter's report — not merely present. The builder already refuses
/// this composition (`Harness::build`); the doctor re-checks it so the rule
/// stays visible in operator tooling.
fn captcha_production_failure(
    guards: &cratefield_core::WriteGuards,
    captcha_effective: bool,
) -> Option<String> {
    if guards.needs_captcha() && !captcha_effective {
        Some(format!(
            "production venture has captcha-guarded public writes from [{}] but the Captcha port \
             is not effectively configured — provide Turnstile with its secret and expected \
             hostname (architecture section 11; issue #133)",
            guards.captcha_modules.join(", ")
        ))
    } else {
        None
    }
}

/// The production-payments rule: a production venture that declares
/// [`Signature`]-guarded routes without a webhook signing secret cannot
/// verify Stripe webhooks, so it would process forged events. Pure so the
/// truth table is unit-tested; the caller supplies the routes (naming each
/// module and the `METHOD /path` it verifies through Payments, issue #595)
/// and the env-derived boolean.
///
/// [`Signature`]: cratefield_core::RoutePolicy::Signature
fn payments_webhook_failure(
    payments_routes: &[String],
    webhook_secret_present: bool,
) -> Option<String> {
    (!payments_routes.is_empty() && !webhook_secret_present).then(|| {
        format!(
            "production venture has signature-guarded routes [{}] but STRIPE_WEBHOOK_SECRET is \
             unset — webhook signatures cannot be verified and forged events would be trusted \
             (issue #102)",
            payments_routes.join(", ")
        )
    })
}

/// The production webhook-secret rule for modules that verify through the
/// core HMAC scheme (issues #533, #595): a deployment missing one has a
/// module whose handler refuses every delivery, and the message names the
/// route that reads the key so a two-verifier module says which endpoint
/// is blocked. Keys are named, never values; pure so the truth table is
/// unit-tested, the caller supplies the config.
fn hmac_webhook_failure(
    hmac_modules: &[(String, String, Vec<String>)],
    config: &dyn cratefield_core::Config,
) -> Option<String> {
    let missing: Vec<String> = hmac_modules
        .iter()
        .filter(|(_, key, _)| config.get(key).is_none_or(|value| value.trim().is_empty()))
        .map(|(_, key, routes)| {
            if routes.is_empty() {
                key.clone()
            } else {
                format!("{key} on {}", routes.join(", "))
            }
        })
        .collect();
    (!missing.is_empty()).then(|| {
        format!(
            "production venture verifies webhook signatures with the webhook_signature HMAC \
             scheme but {} is not set — those handlers refuse every delivery until the \
             provider's signing secret is configured (issue #533)",
            missing.join(", ")
        )
    })
}

/// The production rate-limiter rule (issue #437): a venture that takes
/// public writes or admin routes needs a limiter the runtime can actually
/// consult — an admin bearer token is guessed rather than submitted and has
/// no captcha or cooldown behind it, and every public write runs without a
/// budget otherwise. The boot gate (`production_readiness`, via
/// `Harness::build` and `Harness::router`) enforces the same rule from the
/// same two answers; the doctor re-checks it so the operator learns before
/// the deploy, which is where this command runs.
fn rate_limiter_production_failure(
    needs_rate_limiter: bool,
    rate_limiter_effective: bool,
    declaring: &[String],
) -> Option<String> {
    if needs_rate_limiter && !rate_limiter_effective {
        // Modules that declare the port (issue #562) are named with their
        // routes, the way the boot gate names them — the operator should
        // be able to fix the named thing without re-deriving which route
        // made the rule fire, and the consequence named is theirs, not a
        // public-write venture's.
        let (who, cost) = if declaring.is_empty() {
            (
                "takes public writes or admin routes".to_owned(),
                "admin bearer routes have no brute-force backstop behind the limiter and \
                 every public write runs without a budget"
                    .to_owned(),
            )
        } else {
            (
                format!(
                    "declares RateLimiter for public routes from [{}]",
                    declaring.join(", ")
                ),
                "those routes — and any public write or admin route beside them — run \
                 without a budget"
                    .to_owned(),
            )
        };
        Some(format!(
            "production venture {who} but the RateLimiter port is not resolved: {cost} — \
                 resolve the binding so the runtime actually hands over a limiter, or set \
                 HARNESS_ALLOW_UNLIMITED_PUBLIC_ROUTES to a reason to serve unlimited \
                 (issues #437, #562)"
        ))
    } else {
        None
    }
}

/// The admin-token floor (issue #437). `HarnessConfig::from_config`
/// refuses a set-but-short `ADMIN_TOKEN` in **every** environment
/// (`MIN_SECRET_BYTES`), so this rule runs unconditionally too: gated on
/// production it would leave the doctor more permissive than the boot it
/// predicts. Absent stays legal — the token is what turns the admin plane
/// on, and removing it is the documented way to keep that plane disabled.
fn admin_token_failure(admin_token_bytes: Option<usize>) -> Option<String> {
    let bytes = admin_token_bytes?;
    (bytes < MIN_SECRET_BYTES).then(|| {
        format!(
            "ADMIN_TOKEN is {bytes} bytes but must be at least {MIN_SECRET_BYTES} when set — \
             the boot refuses it in every environment, so this deploy cannot start. Lengthen \
             it, or remove it to keep the admin routes disabled (issue #437)"
        )
    })
}

#[cfg(test)]
mod tests {
    use super::{
        CODES, DoctorFailure, DoctorReport, Output, PushDeclaration, admin_token_failure,
        captcha_production_failure, hmac_webhook_failure, payments_webhook_failure,
        push_wiring_checks, rate_limiter_production_failure,
    };
    use cratefield_core::{
        Config, ConfigError, Harness, MapConfig, Migrations, Module, ModuleContext, Port, Runtime,
        Venture, VentureEnv, WriteGuards,
    };
    use cratefield_push_wiring::{PUSH_ENV, PushKey, PushWiring, inspect_push};
    use std::sync::Arc;

    /// A module that must know who is calling.
    pub(super) struct NeedsAuth;

    /// One that does not.
    pub(super) struct Nothing;

    macro_rules! bare_module {
        ($ty:ty, $name:literal, $ports:expr) => {
            impl Module for $ty {
                fn name(&self) -> &'static str {
                    $name
                }
                fn version(&self) -> &'static str {
                    "0.0.0"
                }
                fn requires(&self) -> &'static [Port] {
                    $ports
                }
                fn migrations(&self) -> Migrations {
                    Migrations::default()
                }
                fn validate_config(&self, _cfg: &dyn Config) -> Result<(), ConfigError> {
                    Ok(())
                }
                fn router(&self, _ctx: ModuleContext) -> cratefield_core::axum::Router {
                    cratefield_core::axum::Router::new()
                }
            }
        };
    }

    /// A module with a signature-guarded write — a webhook receiver.
    pub(super) struct TakesWebhooks;

    bare_module!(NeedsAuth, "needs-auth", &[Port::Auth]);
    bare_module!(Nothing, "nothing", &[]);
    impl Module for TakesWebhooks {
        fn name(&self) -> &'static str {
            "takes-webhooks"
        }
        fn version(&self) -> &'static str {
            "0.0.0"
        }
        fn requires(&self) -> &'static [Port] {
            &[Port::Payments]
        }
        fn migrations(&self) -> Migrations {
            Migrations::default()
        }
        fn validate_config(&self, _cfg: &dyn Config) -> Result<(), ConfigError> {
            Ok(())
        }
        fn surface(&self) -> cratefield_core::Surface {
            Self::surface()
        }
        fn router(&self, _ctx: ModuleContext) -> cratefield_core::axum::Router {
            cratefield_core::axum::Router::new()
        }
    }

    impl TakesWebhooks {
        /// Declared through the surface, which is where `WriteGuards`
        /// reads a route's policy from.
        fn surface() -> cratefield_core::Surface {
            cratefield_core::Surface::new().action(
                cratefield_core::Action::post("receive", "/receive")
                    .policy(cratefield_core::RoutePolicy::Signature)
                    .outcome(cratefield_core::Outcome::Json),
            )
        }
    }

    /// A billing module with two webhooks and two verifiers (issue #595):
    /// `POST /webhooks/stripe` verifies through `Payments`, and
    /// `POST /webhooks/revenuecat` names its own HMAC secret — so the two
    /// production webhook rules must name the route each one blocks.
    pub(super) struct TwoVerifiers;

    impl Module for TwoVerifiers {
        fn name(&self) -> &'static str {
            "billing"
        }
        fn version(&self) -> &'static str {
            "0.0.0"
        }
        fn requires(&self) -> &'static [Port] {
            &[Port::Payments]
        }
        fn migrations(&self) -> Migrations {
            Migrations::default()
        }
        fn validate_config(&self, _cfg: &dyn Config) -> Result<(), ConfigError> {
            Ok(())
        }
        fn surface(&self) -> cratefield_core::Surface {
            Self::surface()
        }
        fn router(&self, _ctx: ModuleContext) -> cratefield_core::axum::Router {
            cratefield_core::axum::Router::new()
        }
    }

    impl TwoVerifiers {
        fn surface() -> cratefield_core::Surface {
            cratefield_core::Surface::new()
                .action(
                    cratefield_core::Action::post("webhook-stripe", "/webhooks/stripe")
                        .policy(cratefield_core::RoutePolicy::Signature)
                        .outcome(cratefield_core::Outcome::Json),
                )
                .action(
                    cratefield_core::Action::post("webhook-revenuecat", "/webhooks/revenuecat")
                        .policy(cratefield_core::RoutePolicy::Signature)
                        .verification(cratefield_core::SignatureVerification::Hmac {
                            secret: "REVENUECAT_WEBHOOK_SECRET",
                        })
                        .outcome(cratefield_core::Outcome::Json),
                )
        }
    }

    /// A runtime that provides everything, so composition succeeds and the
    /// production checks are what the test is about.
    struct AllPorts;

    impl Runtime for AllPorts {
        fn provides(&self) -> Vec<Port> {
            Port::ALL.to_vec()
        }
    }

    /// A composed harness mounting one module.
    pub(super) fn harness_of(module: Arc<dyn Module>) -> Harness {
        Harness::builder()
            .venture(
                Venture::new("acme", "acme.test")
                    .cors_origins(["https://acme.test"])
                    .env(VentureEnv::Production),
            )
            .module_arc(module)
            .runtime(AllPorts)
            .build()
            .expect("the fixture composes")
    }

    /// A venture whose modules only *may* use push: the wiring is checked,
    /// but nothing has to be routed.
    const OPTIONAL: PushDeclaration = PushDeclaration {
        declared: true,
        required: false,
    };

    /// A venture whose modules cannot work without a transport.
    const REQUIRED: PushDeclaration = PushDeclaration {
        declared: true,
        required: true,
    };

    fn guards(needs_captcha: bool) -> WriteGuards {
        WriteGuards {
            captcha_modules: if needs_captcha {
                vec!["writer".to_owned()]
            } else {
                Vec::new()
            },
            signature_modules: Vec::new(),
            signed_link_modules: Vec::new(),
            // The rule under test reads only `captcha_modules`; the
            // limiter-leg answers come from `Default`.
            ..WriteGuards::default()
        }
    }

    #[test]
    fn a_venture_that_needs_no_caller_is_not_asked_for_an_issuer() {
        // Most ventures do not mount a module that identifies callers.
        // Demanding an auth service from them would be demanding one they
        // have no use for.
        assert_eq!(super::auth_production_failure(false, false, false), None);
    }

    #[test]
    fn a_module_that_must_know_who_is_calling_needs_an_issuer_in_production() {
        // The runtime provides the port either way, so the failure is not
        // at boot: it is every route that needs a caller answering 503,
        // which is the thing an operator wants told before the deploy.
        let refusal = super::auth_production_failure(true, false, false)
            .expect("a module needs a caller and nothing can name one");
        assert!(refusal.contains("AUTH_ISSUER"), "{refusal}");
        assert!(refusal.contains("AUTH_CLIENT_ID"), "{refusal}");
        assert!(refusal.contains("503"), "say what goes wrong: {refusal}");
    }

    #[test]
    fn half_a_verifier_is_still_a_failure_and_names_the_half() {
        // Setting one of the two is the likely mistake, and a message
        // that said "auth is not configured" would send an operator to
        // check the one they already set.
        let missing_id = super::auth_production_failure(true, true, false).expect("half");
        assert!(missing_id.contains("AUTH_CLIENT_ID"), "{missing_id}");
        assert!(!missing_id.contains("AUTH_ISSUER"), "{missing_id}");

        let missing_issuer = super::auth_production_failure(true, false, true).expect("half");
        assert!(missing_issuer.contains("AUTH_ISSUER"), "{missing_issuer}");
        assert!(
            !missing_issuer.contains("AUTH_CLIENT_ID"),
            "{missing_issuer}"
        );
    }

    #[test]
    fn a_configured_verifier_is_no_failure() {
        assert_eq!(super::auth_production_failure(true, true, true), None);
    }

    #[test]
    fn captcha_rule_keys_on_declared_policies_and_effectiveness() {
        // Guarded writes without an effective port fail — even if the port
        // is *present*, since effectiveness is what the gate measures.
        let failure = captcha_production_failure(&guards(true), false).expect("fails closed");
        assert!(failure.contains("writer"));
        assert!(failure.contains("effectively configured"));
        assert!(captcha_production_failure(&guards(true), true).is_none());
        assert!(captcha_production_failure(&guards(false), false).is_none());

        // The waiver this failure is matched against is the *reason*,
        // not the flag. `--allow-no-captcha ""` used to reach the arm
        // that downgrades the failure to a warning and logs the
        // operator's acceptance — with an empty reason in the log.
        // `docs/SECURITY.md` says "A blank reason is not an
        // acceptance"; this is the half of that sentence the CLI owns.
        for nothing in ["", "   "] {
            assert!(
                cratefield_core::stated_reason(Some(nothing)).is_none(),
                "an empty reason counted as an acceptance: {nothing:?}"
            );
        }
        assert_eq!(
            cratefield_core::stated_reason(Some("  previewing the new form  ")),
            Some("previewing the new form"),
            "a real reason still overrides, trimmed"
        );
    }

    #[test]
    fn payments_in_production_needs_a_webhook_secret() {
        // The route is named, so a module with two verifiers says which
        // endpoint the missing secret blocks (issue #595).
        let payments = vec!["billing (POST /v1/billing/webhooks/stripe)".to_owned()];
        let failure = payments_webhook_failure(&payments, false).expect("no secret fails");
        assert!(
            failure.contains("POST /v1/billing/webhooks/stripe"),
            "{failure}"
        );
        assert!(payments_webhook_failure(&payments, true).is_none());
        assert!(payments_webhook_failure(&[], false).is_none());

        // The other half (issue #533): the module's own key, unset or
        // blank, fails; set passes; no such module never fails. The route
        // that reads the key is named too (issue #595).
        let modules = vec![(
            "pos".to_owned(),
            "POS_WEBHOOK_SECRET".to_owned(),
            vec!["POST /webhook".to_owned()],
        )];
        let failure = hmac_webhook_failure(&modules, &MapConfig::default()).expect("unset fails");
        assert!(failure.contains("POS_WEBHOOK_SECRET"), "{failure}");
        assert!(failure.contains("POST /webhook"), "{failure}");
        assert!(
            hmac_webhook_failure(
                &modules,
                &MapConfig::from_pairs([("POS_WEBHOOK_SECRET", "   ")])
            )
            .is_some()
        );
        assert!(
            hmac_webhook_failure(
                &modules,
                &MapConfig::from_pairs([("POS_WEBHOOK_SECRET", "x")])
            )
            .is_none()
        );
        assert!(hmac_webhook_failure(&[], &MapConfig::default()).is_none());
    }

    #[test]
    fn a_venture_with_public_writes_or_admin_routes_needs_a_resolved_limiter() {
        // Fails closed on exactly the two answers the boot gate reads:
        // something writable or an admin plane, and no effective limiter.
        let failure =
            rate_limiter_production_failure(true, false, &[]).expect("an unlimited venture fails");
        assert!(failure.contains("RateLimiter"), "{failure}");
        assert!(
            failure.contains("HARNESS_ALLOW_UNLIMITED_PUBLIC_ROUTES"),
            "the hatch is named the way the captcha rule names its own: {failure}"
        );
        assert!(rate_limiter_production_failure(true, true, &[]).is_none());
        // Nothing writable, no admin plane: no budget is owed.
        assert!(rate_limiter_production_failure(false, false, &[]).is_none());
    }

    #[test]
    fn a_doctor_limiter_failure_names_the_declaring_modules() {
        // Issue #562: when a module declares the port, the doctor message
        // names it and its routes, the way the boot gate's message does.
        let failure = rate_limiter_production_failure(
            true,
            false,
            &["waitlist (POST /v1/waitlist, GET /v1/waitlist/search)".to_owned()],
        )
        .expect("a declaring venture fails");
        assert!(failure.contains("declares RateLimiter"), "{failure}");
        assert!(
            failure.contains("waitlist (POST /v1/waitlist, GET /v1/waitlist/search)"),
            "{failure}"
        );
    }

    #[test]
    fn an_absent_admin_token_is_not_a_failure() {
        // Absent is how the admin plane stays disabled, and the boot
        // treats it the same in every environment.
        assert_eq!(admin_token_failure(None), None);
    }

    #[test]
    fn a_set_but_short_admin_token_fails_at_the_floor_the_boot_enforces() {
        let refusal = admin_token_failure(Some(8)).expect("a short token");
        assert!(refusal.contains("ADMIN_TOKEN"), "{refusal}");
        assert!(refusal.contains("32"), "name the floor: {refusal}");
        assert!(
            refusal.contains("every environment"),
            "the rule is not production-only, and says so: {refusal}"
        );

        // At the floor and above is what the boot accepts, so the doctor
        // agrees at the boundary rather than one byte stricter.
        assert_eq!(admin_token_failure(Some(32)), None);
        assert_eq!(admin_token_failure(Some(64)), None);
    }

    /// The `--json` wire contract (harness #140): field order, the schema
    /// number, and the code string straight from the catalogue.
    #[test]
    fn report_renders_the_json_contract() {
        let report = DoctorReport {
            failures: vec![
                DoctorFailure {
                    code: &CODES.captcha_not_effective,
                    message: "production venture has captcha-guarded writes".to_owned(),
                },
                DoctorFailure {
                    code: &CODES.payments_webhook_secret_missing,
                    message: "STRIPE_WEBHOOK_SECRET is unset".to_owned(),
                },
            ],
        };
        let payload = report.render_json().expect("serializes");
        assert_eq!(
            payload,
            "{\"schema\":1,\"ok\":false,\"failures\":[\
             {\"code\":\"captcha-not-effective\",\"message\":\"production venture has \
             captcha-guarded writes\"},\
             {\"code\":\"payments-webhook-secret-missing\",\"message\":\"STRIPE_WEBHOOK_SECRET \
             is unset\"}]}"
        );
        assert!(!report.ok());

        let clean = DoctorReport {
            failures: Vec::new(),
        };
        assert_eq!(
            clean.render_json().expect("serializes"),
            "{\"schema\":1,\"ok\":true,\"failures\":[]}"
        );
        assert!(clean.ok());
    }

    /// A half-wired push environment, built through the real reader so this
    /// test names no environment variable of its own — the property
    /// `cli-acceptance`'s guard enforces across the workspace.
    fn half_wired() -> PushWiring {
        let var = PUSH_ENV
            .iter()
            .find(|var| var.required)
            .expect("the table has a required variable");
        let wiring = inspect_push(&MapConfig::from_pairs([(
            var.name().to_owned(),
            "set-but-alone".to_owned(),
        )]));
        assert!(
            !wiring.problems().is_empty(),
            "one variable of a transport is a half-wired transport: {}",
            wiring.summary()
        );
        wiring
    }

    #[test]
    fn a_half_wired_transport_fails_the_doctor_in_production_only() {
        let wiring = half_wired();

        let mut failures = Vec::new();
        push_wiring_checks(
            &wiring,
            VentureEnv::Production,
            OPTIONAL,
            Output::Human,
            &mut failures,
        );
        assert_eq!(failures.len(), 1, "{failures:?}");
        for problem in wiring.problems() {
            assert!(failures[0].message.contains(&problem), "{failures:?}");
        }
        // The slug itself, written out: the catalogue's contract is that a
        // code is never renamed, and a test that reads it back off `CODES`
        // would rename with it.
        assert_eq!(failures[0].code.code, "push-transport-misconfigured");

        // Below production it is a printed warning, not a failure: wiring a
        // transport one variable at a time is what development looks like.
        for env in [VentureEnv::Development, VentureEnv::Staging] {
            let mut failures = Vec::new();
            push_wiring_checks(&wiring, env, OPTIONAL, Output::Human, &mut failures);
            assert!(failures.is_empty(), "{env:?}: {failures:?}");
        }
    }

    #[test]
    fn a_venture_that_wires_no_push_at_all_is_silent() {
        // Absent is a choice, not a defect — the doctor must not nag a
        // venture that sends no notifications, even in production.
        let wiring = inspect_push(&MapConfig::from_pairs(Vec::<(String, String)>::new()));
        let mut failures = Vec::new();
        push_wiring_checks(
            &wiring,
            VentureEnv::Production,
            OPTIONAL,
            Output::Human,
            &mut failures,
        );
        assert!(failures.is_empty(), "{failures:?}");
        assert!(wiring.problems().is_empty(), "{:?}", wiring.problems());
    }

    #[test]
    fn a_module_that_requires_push_needs_a_transport_that_can_send() {
        // The build-time gate `push_from_env()` cannot keep: on Workers the
        // `Env` does not exist when `Harness::build` runs, so a module
        // requiring Push compiles and deploys against an environment that
        // configures nothing and answers NotConfigured for every send. The
        // doctor runs where the environment is known, so it refuses.
        let nothing = inspect_push(&MapConfig::from_pairs(Vec::<(String, String)>::new()));
        assert!(!nothing.any_routed());

        let mut failures = Vec::new();
        push_wiring_checks(
            &nothing,
            VentureEnv::Production,
            REQUIRED,
            Output::Human,
            &mut failures,
        );
        assert_eq!(failures.len(), 1, "{failures:?}");
        assert!(
            failures[0].message.contains("require the Push port"),
            "{failures:?}"
        );
        // Its own code, not the half-wired one: nothing here is half-wired,
        // and the operator's move is to route a transport or to drop the
        // modules that require one.
        assert_eq!(failures[0].code.code, "push-required-but-unrouted");

        // Only in production, and only when a module actually requires it.
        for env in [VentureEnv::Development, VentureEnv::Staging] {
            let mut failures = Vec::new();
            push_wiring_checks(&nothing, env, REQUIRED, Output::Human, &mut failures);
            assert!(failures.is_empty(), "{env:?}: {failures:?}");
        }
        let mut failures = Vec::new();
        push_wiring_checks(
            &nothing,
            VentureEnv::Production,
            OPTIONAL,
            Output::Human,
            &mut failures,
        );
        assert!(failures.is_empty(), "{failures:?}");
    }

    #[test]
    fn the_push_rules_are_gated_on_what_the_venture_declares() {
        // The process environment is shared. A venture that mounts no push
        // module at all must not fail its production doctor run because the
        // env file it inherits exports somebody else's VAPID_SUBJECT — every
        // other production-only rule here keys on the harness, and this one
        // did not.
        let none = PushDeclaration::from_ports([(&[Port::Db][..], &[][..])].into_iter());
        assert!(!none.declared);
        assert!(!none.required);

        let optional =
            PushDeclaration::from_ports([(&[Port::Db][..], &[Port::Push][..])].into_iter());
        assert!(optional.declared);
        assert!(
            !optional.required,
            "a module that only *may* use push does not make one mandatory"
        );

        let required = PushDeclaration::from_ports(
            [(&[Port::Db][..], &[][..]), (&[Port::Push][..], &[][..])].into_iter(),
        );
        assert!(required.declared);
        assert!(required.required);
    }

    #[test]
    fn a_configured_transport_satisfies_a_module_that_requires_push() {
        // The rule must pass on the deployment it is meant to allow, or it
        // is just a way to fail every production doctor run.
        //
        // The variables are named through `PushKey`, not written out: the
        // guard in `cli-acceptance` allows exactly one source in the
        // workspace to spell one of these names, and it is not this one.
        let key = "-----BEGIN PRIVATE KEY-----\nMIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQg\
                   cXMgRpW+eLn7ZvCx\nIuTdd8csWMZ69azlRzS0dy2FN6GhRANCAATJ6GazR2lhWcC3JYsazLR0\
                   uWOyDKrC\nmeP4HPWghRmfoa4z3Ux7mG3Ylz+auRaBukKGicSdSvVG+jGeQwr3fNag\n\
                   -----END PRIVATE KEY-----";
        let wiring = inspect_push(&MapConfig::from_pairs([
            (PushKey::VapidPrivateKey.name().to_owned(), key.to_owned()),
            (
                PushKey::VapidSubject.name().to_owned(),
                "mailto:ops@example.test".to_owned(),
            ),
        ]));
        assert!(wiring.any_routed(), "{}", wiring.summary());

        let mut failures = Vec::new();
        push_wiring_checks(
            &wiring,
            VentureEnv::Production,
            REQUIRED,
            Output::Human,
            &mut failures,
        );
        assert!(failures.is_empty(), "{failures:?}");
    }
}

#[cfg(test)]
mod production_wiring {
    //! That the production checks are *called*, not only correct.
    //!
    //! Falsifying the auth rule showed the gap: deleting its call from
    //! `production_port_checks` left every test green, because each check
    //! was only ever exercised as a pure function. The same was true of
    //! the payments rule, and both are covered here.
    //!
    //! The captcha rule is **not**, and cannot be: `Harness::build`
    //! refuses a production composition with a form-guarded write and an
    //! ineffective Captcha port, so a harness that would trip the
    //! doctor's copy of that rule cannot be composed to hand it. The
    //! doctor re-checks it deliberately — to keep the rule visible in
    //! operator tooling — and the only path that reaches it is a venture
    //! built with `--allow-no-captcha`, which is the override, not the
    //! failure. Its own pure test stands.
    //!
    //! The rate-limiter leg (issue #437) joins that club for the same
    //! reason: `Harness::build` runs `append_production_readiness` over
    //! the same two answers the doctor reads — `WriteGuards::collect`
    //! plus `rate_limiter_effective` — so a production harness that
    //! would trip the doctor's copy cannot be composed either (the
    //! escape is the `HARNESS_ALLOW_UNLIMITED_PUBLIC_ROUTES` env var,
    //! which a test must not export into a shared process). Its own
    //! pure test stands.
    //!
    //! The admin-token check is wired differently — it is called from
    //! `run_checks` directly, because the boot enforces it in *every*
    //! environment — so its proof runs the whole doctor against a
    //! crafted [`Configured`], no process environment involved.

    use super::tests::{NeedsAuth, Nothing, harness_of};
    use super::{CODES, Configured, DoctorFailure, Output, production_port_checks, run_checks};

    fn failures(
        module: std::sync::Arc<dyn cratefield_core::Module>,
        configured: Configured,
    ) -> Vec<String> {
        let harness = harness_of(module);
        let mut out = Vec::new();
        production_port_checks(
            &harness,
            configured,
            &cratefield_core::MapConfig::default(),
            None,
            Output::Json,
            &mut out,
        );
        out.iter()
            .map(|failure: &DoctorFailure| failure.code.code.to_owned())
            .collect()
    }

    /// The `(code, message)` pairs, so a test can assert a message names
    /// the route a missing secret blocks (issue #595).
    fn messages(
        module: std::sync::Arc<dyn cratefield_core::Module>,
        configured: Configured,
        config: &dyn cratefield_core::Config,
    ) -> Vec<(String, String)> {
        let harness = harness_of(module);
        let mut out = Vec::new();
        production_port_checks(&harness, configured, config, None, Output::Json, &mut out);
        out.iter()
            .map(|failure| (failure.code.code.to_owned(), failure.message.clone()))
            .collect()
    }

    const NONE_SET: Configured = Configured {
        auth_issuer: false,
        auth_client_id: false,
        stripe_webhook_secret: false,
        admin_token_bytes: None,
    };

    #[test]
    fn a_module_needing_a_caller_with_no_issuer_is_reported() {
        assert!(
            failures(std::sync::Arc::new(NeedsAuth), NONE_SET)
                .contains(&CODES.auth_not_configured.code.to_owned()),
            "the rule is correct and nothing calls it"
        );
    }

    #[test]
    fn the_same_module_with_an_issuer_is_not() {
        let configured = Configured {
            auth_issuer: true,
            auth_client_id: true,
            ..NONE_SET
        };
        assert!(
            !failures(std::sync::Arc::new(NeedsAuth), configured)
                .contains(&CODES.auth_not_configured.code.to_owned())
        );
    }

    #[test]
    fn a_webhook_receiver_with_no_secret_is_reported() {
        // The same gap the auth rule had: the payments rule was correct
        // and only ever tested as a pure function, so deleting its call
        // left every test green.
        assert!(
            failures(std::sync::Arc::new(super::tests::TakesWebhooks), NONE_SET)
                .contains(&CODES.payments_webhook_secret_missing.code.to_owned()),
            "the payments rule is correct and nothing calls it"
        );
    }

    #[test]
    fn the_same_receiver_with_a_secret_is_not() {
        let configured = Configured {
            stripe_webhook_secret: true,
            ..NONE_SET
        };
        assert!(
            !failures(std::sync::Arc::new(super::tests::TakesWebhooks), configured)
                .contains(&CODES.payments_webhook_secret_missing.code.to_owned())
        );
    }

    #[test]
    fn a_module_needing_no_caller_is_never_asked() {
        assert!(
            !failures(std::sync::Arc::new(Nothing), NONE_SET)
                .contains(&CODES.auth_not_configured.code.to_owned())
        );
    }

    #[test]
    fn a_two_verifier_module_names_the_route_each_webhook_rule_blocks() {
        // Issue #595: `billing` verifies `POST /webhooks/stripe` through
        // `Payments` and `POST /webhooks/revenuecat` with its own HMAC
        // secret, so each missing secret must name the mounted route it
        // blocks — `POST /v1/billing/webhooks/...`.
        use cratefield_core::MapConfig;
        let message = |pairs: &[(String, String)], code: &str| -> Option<String> {
            pairs
                .iter()
                .find(|(found, _)| found == code)
                .map(|(_, message)| message.clone())
        };
        let two = || std::sync::Arc::new(super::tests::TwoVerifiers);
        let stripe_set = Configured {
            stripe_webhook_secret: true,
            ..NONE_SET
        };
        let secret = MapConfig::from_pairs([("BILLING_REVENUECAT_WEBHOOK_SECRET", "whsec-dummy")]);

        // Both secrets present: neither webhook rule fires.
        let both = messages(two(), stripe_set, &secret);
        assert!(message(&both, "payments-webhook-secret-missing").is_none());
        assert!(message(&both, "hmac-webhook-secret-missing").is_none());

        // Stripe's secret missing: the payments rule names its route and
        // says nothing about the HMAC one.
        let no_stripe = messages(two(), NONE_SET, &secret);
        let payment = message(&no_stripe, "payments-webhook-secret-missing").expect("fires");
        assert!(
            payment.contains("POST /v1/billing/webhooks/stripe"),
            "{payment}"
        );
        assert!(
            !payment.contains("POST /v1/billing/webhooks/revenuecat"),
            "{payment}"
        );

        // RevenueCat's secret missing: the HMAC rule names its route.
        let no_revenuecat = messages(two(), stripe_set, &MapConfig::default());
        let hmac = message(&no_revenuecat, "hmac-webhook-secret-missing").expect("fires");
        assert!(
            hmac.contains("POST /v1/billing/webhooks/revenuecat"),
            "{hmac}"
        );
        assert!(!hmac.contains("POST /v1/billing/webhooks/stripe"), "{hmac}");
    }

    /// Runs the whole doctor — the path `fz doctor` actually takes — so a
    /// deleted call to the admin-token rule fails here instead of leaving
    /// the pure tests green. The module declares nothing, the migrations
    /// directory does not exist (`read_lock` treats that as an empty
    /// lock), and no sidecars are mounted: the report is empty unless the
    /// rule under test fires.
    fn run_report(configured: Configured) -> Vec<String> {
        let harness = harness_of(std::sync::Arc::new(Nothing));
        run_checks(
            &harness,
            std::path::Path::new("no-migrations-here"),
            None,
            None,
            Output::Json,
            configured,
        )
        .failures
        .iter()
        .map(|failure: &DoctorFailure| failure.code.code.to_owned())
        .collect()
    }

    #[test]
    fn a_short_admin_token_reaches_the_report() {
        let configured = Configured {
            admin_token_bytes: Some(4),
            ..NONE_SET
        };
        assert_eq!(
            run_report(configured),
            vec![CODES.admin_token_too_short.code.to_owned()],
            "the rule is correct and nothing calls it"
        );
    }

    #[test]
    fn an_absent_admin_token_leaves_the_report_clean() {
        assert!(
            run_report(NONE_SET).is_empty(),
            "{:?}",
            run_report(NONE_SET)
        );
    }
}