trusty-mpm 1.8.2

trusty-mpm: unified multi-agent orchestration platform (core, daemon, CLI, TUI, Telegram)
//! `tm doctor` hook-hygiene probe: tm hook contamination + foreign harness
//! conflicts in project-level Claude settings files (issue #2940).
//!
//! Why: split out of `doctor.rs` to keep it under the 500-SLOC production
//! cap, mirroring the existing `doctor_output_style.rs` / `doctor_fs_checks.rs`
//! splits. A pre-#2940 `tm install` wired tm hooks into every project's
//! `.claude/settings.json`/`settings.local.json` it discovered under `$HOME`;
//! this probe surfaces machines still carrying that damage on the projects
//! `tm doctor` actually knows about (`tm hooks clean` is the fix — its
//! default, no-`--path` mode does the exhaustive `$HOME`-wide sweep this
//! probe deliberately does NOT do), and separately warns when a project's OWN
//! pre-existing claude-mpm hooks would fire inside a tm session
//! (informational only — tm never touches another harness's hooks).
//! What: [`check_hooks_hygiene`] scans `project_dir` (when given) plus every
//! currently-live `active_workspace_paths` — the SAME bounded scope
//! `check_agents`/`check_skills` use — and returns THREE checks:
//! `hooks_contamination` (`Warn` when any file carries a tm-owned hook group),
//! `hooks_foreign_conflict` (`Warn` when any file carries a foreign claude-mpm
//! hook group), `hooks_build_tree_binary` (#7262 — `Warn` naming each hook
//! or `statusLine` command whose executable lives in a Cargo build tree), and
//! `hooks_missing_tm_group` (#7490 — `Warn` naming each lifecycle event a
//! tm-provisioned file carries NO tm hook group for).
//! Test: the `tests` module below covers all four checks against temp
//! directories.

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

use crate::core::doctor::{CheckStatus, DoctorCheck};
use crate::core::standalone::hooks::cleanup::{
    build_tree_hook_commands, build_tree_statusline_command, foreign_hook_event_names,
    tm_hook_event_names,
};

/// Every `.claude/settings*.json` this probe should inspect.
///
/// Why (performance): an earlier revision of this probe walked the entire
/// `$HOME` tree via `discover_claude_settings` on every `tm doctor` call —
/// on a real developer machine with hundreds of checked-out projects that
/// walk alone took 15+ seconds, turning `tm doctor` (and `GET
/// /api/v1/doctor`) into a timeout risk for every caller. `tm doctor` must
/// stay fast, so this probe is scoped to exactly the projects it already
/// knows about: `project_dir` (the workspace doctor was invoked for) and
/// `active_workspace_paths` (every workspace path the daemon currently
/// tracks) — mirroring the scoping [`super::check_agents`] /
/// [`super::check_skills`] already use. The exhaustive `$HOME`-wide sweep
/// (needed to find contamination in projects tm is not currently tracking)
/// remains available as an explicit, user-invoked operation via `tm hooks
/// clean` (no `--path`), which pays that cost once rather than on every
/// diagnostic call.
/// What: returns the deduplicated set of `<dir>/.claude/{settings.json,
/// settings.local.json}` for `project_dir` plus every `active_workspace_paths`
/// entry, filtered to files that actually exist.
pub(crate) fn candidate_settings_files(
    project_dir: Option<&Path>,
    active_workspace_paths: &[PathBuf],
) -> Vec<PathBuf> {
    let mut set: BTreeSet<PathBuf> = BTreeSet::new();
    let roots = project_dir
        .into_iter()
        .chain(active_workspace_paths.iter().map(PathBuf::as_path));
    for root in roots {
        for name in ["settings.json", "settings.local.json"] {
            let candidate = root.join(".claude").join(name);
            if candidate.is_file() {
                set.insert(candidate);
            }
        }
    }
    set.into_iter().collect()
}

/// Read and parse one settings file, tolerating a missing or malformed file.
pub(crate) fn read_settings(path: &Path) -> Option<serde_json::Value> {
    let text = std::fs::read_to_string(path).ok()?;
    let val: serde_json::Value = serde_json::from_str(&text).ok()?;
    val.is_object().then_some(val)
}

/// Probe every reachable project settings file for tm hook contamination and
/// foreign (claude-mpm) hook conflicts (issue #2940).
///
/// Why: an operator running a pre-#2940 build has tm hooks baked into project
/// files that `tm doctor` should surface so `tm hooks clean` gets run; a
/// project carrying its own claude-mpm hooks is a silent double-firing risk
/// the operator needs to know about even though tm must never touch it. See
/// [`candidate_settings_files`] for why the scan is scoped rather than
/// `$HOME`-wide.
/// What: scans [`candidate_settings_files`] and returns two checks:
/// `hooks_contamination` — `Ok` when no file carries a tm-owned hook group,
/// else `Warn` naming up to 3 affected files (plus a count) and suggesting
/// `tm hooks clean`; `hooks_foreign_conflict` — `Ok` when no file carries a
/// foreign claude-mpm hook group, else `Warn` naming up to 3 affected files
/// (plus a count), purely informational (never suggests removal — that is
/// the operator's call).
/// #7262: a THIRD check, `hooks_build_tree_binary`, names every hook or
/// `statusLine` command whose executable lives in a Cargo build tree, with its
/// file path and the command string itself. `hooks_contamination` counts files
/// and cannot say which command is broken, which is the only fact that matters
/// for this shape: the entry is tm's own, and the fault is the path inside it.
/// Test: `check_hooks_hygiene_ok_when_clean`,
/// `check_hooks_hygiene_warns_on_tm_contamination`,
/// `check_hooks_hygiene_warns_on_foreign_conflict`,
/// `check_hooks_hygiene_never_double_counts_active_workspace_dupes`,
/// `check_hooks_hygiene_reports_the_build_tree_incident_shape`,
/// `check_hooks_hygiene_reports_a_build_tree_statusline`,
/// `check_hooks_hygiene_build_tree_check_is_ok_for_an_installed_binary`.
///
/// #7490: a FOURTH check, `hooks_missing_tm_group`, names each lifecycle event
/// a tm-provisioned file has no tm hook group for. It is the inverse of
/// `hooks_contamination` and does not overlap it: contamination reports an
/// entry that should not be there, this reports one that should be and is not.
/// Test: `check_hooks_hygiene_reports_a_missing_sessionstart_group`,
/// `check_hooks_hygiene_missing_group_check_is_ok_for_a_complete_file`,
/// `check_hooks_hygiene_missing_group_check_ignores_a_foreign_project`.
pub(super) fn check_hooks_hygiene(
    project_dir: Option<&Path>,
    active_workspace_paths: &[PathBuf],
) -> (DoctorCheck, DoctorCheck, DoctorCheck, DoctorCheck) {
    check_hooks_hygiene_with_exe(project_dir, active_workspace_paths, None)
}

/// [`check_hooks_hygiene`] with the hook binary pinned by the caller.
///
/// Why (#7849): the toggle-driven probe compares the file against the COMMANDS
/// the writer would produce, and resolving those needs an installed binary. A
/// test process is a build artifact, so with `None` the probe's answer depends
/// on whether the host happens to have `tm` installed — green on a developer
/// machine, a resolution error on CI, for the same fixture. Mirrors the seam
/// [`crate::core::deploy_validate::validate_workspace_with_exe`] carries.
/// What: as [`check_hooks_hygiene`]; production passes `None`.
/// Test: `check_hooks_hygiene_reports_a_missing_toggle_driven_group`.
pub(super) fn check_hooks_hygiene_with_exe(
    project_dir: Option<&Path>,
    active_workspace_paths: &[PathBuf],
    hook_exe: Option<&Path>,
) -> (DoctorCheck, DoctorCheck, DoctorCheck, DoctorCheck) {
    let files = candidate_settings_files(project_dir, active_workspace_paths);

    let mut contaminated: Vec<PathBuf> = Vec::new();
    let mut foreign: Vec<PathBuf> = Vec::new();
    let mut build_tree: Vec<(PathBuf, String)> = Vec::new();
    let mut missing: Vec<(PathBuf, Vec<String>)> = Vec::new();
    for path in &files {
        let Some(val) = read_settings(path) else {
            continue;
        };
        // #7490: empty for a file tm never provisioned, so a foreign project
        // never grows a finding telling it to adopt tm's hooks.
        let mut gaps = crate::core::session_launch::missing_lifecycle_hook_events(&val);
        // #7849: `missing_lifecycle_hook_events` tests the base triad only, so a
        // toggle-driven group — the #7688 `Stop`/`SubagentStop` capture, the
        // #6887 diversion groups — was unreported on a project whose flag
        // flipped on after its settings file was written.
        gaps.extend(toggle_driven_gaps(path, hook_exe));
        if !gaps.is_empty() {
            missing.push((path.clone(), gaps));
        }
        if !tm_hook_event_names(&val).is_empty() {
            contaminated.push(path.clone());
        }
        if !foreign_hook_event_names(&val).is_empty() {
            foreign.push(path.clone());
        }
        // #7262: hooks first, then the statusLine key, so the report reads in
        // the order the file does.
        for cmd in build_tree_hook_commands(&val) {
            build_tree.push((path.clone(), cmd));
        }
        if let Some(cmd) = build_tree_statusline_command(&val) {
            build_tree.push((path.clone(), cmd));
        }
    }

    let contamination_check = if contaminated.is_empty() {
        DoctorCheck::new(
            "hooks_contamination",
            CheckStatus::Ok,
            "no tm hook entries found in project-level settings files",
        )
    } else {
        DoctorCheck::new(
            "hooks_contamination",
            CheckStatus::Warn,
            format!(
                // #4948: name the in-place repair first — an operator reading
                // this is already running `tm doctor`, and a remedy they have
                // to remember a different command for is the reason these
                // findings sat unactioned for weeks.
                "{} project settings file{} carr{} tm hook entries — `tm doctor --fix` \
                 previews the removal for this project and `--yes` applies it; \
                 `tm hooks clean` sweeps every project under $HOME: {}",
                contaminated.len(),
                if contaminated.len() == 1 { "" } else { "s" },
                if contaminated.len() == 1 { "ies" } else { "y" },
                summarize_paths(&contaminated),
            ),
        )
    };

    let foreign_check = if foreign.is_empty() {
        DoctorCheck::new(
            "hooks_foreign_conflict",
            CheckStatus::Ok,
            "no foreign (claude-mpm) hook entries found in project-level settings files",
        )
    } else {
        DoctorCheck::new(
            "hooks_foreign_conflict",
            CheckStatus::Warn,
            format!(
                "{} project settings file{} carr{} claude-mpm hook entries that would fire \
                 inside a tm session — this is informational only, tm never removes another \
                 harness's hooks: {}",
                foreign.len(),
                if foreign.len() == 1 { "" } else { "s" },
                if foreign.len() == 1 { "ies" } else { "y" },
                summarize_paths(&foreign),
            ),
        )
    };

    (
        contamination_check,
        foreign_check,
        build_tree_check(&build_tree),
        missing_group_check(&missing),
    )
}

/// The toggle-driven hook groups one settings file is missing (#7849).
///
/// Why: `missing_lifecycle_hook_events` answers only for the six-event
/// lifecycle triad, which every toggle is additive to — so the group a flipped
/// flag asks for was invisible to `tm doctor` exactly as it was to
/// `tm validate`. This reuses the SAME diff the validator reports from, so the
/// two surfaces cannot disagree about what is missing.
/// What: `<event> (asks for `<command>`)` per missing group, and nothing at all
/// for a file whose project directory cannot be derived, that tm's project tier
/// never provisioned, or for which no stable binary resolves — this check names
/// a COMMAND, which an unresolved binary cannot supply; the missing-hook-set
/// verdict for that case is `check_deployment_completeness`'s
/// `ProjectHookDiagnosticIncomplete`, not a second copy here. The STALE half of
/// the diff is deliberately not reported here: `tm doctor`'s own
/// `hooks_contamination` check already owns "an entry that should not be
/// there", and this check is the inverse of it (#7490).
/// Test: `check_hooks_hygiene_reports_a_missing_toggle_driven_group`.
fn toggle_driven_gaps(settings_path: &Path, hook_exe: Option<&Path>) -> Vec<String> {
    // `<project>/.claude/settings.json` → `<project>`.
    let Some(project_dir) = settings_path.parent().and_then(Path::parent) else {
        return Vec::new();
    };
    let Some(val) = read_settings(settings_path) else {
        return Vec::new();
    };
    let fw = crate::core::paths::FrameworkPaths::for_managed_workspace(project_dir);
    let Ok(gaps) =
        crate::core::session_launch::project_hook_group_gaps(&fw, project_dir, &val, hook_exe)
    else {
        return Vec::new();
    };
    if gaps.is_empty() {
        return Vec::new();
    }
    gaps.missing
        .into_iter()
        .map(|(event, command)| format!("{event} (asks for `{command}`)"))
        .collect()
}

/// Render the `hooks_missing_tm_group` check from the collected gaps (#7490).
///
/// Why: the operator has to see WHICH event is unwired and in which file. The
/// incident was a single missing `SessionStart` group in a file that carried
/// every other group, and the consequence — no savings row, no 💸 segment —
/// is invisible from the file itself. It also has to name the repair, because
/// a finding whose remedy the operator has to guess is how #4948 happened.
/// What: `Ok` when `gaps` is empty. Otherwise `Warn`, listing up to 5
/// `<path>: <event>, <event>` pairs plus an overflow count, and naming
/// `tm doctor --fix`.
/// Test: `check_hooks_hygiene_reports_a_missing_sessionstart_group`,
/// `check_hooks_hygiene_missing_group_check_is_ok_for_a_complete_file`,
/// `check_hooks_hygiene_missing_group_check_ignores_a_foreign_project`.
fn missing_group_check(gaps: &[(PathBuf, Vec<String>)]) -> DoctorCheck {
    if gaps.is_empty() {
        return DoctorCheck::new(
            "hooks_missing_tm_group",
            CheckStatus::Ok,
            "every tm-provisioned settings file carries the full tm hook lifecycle group",
        );
    }
    let shown: Vec<String> = gaps
        .iter()
        .take(5)
        .map(|(path, events)| format!("{}: {}", path.display(), events.join(", ")))
        .collect();
    let overflow = if gaps.len() > 5 {
        format!(" (+{} more)", gaps.len() - 5)
    } else {
        String::new()
    };
    DoctorCheck::new(
        "hooks_missing_tm_group",
        CheckStatus::Warn,
        format!(
            "{} tm-provisioned settings file{} lack{} a tm hook group this project asks \
             for, so `tm hook` never fires for it — a missing `SessionStart` costs the \
             session its savings row and the 💸 statusline segment, and a missing \
             `Stop (asks for … --prompt-feedback)` costs it every prompt-feedback row \
             (#7849). `tm doctor --fix` previews merging the groups back in and `--yes` \
             applies it, preserving every other entry (issue #7490); \
             `tm validate --repair --path <project>` resyncs one project in place. {}{}",
            gaps.len(),
            if gaps.len() == 1 { "" } else { "s" },
            if gaps.len() == 1 { "s" } else { "" },
            shown.join("; "),
            overflow,
        ),
    )
}

/// Render the `hooks_build_tree_binary` check from the collected offenders
/// (issue #7262).
///
/// Why: split out of [`check_hooks_hygiene`] so the message construction — the
/// part with a real contract, since an operator acts on the exact command
/// string — is readable on its own and the scan loop stays a scan loop.
/// What: `Ok` when `offenders` is empty. Otherwise `Warn`, listing up to 5
/// `<path>: <command>` pairs plus an overflow count, and naming
/// `tm doctor --fix` as the repair. #7262 (reopened): that repair now REPOINTS
/// each command at the installed binary rather than removing it, `statusLine`
/// included, and sweeps every settings file on the machine — the message says
/// both, because silently under-delivering on a named remedy is how #4948
/// happened and naming a project-scoped remedy for machine-wide damage is how
/// this one came back.
/// Test: `check_hooks_hygiene_reports_the_build_tree_incident_shape`,
/// `check_hooks_hygiene_reports_a_build_tree_statusline`,
/// `check_hooks_hygiene_build_tree_check_is_ok_for_an_installed_binary`.
fn build_tree_check(offenders: &[(PathBuf, String)]) -> DoctorCheck {
    if offenders.is_empty() {
        return DoctorCheck::new(
            "hooks_build_tree_binary",
            CheckStatus::Ok,
            "no hook or statusLine command points into a Cargo build tree",
        );
    }
    let shown: Vec<String> = offenders
        .iter()
        .take(5)
        .map(|(path, cmd)| format!("{}: `{cmd}`", path.display()))
        .collect();
    let overflow = if offenders.len() > 5 {
        format!(" (+{} more)", offenders.len() - 5)
    } else {
        String::new()
    };
    DoctorCheck::new(
        "hooks_build_tree_binary",
        CheckStatus::Warn,
        format!(
            "{} command{} point into a Cargo build tree and stop working the moment that \
             artifact is rebuilt away — `tm doctor --fix` previews repointing every one of \
             them at the installed tm binary and `--yes` applies it, snapshotting each file \
             first. That repair sweeps every settings file on the machine, not just this \
             project, and repairs a `statusLine` command the same way (#7262). {}{}",
            offenders.len(),
            if offenders.len() == 1 { "" } else { "s" },
            shown.join("; "),
            overflow,
        ),
    )
}

/// Render up to 3 paths plus an overflow count for a check message.
fn summarize_paths(paths: &[PathBuf]) -> String {
    let shown: Vec<String> = paths
        .iter()
        .take(3)
        .map(|p| p.display().to_string())
        .collect();
    if paths.len() > 3 {
        format!("{} (+{} more)", shown.join(", "), paths.len() - 3)
    } else {
        shown.join(", ")
    }
}

#[cfg(test)]
#[path = "doctor_hooks_hygiene_tests.rs"]
mod tests;