car_server_core/coder/

heal_service.rs

//! The thing that actually runs the self-healing loop.
//!
//! T7 of `docs/proposals/self-healing-issue-loop.md`. Until this existed,
//! [`super::heal_tick::tick`] had no caller — a working half, not a feature.
//!
//! ## A daemon subsystem, not a scheduler task
//!
//! The proposal originally said `scheduler.os_install`, arguing against a
//! Claude Code `/loop` because a `/loop` dies with its session. True, and
//! beside the point: a daemon subsystem does not have that problem, because
//! the daemon *is* the durable process. `car-scheduler`'s tasks are either
//! prompt-driven through an agent runner or a shell `CommandSpec`, and this
//! tick is neither — it is deterministic Rust composition that needs
//! `coder.start`, so a shell task would mean re-entering by subprocess to
//! reach a process we were already inside.
//!
//! The precedent this follows is [`crate::selfheal`]: a service on
//! `ServerState`, a cadence spawner, an interval with an env override, and a
//! manual-run RPC.
//!
//! ## Three things the reviews said this had to settle
//!
//! **Who persists the ledger.** This service does, around every tick, before
//! and after. The claim must reach disk *before* the coder session starts, or a
//! crash mid-session loses it and the next start re-picks an item whose work is
//! already in flight.
//!
//! **What `run_id` is.** A fresh id per tick. A stable one would make every
//! tick the holder of every claim it ever took — `claim()` treats a matching
//! run id as a refresh — and the mechanism would silently do nothing.
//!
//! **Overlap.** `CLAIM_TTL_MS` is 90 minutes and the cadence is shorter, so
//! ticks *will* collide. This try-locks and skips rather than queueing: a
//! queue of ticks against one repository is just a slower way to do the same
//! work twice.

use std::sync::Arc;

use tokio::sync::Mutex;

use super::heal_claims::ClaimStore;
use super::heal_config::HealConfig;
use super::heal_config::HEAL_CONFIG_FILE;
use super::heal_tick::{tick, ClaimSink, TickIo, TickOutcome};
use crate::session::ServerState;

/// The engine the loop uses when the configuration does not name one.
///
/// `foreman` rather than `auto`: the work is unattended, so decomposing an
/// intent and gating each patch plus the integrated union is worth more here
/// than it is in an interactive session someone is watching. Foreman declines
/// to single-session when the plan has no parallelism, so this is not a cost on
/// small fixes.
pub const DEFAULT_ENGINE: &str = "foreman";

/// Env override for the cadence, mirroring `CAR_SELFHEAL_INTERVAL_SECS`.
pub const HEAL_INTERVAL_ENV: &str = "CAR_HEAL_INTERVAL_SECS";

/// Default cadence. Deliberately unhurried: the queue is human-authored, so
/// polling faster mostly means asking GitHub the same question more often.
pub const DEFAULT_INTERVAL_SECS: u64 = 15 * 60;

/// The loop's state inside the daemon.
pub struct HealService {
    /// The configuration as of the last read. Behind a lock because the file is
    /// re-read every sweep: an operator who fixes a typo'd repo spec and reruns
    /// `car heal status` must not be shown the boot-time config presented as
    /// current, and enabling the loop must not require a daemon restart.
    ///
    /// Re-reading is one small TOML at a 15-minute cadence. The alternative —
    /// read once at `ServerState` construction — made "the loop never does
    /// anything" indistinguishable from "your edit has not been picked up",
    /// which is the failure mode this whole file's diagnostics exist to
    /// prevent.
    config: std::sync::RwLock<HealConfig>,
    /// Where `heal.toml` lives, so a sweep can re-read it.
    ///
    /// `None` means the configuration was supplied directly and there is no
    /// file behind it: re-reading would discard what the caller passed in, so
    /// this service simply never reloads.
    config_dir: Option<std::path::PathBuf>,
    /// Held across a whole tick. See the module docs: try-lock and skip.
    running: Mutex<()>,
    state_dir: std::path::PathBuf,
    interval_secs: u64,
    /// Why the boot-time assembly failed, when it did.
    ///
    /// `is_enabled` reads the config; assembly additionally validates the model
    /// names, refuses a coder that sits on its own panel, and requires at least
    /// two serving providers. A configuration
    /// that passes the first and fails the second leaves the loop enabled and
    /// dead — which is precisely the state `heal.status` exists to make
    /// impossible ("an idle loop and a misconfigured one are indistinguishable
    /// from outside"). Recorded here so `disabled_reason` can say it.
    assembly_error: std::sync::RwLock<Option<String>>,
    /// Why the most recent MANUAL `heal.run` refused to assemble, cleared when
    /// one succeeds.
    ///
    /// Separate from `assembly_error` because the two have different lifetimes
    /// and a single slot gets both wrong. The cadence assembles ONCE at boot
    /// and does not start on failure, so its error is true until the daemon
    /// restarts — sticky is correct. `run_tick` re-assembles per call, so its
    /// failure is a statement about the config as it stands now. Writing that
    /// into the boot slot made a healthy, sweeping cadence report
    /// `disabled_reason` permanently, which is the same lie this field exists
    /// to prevent, pointed the other way.
    run_refusal: std::sync::RwLock<Option<String>>,
}

/// What one pass over every configured target did.
#[derive(Debug, Clone, Default, PartialEq, Eq, serde::Serialize)]
pub struct SweepReport {
    pub outcomes: Vec<(String, TickOutcome)>,
    /// True when another sweep held the lock and this one stood down.
    pub skipped_overlap: bool,
}

/// What `heal.status` reports.
///
/// Carries `disabled_reason` and `rejected` because a loop that is idle and a
/// loop that is misconfigured look identical from outside, and on first setup
/// the second is the common case. "It never does anything" is the hardest
/// failure to notice in a subsystem whose normal state is doing nothing.
#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)]
pub struct HealStatus {
    /// Where the configuration was read from, so an operator editing the wrong
    /// file finds out from `heal.status` instead of from silence.
    pub config_path: String,
    pub enabled: bool,
    pub cadence_secs: u64,
    /// Why the loop will not run, when it is configured but disabled.
    pub disabled_reason: Option<String>,
    /// `owner/name` per configured target.
    pub targets: Vec<String>,
    /// Targets that were named but could not be used, and why.
    pub rejected: Vec<RejectedTargetView>,
    /// The review panel, by model id.
    pub review_models: Vec<String>,
    /// The panel with each seat resolved to the vendor that serves it, after
    /// the same deduplication the live panel applies.
    pub panel: Vec<super::heal_review::PanelSeat>,
    /// Why a panel that clears the two-provider construction floor still
    /// cannot be shown to be independent of itself — see `correlation_warning`.
    pub panel_warning: Option<String>,
    /// Why the most recent manual `heal.run` refused to assemble, if one did
    /// and none has succeeded since.
    ///
    /// NOT `disabled_reason`: a cadence that assembled cleanly at boot keeps
    /// sweeping on its boot-time io, so a manual refusal says the config AS IT
    /// STANDS NOW would not assemble — a warning about the next state, not a
    /// statement that the loop is off.
    pub run_refusal: Option<String>,
    /// The model the coder will run on, and which file pinned it.
    ///
    /// Two states, not three. `None` means unpinned — neither `heal.toml`'s
    /// `coder_model` nor `coder.toml`'s `[coder] model` names one, so adaptive
    /// routing picks per request and nothing before the run can say what it
    /// will pick. ("Pinned to something the panel rejects" is a
    /// `disabled_reason` state, not one of this field's.)
    ///
    /// **What the RUNNING cadence uses may differ.** `spawn_heal_cadence`
    /// calls `live_io` once at boot and bakes the pin into the runner, so this
    /// reports the current files while a cadence started earlier keeps using
    /// the boot-time value — the same freeze `panel_composition` documents for
    /// the panel. Read it as what the next `heal.run` would use.
    ///
    /// Reported because car#1334 made this pin load-bearing: the panel
    /// independence check refuses a sweep when the coder is also a review seat,
    /// and the pin it checks lives in whichever of two files won. Deriving that
    /// by hand means reading both and re-implementing the precedence — which is
    /// exactly the re-derivation that produced car#1360.
    pub coder_pin: Option<CoderPinView>,
    pub engine: String,
}

/// The resolved coder pin, as `heal.status` reports it.
#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)]
pub struct CoderPinView {
    /// The model id, verbatim — not canonicalized. A `coder.toml` value is
    /// legitimately outside CAR's registry (on the external rung it is
    /// forwarded to a third-party CLI's own namespace), so showing the
    /// operator what the file says is the useful answer.
    pub model: String,
    /// Which file won, as a machine value: `heal_toml` or `coder_toml`.
    ///
    /// Not prose. Every non-CLI consumer — a host app, a script, a human
    /// reading raw JSON — would otherwise have to strip markdown backticks out
    /// of a JSON-RPC field; the operator-facing wording is the renderer's job.
    pub source: String,
}

#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)]
pub struct RejectedTargetView {
    pub repo: String,
    pub reason: String,
}

impl HealService {
    /// Construct with a config already loaded, and the directory it came from.
    pub fn with_config_dir(
        config: HealConfig,
        state_dir: std::path::PathBuf,
        config_dir: std::path::PathBuf,
    ) -> Self {
        let mut s = Self::new(config, state_dir);
        s.config_dir = Some(config_dir);
        s
    }

    /// Re-read `heal.toml`, returning the current configuration.
    ///
    /// A read failure is not distinguishable from an absent file by design
    /// (`HealConfig::load` returns an empty config either way), which is why
    /// the empty case disables the loop rather than doing anything.
    fn reload(&self) -> HealConfig {
        // No file behind this service: the held config IS the configuration,
        // and re-reading would throw away what the caller supplied.
        let Some(dir) = &self.config_dir else {
            return self.held();
        };
        let fresh = HealConfig::load(dir);
        if let Ok(mut held) = self.config.write() {
            *held = fresh.clone();
        }
        fresh
    }

    /// The configuration as last read, without touching the disk.
    fn held(&self) -> HealConfig {
        match self.config.read() {
            Ok(c) => c.clone(),
            // A poisoned lock means a panic while holding it. An empty config
            // disables the loop, which is the safe answer when the state that
            // says what to do cannot be trusted.
            Err(_) => HealConfig::default(),
        }
    }

    fn config(&self) -> HealConfig {
        self.held()
    }

    pub fn new(config: HealConfig, state_dir: std::path::PathBuf) -> Self {
        let interval_secs = std::env::var(HEAL_INTERVAL_ENV)
            .ok()
            .and_then(|v| v.parse::<u64>().ok())
            .filter(|v| *v > 0)
            .unwrap_or(DEFAULT_INTERVAL_SECS);
        Self {
            config: std::sync::RwLock::new(config),
            config_dir: None,
            running: Mutex::new(()),
            state_dir,
            interval_secs,
            assembly_error: std::sync::RwLock::new(None),
            run_refusal: std::sync::RwLock::new(None),
        }
    }

    /// Record why the loop could not be assembled, so `heal.status` can report
    /// an enabled-but-dead loop instead of leaving it to a boot log line.
    pub fn record_assembly_error(&self, error: &str) {
        if let Ok(mut slot) = self.assembly_error.write() {
            *slot = Some(error.to_string());
        }
    }

    /// The coder pin in force, resolved ONCE for both `status` and
    /// `live_io_for`.
    ///
    /// Used by both so the two cannot disagree — the same reason
    /// `panel_composition` exists. `config::session_model` is the single copy
    /// of the PRECEDENCE rule; this is the single copy of "read `coder.toml`
    /// only when `heal.toml` does not pin", which is a second rule and was
    /// briefly a second copy.
    ///
    /// `coder.toml` is read only when it can matter: a malformed one should not
    /// warn on every sweep for a value `heal.toml` overrides. Its unreadable,
    /// malformed and absent cases all collapse to an empty config, so all three
    /// read as unpinned here.
    fn resolved_coder_pin(config: &HealConfig) -> Option<(String, super::config::PinSource)> {
        let coder_toml = config
            .coder_model
            .is_none()
            .then(super::config::CoderConfig::load);
        super::config::session_model(
            config.coder_model.as_deref(),
            coder_toml.as_ref().and_then(|c| c.model.as_deref()),
        )
        .map(|(m, src)| (m.to_string(), src))
    }

    /// Record why a manual `heal.run` refused, or clear it on a run that got
    /// past assembly. Both arms matter: without the clearing one an operator
    /// who fixes the config keeps seeing the old refusal until they restart the
    /// daemon.
    fn set_run_refusal(&self, error: Option<&str>) {
        if let Ok(mut slot) = self.run_refusal.write() {
            *slot = error.map(str::to_string);
        }
    }

    pub fn is_enabled(&self) -> bool {
        self.config().is_enabled()
    }

    pub fn interval_secs(&self) -> u64 {
        self.interval_secs
    }

    /// Every target that could not be used, so a misconfiguration is visible
    /// rather than presenting as a loop that never does anything.
    pub fn rejected(&self) -> Vec<super::heal_config::RejectedTarget> {
        self.config().rejected
    }

    /// What the loop is configured to do, and why it is not doing it.
    pub fn status(&self, engine: &Arc<car_inference::InferenceEngine>) -> HealStatus {
        // Re-read, so an operator who just fixed `heal.toml` is shown what the
        // next sweep will actually use rather than what booted.
        let config = self.reload();
        let panel = self.panel_composition(engine, &config.review_models);
        let coder_pin = Self::resolved_coder_pin(&config);
        HealStatus {
            config_path: self
                .config_dir
                .as_ref()
                .map(|d| d.join(HEAL_CONFIG_FILE).display().to_string())
                .unwrap_or_else(|| "(supplied directly; no file)".into()),
            enabled: config.is_enabled(),
            cadence_secs: self.interval_secs,
            // Config first (it names a missing panel or missing targets), then
            // the assembly failure — which is the only one that can leave
            // `enabled` true.
            disabled_reason: config.disabled_reason().map(str::to_string).or_else(|| {
                self.assembly_error
                    .read()
                    .ok()
                    .and_then(|e| e.clone())
                    .map(|e| format!("the loop could not be assembled: {e}"))
            }),
            run_refusal: self.run_refusal.read().ok().and_then(|e| e.clone()),
            targets: config.targets.iter().map(|t| t.repo.clone()).collect(),
            rejected: config
                .rejected
                .iter()
                .map(|r| RejectedTargetView {
                    repo: r.repo.clone(),
                    reason: r.reason.clone(),
                })
                .collect(),
            review_models: config.review_models.clone(),
            // Composition, not just names: a single-vendor panel is invisible in
            // a list of model ids, and the operator reading `3/3 approved` is
            // the person who needs to know it was one vendor three times.
            panel: panel.clone(),
            panel_warning: super::heal_review::correlation_warning(&panel),
            // Resolved through the same `session_model` the sweep's check uses,
            // not a second copy of the precedence rule. `coder.toml` is read
            // only when `heal.toml` does not pin, matching the sweep — an
            // unreadable `coder.toml` that nothing consults must not turn into
            // a status field that disagrees with what will run.
            coder_pin: coder_pin.map(|(model, source)| CoderPinView {
                model,
                // Named for the FILE, not for `PinSource`'s generic
                // request/config spelling: heal passes `heal.toml`'s
                // `coder_model` as the request pin, so "request" would tell an
                // operator nothing about which file to open.
                source: match source {
                    super::config::PinSource::Request => "heal_toml",
                    super::config::PinSource::Config => "coder_toml",
                }
                .to_string(),
            }),
            engine: config
                .engine
                .clone()
                .unwrap_or_else(|| DEFAULT_ENGINE.to_string()),
        }
    }

    /// Resolve each configured seat to its serving vendor.
    ///
    /// Used by both `status` and `live_io_for` so the two cannot disagree about
    /// how a seat resolves. They can still describe different PANELS: `status`
    /// re-reads `heal.toml` every call, while the cadence's `TickIo` is
    /// assembled once at boot and reused, so an edit shows up in `heal.status`
    /// before the running loop picks it up (it takes a restart). That freeze
    /// predates this and is not what this function is claiming to fix.
    fn panel_composition(
        &self,
        engine: &std::sync::Arc<car_inference::InferenceEngine>,
        models: &[String],
    ) -> Vec<super::heal_review::PanelSeat> {
        super::heal_review::composition(models, |m| {
            engine
                .model_schema(m)
                .and_then(|s| s.vendor())
                .map(str::to_string)
        })
    }

    /// Assemble the live loop: real GitHub, real coder, real review panel.
    ///
    /// **One constructor, one call site.** Every dependency below already
    /// existed and none of them were ever wired together — `LiveTickIo` had no
    /// construction site in the workspace at all, so the loop was a complete,
    /// tested, unreachable subsystem. A second assembly point (a CLI building
    /// its own in-process) would be the next copy of that mistake, and would
    /// also bypass the single-sweep try-lock, so `heal.run` and the cadence
    /// both come through here.
    pub fn live_io(&self, state: &Arc<ServerState>) -> Result<Arc<dyn TickIo>, String> {
        self.live_io_for(state, &self.config())
    }

    fn live_io_for(
        &self,
        state: &Arc<ServerState>,
        config: &HealConfig,
    ) -> Result<Arc<dyn TickIo>, String> {
        if config.review_models.is_empty() {
            // `decide` refuses a panel of zero, so continuing would run a full
            // coder session per item and reject every one of them.
            return Err("no `review_models` configured; refusing to run without a panel".into());
        }
        // Validate the panel HERE, where the catalog is reachable, rather than
        // discovering a typo after a full coder session: an unknown id errors
        // at generation, which the gate reads as an unreachable seat, so a
        // misspelled model costs a session and a backoff per tick before
        // anyone learns why. Refusing is not the same as dropping the seat — a
        // panel silently shrunk from three to two is the halved threshold the
        // `PanelIncomplete` design exists to prevent.
        //
        // `knows_model`, not `list_models`: the latter is the ON-DEVICE
        // catalog, so checking a cloud model id against it reports every
        // frontier model as unknown and would refuse a correct configuration —
        // a false refusal disables the whole feature, which is worse than the
        // cost it was added to avoid.
        let engine_handle = crate::handler::get_inference_engine(state);
        // The coder is validated with the seats, not after them: the argument
        // above applies with more force to the model that runs the EXPENSIVE
        // half, and an unvalidated coder name is also what would force the
        // disjointness check below to guess at a canonical form.
        let unknown: Vec<&str> = config
            .review_models
            .iter()
            .chain(config.coder_model.iter())
            .filter(|m| !engine_handle.knows_model(m))
            .map(String::as_str)
            .collect();
        if !unknown.is_empty() {
            return Err(format!(
                "unknown model(s) in `heal.toml`: {} — a seat that cannot be reached is \
                 not a reviewer, and dropping it would quietly shrink the panel. Run \
                 `car models list` for the names this daemon knows.",
                unknown.join(", ")
            ));
        }

        // Which model the coder will run on, resolved ONCE. `coder.start`
        // takes `coder_model` as its request pin and falls back to
        // `~/.car/coder.toml`'s `[coder] model`, so an unset `coder_model` does
        // not mean the coder is unpinned — it means it runs on that file's
        // pin, which nothing checked against the panel (car#1360). Read only
        // when it can matter; a malformed coder.toml should not warn on every
        // sweep for a value heal.toml overrides.
        let coder_pin = Self::resolved_coder_pin(config);

        // Refused rather than dropped: a seat quietly removed lowers the
        // majority threshold without saying so, which is what `PanelIncomplete`
        // exists to prevent. The rule and its message live in
        // `check_coder_pin`, where they are table-tested; the registry reaches
        // it as the one closure.
        //
        // Every SEAT cleared `knows_model` above, so seats always canonicalize
        // to a real id — but the coder pin may not have, because a coder.toml
        // value is legitimately outside CAR's registry: on the external rung it
        // is forwarded verbatim to a third-party CLI's own namespace. That is
        // what `canonicalizer`'s fallback is for.
        check_coder_pin(
            coder_pin.as_ref().map(|(m, src)| (m.as_str(), *src)),
            &config.review_models,
            &canonicalizer(|m| engine_handle.model_schema(m).map(|s| s.id.clone())),
        )?;

        let engine = match config.engine.as_deref() {
            Some(name) => super::router::EngineChoice::parse(name)?,
            None => super::router::EngineChoice::parse(DEFAULT_ENGINE)?,
        };

        // The minimum diversity floor is enforced here, after every seat has a
        // catalog-backed serving vendor. Refuse rather than dropping seats: a
        // smaller panel has a different majority threshold and is a different
        // configuration. The stronger majority-capture condition remains a
        // warning for panels that do span two providers but can still be
        // carried by one of them.
        let panel = self.panel_composition(engine_handle, &config.review_models);
        if let Some(error) = super::heal_review::panel_diversity_error(&panel) {
            return Err(error);
        }
        if let Some(warning) = super::heal_review::correlation_warning(&panel) {
            tracing::warn!(target: "car::heal", "{warning}");
        }

        // Adaptive inference reports `ModelSchema.name`, so pass the panel in
        // that same namespace. `build_exclude_set` resolves either names or ids,
        // but using the result namespace here also makes the list directly
        // comparable with the durable author attribution the delivery backstop
        // reads. Validation above guarantees every seat resolves.
        let mut routing_exclusions = Vec::new();
        for seat in &config.review_models {
            if let Some(name) = engine_handle.model_schema(seat).map(|schema| &schema.name) {
                if !routing_exclusions.contains(name) {
                    routing_exclusions.push(name.clone());
                }
            }
        }

        let runner = super::heal_runner::LiveCoderRunner {
            state: state.clone(),
            // The production `TurnGenerator` is the inference engine itself.
            generator: crate::handler::get_inference_engine(state).clone(),
            state_dir: self.state_dir.clone(),
            reviewers: super::heal_review::panel(state, &config.review_models),
            max_wall_secs: super::heal_runner::DEFAULT_ITEM_WALL_SECS,
            max_iterations: None,
            engine,
            // The value that was CHECKED above, not `coder_model` — otherwise
            // `coder.start` re-reads coder.toml at session time and the pin
            // the runner ends up on is not the one the panel check saw.
            model: coder_pin.map(|(m, _)| m),
            routing_exclusions,
            // The registry resolver the gate's self-review check canonicalizes
            // through, on the same fallback policy as the assembly check above
            // — and it matters more here, because the gate's author comes from
            // the engine unvalidated.
            canonical_model: {
                let engine_handle = engine_handle.clone();
                Arc::new(canonicalizer(move |m: &str| {
                    engine_handle.model_schema(m).map(|s| s.id.clone())
                }))
            },
            github: Arc::new(super::merge::GhCli::default()),
        };

        Ok(Arc::new(super::heal_live::LiveTickIo {
            issues: Arc::new(super::fix_issues::GhIssues),
            prs: Arc::new(super::heal_intake::GhPullRequests),
            oracle: Arc::new(super::provenance::GhPermissions),
            coder: Arc::new(runner),
            // EMPTY, deliberately, and it narrows what the loop will act on.
            //
            // `LocalSignatures` holds the signatures of issues this runtime
            // itself filed, and it is the only route to the `Runtime` tier —
            // the one tier that may source an outcome contract from an issue
            // body. Nothing in the daemon files issues today (`car-selfheal` is
            // watch-only by design), so there are no such signatures to supply
            // and inventing a non-empty set would be asserting authorship this
            // process cannot demonstrate.
            //
            // The consequence, stated rather than discovered: every item the
            // loop acts on is maintainer-authored or better, and no issue body
            // ever sources a contract. That is the safer posture, and it is the
            // one the proposal asks be revisited *before* runtime filing is
            // enabled — not after.
            local_signatures: super::provenance::LocalSignatures::from_proposals(&[]),
            redactor: car_selfheal::redact::Redactor::from_env(std::env::vars()),
            panel,
        }))
    }

    /// One sweep, assembling the live loop first. The manual-run entry point.
    pub async fn run_tick(&self, state: &Arc<ServerState>) -> Result<SweepReport, String> {
        // Re-read first: a manual run is exactly when an operator has just
        // edited the file.
        let config = self.reload();
        // Recorded, not just returned. `spawn_heal_cadence` records its
        // assembly failure so a boot-time refusal still reaches `heal.status` a
        // week later; a manual `heal.run` refusing for the same reason left no
        // trace, so `heal.status` could show a config `heal.run` will not run
        // on. The caller still gets the error — this only stops it from being
        // the ONLY place it appears.
        //
        // A `match` rather than `inspect_err`: this needs a SUCCESS arm. The
        // slot has to be cleared by a run that assembles, or an operator who
        // fixes the config keeps reading the old refusal forever.
        let io = match self.live_io_for(state, &config) {
            Ok(io) => {
                self.set_run_refusal(None);
                io
            }
            Err(e) => {
                self.set_run_refusal(Some(&e));
                return Err(e);
            }
        };
        Ok(self.sweep_with(&io, &config).await)
    }

    /// One pass over every configured target, at most one item each.
    ///
    /// Every target, not "the first with work": stopping early would let a busy
    /// repository starve every entry after it in the config, and the operator
    /// who listed them has no way to see that happening.
    pub async fn sweep(&self, io: &Arc<dyn TickIo>) -> SweepReport {
        let config = self.reload();
        self.sweep_with(io, &config).await
    }

    async fn sweep_with(&self, io: &Arc<dyn TickIo>, config: &HealConfig) -> SweepReport {
        let Ok(_guard) = self.running.try_lock() else {
            // A tick is already running. Queueing would just do the same work
            // twice, more slowly.
            return SweepReport {
                outcomes: Vec::new(),
                skipped_overlap: true,
            };
        };

        let mut claims = ClaimStore::load(&self.state_dir);
        let mut outcomes = Vec::new();
        let sink = FileClaimSink {
            dir: self.state_dir.clone(),
        };

        for target in &config.targets {
            // A fresh id per tick. A stable one would make this run the holder
            // of every claim it ever took, and claiming would silently do
            // nothing.
            let run_id = format!("heal-{}", uuid::Uuid::new_v4().simple());
            let out = tick(io, target, &mut claims, &run_id, &sink).await;

            // Again after the target, to record the outcome — the CLAIM
            // already reached disk inside `tick`, before any work started.
            // Saving only here lost it whenever the daemon died during the
            // 45-minute session the claim was taken for, and the next start
            // re-picked an item whose work was still in flight.
            sink.persist(&claims, io.now_ms());
            outcomes.push((target.repo.clone(), out));
        }

        SweepReport {
            outcomes,
            skipped_overlap: false,
        }
    }
}

// There is deliberately NO worktree reaper here.
//
// There was one, and it was worse than nothing: it swept
// `<state_dir>/heal-worktrees`, a directory nothing has ever created. Coder
// sessions provision under `<state_dir>/worktrees` (`CoderSession::
// provision_workspace`). So the guard whose rationale was the 102 GB incident
// scanned an empty path, always returned 0, and logged nothing — a
// disk-exhaustion protection that reported success while protecting nothing.
//
// Repointing it at `worktrees` would have been worse still: that directory is
// shared with every *interactive* `coder.start`, so an age-based sweep would
// delete a human's overnight `NeedsApproval` worktree.
//
// The real fix was upstream. `LiveCoderRunner` now drives every session it
// starts to a terminal state — `Merged` on delivery, `Abandoned` on rejection,
// `Failed` on timeout — and a terminal transition drops the `AgentWorkspace`
// RAII handle, which is what removes the worktree and its `git worktree`
// registration. Sessions clean up after themselves; nothing has to sweep.

/// Writes the claim ledger to the loop's state directory.
///
/// A failure is logged, not propagated: a disk that refuses the write must not
/// fail a sweep that is otherwise fine, and the in-memory ledger still holds
/// the claim for the rest of this sweep.
struct FileClaimSink {
    dir: std::path::PathBuf,
}

impl ClaimSink for FileClaimSink {
    fn persist(&self, claims: &ClaimStore, now_ms: u64) {
        if let Err(e) = claims.save(&self.dir, now_ms) {
            tracing::warn!(error = %e, "could not persist heal claims");
        }
    }
}

/// Run the loop on its cadence until the daemon stops.
///
/// Returns `None` when no targets are configured, so a daemon with no
/// `heal.toml` spawns nothing at all rather than a task that wakes to do
/// nothing forever.
pub fn spawn_heal_cadence(state: Arc<ServerState>) -> Option<tokio::task::JoinHandle<()>> {
    let service = state.heal.clone();
    if !service.is_enabled() {
        // Named, not silent. `disabled_reason` distinguishes "nothing is
        // configured" from "targets are configured but there is no review
        // panel", and the second reads exactly like the first from a log that
        // only says "not starting".
        tracing::info!(
            reason = service
                .config()
                .disabled_reason()
                .unwrap_or("no targets configured"),
            "self-healing loop not started"
        );
        return None;
    }
    for r in service.rejected() {
        tracing::warn!(repo = %r.repo, reason = %r.reason, "heal target ignored");
    }

    // Assemble ONCE, at startup, so a missing credential or an unparseable
    // engine name is a boot-time error in the log rather than a failure
    // rediscovered on every tick forever.
    let io = match service.live_io(&state) {
        Ok(io) => io,
        Err(e) => {
            // Recorded before returning: a log line at boot is not reachable by
            // the operator who runs `car heal status` a week later.
            service.record_assembly_error(&e);
            tracing::warn!(error = %e, "self-healing loop could not be assembled; not starting");
            return None;
        }
    };
    let secs = service.interval_secs();
    tracing::info!(interval_secs = secs, "self-healing loop started");
    Some(tokio::spawn(async move {
        let mut ticker = tokio::time::interval(std::time::Duration::from_secs(secs));
        // The first tick fires immediately; skip it so a daemon restart does
        // not start a coder session before the operator has seen it come up.
        ticker.tick().await;
        loop {
            ticker.tick().await;
            let report = service.sweep(&io).await;
            if report.skipped_overlap {
                tracing::debug!("heal sweep skipped: previous sweep still running");
                continue;
            }
            for (repo, out) in &report.outcomes {
                match out {
                    TickOutcome::Opened {
                        number,
                        pr_url,
                        ci,
                        delivery,
                        ..
                    } => tracing::info!(
                        %repo,
                        number,
                        %pr_url,
                        head_sha = %ci.head_sha,
                        ci_state = ?ci.state,
                        %delivery,
                        "self-heal opened a pull request"
                    ),
                    TickOutcome::Rejected { number, gate, .. } => {
                        tracing::info!(%repo, number, %gate, "self-heal stopped at the gate")
                    }
                    TickOutcome::Failed { detail } => {
                        tracing::warn!(%repo, %detail, "self-heal tick failed")
                    }
                    TickOutcome::Idle { .. } => {}
                }
            }
        }
    }))
}

/// The registry canonicalizer both self-review checks compare through.
///
/// `unwrap_or_else(|| m.to_string())`, never `unwrap_or_default()`. Two names
/// the registry cannot resolve would both collapse to `""` and match each
/// other, and here that means refusing every session on a daemon whose catalog
/// does not hold the model that ran. Falling back to the name itself degrades
/// to a spelling comparison instead — the honest answer when the registry has
/// nothing to say. Written once so the policy has one place to be wrong.
fn canonicalizer(resolve: impl Fn(&str) -> Option<String>) -> impl Fn(&str) -> String {
    move |m| resolve(m).unwrap_or_else(|| m.to_string())
}

/// Refuse an assembly where the model that will WRITE the change also sits on
/// the panel that will judge it.
///
/// Takes the pin already resolved — `(model, source)` from
/// [`config::session_model`](super::config::session_model) — rather than the
/// two raw sources. Two adjacent `Option<&str>` parameters can be transposed
/// silently, and getting the precedence backwards is the defect this function
/// exists to fix; there is nothing to transpose here. The source rides along so
/// the message can name the file the operator has to edit, without this
/// re-deriving precedence from its own copy of the rule.
///
/// Refused rather than dropping the seat: a panel silently shrunk from three
/// to two is the halved threshold `PanelIncomplete` exists to prevent. Two
/// cases are deliberately NOT refused here:
///
/// - **An unpinned coder** (neither file pins, the default). The router picks
///   per request, so only `heal_runner`'s gate, reading what actually authored
///   the change, can answer it (car#1299).
/// - **A pin this daemon's registry does not know.** heal validates its own
///   `coder_model` against the catalog, but `coder.toml` is a shared file no
///   other consumer validates, and on the external rung its value is forwarded
///   verbatim to a CLI's own namespace — so a name `car models list` has never
///   heard of can be exactly right for the rung that runs. Refusing would take
///   the whole loop offline over a working configuration, which is the false
///   refusal `correlation_warning` and the `knows_model` comment upstream both
///   decline to make.
fn check_coder_pin(
    pin: Option<(&str, super::config::PinSource)>,
    seats: &[String],
    canonical: &dyn Fn(&str) -> String,
) -> Result<(), String> {
    let Some((coder, source)) = pin else {
        return Ok(());
    };
    let Some(seat) = super::heal_review::coder_on_panel(coder, seats, canonical) else {
        return Ok(());
    };
    Err(format!(
        "the coder model {} ({}) is also a review seat ({}) — a model cannot review its \
         own output, and counting it as a reviewer reports an independence the panel does \
         not have. Remove it from `review_models`, or pin a different coder.",
        coder,
        match source {
            super::config::PinSource::Request => "`coder_model` in `heal.toml`",
            super::config::PinSource::Config => "`model` in `coder.toml`",
        },
        seat
    ))
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::coder::heal_intake::{Checkout, HealTarget};

    struct NoopIo;

    #[async_trait::async_trait]
    impl TickIo for NoopIo {
        async fn candidates(
            &self,
            _t: &HealTarget,
        ) -> Result<Vec<crate::coder::heal_select::Candidate>, String> {
            Ok(vec![])
        }
        async fn open_prs(
            &self,
            _t: &HealTarget,
        ) -> Result<Vec<crate::coder::heal_intake::RawPullRequest>, String> {
            Ok(vec![])
        }
        async fn intent_for(
            &self,
            _i: &crate::coder::heal_select::Candidate,
        ) -> Result<crate::coder::heal_tick::Intent, String> {
            Ok(crate::coder::heal_tick::Intent::Gone)
        }
        fn redact(&self, text: &str) -> String {
            text.to_string()
        }
        async fn run_coder(
            &self,
            _t: &HealTarget,
            _i: &crate::coder::heal_select::Candidate,
            _s: &crate::coder::provenance::SessionSeed,
        ) -> Result<crate::coder::heal_tick::Attempt, crate::coder::heal_tick::RunFailure> {
            Err(crate::coder::heal_tick::RunFailure::early("not reached"))
        }
        async fn deliver(
            &self,
            _t: &HealTarget,
            _i: &crate::coder::heal_select::Candidate,
            _s: &str,
            _g: &crate::coder::heal_gate::GateOutcome,
        ) -> Result<crate::coder::merge::PrDeliveryOutcome, crate::coder::heal_tick::DeliverRefusal>
        {
            Err(crate::coder::heal_tick::DeliverRefusal::retriable(
                "not reached",
            ))
        }
        async fn abandon(&self, _s: &str) {}
        async fn comment(
            &self,
            _i: &crate::coder::heal_select::Candidate,
            _t: &str,
        ) -> Result<(), String> {
            Ok(())
        }
        fn now_ms(&self) -> u64 {
            1_000_000
        }
    }

    fn cfg(targets: Vec<HealTarget>) -> HealConfig {
        HealConfig {
            targets,
            rejected: vec![],
            review_models: vec!["reviewer-a".into()],
            engine: None,
            coder_model: None,
        }
    }

    fn target(repo: &str) -> HealTarget {
        HealTarget {
            repo: repo.into(),
            fix_repo: None,
            checkout: Some(Checkout::Project("p".into())),
            label: "self-heal".into(),
            base: "main".into(),
        }
    }

    /// `status` resolves each seat's vendor through the registry, so it needs
    /// the engine — and only the engine.
    fn engine() -> Arc<car_inference::InferenceEngine> {
        Arc::new(car_inference::InferenceEngine::new(Default::default()))
    }

    /// A daemon with no `heal.toml` spawns nothing at all — not a task that
    /// wakes every fifteen minutes to find no targets.
    #[test]
    fn no_targets_means_the_loop_is_disabled() {
        let dir = tempfile::tempdir().unwrap();
        let s = HealService::new(cfg(vec![]), dir.path().into());
        assert!(!s.is_enabled());
        assert_eq!(
            s.status(&engine()).disabled_reason.as_deref(),
            Some("no usable targets are configured")
        );
    }

    /// A registry stand-in with the car#889 hazard in it: `alias` is a name
    /// whose canonical id is something else, so a spelling comparison between
    /// the two never fires. Everything else is unknown to it.
    ///
    /// Built through the PRODUCTION `canonicalizer`, not hand-rolled — so the
    /// unresolvable-name policy these tests rely on is the one that ships, and
    /// changing that fallback turns them red.
    fn canon(m: &str) -> String {
        canonicalizer(|m: &str| (m == "alias").then(|| "vendor/real".to_string()))(m)
    }

    fn seats() -> Vec<String> {
        vec!["gpt-5.6".into(), "vendor/real".into(), "gpt-5.4".into()]
    }

    /// Resolve exactly as `live_io_for` does, so these cases exercise the
    /// precedence rule and the check together rather than the check alone.
    fn check(heal: Option<&str>, coder_toml: Option<&str>) -> Result<(), String> {
        check_coder_pin(
            super::super::config::session_model(heal, coder_toml),
            &seats(),
            &canon,
        )
    }

    /// The defect car#1360 is: heal.toml pinning nothing does not mean the
    /// coder is unpinned, it means `coder.toml`'s `model` is the pin — and
    /// that source was never checked against the panel.
    #[test]
    fn a_coder_toml_pin_that_is_a_review_seat_is_refused() {
        let err = check(None, Some("gpt-5.6")).expect_err("coder.toml pinned a seat");
        assert!(err.contains("gpt-5.6"), "{err}");
        assert!(err.contains("`model` in `coder.toml`"), "{err}");
        // Naming the wrong file sends the operator to edit a key that isn't
        // set, which is how a correct refusal still costs an afternoon.
        assert!(!err.contains("`coder_model` in `heal.toml`"), "{err}");
    }

    /// Canonicalization is load-bearing, not cosmetic: the pin is spelled as a
    /// name and the seat as an id. A gate that cannot fire reads as covered.
    #[test]
    fn a_coder_toml_pin_is_matched_through_the_registry_not_by_spelling() {
        let err = check(None, Some("alias")).expect_err("alias canonicalizes onto vendor/real");
        assert!(err.contains("vendor/real"), "{err}");
    }

    /// heal.toml still wins where it is set, and the message says so — the
    /// coder.toml value is not what runs, so refusing on it would be wrong.
    #[test]
    fn the_heal_toml_pin_wins_and_is_the_one_checked() {
        let err = check(Some("gpt-5.4"), Some("claude-sonnet-5")).expect_err("heal.toml pinned");
        assert!(err.contains("`coder_model` in `heal.toml`"), "{err}");

        // And the converse: a coder.toml pin sitting on the panel is harmless
        // when heal.toml overrides it, because it never runs.
        check(Some("claude-sonnet-5"), Some("gpt-5.6")).expect("the pin that runs is not a seat");
    }

    /// A blank `coder_model` is "unset", not "pin blank" — the only input where
    /// precedence and the source label could disagree and name the wrong file.
    #[test]
    fn a_blank_heal_pin_falls_through_and_the_message_names_coder_toml() {
        let err = check(Some("   "), Some("gpt-5.6")).expect_err("blank falls through");
        assert!(err.contains("`model` in `coder.toml`"), "{err}");
    }

    /// Neither source pins: the router picks per request, so nothing here can
    /// say what it will pick. `heal_runner`'s gate answers that one, against
    /// what actually authored the change.
    #[test]
    fn an_unpinned_coder_is_not_this_checks_to_refuse() {
        check(None, None).expect("unpinned is not a seat");
    }

    /// A pin outside CAR's registry is NOT refused. `coder.toml` is shared with
    /// `coder.start` and `car code-task`, neither of which validates it, and on
    /// the external rung the value is handed verbatim to a CLI's own namespace
    /// — so an unknown name can be exactly right for the rung that runs, and
    /// taking the whole loop offline over it is the false refusal this file
    /// declines to make elsewhere.
    ///
    /// It still has to be COMPARED, though. The canonicalizer falls back to the
    /// name itself rather than `""`, so an unresolvable pin degrades to a
    /// spelling match instead of silently matching nothing.
    #[test]
    fn a_pin_outside_the_registry_is_compared_not_refused() {
        // Unknown and not a seat: allowed through.
        check(None, Some("codex-mini")).expect("an unknown pin is not by itself a refusal");
        // Unknown to `canon` but spelled exactly like a seat: still caught. A
        // canonicalizer collapsing the unknown to `""` would miss this.
        let err = check(None, Some("gpt-5.4")).expect_err("spelling still matches a seat");
        assert!(err.contains("gpt-5.4"), "{err}");
    }

    /// Targets but no panel is a DIFFERENT disabled    /// Targets but no panel is a DIFFERENT disabled, and the difference is the
    /// whole point of reporting a reason: `decide` refuses a panel of zero, so
    /// running would mean a full coder session per item followed by a
    /// guaranteed rejection.
    #[test]
    fn targets_without_a_review_panel_do_not_enable_the_loop() {
        let dir = tempfile::tempdir().unwrap();
        let mut c = cfg(vec![target("acme/one")]);
        c.review_models.clear();
        let s = HealService::new(c, dir.path().into());
        assert!(!s.is_enabled());
        assert!(s
            .status(&engine())
            .disabled_reason
            .unwrap()
            .contains("review_models"));
    }

    /// `heal.status` must report the panel it will actually use, resolved.
    ///
    /// The wiring, not the rule: `correlation_warning` is table-tested next
    /// door, and every defect this file has had was in getting the right value
    /// to it.
    #[test]
    fn status_resolves_the_panel_and_reports_a_correlated_one() {
        let dir = tempfile::tempdir().unwrap();
        let mut c = cfg(vec![target("acme/one")]);
        // Real catalog ids: both are OpenAI, so one vendor holds every seat.
        c.review_models = vec!["gpt-5.4".into(), "gpt-5.5".into()];
        let s = HealService::new(c, dir.path().into());
        let st = s.status(&engine());

        assert_eq!(
            st.panel
                .iter()
                .map(|p| p.model.as_str())
                .collect::<Vec<_>>(),
            vec!["gpt-5.4", "gpt-5.5"]
        );
        assert!(
            st.panel
                .iter()
                .all(|p| p.vendor.as_deref() == Some("openai")),
            "both seats must resolve to openai: {:?}",
            st.panel
        );
        let w = st
            .panel_warning
            .expect("a one-vendor panel must be reported");
        assert!(w.contains("openai serves 2 of the 2 seats"), "{w}");
    }

    /// An independent panel carries no warning, so the warning means something.
    #[test]
    fn status_does_not_warn_about_a_panel_spanning_vendors() {
        let dir = tempfile::tempdir().unwrap();
        let mut c = cfg(vec![target("acme/one")]);
        c.review_models = vec!["gpt-5.4".into(), "claude-opus-5".into()];
        let s = HealService::new(c, dir.path().into());
        let st = s.status(&engine());
        assert_eq!(
            st.panel
                .iter()
                .filter_map(|p| p.vendor.as_deref())
                .collect::<Vec<_>>(),
            vec!["openai", "anthropic"]
        );
        assert_eq!(st.panel_warning, None);
    }

    #[test]
    fn assembly_refuses_a_three_seat_single_provider_panel() {
        let dir = tempfile::tempdir().unwrap();
        let state = Arc::new(ServerState::standalone(dir.path().join("journal")));
        let mut c = cfg(vec![target("acme/one")]);
        c.review_models = vec!["gpt-5.4".into(), "gpt-5.5".into(), "gpt-5.6-sol".into()];
        c.coder_model = Some("claude-opus-5".into());
        let s = HealService::new(c.clone(), dir.path().join("coder"));

        let error = s
            .live_io_for(&state, &c)
            .err()
            .expect("one serving provider must refuse assembly");
        assert!(error.contains("openai"), "{error}");
        for model in &c.review_models {
            assert!(error.contains(model), "{model} is missing from: {error}");
        }
    }

    #[test]
    fn assembly_refuses_a_coder_that_is_also_a_review_seat() {
        let dir = tempfile::tempdir().unwrap();
        let state = Arc::new(ServerState::standalone(dir.path().join("journal")));
        let mut c = cfg(vec![target("acme/one")]);
        c.review_models = vec!["gpt-5.4".into(), "claude-opus-5".into()];
        c.coder_model = Some("gpt-5.4".into());
        let s = HealService::new(c.clone(), dir.path().join("coder"));

        let error = s
            .live_io_for(&state, &c)
            .err()
            .expect("a coder on its panel must refuse assembly");
        assert!(error.contains("gpt-5.4"), "{error}");
        assert!(error.contains("also a review seat"), "{error}");
    }

    #[test]
    fn assembly_accepts_a_two_provider_panel_with_a_disjoint_coder() {
        let dir = tempfile::tempdir().unwrap();
        let state = Arc::new(ServerState::standalone(dir.path().join("journal")));
        let mut c = cfg(vec![target("acme/one")]);
        c.review_models = vec!["gpt-5.4".into(), "claude-opus-5".into()];
        c.coder_model = Some("gpt-5.5".into());
        let s = HealService::new(c.clone(), dir.path().join("coder"));

        assert!(s.live_io_for(&state, &c).is_ok());
    }

    /// A loop that is enabled and DEAD is the state `heal.status` exists to
    /// make impossible. Assembly validates things `is_enabled` never sees — an
    /// unknown model, a coder sitting on its own panel — so its failure has to
    /// reach `disabled_reason` rather than only the boot log.
    #[test]
    fn an_assembly_failure_is_reported_as_the_disabled_reason() {
        let dir = tempfile::tempdir().unwrap();
        let s = HealService::new(cfg(vec![target("acme/one")]), dir.path().into());
        assert_eq!(s.status(&engine()).disabled_reason, None);

        s.record_assembly_error("`coder_model` x is also a review seat (x)");
        let st = s.status(&engine());
        assert!(
            st.enabled,
            "the config is still valid; the assembly was not"
        );
        assert!(
            st.disabled_reason
                .as_deref()
                .is_some_and(|r| r.contains("also a review seat")),
            "{:?}",
            st.disabled_reason
        );
    }

    /// car#1334 made this pin decide whether a sweep runs at all, and the value
    /// lives in whichever of two files won. An operator reading a refusal had
    /// to open both and re-derive the precedence by hand — the re-derivation
    /// that produced car#1360.
    #[test]
    fn status_reports_the_coder_pin_from_heal_toml() {
        let dir = tempfile::tempdir().unwrap();
        let mut c = cfg(vec![target("acme/one")]);
        c.coder_model = Some("gpt-5.5".into());
        let s = HealService::new(c, dir.path().into());

        let pin = s
            .status(&engine())
            .coder_pin
            .expect("a pinned coder must be reported");
        assert_eq!(pin.model, "gpt-5.5");
        // Naming the wrong file sends the operator to edit a line that is not
        // the one in force, which is worse than saying nothing.
        assert_eq!(pin.source, "heal_toml");
    }

    /// An absent `coder_model` does NOT mean unpinned — it means `coder.toml`
    /// decides, and that is the case the operator is least able to work out
    /// from `heal.toml` alone.
    #[test]
    fn status_falls_through_to_coder_toml_and_says_so() {
        let _guard = crate::coder::config::config_env_lock()
            .lock()
            .unwrap_or_else(|e| e.into_inner());
        let dir = tempfile::tempdir().unwrap();
        let cfg_path = dir.path().join("coder.toml");
        std::fs::write(&cfg_path, "[coder]\nmodel = \"claude-opus-5\"\n").unwrap();
        let prev = std::env::var_os("CAR_CODER_CONFIG");
        std::env::set_var("CAR_CODER_CONFIG", &cfg_path);

        let mut c = cfg(vec![target("acme/one")]);
        c.coder_model = None;
        let st = HealService::new(c, dir.path().into()).status(&engine());

        match prev {
            Some(v) => std::env::set_var("CAR_CODER_CONFIG", v),
            None => std::env::remove_var("CAR_CODER_CONFIG"),
        }

        let pin = st.coder_pin.expect("coder.toml pins it");
        assert_eq!(pin.model, "claude-opus-5");
        assert_eq!(pin.source, "coder_toml");
    }

    /// Unpinned is a THIRD state, not a missing value. Adaptive routing picks
    /// per request, so nothing before the run can say what it will pick — and
    /// an operator has to be able to tell that from "pinned to something the
    /// panel rejects".
    #[test]
    fn status_reports_no_pin_when_neither_file_names_one() {
        let _guard = crate::coder::config::config_env_lock()
            .lock()
            .unwrap_or_else(|e| e.into_inner());
        let dir = tempfile::tempdir().unwrap();
        let cfg_path = dir.path().join("coder.toml");
        // A file that EXISTS and parses, with `[coder]` present and no `model`.
        // Pointing at an absent path would pass identically if `status` never
        // read the file at all, if the `.then()` guard were inverted, or if the
        // env override were ignored — the test would then be agreeing with the
        // behaviour rather than pinning it.
        std::fs::write(&cfg_path, "[coder]\ndefault_max_iterations = 3\n").unwrap();
        let prev = std::env::var_os("CAR_CODER_CONFIG");
        std::env::set_var("CAR_CODER_CONFIG", &cfg_path);

        let mut c = cfg(vec![target("acme/one")]);
        c.coder_model = None;
        let st = HealService::new(c, dir.path().into()).status(&engine());

        match prev {
            Some(v) => std::env::set_var("CAR_CODER_CONFIG", v),
            None => std::env::remove_var("CAR_CODER_CONFIG"),
        }

        assert_eq!(st.coder_pin, None);
    }

    /// `heal.run` refusing for a reason `heal.status` will not show leaves an
    /// operator with a status that describes a config the manual door will not
    /// run on.
    ///
    /// Reported as `run_refusal`, NOT as `disabled_reason`. A cadence that
    /// assembled cleanly at boot keeps sweeping on its boot-time io, so a
    /// manual refusal is a warning about the config as it stands now — calling
    /// it "disabled" would report a dead loop that is in fact running.
    #[tokio::test]
    async fn a_manual_run_records_its_assembly_refusal() {
        let dir = tempfile::tempdir().unwrap();
        let state = Arc::new(ServerState::standalone(dir.path().join("journal")));
        let mut c = cfg(vec![target("acme/one")]);
        c.review_models = vec!["gpt-5.4".into(), "claude-opus-5".into()];
        // The coder sitting on its own panel: assembly refuses.
        c.coder_model = Some("gpt-5.4".into());
        let s = HealService::new(c, dir.path().join("coder"));

        assert_eq!(
            s.status(&engine()).run_refusal,
            None,
            "nothing has refused yet"
        );
        let err = s
            .run_tick(&state)
            .await
            .expect_err("a coder on its panel must refuse");
        assert!(err.contains("also a review seat"), "{err}");

        let st = s.status(&engine());
        assert!(
            st.run_refusal
                .as_deref()
                .is_some_and(|r| r.contains("also a review seat")),
            "the refusal must reach heal.status, not just the caller: {:?}",
            st.run_refusal
        );
        assert_eq!(
            st.disabled_reason, None,
            "the cadence was never assembled here, and a manual refusal is not \
             a statement that the loop is disabled"
        );
    }

    /// The other half of the manual-refusal contract: a run that gets past
    /// assembly clears the refusal.
    ///
    /// Without this the first version traded one lie for a worse one — an
    /// operator who fixed the config kept reading the old refusal until the
    /// daemon restarted, because the only writer was `record_*` and nothing
    /// cleared.
    #[tokio::test]
    async fn a_successful_manual_run_clears_the_refusal() {
        let dir = tempfile::tempdir().unwrap();
        let state = Arc::new(ServerState::standalone(dir.path().join("journal")));
        let mut bad = cfg(vec![target("acme/one")]);
        bad.review_models = vec!["gpt-5.4".into(), "claude-opus-5".into()];
        bad.coder_model = Some("gpt-5.4".into());
        let s = HealService::new(bad, dir.path().join("coder"));

        s.run_tick(&state)
            .await
            .expect_err("a coder on its panel refuses");
        assert!(s.status(&engine()).run_refusal.is_some());

        // The operator fixes it: same panel, a coder that is not a seat.
        let mut good = cfg(vec![target("acme/one")]);
        good.review_models = vec!["gpt-5.4".into(), "claude-opus-5".into()];
        good.coder_model = Some("gpt-5.5".into());
        if let Ok(mut held) = s.config.write() {
            *held = good;
        }

        s.run_tick(&state)
            .await
            .expect("the fixed config assembles");
        assert_eq!(
            s.status(&engine()).run_refusal,
            None,
            "a run that assembled must clear the refusal it is contradicting"
        );
    }

    /// A manual run must not touch the BOOT slot. The two have different
    /// lifetimes: the cadence assembles once and does not start on failure, so
    /// its error is true until the daemon restarts, while a manual run's is a
    /// statement about the config right now.
    ///
    /// Conflating them broke both directions — a stale refusal that never
    /// cleared, AND a healthy sweeping cadence reporting `disabled_reason`
    /// permanently because someone ran `heal.run` on a bad edit.
    #[tokio::test]
    async fn a_manual_run_neither_sets_nor_clears_the_boot_assembly_error() {
        let dir = tempfile::tempdir().unwrap();
        let state = Arc::new(ServerState::standalone(dir.path().join("journal")));
        let mut c = cfg(vec![target("acme/one")]);
        c.review_models = vec!["gpt-5.4".into(), "claude-opus-5".into()];
        c.coder_model = Some("gpt-5.4".into());
        let s = HealService::new(c, dir.path().join("coder"));

        // The cadence died at boot. That stays true until a restart.
        s.record_assembly_error("boot: something the cadence could not assemble");

        s.run_tick(&state).await.expect_err("still refuses");
        let st = s.status(&engine());
        assert!(
            st.disabled_reason
                .as_deref()
                .is_some_and(|r| r.contains("boot: something")),
            "a manual run must not overwrite the boot error: {:?}",
            st.disabled_reason
        );
        assert!(st.run_refusal.is_some(), "and must record its own");

        // Now a manual run succeeds. The cadence is STILL dead — it returned
        // early at boot and nothing restarted it — so clearing the boot error
        // here would report a working loop that does not exist.
        let mut good = cfg(vec![target("acme/one")]);
        good.review_models = vec!["gpt-5.4".into(), "claude-opus-5".into()];
        good.coder_model = Some("gpt-5.5".into());
        if let Ok(mut held) = s.config.write() {
            *held = good;
        }
        s.run_tick(&state).await.expect("assembles now");

        let st = s.status(&engine());
        assert_eq!(st.run_refusal, None);
        assert!(
            st.disabled_reason
                .as_deref()
                .is_some_and(|r| r.contains("boot: something")),
            "the cadence is still dead until restart: {:?}",
            st.disabled_reason
        );
    }

    /// The status a `heal.status` caller gets before anything has run.
    #[test]
    fn status_names_the_targets_the_panel_and_the_engine() {
        let dir = tempfile::tempdir().unwrap();
        let s = HealService::new(cfg(vec![target("acme/one")]), dir.path().into());
        let st = s.status(&engine());
        assert!(st.enabled);
        assert_eq!(st.disabled_reason, None);
        assert_eq!(st.targets, vec!["acme/one".to_string()]);
        assert_eq!(st.review_models, vec!["reviewer-a".to_string()]);
        // The engine is reported even when it was defaulted, so an operator
        // never has to know what the default is to know what will run.
        assert_eq!(st.engine, DEFAULT_ENGINE);
    }

    /// The default must be an engine that actually parses, or every daemon
    /// with a configured target refuses to assemble the loop at boot.
    #[test]
    fn the_default_engine_is_a_real_engine() {
        assert!(crate::coder::router::EngineChoice::parse(DEFAULT_ENGINE).is_ok());
    }

    #[tokio::test]
    async fn a_sweep_visits_every_target_not_just_the_first() {
        // Stopping at the first with work would starve every later entry, and
        // the operator who listed them could not see it happening.
        let dir = tempfile::tempdir().unwrap();
        let s = HealService::new(
            cfg(vec![target("acme/one"), target("acme/two")]),
            dir.path().into(),
        );
        let io: Arc<dyn TickIo> = Arc::new(NoopIo);
        let report = s.sweep(&io).await;
        assert_eq!(report.outcomes.len(), 2);
        assert_eq!(report.outcomes[0].0, "acme/one");
        assert_eq!(report.outcomes[1].0, "acme/two");
    }

    #[tokio::test]
    async fn an_overlapping_sweep_stands_down_rather_than_queueing() {
        let dir = tempfile::tempdir().unwrap();
        let s = Arc::new(HealService::new(
            cfg(vec![target("acme/one")]),
            dir.path().into(),
        ));
        let io: Arc<dyn TickIo> = Arc::new(NoopIo);

        // Hold the lock as a concurrent sweep would.
        let held = s.running.lock().await;
        let report = s.sweep(&io).await;
        assert!(report.skipped_overlap);
        assert!(report.outcomes.is_empty());
        drop(held);

        // And it runs again once the lock is free.
        assert!(!s.sweep(&io).await.skipped_overlap);
    }

    #[tokio::test]
    async fn the_ledger_is_written_even_when_nothing_was_claimed() {
        // The file's existence is what makes the next start's load meaningful.
        let dir = tempfile::tempdir().unwrap();
        let s = HealService::new(cfg(vec![target("acme/one")]), dir.path().into());
        let io: Arc<dyn TickIo> = Arc::new(NoopIo);
        let _ = s.sweep(&io).await;
        assert!(
            dir.path().join("heal-claims.json").exists(),
            "the sweep persists the ledger around every target"
        );
    }

    #[test]
    fn the_interval_can_be_overridden_but_never_to_zero() {
        // A zero interval is a busy loop against someone's API quota.
        let dir = tempfile::tempdir().unwrap();
        std::env::set_var(HEAL_INTERVAL_ENV, "0");
        let s = HealService::new(cfg(vec![]), dir.path().into());
        assert_eq!(s.interval_secs(), DEFAULT_INTERVAL_SECS);
        std::env::set_var(HEAL_INTERVAL_ENV, "60");
        let s = HealService::new(cfg(vec![]), dir.path().into());
        assert_eq!(s.interval_secs(), 60);
        std::env::remove_var(HEAL_INTERVAL_ENV);
    }
}