openlatch-client 0.3.3

//! `openlatch doctor --fix` — auto-heal common issues.
//!
//! Step 4 lands `heal_state` (config.toml, daemon.token, agent_id,
//! telemetry.json, daemon.pid). Steps 5–6 layer in `heal_hooks`,
//! `heal_binaries`, and `heal_daemon` (with auto-rollback on restart
//! failure). Each mutation writes a `.bak` sibling and appends a
//! `FixAction` to a journal at `~/.openlatch/fix-journal.json` so
//! `--restore` can reverse it.

use std::path::{Path, PathBuf};

use chrono::{DateTime, Utc};
use serde::{Deserialize, Serialize};

use crate::cli::commands::doctor::{print_diagnostic_results, run_all_checks, DoctorReport};
use crate::cli::commands::{doctor_restore, lifecycle};
use crate::cli::output::{OutputConfig, OutputFormat};
use crate::cli::DoctorArgs;
use crate::config;
use crate::error::OlError;
use crate::hooks;
use crate::hooks::DetectedAgent;
use crate::telemetry::{self, Event};

/// Filename of the per-run journal stored under the openlatch state dir.
pub(crate) const JOURNAL_FILENAME: &str = "fix-journal.json";

/// Categories of self-heal action recorded in the journal.
///
/// Used by `--restore` to dispatch the correct rollback strategy
/// (surgical merge for hooks, blind file swap for everything else,
/// process actions are not reversed).
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub enum FixKind {
    /// `config.toml` rewritten from defaults (parse failure or missing).
    ConfigRewrite,
    /// `daemon.token` regenerated (missing or empty).
    TokenRegenerate,
    /// `[daemon] agent_id` inserted into `config.toml`.
    AgentIdInsert,
    /// `telemetry.json` reset to a safe default after a parse failure.
    TelemetryReset,
    /// Stale `daemon.pid` removed (PID no longer alive).
    PidStaleRemove,
    /// Hook entries rewritten in the agent's `settings.json`.
    HookReinstall,
    /// Daemon process cycled (stop → restart). Not reversible by `--restore`.
    DaemonRestart,
    /// `~/.openlatch/bin/openlatch-hook` re-staged from the resolved source.
    BinaryCopy,
    /// OS-native supervisor (launchd / systemd-user / Task Scheduler) reinstalled
    /// because config said `mode=active` but the OS artifact was missing. Not
    /// reversible by `--restore` — rerun `openlatch supervision uninstall` to undo.
    SupervisionInstall,
}

/// One self-heal action recorded in the journal.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct FixAction {
    /// OL-XXXX code that motivated the fix (matches the diagnostic check).
    pub ol_code: String,
    /// What kind of mutation was performed.
    pub kind: FixKind,
    /// Path that was mutated (for rollback) or `""` for process actions.
    pub file: PathBuf,
    /// Path to the `.bak` sibling created before the mutation, if any.
    pub backup: Option<PathBuf>,
    /// Whether `--restore` can reverse this action.
    pub reversible: bool,
    /// UTC timestamp when the action was applied.
    pub applied_at: DateTime<Utc>,
    /// Human-readable note shown in the summary output.
    pub note: String,
}

/// The journal of fixes applied during a single `--fix` invocation.
///
/// Serialized to `~/.openlatch/fix-journal.json` at the end of every
/// `--fix` run (overwriting any prior journal). `--restore` reads this
/// file to reverse the most recent run's actions.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Journal {
    /// Per-run identifier (UUIDv4 simple) for telemetry/log correlation.
    pub run_id: String,
    /// UTC timestamp of when the run began.
    pub started_at: DateTime<Utc>,
    /// Ordered list of fixes applied during this run.
    pub actions: Vec<FixAction>,
}

impl Journal {
    /// Start a fresh journal with a new `run_id`.
    pub fn new() -> Self {
        Self {
            run_id: uuid::Uuid::new_v4().simple().to_string(),
            started_at: Utc::now(),
            actions: Vec::new(),
        }
    }

    /// Persist the journal to `<ol_dir>/fix-journal.json`.
    ///
    /// Overwrites any prior journal — `--restore` only ever reverses the
    /// most recent run.
    pub fn save(&self, ol_dir: &Path) -> Result<PathBuf, OlError> {
        let path = ol_dir.join(JOURNAL_FILENAME);
        let raw = serde_json::to_string_pretty(self).map_err(|e| {
            OlError::new(
                crate::error::ERR_DOCTOR_JOURNAL_CORRUPT,
                format!("cannot serialize fix journal: {e}"),
            )
        })?;
        std::fs::write(&path, raw).map_err(|e| {
            OlError::new(
                crate::error::ERR_DOCTOR_JOURNAL_CORRUPT,
                format!("cannot write fix journal '{}': {e}", path.display()),
            )
        })?;
        Ok(path)
    }

    /// Read the journal from `<ol_dir>/fix-journal.json`.
    ///
    /// Returns `OL-1801` if the file is absent (no prior `--fix` to
    /// reverse) and `OL-1800` if it exists but cannot be parsed.
    pub fn load(ol_dir: &Path) -> Result<Self, OlError> {
        let path = ol_dir.join(JOURNAL_FILENAME);
        let raw = match std::fs::read_to_string(&path) {
            Ok(s) => s,
            Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
                return Err(OlError::new(
                    crate::error::ERR_DOCTOR_RESTORE_NO_JOURNAL,
                    format!("no prior --fix run found at '{}'", path.display()),
                )
                .with_suggestion(
                    "Run `openlatch doctor --fix` first; --restore reverses the most recent run.",
                ));
            }
            Err(e) => {
                return Err(OlError::new(
                    crate::error::ERR_DOCTOR_JOURNAL_CORRUPT,
                    format!("cannot read fix journal '{}': {e}", path.display()),
                ))
            }
        };
        serde_json::from_str(&raw).map_err(|e| {
            OlError::new(
                crate::error::ERR_DOCTOR_JOURNAL_CORRUPT,
                format!("fix journal at '{}' is malformed: {e}", path.display()),
            )
            .with_suggestion(
                "Delete `fix-journal.json` and re-run `openlatch init` to reset state.",
            )
        })
    }
}

impl Default for Journal {
    fn default() -> Self {
        Self::new()
    }
}

/// Entry point for `openlatch doctor --fix`.
///
/// Pre-fix: snapshot diagnostic checks (best-effort — a totally broken
/// install may not even pass `Config::load`, in which case the snapshot
/// is `None` and the report only shows post-fix state).
///
/// Heal: state → (steps 5+: hooks, binaries, daemon).
///
/// Post-fix: re-run all diagnostic checks. Exit 0 if all pass; 1 if any
/// remain failing (lists the unfixable OL-XXXX codes in the output).
pub fn run(_args: &DoctorArgs, output: &OutputConfig) -> Result<(), OlError> {
    let started = std::time::Instant::now();
    let ol_dir = config::openlatch_dir();
    // Ensure the directory exists before any heal step touches it; without
    // this a brand-new install would error out on the first `.bak` write.
    std::fs::create_dir_all(&ol_dir).map_err(|e| {
        OlError::new(
            crate::error::ERR_INVALID_CONFIG,
            format!("cannot create openlatch dir '{}': {e}", ol_dir.display()),
        )
    })?;

    crate::cli::header::print(output, &["doctor", "--fix"]);

    // Pre-fix snapshot — best-effort. A corrupt config.toml may make
    // run_all_checks itself error; that's expected and the fix run still
    // proceeds (heal_state will rewrite the bad config).
    let before = run_all_checks(output).ok();

    let pre_fix = capture_pre_fix_state(&ol_dir);
    let mut journal = Journal::new();

    // Stop the daemon before mutating files so it cannot reload a
    // half-written config. We only attempt a restart if it was running
    // before, OR if a fix touched config/token/hooks/binaries.
    if pre_fix.was_running {
        stop_daemon_for_fix(&pre_fix, &ol_dir);
    }

    journal.actions.extend(heal_state(&ol_dir));
    // Hooks and binaries depend on a healed config + token, so they run
    // after heal_state. heal_hooks no-ops when no agent is detected.
    journal.actions.extend(heal_binaries(&ol_dir));
    journal.actions.extend(heal_hooks(&ol_dir));
    journal.actions.extend(heal_supervision(&ol_dir));

    let mut auto_rollback_triggered = false;
    let (daemon_actions, restart_failed) = heal_daemon(&ol_dir, &pre_fix, &journal.actions);
    journal.actions.extend(daemon_actions);

    if restart_failed {
        auto_rollback_triggered = true;
        tracing::error!("doctor --fix: daemon restart failed — rolling back this run's actions");
        // Best-effort rollback. If this also fails the user is no worse
        // off than they would have been after a manual `--restore`.
        let _ = doctor_restore::restore_actions(&journal.actions, &ol_dir);
        if pre_fix.was_running && !lifecycle::start_via_supervisor(pre_fix.port) {
            let token = std::fs::read_to_string(ol_dir.join("daemon.token"))
                .map(|s| s.trim().to_string())
                .unwrap_or_default();
            if !token.is_empty() {
                let _ = lifecycle::spawn_daemon_background(pre_fix.port, &token);
            }
        }
    }

    // Persist the journal even when zero actions were taken — that way
    // `--restore` can give a precise "nothing to reverse" message instead
    // of falling through to OL-1801 ("no prior --fix").
    let journal_path = journal.save(&ol_dir)?;

    let after = run_all_checks(output)?;

    let categories: Vec<&str> = {
        let mut cats: Vec<&str> = Vec::new();
        for a in &journal.actions {
            let cat = match a.kind {
                FixKind::ConfigRewrite
                | FixKind::TokenRegenerate
                | FixKind::AgentIdInsert
                | FixKind::TelemetryReset
                | FixKind::PidStaleRemove => "state",
                FixKind::HookReinstall => "hooks",
                FixKind::DaemonRestart => "daemon",
                FixKind::BinaryCopy => "binary",
                FixKind::SupervisionInstall => "supervision",
            };
            if !cats.contains(&cat) {
                cats.push(cat);
            }
        }
        cats
    };
    // Everything still not green after the run. `--fix` heals defects; it
    // deliberately does not re-enable what the operator switched off, so a
    // disabled boundary shows up here as unresolved rather than being flipped
    // back on behind their back.
    let unresolved = after.unresolved();
    let unfixable: Vec<&str> = unresolved.iter().map(String::as_str).collect();
    let (before_pass, before_fail) = before
        .as_ref()
        .map(|r| (r.pass_count(), r.fail_count()))
        .unwrap_or((0, 0));
    telemetry::capture_global(Event::doctor_fix_run(
        categories,
        before_pass,
        before_fail,
        after.pass_count(),
        after.fail_count(),
        unfixable,
        started.elapsed().as_millis() as u64,
        auto_rollback_triggered,
    ));

    print_fix_results(
        &journal,
        &journal_path,
        before.as_ref(),
        &after,
        auto_rollback_triggered,
        output,
    );

    if after.all_pass() && !auto_rollback_triggered {
        Ok(())
    } else {
        // Exit 1 without going through OlError so the caller's error
        // formatter doesn't double-print. Matches the brainstorm's
        // "exit 1 = unfixable remaining" contract.
        std::process::exit(1);
    }
}

/// Snapshot of the daemon's state before any `--fix` mutation.
///
/// Used to (a) decide whether a restart is necessary and (b) revert the
/// daemon to its pre-fix state if the post-fix restart fails.
#[derive(Debug, Clone)]
#[allow(dead_code)] // was_healthy is captured for telemetry / debug output (step 7)
pub(crate) struct PreFixState {
    pub was_running: bool,
    pub was_healthy: bool,
    pub port: u16,
    pub pid: Option<u32>,
}

/// Capture the daemon's pre-fix state.
pub(crate) fn capture_pre_fix_state(_ol_dir: &Path) -> PreFixState {
    let port = config::Config::load(None, None, false)
        .map(|c| c.port)
        .unwrap_or(config::PORT_RANGE_START);
    let pid = lifecycle::read_pid_file();
    let was_running = pid.map(lifecycle::is_process_alive).unwrap_or(false);
    let was_healthy = if was_running {
        lifecycle::check_health(port)
    } else {
        false
    };
    PreFixState {
        was_running,
        was_healthy,
        port,
        pid,
    }
}

/// Stop the daemon ahead of file mutations.
///
/// Tries the bearer-authenticated POST /shutdown first (graceful), then
/// falls back to force-kill via the PID. Cleans up the PID file when
/// the process is confirmed dead. Best-effort — failures are logged and
/// allowed; heal_daemon's restart attempt will surface anything
/// genuinely broken.
fn stop_daemon_for_fix(state: &PreFixState, ol_dir: &Path) {
    // Under a supervisor, `/shutdown` + SIGTERM is not a stop: `Restart=always`
    // brings the daemon back within `RestartSec`, i.e. in the middle of the
    // heal. `heal_state` rewrites config.toml and daemon.token, so a daemon
    // restarting under it reads half-written state. The supervisor has to be
    // the one told to stop.
    if lifecycle::stop_via_supervisor() {
        let _ = std::fs::remove_file(ol_dir.join("daemon.pid"));
        return;
    }

    let Some(pid) = state.pid else {
        return;
    };

    // Read the token to send the bearer-authenticated shutdown.
    let token = std::fs::read_to_string(ol_dir.join("daemon.token"))
        .map(|s| s.trim().to_string())
        .unwrap_or_default();

    if !token.is_empty() {
        let _ = lifecycle::send_shutdown_request(state.port, &token);
        let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5);
        while std::time::Instant::now() < deadline && lifecycle::is_process_alive(pid) {
            std::thread::sleep(std::time::Duration::from_millis(100));
        }
    }

    if lifecycle::is_process_alive(pid) {
        lifecycle::force_kill(pid);
        let deadline = std::time::Instant::now() + std::time::Duration::from_secs(3);
        while std::time::Instant::now() < deadline && lifecycle::is_process_alive(pid) {
            std::thread::sleep(std::time::Duration::from_millis(100));
        }
    }

    // Best-effort PID file cleanup.
    let _ = std::fs::remove_file(ol_dir.join("daemon.pid"));
}

/// Restart the daemon when needed.
///
/// Returns `(actions, restart_failed)`:
/// - `actions` carries a `DaemonRestart` `FixAction` when a restart was
///   attempted AND `/health` came back 200 within 3 s.
/// - `restart_failed` is `true` only when a restart was attempted and
///   did not finish healthy. The caller then drives auto-rollback.
///
/// Skips the restart entirely when the daemon was off pre-fix AND no
/// fix touched a file the daemon reads on boot — pure churn isn't worth
/// disrupting in-flight hooks.
pub(crate) fn heal_daemon(
    ol_dir: &Path,
    pre_fix: &PreFixState,
    prior_actions: &[FixAction],
) -> (Vec<FixAction>, bool) {
    let mut actions = Vec::new();

    let touched_critical = prior_actions.iter().any(|a| {
        matches!(
            a.kind,
            FixKind::ConfigRewrite
                | FixKind::TokenRegenerate
                | FixKind::AgentIdInsert
                | FixKind::HookReinstall
        )
    });
    let should_restart = pre_fix.was_running || touched_critical;
    if !should_restart {
        return (actions, false);
    }

    // Re-probe in case our own port is now contested by something else
    // that grabbed it during the stop window. We start the probe at the
    // pre-fix port so the common case (still free) returns immediately.
    let port = config::probe_free_port(pre_fix.port, config::PORT_RANGE_END)
        .or_else(|_| config::probe_free_port(config::PORT_RANGE_START, config::PORT_RANGE_END))
        .unwrap_or(pre_fix.port);
    if port != pre_fix.port {
        let _ = config::write_port_file(port);
    }

    let token = match std::fs::read_to_string(ol_dir.join("daemon.token")) {
        Ok(s) if !s.trim().is_empty() => s.trim().to_string(),
        _ => {
            tracing::error!(
                "doctor --fix: cannot restart daemon — daemon.token missing/empty after heal_state"
            );
            return (actions, true);
        }
    };

    // Hand the restart back to the supervisor when it owns the daemon, so the
    // heal ends with the one supervised daemon it started with — not with an
    // unsupervised one this process spawned next to a unit that will start its
    // own at the next login.
    if lifecycle::start_via_supervisor(port) {
        actions.push(FixAction {
            ol_code: crate::error::ERR_DAEMON_START_FAILED.to_string(),
            kind: FixKind::DaemonRestart,
            file: PathBuf::new(),
            backup: None,
            reversible: false,
            applied_at: Utc::now(),
            note: format!("supervised daemon restarted on port {port} (/health=200)"),
        });
        return (actions, false);
    }

    let pid = match lifecycle::spawn_daemon_background(port, &token) {
        Ok(pid) => pid,
        Err(e) => {
            tracing::error!(error = %e.message, code = e.code, "doctor --fix: daemon spawn failed");
            return (actions, true);
        }
    };

    if !lifecycle::wait_for_health(port, 3) {
        tracing::error!(
            pid = pid,
            port = port,
            "doctor --fix: daemon spawned but /health did not return 200 within 3s"
        );
        return (actions, true);
    }

    actions.push(FixAction {
        ol_code: crate::error::ERR_DAEMON_START_FAILED.to_string(),
        kind: FixKind::DaemonRestart,
        file: PathBuf::new(),
        backup: None,
        reversible: false,
        applied_at: Utc::now(),
        note: format!("daemon restarted on port {port} (PID {pid}, /health=200)"),
    });
    (actions, false)
}

/// Heal state files (config.toml, daemon.token, agent_id, telemetry.json,
/// daemon.pid).
///
/// Best-effort: a single mutation failure does not abort the rest of the
/// category. Each successful mutation appends a `FixAction`. Failures are
/// logged via `tracing::warn!` so the post-fix `run_all_checks` still
/// reports the underlying diagnostic.
///
/// Takes `&Path` rather than calling `openlatch_dir()` internally so
/// tests can drive the function with a tempdir without env-var
/// manipulation.
pub(crate) fn heal_state(ol_dir: &Path) -> Vec<FixAction> {
    let mut actions = Vec::new();

    // 1. config.toml — parse-or-regenerate.
    let config_path = ol_dir.join("config.toml");
    let config_needs_rewrite = match std::fs::read_to_string(&config_path) {
        Ok(raw) => toml::from_str::<toml::Value>(&raw).is_err(),
        Err(_) => true,
    };
    if config_needs_rewrite {
        let backup = if config_path.exists() {
            backup_file(&config_path).ok()
        } else {
            None
        };
        let content = config::generate_default_config_toml(config::PORT_RANGE_START);
        if let Err(e) = std::fs::write(&config_path, content) {
            tracing::warn!(error = %e, path = %config_path.display(), "doctor --fix: config rewrite failed");
        } else {
            actions.push(FixAction {
                ol_code: crate::error::ERR_INVALID_CONFIG.to_string(),
                kind: FixKind::ConfigRewrite,
                file: config_path.clone(),
                backup,
                reversible: true,
                applied_at: Utc::now(),
                note: format!("regenerated {} from defaults", config_path.display()),
            });
        }
    }

    // 2. daemon.token — regenerate if missing or empty.
    let token_path = ol_dir.join("daemon.token");
    let token_needs_regen = match std::fs::read_to_string(&token_path) {
        Ok(raw) => raw.trim().is_empty(),
        Err(_) => true,
    };
    if token_needs_regen {
        let backup = if token_path.exists() {
            backup_file(&token_path).ok()
        } else {
            None
        };
        // Force regeneration: ensure_token only generates when the file
        // is absent, so an empty file would otherwise survive.
        if token_path.exists() {
            let _ = std::fs::remove_file(&token_path);
        }
        match config::ensure_token(ol_dir) {
            Ok(_) => {
                actions.push(FixAction {
                    ol_code: crate::error::ERR_INVALID_CONFIG.to_string(),
                    kind: FixKind::TokenRegenerate,
                    file: token_path.clone(),
                    backup,
                    reversible: true,
                    applied_at: Utc::now(),
                    note: format!("regenerated {} (mode 0600 on Unix)", token_path.display()),
                });
            }
            Err(e) => {
                tracing::warn!(error = %e.message, code = e.code, "doctor --fix: token regenerate failed");
            }
        }
    }

    // 3. agent_id — ensure inserted into [daemon] section.
    if config_path.exists() {
        let needs_insert = std::fs::read_to_string(&config_path)
            .map(|raw| !raw.contains("agent_id"))
            .unwrap_or(false);
        if needs_insert {
            let backup = backup_file(&config_path).ok();
            match config::ensure_agent_id(&config_path) {
                Ok(id) => {
                    actions.push(FixAction {
                        ol_code: crate::error::ERR_INVALID_CONFIG.to_string(),
                        kind: FixKind::AgentIdInsert,
                        file: config_path.clone(),
                        backup,
                        reversible: true,
                        applied_at: Utc::now(),
                        note: format!("inserted agent_id={id} into [daemon] section"),
                    });
                }
                Err(e) => {
                    tracing::warn!(error = %e.message, code = e.code, "doctor --fix: agent_id insert failed");
                }
            }
        }
    }

    // 4. telemetry.json — reset to a safe default if corrupt.
    //    Resetting to `enabled: false` requires re-consent via the next
    //    `openlatch init` invocation — never silently flips a user back
    //    to opted-in (telemetry.md invariant I9).
    let telem_path = ol_dir.join("telemetry.json");
    if telem_path.exists() {
        let valid = std::fs::read_to_string(&telem_path)
            .ok()
            .and_then(|raw| serde_json::from_str::<serde_json::Value>(&raw).ok())
            .is_some();
        if !valid {
            let backup = backup_file(&telem_path).ok();
            let reset = serde_json::json!({
                "enabled": false,
                "schema_version": 1,
                "notice_shown_at": null,
            });
            match serde_json::to_string_pretty(&reset)
                .map_err(std::io::Error::other)
                .and_then(|s| std::fs::write(&telem_path, s))
            {
                Ok(_) => {
                    actions.push(FixAction {
                        ol_code: crate::error::ERR_TELEMETRY_CONFIG_CORRUPT.to_string(),
                        kind: FixKind::TelemetryReset,
                        file: telem_path.clone(),
                        backup,
                        reversible: true,
                        applied_at: Utc::now(),
                        note: format!(
                            "reset {} to enabled=false (re-consent required via init)",
                            telem_path.display()
                        ),
                    });
                }
                Err(e) => {
                    tracing::warn!(error = %e, "doctor --fix: telemetry reset failed");
                }
            }
        }
    }

    // 5. daemon.pid — remove if PID no longer alive.
    let pid_path = ol_dir.join("daemon.pid");
    if pid_path.exists() {
        let pid_alive = std::fs::read_to_string(&pid_path)
            .ok()
            .and_then(|s| s.trim().parse::<u32>().ok())
            .map(lifecycle::is_process_alive)
            .unwrap_or(false);
        if !pid_alive {
            if let Err(e) = std::fs::remove_file(&pid_path) {
                tracing::warn!(error = %e, path = %pid_path.display(), "doctor --fix: stale PID removal failed");
            } else {
                actions.push(FixAction {
                    ol_code: crate::error::ERR_ALREADY_RUNNING.to_string(),
                    kind: FixKind::PidStaleRemove,
                    file: pid_path.clone(),
                    backup: None,
                    reversible: false,
                    applied_at: Utc::now(),
                    note: format!("removed stale {} (process not alive)", pid_path.display()),
                });
            }
        }
    }

    actions
}

/// Heal every detected agent's hook installation.
///
/// When no supported agent is detected, this is a no-op. When an agent is
/// present but its config is missing one of its own load-bearing entries, or
/// names a hook binary that does not resolve, the file is backed up and the
/// full hook set is reinstalled via the idempotent `hooks::install_hooks()`
/// (which preserves non-OpenLatch entries through the `_openlatch` marker
/// contract).
///
/// Re-reads `port` and `token` from the freshly-healed state files so
/// the rewrite always uses the canonical pair.
pub(crate) fn heal_hooks(ol_dir: &Path) -> Vec<FixAction> {
    heal_hooks_for(&hooks::detect_agents(), ol_dir)
}

/// The seam `heal_hooks` is a shim over.
///
/// `heal_hooks` resolves its agents internally, and the two tests that drive it
/// go through `CLAUDE_CONFIG_DIR` — which can only ever yield one agent, on a
/// build that detects one. Taking the slice is what lets a test state the
/// multi-agent case at all.
fn heal_hooks_for(detected: &[DetectedAgent], ol_dir: &Path) -> Vec<FixAction> {
    let mut actions = Vec::new();

    // An empty slice is the old `Err(_) => return actions` arm: OL-1400 is
    // surfaced by run_all_checks and there is no fix to apply. Notably NOT a
    // FixAction — and the return has to come before the reads below, which is
    // what today's early return also achieved.
    if detected.is_empty() {
        return actions;
    }

    let token_path = ol_dir.join("daemon.token");
    let token = match std::fs::read_to_string(&token_path) {
        Ok(s) if !s.trim().is_empty() => s.trim().to_string(),
        _ => {
            tracing::warn!(
                path = %token_path.display(),
                "doctor --fix: skipping hook reinstall — daemon.token missing/empty after heal_state"
            );
            return actions;
        }
    };

    let port = config::Config::load(None, None, false)
        .map(|c| c.port)
        .unwrap_or(config::PORT_RANGE_START);

    // Stage the binary before rewriting any command: reinstalling a command
    // that points at the same missing path would be a no-op repair.
    //
    // Hoisted, with the token and the port, above the per-agent work: all three
    // are per-INSTANCE facts, not per-agent ones. Re-reading `daemon.token`
    // once per agent is a second chance for two agents to be pinned to
    // different tokens, and a staging failure is a reason to leave the whole
    // host alone rather than the first agent of it.
    if let Err(e) = hooks::staging::stage_hook_binary(ol_dir) {
        tracing::warn!(
            error = %e.message,
            code = e.code,
            "doctor --fix: cannot stage hook binary — leaving hooks alone rather than rewriting a dead command"
        );
        return actions;
    }

    for agent in detected {
        let settings_path = agent.settings_path();

        // Share the predicate with the diagnostic that sent the user here.
        //
        // This used to be four substring tests over the raw JSONC, which only
        // ever asked "is an entry present?". An entry that existed but pointed
        // at a binary that did not exist passed — so on the machine in #165,
        // `doctor` printed `ERR Hook binary missing: openlatch-hook` twelve
        // times and `--fix` staged the binary and left all twelve commands
        // untouched, because by its own test nothing was wrong. A diagnostic
        // and its own fix must not be allowed to disagree; `hooks::health` is
        // now the single answer.
        let needs_reinstall = match hooks::health::inspect_file(&settings_path, &*agent.binding) {
            Ok(health) => health.needs_reinstall(),
            Err(_) => true, // absent or unparseable → install_hooks rewrites it
        };

        // `continue`, never `return`. A healthy agent is a reason to move on to
        // the next one, not to end the walk: detection order puts Claude Code
        // first, so a bare return here would silently abandon every later agent
        // on the realistic dual-agent host — healthy Claude Code in front of the
        // broken agent the remedy told the user this command would repair.
        if !needs_reinstall {
            continue;
        }

        let backup = if settings_path.exists() {
            backup_file(&settings_path).ok()
        } else {
            None
        };

        match hooks::install_hooks(&*agent.binding, port, &token) {
            Ok(_) => {
                actions.push(FixAction {
                    ol_code: crate::error::ERR_HOOK_WRITE_FAILED.to_string(),
                    kind: FixKind::HookReinstall,
                    // The only thing naming WHICH agent this healed: `FixAction`
                    // has no agent field and gains none.
                    file: settings_path.clone(),
                    backup,
                    reversible: true,
                    applied_at: Utc::now(),
                    note: format!("reinstalled hooks in {}", settings_path.display()),
                });
            }
            Err(e) => {
                tracing::warn!(
                    error = %e.message,
                    code = e.code,
                    "doctor --fix: hook reinstall failed"
                );
            }
        }
    }

    actions
}

/// Re-stage the `openlatch-hook` binary into the canonical install
/// location (`<ol_dir>/bin/openlatch-hook[.exe]`) when missing.
///
/// Thin wrapper over [`hooks::staging::stage_hook_binary`], which `init` calls
/// too — the staging itself must be one implementation, or the "canonical
/// install location" is only canonical on whichever path last touched it.
///
/// A failure records nothing rather than propagating: `--fix` applies what it
/// can and the post-fix diagnostics still surface the missing-binary check, so
/// the user gets a remediation hint instead of an aborted repair run.
pub(crate) fn heal_binaries(ol_dir: &Path) -> Vec<FixAction> {
    let mut actions = Vec::new();

    match hooks::staging::stage_hook_binary(ol_dir) {
        Ok(hooks::staging::StageOutcome::AlreadyStaged { .. }) => {}
        Ok(hooks::staging::StageOutcome::Staged { target, source }) => {
            actions.push(FixAction {
                ol_code: crate::error::ERR_HOOK_BINARY_UNRESOLVABLE.to_string(),
                kind: FixKind::BinaryCopy,
                file: target.clone(),
                backup: None, // freshly staged — no prior file to preserve
                reversible: true,
                applied_at: Utc::now(),
                note: format!("staged {} from {}", target.display(), source.display()),
            });
        }
        Err(e) => {
            tracing::warn!(
                error = %e.message,
                code = e.code,
                "doctor --fix: cannot stage hook binary"
            );
        }
    }

    actions
}

/// Reinstall the OS supervisor when config says `mode=active` but the OS
/// artifact (plist / unit / Task Scheduler task) is missing. No-op in every
/// other state — keep the fix surgical so `--fix` never silently re-opts a
/// user back into persistence they disabled.
pub(crate) fn heal_supervision(ol_dir: &Path) -> Vec<FixAction> {
    use crate::supervision::{select_supervisor, SupervisionMode};
    let mut actions = Vec::new();

    let cfg = match config::Config::load(None, None, false) {
        Ok(c) => c,
        Err(_) => return actions,
    };

    if !matches!(cfg.supervision.mode, SupervisionMode::Active) {
        return actions;
    }

    let Some(supervisor) = select_supervisor() else {
        return actions;
    };

    let status_ok = supervisor.status().map(|s| s.installed).unwrap_or(false);
    if status_ok {
        return actions;
    }

    let exe_path =
        std::env::current_exe().unwrap_or_else(|_| std::path::PathBuf::from("openlatch"));
    if let Err(e) = supervisor.install(&exe_path) {
        tracing::warn!(error = %e.message, code = %e.code, "doctor --fix: supervisor reinstall failed");
        return actions;
    }

    let config_path = ol_dir.join("config.toml");
    let _ = config::persist_supervision_state(
        &config_path,
        &SupervisionMode::Active,
        &supervisor.kind(),
        None,
    );

    actions.push(FixAction {
        ol_code: crate::supervision::ERR_SUPERVISION_INSTALL_FAILED.to_string(),
        kind: FixKind::SupervisionInstall,
        file: std::path::PathBuf::new(),
        backup: None,
        reversible: false,
        applied_at: Utc::now(),
        note: "Reinstalled missing OS supervisor (config said active)".to_string(),
    });

    actions
}

/// Copy `path` to a sibling `<filename>.bak`, returning the backup path.
///
/// Single-level backup — overwrites any prior `.bak`. Multi-level history
/// is intentionally not kept (disk cost outweighs value; the journal
/// itself records intent and timestamps).
pub(crate) fn backup_file(path: &Path) -> Result<PathBuf, OlError> {
    let bak = bak_path_for(path);
    std::fs::copy(path, &bak).map_err(|e| {
        OlError::new(
            crate::error::ERR_INVALID_CONFIG,
            format!(
                "cannot create backup '{}' for '{}': {e}",
                bak.display(),
                path.display()
            ),
        )
    })?;
    Ok(bak)
}

/// Compute the `.bak` sibling path for an arbitrary file path.
///
/// Appends `.bak` to the file name (preserving any existing extension):
/// `config.toml` → `config.toml.bak`, `daemon.token` → `daemon.token.bak`.
pub(crate) fn bak_path_for(path: &Path) -> PathBuf {
    let mut bak = path.to_path_buf();
    let new_name = match path.file_name() {
        Some(name) => format!("{}.bak", name.to_string_lossy()),
        None => "unknown.bak".to_string(),
    };
    bak.set_file_name(new_name);
    bak
}

/// Render the fix run as either a JSON object (for scripting) or a
/// before/after delta (for human consumption).
fn print_fix_results(
    journal: &Journal,
    journal_path: &Path,
    before: Option<&DoctorReport>,
    after: &DoctorReport,
    auto_rollback_triggered: bool,
    output: &OutputConfig,
) {
    let unresolved = after.unresolved();
    let unfixable: Vec<&str> = unresolved.iter().map(String::as_str).collect();

    if output.format == OutputFormat::Json {
        let actions_json: Vec<serde_json::Value> = journal
            .actions
            .iter()
            .map(|a| {
                serde_json::json!({
                    "ol_code": a.ol_code,
                    "kind": a.kind,
                    "file": a.file.display().to_string(),
                    "backup": a.backup.as_ref().map(|p| p.display().to_string()),
                    "reversible": a.reversible,
                    "applied_at": a.applied_at,
                    "note": a.note,
                })
            })
            .collect();
        let backups: Vec<String> = journal
            .actions
            .iter()
            .filter_map(|a| a.backup.as_ref().map(|p| p.display().to_string()))
            .collect();
        let exit_code = if after.all_pass() && !auto_rollback_triggered {
            0
        } else {
            1
        };
        output.print_json(&serde_json::json!({
            "command": "doctor_fix",
            "run_id": journal.run_id,
            "started_at": journal.started_at,
            "journal_path": journal_path.display().to_string(),
            "checks_before": before.map(|r| serde_json::json!({
                "pass": r.pass_count(),
                "fail": r.fail_count(),
            })),
            "checks_after": serde_json::json!({
                "pass": after.pass_count(),
                "fail": after.fail_count(),
            }),
            "fixes_applied": actions_json,
            "fixes_count": journal.actions.len(),
            "unfixable": unfixable,
            "backups": backups,
            "auto_rollback_triggered": auto_rollback_triggered,
            "exit_code": exit_code,
        }));
        return;
    }

    if output.quiet {
        return;
    }

    print_diagnostic_results(after, output);

    if auto_rollback_triggered {
        eprintln!();
        eprintln!("Fix attempted but daemon failed to restart — rolled back to pre-fix state.");
        eprintln!("  Run `openlatch doctor --rescue` to file a bug with diagnostics.");
        return;
    }

    if !journal.actions.is_empty() {
        eprintln!();
        eprintln!("Fixes applied ({}):", journal.actions.len());
        for action in &journal.actions {
            eprintln!("  • {} [{}]", action.note, action.ol_code);
        }
        let backups: Vec<String> = journal
            .actions
            .iter()
            .filter_map(|a| a.backup.as_ref().map(|p| p.display().to_string()))
            .collect();
        if !backups.is_empty() {
            eprintln!();
            eprintln!("Backups created: {}", backups.join(", "));
            eprintln!("Roll back with: openlatch doctor --restore");
        }
    }

    eprintln!();
    if after.all_pass() {
        eprintln!(
            "Summary: {} fix{} applied, 0 issues remaining.",
            journal.actions.len(),
            if journal.actions.len() == 1 { "" } else { "es" }
        );
    } else {
        eprintln!(
            "Summary: {} fix{} applied, {} issue{} remaining.",
            journal.actions.len(),
            if journal.actions.len() == 1 { "" } else { "es" },
            after.fail_count(),
            if after.fail_count() == 1 { "" } else { "s" }
        );
    }
}

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

    // Env vars are process-global and `cargo test` runs tests in parallel
    // threads of ONE process, so two tests mutating OPENLATCH_HOOK_BIN clobber
    // each other: whichever sets it second wins, and the first sees the wrong
    // value (or a removal) mid-run. This failed non-deterministically under the
    // default parallel runner while passing under --test-threads=1, which reads
    // as flake rather than a race. Same pattern as core::telemetry::consent and
    // core::config.
    // ONE lock per process-wide variable, not one per module. This used to be a
    // private mutex here, which was correct while it was the only writer in this
    // binary. It is not any more: the Codex install tests in `hooks::mod` mutate
    // the same `OPENLATCH_HOOK_BIN` under `staging::HOOK_BIN_ENV_LOCK`, and two
    // different mutexes guarding one variable serialise nothing — the exact shape
    // commit 696687b already had to fix once. Aliasing rather than deleting keeps
    // every `ENV_LOCK.lock()` site below reading the same, and makes the sharing a
    // single line rather than six.
    use crate::hooks::staging::HOOK_BIN_ENV_LOCK as ENV_LOCK;
    use tempfile::TempDir;

    fn empty_dir() -> TempDir {
        TempDir::new().expect("tempdir must be created")
    }

    #[test]
    fn test_bak_path_for_appends_bak_suffix() {
        let p = Path::new("/tmp/config.toml");
        assert_eq!(bak_path_for(p), Path::new("/tmp/config.toml.bak"));
    }

    #[test]
    fn test_bak_path_for_handles_no_extension() {
        let p = Path::new("/tmp/daemon.token");
        assert_eq!(bak_path_for(p), Path::new("/tmp/daemon.token.bak"));
    }

    #[test]
    fn test_backup_file_round_trip() {
        let tmp = empty_dir();
        let src = tmp.path().join("config.toml");
        std::fs::write(&src, "port = 7443\n").unwrap();
        let bak = backup_file(&src).expect("backup must succeed");
        assert_eq!(bak, src.with_file_name("config.toml.bak"));
        assert_eq!(std::fs::read_to_string(&bak).unwrap(), "port = 7443\n");
    }

    #[test]
    fn test_heal_state_creates_config_when_missing() {
        let tmp = empty_dir();
        let actions = heal_state(tmp.path());
        let config_path = tmp.path().join("config.toml");
        assert!(config_path.exists(), "config.toml must be created");
        assert!(
            actions
                .iter()
                .any(|a| a.kind == FixKind::ConfigRewrite && a.backup.is_none()),
            "expected ConfigRewrite action with no backup (no prior file)"
        );
    }

    #[test]
    fn test_heal_state_rewrites_corrupt_config_with_backup() {
        let tmp = empty_dir();
        let config_path = tmp.path().join("config.toml");
        std::fs::write(&config_path, "this is not valid TOML {{{").unwrap();
        let actions = heal_state(tmp.path());
        let bak = config_path.with_file_name("config.toml.bak");
        assert!(bak.exists(), ".bak must be created for corrupt config");
        // After rewrite the config must parse
        let raw = std::fs::read_to_string(&config_path).unwrap();
        assert!(toml::from_str::<toml::Value>(&raw).is_ok());
        assert!(actions
            .iter()
            .any(|a| a.kind == FixKind::ConfigRewrite && a.backup.is_some()));
    }

    #[test]
    fn test_heal_state_regenerates_missing_token() {
        let tmp = empty_dir();
        let token_path = tmp.path().join("daemon.token");
        let actions = heal_state(tmp.path());
        assert!(token_path.exists(), "token must be regenerated");
        let token = std::fs::read_to_string(&token_path).unwrap();
        assert_eq!(token.trim().len(), 64, "token must be 64 hex chars");
        assert!(actions.iter().any(|a| a.kind == FixKind::TokenRegenerate));
    }

    #[test]
    fn test_heal_state_regenerates_empty_token_with_backup() {
        let tmp = empty_dir();
        let token_path = tmp.path().join("daemon.token");
        std::fs::write(&token_path, "").unwrap();
        let actions = heal_state(tmp.path());
        let bak = token_path.with_file_name("daemon.token.bak");
        assert!(bak.exists(), "empty token must be backed up");
        let token = std::fs::read_to_string(&token_path).unwrap();
        assert_eq!(token.trim().len(), 64);
        assert!(actions.iter().any(|a| a.kind == FixKind::TokenRegenerate));
    }

    #[test]
    fn test_heal_state_inserts_agent_id_into_existing_config() {
        let tmp = empty_dir();
        let config_path = tmp.path().join("config.toml");
        std::fs::write(&config_path, "[daemon]\nport = 7443\n").unwrap();
        let actions = heal_state(tmp.path());
        let raw = std::fs::read_to_string(&config_path).unwrap();
        assert!(raw.contains("agent_id"), "agent_id must be inserted");
        assert!(actions.iter().any(|a| a.kind == FixKind::AgentIdInsert));
    }

    #[test]
    fn test_heal_state_resets_corrupt_telemetry_json() {
        let tmp = empty_dir();
        let telem_path = tmp.path().join("telemetry.json");
        std::fs::write(&telem_path, "{ broken json").unwrap();
        let actions = heal_state(tmp.path());
        let raw = std::fs::read_to_string(&telem_path).unwrap();
        let parsed: serde_json::Value =
            serde_json::from_str(&raw).expect("telemetry must be valid JSON");
        assert_eq!(parsed.get("enabled"), Some(&serde_json::json!(false)));
        assert!(actions.iter().any(|a| a.kind == FixKind::TelemetryReset));
    }

    #[test]
    fn test_heal_state_removes_stale_pid_file() {
        let tmp = empty_dir();
        let pid_path = tmp.path().join("daemon.pid");
        // Use PID 0 — guaranteed not alive on any platform (kernel-reserved).
        std::fs::write(&pid_path, "0").unwrap();
        let actions = heal_state(tmp.path());
        assert!(!pid_path.exists(), "stale PID file must be removed");
        assert!(actions.iter().any(|a| a.kind == FixKind::PidStaleRemove));
    }

    #[test]
    fn test_heal_state_idempotent_on_clean_install() {
        let tmp = empty_dir();
        // First run brings the install up from scratch.
        let first = heal_state(tmp.path());
        assert!(!first.is_empty(), "first run must apply at least one fix");
        // Second run on the same dir should be a no-op (or no-mutation: only
        // agent_id may already be set after the first run).
        let second = heal_state(tmp.path());
        assert!(
            second.is_empty(),
            "second run on a healthy install must apply zero fixes (got {second:?})"
        );
    }

    #[test]
    fn test_heal_binaries_noop_when_target_exists() {
        let tmp = empty_dir();
        let bin_dir = tmp.path().join("bin");
        std::fs::create_dir_all(&bin_dir).unwrap();
        let bin_name = if cfg!(windows) {
            "openlatch-hook.exe"
        } else {
            "openlatch-hook"
        };
        std::fs::write(bin_dir.join(bin_name), b"existing").unwrap();
        let actions = heal_binaries(tmp.path());
        assert!(actions.is_empty(), "no action when target already exists");
    }

    #[test]
    fn test_heal_binaries_copies_from_env_override() {
        let tmp = empty_dir();
        let src_dir = empty_dir();
        let bin_name = if cfg!(windows) {
            "openlatch-hook.exe"
        } else {
            "openlatch-hook"
        };
        let src = src_dir.path().join(bin_name);
        std::fs::write(&src, b"hook bytes").unwrap();

        // Serialize against the other OPENLATCH_HOOK_BIN test — cargo does NOT
        // isolate env per test, they share one process.
        let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
        let prev = std::env::var("OPENLATCH_HOOK_BIN").ok();
        std::env::set_var("OPENLATCH_HOOK_BIN", &src);

        let actions = heal_binaries(tmp.path());

        if let Some(p) = prev {
            std::env::set_var("OPENLATCH_HOOK_BIN", p);
        } else {
            std::env::remove_var("OPENLATCH_HOOK_BIN");
        }

        let target = tmp.path().join("bin").join(bin_name);
        assert!(target.exists(), "binary must be staged");
        assert_eq!(std::fs::read(&target).unwrap(), b"hook bytes");
        assert!(actions.iter().any(|a| a.kind == FixKind::BinaryCopy));
    }

    #[test]
    fn test_heal_binaries_no_action_when_no_source_locatable() {
        let tmp = empty_dir();
        let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
        let prev = std::env::var("OPENLATCH_HOOK_BIN").ok();
        std::env::set_var("OPENLATCH_HOOK_BIN", "");
        let actions = heal_binaries(tmp.path());
        if let Some(p) = prev {
            std::env::set_var("OPENLATCH_HOOK_BIN", p);
        } else {
            std::env::remove_var("OPENLATCH_HOOK_BIN");
        }
        // We cannot assert strictly that actions is empty: the test runner
        // binary itself may sit next to an `openlatch-hook` artifact in the
        // target dir, in which case heal_binaries will (correctly) stage it.
        // We only assert the staged action, when present, points at the
        // tempdir target — never at a path outside.
        for a in &actions {
            assert!(a.file.starts_with(tmp.path()));
        }
    }

    #[test]
    fn test_journal_save_and_load_round_trip() {
        let tmp = empty_dir();
        let mut journal = Journal::new();
        journal.actions.push(FixAction {
            ol_code: crate::error::ERR_INVALID_CONFIG.to_string(),
            kind: FixKind::ConfigRewrite,
            file: tmp.path().join("config.toml"),
            backup: Some(tmp.path().join("config.toml.bak")),
            reversible: true,
            applied_at: Utc::now(),
            note: "test".to_string(),
        });
        journal.save(tmp.path()).expect("save must succeed");
        let loaded = Journal::load(tmp.path()).expect("load must succeed");
        assert_eq!(loaded.run_id, journal.run_id);
        assert_eq!(loaded.actions.len(), 1);
        assert_eq!(loaded.actions[0].kind, FixKind::ConfigRewrite);
    }

    #[test]
    fn test_journal_load_returns_no_journal_error_when_absent() {
        let tmp = empty_dir();
        let err = Journal::load(tmp.path()).expect_err("load must fail when absent");
        assert_eq!(err.code, crate::error::ERR_DOCTOR_RESTORE_NO_JOURNAL);
    }

    #[test]
    fn test_journal_load_returns_corrupt_error_when_unparsable() {
        let tmp = empty_dir();
        std::fs::write(tmp.path().join(JOURNAL_FILENAME), "{ broken").unwrap();
        let err = Journal::load(tmp.path()).expect_err("load must fail on bad JSON");
        assert_eq!(err.code, crate::error::ERR_DOCTOR_JOURNAL_CORRUPT);
    }

    /// The #165 repair gap, end to end.
    ///
    /// settings.json holds all the entries it is supposed to hold, so the old
    /// substring predicate declared the install healthy — while every command
    /// pointed at a binary that did not exist and every agent tool call died
    /// with `openlatch-hook: command not found`. `doctor` diagnosed it exactly
    /// (`ERR Hook binary missing` ×12) and `--fix` did nothing, which is what
    /// left the tamper-response path as the only repair tool on the machine.
    ///
    /// Also asserts the token is NOT rotated: `init` regenerates it
    /// unconditionally, and agent sessions already running hold the old value
    /// in their process env — sending an operator to `init` to fix a dead hook
    /// command costs them capture on every live session.
    #[test]
    fn heal_hooks_rewrites_a_command_that_points_at_a_missing_binary() {
        let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());

        let ol = empty_dir();
        let claude = empty_dir();
        let src = empty_dir();

        // A real binary to stage from, reached via the documented override.
        let hook_src = src.path().join(crate::hooks::staging::hook_bin_name());
        std::fs::write(&hook_src, b"hook bytes").unwrap();

        // The token an already-running agent session is holding.
        std::fs::write(ol.path().join("daemon.token"), "live-session-token").unwrap();

        // settings.json as `init` left it on the broken machine: every entry
        // present, every command naming a binary that is not there.
        let settings_path = claude.path().join("settings.json");
        let dead = serde_json::json!({
            "env": { "OPENLATCH_TOKEN": "live-session-token", "OPENLATCH_PORT": "7443" },
            "hooks": {
                "PreToolUse": [{ "matcher": "*", "_openlatch": { "entry_id": "a" },
                    "hooks": [{ "type": "command", "command": "\"openlatch-hook\" --event PreToolUse" }] }],
                "UserPromptSubmit": [{ "matcher": "*", "_openlatch": { "entry_id": "b" },
                    "hooks": [{ "type": "command", "command": "\"openlatch-hook\" --event UserPromptSubmit" }] }],
                "Stop": [{ "matcher": "*", "_openlatch": { "entry_id": "c" },
                    "hooks": [{ "type": "command", "command": "\"openlatch-hook\" --event Stop" }] }],
            }
        });
        std::fs::write(&settings_path, serde_json::to_string_pretty(&dead).unwrap()).unwrap();

        // Lock order (daemon/mod.rs): OPENLATCH_DIR -> claude_code -> HOOK_BIN
        // -> codex_cli. This test writes `OPENLATCH_DIR` and took every lock
        // but that one, so it raced every other test that holds it.
        let _dir_env = crate::config::OPENLATCH_DIR_ENV_LOCK
            .lock()
            .unwrap_or_else(|e| e.into_inner());
        let _env = crate::hooks::claude_code::CONFIG_DIR_ENV_LOCK
            .lock()
            .unwrap_or_else(|e| e.into_inner());
        // `heal_hooks` resolves its own agents, so EVERY detector has to be
        // pointed inside this test's tempdirs — not just Claude Code's. Pointed
        // at a path that does not exist, `codex_cli::detect()` answers `None`
        // and the walk stays single-agent, which is what this test is about.
        // Without it the walk reaches the developer's real `~/.codex` and
        // `install_hooks` rewrites it. Codex's lock is taken last, everywhere.
        let _codex_env = crate::hooks::codex_cli::CONFIG_DIR_ENV_LOCK
            .lock()
            .unwrap_or_else(|e| e.into_inner());
        let prev = (
            std::env::var("OPENLATCH_DIR").ok(),
            std::env::var("CLAUDE_CONFIG_DIR").ok(),
            std::env::var("OPENLATCH_HOOK_BIN").ok(),
            std::env::var(crate::hooks::codex_cli::CONFIG_DIR_ENV).ok(),
        );
        std::env::set_var("OPENLATCH_DIR", ol.path());
        std::env::set_var("CLAUDE_CONFIG_DIR", claude.path());
        std::env::set_var("OPENLATCH_HOOK_BIN", &hook_src);
        std::env::set_var(
            crate::hooks::codex_cli::CONFIG_DIR_ENV,
            ol.path().join("absent-codex"),
        );

        let actions = heal_hooks(ol.path());

        let restore = |key: &str, v: Option<String>| match v {
            Some(v) => std::env::set_var(key, v),
            None => std::env::remove_var(key),
        };
        restore("OPENLATCH_DIR", prev.0);
        restore("CLAUDE_CONFIG_DIR", prev.1);
        restore("OPENLATCH_HOOK_BIN", prev.2);
        restore(crate::hooks::codex_cli::CONFIG_DIR_ENV, prev.3);

        assert!(
            actions.iter().any(|a| a.kind == FixKind::HookReinstall),
            "a dangling command must be repaired, not declared healthy"
        );

        // Read the settings back as JSON, not as text.
        //
        // Substring-matching a path into the serialized file passes on Unix and
        // cannot pass on Windows: `hook_src.display()` renders `C:\Users\…`
        // with single separators while the file holds `C:\\Users\\…`, because
        // JSON escapes every backslash. That one assertion kept `Windows
        // Checks` red on `main` for two releases while `heal_hooks` was doing
        // exactly the right thing. Decoding first also sidesteps 8.3 short
        // paths (`RUNNER~1`), which the runner's temp directory really does
        // produce.
        //
        // It is the stronger check too: every command is inspected rather than
        // the file merely containing the path somewhere.
        let settings: serde_json::Value =
            serde_json::from_str(&std::fs::read_to_string(&settings_path).unwrap()).unwrap();
        let commands = hook_commands(&settings);
        assert!(
            !commands.is_empty(),
            "the repair must leave hook commands behind: {settings:#}"
        );
        for command in &commands {
            assert!(
                !command.starts_with("\"openlatch-hook\""),
                "the bare-name command must be gone: {command}"
            );
            assert!(
                command.contains(&hook_src.display().to_string()),
                "commands must now name an existing binary: {command}"
            );
        }
        assert_eq!(
            settings
                .pointer("/env/OPENLATCH_TOKEN")
                .and_then(serde_json::Value::as_str),
            Some("live-session-token"),
            "the token must NOT be rotated — running sessions still hold it"
        );
    }

    /// The Windows failure, made reproducible on any platform.
    ///
    /// `Windows Checks` was red on `main` for two releases over one assertion
    /// that substring-matched a raw path into the serialized settings file. It
    /// could not fail on Unix — there is no separator to escape — so nothing
    /// short of a Windows runner could catch it, and the PR gate does not run
    /// one.
    ///
    /// This pins the mechanism instead of the platform: a command carrying a
    /// path with backslashes is escaped by `serde_json`, so the text search
    /// misses while the decoded search hits. Reintroducing the substring form
    /// now fails here, on the machine of whoever writes it.
    #[test]
    fn a_windows_path_survives_the_json_round_trip_only_when_decoded() {
        let windows_path = r"C:\Users\RUNNER~1\AppData\Local\Temp\.tmpZBZOCa\openlatch-hook.exe";
        let settings = serde_json::json!({
            "hooks": {
                "PreToolUse": [{
                    "hooks": [{
                        "type": "command",
                        "command": format!("\"{windows_path}\" --event PreToolUse"),
                    }]
                }]
            }
        });
        let serialized = serde_json::to_string_pretty(&settings).unwrap();

        assert!(
            !serialized.contains(windows_path),
            "the premise: JSON escapes every separator, so the raw path is not \
             in the serialized text — this is exactly what the old assertion \
             searched for:\n{serialized}"
        );

        let parsed: serde_json::Value = serde_json::from_str(&serialized).unwrap();
        let commands = hook_commands(&parsed);
        assert_eq!(commands.len(), 1);
        assert!(
            commands[0].contains(windows_path),
            "decoded first, the path is found: {}",
            commands[0]
        );
    }

    /// Every `command` string under `hooks.<event>[].hooks[]`, decoded.
    ///
    /// Exists so assertions compare what an agent would actually execute rather
    /// than how `serde_json` chose to spell it — see the comment above.
    fn hook_commands(settings: &serde_json::Value) -> Vec<String> {
        settings
            .get("hooks")
            .and_then(serde_json::Value::as_object)
            .map(|events| {
                events
                    .values()
                    .filter_map(serde_json::Value::as_array)
                    .flatten()
                    .filter_map(|entry| entry.get("hooks")?.as_array())
                    .flatten()
                    .filter_map(|h| h.get("command")?.as_str())
                    .map(str::to_string)
                    .collect()
            })
            .unwrap_or_default()
    }

    /// The predicate must not churn a healthy install: `--fix` is expected to be
    /// safe to run repeatedly.
    #[test]
    fn heal_hooks_leaves_a_healthy_install_alone() {
        let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());

        let ol = empty_dir();
        let claude = empty_dir();

        let staged_dir = ol.path().join("bin");
        std::fs::create_dir_all(&staged_dir).unwrap();
        let staged = staged_dir.join(crate::hooks::staging::hook_bin_name());
        std::fs::write(&staged, b"hook bytes").unwrap();

        std::fs::write(ol.path().join("daemon.token"), "tok").unwrap();

        let settings_path = claude.path().join("settings.json");
        let cmd = format!("\"{}\" --event x", staged.display());
        let healthy = serde_json::json!({
            "hooks": {
                "PreToolUse": [{ "_openlatch": { "entry_id": "a" },
                    "hooks": [{ "type": "command", "command": cmd }] }],
                "UserPromptSubmit": [{ "_openlatch": { "entry_id": "b" },
                    "hooks": [{ "type": "command", "command": cmd }] }],
                "Stop": [{ "_openlatch": { "entry_id": "c" },
                    "hooks": [{ "type": "command", "command": cmd }] }],
            }
        });
        let before = serde_json::to_string_pretty(&healthy).unwrap();
        std::fs::write(&settings_path, &before).unwrap();

        // Lock order (daemon/mod.rs): OPENLATCH_DIR -> claude_code -> HOOK_BIN
        // -> codex_cli. This test writes `OPENLATCH_DIR` and took every lock
        // but that one, so it raced every other test that holds it.
        let _dir_env = crate::config::OPENLATCH_DIR_ENV_LOCK
            .lock()
            .unwrap_or_else(|e| e.into_inner());
        let _env = crate::hooks::claude_code::CONFIG_DIR_ENV_LOCK
            .lock()
            .unwrap_or_else(|e| e.into_inner());
        // See the sibling test: `heal_hooks` detects its own agents, so a
        // `CODEX_HOME` left pointing at the developer's real `~/.codex` makes
        // this "healthy install" walk reinstall hooks into it — which is both a
        // false failure here and a write nobody asked for.
        let _codex_env = crate::hooks::codex_cli::CONFIG_DIR_ENV_LOCK
            .lock()
            .unwrap_or_else(|e| e.into_inner());
        let prev = (
            std::env::var("OPENLATCH_DIR").ok(),
            std::env::var("CLAUDE_CONFIG_DIR").ok(),
            std::env::var(crate::hooks::codex_cli::CONFIG_DIR_ENV).ok(),
        );
        std::env::set_var("OPENLATCH_DIR", ol.path());
        std::env::set_var("CLAUDE_CONFIG_DIR", claude.path());
        std::env::set_var(
            crate::hooks::codex_cli::CONFIG_DIR_ENV,
            ol.path().join("absent-codex"),
        );

        let actions = heal_hooks(ol.path());

        match prev.0 {
            Some(v) => std::env::set_var("OPENLATCH_DIR", v),
            None => std::env::remove_var("OPENLATCH_DIR"),
        }
        match prev.1 {
            Some(v) => std::env::set_var("CLAUDE_CONFIG_DIR", v),
            None => std::env::remove_var("CLAUDE_CONFIG_DIR"),
        }
        match prev.2 {
            Some(v) => std::env::set_var(crate::hooks::codex_cli::CONFIG_DIR_ENV, v),
            None => std::env::remove_var(crate::hooks::codex_cli::CONFIG_DIR_ENV),
        }

        assert!(actions.is_empty(), "healthy install must not be rewritten");
        assert_eq!(std::fs::read_to_string(&settings_path).unwrap(), before);
    }

    // -----------------------------------------------------------------------
    // The seam: `heal_hooks_for` must reach every detected agent
    // -----------------------------------------------------------------------

    /// Redirects everything `heal_hooks_for` resolves from the environment into
    /// the test's own tempdirs, and restores it on unwind as well as on
    /// success: a failing assertion must not leak a redirected state directory
    /// into the next test in this binary.
    ///
    /// `OPENLATCH_SKIP_KEYRING` is not optional here. `install_hooks` resolves
    /// the HMAC key, and with no `hmac.key` in the (fresh) state directory that
    /// read reaches the OS keychain — which on macOS blocks on a GUI
    /// authorization dialog, hanging the run rather than failing it. Bypassing
    /// the keychain in tests is this repo's convention
    /// (`.claude/rules/credential-lookup.md`).
    struct HookEnvGuard(Vec<(&'static str, Option<std::ffi::OsString>)>);

    impl HookEnvGuard {
        fn set(ol_dir: &Path, hook_bin: &Path) -> Self {
            let pairs: [(&'static str, std::ffi::OsString); 3] = [
                ("OPENLATCH_DIR", ol_dir.as_os_str().to_owned()),
                ("OPENLATCH_HOOK_BIN", hook_bin.as_os_str().to_owned()),
                ("OPENLATCH_SKIP_KEYRING", std::ffi::OsString::from("1")),
            ];
            let saved = pairs
                .iter()
                .map(|(key, _)| (*key, std::env::var_os(key)))
                .collect();
            for (key, value) in pairs {
                std::env::set_var(key, value);
            }
            Self(saved)
        }
    }

    impl Drop for HookEnvGuard {
        fn drop(&mut self) {
            for (key, value) in &self.0 {
                match value {
                    Some(v) => std::env::set_var(key, v),
                    None => std::env::remove_var(key),
                }
            }
        }
    }

    /// Two detected agents whose settings files the caller writes.
    ///
    /// The first is the real binding with its `pub` fields set under a tempdir
    /// — no `CLAUDE_CONFIG_DIR` and no config-dir lock, which is the whole
    /// point of taking the slice. The second is `FakeBinding`, the only way to
    /// reach a second `agent_type` on a build that detects one agent; adding an
    /// `AgentKind` variant or a second real binding to get one would put
    /// agent-two code in a unit whose premise is that there is none.
    use crate::hooks::binding::test_support::two_detected_agents as two_agents;

    /// settings.json as `init` left it on the broken machine: every
    /// load-bearing entry present, every command naming a binary that is not
    /// there.
    fn write_dead_settings(path: &Path) {
        let dead = serde_json::json!({
            "env": { "OPENLATCH_TOKEN": "live-session-token", "OPENLATCH_PORT": "7443" },
            "hooks": {
                "PreToolUse": [{ "matcher": "*", "_openlatch": { "entry_id": "a" },
                    "hooks": [{ "type": "command", "command": "\"openlatch-hook\" --event PreToolUse" }] }],
                "UserPromptSubmit": [{ "matcher": "*", "_openlatch": { "entry_id": "b" },
                    "hooks": [{ "type": "command", "command": "\"openlatch-hook\" --event UserPromptSubmit" }] }],
                "Stop": [{ "matcher": "*", "_openlatch": { "entry_id": "c" },
                    "hooks": [{ "type": "command", "command": "\"openlatch-hook\" --event Stop" }] }],
            }
        });
        std::fs::write(path, serde_json::to_string_pretty(&dead).unwrap()).unwrap();
    }

    /// The same file in the state `install_hooks` leaves behind: every
    /// load-bearing entry present, every command naming the binary this install
    /// actually resolves to.
    fn write_healthy_settings(path: &Path, bin: &Path) {
        let cmd = format!("\"{}\" --event x", bin.display());
        let healthy = serde_json::json!({
            "hooks": {
                "PreToolUse": [{ "_openlatch": { "entry_id": "a" },
                    "hooks": [{ "type": "command", "command": cmd }] }],
                "UserPromptSubmit": [{ "_openlatch": { "entry_id": "b" },
                    "hooks": [{ "type": "command", "command": cmd }] }],
                "Stop": [{ "_openlatch": { "entry_id": "c" },
                    "hooks": [{ "type": "command", "command": cmd }] }],
            }
        });
        std::fs::write(path, serde_json::to_string_pretty(&healthy).unwrap()).unwrap();
    }

    /// The Hooks check's own remedy is *"Run `openlatch doctor --fix` to
    /// reinstall them"*. On a host where a second agent is broken, that remedy
    /// has to be true.
    #[test]
    fn heal_hooks_repairs_every_detected_agent() {
        let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());

        let ol = empty_dir();
        let src = empty_dir();
        let agents = empty_dir();

        // A real binary to stage from, reached via the documented override.
        let hook_src = src.path().join(crate::hooks::staging::hook_bin_name());
        std::fs::write(&hook_src, b"hook bytes").unwrap();
        std::fs::write(ol.path().join("daemon.token"), "tok").unwrap();

        let _env = HookEnvGuard::set(ol.path(), &hook_src);

        let detected = two_agents(agents.path());
        for agent in &detected {
            write_dead_settings(&agent.settings_path());
        }

        let actions = heal_hooks_for(&detected, ol.path());

        let healed: Vec<PathBuf> = actions
            .iter()
            .filter(|a| a.kind == FixKind::HookReinstall)
            .map(|a| a.file.clone())
            .collect();
        assert_eq!(
            healed,
            detected
                .iter()
                .map(DetectedAgent::settings_path)
                .collect::<Vec<_>>(),
            "every dead agent must be repaired — `file` is what names which one"
        );
    }

    /// The `return` → `continue` conversion, and nothing else, decides this.
    ///
    /// The test above cannot: both its agents are dead, so the healthy branch
    /// is never reached and a bare `return` there is indistinguishable from a
    /// `continue`. Detection order is fixed with Claude Code first, so the
    /// realistic dual-agent host — Claude Code healthy, the second one broken —
    /// is precisely the case a bare `return` leaves broken forever.
    #[test]
    fn heal_hooks_skips_a_healthy_agent_without_abandoning_the_rest() {
        let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());

        let ol = empty_dir();
        let src = empty_dir();
        let agents = empty_dir();

        let hook_src = src.path().join(crate::hooks::staging::hook_bin_name());
        std::fs::write(&hook_src, b"hook bytes").unwrap();
        std::fs::write(ol.path().join("daemon.token"), "tok").unwrap();

        let _env = HookEnvGuard::set(ol.path(), &hook_src);

        let detected = two_agents(agents.path());
        write_healthy_settings(&detected[0].settings_path(), &hook_src);
        write_dead_settings(&detected[1].settings_path());

        // The fixture proves nothing unless the first agent really is healthy:
        // if `needs_reinstall` were true for it too, the branch this test
        // exists to pin would never be reached.
        assert!(
            !hooks::health::inspect_file(&detected[0].settings_path(), &*detected[0].binding)
                .expect("the healthy settings file must parse")
                .needs_reinstall(),
            "the first agent must start out healthy"
        );

        let actions = heal_hooks_for(&detected, ol.path());

        assert_eq!(
            actions.iter().map(|a| a.file.clone()).collect::<Vec<_>>(),
            vec![detected[1].settings_path()],
            "a healthy first agent must not end the walk — the broken one behind it \
             still needs its fix"
        );
    }
}