devflow 1.8.0

//! Every CLI subcommand handler and the display/rendering helpers they
//! share: `start`, the gate/status/logs/history family, worktree listing,
//! recovery, and `devflow doctor`'s project-aware reconciliation core.
//!
//! D-07: this is deliberately one flat file, not a `commands/`
//! subdirectory. Mapping Phase 18's plans onto clusters showed this
//! cluster absorbed only 2 of 7 plans (pipeline absorbed 3), so a
//! per-subcommand directory buys zero measured wave reduction — and it
//! tends to re-centralise the shared display helpers this file already
//! keeps flat into a `common.rs`, recreating exactly the contention the
//! split is meant to remove.

use crate::CliError;
use crate::config_parse::GATE_ESCALATION_THRESHOLD_SECS;
use crate::parallel::ensure_phase_worktree;
use crate::pipeline_gate::print_dry_run;
use crate::pipeline_launch::{launch_stage, single_active_phase};
use crate::pipeline_outcomes::render_gate_context;
use crate::preflight::{agent_program, ensure_agent_binary};
use crate::staleness::run_git_stdout;
use devflow_core::agent;
use devflow_core::agent_result;
use devflow_core::agents;
use devflow_core::config::{DEVELOP, FEATURE_PREFIX};
use devflow_core::events;
use devflow_core::gates::{GateAction, GateResponse, Gates, OpenGate};
use devflow_core::git::GitFlow;
use devflow_core::history;
use devflow_core::mode::Mode;
use devflow_core::recover;
use devflow_core::stage::Stage;
use devflow_core::state::{AgentKind, State};
use devflow_core::version;
use devflow_core::workflow;
use devflow_core::worktree;
use std::path::{Path, PathBuf};
use std::time::{SystemTime, UNIX_EPOCH};

pub(crate) fn resolve_gate_target(
    positional: Option<String>,
    legacy_project: Option<PathBuf>,
    stage_option: Option<Stage>,
    project: PathBuf,
) -> Result<(Option<Stage>, PathBuf), CliError> {
    let Some(positional) = positional else {
        return Ok((stage_option, project));
    };
    if let Ok(positional_stage) = positional.parse::<Stage>() {
        if let Some(flagged_stage) = stage_option
            && flagged_stage != positional_stage
        {
            return Err(CliError::Message(format!(
                "conflicting stages: positional {positional_stage} and --stage {flagged_stage}"
            )));
        }
        let target = legacy_project.unwrap_or(project);
        return Ok((Some(stage_option.unwrap_or(positional_stage)), target));
    }
    if legacy_project.is_some() {
        return Err(CliError::Message(format!(
            "unsupported stage `{positional}`; expected define, plan, code, validate, or ship"
        )));
    }
    if project.as_path() != Path::new(".") {
        return Err(CliError::Message(
            "project was supplied both positionally and with --project".into(),
        ));
    }
    Ok((stage_option, PathBuf::from(positional)))
}

// ---------------------------------------------------------------------------
// start / pipeline driving
// ---------------------------------------------------------------------------

#[allow(clippy::too_many_arguments)]
/// Whether phase `{NN}`'s GSD planning artifact (a `.planning/phases/{NN}-*/`
/// file ending in `suffix`, e.g. `-CONTEXT.md`) exists on `develop` — the
/// branch phase worktrees fork from. Fail-open on git errors (missing
/// develop, not a repo): pre-flight must never block a run the later, more
/// specific checks would allow.
pub(crate) fn phase_artifact_on_develop(project_root: &Path, phase: u32, suffix: &str) -> bool {
    let prefix = format!(".planning/phases/{phase:02}-");
    let output = std::process::Command::new("git")
        .args([
            "ls-tree",
            "-r",
            "--name-only",
            "develop",
            "--",
            ".planning/phases/",
        ])
        .current_dir(project_root)
        .output();
    let Ok(out) = output else { return true };
    if !out.status.success() {
        return true;
    }
    String::from_utf8_lossy(&out.stdout).lines().any(|path| {
        path.strip_prefix(&prefix)
            .is_some_and(|rest| rest.contains('/') && rest.ends_with(suffix))
    })
}

#[allow(clippy::too_many_arguments)]
pub(crate) fn start(
    project_root: &Path,
    phase: u32,
    agent: AgentKind,
    mode: Mode,
    force: bool,
    worktree: bool,
    dry_run: bool,
    until: Option<Stage>,
) -> Result<(), CliError> {
    let mut state = State::new(phase, agent, mode, project_root.to_path_buf());
    state.stop_until = until;

    if dry_run {
        print_dry_run(&state);
        return Ok(());
    }

    // 14-CR-05: fail on a missing agent binary BEFORE any branch/worktree is
    // scaffolded (launch_stage re-checks for the advance-time launch paths).
    ensure_agent_binary(agent_program(agent))?;

    // 13-06 dogfood pre-flight (Codex leg): a fresh headless Codex run can
    // never pass Define — GSD's discuss-phase is an interview, and Codex's
    // exec mode cannot answer it (`request_user_input is unavailable in
    // Default mode`). Fail in one second with instructions instead of after
    // a burned agent run and a dead-end gate. Checked on `develop` (the
    // branch worktrees fork from), so the result does not depend on what the
    // primary checkout happens to have checked out.
    if agent == AgentKind::Codex {
        if !phase_artifact_on_develop(project_root, phase, "-CONTEXT.md") {
            return Err(CliError::Message(format!(
                "phase {phase} has no CONTEXT.md on develop, and codex cannot run an \
                 interactive discussion headless. Run /gsd-discuss-phase {phase} \
                 interactively first (any agent), or use --agent claude."
            )));
        }
        if !phase_artifact_on_develop(project_root, phase, "-PLAN.md") {
            println!(
                "warning: phase {phase} has no PLAN.md on develop — headless codex \
                 planning is untested and may need input; pre-writing plans is safer"
            );
        }
    }

    // Pre-start divergence check: runs on current HEAD before any git
    // mutation. WR-10 (13-REVIEW.md): only meaningful for the --no-worktree
    // (branch-in-place) flow, where `start` actually branches from the main
    // checkout's current HEAD. In worktree mode (the default) the agent's
    // work always forks fresh from `develop` via `worktree::add`, independent
    // of whatever happens to be checked out in the main repo — checking the
    // main checkout's divergence there is unrelated to what's about to
    // happen and can either hard-fail on a stale unrelated branch or
    // silently no-op if the main checkout happens to be on develop.
    if !worktree && let Ok((_ahead, behind)) = GitFlow::new(project_root).divergence_from_develop()
    {
        if behind > 50 {
            return Err(CliError::Message(format!(
                "develop is {behind} commits ahead — your branch is too far behind. \
                 Rebase onto develop first, or use --force to override."
            )));
        }
        if behind > 10 {
            println!("warning: develop is {behind} commits ahead — consider rebasing first");
        }
    }

    if worktree {
        let wt = ensure_phase_worktree(project_root, phase, force)?;
        println!(
            "created worktree: {} (branch {FEATURE_PREFIX}phase-{phase:02})",
            wt.display(),
        );
        state.worktree_path = Some(wt);
    } else {
        let git = GitFlow::new(project_root);
        let result = if force {
            git.feature_start_force(phase)
        } else {
            git.feature_start(phase)
        };
        match result {
            Ok(branch) => println!("created feature branch: {branch}"),
            Err(err) => {
                if !force {
                    return Err(CliError::Message(format!(
                        "{err}\nUse --force to overwrite the existing branch."
                    )));
                }
                return Err(err.into());
            }
        }
    }

    // WR-11 (13-REVIEW.md), revised: state must be on disk BEFORE the monitor
    // exists. launch_stage spawns the detached monitor, which runs `devflow
    // advance` the moment the agent exits — and advance begins with
    // load_state. Launching first (the previous WR-11 order) raced a
    // fast-exiting agent against this save: the monitor's advance found no
    // state.json, died silently into /dev/null, and the save below then wrote
    // an in-progress state nothing would ever advance. Save first; if the
    // launch fails, clear the just-saved state so `devflow status`/`recover`
    // don't report a phantom run (the failure WR-11 originally targeted).
    workflow::save_state(&state)?;
    events::emit(
        project_root,
        phase,
        "workflow_started",
        workflow_started_payload(&state),
    );
    if let Err(err) = launch_stage(&mut state, None, None) {
        if let Err(clear_err) = workflow::clear_state(project_root, phase) {
            eprintln!("warning: could not clear state after failed launch: {clear_err}");
        }
        return Err(err);
    }
    println!(
        "started phase {} in {mode} mode at {} — monitor will auto-advance",
        state.phase, state.started_at
    );
    println!("  watch live: devflow logs -f --phase {phase}");
    Ok(())
}

// ---------------------------------------------------------------------------
// 17d: build provenance + self-dogfood staleness gate (D-17-D-21).
// ---------------------------------------------------------------------------

/// D-21: the `workflow_started` event payload, including build provenance —
/// factored out of `start()` so the payload shape is directly unit-testable
/// without spawning a real agent (`start()` calls `launch_stage` immediately
/// after emitting this event).
fn workflow_started_payload(state: &State) -> serde_json::Value {
    serde_json::json!({
        "agent": state.agent.to_string(),
        "mode": state.mode.to_string(),
        "worktree": state.worktree_path.as_ref().map(|p| p.display().to_string()),
        "version": env!("CARGO_PKG_VERSION"),
        "commit": env!("DEVFLOW_BUILD_COMMIT"),
        "dirty": env!("DEVFLOW_BUILD_DIRTY"),
        // WR-02: filename only, never the full path (leaks home dir/username
        // into OPERATIONS.md's tail-and-paste file); to_string_lossy (not
        // to_str) so non-UTF-8 names still yield a string, not null.
        "exe_path": std::env::current_exe()
            .ok()
            .and_then(|p| p.file_name().map(|n| n.to_string_lossy().into_owned())),
    })
}

// ---------------------------------------------------------------------------
// reference / cleanup / list / status / recover
// ---------------------------------------------------------------------------

/// Create or refresh the static reference worktree.
pub(crate) fn reference(
    project_root: &Path,
    branch: Option<String>,
    refresh: bool,
) -> Result<(), CliError> {
    let branch = branch.unwrap_or_else(|| DEVELOP.to_string());
    let path = worktree::reference_path(project_root);

    // Detached snapshot: `branch` may already be checked out in the main
    // worktree, so we pin a detached HEAD at its tip rather than checking it out.
    if path.exists() {
        if !refresh {
            println!(
                "reference exists at {} (use --refresh to update it)",
                path.display()
            );
            return Ok(());
        }
        worktree::remove(project_root, &path, true)?;
        worktree::add_detached(project_root, &path, &branch)?;
        println!(
            "refreshed reference worktree at {} (snapshot of {branch})",
            path.display()
        );
    } else {
        worktree::add_detached(project_root, &path, &branch)?;
        println!(
            "created reference worktree at {} (snapshot of {branch})",
            path.display()
        );
    }
    Ok(())
}

/// Parse the phase number encoded in a `.worktrees/phase-NN[-agent]` path.
/// Used only as a fallback join key when no persisted `State.worktree_path`
/// matches the worktree entry (review: Codex MEDIUM — worktree->phase join).
/// Returns `None` for paths that don't follow this naming (e.g. the static
/// `reference` worktree), which correctly excludes it from the liveness
/// guard — a snapshot has no owning phase/agent to be alive.
fn phase_from_worktree_path(worktrees_dir: &Path, path: &Path) -> Option<u32> {
    let name = path.strip_prefix(worktrees_dir).ok()?.to_str()?;
    let rest = name.strip_prefix("phase-")?;
    let digits: String = rest.chars().take_while(|c| c.is_ascii_digit()).collect();
    if digits.is_empty() {
        return None;
    }
    digits.parse().ok()
}

/// Join a `git worktree list` entry to its owning phase `State`, preferring
/// the persisted `worktree_path` (set by `start`/`parallel`) and falling back
/// to worktree-directory-name or branch-name matching only when no
/// `worktree_path` match exists (review: Codex MEDIUM). Returns `None` when
/// no owning state can be found at all (e.g. the phase already shipped and
/// its state was cleared) — callers treat that as "no liveness signal",
/// not as an implicit "safe to remove."
fn state_for_worktree<'a>(
    states: &'a [State],
    worktrees_dir: &Path,
    wt: &worktree::WorktreeInfo,
) -> Option<&'a State> {
    if let Some(state) = states
        .iter()
        .find(|s| s.worktree_path.as_deref() == Some(wt.path.as_path()))
    {
        return Some(state);
    }
    if let Some(phase) = phase_from_worktree_path(worktrees_dir, &wt.path)
        && let Some(state) = states.iter().find(|s| s.phase == phase)
    {
        return Some(state);
    }
    if let Some(branch) = &wt.branch {
        return states
            .iter()
            .find(|s| *branch == format!("{FEATURE_PREFIX}phase-{:02}", s.phase));
    }
    None
}

/// Bounded-backoff retry around `worktree::remove`, absorbing the transient
/// `Directory not empty` race that can occur even after a phase is confirmed
/// dead (a lingering fd/writer from the just-exited agent). NOT a substitute
/// for the liveness guard above — only reached once a phase is confirmed
/// dead (agent dead AND monitor not active). `git worktree prune` is
/// deliberately not used here: it only clears metadata for already-absent
/// directories and would orphan leftover files on disk (Pitfall 3).
fn remove_worktree_with_retry(
    project_root: &Path,
    path: &Path,
    force: bool,
) -> Result<(), worktree::WorktreeError> {
    const ATTEMPTS: u32 = 3;
    const BASE_DELAY_MS: u64 = 50;
    let mut last_err = None;
    for attempt in 0..ATTEMPTS {
        match worktree::remove(project_root, path, force) {
            Ok(()) => return Ok(()),
            Err(err) => {
                last_err = Some(err);
                if attempt + 1 < ATTEMPTS {
                    std::thread::sleep(std::time::Duration::from_millis(
                        BASE_DELAY_MS * 2u64.pow(attempt),
                    ));
                }
            }
        }
    }
    Err(last_err.expect("loop runs ATTEMPTS >= 1 times"))
}

/// Remove phase worktrees (and the reference with --force), deleting their
/// associated feature branches, then prune and clean up merged branches.
///
/// Hard-refuses (D-06, no override flag) removal of any worktree whose owning
/// phase has a live agent (any monitor state, including Unknown/Stuck) or an
/// active monitor (Healthy/BetweenStages) — closing the race where a real
/// `cleanup --force` run could delete a worktree a live agent/monitor is
/// still writing into (review: Codex HIGH, fail-closed on a live agent).
pub(crate) fn cleanup(project_root: &Path, force: bool) -> Result<(), CliError> {
    let git = GitFlow::new(project_root);
    let worktrees_dir = worktree::worktrees_dir(project_root);
    let reference = worktree::reference_path(project_root);
    let states = workflow::list_states(project_root);

    let worktrees = worktree::list(project_root)?;
    let mut removed = 0usize;
    for wt in &worktrees {
        // Only touch worktrees under `.worktrees/` (never the main checkout).
        if !wt.path.starts_with(&worktrees_dir) {
            continue;
        }
        if wt.path == reference && !force {
            println!("keeping reference worktree (use --force to remove it)");
            continue;
        }

        let matched_state = state_for_worktree(&states, &worktrees_dir, wt);
        let phase = matched_state
            .map(|s| s.phase)
            .or_else(|| phase_from_worktree_path(&worktrees_dir, &wt.path));
        let agent_alive = phase
            .and_then(|p| agent_pid_from_file(project_root, p))
            .is_some_and(agent::agent_running);
        let monitor_pid = matched_state.and_then(|s| s.monitor_pid);
        let monitor_alive = monitor_pid.is_some_and(agent::agent_running);
        let phase_liveness = liveness(monitor_pid, monitor_alive, agent_alive);

        // A phase halted via `devflow start --until <stage>` (20c) clears
        // `monitor_pid` and its agent has already exited by design — that
        // reads as `Liveness::Unknown` with `agent_alive == false`, which
        // would otherwise sail straight through the live-agent refusal
        // below. Treat it the same way `doctor`'s `check_dead_agent`/
        // `check_dead_monitor` were taught about `facts.stopped` in this
        // same phase: an intentionally-parked worktree is never implicitly
        // safe to remove — require `--force`, mirroring the `reference`
        // worktree's own precedent above.
        let stopped = matched_state.is_some_and(|s| s.stopped);
        if stopped && !force {
            let phase_label = phase
                .map(|p| p.to_string())
                .unwrap_or_else(|| "?".to_string());
            println!(
                "keeping worktree {} for phase {phase_label} — halted via --until; run `devflow resume --phase {phase_label}` first, or pass --force to discard it",
                wt.path.display()
            );
            continue;
        }

        // Fail-closed on a live agent: refuse whenever the agent is alive
        // (regardless of monitor liveness — Unknown/Stuck included) OR the
        // monitor is actively running the stage (Healthy/BetweenStages).
        // Only Stuck/Unknown WITHOUT a live agent proceeds.
        if agent_alive || matches!(phase_liveness, Liveness::Healthy | Liveness::BetweenStages) {
            let phase_label = phase
                .map(|p| p.to_string())
                .unwrap_or_else(|| "?".to_string());
            return Err(CliError::Message(format!(
                "refusing to remove worktree {} for phase {phase_label} ({}) — run `devflow resume --phase {phase_label}` or wait for it to finish",
                wt.path.display(),
                phase_liveness.describe(),
            )));
        }

        match remove_worktree_with_retry(project_root, &wt.path, force) {
            Ok(()) => {
                print!("removed worktree {}", wt.path.display());
                match &wt.branch {
                    Some(branch) if branch.starts_with(FEATURE_PREFIX) => {
                        match git.delete_branch(branch, force) {
                            Ok(()) => println!(" + deleted branch {branch}"),
                            Err(err) => println!(" (branch {branch} kept: {err})"),
                        }
                    }
                    _ => println!(),
                }
                removed += 1;
            }
            Err(err) => {
                println!(
                    "warning: could not remove worktree {} after retrying — manually delete this directory: {err}",
                    wt.path.display()
                );
            }
        }
    }

    worktree::prune(project_root)?;
    if removed == 0 {
        println!("no worktrees to clean up");
    }
    match git.cleanup_merged() {
        Ok(merged) => {
            for branch in merged {
                println!("deleted merged branch {branch}");
            }
        }
        Err(err) => println!("warning: could not prune merged branches: {err}"),
    }
    Ok(())
}

/// A phase's monitor/agent liveness, distinguishing a dead monitor (nothing
/// will call `devflow advance` when the agent exits) from a normal
/// between-stages moment (18b — "who watches the watcher").
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum Liveness {
    /// Monitor and agent are both alive — the stage is actively running.
    Healthy,
    /// Monitor is alive, agent has exited — normal between-stages moment;
    /// the monitor will advance the phase shortly.
    BetweenStages,
    /// The recorded monitor is dead. Whether or not the agent is also dead,
    /// nothing will call `devflow advance` for this phase — it needs a
    /// manual `devflow resume`.
    Stuck,
    /// No monitor PID has been recorded for this state — either none has
    /// been spawned yet, or the state was written by a binary predating
    /// this field. Never reported as a problem.
    Unknown,
}

impl Liveness {
    pub(crate) fn describe(self) -> &'static str {
        match self {
            Liveness::Healthy => "healthy",
            Liveness::BetweenStages => "between stages",
            Liveness::Stuck => "stuck — needs devflow resume",
            Liveness::Unknown => "unknown (no monitor recorded)",
        }
    }
}

/// Pure liveness predicate — no I/O. `monitor_pid` is matched `None` first
/// so a state written by a pre-18b binary (carrying no `monitor_pid`) can
/// never be misclassified as `Stuck` (T-18-11).
fn liveness(monitor_pid: Option<u32>, monitor_alive: bool, agent_alive: bool) -> Liveness {
    match monitor_pid {
        None => Liveness::Unknown,
        Some(_) => match (monitor_alive, agent_alive) {
            (true, true) => Liveness::Healthy,
            (true, false) => Liveness::BetweenStages,
            (false, _) => Liveness::Stuck,
        },
    }
}

/// Recovery verbs discoverable from a phase's liveness (21a, D-03) — the
/// pure, testable counterpart to `status`'s old inline Stuck `println!`.
/// Always includes `devflow resume` for `Stuck`; additionally includes
/// `devflow advance` when the phase is gate-pending (the operator answers
/// the gate then advances — the primary footgun this closes; widening the
/// predicate further risks suggesting `advance` where nothing proves it is
/// right, per 21-CONTEXT.md's Review Incorporation). Empty for any other
/// liveness, so a healthy/between-stages/unknown phase prints nothing new.
fn recovery_hints(state: &State, liveness: Liveness) -> Vec<String> {
    if liveness != Liveness::Stuck {
        return Vec::new();
    }
    let mut hints = vec![format!("devflow resume --phase {}", state.phase)];
    if state.gate_pending {
        hints.push(format!("devflow advance --phase {}", state.phase));
    }
    hints
}

/// The `ts` of a phase's most recent `stage_launched` event, or `None` when
/// none has been recorded — the real stage-entry time (21a), mirroring
/// `test_support::stage_launched_count`'s scan but keeping the LAST match's
/// `ts` instead of counting matches. Read-only; deliberately never reads
/// `state.started_at`, which is phase-level and set once in `State::new`
/// (the 3/3 cross-AI review MEDIUM, 21-REVIEWS.md).
fn latest_stage_launched_ts(project_root: &Path, phase: u32) -> Option<u64> {
    std::fs::read_to_string(devflow_core::events::events_path(project_root))
        .unwrap_or_default()
        .lines()
        .filter_map(|line| serde_json::from_str::<serde_json::Value>(line).ok())
        .filter(|event| {
            event.get("phase").and_then(serde_json::Value::as_u64) == Some(u64::from(phase))
                && event.get("event").and_then(serde_json::Value::as_str) == Some("stage_launched")
        })
        .filter_map(|event| event.get("ts").and_then(serde_json::Value::as_u64))
        .next_back()
}

/// `status`'s in-stage progress line: real elapsed time since the phase's
/// most recent `stage_launched` event. `None` (no such event yet) renders
/// the stage name with no age, rather than mislabeling phase age as stage
/// age — never pass `state.started_at`'s age here (3/3 review MEDIUM).
fn render_stage_progress_line(stage: Stage, stage_launched_ts: Option<u64>) -> String {
    match stage_launched_ts {
        Some(ts) => format!(
            "  in stage {stage}: {}",
            recover::format_age(&ts.to_string())
        ),
        None => format!("  in stage {stage}"),
    }
}

pub(crate) fn status(project_root: &Path) -> Result<(), CliError> {
    // 13-DEFERRED-CR-03 acceptance: enumerate every active phase, not just
    // the last one started.
    let states = workflow::list_states(project_root);
    let mut current_worktree: Option<PathBuf> = None;
    if states.is_empty() {
        println!("stage: idle");
        println!("project_root: {}", project_root.display());
    } else {
        // 14-CR-10: one pass over events.jsonl for every phase's last event,
        // instead of a full-file scan per phase.
        let mut last_events = events::last_events_by_phase(project_root);
        println!("project_root: {}", project_root.display());
        println!(
            "active phases: {}",
            states
                .iter()
                .map(|s| s.phase.to_string())
                .collect::<Vec<_>>()
                .join(", ")
        );
        for state in &states {
            let gate = if state.gate_pending {
                "pending"
            } else {
                "none"
            };
            println!("\nphase {}:", state.phase);
            println!(
                "  stage: {} | mode: {} | gate: {}",
                state.stage, state.mode, gate
            );
            println!("  agent: {}", agents::adapter_for(state.agent).name());
            if state.consecutive_failures > 0 {
                println!("  validate failures: {}", state.consecutive_failures);
            }
            println!(
                "  started: {} ({})",
                state.started_at,
                recover::format_age(&state.started_at)
            );
            if let Some(ref wt) = state.worktree_path {
                println!("  worktree: {}", wt.display());
            }
            current_worktree = current_worktree.or_else(|| state.worktree_path.clone());
            let agent_pid = agent_pid_from_file(project_root, state.phase);
            match agent_pid {
                Some(pid) => {
                    println!(
                        "  agent_pid: {pid} (running: {})",
                        agent::agent_running(pid)
                    );
                }
                None => println!("  agent_pid: none"),
            }
            match state.monitor_pid {
                Some(pid) => {
                    println!(
                        "  monitor_pid: {pid} (running: {})",
                        agent::agent_running(pid)
                    );
                }
                None => println!("  monitor_pid: none"),
            }
            let agent_alive = agent_pid.is_some_and(agent::agent_running);
            let monitor_alive = state.monitor_pid.is_some_and(agent::agent_running);
            let phase_liveness = liveness(state.monitor_pid, monitor_alive, agent_alive);
            println!("  liveness: {}", phase_liveness.describe());
            println!(
                "{}",
                render_stage_progress_line(
                    state.stage,
                    latest_stage_launched_ts(project_root, state.phase)
                )
            );
            for hint in recovery_hints(state, phase_liveness) {
                println!("    → {hint}");
            }
            if let Some(event) = last_events.remove(&state.phase) {
                let ago = event
                    .get("ts")
                    .and_then(|t| t.as_u64())
                    .map(|t| format!(" ({})", recover::format_age(&t.to_string())))
                    .unwrap_or_default();
                println!("  last action: {}{ago}", events::describe(&event));
            }
        }
    }
    let now = SystemTime::now()
        .duration_since(UNIX_EPOCH)
        .unwrap_or_default()
        .as_secs();
    if let Some(banner) = render_pending_gate_banner(&Gates::list_open(project_root), now) {
        println!("\n{banner}");
    }
    if let Some(section) = render_sequentagent_status(project_root) {
        println!("\n{section}");
    }
    print_open_branches(project_root);
    print_worktrees(project_root, current_worktree.as_deref());
    for hint in cron_instruction_hints(project_root) {
        println!("\n{hint}");
    }
    Ok(())
}

/// Build the persistent status-side signal for gates awaiting an operator.
/// Context is agent-controlled, so it must use the same bounded rendering as
/// gate notifications and failure events.
fn render_pending_gate_banner(open: &[OpenGate], now: u64) -> Option<String> {
    if open.is_empty() {
        return None;
    }

    let mut banner = String::from("==================== PENDING GATE ====================\n");
    for gate in open {
        let timestamp = gate.timestamp.parse::<u64>().ok();
        let escalated = timestamp
            .and_then(|timestamp| now.checked_sub(timestamp))
            .is_some_and(|age| age >= GATE_ESCALATION_THRESHOLD_SECS);
        let marker = if escalated { "!!! ESCALATED" } else { "!!!" };
        let context = render_gate_context(&gate.context, 300);
        let stage = gate.stage.to_string();
        banner.push_str(&format!(
            "{marker}: phase {} {stage} ({})\n  {context}\n  approve: devflow gate approve {} --stage {stage}\n  reject:  devflow gate reject {} --stage {stage} --note <reason>\n",
            gate.phase,
            recover::format_age(&gate.timestamp),
            gate.phase,
            gate.phase,
        ));
    }
    banner.push_str("======================================================");
    Some(banner)
}

/// List every gate awaiting a human response.
pub(crate) fn gate_list(project_root: &Path) -> Result<(), CliError> {
    let open = Gates::list_open(project_root);
    if open.is_empty() {
        println!("no open gates");
        return Ok(());
    }
    println!("{:<6} {:<9} {:<9} CONTEXT", "PHASE", "STAGE", "AGE");
    for gate in &open {
        let context = render_gate_context(&gate.context, 100);
        println!(
            "{:<6} {:<9} {:<9} {context}",
            gate.phase,
            gate.stage.to_string(),
            recover::format_age(&gate.timestamp),
        );
    }
    println!(
        "\nanswer with: devflow gate approve <phase> [--note ...] | \
         devflow gate reject <phase> --note ... (note with \"abort\" ends the phase)"
    );
    Ok(())
}

/// Answer an open gate from the CLI — the dogfood-facing replacement for
/// hand-writing `.devflow/gates/NN-stage.response.json` (15a).
pub(crate) fn gate_respond(
    project_root: &Path,
    phase: u32,
    stage: Option<Stage>,
    approved: bool,
    note: Option<String>,
) -> Result<(), CliError> {
    let stage = match stage {
        Some(stage) => stage,
        None => {
            let open: Vec<_> = Gates::list_open(project_root)
                .into_iter()
                .filter(|g| g.phase == phase)
                .collect();
            match open.as_slice() {
                [] => {
                    return Err(CliError::Message(format!(
                        "no open gate for phase {phase} — see `devflow gate list`"
                    )));
                }
                [one] => one.stage,
                many => {
                    return Err(CliError::Message(format!(
                        "phase {phase} has several open gates ({}) — pass --stage",
                        many.iter()
                            .map(|g| g.stage.to_string())
                            .collect::<Vec<_>>()
                            .join(", ")
                    )));
                }
            }
        }
    };
    let responded_by = std::env::var("USER")
        .ok()
        .filter(|user| !user.is_empty())
        .unwrap_or_else(|| "devflow-cli".into());
    let response = GateResponse {
        approved,
        note,
        responded_by: Some(responded_by),
    };
    let path = Gates::respond(project_root, phase, stage, &response)?;
    events::emit(
        project_root,
        phase,
        "gate_response_written",
        serde_json::json!({
            "stage": stage.to_string(),
            "approved": approved,
            "via": "cli",
        }),
    );
    let outcome = match GateAction::from_response(&response) {
        GateAction::Advance => "workflow will advance",
        GateAction::LoopBack(_) => "workflow will loop back to Code",
        GateAction::Abort(_) => "phase will abort",
    };
    println!(
        "{} gate for phase {phase} {stage} — {outcome} once the waiting monitor polls it \
         (response at {})",
        if approved { "approved" } else { "rejected" },
        path.display()
    );
    Ok(())
}

/// Print an open gate's full, untruncated (but sanitized) context — the
/// discoverability counterpart to `gate_list`'s 100-char table truncation
/// (21a, D-03). Mirrors `gate_respond`'s stage auto-resolve-single-open-gate
/// logic (`[]` → error pointing at `devflow gate list`; `[one]` → that
/// stage; `many` → error listing stages and asking for `--stage`) so the two
/// commands' gate-resolution behavior can never drift.
pub(crate) fn gate_show(
    project_root: &Path,
    phase: u32,
    stage: Option<Stage>,
) -> Result<(), CliError> {
    let stage = match stage {
        Some(stage) => stage,
        None => {
            let open: Vec<_> = Gates::list_open(project_root)
                .into_iter()
                .filter(|g| g.phase == phase)
                .collect();
            match open.as_slice() {
                [] => {
                    return Err(CliError::Message(format!(
                        "no open gate for phase {phase} — see `devflow gate list`"
                    )));
                }
                [one] => one.stage,
                many => {
                    return Err(CliError::Message(format!(
                        "phase {phase} has several open gates ({}) — pass --stage",
                        many.iter()
                            .map(|g| g.stage.to_string())
                            .collect::<Vec<_>>()
                            .join(", ")
                    )));
                }
            }
        }
    };
    let gate = Gates::list_open(project_root)
        .into_iter()
        .find(|g| g.phase == phase && g.stage == stage)
        .ok_or_else(|| {
            CliError::Message(format!(
                "no open gate for phase {phase} stage {stage} — see `devflow gate list`"
            ))
        })?;
    println!("{}", render_gate_show(&gate));
    Ok(())
}

/// Pure render for `gate_show`'s output block — the FULL context via
/// `render_gate_context(.., usize::MAX)` (sanitize, never truncate; contrast
/// `gate_list`'s `render_gate_context(.., 100)`). Factored out of `gate_show`
/// so the untruncated-context guarantee is unit-testable without capturing
/// process stdout.
fn render_gate_show(gate: &OpenGate) -> String {
    format!(
        "phase {} {} ({})\n{}",
        gate.phase,
        gate.stage,
        recover::format_age(&gate.timestamp),
        render_gate_context(&gate.context, usize::MAX),
    )
}

/// Print (or follow) a phase's captured agent output.
pub(crate) fn logs(
    project_root: &Path,
    phase: Option<u32>,
    follow: bool,
    stderr: bool,
) -> Result<(), CliError> {
    let phase = match phase {
        Some(p) => p,
        None => default_logs_phase(project_root)?,
    };
    let path = if stderr {
        agent_result::stderr_path(project_root, phase)
    } else {
        agent_result::stdout_path(project_root, phase)
    };
    if !path.exists() && !follow {
        return Err(CliError::Message(format!(
            "no capture file for phase {phase} at {}",
            path.display()
        )));
    }
    eprintln!("== phase {phase}: {} ==", path.display());
    let mut offset = print_capture_from(&path, 0)?;
    if !follow {
        return Ok(());
    }
    // Follow until the agent's exit code lands AND one further quiescent
    // poll produced no new bytes — the natural end of a run. (An operator
    // can always Ctrl-C sooner.)
    let exit_path = agent_result::exit_code_path(project_root, phase);
    loop {
        std::thread::sleep(std::time::Duration::from_millis(500));
        // 14-CR-03: a stage transition archives and recreates the capture
        // file (launch_stage → archive_phase_files), so a shrunken file
        // means the next stage started — reset to the top instead of
        // seeking past EOF forever and silently skipping its output.
        let base = rollover_offset(&path, offset);
        if base != offset {
            eprintln!("== capture restarted (next stage) — following from the top ==");
        }
        let new_offset = print_capture_from(&path, base)?;
        // Quiescent only if no rollover happened AND no new bytes appeared.
        if exit_path.exists() && base == offset && new_offset == offset {
            if let Ok(code) = std::fs::read_to_string(&exit_path) {
                eprintln!("== agent exited with code {} ==", code.trim());
            }
            return Ok(());
        }
        offset = new_offset;
    }
}

/// Render the read-only cross-attempt view for one phase.
pub(crate) fn history_cmd(project_root: &Path, phase: Option<u32>) -> Result<(), CliError> {
    let phase = match phase {
        Some(phase) => phase,
        None => single_active_phase(project_root)?.ok_or_else(|| {
            CliError::Message("no active phase — pass a phase number to `devflow history`".into())
        })?,
    };
    println!(
        "{}",
        history::render_timeline(&history::attempt_timeline(project_root, phase))
    );
    Ok(())
}

/// Detect capture-file rollover for `logs --follow` (14-CR-03): a file
/// shorter than the follower's offset was deleted and recreated by the next
/// stage's monitor, so following must restart from 0. A missing file (the
/// mid-rollover gap) keeps the current offset — the recreated file's shorter
/// length triggers the reset on a later poll if output restarted.
fn rollover_offset(path: &Path, offset: u64) -> u64 {
    match std::fs::metadata(path) {
        Ok(meta) if meta.len() < offset => 0,
        _ => offset,
    }
}

/// Print the capture file's contents from `offset`, returning the new offset.
/// A missing file is treated as empty (it may not exist yet under --follow).
fn print_capture_from(path: &Path, offset: u64) -> Result<u64, CliError> {
    let stdout = std::io::stdout();
    write_capture_from(path, offset, &mut stdout.lock())
}

fn write_capture_from(
    path: &Path,
    offset: u64,
    output: &mut impl std::io::Write,
) -> Result<u64, CliError> {
    use std::io::{Read, Seek, SeekFrom};
    let Ok(mut file) = std::fs::File::open(path) else {
        return Ok(offset);
    };
    file.seek(SeekFrom::Start(offset))
        .map_err(|err| CliError::Message(format!("could not seek capture file: {err}")))?;
    let mut buf = Vec::new();
    file.read_to_end(&mut buf)
        .map_err(|err| CliError::Message(format!("could not read capture file: {err}")))?;
    if !buf.is_empty() {
        let _ = output.write_all(&buf);
        let _ = output.flush();
    }
    Ok(offset + buf.len() as u64)
}

/// Pick the phase `devflow logs` should show when none is given: the single
/// active phase, else the phase with the most recently modified capture file.
fn default_logs_phase(project_root: &Path) -> Result<u32, CliError> {
    if let Some(phase) = single_active_phase(project_root)? {
        return Ok(phase);
    }
    // No active state: fall back to the newest capture file on disk.
    let devflow = workflow::devflow_dir(project_root);
    let mut newest: Option<(std::time::SystemTime, u32)> = None;
    if let Ok(entries) = std::fs::read_dir(&devflow) {
        for entry in entries.flatten() {
            let name = entry.file_name();
            let Some(name) = name.to_str() else { continue };
            let Some(phase) = name
                .strip_prefix("phase-")
                .and_then(|rest| rest.strip_suffix("-stdout"))
                .and_then(|num| num.parse::<u32>().ok())
            else {
                continue;
            };
            let Ok(modified) = entry.metadata().and_then(|m| m.modified()) else {
                continue;
            };
            if newest.is_none_or(|(when, _)| modified > when) {
                newest = Some((modified, phase));
            }
        }
    }
    newest.map(|(_, phase)| phase).ok_or_else(|| {
        CliError::Message("no active phase and no capture files — nothing to show".into())
    })
}

/// Read the launched agent PID the monitor recorded for `phase`, if present.
fn agent_pid_from_file(project_root: &Path, phase: u32) -> Option<u32> {
    let path = agent_result::agent_pid_path(project_root, phase);
    std::fs::read_to_string(path).ok()?.trim().parse().ok()
}

/// Enumerate every recorded `sequentagent` slot and render one status line
/// per phase, naming the running agent slot (A/B), its `AgentKind`, and
/// liveness (21c, D-06) — a sequentagent phase has no entry in
/// `workflow::list_states`, so without this the second agent is otherwise
/// entirely invisible to `status`. Pure and read-only: no writes, no
/// `save_state`/`transition` call.
///
/// Scans `.devflow/phase-*-sequentagent` (mirroring `default_logs_phase`'s
/// `read_dir` + `strip_prefix`/`strip_suffix` idiom) since there is no
/// `State` to enumerate this from.
///
/// Liveness is distinguished by cross-referencing the slot record against
/// the existing agent-pid file (`agent_pid_from_file` + `agent::agent_running`):
/// - `running` — the agent-pid file exists and the process is alive.
/// - `starting` — the slot record exists but the agent-pid file has not
///   appeared yet (the monitor writes it asynchronously, after the slot
///   record — cross-AI review LOW pid-race); an honest transient rather than
///   a misleading "dead" agent.
/// - `not running` — the agent-pid file exists but the process is dead (a
///   stale record never renders a false-live agent, T-21c-02).
///
/// `doctor` integration is intentionally out of scope for this plan: `status`
/// is the live-observability command, while `doctor` reconciles persisted
/// `State` — surfacing this in `doctor` without a matching `--json` key would
/// reintroduce the WR-01 human/json split (21-04-PLAN.md Review
/// Incorporation).
fn render_sequentagent_status(project_root: &Path) -> Option<String> {
    let devflow = workflow::devflow_dir(project_root);
    let mut phases: Vec<u32> = std::fs::read_dir(&devflow)
        .into_iter()
        .flatten()
        .flatten()
        .filter_map(|entry| {
            let name = entry.file_name();
            name.to_str()?
                .strip_prefix("phase-")?
                .strip_suffix("-sequentagent")?
                .parse::<u32>()
                .ok()
        })
        .collect();
    phases.sort_unstable();
    phases.dedup();

    let lines: Vec<String> = phases
        .into_iter()
        .filter_map(|phase| {
            let slot = agent_result::read_sequentagent_slot(project_root, phase)?;
            let pid = agent_pid_from_file(project_root, phase);
            let (state, pid_suffix) = match pid {
                Some(pid) if agent::agent_running(pid) => ("running", format!(" (pid {pid})")),
                Some(pid) => ("not running", format!(" (pid {pid})")),
                None => ("starting", String::new()),
            };
            Some(format!(
                "sequentagent phase {phase}: agent {} ({}) {state}{pid_suffix}",
                slot.slot, slot.agent
            ))
        })
        .collect();

    if lines.is_empty() {
        None
    } else {
        Some(lines.join("\n"))
    }
}

fn cron_instruction_hints(project_root: &Path) -> Vec<String> {
    devflow_core::ship::list_cron_instructions(project_root)
        .iter()
        .map(|instructions| cron_hint_line(instructions, project_root))
        .collect()
}

/// Build one cron-instruction hint line, appending a sanitized rate-limit
/// reset segment when `instructions.retry_after` is non-empty (21a, D-03) —
/// the reset time is already computed and persisted (`CronInstructions.
/// retry_after`, ship.rs), this only presents it; no new detection logic.
/// Pure so it's unit-testable without capturing process stdout.
fn cron_hint_line(
    instructions: &devflow_core::ship::CronInstructions,
    project_root: &Path,
) -> String {
    let base = format!(
        "Cron instruction pending (phase {}): hermes cron create --from-devflow {}",
        instructions.phase,
        project_root.display()
    );
    let retry_after = instructions.retry_after.trim();
    if retry_after.is_empty() {
        base
    } else {
        let reset = render_gate_context(retry_after, 100);
        format!("{base} (rate-limit resets: {reset})")
    }
}

/// Print active phase worktrees with branch and inferred phase/agent.
fn print_worktrees(project_root: &Path, current: Option<&Path>) {
    let worktrees_dir = worktree::worktrees_dir(project_root);
    let worktrees = match worktree::list(project_root) {
        Ok(w) => w,
        Err(_) => return,
    };
    let active: Vec<_> = worktrees
        .iter()
        .filter(|w| w.path.starts_with(&worktrees_dir))
        .collect();
    if active.is_empty() {
        return;
    }
    println!("\nactive worktrees:");
    for wt in active {
        let label = wt
            .path
            .file_name()
            .map(|n| describe_worktree_dir(&n.to_string_lossy()))
            .unwrap_or_default();
        let branch = wt.branch.as_deref().unwrap_or("(detached)");
        let marker = if current == Some(wt.path.as_path()) {
            " *"
        } else {
            ""
        };
        println!("  {} [{branch}]{label}{marker}", wt.path.display());
    }
}

/// Turn a worktree dir name like `phase-07-claude` into ` — phase 7, agent claude`.
fn describe_worktree_dir(name: &str) -> String {
    let Some(rest) = name.strip_prefix("phase-") else {
        return String::new();
    };
    match rest.split_once('-') {
        Some((phase, agent)) => {
            format!(" — phase {}, agent {agent}", phase.trim_start_matches('0'))
        }
        None => format!(" — phase {}", rest.trim_start_matches('0')),
    }
}

pub(crate) fn list(project_root: &Path) -> Result<(), CliError> {
    let git = GitFlow::new(project_root);
    let branches = git.list_feature_branches()?;
    if branches.is_empty() {
        println!("no open feature branches");
        return Ok(());
    }
    println!(
        "{:<25} {:>6} {:>7}  LAST COMMIT",
        "BRANCH", "AHEAD", "BEHIND"
    );
    for b in &branches {
        println!(
            "{:<25} {:>6} {:>7}  {}",
            b.name, b.ahead, b.behind, b.last_commit
        );
    }
    Ok(())
}

fn print_open_branches(project_root: &Path) {
    let git = GitFlow::new(project_root);
    let branches = match git.list_feature_branches() {
        Ok(b) => b,
        Err(_) => return,
    };
    if branches.is_empty() {
        return;
    }
    println!("\nopen branches:");
    for b in &branches {
        let staleness = if b.behind > 0 {
            format!(" ({} behind develop)", b.behind)
        } else {
            String::new()
        };
        println!("  {} — {} ahead{staleness}", b.name, b.ahead);
    }
}

pub(crate) fn recover_cmd(
    project_root: &Path,
    do_clean: bool,
    phase: Option<u32>,
) -> Result<(), CliError> {
    if do_clean {
        let warnings = match phase {
            // Explicit phase: clear it regardless of staleness (14-CR-01's
            // escape hatch for a wedged-but-fresh run).
            Some(phase) => recover::clean_phase(project_root, phase)?,
            // Implicit sweep: stale phases only.
            None => recover::clean(project_root)?,
        };
        for warning in &warnings {
            println!("warning: {warning}");
        }
        match phase {
            Some(phase) => println!("cleaned up workflow state for phase {phase}"),
            None => println!("cleaned up stale workflow state"),
        }
        return Ok(());
    }

    let statuses = match recover::inspect_all(project_root) {
        Ok(s) => s,
        Err(recover::RecoverError::NothingToRecover) => {
            println!("no state to recover — project is idle");
            return Ok(());
        }
        Err(err) => {
            return Err(CliError::Message(format!(
                "recover inspection failed: {err}"
            )));
        }
    };

    let mut any_stale = false;
    for status in &statuses {
        if let Some(only) = phase
            && status.state.phase != only
        {
            continue;
        }
        println!("phase: {}", status.state.phase);
        println!("  stage: {}", status.state.stage);
        println!("  mode: {}", status.state.mode);
        println!(
            "  agent: {}",
            agents::adapter_for(status.state.agent).name()
        );
        println!("  started: {} ({})", status.state.started_at, status.age);
        match agent_pid_from_file(project_root, status.state.phase) {
            Some(pid) => {
                let running = agent::agent_running(pid);
                println!("  agent_pid: {pid} (running: {running})");
                if !running {
                    println!("  agent is not running — the monitor may have already advanced");
                }
            }
            None => println!("  agent_pid: none"),
        }
        if status.is_stale {
            any_stale = true;
            println!("  state is stale");
        }
    }

    if any_stale {
        println!(
            "\nstale state found — `devflow recover --clean` clears stale phases only; \
             use `--clean --phase N` for a specific phase"
        );
    }

    Ok(())
}

/// Run the local quality gate: cargo test, clippy, and fmt --check.
pub(crate) fn test_cmd(project_root: &Path) -> Result<(), CliError> {
    let checks = [
        ("cargo test", "cargo test"),
        (
            "cargo clippy",
            "cargo clippy --workspace --all-targets -- -D warnings",
        ),
        ("cargo fmt --check", "cargo fmt --check"),
    ];
    let mut failures = Vec::new();
    for (label, cmd) in checks {
        println!("=== {label} ===");
        let status = std::process::Command::new("sh")
            .arg("-c")
            .arg(cmd)
            .current_dir(project_root)
            .status()
            .map_err(|err| CliError::Message(format!("could not run `{cmd}`: {err}")))?;
        if status.success() {
            println!("  ✓ {label}");
        } else {
            println!("  ✗ {label}");
            failures.push(label);
        }
    }
    if failures.is_empty() {
        println!("\nall checks passed");
        Ok(())
    } else {
        Err(CliError::Message(format!(
            "quality checks failed: {}",
            failures.join(", ")
        )))
    }
}

// ---------------------------------------------------------------------------
// doctor
// ---------------------------------------------------------------------------

/// One tool/environment check from `doctor`'s pre-existing audit (git,
/// cargo, agent CLIs, `RUST_LOG`, ...). Module-level (WR-01, 18-fix) so
/// `checks_json_value` and `doctor_json_body` can compose it into
/// `doctor --json`'s single output document without living inside `doctor`
/// itself.
pub(crate) struct Check {
    pub(crate) name: String,
    pub(crate) status: String,
    pub(crate) version: Option<String>,
    pub(crate) install_hint: Option<String>,
}

/// Audit the environment and report what's installed, missing, or broken.
pub(crate) fn doctor(project_root: &Path, json: bool) -> Result<(), CliError> {
    use std::process::Command;

    fn cmd_check(name: &str, cmd: &str, version_arg: &str, install_hint: &str) -> Check {
        match Command::new(cmd).arg(version_arg).output() {
            Ok(out) if out.status.success() => {
                let version = String::from_utf8_lossy(&out.stdout)
                    .lines()
                    .next()
                    .unwrap_or("unknown")
                    .trim()
                    .to_string();
                Check {
                    name: name.into(),
                    status: "ok".into(),
                    version: Some(version),
                    install_hint: None,
                }
            }
            Ok(out) => {
                let detail = String::from_utf8_lossy(&out.stderr)
                    .lines()
                    .next()
                    .unwrap_or("unknown")
                    .trim()
                    .to_string();
                Check {
                    name: name.into(),
                    status: "warn".into(),
                    version: Some(detail),
                    install_hint: Some(format!(
                        "`{cmd} {version_arg}` exited non-zero — reinstall or check PATH"
                    )),
                }
            }
            Err(_) => Check {
                name: name.into(),
                status: "missing".into(),
                version: None,
                install_hint: Some(install_hint.into()),
            },
        }
    }

    fn bool_check(name: &str, ok: bool, version: &str, install_hint: &str) -> Check {
        Check {
            name: name.into(),
            status: if ok { "ok".into() } else { "missing".into() },
            version: Some(version.into()),
            install_hint: if ok { None } else { Some(install_hint.into()) },
        }
    }

    let devflow_version = env!("CARGO_PKG_VERSION");

    // RUST_LOG environment check: validate the value is a parsable log directive.
    let (rust_log_status, rust_log_version, rust_log_hint) = match std::env::var("RUST_LOG") {
        Ok(ref val) if val.is_empty() => (
            "warn",
            Some("empty (logging disabled)".into()),
            Some("Set RUST_LOG=info for better diagnostics".into()),
        ),
        Ok(val) => {
            let all_valid = val.split(',').all(|directive| {
                let directive = directive.trim();
                if let Some((_target, level)) = directive.split_once('=') {
                    matches!(level.trim(), "error" | "warn" | "info" | "debug" | "trace")
                } else {
                    matches!(directive, "error" | "warn" | "info" | "debug" | "trace")
                }
            });
            if all_valid {
                ("ok", Some(val), None)
            } else {
                (
                    "warn",
                    Some(val),
                    Some("RUST_LOG value may be invalid — expected error, warn, info, debug, or trace".into()),
                )
            }
        }
        Err(_) => (
            "missing",
            Some("not set — defaulting to info".into()),
            Some("Set RUST_LOG=info for better diagnostics".into()),
        ),
    };

    let checks: Vec<Check> = vec![
        cmd_check(
            "git",
            "git",
            "--version",
            "Install from https://git-scm.com/downloads",
        ),
        bool_check("sh (POSIX shell)", cfg!(unix), "built-in", "Unsupported OS"),
        cmd_check(
            "cargo/rust",
            "cargo",
            "--version",
            "curl https://sh.rustup.rs -sSf | sh",
        ),
        cmd_check(
            "gh CLI",
            "gh",
            "--version",
            "brew install gh / apt install gh",
        ),
        cmd_check(
            "claude",
            "claude",
            "--version",
            "npm i -g @anthropic-ai/claude-code",
        ),
        cmd_check("codex", "codex", "--version", "npm i -g @openai/codex"),
        cmd_check(
            "opencode",
            "opencode",
            "--version",
            "cargo install opencode",
        ),
        Check {
            name: format!("devflow v{devflow_version}"),
            status: "ok".into(),
            version: Some(devflow_version.into()),
            install_hint: None,
        },
        Check {
            name: "RUST_LOG".into(),
            status: rust_log_status.into(),
            version: rust_log_version,
            install_hint: rust_log_hint,
        },
    ];

    let facts = collect_phase_facts(project_root);
    let doc_findings = collect_planning_doc_findings(project_root);

    if json {
        // WR-01 (18-fix): a single top-level JSON document —
        // `{"environment": [...], "reconciliation": [...],
        // "planning_doc_staleness": [...]}` — instead of the pre-fix
        // behavior of printing the tool checks as one top-level `[...]`
        // array and then printing a SECOND, independent top-level array
        // right after it. That concatenation is not valid single-document
        // JSON for any parser that isn't NDJSON-aware (`json.load` raised
        // "Extra data"). 21b's planning-doc check (D-05) extends this SAME
        // object with a third key rather than forking a second array.
        let body = doctor_json_body(&checks, &facts, &doc_findings);
        println!(
            "{}",
            serde_json::to_string_pretty(&body).expect("doctor --json body must serialize")
        );
    } else {
        for c in &checks {
            let icon = match c.status.as_str() {
                "ok" => "✓",
                "missing" => "✗",
                "warn" => "⚠",
                _ => "?",
            };
            let version_str = c.version.as_deref().unwrap_or("-");
            print!("  {:<20} {:<20} {}", c.name, version_str, icon);
            #[allow(clippy::collapsible_if)]
            if c.status == "missing" || c.status == "warn" {
                if let Some(hint) = &c.install_hint {
                    print!(" — {}", hint);
                }
            }
            println!();
        }
        print!("{}", render_reconciliation_text(&facts));
        print!("{}", render_planning_doc_text(&doc_findings));
    }

    Ok(())
}

// ---------------------------------------------------------------------------
// release --check (20d)
// ---------------------------------------------------------------------------

/// Read-only release-cut preflight. Ceiling is `--check` only (D-03): no
/// state-mutating helper, no `git tag`/publish, and (once Task 2/3 land) no
/// `git fetch` — every check here reads already-available local state.
/// Follows `doctor`'s `Check`-list-then-report shape (reuses the same
/// `Check` struct) so the two commands stay visually consistent.
pub(crate) fn release_check(project_root: &Path) -> Result<(), CliError> {
    let checks: Vec<Check> = vec![
        check_self_pin(project_root),
        check_divergence(project_root),
        check_publish_order(project_root),
        check_signing(project_root),
    ];

    let mut failed = false;
    for c in &checks {
        let icon = match c.status.as_str() {
            "ok" => "✓",
            "warn" => "⚠",
            "fail" => "✗",
            _ => "?",
        };
        let detail = c.version.as_deref().unwrap_or("-");
        println!("  {:<32} {icon}  {detail}", c.name);
        if matches!(c.status.as_str(), "warn" | "fail")
            && let Some(hint) = &c.install_hint
        {
            println!("      — {hint}");
        }
        if c.status == "fail" {
            failed = true;
        }
    }

    if failed {
        Err(CliError::Message(
            "release preflight failed — see checks above".into(),
        ))
    } else {
        println!("\nrelease preflight passed");
        Ok(())
    }
}

/// Self-pin check (asserts 20a's invariant): every local-path
/// `[workspace.dependencies]` self-pin must equal `[workspace.package]
/// version`, compared dynamically — never against a hardcoded expected
/// version.
fn check_self_pin(project_root: &Path) -> Check {
    const NAME: &str = "self-pin (workspace member versions)";

    let cargo_toml = project_root.join("Cargo.toml");
    let contents = match std::fs::read_to_string(&cargo_toml) {
        Ok(contents) => contents,
        Err(err) => {
            return Check {
                name: NAME.into(),
                status: "warn".into(),
                version: Some(format!("could not read Cargo.toml: {err}")),
                install_hint: None,
            };
        }
    };

    let (workspace_version, pins) = version::read_workspace_self_pins(&contents);
    let Some(workspace_version) = workspace_version else {
        return Check {
            name: NAME.into(),
            status: "warn".into(),
            version: Some("not a workspace Cargo.toml (no [workspace.package] version)".into()),
            install_hint: None,
        };
    };

    let drifted: Vec<String> = pins
        .iter()
        .filter(|pin| pin.version != workspace_version)
        .map(|pin| format!("{} pinned {} != {workspace_version}", pin.name, pin.version))
        .collect();

    if drifted.is_empty() {
        Check {
            name: NAME.into(),
            status: "ok".into(),
            version: Some(format!(
                "{} member pin(s) match {workspace_version}",
                pins.len()
            )),
            install_hint: None,
        }
    } else {
        Check {
            name: NAME.into(),
            status: "fail".into(),
            version: Some(drifted.join("; ")),
            install_hint: Some(format!(
                "every [workspace.dependencies] self-pin must equal [workspace.package] \
                 version = \"{workspace_version}\" — VersionBump should have rewritten this; \
                 see 20a/DEN-49"
            )),
        }
    }
}

/// Divergence check: whether `origin/main` is an ancestor of `HEAD` — i.e.
/// whether `scripts/sync-main-to-develop.sh` would be a no-op — read
/// against ALREADY-FETCHED local refs, issuing NO `git fetch` (review:
/// Codex HIGH — a "read-only" preflight must not depend on the network).
fn check_divergence(project_root: &Path) -> Check {
    const NAME: &str = "develop/main divergence (origin/main ancestor)";
    match devflow_core::git::origin_main_ancestor_status(project_root) {
        devflow_core::git::AncestorStatus::Ancestor => Check {
            name: NAME.into(),
            status: "ok".into(),
            version: Some("origin/main is an ancestor of HEAD — sync would be a no-op".into()),
            install_hint: None,
        },
        devflow_core::git::AncestorStatus::Diverged => Check {
            name: NAME.into(),
            status: "fail".into(),
            version: Some("origin/main is NOT an ancestor of HEAD — develop has diverged".into()),
            install_hint: Some(
                "run scripts/sync-main-to-develop.sh before cutting the next release PR".into(),
            ),
        },
        devflow_core::git::AncestorStatus::RefAbsent => Check {
            name: NAME.into(),
            status: "warn".into(),
            version: Some("origin/main not fetched — cannot determine divergence".into()),
            install_hint: Some("run `git fetch` first, then re-run this check".into()),
        },
    }
}

/// Publish-order check: crates.io requires `devflow-core` to be live before
/// `devflow` (path-dependency `--dry-run`/verify resolves against the
/// *published* registry version, not local source). Sourced from the
/// workspace's own members/dependency graph, never a hardcoded prose
/// string.
fn check_publish_order(project_root: &Path) -> Check {
    const NAME: &str = "crates.io publish order";
    let order = devflow_core::git::publish_order(project_root);
    if order.is_empty() {
        return Check {
            name: NAME.into(),
            status: "warn".into(),
            version: Some("could not determine workspace publish order".into()),
            install_hint: None,
        };
    }
    Check {
        name: NAME.into(),
        status: "ok".into(),
        version: Some(format!("publish in order: {}", order.join(" -> "))),
        install_hint: None,
    }
}

/// Tag-signing viability check (20d, Pattern 4): `gpg.format`-aware,
/// fail-soft, and reports only boolean viability + an optional PUBLIC key
/// fingerprint — never private key material or a full filesystem path
/// (T-20-04, ASVS V6 / WR-02).
fn check_signing(project_root: &Path) -> Check {
    const NAME: &str = "tag-signing viability";
    match devflow_core::git::check_signing_viability(project_root) {
        devflow_core::git::SigningViability::Viable { fingerprint } => Check {
            name: NAME.into(),
            status: "ok".into(),
            version: Some(match fingerprint {
                Some(fp) => format!("signing viable ({fp})"),
                None => "signing viable".into(),
            }),
            install_hint: None,
        },
        devflow_core::git::SigningViability::NotViable { reason } => Check {
            name: NAME.into(),
            status: "fail".into(),
            version: Some(reason),
            install_hint: Some("resolve before attempting the signed release tag".into()),
        },
        devflow_core::git::SigningViability::Unknown { reason } => Check {
            name: NAME.into(),
            status: "warn".into(),
            version: Some(reason),
            install_hint: None,
        },
    }
}

// ---------------------------------------------------------------------------
// doctor reconciliation (18a)
// ---------------------------------------------------------------------------

/// Severity of a reconciliation finding, matching the existing `Check.status`
/// convention (lowercase strings) so both `doctor` renderers stay consistent.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum Severity {
    Ok,
    Warn,
    Problem,
}

impl Severity {
    pub(crate) fn label(self) -> &'static str {
        match self {
            Severity::Ok => "ok",
            Severity::Warn => "warn",
            Severity::Problem => "problem",
        }
    }
}

/// The read-only facts `doctor` gathers for one active phase before
/// reconciling them. Collected by `collect_phase_facts` (all I/O); consumed
/// with zero I/O by `reconcile_phase`.
pub(crate) struct PhaseFacts {
    pub(crate) phase: u32,
    pub(crate) stage: Stage,
    pub(crate) gate_pending: bool,
    pub(crate) agent_pid: Option<u32>,
    pub(crate) agent_alive: bool,
    /// The monitor pid recorded in `State.monitor_pid` (18b). `None` means
    /// no monitor has been spawned for this state yet, or the state was
    /// written by a binary predating the field — never treated as a problem.
    pub(crate) monitor_pid: Option<u32>,
    pub(crate) monitor_alive: bool,
    /// The most recent event's `event` field value, for display context.
    pub(crate) last_event: Option<String>,
    /// The `stage` field of the most recent `stage_launched` event; `None`
    /// when the last event recorded for this phase is not a launch.
    pub(crate) last_launched_stage: Option<Stage>,
    pub(crate) open_gate_stages: Vec<Stage>,
    pub(crate) feature_branch_exists: bool,
    /// Whether this phase was intentionally halted by `devflow start --until
    /// <stage>` (20c) — `State.stopped`. A stopped phase's dead agent
    /// pid/stale monitor pid are expected, not a crash; both
    /// `check_dead_agent` and `check_dead_monitor` must recognize this
    /// marker instead of reporting a `Problem` (review: Codex HIGH — the
    /// doctor gap is bigger than `check_dead_agent` alone).
    pub(crate) stopped: bool,
}

/// One diagnostic finding for a phase, with a copy-pasteable repair command
/// when one exists. Never carries a filesystem path or username (T-18-01) —
/// only phase numbers, stage names, and pids identify the disagreement.
pub(crate) struct PhaseFinding {
    pub(crate) phase: u32,
    pub(crate) severity: Severity,
    pub(crate) detail: String,
    pub(crate) repair: Option<String>,
}

/// `gate_pending` is set but no gate file is open for this phase — the gate
/// answer path is stuck. `doctor` only reports this; it never repairs it
/// (T-18-02).
fn check_gate_pending_without_gate(facts: &PhaseFacts) -> Option<PhaseFinding> {
    if !facts.gate_pending || !facts.open_gate_stages.is_empty() {
        return None;
    }
    Some(PhaseFinding {
        phase: facts.phase,
        severity: Severity::Problem,
        detail: format!(
            "phase {}: gate_pending is true at stage {} but no gate file is open",
            facts.phase, facts.stage
        ),
        repair: Some(format!("devflow resume --phase {}", facts.phase)),
    })
}

/// An open gate file exists but `gate_pending` is false — an unanswered
/// operator question that `status`/`doctor` isn't surfacing as pending.
fn check_orphan_gate(facts: &PhaseFacts) -> Option<PhaseFinding> {
    if facts.gate_pending || facts.open_gate_stages.is_empty() {
        return None;
    }
    let gate_stage = facts.open_gate_stages[0];
    Some(PhaseFinding {
        phase: facts.phase,
        severity: Severity::Problem,
        detail: format!(
            "phase {}: gate open for stage {} but state.gate_pending is false",
            facts.phase, gate_stage
        ),
        repair: Some(format!(
            "devflow gate approve {} --stage {}",
            facts.phase, gate_stage
        )),
    })
}

/// The recorded agent pid is not alive while the phase sits at an
/// agent-driven stage — the "who watches the watcher" class of silent death
/// CONTEXT.md cites (two incidents, ~4h lost, found only via `ps`).
fn check_dead_agent(facts: &PhaseFacts) -> Option<PhaseFinding> {
    let pid = facts.agent_pid?;
    if facts.stopped || facts.agent_alive || !facts.stage.is_agent_stage() {
        return None;
    }
    Some(PhaseFinding {
        phase: facts.phase,
        severity: Severity::Problem,
        detail: format!(
            "phase {}: agent pid {pid} recorded but not running at stage {}",
            facts.phase, facts.stage
        ),
        repair: Some(format!("devflow resume --phase {}", facts.phase)),
    })
}

/// The recorded monitor pid is dead — nothing will call `devflow advance`
/// for this phase, whether or not the agent is also dead (an agent that
/// outlived its monitor is orphaned too, since nothing will advance it when
/// it exits either). Reuses `liveness` rather than re-deriving the matrix,
/// so the two copies can never drift (18b, T-18-11's `Unknown` guard applies
/// here transitively — an unrecorded monitor is silently `Unknown`, never a
/// finding).
fn check_dead_monitor(facts: &PhaseFacts) -> Option<PhaseFinding> {
    if facts.stopped
        || liveness(facts.monitor_pid, facts.monitor_alive, facts.agent_alive) != Liveness::Stuck
    {
        return None;
    }
    let pid = facts.monitor_pid?;
    Some(PhaseFinding {
        phase: facts.phase,
        severity: Severity::Problem,
        detail: format!(
            "phase {}: monitor pid {pid} recorded but not running at stage {}",
            facts.phase, facts.stage
        ),
        repair: Some(format!("devflow resume --phase {}", facts.phase)),
    })
}

/// The last `stage_launched` event named a different stage than
/// `state.stage`. A `Warn`, not a `Problem` — a healthy pipeline legitimately
/// has one stage in flight between the launch event and the next
/// transition; exact equality is agreement, never an off-by-one mismatch.
fn check_stage_event_drift(facts: &PhaseFacts) -> Option<PhaseFinding> {
    let launched = facts.last_launched_stage?;
    if launched == facts.stage {
        return None;
    }
    Some(PhaseFinding {
        phase: facts.phase,
        severity: Severity::Warn,
        detail: format!(
            "phase {}: last stage_launched event named {launched} but state.stage is {}",
            facts.phase, facts.stage
        ),
        repair: None,
    })
}

/// The phase's feature branch does not exist even though its stage is past
/// `Define`. A `Warn` — a not-yet-pushed or manually deleted branch is
/// recoverable without state surgery.
fn check_missing_branch(facts: &PhaseFacts) -> Option<PhaseFinding> {
    if facts.feature_branch_exists || facts.stage == Stage::Define {
        return None;
    }
    Some(PhaseFinding {
        phase: facts.phase,
        severity: Severity::Warn,
        detail: format!(
            "phase {}: feature/phase-{:02} does not exist but stage is {}",
            facts.phase, facts.phase, facts.stage
        ),
        repair: None,
    })
}

/// Pure reconciliation core: diffs `state.stage` against the latest event,
/// live agent pid, open gates, and branch existence, evaluating checks in a
/// fixed order so the returned findings never depend on how `facts` was
/// assembled (ordering edge). Takes no path, performs no I/O, and mutates
/// nothing (T-18-02) — directly unit-testable without a repository.
fn reconcile_phase(facts: &PhaseFacts) -> Vec<PhaseFinding> {
    [
        check_gate_pending_without_gate(facts),
        check_orphan_gate(facts),
        check_dead_agent(facts),
        check_dead_monitor(facts),
        check_stage_event_drift(facts),
        check_missing_branch(facts),
    ]
    .into_iter()
    .flatten()
    .collect()
}

/// Gather the read-only facts `reconcile_phase` needs for every active
/// phase, sorted by phase ascending so output ordering never depends on
/// directory-read order (ordering edge). Every call here is a read-only
/// primitive already used elsewhere (`status`, `recover::inspect_all`) —
/// none of it is reimplemented.
fn collect_phase_facts(project_root: &Path) -> Vec<PhaseFacts> {
    let states = workflow::list_states(project_root);
    // 14-CR-10: one pass over events.jsonl for every phase's last event,
    // matching status()'s optimization, not a per-phase rescan.
    let mut last_events = events::last_events_by_phase(project_root);
    let open_gates = Gates::list_open(project_root);

    let mut facts: Vec<PhaseFacts> = states
        .into_iter()
        .map(|state| build_phase_facts(project_root, state, &mut last_events, &open_gates))
        .collect();

    facts.sort_by_key(|f| f.phase);
    facts
}

/// Build one phase's [`PhaseFacts`] from already-fetched state, events, and
/// gates — the per-phase half of `collect_phase_facts`, split out to keep
/// that function short.
fn build_phase_facts(
    project_root: &Path,
    state: State,
    last_events: &mut std::collections::HashMap<u32, serde_json::Value>,
    open_gates: &[OpenGate],
) -> PhaseFacts {
    let phase = state.phase;
    let stopped = state.stopped;
    let agent_pid = agent_pid_from_file(project_root, phase);
    let agent_alive = agent_pid.is_some_and(agent::agent_running);
    let monitor_pid = state.monitor_pid;
    let monitor_alive = monitor_pid.is_some_and(agent::agent_running);
    let last_event = last_events.remove(&phase);
    let last_launched_stage = last_event.as_ref().and_then(last_launched_stage_from_event);
    let last_event_name = last_event
        .as_ref()
        .and_then(|e| e.get("event"))
        .and_then(|e| e.as_str())
        .map(str::to_string);
    let open_gate_stages = open_gates
        .iter()
        .filter(|g| g.phase == phase)
        .map(|g| g.stage)
        .collect();
    let branch_ref = format!("refs/heads/feature/phase-{phase:02}");
    let feature_branch_exists =
        run_git_stdout(project_root, &["rev-parse", "--verify", &branch_ref]).is_some();

    PhaseFacts {
        phase,
        stage: state.stage,
        gate_pending: state.gate_pending,
        agent_pid,
        agent_alive,
        monitor_pid,
        monitor_alive,
        last_event: last_event_name,
        last_launched_stage,
        open_gate_stages,
        feature_branch_exists,
        stopped,
    }
}

/// Derive the stage named by an event's `stage` field, but only when the
/// event's `event` field is `"stage_launched"` — any other event kind (or
/// an unparsable stage name) yields `None`, never a panic.
fn last_launched_stage_from_event(event: &serde_json::Value) -> Option<Stage> {
    if event.get("event").and_then(|e| e.as_str()) != Some("stage_launched") {
        return None;
    }
    event
        .get("stage")
        .and_then(|s| s.as_str())
        .and_then(|s| s.parse::<Stage>().ok())
}

/// The findings to display for one phase: real findings when any exist,
/// otherwise a single synthetic `ok` finding — the display-only counterpart
/// to `reconcile_phase`'s "zero findings" agreement case, shared by both
/// the text and `--json` renderers.
fn findings_for_display(facts: &PhaseFacts) -> Vec<PhaseFinding> {
    let findings = reconcile_phase(facts);
    if !findings.is_empty() {
        return findings;
    }
    vec![PhaseFinding {
        phase: facts.phase,
        severity: Severity::Ok,
        detail: format!("phase {}: ok", facts.phase),
        repair: None,
    }]
}

/// Build `doctor`'s per-phase reconciliation section (after the existing
/// tool/env checks), read-only: it never calls `workflow::save_state`,
/// `events::emit`, `Gates::cleanup`/`Gates::write`, or any `recover::clean*`
/// function (T-18-02). A pure string builder (not a direct `println!`) so
/// it's directly assertable in tests without capturing process stdout.
fn render_reconciliation_text(facts: &[PhaseFacts]) -> String {
    let mut out = String::from("\nreconciliation:\n");
    if facts.is_empty() {
        out.push_str("  no active phases — nothing to reconcile\n");
        return out;
    }
    for phase_facts in facts {
        for finding in findings_for_display(phase_facts) {
            out.push_str(&format!("  {}\n", finding.detail));
            if let Some(repair) = &finding.repair {
                out.push_str(&format!("    repair: {repair}\n"));
            }
        }
    }
    out
}

/// Build the `--json` reconciliation array as a `serde_json::Value` (WR-01,
/// 18-fix). No longer prints its own top-level `[...]` document — `doctor()`
/// nests this under `"reconciliation"` in the single object
/// `doctor_json_body` composes alongside `checks_json_value`'s
/// `"environment"` array.
fn render_reconciliation_json(facts: &[PhaseFacts]) -> serde_json::Value {
    // Pair each finding with its originating phase's last recorded event, so
    // a `--json` consumer gets that context without re-reading events.jsonl.
    let findings: Vec<(&PhaseFacts, PhaseFinding)> = facts
        .iter()
        .flat_map(|pf| findings_for_display(pf).into_iter().map(move |f| (pf, f)))
        .collect();
    serde_json::Value::Array(
        findings
            .iter()
            .map(|(phase_facts, finding)| {
                serde_json::json!({
                    "phase": finding.phase,
                    "severity": finding.severity.label(),
                    "detail": finding.detail,
                    "repair": finding.repair,
                    "last_event": phase_facts.last_event,
                })
            })
            .collect(),
    )
}

/// Build `doctor --json`'s `"environment"` array from the pre-existing
/// tool/env checks (WR-01, 18-fix). Extracted so it can be composed with
/// `render_reconciliation_json`'s array into ONE JSON document instead of
/// being printed as its own top-level array.
fn checks_json_value(checks: &[Check]) -> serde_json::Value {
    serde_json::Value::Array(
        checks
            .iter()
            .map(|c| {
                serde_json::json!({
                    "name": c.name,
                    "status": c.status,
                    "version": c.version,
                    "install_hint": c.install_hint,
                })
            })
            .collect(),
    )
}

/// Compose `doctor --json`'s single JSON document (WR-01, 18-fix). Pre-fix,
/// `doctor()` printed the tool checks as one top-level `[...]` array and
/// then printed `render_reconciliation_json`'s array as a SECOND,
/// independent top-level array right after it — invalid single-document
/// JSON for any parser that isn't NDJSON-aware (`json.load` raised "Extra
/// data" against a live fixture with one active phase). There is now
/// exactly one top-level value: `{"environment": [...], "reconciliation":
/// [...], "planning_doc_staleness": [...]}` — 21b's addition (D-05) extends
/// this SAME object with a third key rather than forking a second reporter
/// or printing a second top-level array.
fn doctor_json_body(
    checks: &[Check],
    facts: &[PhaseFacts],
    doc_findings: &[PlanningDocFinding],
) -> serde_json::Value {
    serde_json::json!({
        "environment": checks_json_value(checks),
        "reconciliation": render_reconciliation_json(facts),
        "planning_doc_staleness": render_planning_doc_findings_json(doc_findings),
    })
}

// ---------------------------------------------------------------------------
// doctor planning-doc staleness reconciliation (21b, D-04/D-05)
// ---------------------------------------------------------------------------

/// The `v1.5.0` numeric-tuple cutoff (RESEARCH Pitfall #2): the first phase
/// whose version claim was consistently tagged. A claimed version at or
/// after this cutoff with no matching/reachable git tag is a real
/// `Severity::Problem`; a claim before it is legacy history and downgrades
/// to `Severity::Warn` — otherwise a naive per-row check floods `doctor`
/// with pre-Phase-18 noise, the exact alert-fatigue class 999.14 exists to
/// prevent. Compared as a NUMERIC `(major, minor, patch)` tuple, never a
/// string, so a real future `v1.10.0` is correctly treated as post-cutoff
/// (a lexicographic `"1.10.0" < "1.5.0"` would wrongly sort it as legacy).
const PLANNING_DOC_STALENESS_CUTOFF: (u32, u32, u32) = (1, 5, 0);

/// One detection-only finding produced by reconciling a `ROADMAP.md`/
/// `STATE.md` version claim against the repo's git tags. A sibling of
/// `PhaseFinding`, not a reuse of it: most claims here are about
/// already-shipped phases with no active `state-NN.json`/`PhaseFacts`
/// (RESEARCH Open Q1). `repair` is always `None` — D-04 forbids any
/// auto-correction of planning-doc prose; nothing in this module has a
/// write path to either file. Never carries a filesystem path or username
/// (T-18-01 discipline) — only the source label, the claimed version, and
/// a git tag name.
pub(crate) struct PlanningDocFinding {
    pub(crate) source: String,
    pub(crate) claim: String,
    pub(crate) severity: Severity,
    pub(crate) detail: String,
    pub(crate) repair: Option<String>,
}

/// Parse a table cell as a bare `(major, minor, patch)` semver tuple,
/// stripping an optional leading `v`. Returns `None` for anything that
/// isn't EXACTLY three dot-separated numeric components — this is what
/// keeps version ranges (`0.1.0–0.6.0`), em-dash placeholders (`—`), and
/// any other non-semver cell out of every downstream finding (RESEARCH
/// Pitfall #2), without needing a regex crate: the range's `–` makes its
/// middle component fail `str::parse::<u32>`, and the em-dash fails
/// outright.
pub(crate) fn parse_semver(cell: &str) -> Option<(u32, u32, u32)> {
    let cell = cell.strip_prefix('v').unwrap_or(cell);
    let mut parts = cell.split('.');
    let major = parts.next()?.trim().parse().ok()?;
    let minor = parts.next()?.trim().parse().ok()?;
    let patch = parts.next()?.trim().parse().ok()?;
    if parts.next().is_some() {
        return None; // more than three `.`-separated components
    }
    Some((major, minor, patch))
}

/// Scan a `## Shipped`/`## Completed`-shaped markdown table for `(label,
/// version)` rows whose version cell is a bare or `v`-prefixed single
/// semver. Hand-scans lines split on `|` rather than pulling in a
/// markdown-table parser crate — the `is_self_dogfood_workspace` (D-17)
/// convention this codebase already follows for small, fixed-shape
/// structured text. Skips the header row, the `|---|---|` separator row,
/// and any cell that isn't a bare semver (ranges, em-dashes, anything
/// else) outright; never panics on a malformed row (T-21b-03 — parse
/// defensively, degrade rather than die). `source` (e.g. `"ROADMAP.md"`)
/// is folded into the returned label so a caller that concatenates rows
/// from multiple documents can still tell them apart downstream.
pub(crate) fn parse_planning_doc_versions(text: &str, source: &str) -> Vec<(String, String)> {
    let mut rows = Vec::new();
    for line in text.lines() {
        let trimmed = line.trim();
        if !trimmed.starts_with('|') {
            continue;
        }
        let cells: Vec<&str> = trimmed
            .trim_matches('|')
            .split('|')
            .map(str::trim)
            .collect();
        if cells.len() < 2 {
            continue;
        }
        let label = cells[0];
        // Header row (`Phase | Name | Version`) and the `|---|---|---|`
        // separator row both have a first cell that is never a real phase
        // label — skip both rather than trying to special-case each shape.
        if label.is_empty()
            || label.eq_ignore_ascii_case("phase")
            || label.chars().all(|c| c == '-')
        {
            continue;
        }
        // The version column's position differs between ROADMAP.md's
        // `## Shipped` table (Phase | Name | Version) and STATE.md's
        // `## Completed` table (Phase | Description | Version | Date) —
        // scan every non-label cell and keep whichever ones parse as a
        // bare semver, rather than hardcoding a column index.
        for cell in &cells[1..] {
            if parse_semver(cell).is_some() {
                rows.push((format!("{source} phase {label}"), (*cell).to_string()));
            }
        }
    }
    rows
}

/// Whether `tag` exists in `project_root` AND is reachable from
/// `base_branch` — argv-array `git` shelling only (T-21b-02: tag strings
/// passed in here are already validated `^v?\d+\.\d+\.\d+$` cells, never
/// free-form; no `sh -c`). Two separate invocations, mirroring
/// `staleness::run_git_stdout`'s idiom: existence first, so a missing tag
/// short-circuits before the (more expensive) ancestry check.
pub(crate) fn tag_exists_and_reachable(project_root: &Path, tag: &str, base_branch: &str) -> bool {
    let exists = std::process::Command::new("git")
        .args(["rev-parse", "--verify", &format!("refs/tags/{tag}")])
        .current_dir(project_root)
        .output()
        .is_ok_and(|o| o.status.success());
    exists
        && std::process::Command::new("git")
            .args(["merge-base", "--is-ancestor", tag, base_branch])
            .current_dir(project_root)
            .output()
            .is_ok_and(|o| o.status.success())
}

/// Pure reconciliation core: for each `(label, version_cell)` row, ask the
/// caller-supplied `tag_lookup` closure whether a `v`-normalized tag exists
/// and is reachable — kept injectable (rather than calling
/// `tag_exists_and_reachable` directly) so this is unit-testable without a
/// real repository, mirroring `reconcile_phase`'s zero-I/O discipline. A
/// miss becomes a `PlanningDocFinding` at `Severity::Problem` when the
/// claimed version is at or after the `v1.5.0` cutoff — compared as a
/// NUMERIC tuple via `parse_semver`, never lexicographically — else
/// `Severity::Warn` (RESEARCH Pitfall #2). `repair` is always `None` (D-04:
/// detection-only). Skips any row whose cell doesn't parse as a semver,
/// defensively — `parse_planning_doc_versions` already filters these out,
/// but this must never panic even if called with a stray malformed row.
pub(crate) fn reconcile_planning_docs(
    rows: &[(String, String)],
    tag_lookup: &mut impl FnMut(&str) -> bool,
) -> Vec<PlanningDocFinding> {
    let mut findings = Vec::new();
    for (label, version_cell) in rows {
        let Some(parsed) = parse_semver(version_cell) else {
            continue;
        };
        let tag = if version_cell.starts_with('v') {
            version_cell.clone()
        } else {
            format!("v{version_cell}")
        };
        if tag_lookup(&tag) {
            continue;
        }
        let severity = if parsed >= PLANNING_DOC_STALENESS_CUTOFF {
            Severity::Problem
        } else {
            Severity::Warn
        };
        findings.push(PlanningDocFinding {
            source: label.clone(),
            claim: format!("{label} claims {tag}"),
            severity,
            detail: format!(
                "{label} claims {tag}, but no git tag `{tag}` exists (or it isn't reachable from the base branch)"
            ),
            repair: None,
        });
    }
    findings
}

/// Read `.planning/ROADMAP.md`'s `## Shipped` table and `.planning/STATE.md`'s
/// `## Completed` table (best-effort — a MISSING file yields no rows for
/// that document, never an error; `doctor` must not fabricate a `Problem`
/// from an absent doc), parse both, and reconcile every row against the
/// repo's git tags via `tag_exists_and_reachable(project_root, tag,
/// "main")`. `"main"` is a LOCAL branch in this repo (verified: `git
/// branch --list main`; `git merge-base --is-ancestor v1.7.0 main`
/// succeeds offline) — deliberately not `origin/main` (no network
/// dependency in `doctor`'s read-only contract) and not `develop` (wrong
/// base). The only I/O here is two `std::fs::read_to_string` calls plus
/// `tag_exists_and_reachable`'s `git` subprocesses — `doctor` stays
/// read-only (no write path to either file).
fn collect_planning_doc_findings(project_root: &Path) -> Vec<PlanningDocFinding> {
    let roadmap =
        std::fs::read_to_string(project_root.join(".planning/ROADMAP.md")).unwrap_or_default();
    let state =
        std::fs::read_to_string(project_root.join(".planning/STATE.md")).unwrap_or_default();

    let mut rows = parse_planning_doc_versions(&roadmap, "ROADMAP.md");
    rows.extend(parse_planning_doc_versions(&state, "STATE.md"));

    let mut lookup = |tag: &str| tag_exists_and_reachable(project_root, tag, "main");
    reconcile_planning_docs(&rows, &mut lookup)
}

/// Build `doctor --json`'s `"planning_doc_staleness"` array (D-05, Pattern
/// 2), mirroring `render_reconciliation_json`'s array-building idiom.
fn render_planning_doc_findings_json(findings: &[PlanningDocFinding]) -> serde_json::Value {
    serde_json::Value::Array(
        findings
            .iter()
            .map(|f| {
                serde_json::json!({
                    "source": f.source,
                    "claim": f.claim,
                    "severity": f.severity.label(),
                    "detail": f.detail,
                    "repair": f.repair,
                })
            })
            .collect(),
    )
}

/// Build `doctor`'s text planning-docs section, printed after the
/// reconciliation section. A pure string builder (not a direct
/// `println!`), mirroring `render_reconciliation_text`'s shape so it's
/// directly assertable in tests without capturing process stdout. No
/// findings prints a single `"planning docs: consistent with git tags"`
/// line, matching the action spec.
fn render_planning_doc_text(findings: &[PlanningDocFinding]) -> String {
    if findings.is_empty() {
        return "\nplanning docs: consistent with git tags\n".to_string();
    }
    let mut out = String::from("\nplanning docs:\n");
    for finding in findings {
        out.push_str(&format!(
            "  [{}] {}\n",
            finding.severity.label(),
            finding.detail
        ));
    }
    out
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::{Cli, Command, GateCmd};
    use clap::Parser;

    #[test]
    fn gate_approve_arg_parsing_accepts_positional_stage() {
        let cli = Cli::try_parse_from(["devflow", "gate", "approve", "15", "ship"]).unwrap();
        let Command::Gate {
            action: GateCmd::Approve { stage, project, .. },
        } = cli.command
        else {
            panic!("expected gate approve command");
        };

        assert_eq!(stage.as_deref(), Some("ship"));
        assert_eq!(project, PathBuf::from("."));

        let flagged =
            Cli::try_parse_from(["devflow", "gate", "approve", "15", "--stage", "ship"]).unwrap();
        let Command::Gate {
            action:
                GateCmd::Approve {
                    stage,
                    stage_option,
                    ..
                },
        } = flagged.command
        else {
            panic!("expected flagged gate approve command");
        };
        assert_eq!(stage, None);
        assert_eq!(stage_option, Some(Stage::Ship));

        let bare = Cli::try_parse_from(["devflow", "gate", "approve", "15"]).unwrap();
        let Command::Gate {
            action:
                GateCmd::Approve {
                    stage,
                    stage_option,
                    ..
                },
        } = bare.command
        else {
            panic!("expected bare gate approve command");
        };
        assert_eq!(stage, None);
        assert_eq!(stage_option, None);

        let legacy =
            Cli::try_parse_from(["devflow", "gate", "approve", "15", "/tmp/example-project"])
                .unwrap();
        let Command::Gate {
            action:
                GateCmd::Approve {
                    stage,
                    legacy_project,
                    stage_option,
                    project,
                    ..
                },
        } = legacy.command
        else {
            panic!("expected legacy gate approve command");
        };
        let (stage, project) =
            resolve_gate_target(stage, legacy_project, stage_option, project).unwrap();
        assert_eq!(stage, None);
        assert_eq!(project, PathBuf::from("/tmp/example-project"));
    }

    #[test]
    fn gate_show_arg_parsing_accepts_phase_and_optional_stage() {
        let bare = Cli::try_parse_from(["devflow", "gate", "show", "15"]).unwrap();
        let Command::Gate {
            action: GateCmd::Show { phase, stage, .. },
        } = bare.command
        else {
            panic!("expected gate show command");
        };
        assert_eq!(phase, 15);
        assert_eq!(stage, None);

        let flagged =
            Cli::try_parse_from(["devflow", "gate", "show", "15", "--stage", "ship"]).unwrap();
        let Command::Gate {
            action: GateCmd::Show { phase, stage, .. },
        } = flagged.command
        else {
            panic!("expected gate show command with stage");
        };
        assert_eq!(phase, 15);
        assert_eq!(stage, Some(Stage::Ship));
    }

    #[test]
    fn gate_show_renders_full_untruncated_sanitized_context() {
        let dir = tempfile::tempdir().unwrap();
        let context = format!("first line\n\u{1b}[2J{}", "x".repeat(150));
        Gates::write_gate(dir.path(), 15, Stage::Ship, &context).unwrap();
        let gate = Gates::list_open(dir.path())
            .into_iter()
            .find(|g| g.phase == 15)
            .unwrap();

        let rendered = render_gate_show(&gate);

        assert!(rendered.contains(&"x".repeat(150)));
        assert!(!rendered.contains("[truncated"));
        assert!(!rendered.contains('\u{1b}'));
    }

    #[test]
    fn gate_show_errors_naming_gate_list_when_no_open_gate() {
        let dir = tempfile::tempdir().unwrap();
        let err = gate_show(dir.path(), 15, None).unwrap_err();
        assert!(err.to_string().contains("devflow gate list"));
    }

    #[test]
    fn gate_show_errors_asking_for_stage_with_several_open_gates() {
        let dir = tempfile::tempdir().unwrap();
        Gates::write_gate(dir.path(), 15, Stage::Ship, "ctx1").unwrap();
        Gates::write_gate(dir.path(), 15, Stage::Validate, "ctx2").unwrap();

        let err = gate_show(dir.path(), 15, None).unwrap_err();

        assert!(err.to_string().contains("--stage"));
    }

    #[test]
    fn gate_show_auto_resolves_single_open_gate() {
        let dir = tempfile::tempdir().unwrap();
        Gates::write_gate(dir.path(), 15, Stage::Ship, "the only open gate").unwrap();

        assert!(gate_show(dir.path(), 15, None).is_ok());
    }

    #[test]
    fn describe_worktree_dir_infers_phase_and_agent() {
        assert_eq!(
            describe_worktree_dir("phase-07-claude"),
            " — phase 7, agent claude"
        );
        assert_eq!(describe_worktree_dir("phase-08"), " — phase 8");
        assert_eq!(describe_worktree_dir("reference"), "");
    }

    #[test]
    fn cron_instruction_hints_include_hermes_command_per_phase() {
        let dir = tempfile::tempdir().unwrap();
        // Empty retry_after here so the exact-match assertion below isolates
        // the base hermes-command hint from 21a's reset-time fragment
        // (covered separately by cron_hint_line_* below).
        for phase in [7, 9] {
            let instructions =
                devflow_core::ship::build_cron_instructions(dir.path(), phase, "", "claude,codex");
            devflow_core::ship::write_cron_instructions(dir.path(), &instructions).unwrap();
        }

        let hints = cron_instruction_hints(dir.path());

        assert_eq!(hints.len(), 2);
        assert_eq!(
            hints[0],
            format!(
                "Cron instruction pending (phase 7): hermes cron create --from-devflow {}",
                dir.path().display()
            )
        );
        assert!(hints[1].contains("(phase 9)"));
    }

    #[test]
    fn cron_hint_line_appends_sanitized_reset_when_retry_after_present() {
        let dir = tempfile::tempdir().unwrap();
        let instructions = devflow_core::ship::build_cron_instructions(
            dir.path(),
            7,
            "2026-06-18T15:45:30Z",
            "claude,codex",
        );

        let hint = cron_hint_line(&instructions, dir.path());

        assert!(hint.starts_with(&format!(
            "Cron instruction pending (phase 7): hermes cron create --from-devflow {}",
            dir.path().display()
        )));
        assert!(hint.contains("(rate-limit resets: 2026-06-18T15:45:30Z)"));
    }

    #[test]
    fn cron_hint_line_omits_reset_fragment_when_retry_after_empty() {
        let dir = tempfile::tempdir().unwrap();
        let instructions = devflow_core::ship::build_cron_instructions(dir.path(), 7, "", "claude");

        let hint = cron_hint_line(&instructions, dir.path());

        assert_eq!(
            hint,
            format!(
                "Cron instruction pending (phase 7): hermes cron create --from-devflow {}",
                dir.path().display()
            )
        );
        assert!(!hint.contains("resets"));
    }

    #[test]
    fn default_logs_phase_prefers_single_active_state() {
        let dir = tempfile::tempdir().unwrap();
        let state = State::new(6, AgentKind::Claude, Mode::Auto, dir.path().to_path_buf());
        workflow::save_state(&state).unwrap();

        assert_eq!(default_logs_phase(dir.path()).unwrap(), 6);
    }

    #[test]
    fn default_logs_phase_is_ambiguous_with_two_active_states() {
        let dir = tempfile::tempdir().unwrap();
        for phase in [6, 7] {
            let state = State::new(
                phase,
                AgentKind::Claude,
                Mode::Auto,
                dir.path().to_path_buf(),
            );
            workflow::save_state(&state).unwrap();
        }

        let err = default_logs_phase(dir.path()).unwrap_err();
        assert!(err.to_string().contains("--phase"));
    }

    #[test]
    fn default_logs_phase_falls_back_to_newest_capture_file() {
        let dir = tempfile::tempdir().unwrap();
        std::fs::create_dir_all(dir.path().join(".devflow")).unwrap();
        std::fs::write(agent_result::stdout_path(dir.path(), 3), "old").unwrap();
        // Ensure a strictly newer mtime on the second capture.
        std::thread::sleep(std::time::Duration::from_millis(20));
        std::fs::write(agent_result::stdout_path(dir.path(), 5), "new").unwrap();

        assert_eq!(default_logs_phase(dir.path()).unwrap(), 5);
    }

    #[test]
    fn default_logs_phase_errors_with_nothing_to_show() {
        let dir = tempfile::tempdir().unwrap();
        assert!(default_logs_phase(dir.path()).is_err());
    }

    /// 18b: a state with no recorded monitor is never reported as stuck,
    /// regardless of the (unreliable, since no monitor was ever recorded)
    /// liveness bits passed alongside it.
    #[test]
    fn liveness_unknown_when_no_monitor_recorded() {
        assert_eq!(liveness(None, false, false), Liveness::Unknown);
        assert_eq!(liveness(None, false, true), Liveness::Unknown);
        assert_eq!(liveness(None, true, false), Liveness::Unknown);
        assert_eq!(liveness(None, true, true), Liveness::Unknown);
    }

    /// 18b: the full four-row matrix for a recorded monitor pid. A dead
    /// agent with a dead monitor OR a live monitor with a dead agent are
    /// different states — only the former is `Stuck` (nothing will call
    /// `devflow advance`); the latter is a normal between-stages moment. An
    /// agent that outlived its monitor is also `Stuck` — orphaned, since
    /// nothing will advance it when it exits either.
    #[test]
    fn liveness_matrix_covers_all_four_rows() {
        let pid = Some(4242);
        assert_eq!(liveness(pid, true, true), Liveness::Healthy);
        assert_eq!(liveness(pid, true, false), Liveness::BetweenStages);
        assert_eq!(liveness(pid, false, false), Liveness::Stuck);
        assert_eq!(liveness(pid, false, true), Liveness::Stuck);
    }

    /// 18b: a corrupt pid (0, or above `i32::MAX`) must never read as alive
    /// — `liveness` relies entirely on `agent::agent_running`'s existing
    /// hardening (no second probe is written), so it can only ever produce
    /// `Stuck` or `Unknown` for a corrupt pid, never a false `Healthy`.
    #[test]
    fn liveness_treats_zero_and_overflow_pids_as_dead() {
        assert!(!agent::agent_running(0));
        assert!(!agent::agent_running(u32::MAX));
    }

    /// A live slot renders `agent B (codex) running`, cross-referencing the
    /// agent-pid file the monitor already writes (21c, D-06).
    #[test]
    fn sequentagent_status_renders_running_slot() {
        let dir = tempfile::tempdir().unwrap();
        let root = dir.path();
        agent_result::write_sequentagent_slot(
            root,
            7,
            agent_result::SequentagentSlotKind::B,
            AgentKind::Codex,
        )
        .unwrap();
        std::fs::write(
            agent_result::agent_pid_path(root, 7),
            std::process::id().to_string(),
        )
        .unwrap();

        let rendered = render_sequentagent_status(root).unwrap();

        assert!(rendered.contains("sequentagent"));
        assert!(rendered.contains("agent B"));
        assert!(rendered.contains("codex"));
        assert!(rendered.contains("running"));
        assert!(!rendered.contains("not running"));
    }

    /// A slot record whose agent-pid file names a dead process renders
    /// "not running" — a stale record never claims a live agent (T-21c-02).
    #[test]
    fn sequentagent_status_renders_dead_pid_as_not_running() {
        let dir = tempfile::tempdir().unwrap();
        let root = dir.path();
        agent_result::write_sequentagent_slot(
            root,
            8,
            agent_result::SequentagentSlotKind::A,
            AgentKind::Claude,
        )
        .unwrap();
        std::fs::write(agent_result::agent_pid_path(root, 8), "0").unwrap();

        let rendered = render_sequentagent_status(root).unwrap();

        assert!(rendered.contains("not running"));
    }

    /// A slot record present but with no agent-pid file yet (the monitor
    /// writes it asynchronously — the launch/pid-write race) renders
    /// "starting", not "not running" — an honest transient state
    /// (cross-AI review LOW pid-race).
    #[test]
    fn sequentagent_status_renders_starting_when_pid_file_missing() {
        let dir = tempfile::tempdir().unwrap();
        let root = dir.path();
        agent_result::write_sequentagent_slot(
            root,
            9,
            agent_result::SequentagentSlotKind::A,
            AgentKind::Claude,
        )
        .unwrap();

        let rendered = render_sequentagent_status(root).unwrap();

        assert!(rendered.contains("starting"));
        assert!(!rendered.contains("not running"));
    }

    /// No slot records at all → `None`, so `status` prints nothing extra for
    /// the common (non-sequentagent) case.
    #[test]
    fn sequentagent_status_none_when_no_records() {
        let dir = tempfile::tempdir().unwrap();
        assert!(render_sequentagent_status(dir.path()).is_none());
    }

    /// 18b: persisting `monitor_pid` for one phase must not disturb a
    /// concurrently-active sibling phase's `monitor_pid` (concurrency edge).
    #[test]
    fn monitor_pid_persisted_for_one_phase_does_not_disturb_a_sibling() {
        let dir = tempfile::tempdir().unwrap();
        let root = dir.path();

        let mut phase7 = State::new(7, AgentKind::Claude, Mode::Auto, root.to_path_buf());
        phase7.monitor_pid = Some(111);
        workflow::save_state(&phase7).unwrap();

        let mut phase8 = State::new(8, AgentKind::Claude, Mode::Auto, root.to_path_buf());
        phase8.monitor_pid = Some(222);
        workflow::save_state(&phase8).unwrap();

        let reloaded7 = workflow::load_state(root, 7).unwrap();
        let reloaded8 = workflow::load_state(root, 8).unwrap();
        assert_eq!(reloaded7.monitor_pid, Some(111));
        assert_eq!(reloaded8.monitor_pid, Some(222));
    }

    /// 18b (idempotency edge): running `devflow status` twice must produce
    /// byte-identical `.devflow/` state — the new monitor liveness probe is
    /// purely a read, same as the existing agent liveness probe it sits
    /// beside. Also exercises the `u32::MAX` boundary pid (precision edge,
    /// via `agent::agent_running`'s existing hardening) so the probe can
    /// only ever report `Stuck`, never a false `Healthy`.
    #[test]
    fn status_reading_monitor_liveness_writes_no_state_and_no_event() {
        let dir = tempfile::tempdir().unwrap();
        let root = dir.path();
        let phase = 66;
        let mut state = State::new(phase, AgentKind::Claude, Mode::Auto, root.to_path_buf());
        state.monitor_pid = Some(u32::MAX);
        workflow::save_state(&state).unwrap();

        let state_path = workflow::state_path(root, phase);
        let before_len = std::fs::metadata(&state_path).unwrap().len();
        let before_modified = std::fs::metadata(&state_path).unwrap().modified().unwrap();
        let events_log = events::events_path(root);
        let before_lines = std::fs::read_to_string(&events_log)
            .unwrap_or_default()
            .lines()
            .count();

        status(root).unwrap();
        status(root).unwrap();

        let after_len = std::fs::metadata(&state_path).unwrap().len();
        let after_modified = std::fs::metadata(&state_path).unwrap().modified().unwrap();
        let after_lines = std::fs::read_to_string(&events_log)
            .unwrap_or_default()
            .lines()
            .count();

        assert_eq!(
            before_len, after_len,
            "status must not rewrite the state file"
        );
        assert_eq!(
            before_modified, after_modified,
            "status must not touch the state file's mtime"
        );
        assert_eq!(
            before_lines, after_lines,
            "status must not append to events.jsonl"
        );
    }

    /// 15a: `devflow gate approve` resolves the stage automatically when a
    /// phase has exactly one open gate and writes a response the workflow's
    /// poller will consume.
    #[test]
    fn gate_respond_auto_resolves_single_open_gate() {
        let dir = tempfile::tempdir().unwrap();
        let root = dir.path();
        Gates::write_gate(root, 15, Stage::Ship, "approve merge?").unwrap();

        gate_respond(root, 15, None, true, Some("lgtm".into())).unwrap();

        let polled = Gates::poll_response(root, 15, Stage::Ship, 1).expect("response readable");
        assert!(polled.approved);
        assert_eq!(polled.note.as_deref(), Some("lgtm"));
        let event = devflow_core::events::last_event_for_phase(root, 15).unwrap();
        assert_eq!(event["event"], "gate_response_written");
        assert_eq!(event["stage"], "ship");
    }

    #[test]
    fn gate_respond_requires_stage_when_ambiguous_and_errors_when_none_open() {
        let dir = tempfile::tempdir().unwrap();
        let root = dir.path();

        let err = gate_respond(root, 15, None, true, None).unwrap_err();
        assert!(err.to_string().contains("no open gate"), "{err}");

        Gates::write_gate(root, 15, Stage::Validate, "a").unwrap();
        Gates::write_gate(root, 15, Stage::Ship, "b").unwrap();
        let err = gate_respond(root, 15, None, false, Some("nope".into())).unwrap_err();
        assert!(err.to_string().contains("--stage"), "{err}");

        // Explicit --stage disambiguates.
        gate_respond(root, 15, Some(Stage::Validate), false, Some("gaps".into())).unwrap();
        assert!(
            Gates::response_path(root, 15, Stage::Validate).exists(),
            "explicit-stage rejection must land"
        );
        assert!(!Gates::response_path(root, 15, Stage::Ship).exists());
    }

    /// 14-CR-03: a capture file SHORTER than the follower's offset means the
    /// next stage's monitor deleted and recreated it — the follower must
    /// restart from 0, not seek past EOF forever.
    #[test]
    fn rollover_offset_resets_on_shrunken_capture() {
        let dir = tempfile::tempdir().unwrap();
        let path = dir.path().join("capture");
        std::fs::write(&path, "abc").unwrap();

        // File (3 bytes) shorter than offset 10 → rollover → 0.
        assert_eq!(rollover_offset(&path, 10), 0);
        // File longer than or equal to the offset → keep the offset.
        assert_eq!(rollover_offset(&path, 3), 3);
        assert_eq!(rollover_offset(&path, 2), 2);
        // Missing file (mid-rollover gap) → keep the offset for now.
        assert_eq!(rollover_offset(&dir.path().join("gone"), 7), 7);
    }

    #[test]
    fn print_capture_from_tracks_offsets_across_appends() {
        let dir = tempfile::tempdir().unwrap();
        let path = dir.path().join("capture");
        std::fs::write(&path, "hello ").unwrap();
        let mut output = Vec::new();

        let offset = write_capture_from(&path, 0, &mut output).unwrap();
        assert_eq!(offset, 6);
        assert_eq!(output, b"hello ");

        // Nothing new: offset unchanged.
        output.clear();
        assert_eq!(write_capture_from(&path, offset, &mut output).unwrap(), 6);
        assert!(output.is_empty());

        use std::io::Write as _;
        let mut f = std::fs::OpenOptions::new()
            .append(true)
            .open(&path)
            .unwrap();
        f.write_all(b"world").unwrap();
        drop(f);
        assert_eq!(write_capture_from(&path, offset, &mut output).unwrap(), 11);
        assert_eq!(output, b"world");

        // Missing file is treated as "no new bytes yet".
        output.clear();
        assert_eq!(
            write_capture_from(Path::new("/nonexistent/x"), 4, &mut output).unwrap(),
            4
        );
        assert!(output.is_empty());
    }

    /// 13-06 dogfood regression (Codex leg): a fresh headless Codex run can
    /// never pass Define, so `start --agent codex` pre-flights on the
    /// phase's CONTEXT.md existing on develop.
    #[test]
    fn phase_artifact_on_develop_detects_context_and_fails_open() {
        let dir = tempfile::tempdir().unwrap();
        let root = dir.path();
        let run = |args: &[&str]| {
            let out = std::process::Command::new("git")
                .args(args)
                .current_dir(root)
                .output()
                .expect("spawn git");
            assert!(out.status.success(), "git {args:?} failed");
        };
        run(&["init", "-q", "-b", "main"]);
        run(&["config", "user.email", "t@e.st"]);
        run(&["config", "user.name", "t"]);
        run(&["config", "commit.gpgsign", "false"]);
        run(&["config", "core.hooksPath", "/dev/null"]);
        std::fs::create_dir_all(root.join(".planning/phases/03-widget")).unwrap();
        std::fs::write(root.join(".planning/phases/03-widget/03-CONTEXT.md"), "ctx").unwrap();
        run(&["add", "-A"]);
        run(&["commit", "-q", "-m", "init"]);
        run(&["branch", "develop"]);

        assert!(phase_artifact_on_develop(root, 3, "-CONTEXT.md"));
        assert!(!phase_artifact_on_develop(root, 3, "-PLAN.md"));
        assert!(!phase_artifact_on_develop(root, 4, "-CONTEXT.md"));

        // Fail-open: outside a repo (or with no develop branch) the
        // pre-flight must not block.
        let empty = tempfile::tempdir().unwrap();
        assert!(phase_artifact_on_develop(empty.path(), 3, "-CONTEXT.md"));
    }

    // -----------------------------------------------------------------
    // 17d: build provenance + self-dogfood staleness gate (D-17-D-21, Task 2)
    // -----------------------------------------------------------------

    /// D-21: the `workflow_started` payload carries every provenance field,
    /// tested directly without spawning a real agent. No `build_timestamp`
    /// field any more (CR-02, 17-11) — it was removed from `build.rs`
    /// entirely, not just this payload. Also pins the WR-02 redaction: the
    /// `exe_path` field must never carry a directory component (the
    /// operator's home directory / OS username), since `OPERATIONS.md`
    /// documents `events.jsonl` as a file that's safe to tail and paste.
    #[test]
    fn workflow_started_payload_carries_build_provenance() {
        let state = State::new(66, AgentKind::Claude, Mode::Auto, PathBuf::from("/repo"));
        let payload = workflow_started_payload(&state);
        assert_eq!(payload["agent"], "claude");
        assert_eq!(payload["mode"], "auto");
        assert!(payload["version"].as_str().is_some());
        assert!(payload["commit"].is_string());
        assert!(payload["dirty"].is_string());
        assert!(
            payload.get("build_timestamp").is_none(),
            "build_timestamp was removed (CR-02) and must not reappear"
        );
        assert!(
            payload.get("exe_path").is_some(),
            "WR-02: exe_path key must still exist — a future refactor must not \
             satisfy the redaction assertion by deleting the field"
        );
        assert!(payload["exe_path"].is_string() || payload["exe_path"].is_null());
        if let Some(exe_path) = payload["exe_path"].as_str() {
            assert!(
                !exe_path.contains('/') && !exe_path.contains('\\'),
                "WR-02: exe_path must be a bare filename with no directory \
                 separator — OPERATIONS.md documents events.jsonl as safe to \
                 tail and paste, so a full absolute path here leaks the \
                 operator's home directory and OS username; got {exe_path:?}"
            );
        }
    }

    #[test]
    fn status_shows_pending_gate_prominently() {
        let dir = tempfile::tempdir().unwrap();
        let context = format!("first line\n\u{1b}[2J{}", "sensitive detail ".repeat(80));
        Gates::write_gate(dir.path(), 16, Stage::Ship, &context).unwrap();
        let open = Gates::list_open(dir.path());

        let banner = render_pending_gate_banner(&open, u64::MAX).unwrap();

        assert!(banner.contains("PENDING GATE"));
        assert!(banner.contains("phase 16"));
        assert!(banner.contains("ship"));
        assert!(banner.contains("devflow gate approve 16 --stage ship"));
        assert!(banner.contains("devflow gate reject 16 --stage ship"));
        assert!(banner.contains("[truncated; full output in .devflow/]"));
        assert!(!banner.contains(&context));
        assert!(!banner.contains('\u{1b}'));
        assert!(banner.contains("ESCALATED"));
    }

    /// 21a: `recovery_hints` returns a `resume` hint for a stuck phase,
    /// additionally an `advance` hint when the phase is gate-pending
    /// (answer the gate, then advance), and nothing for a non-stuck phase.
    #[test]
    fn recovery_hints_includes_resume_for_stuck() {
        let dir = tempfile::tempdir().unwrap();
        let state = State::new(7, AgentKind::Claude, Mode::Auto, dir.path().to_path_buf());

        let hints = recovery_hints(&state, Liveness::Stuck);

        assert_eq!(hints, vec!["devflow resume --phase 7".to_string()]);
    }

    #[test]
    fn recovery_hints_includes_advance_when_stuck_and_gate_pending() {
        let dir = tempfile::tempdir().unwrap();
        let mut state = State::new(7, AgentKind::Claude, Mode::Auto, dir.path().to_path_buf());
        state.gate_pending = true;

        let hints = recovery_hints(&state, Liveness::Stuck);

        assert_eq!(
            hints,
            vec![
                "devflow resume --phase 7".to_string(),
                "devflow advance --phase 7".to_string(),
            ]
        );
    }

    #[test]
    fn recovery_hints_empty_for_healthy() {
        let dir = tempfile::tempdir().unwrap();
        let state = State::new(7, AgentKind::Claude, Mode::Auto, dir.path().to_path_buf());

        assert!(recovery_hints(&state, Liveness::Healthy).is_empty());
    }

    /// 21a: `latest_stage_launched_ts` scans the event log for the LAST
    /// `stage_launched` event's `ts` — the real stage-entry time — and is
    /// `None` without one, never falling back to any other field.
    #[test]
    fn latest_stage_launched_ts_none_without_event() {
        let dir = tempfile::tempdir().unwrap();
        assert_eq!(latest_stage_launched_ts(dir.path(), 7), None);
    }

    /// The closing proof for the 3/3 cross-AI review MEDIUM
    /// (21-REVIEWS.md): a phase whose latest `stage_launched` event is ~90s
    /// old but whose phase-level `started_at` is ~30m old must report the
    /// ~90s stage age — `latest_stage_launched_ts` must never be sourced
    /// from `state.started_at`.
    #[test]
    fn latest_stage_launched_ts_reflects_event_age_not_phase_started_at() {
        let dir = tempfile::tempdir().unwrap();
        let root = dir.path();
        let now = SystemTime::now()
            .duration_since(UNIX_EPOCH)
            .unwrap()
            .as_secs();
        let stage_ts = now - 90;
        let phase_started_at = now - 30 * 60;

        let mut state = State::new(7, AgentKind::Claude, Mode::Auto, root.to_path_buf());
        state.started_at = phase_started_at.to_string();
        workflow::save_state(&state).unwrap();

        events::emit(
            root,
            7,
            "stage_launched",
            serde_json::json!({"stage": "code", "agent": "claude", "monitor_pid": 1}),
        );
        // events::emit always stamps `ts` with the current time; rewrite it
        // to a fixed, known-past value so the assertion is deterministic
        // instead of racing the live clock.
        let events_path = devflow_core::events::events_path(root);
        let rewritten: String = std::fs::read_to_string(&events_path)
            .unwrap()
            .lines()
            .map(|line| {
                let mut value: serde_json::Value = serde_json::from_str(line).unwrap();
                value["ts"] = serde_json::json!(stage_ts);
                value.to_string()
            })
            .collect::<Vec<_>>()
            .join("\n")
            + "\n";
        std::fs::write(&events_path, rewritten).unwrap();

        let ts = latest_stage_launched_ts(root, 7);
        assert_eq!(ts, Some(stage_ts));

        let line = render_stage_progress_line(Stage::Code, ts);
        assert!(line.contains("1m ago"), "expected ~90s age, got: {line}");
        assert!(
            !line.contains("30m ago"),
            "must not render phase-level started_at age: {line}"
        );
    }

    #[test]
    fn render_stage_progress_line_omits_age_without_stage_launched_event() {
        assert_eq!(
            render_stage_progress_line(Stage::Plan, None),
            "  in stage plan"
        );
    }

    /// Unit tests for the pure `doctor` reconciliation core (18a). Each test
    /// builds a `PhaseFacts` directly — no repository, no I/O — proving
    /// `reconcile_phase` is a predicate over facts alone.
    #[cfg(test)]
    mod doctor_reconciliation {
        use super::*;

        /// A fully-agreeing baseline: `reconcile_phase` over this returns
        /// zero findings. Each test overrides only the field(s) needed to
        /// trigger the one check it's proving.
        fn agreeing_facts(phase: u32) -> PhaseFacts {
            PhaseFacts {
                phase,
                stage: Stage::Code,
                gate_pending: false,
                agent_pid: Some(4242),
                agent_alive: true,
                monitor_pid: Some(4343),
                monitor_alive: true,
                last_event: Some("stage_launched".into()),
                last_launched_stage: Some(Stage::Code),
                open_gate_stages: Vec::new(),
                feature_branch_exists: true,
                stopped: false,
            }
        }

        #[test]
        fn reconcile_phase_returns_no_findings_when_all_agree() {
            let facts = agreeing_facts(1);
            assert!(reconcile_phase(&facts).is_empty());
        }

        #[test]
        fn reconcile_phase_flags_gate_pending_without_open_gate() {
            let facts = PhaseFacts {
                gate_pending: true,
                open_gate_stages: Vec::new(),
                ..agreeing_facts(2)
            };
            let findings = reconcile_phase(&facts);
            assert_eq!(findings.len(), 1);
            assert_eq!(findings[0].severity, Severity::Problem);
            assert!(findings[0].detail.contains("gate_pending is true"));
            assert_eq!(
                findings[0].repair.as_deref(),
                Some("devflow resume --phase 2")
            );
        }

        #[test]
        fn reconcile_phase_flags_orphan_open_gate() {
            let facts = PhaseFacts {
                gate_pending: false,
                open_gate_stages: vec![Stage::Validate],
                ..agreeing_facts(3)
            };
            let findings = reconcile_phase(&facts);
            assert_eq!(findings.len(), 1);
            assert_eq!(findings[0].severity, Severity::Problem);
            assert!(findings[0].detail.contains("gate open for stage validate"));
            assert_eq!(
                findings[0].repair.as_deref(),
                Some("devflow gate approve 3 --stage validate")
            );
        }

        #[test]
        fn reconcile_phase_flags_dead_agent_at_agent_stage() {
            let facts = PhaseFacts {
                agent_pid: Some(999_999),
                agent_alive: false,
                ..agreeing_facts(4)
            };
            let findings = reconcile_phase(&facts);
            assert_eq!(findings.len(), 1);
            assert_eq!(findings[0].severity, Severity::Problem);
            assert!(findings[0].detail.contains("agent pid 999999"));
            assert_eq!(
                findings[0].repair.as_deref(),
                Some("devflow resume --phase 4")
            );
        }

        #[test]
        fn reconcile_phase_flags_stage_event_drift() {
            let facts = PhaseFacts {
                stage: Stage::Validate,
                last_launched_stage: Some(Stage::Code),
                ..agreeing_facts(5)
            };
            let findings = reconcile_phase(&facts);
            assert_eq!(findings.len(), 1);
            assert_eq!(findings[0].severity, Severity::Warn);
            assert!(
                findings[0]
                    .detail
                    .contains("last stage_launched event named code")
            );
            assert!(findings[0].repair.is_none());
        }

        #[test]
        fn reconcile_phase_flags_missing_feature_branch() {
            let facts = PhaseFacts {
                stage: Stage::Plan,
                last_launched_stage: Some(Stage::Plan),
                feature_branch_exists: false,
                ..agreeing_facts(6)
            };
            let findings = reconcile_phase(&facts);
            assert_eq!(findings.len(), 1);
            assert_eq!(findings[0].severity, Severity::Warn);
            assert!(findings[0].detail.contains("feature/phase-06"));
            assert!(findings[0].repair.is_none());
        }

        /// 18b: a dead monitor with a dead agent is `Stuck` — nothing will
        /// call `devflow advance` for this phase — and reports a `Problem`
        /// finding with a `devflow resume --phase N` repair.
        #[test]
        fn reconcile_reports_stuck_when_monitor_and_agent_are_both_dead() {
            let facts = PhaseFacts {
                monitor_pid: Some(5150),
                monitor_alive: false,
                agent_pid: Some(4242),
                agent_alive: false,
                ..agreeing_facts(8)
            };
            let findings = reconcile_phase(&facts);
            let monitor_finding = findings
                .iter()
                .find(|f| f.detail.contains("monitor pid"))
                .expect("expected a monitor finding when monitor and agent are both dead");
            assert_eq!(monitor_finding.severity, Severity::Problem);
            assert!(monitor_finding.detail.contains("monitor pid 5150"));
            assert_eq!(
                monitor_finding.repair.as_deref(),
                Some("devflow resume --phase 8")
            );
        }

        /// 20c (D-09 + review: Codex HIGH — the doctor gap is bigger than
        /// `check_dead_agent`): a phase intentionally halted by `devflow
        /// start --until <stage>` sits at an agent stage with a dead agent
        /// pid on disk. `check_dead_agent` must recognize `facts.stopped`
        /// and report ZERO findings — this is not a crash.
        #[test]
        fn reconcile_phase_ignores_dead_agent_when_stopped() {
            let facts = PhaseFacts {
                stage: Stage::Plan,
                agent_pid: Some(999_999),
                agent_alive: false,
                stopped: true,
                ..agreeing_facts(11)
            };
            let findings = reconcile_phase(&facts);
            assert!(
                findings.iter().all(|f| f.severity != Severity::Problem),
                "a --until-stopped phase must yield zero Problem findings, got: \
                 {:?}",
                findings.iter().map(|f| &f.detail).collect::<Vec<_>>()
            );
        }

        /// 20c (D-09 + review: Codex HIGH — the doctor gap is bigger than
        /// `check_dead_agent`): the same stopped phase may also carry a
        /// stale `monitor_pid` (though the stop path clears it — this
        /// proves the guard holds even if that clear were ever bypassed).
        /// `check_dead_monitor` must also recognize `facts.stopped` and
        /// report ZERO findings.
        #[test]
        fn reconcile_phase_ignores_dead_monitor_when_stopped() {
            let facts = PhaseFacts {
                stage: Stage::Plan,
                monitor_pid: Some(5150),
                monitor_alive: false,
                agent_pid: Some(4242),
                agent_alive: false,
                stopped: true,
                ..agreeing_facts(12)
            };
            let findings = reconcile_phase(&facts);
            assert!(
                findings.iter().all(|f| f.severity != Severity::Problem),
                "a --until-stopped phase must yield zero Problem findings even with a \
                 stale monitor_pid, got: {:?}",
                findings.iter().map(|f| &f.detail).collect::<Vec<_>>()
            );
        }

        /// 18b (T-18-11): an unrecorded monitor is unknown, not a problem —
        /// a state file written by a pre-18b binary must never render as
        /// stuck.
        #[test]
        fn reconcile_is_silent_when_monitor_pid_is_unrecorded() {
            let facts = PhaseFacts {
                monitor_pid: None,
                monitor_alive: false,
                ..agreeing_facts(9)
            };
            assert!(
                reconcile_phase(&facts).is_empty(),
                "an unrecorded monitor must never produce a finding"
            );
        }

        /// 18b: a live monitor with a dead agent is a normal between-stages
        /// moment (the monitor hasn't advanced the phase yet), not a monitor
        /// finding. `check_dead_agent`'s own pre-existing finding for the
        /// dead agent pid is unrelated to this check and out of this plan's
        /// scope.
        #[test]
        fn reconcile_is_silent_when_monitor_alive_and_agent_dead() {
            let facts = PhaseFacts {
                monitor_pid: Some(5150),
                monitor_alive: true,
                agent_alive: false,
                ..agreeing_facts(10)
            };
            let findings = reconcile_phase(&facts);
            assert!(
                findings.iter().all(|f| !f.detail.contains("monitor pid")),
                "a live monitor with a dead agent must not produce a monitor finding"
            );
        }

        /// Several checks trigger simultaneously; the returned findings must
        /// come back in the fixed order `reconcile_phase` evaluates checks
        /// in, not in whatever order the facts happen to be populated.
        #[test]
        fn reconcile_phase_ordering_is_input_order_independent() {
            let facts = PhaseFacts {
                gate_pending: true,
                agent_pid: Some(999_999),
                agent_alive: false,
                monitor_pid: Some(999_998),
                monitor_alive: false,
                last_launched_stage: Some(Stage::Validate),
                open_gate_stages: Vec::new(),
                feature_branch_exists: false,
                ..agreeing_facts(7)
            };
            let findings = reconcile_phase(&facts);
            let severities: Vec<Severity> = findings.iter().map(|f| f.severity).collect();
            assert_eq!(
                severities,
                vec![
                    Severity::Problem, // check_gate_pending_without_gate
                    Severity::Problem, // check_dead_agent
                    Severity::Problem, // check_dead_monitor
                    Severity::Warn,    // check_stage_event_drift
                    Severity::Warn,    // check_missing_branch
                ]
            );
            assert!(findings[0].detail.contains("gate_pending is true"));
            assert!(findings[1].detail.contains("agent pid 999999"));
            assert!(findings[2].detail.contains("monitor pid 999998"));
            assert!(
                findings[3]
                    .detail
                    .contains("last stage_launched event named validate")
            );
            assert!(findings[4].detail.contains("feature/phase-07"));
        }

        /// `doctor`'s idle-project path (Task 2, 18a): the exact code path
        /// `doctor(root, false)` runs for its reconciliation section is
        /// `collect_phase_facts` + `render_reconciliation_text` — asserted
        /// directly here rather than capturing process stdout, since this
        /// codebase has no stdout-capture dependency and this phase adds no
        /// new ones (18-RESEARCH.md).
        #[test]
        fn doctor_reports_no_active_phases_when_idle() {
            let dir = tempfile::tempdir().unwrap();
            let facts = collect_phase_facts(dir.path());
            assert!(facts.is_empty());
            assert!(render_reconciliation_text(&facts).contains("no active phases"));
        }

        #[test]
        fn doctor_reports_gate_pending_without_gate_file() {
            let dir = tempfile::tempdir().unwrap();
            let root = dir.path();
            let phase = 90;
            let mut state = State::new(phase, AgentKind::Claude, Mode::Auto, root.to_path_buf());
            state.stage = Stage::Validate;
            state.gate_pending = true;
            workflow::save_state(&state).unwrap();

            let facts = collect_phase_facts(root);
            assert_eq!(facts.len(), 1);
            let text = render_reconciliation_text(&facts);
            assert!(text.contains(&format!("phase {phase}: gate_pending is true")));
            assert!(text.contains(&format!("repair: devflow resume --phase {phase}")));
        }

        /// WR-01 (18-fix): `doctor --json` must emit ONE JSON document, not
        /// two concatenated top-level arrays. Exercises the exact
        /// composition `doctor()`'s `--json` path uses (`doctor_json_body`),
        /// then round-trips it through `serde_json::to_string`/`from_str` —
        /// the failure mode this reproduces (pre-fix) is a single-document
        /// parser (`json.load`, `JSON.parse`) raising "Extra data" on the
        /// old two-array output; `jq` tolerated it (NDJSON-style streaming),
        /// which is why it went unnoticed.
        #[test]
        fn doctor_json_is_a_single_object_with_environment_and_reconciliation() {
            let checks = vec![Check {
                name: "git".into(),
                status: "ok".into(),
                version: Some("2.40.0".into()),
                install_hint: None,
            }];

            let dir = tempfile::tempdir().unwrap();
            let root = dir.path();
            let phase = 92;
            let mut state = State::new(phase, AgentKind::Claude, Mode::Auto, root.to_path_buf());
            state.stage = Stage::Validate;
            state.gate_pending = true; // mismatched: no gate file — produces a finding
            workflow::save_state(&state).unwrap();
            let facts = collect_phase_facts(root);

            let body = doctor_json_body(&checks, &facts, &[]);
            let serialized = serde_json::to_string(&body).unwrap();
            let reparsed: serde_json::Value = serde_json::from_str(&serialized)
                .expect("doctor --json must be single-document JSON, not two concatenated arrays");

            assert!(
                reparsed.get("environment").is_some(),
                "must carry the tool checks under \"environment\": {reparsed}"
            );
            assert!(
                reparsed.get("reconciliation").is_some(),
                "must carry the reconciliation findings under \"reconciliation\": {reparsed}"
            );
            assert!(
                reparsed.get("planning_doc_staleness").is_some(),
                "21b: must carry the planning-doc findings under a THIRD key, \
                 never a second concatenated array: {reparsed}"
            );
            assert_eq!(
                reparsed.as_object().unwrap().len(),
                3,
                "doctor --json must have exactly three top-level keys: {reparsed}"
            );
            assert!(reparsed["environment"].is_array());
            assert!(reparsed["reconciliation"].is_array());
            assert!(reparsed["planning_doc_staleness"].is_array());
            let reconciliation = reparsed["reconciliation"].as_array().unwrap();
            assert!(
                !reconciliation.is_empty(),
                "the mismatched gate_pending fixture must produce at least one finding"
            );
            assert!(
                reconciliation.iter().any(|f| f["detail"]
                    .as_str()
                    .unwrap_or("")
                    .contains("gate_pending is true")),
                "must carry the gate_pending finding: {reconciliation:?}"
            );
        }

        /// T-18-02: running `doctor` twice against a mismatched fixture must
        /// leave `.devflow/` byte-identical — no state rewrite, no event
        /// append, no gate file appears or disappears.
        #[test]
        fn doctor_is_read_only_on_a_mismatched_project() {
            let dir = tempfile::tempdir().unwrap();
            let root = dir.path();
            let phase = 91;
            let mut state = State::new(phase, AgentKind::Claude, Mode::Auto, root.to_path_buf());
            state.stage = Stage::Validate;
            state.gate_pending = true; // mismatched: no gate file will exist
            workflow::save_state(&state).unwrap();
            events::emit(
                root,
                phase,
                "stage_launched",
                serde_json::json!({"stage": "code"}),
            );

            let state_path = workflow::state_path(root, phase);
            let before_len = std::fs::metadata(&state_path).unwrap().len();
            let before_modified = std::fs::metadata(&state_path).unwrap().modified().unwrap();
            let events_log = events::events_path(root);
            let before_lines = std::fs::read_to_string(&events_log)
                .unwrap()
                .lines()
                .count();

            doctor(root, false).unwrap();
            doctor(root, false).unwrap();

            let after_len = std::fs::metadata(&state_path).unwrap().len();
            let after_modified = std::fs::metadata(&state_path).unwrap().modified().unwrap();
            let after_lines = std::fs::read_to_string(&events_log)
                .unwrap()
                .lines()
                .count();

            assert_eq!(
                before_len, after_len,
                "doctor must not rewrite the state file"
            );
            assert_eq!(
                before_modified, after_modified,
                "doctor must not touch the state file's mtime"
            );
            assert_eq!(
                before_lines, after_lines,
                "doctor must not append to events.jsonl"
            );
        }
    }

    /// Unit tests for the pure planning-doc staleness core (21b, D-04/D-05).
    /// `reconcile_planning_docs` takes an injected `tag_lookup` closure, so
    /// every test here runs with zero I/O and no real repository — mirrors
    /// `doctor_reconciliation`'s zero-I/O discipline above.
    #[cfg(test)]
    mod planning_doc_staleness {
        use super::*;

        const SAMPLE_TABLE: &str = "\
| Phase | Name | Version |
|---|---|---|
| 20 | Release Correctness | 1.7.0 |
| 10 | Logging | — |
| 1–5 | Core workflow | 0.1.0–0.6.0 |
| 9 | OSS Polish | 1.2.0 |
| 11 | GSD-Native | 1.2.0 |
";

        #[test]
        fn parse_planning_doc_versions_skips_non_semver_cells() {
            let rows = parse_planning_doc_versions(SAMPLE_TABLE, "ROADMAP.md");
            assert_eq!(
                rows,
                vec![
                    ("ROADMAP.md phase 20".to_string(), "1.7.0".to_string()),
                    ("ROADMAP.md phase 9".to_string(), "1.2.0".to_string()),
                    ("ROADMAP.md phase 11".to_string(), "1.2.0".to_string()),
                ],
                "em-dash and range cells must be skipped; duplicate versions across \
                 phases (9 and 11 both claim 1.2.0) must both still parse"
            );
        }

        #[test]
        fn parse_planning_doc_versions_accepts_v_prefixed_cells() {
            let text = "| Phase | Description | Version | Date |\n\
                         |---|---|---|---|\n\
                         | 18 | Dogfood Hardening | v1.5.0 | 2026-07-21 |\n";
            let rows = parse_planning_doc_versions(text, "STATE.md");
            assert_eq!(
                rows,
                vec![("STATE.md phase 18".to_string(), "v1.5.0".to_string())]
            );
        }

        #[test]
        fn parse_semver_rejects_ranges_and_em_dash() {
            assert_eq!(parse_semver("1.7.0"), Some((1, 7, 0)));
            assert_eq!(parse_semver("v1.7.0"), Some((1, 7, 0)));
            assert_eq!(parse_semver("0.1.0–0.6.0"), None);
            assert_eq!(parse_semver("—"), None);
            assert_eq!(parse_semver("1.7"), None);
            assert_eq!(parse_semver("1.7.0.1"), None);
        }

        #[test]
        fn reconcile_planning_docs_flags_problem_for_unreachable_post_cutoff_version() {
            let rows = vec![("ROADMAP.md phase 20".to_string(), "1.7.0".to_string())];
            let mut lookup = |_tag: &str| false; // no tag exists / unreachable
            let findings = reconcile_planning_docs(&rows, &mut lookup);
            assert_eq!(findings.len(), 1);
            assert_eq!(findings[0].severity, Severity::Problem);
            assert!(
                findings[0].repair.is_none(),
                "D-04: detection-only, no repair"
            );
            assert!(findings[0].detail.contains("v1.7.0"));
        }

        #[test]
        fn reconcile_planning_docs_downgrades_pre_cutoff_mismatch_to_warn() {
            // Phase 7 claims 1.0.0 in this repo's real ROADMAP.md/STATE.md,
            // but no v1.0.0 tag exists (tags start at v1.0.1) — must never
            // surface as Problem (RESEARCH Pitfall #2).
            let rows = vec![("ROADMAP.md phase 7".to_string(), "1.0.0".to_string())];
            let mut lookup = |_tag: &str| false;
            let findings = reconcile_planning_docs(&rows, &mut lookup);
            assert_eq!(findings.len(), 1);
            assert_eq!(
                findings[0].severity,
                Severity::Warn,
                "pre-v1.5.0 mismatches must downgrade to Warn, never Problem"
            );
            assert!(findings[0].repair.is_none());
        }

        #[test]
        fn reconcile_planning_docs_numeric_cutoff_is_not_lexicographic() {
            // A lexicographic string compare would sort "1.10.0" < "1.5.0"
            // and wrongly downgrade a real future release to Warn (Codex
            // MEDIUM, cross-AI review). The cutoff must compare
            // parse_semver's numeric tuple instead.
            let rows = vec![
                ("label A".to_string(), "1.10.0".to_string()),
                ("label B".to_string(), "1.4.0".to_string()),
            ];
            let mut lookup = |_tag: &str| false;
            let findings = reconcile_planning_docs(&rows, &mut lookup);
            assert_eq!(findings.len(), 2);
            assert_eq!(
                findings[0].severity,
                Severity::Problem,
                "1.10.0 is numerically >= v1.5.0 (post-cutoff), even though \
                 \"1.10.0\" < \"1.5.0\" as a string"
            );
            assert_eq!(
                findings[1].severity,
                Severity::Warn,
                "1.4.0 is numerically < v1.5.0 (pre-cutoff)"
            );
        }

        #[test]
        fn reconcile_planning_docs_produces_no_finding_when_tag_is_reachable() {
            let rows = vec![("ROADMAP.md phase 20".to_string(), "1.7.0".to_string())];
            let mut lookup = |_tag: &str| true; // tag exists and is reachable
            let findings = reconcile_planning_docs(&rows, &mut lookup);
            assert!(findings.is_empty());
        }

        #[test]
        fn reconcile_planning_docs_normalizes_bare_cell_to_v_prefixed_tag() {
            let rows = vec![("ROADMAP.md phase 20".to_string(), "1.7.0".to_string())];
            let mut seen_tag = None;
            let mut lookup = |tag: &str| {
                seen_tag = Some(tag.to_string());
                true
            };
            reconcile_planning_docs(&rows, &mut lookup);
            assert_eq!(seen_tag.as_deref(), Some("v1.7.0"));
        }

        #[test]
        fn reconcile_planning_docs_skips_a_malformed_row_defensively() {
            // Defensive path: reconcile must never panic even if handed a
            // row whose version cell isn't a semver (parse_planning_doc_versions
            // already filters this upstream, but reconcile must degrade, not die).
            let rows = vec![("bad row".to_string(), "not-a-version".to_string())];
            let mut lookup = |_tag: &str| false;
            let findings = reconcile_planning_docs(&rows, &mut lookup);
            assert!(findings.is_empty());
        }

        /// Fixture-backed proof of `tag_exists_and_reachable`'s two-check
        /// contract, mirroring `staleness::init_repo_with_diverged_commit`'s
        /// idiom: a real tempdir git repo with a tagged, reachable commit,
        /// an untagged commit, and a commit on a diverged, unreachable branch.
        fn init_tagged_repo(root: &Path) {
            let git = |args: &[&str]| {
                assert!(
                    std::process::Command::new("git")
                        .args(args)
                        .current_dir(root)
                        .output()
                        .unwrap()
                        .status
                        .success(),
                    "git {args:?} failed"
                );
            };
            git(&["init", "-q", "-b", "main"]);
            git(&["config", "user.email", "t@e.st"]);
            git(&["config", "user.name", "t"]);
            git(&["config", "commit.gpgsign", "false"]);
            git(&["config", "tag.gpgsign", "false"]);
            git(&["config", "core.hooksPath", "/dev/null"]);
            std::fs::write(root.join("a.txt"), "one").unwrap();
            git(&["add", "."]);
            git(&["commit", "-q", "-m", "base"]);
            git(&["tag", "v1.7.0"]);

            git(&["checkout", "-q", "-b", "side"]);
            std::fs::write(root.join("side.txt"), "s").unwrap();
            git(&["add", "."]);
            git(&["commit", "-q", "-m", "side"]);
            git(&["tag", "v9.9.9"]); // tagged, but only reachable from `side`, not `main`

            git(&["checkout", "-q", "main"]);
        }

        #[test]
        fn tag_exists_and_reachable_true_for_a_tagged_ancestor() {
            let dir = tempfile::tempdir().unwrap();
            init_tagged_repo(dir.path());
            assert!(tag_exists_and_reachable(dir.path(), "v1.7.0", "main"));
        }

        #[test]
        fn tag_exists_and_reachable_false_for_a_missing_tag() {
            let dir = tempfile::tempdir().unwrap();
            init_tagged_repo(dir.path());
            assert!(!tag_exists_and_reachable(dir.path(), "v0.0.1", "main"));
        }

        #[test]
        fn tag_exists_and_reachable_false_for_a_tag_unreachable_from_base() {
            let dir = tempfile::tempdir().unwrap();
            init_tagged_repo(dir.path());
            assert!(!tag_exists_and_reachable(dir.path(), "v9.9.9", "main"));
        }

        /// D-05/D-04: a MISSING `.planning/ROADMAP.md`/`STATE.md` must yield
        /// no findings and never an error — `doctor` must not fabricate a
        /// `Problem` from an absent doc. Proven against a tempdir with no
        /// `.planning/` directory at all, not just asserted.
        #[test]
        fn collect_planning_doc_findings_missing_files_yield_no_findings_not_error() {
            let dir = tempfile::tempdir().unwrap();
            let findings = collect_planning_doc_findings(dir.path());
            assert!(
                findings.is_empty(),
                "a project with no .planning/ dir at all must yield zero findings, not an error"
            );
        }

        #[test]
        fn render_planning_doc_text_reports_consistent_when_no_findings() {
            assert_eq!(
                render_planning_doc_text(&[]),
                "\nplanning docs: consistent with git tags\n"
            );
        }

        #[test]
        fn render_planning_doc_text_lists_each_finding_detail() {
            let findings = vec![PlanningDocFinding {
                source: "ROADMAP.md phase 20".to_string(),
                claim: "ROADMAP.md phase 20 claims v1.7.0".to_string(),
                severity: Severity::Problem,
                detail: "ROADMAP.md phase 20 claims v1.7.0, but no git tag `v1.7.0` exists"
                    .to_string(),
                repair: None,
            }];
            let text = render_planning_doc_text(&findings);
            assert!(text.contains("[problem]"));
            assert!(text.contains("ROADMAP.md phase 20 claims v1.7.0"));
        }

        #[test]
        fn render_planning_doc_findings_json_is_an_array_of_objects() {
            let findings = vec![PlanningDocFinding {
                source: "ROADMAP.md phase 20".to_string(),
                claim: "ROADMAP.md phase 20 claims v1.7.0".to_string(),
                severity: Severity::Problem,
                detail: "detail text".to_string(),
                repair: None,
            }];
            let value = render_planning_doc_findings_json(&findings);
            assert!(value.is_array());
            let arr = value.as_array().unwrap();
            assert_eq!(arr.len(), 1);
            assert_eq!(arr[0]["severity"], "problem");
            assert_eq!(arr[0]["source"], "ROADMAP.md phase 20");
            assert_eq!(arr[0]["repair"], serde_json::Value::Null);
        }

        /// D-05/Pattern 2: `doctor --json` must stay a SINGLE JSON object
        /// with `planning_doc_staleness` as a THIRD key, never a second
        /// top-level array — the exact WR-01 regression class this phase
        /// must not reintroduce.
        #[test]
        fn doctor_json_body_carries_planning_doc_staleness_as_a_third_key() {
            let checks: Vec<Check> = Vec::new();
            let facts: Vec<PhaseFacts> = Vec::new();
            let doc_findings = vec![PlanningDocFinding {
                source: "ROADMAP.md phase 20".to_string(),
                claim: "claim".to_string(),
                severity: Severity::Problem,
                detail: "detail".to_string(),
                repair: None,
            }];
            let body = doctor_json_body(&checks, &facts, &doc_findings);
            let obj = body.as_object().unwrap();
            assert_eq!(
                obj.len(),
                3,
                "must be exactly {{environment, reconciliation, planning_doc_staleness}}: {body}"
            );
            assert!(obj.contains_key("environment"));
            assert!(obj.contains_key("reconciliation"));
            let staleness = obj["planning_doc_staleness"].as_array().unwrap();
            assert_eq!(staleness.len(), 1);
        }
    }
}