tix-tracker 0.8.0

//! `tix lint`: validate frontmatter option values in every issue file
//! (`contracts/cli.md` §lint, FR-015..FR-018).
//!
//! Checks per issue (FR-016): `status` ∈ `tix.toml [columns]`, `priority` ∈
//! {low, medium, high}, `created`/`updated` parse as `YYYY-MM-DD`, `estimate`
//! absent-or-numeric, `number` integer. Unknown frontmatter keys are never
//! flagged.
//!
//! Lint walks the store itself (mirroring [`crate::storage::Store::list`]
//! layout: one segment level + flat root, skipping `.tix-tmp-*` dirs and
//! non-`.md` files) instead of calling [`Store::list`], because `list` abort
//! on the first malformed file. Lint must keep scanning and report each
//! broken file as a `missing`/`parse` diagnostic — never a panic (FR-018).
//!
//! Diagnostics render through `annotate-snippets` (C-004): one rustc-style
//! block per finding, and every block names the file, line, offending value
//! and valid set in a single reading (NFR-006). Diagnostics go to **stderr**;
//! stdout stays parseable and only carries the `Lint OK: N issues checked.`
//! summary on a healthy store. Exit semantics live at the CLI boundary
//! (`cli::run_lint`): `0` healthy, `1` findings, `2` usage/internal.
//!
//! **Only errors set the exit code** (#1361). A finding carries a
//! [`Severity`]: an unrecognised top-level frontmatter key is a *warning*,
//! because the key is preserved on write and the file is therefore valid. It is
//! rendered with `Level::WARNING` and a `W-` id so the output agrees with the
//! exit code, and `[custom_attributes]` in `tix.toml` silences it for keys the
//! project uses on purpose.

use std::collections::BTreeMap;
use std::collections::HashSet;
use std::fs;
use std::ops::Range;
use std::path::{Path, PathBuf};

use crate::commands::create::planner_for;
use crate::config::Config;
use crate::diagnostics::Diagnostic;
use crate::error::{Result, TixError};
use crate::model::{Issue, Priority};
use crate::planning::Planner;
use crate::related::{self, RelatedRef};
use crate::relationships::{IssueView, RelationshipGraph};
use crate::storage::Store;
use crate::workflow::{DiagnosticKind, validate_config};

/// How much a finding should worry the reader.
///
/// Added for #1361. Everything that existed before is an [`Severity::Error`], and
/// the distinction is exactly one of consequence: only errors set the exit code.
/// A file carrying an unrecognised key is still a **valid** file — it parses, and
/// the key is now preserved on write — so failing a build over it would be
/// reporting a non-problem as a broken one.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Severity {
    /// Sets the exit code. A real problem with the file.
    Error,
    /// Printed, counted, and **not** fatal. Worth knowing; not worth failing over.
    Warning,
}

/// One lint finding, carrying enough location to render a rustc-style
/// diagnostic (NFR-006). `line`/`column` are 1-based; file-level findings
/// (`missing`/`parse`) use `line == 0`.
#[derive(Debug)]
pub struct TixIssueFinding {
    /// Whether this finding sets the exit code. See [`Severity`].
    pub severity: Severity,
    /// Issue file, relative to the issues dir.
    pub file: String,
    /// 1-based line of the offending value; `0` for file-level findings.
    pub line: usize,
    /// 1-based column of the offending value; `0` for file-level findings.
    pub column: usize,
    /// The offending value (or an empty string for file-level findings).
    pub value: String,
    /// `status`/`priority`/`created`/`updated`/`estimate`/`number`/`missing`/`parse`.
    pub kind: String,
    /// Human message, e.g. the valid set or the parse error.
    pub message: String,
}

/// Lint every issue file in the store.
///
/// Returns `true` when at least one finding was reported (the CLI boundary
/// turns that into exit `1`; `false` stays exit `0`).
pub fn run(config: &Config, store: &Store) -> Result<bool> {
    run_with(config, store, config.lint_foreign_repositories)
}

/// As [`run`], with an explicit foreign-lint switch for the CLI's `--no-foreign`.
pub fn run_with(config: &Config, store: &Store, foreign: bool) -> Result<bool> {
    let findings = collect_with(config, store, foreign)?;
    if findings.is_empty() {
        println!("Lint OK: {} issues checked.", issue_files(store)?.len());
        return Ok(false);
    }
    for finding in &findings {
        eprintln!("{}", render_diagnostic(&store.issues_dir, finding));
    }
    // **Only errors set the exit code** (#1361). A warning is printed and
    // counted, and a file with warnings still exits 0: the unknown key it carries
    // is preserved on write, so the file is valid, and failing a build over it
    // would report a non-problem as a broken file.
    let errors = findings
        .iter()
        .filter(|f| f.severity == Severity::Error)
        .count();
    let warnings = findings.len() - errors;
    if errors == 0 {
        println!(
            "Lint OK with {warnings} warning{}: {} issues checked.",
            if warnings == 1 { "" } else { "s" },
            issue_files(store)?.len()
        );
    }
    Ok(errors > 0)
}

/// The lint core (WP03): walk every issue file and return the raw
/// [`TixIssueFinding`] vector — no rendering, no printing (C-005).
///
/// This is what MCP's `lint_issues` tool (WP04) calls, mapping findings to
/// `structuredContent`. The validation walk continues past malformed files
/// (`missing`/`parse` findings, FR-018) exactly as the CLI path did; the
/// `annotate-snippets` rendering and the exit-bool semantics stay in [`run`].
pub fn collect(config: &Config, store: &Store) -> Result<Vec<TixIssueFinding>> {
    collect_with(config, store, config.lint_foreign_repositories)
}

/// As [`collect`], but with an explicit foreign-lint switch — the CLI's
/// `--no-foreign` and the MCP `foreign` argument override the config default for
/// one run.
pub fn collect_with(config: &Config, store: &Store, foreign: bool) -> Result<Vec<TixIssueFinding>> {
    let mut findings = collect_local(config, store)?;
    if foreign {
        let mut visited = HashSet::new();
        // Seed with this store's own root so a reference back into it is skipped.
        if let Some(root) = Config::project_root(&store.issues_dir) {
            visited.insert(root);
        }
        collect_foreign(store, &mut visited, &mut findings);
    }
    Ok(findings)
}

fn collect_local(config: &Config, store: &Store) -> Result<Vec<TixIssueFinding>> {
    let files = issue_files(store)?;
    let planner = planner_for(config);
    let milestone_names = planner.milestone_names().unwrap_or_default();
    let mut findings: Vec<TixIssueFinding> = Vec::new();
    let mut parsed: Vec<ParsedFile> = Vec::new();
    for rel in &files {
        if let Some(entry) = collect_for_file(
            config,
            store,
            &planner,
            &milestone_names,
            rel,
            &mut findings,
        )? {
            parsed.push(entry);
        }
    }
    // One sweep over what that walk produced, after the per-file pass, so the
    // relationship findings are not interleaved with per-file ones (T026 step 2).
    // A file that failed to parse is already reported as `parse`/`missing` and is
    // simply absent from the graph — which is the correct scope for a snapshot
    // check, since the issue's relationships are unknowable when it cannot be read.
    collect_relationships(&parsed, store, &mut findings);
    // The same shape for the hierarchy: one whole-snapshot pass, after the
    // per-file one, so hierarchy findings are not interleaved with per-file ones.
    collect_hierarchy(&parsed, &mut findings);
    // WP09 item A: the project-reference family — one whole-snapshot pass over the
    // service's `validate_references`, so a dangling association the registry no
    // longer holds is reported by name (C-005, `contracts/cli-surface.md` §4). Like
    // the hierarchy pass beside it, this is observational and reports nothing for a
    // file that failed to parse (already reported as `parse`/`missing`).
    collect_project_references(config, &parsed, &mut findings);
    // And the epic-reference family (WP09 item A): the same whole-snapshot shape,
    // from the epic service's own `resolve_references` (C-005, contracts
    // `cli-surface.md` §4 / `epic-registry.md` §5). It covers **every** reference
    // kind the service diagnoses — issue-held epic ids and an epic's own
    // project/milestone/cycle/parent references — with the service's own reason
    // wording, so lint and the epic surfaces cannot describe one dangling reference
    // two ways.
    collect_epic_references(config, &planner, &parsed, &mut findings);
    // Per-entry references need the store to resolve local selectors, so this is
    // a snapshot pass like the two above rather than a per-file rule.
    collect_related_issues(&parsed, store, &mut findings);
    collect_layout_mismatch(config, &parsed, &mut findings);
    // And the same for the two workflow families (WP06 T026/T027): the
    // configuration check, which needs no issue at all, and the direct-edit check,
    // which needs the parsed issues and their History. Both reuse the service's
    // diagnostics rather than re-deriving the rules, so `tix lint` and
    // `tix workflow show` cannot report one condition two ways (C-003).
    collect_workflow_config(config, &mut findings);
    collect_workflow_direct_edits(config, &parsed, &mut findings);
    // And the WIP family (WP07 T032, mission `configurable-wip-limits-01M3C6WX`):
    // one whole-snapshot pass, from the same shared aggregate the board renders, so
    // lint and the board cannot disagree about how full a column is. Lint **reports**
    // and never repairs (C-007): the findings below are the whole of its
    // contribution, and no file is rewritten.
    collect_wip(config, &parsed, &mut findings);
    Ok(findings)
}

/// The WIP findings: a column over its limit, and an estimate nobody can count
/// (WP07 T032/T033).
///
/// Two shapes, deliberately different:
///
/// - an **over-limit column** is one finding with no source file — the condition is
///   about a column, and pointing it at one of the cards in it would blame a file
///   that is fine. `line`/`column` are `0` and `file` is empty, which is what the
///   renderer treats as a configuration finding;
/// - an **uncountable estimate** names the issue's own file and value, because that
///   is a fact about that file and the reader can fix it there.
///
/// Both are warnings: a column over its limit is a policy violation worth knowing
/// about, not a broken file, and failing a build over it would make every
/// over-quota repository red.
fn collect_wip(config: &Config, parsed: &[ParsedFile], findings: &mut Vec<TixIssueFinding>) {
    if parsed.is_empty() {
        return;
    }
    let issues: Vec<Issue> = parsed.iter().map(|p| p.issue.clone()).collect();
    let snapshot = crate::wip::WipSnapshot::build(config.default_config(), &issues);
    if !snapshot.is_configured() {
        return;
    }
    for usage in snapshot.violations() {
        findings.push(TixIssueFinding {
            severity: Severity::Warning,
            file: String::new(),
            line: 0,
            column: 0,
            value: usage.cell(),
            kind: crate::wip::WipDiagnosticKind::OverLimit
                .as_str()
                .to_string(),
            message: format!(
                "`{}` holds {} against {}",
                usage.column,
                usage.cell(),
                usage
                    .limit
                    .as_ref()
                    .map_or_else(String::new, describe_wip_limit)
            ),
        });
    }
    // An uncountable estimate makes the column's total unknown, which is the reason
    // a limit cannot be judged there — so it is reported per file, with the value
    // that could not be read.
    for diagnostic in snapshot.diagnostics() {
        let Some(entry) = parsed.iter().find(|parsed| {
            diagnostic
                .message
                .contains(&format!("#{}", parsed.issue.number))
        }) else {
            continue;
        };
        findings.push(TixIssueFinding {
            severity: Severity::Warning,
            file: entry.rel.clone(),
            line: 0,
            column: 0,
            value: entry.issue.estimate.clone().unwrap_or_default(),
            kind: diagnostic.kind.as_str().to_string(),
            message: diagnostic.message.clone(),
        });
    }
}

/// A limit as one phrase, for a lint message.
fn describe_wip_limit(limit: &crate::wip::ColumnLimit) -> String {
    let mut parts = Vec::new();
    if let Some(max) = limit.max_items {
        parts.push(format!("max_items {max}"));
    }
    if let Some(max) = limit.max_estimate {
        match limit.unit.as_deref() {
            Some(unit) => parts.push(format!("max_estimate {max}{unit}")),
            None => parts.push(format!("max_estimate {max}")),
        }
    }
    match limit.policy {
        Some(policy) => parts.push(policy.as_str().to_string()),
        None => parts.push("no policy".to_string()),
    }
    parts.join(", ")
}

/// Lint the foreign repositories reachable from this store's `related_issues`
/// path references, merging their findings (prefixed and de-duplicated).
///
/// Bounded: each repository is linted once per run (a `visited` set), one hop
/// deep (a foreign repo's own foreign repositories are not entered — the config
/// flag governs the *local* run only), and a hard cap. Best-effort: an
/// unreachable reference, a URL, or an unparsable foreign `tix.toml` becomes one
/// non-fatal note, never an error that hides the local results.
fn collect_foreign(
    store: &Store,
    visited: &mut HashSet<PathBuf>,
    findings: &mut Vec<TixIssueFinding>,
) {
    const MAX_REPOS: usize = 64;
    let Ok(issues) = store.list_all() else {
        return;
    };
    let mut candidates: Vec<PathBuf> = Vec::new();
    let mut remote: Vec<String> = Vec::new();
    let mut missing: Vec<String> = Vec::new();
    for issue in &issues {
        let base = related::base_dir(&store.issues_dir, &issue.filename);
        for r in related::classify_all(&issue.related_issues) {
            match &r {
                RelatedRef::Path { .. } => {
                    if let Some(target) = related::resolve_path(&base, &r) {
                        if !target.exists() {
                            // A path that points nowhere is worth a note; a path
                            // that exists but is not a tix repository (e.g. the
                            // chalkboard) is intentionally foreign and stays
                            // silent.
                            missing.push(r.display());
                            continue;
                        }
                        let start = if target.is_dir() {
                            target
                        } else {
                            target
                                .parent()
                                .map_or_else(|| target.clone(), Path::to_path_buf)
                        };
                        if let Some(root) = Config::project_root(&start) {
                            candidates.push(root);
                        }
                    }
                }
                RelatedRef::Url { .. } => remote.push(r.display()),
                RelatedRef::Local(_) => {}
            }
        }
    }
    candidates.sort();
    candidates.dedup();

    let mut linted = 0usize;
    for root in candidates {
        if linted >= MAX_REPOS {
            break;
        }
        if visited.contains(&root) {
            continue;
        }
        visited.insert(root.clone());
        match lint_foreign_repo(&root) {
            Ok((label, mut foreign)) => {
                for f in &mut foreign {
                    // A config-level finding has an empty file; prefix it with the
                    // repo alone so it does not render as `[repo] `.
                    f.file = if f.file.is_empty() {
                        format!("[{label}]")
                    } else {
                        format!("[{label}] {}", f.file)
                    };
                }
                findings.extend(foreign);
                linted += 1;
            }
            Err(reason) => findings.push(foreign_note(root.to_string_lossy().to_string(), reason)),
        }
    }
    remote.sort();
    remote.dedup();
    for url in remote {
        findings.push(foreign_note(
            url.clone(),
            format!("{url}: not linted (remote)"),
        ));
    }
    missing.sort();
    missing.dedup();
    for path in missing {
        findings.push(foreign_note(
            path.clone(),
            format!("{path}: related path does not exist"),
        ));
    }
}

/// Load, open, and lint one foreign repository rooted at `root`.
fn lint_foreign_repo(root: &Path) -> std::result::Result<(String, Vec<TixIssueFinding>), String> {
    let cfg_path = root.join("tix.toml");
    let mut cfg = Config::load_with_warning(&cfg_path, |_| {})
        .map_err(|e| format!("{}: not linted ({e})", root.display()))?;
    if cfg.issues_dir.is_relative() {
        cfg.issues_dir = root.join(&cfg.issues_dir);
    }
    if !cfg.issues_dir.is_dir() {
        return Err(format!(
            "{}: not linted (issues dir {} is not a directory)",
            root.display(),
            cfg.issues_dir.display()
        ));
    }
    let store = Store::open(&cfg).map_err(|e| format!("{}: not linted ({e})", root.display()))?;
    // `collect_local`, not `collect`: one hop, never re-entering the foreign
    // repo's own foreign repositories.
    let foreign =
        collect_local(&cfg, &store).map_err(|e| format!("{}: not linted ({e})", root.display()))?;
    let label = root
        .file_name()
        .and_then(|n| n.to_str())
        .unwrap_or("foreign")
        .to_string();
    Ok((label, foreign))
}

/// A non-fatal, file-level note about a foreign repository that could not be
/// linted (or was skipped as remote).
fn foreign_note(value: String, message: String) -> TixIssueFinding {
    TixIssueFinding {
        severity: Severity::Warning,
        file: String::new(),
        line: 0,
        column: 0,
        value,
        kind: "foreign_repo_not_linted".to_string(),
        message,
    }
}

/// The configuration findings (`direct_edit_policy_violation`'s sibling family),
/// from the service — WP06 T026.
///
/// **Mostly unreachable, and that is not a gap in this function.** `validate_config`
/// reports seven kinds, and three of them — `missing_completion_category`,
/// `unknown_category`, `mapping_for_unknown_column` — are refused at **config load**.
/// `tix lint` cannot run on a config that will not load, so the only finding that can
/// actually reach here is `unmapped_column_default`, plus `direct_edit_policy_violation`
/// once an issue exists to contradict. T029's configuration scenarios say otherwise,
/// and they are unreachable for the same reason WP05's `workflow show` test was: the
/// command refuses first. Recorded rather than worked around with a test that asserts
/// a success the tool never reaches.
///
/// `unmapped_column_default` is a [`Severity::Warning`]: it is **not** an error and
/// does not set the exit code. A column relying on the documented safe default is a
/// perfectly valid configuration — it is what every repository in the world has — and
/// failing a build over it would be reporting a non-problem as a broken one. The
/// decision is recorded here because it is the kind of thing that gets silently
/// changed by a later "make lint stricter" edit.
///
/// The finding is reported against the config, not a file: `file` is empty, which the
/// renderer already handles as a file-level finding.
fn collect_workflow_config(config: &Config, findings: &mut Vec<TixIssueFinding>) {
    // WP06, revised after the change's blast radius was measured: the safe-default
    // finding is aggregated to **one per board**, naming every column.
    //
    // T026 step 3 says "surface `unmapped_column_default` for every column relying on
    // the safe default", and the service does return one diagnostic per column — that
    // is the right granularity for a diagnostic, and `tix workflow show` prints them
    // per column in its table. But lint is a *summary* surface, and no repository in
    // the world declares a `category_map`, so per-column reporting means every
    // `tix lint` in existence goes from silent to three or four warnings for a
    // perfectly valid configuration. A linter that always has something to say is a
    // linter whose output stops being read.
    //
    // Aggregating keeps the contract's substance — every column relying on the
    // default is named, in the message — and keeps the service's granularity intact
    // for the surfaces that render per column.
    let mut by_board: BTreeMap<String, Vec<String>> = BTreeMap::new();
    for diagnostic in validate_config(config) {
        match diagnostic.kind {
            DiagnosticKind::UnmappedColumnDefault => {
                // `subject` is `{board}.{column}`; split on the last dot so a board
                // name containing a dot still works.
                let (board, column) = match diagnostic.subject.rsplit_once('.') {
                    Some((board, column)) => (board.to_string(), column.to_string()),
                    None => (diagnostic.subject.clone(), String::new()),
                };
                by_board.entry(board).or_default().push(column);
            }
            // Every other kind is load-fatal and cannot arrive; if one ever does it is
            // a real problem with the config and must be loud.
            _ => findings.push(TixIssueFinding {
                severity: Severity::Error,
                file: String::new(),
                line: 0,
                column: 0,
                value: diagnostic.subject.clone(),
                kind: diagnostic.kind.as_str().to_string(),
                message: diagnostic.message,
            }),
        }
    }
    for (board, columns) in by_board {
        let count = columns.len();
        findings.push(TixIssueFinding {
            severity: Severity::Warning,
            file: String::new(),
            line: 0,
            column: 0,
            value: board.clone(),
            kind: DiagnosticKind::UnmappedColumnDefault.as_str().to_string(),
            message: format!(
                "{count} of {count} column(s) on board `{board}` have no `category_map` \
                 entry and resolve to the safe default `{}`: {}. This is valid \
                 configuration — it is what every repository has — and it does not \
                 affect the exit code. To map them, add a \
                 [kanban.{board}.category_map] table.",
                config.categories.default,
                columns.join(", ")
            ),
        });
    }
}

/// `direct_edit_policy_violation`: files whose `status` contradicts their History —
/// WP06 T027.
///
/// **This function writes nothing, and neither does anything it calls.** It does not
/// repair the file, does not append a History entry, does not rewrite existing
/// History, and offers no "fix" affordance (C-005, decision
/// `01M3C3XBMTR5WFV7059NXM04AJ`). A hand-edited file is left exactly as the user left
/// it and stays readable — lint *reports*, and the contradiction is theirs to resolve.
///
/// That note is here, next to the code, because "helpfully" repairing a contradiction
/// is the single most likely way for a future contributor to turn this into a scope
/// violation: a lint that fixes user data is no longer observational, and the user
/// loses the ability to see what they changed.
///
/// One pass over the parsed set, and the service reports each file **once** — a file
/// that both diverged and whose decision now contradicts policy is one finding, not
/// two.
fn collect_workflow_direct_edits(
    config: &Config,
    parsed: &[ParsedFile],
    findings: &mut Vec<TixIssueFinding>,
) {
    if parsed.is_empty() {
        return;
    }
    let (_, resolution) = crate::workflow::active_resolution(config);
    let issues: Vec<Issue> = parsed.iter().map(|p| p.issue.clone()).collect();
    for finding in crate::workflow::lint_direct_edits(&issues, &resolution, config) {
        let Some(entry) = parsed.iter().find(|p| p.issue.number == finding.number) else {
            continue;
        };
        // The `kind` is the contract's own `as_str()` (contract §6). Both columns go
        // into `value` so a diagnostic names the disagreement rather than saying
        // "something changed", and the `status:` line is the span so the rendered
        // diagnostic points at the value in question.
        let value = match &finding.recorded {
            Some(recorded) => format!("{recorded} -> {}", finding.current),
            None => finding.current.clone(),
        };
        findings.push(finding_at(
            &entry.rel,
            &entry.text,
            finding.kind.as_str(),
            "status",
            &value,
            finding.message,
        ));
    }
}

/// The hierarchy check family (WP05 T022), as **one** pass over the snapshot.
///
/// `unresolved_parent` only, and nothing else: the other hierarchy rules —
/// `self_parent`, `cycle` — are **write-time** rules the service refuses a
/// mutation on (contract §3), so a file that contains one was either hand-edited or
/// written by a build that predates the field. They are still worth reporting, and
/// `hierarchy::validate_hierarchy` produces all three kinds; taking the whole result
/// rather than filtering keeps this function from becoming a place where a rule is
/// silently dropped.
///
/// Observational only. Nothing here writes, repairs, or clears: lint reports, and
/// `tix edit --parent` repairs (contract §5).
fn collect_hierarchy(parsed: &[ParsedFile], findings: &mut Vec<TixIssueFinding>) {
    let by_number: BTreeMap<u32, &ParsedFile> =
        parsed.iter().map(|p| (p.issue.number, p)).collect();
    let snapshot: Vec<Issue> = parsed.iter().map(|p| p.issue.clone()).collect();
    let result = crate::hierarchy::validate_hierarchy(&snapshot);
    for diagnostic in &result.errors {
        // The finding's `kind` is the diagnostic's own `as_str()`. Inventing a
        // different spelling would be a contract violation, exactly as it is for the
        // relationship findings above.
        let kind = diagnostic.kind.as_str();
        // A diagnostic about an issue that is not in the readable set cannot be
        // attributed to a file, and is skipped: a file that failed to parse is
        // already reported as `parse`/`missing`, and re-reporting it here would
        // double-count one problem as two.
        if let Some(entry) = by_number.get(&diagnostic.issue) {
            findings.push(finding_at(
                &entry.rel,
                &entry.text,
                kind,
                &diagnostic.field,
                // The offending value is the recorded reference, read from the
                // issue's own field. The first version passed `""`, and the
                // renderer printed `invalid unresolved_parent value ''` — a
                // diagnostic saying the value is empty when the file plainly
                // contains `parent: 0`. Read from the field rather than parsed out
                // of the message, so the two cannot disagree.
                &entry
                    .issue
                    .parent
                    .map(|p| p.to_string())
                    .unwrap_or_default(),
                diagnostic.message.clone(),
            ));
        }
    }
}

/// The project-reference check family (WP09 item A), as **one** pass over the
/// snapshot.
///
/// [`crate::projects::validate_references`] performs the pass — one walk of the
/// issue set, not one lookup per project — and returns one entry per association
/// whose project the registry does not hold, ordered by `(issue number, project
/// id)` so the output is stable. Lint reports each as the
/// [`crate::projects::MISSING_REFERENCE_KIND`] the service names, carrying the
/// service's own reason string: a second wording here would let `tix lint` and the
/// `tix project delete` notice describe one dangling reference two ways (C-005).
///
/// Observational, like every other lint family: nothing is written, repaired, or
/// cleared. A reference whose project is still declared is not a finding, and a
/// clean store produces none.
fn collect_project_references(
    config: &Config,
    parsed: &[ParsedFile],
    findings: &mut Vec<TixIssueFinding>,
) {
    if parsed.is_empty() {
        return;
    }
    let snapshot: Vec<Issue> = parsed.iter().map(|p| p.issue.clone()).collect();
    let by_number: BTreeMap<u32, &ParsedFile> =
        parsed.iter().map(|p| (p.issue.number, p)).collect();
    for missing in crate::projects::validate_references(config, &snapshot) {
        // A diagnostic about an issue absent from the readable set cannot be
        // attributed to a file; a file that failed to parse is already reported as
        // `parse`/`missing`, and re-reporting it here would double-count one problem
        // as two.
        let Some(entry) = by_number.get(&missing.issue.number()) else {
            continue;
        };
        findings.push(finding_at(
            &entry.rel,
            &entry.text,
            crate::projects::MISSING_REFERENCE_KIND,
            // The `projects:` key line is where the association lives; the offending
            // value is the recorded id, carried so the annotation names it.
            crate::projects::PROJECTS_KEY,
            &missing.project_id,
            missing.reason.clone(),
        ));
    }
}

/// The epic-reference check family (WP09 item A), as **one** pass over the
/// snapshot.
///
/// [`crate::epics::resolve_references`] is the single owner: it returns one entry
/// per reference that does not resolve, covering all five kinds the service
/// diagnoses — an issue-held `epics` entry that names no live epic or is
/// malformed, and an epic record's own parent, project, milestone and cycle
/// references. Lint reports each as the kind's own
/// [`crate::epics::EpicReferenceKind::diagnostic_kind`], carrying the service's
/// reason verbatim: a second wording here would let `tix lint` and the epic
/// surfaces describe one dangling reference two ways (C-005).
///
/// Two shapes, because the holder decides where the reference lives. An
/// **issue-held** reference points at the issue's own `epics:` block, so the
/// finding is located there. An **epic-held** reference lives in `tix.toml`, not
/// in any issue file, so the finding is a file-level one (`file` empty) whose note
/// names the epic and the reference — the same shape the configuration findings
/// use.
///
/// Observational, like every other lint family: nothing is written, repaired, or
/// cleared. The epic-held half needs the planner's project/milestone/cycle
/// lookups; if the milestone registry cannot be read the per-file milestone check
/// already reports it, so this family is skipped rather than emitting false
/// positives.
fn collect_epic_references(
    config: &Config,
    planner: &Planner,
    parsed: &[ParsedFile],
    findings: &mut Vec<TixIssueFinding>,
) {
    let Ok(targets) = crate::epics::ReferenceTargets::from_planner(planner) else {
        return;
    };
    let issues: Vec<Issue> = parsed.iter().map(|p| p.issue.clone()).collect();
    let by_number: BTreeMap<u32, &ParsedFile> =
        parsed.iter().map(|p| (p.issue.number, p)).collect();
    for missing in crate::epics::resolve_references(&config.epics, &issues, &targets) {
        let kind = missing.target_kind.diagnostic_kind();
        match &missing.holder {
            crate::epics::ReferenceHolder::Issue(key) => {
                // A diagnostic about an issue absent from the readable set cannot be
                // attributed to a file; a file that failed to parse is already
                // reported as `parse`/`missing`, and re-reporting it here would
                // double-count one problem as two.
                let Some(entry) = by_number.get(&key.number()) else {
                    continue;
                };
                findings.push(finding_at(
                    &entry.rel,
                    &entry.text,
                    kind,
                    crate::epics::EPICS_KEY,
                    &missing.target,
                    missing.reason.clone(),
                ));
            }
            crate::epics::ReferenceHolder::Epic(_) => findings.push(TixIssueFinding {
                severity: Severity::Error,
                file: String::new(),
                line: 0,
                column: 0,
                value: missing.target.clone(),
                kind: kind.to_string(),
                message: missing.reason.clone(),
            }),
        }
    }
}

/// The relationship check family (WP06 T025/T026), as **one** pass over the
/// snapshot.
///
/// It calls the service's `validate_snapshot` and converts each diagnostic. It
/// does not re-derive any graph state locally — no adjacency walk, no cycle
/// search, no per-issue rebuild. That is a parity requirement
/// (`contracts/surface-parity.md` §1) and a cost one: `validate_snapshot` is a
/// single `O(V + E)` pass, whereas rebuilding the graph per issue would make lint
/// quadratic on a large tree, and a linter that takes minutes on a big repo gets
/// stopped rather than fixed.
///
/// The per-issue pass above is left alone. This sweep is whole-snapshot by
/// nature — a cycle has no owning file — so there is no per-file form to add and
/// no way to express it inside `collect_for_file`.
fn collect_relationships(
    parsed: &[ParsedFile],
    store: &Store,
    findings: &mut Vec<TixIssueFinding>,
) {
    let done_column = store.relationship_done_column();
    // The graph reports issue *numbers*; a finding needs a *file*. Built once,
    // from the same walk, so the two cannot disagree about what exists.
    let by_number: BTreeMap<u32, &ParsedFile> =
        parsed.iter().map(|p| (p.issue.number, p)).collect();

    let views: Vec<IssueView<'_>> = parsed.iter().map(|p| p.issue.issue_view()).collect();
    let graph = RelationshipGraph::new(&views, &done_column);
    let result = graph.validate_snapshot();

    for diagnostic in &result.errors {
        // The finding's `kind` is the diagnostic's own `as_str()` — the seven
        // names `relationship-service.md` §3 fixes. Inventing a different
        // spelling here would be a contract violation.
        let kind = diagnostic.kind.as_str();
        match diagnostic.issue.and_then(|n| by_number.get(&n)) {
            Some(entry) => findings.push(finding_at(
                &entry.rel,
                &entry.text,
                kind,
                &diagnostic.field,
                &diagnostic.field,
                diagnostic.message.clone(),
            )),
            // A diagnostic with no single subject. It still has to surface, so it
            // is reported against the store root rather than dropped: a linter
            // that silently discards a contract diagnostic is worse than one that
            // reports it in an awkward place.
            None => findings.push(TixIssueFinding {
                severity: Severity::Error,
                file: String::new(),
                line: 0,
                column: 0,
                value: diagnostic.field.clone(),
                kind: kind.to_string(),
                message: diagnostic.message.clone(),
            }),
        }
    }
}

/// Per-issue `related_issues` checks: a blank entry, or a local selector that
/// resolves to nothing.
///
/// Only `Local` entries are resolved — a path or URL may be outside the store or
/// need the network, so lint accepts it verbatim and stays offline. Warnings, not
/// errors: an unresolved reference is a dangling pointer to be repaired, not a
/// broken build.
fn collect_related_issues(
    parsed: &[ParsedFile],
    store: &Store,
    findings: &mut Vec<TixIssueFinding>,
) {
    for entry in parsed {
        for raw in &entry.issue.related_issues {
            let kind = if raw.trim().is_empty() {
                Some("related_issue_empty")
            } else {
                let r = crate::related::classify(raw);
                match r.local_selector() {
                    Some(_) if crate::related::resolve_local(store, raw).is_err() => {
                        Some("related_issue_unresolved")
                    }
                    _ => None,
                }
            };
            let Some(kind) = kind else { continue };
            let (line, column) =
                locate_value(&entry.text, "related_issues").map_or((0, 0), |l| (l.line, l.column));
            let message = match kind {
                "related_issue_empty" => {
                    "empty related-issue entry; remove it or name an issue".to_string()
                }
                _ => format!(
                    "related issue `{raw}` matches no issue in this store; it may be a \
                     path or URL (which lint does not resolve) or a typo"
                ),
            };
            findings.push(TixIssueFinding {
                severity: Severity::Warning,
                file: entry.rel.clone(),
                line,
                column,
                value: raw.clone(),
                kind: kind.to_string(),
                message,
            });
        }
    }
}

/// Warn about an issue whose current path does not match an explicitly
/// configured `[layout]`.
///
/// Gated on a **non-default** layout: with no `[layout]` section the default
/// template is not "expected" of a pre-existing store, and warning on every
/// legacy file would be noise, not signal. Once a project opts in, the warning
/// is the discovery path for files that still need `tix layout migrate`.
fn collect_layout_mismatch(
    config: &Config,
    parsed: &[ParsedFile],
    findings: &mut Vec<TixIssueFinding>,
) {
    // Gate on the *templates*, not on `LayoutConfig::is_default()`: a repo that
    // only turned `move_on_change` off still uses the default layout, and every
    // pre-existing file matching it must not be flagged.
    let default = crate::config::LayoutConfig::default();
    if config.layout.directory == default.directory && config.layout.filename == default.filename {
        return;
    }
    for entry in parsed {
        // Compare the *directory*, not the filename: the filename template
        // governs newly created issues, and a hand-kept name is not a mismatch.
        let expected = crate::layout::render_directory(&config.layout, &entry.issue);
        let current = entry
            .issue
            .filename
            .rsplit_once('/')
            .map_or("", |(dir, _)| dir);
        if current != expected {
            findings.push(TixIssueFinding {
                severity: Severity::Warning,
                file: entry.rel.clone(),
                line: 0,
                column: 0,
                value: entry.issue.filename.clone(),
                kind: "layout_mismatch".to_string(),
                message: format!(
                    "directory does not match `[layout]`: expected `{expected}`; run `tix layout migrate`"
                ),
            });
        }
    }
}

/// Lint a single issue file by relative path.
pub fn collect_file(config: &Config, store: &Store, rel: &str) -> Result<Vec<TixIssueFinding>> {
    let planner = planner_for(config);
    let milestone_names = planner.milestone_names().unwrap_or_default();
    let mut findings = Vec::new();
    collect_for_file(
        config,
        store,
        &planner,
        &milestone_names,
        rel,
        &mut findings,
    )?;
    // Single-file lint is the same command with a narrower scope, so it reports
    // the same findings for that file and no relationship findings — those are a
    // property of the whole snapshot, and a caller asking about one file is not
    // asking about the snapshot. Stated rather than implied, because the obvious
    // alternative (graph just this file) would report a cycle as if it were the
    // only cycle in the repo, which is a different and wrong claim.
    Ok(findings)
}

/// Every issue file in the store, mirroring the storage layout exactly:
/// `.md` files at the flat root plus one level of segment dirs, skipping
/// `.tix-tmp-*` marker dirs.
fn issue_files(store: &Store) -> Result<Vec<String>> {
    let mut files: Vec<String> = Vec::new();
    collect_issue_files(&store.issues_dir, "", &mut files)?;
    files.sort();
    Ok(files)
}

/// Recursive walk mirroring `Store::scan`: any depth, skipping hidden dirs,
/// `.tix-tmp-*` temp dirs, and `<slug>-assets/` dirs.
fn collect_issue_files(dir: &Path, prefix: &str, files: &mut Vec<String>) -> Result<()> {
    if !dir.exists() {
        return Ok(());
    }
    for entry in fs::read_dir(dir)? {
        let entry = entry?;
        let name = entry.file_name().to_string_lossy().to_string();
        let path = entry.path();
        if path.is_dir() {
            if name.starts_with('.') || name.starts_with(".tix-tmp-") || name.ends_with("-assets") {
                continue;
            }
            let child = if prefix.is_empty() {
                name
            } else {
                format!("{prefix}/{name}")
            };
            collect_issue_files(&path, &child, files)?;
        } else if path.is_file() && name.ends_with(".md") {
            files.push(if prefix.is_empty() {
                name
            } else {
                format!("{prefix}/{name}")
            });
        }
    }
    Ok(())
}

/// Collect every finding for one issue file: a `missing`/`parse` diagnostic
/// when the file cannot be parsed, otherwise value validation (FR-016).
/// The outcome of one file's pass: the issue it parsed, if it parsed at all.
///
/// The relationship sweep graphs exactly the issues this walk produced, rather
/// than re-reading the tree through `Store::list`. `list` aborts on the first
/// malformed file — which is exactly why lint walks the store itself (FR-018) —
/// so building the graph that way made one broken file kill the whole command
/// (exit 2, and every relationship problem in the tree went unreported).
struct ParsedFile {
    rel: String,
    text: String,
    issue: Issue,
}

fn collect_for_file(
    config: &Config,
    store: &Store,
    planner: &Planner,
    milestone_names: &[String],
    rel: &str,
    findings: &mut Vec<TixIssueFinding>,
) -> Result<Option<ParsedFile>> {
    let text = match fs::read_to_string(store.issues_dir.join(rel)) {
        Ok(text) => text,
        Err(err) if err.kind() == std::io::ErrorKind::InvalidData => {
            // Non-UTF-8 bytes: a clean diagnostic, keep scanning (FR-018).
            findings.push(TixIssueFinding {
                severity: Severity::Error,
                file: rel.to_string(),
                line: 0,
                column: 0,
                value: String::new(),
                kind: "parse".to_string(),
                message: "file is not valid UTF-8".to_string(),
            });
            return Ok(None);
        }
        Err(err) => return Err(TixError::Io(err)),
    };
    let issue = match Issue::parse(&text) {
        Ok(issue) => issue,
        Err(TixError::Parse(msg)) => {
            findings.push(TixIssueFinding {
                severity: Severity::Error,
                file: rel.to_string(),
                line: 0,
                column: 0,
                value: String::new(),
                kind: if text.starts_with("---\n") {
                    "parse".to_string()
                } else {
                    "missing".to_string()
                },
                message: msg,
            });
            return Ok(None);
        }
        Err(other) => return Err(other),
    };
    validate_issue(
        config,
        planner,
        milestone_names,
        rel,
        &text,
        &issue,
        findings,
    );
    unknown_frontmatter_keys(config, rel, &text, &issue, findings);
    Ok(Some(ParsedFile {
        rel: rel.to_string(),
        text,
        issue,
    }))
}

/// Warn about top-level frontmatter keys this model does not know (#1361).
///
/// A **warning**, not an error, and the difference is the whole design: the key is
/// preserved on write, so the file is valid and `tix lint` exiting 0 is honest. The
/// warning exists because a key `tix` does not recognise is very often a **typo** —
/// `titel:` renders, round-trips, and is never read by anything. Before this, such a
/// key was not merely silent, it was deleted on the next write, so a typo and a
/// deliberate custom key were indistinguishable.
///
/// Suppressed by `[custom_attributes]` in `tix.toml`: a project that genuinely uses
/// a key declares it once and stops being told about it. That is the escape hatch,
/// and it is why the finding is a warning rather than an error — the fix is always
/// available and is never urgent.
///
/// The retired `links:` key is **not** reported here. It is recognised, reported by
/// its own `retired_links_key` finding, and dropped (C-001); warning about it twice
/// would be two findings for one problem.
fn unknown_frontmatter_keys(
    config: &Config,
    rel: &str,
    text: &str,
    issue: &Issue,
    findings: &mut Vec<TixIssueFinding>,
) {
    let declared = config.custom_attributes();
    for key in issue.extra_frontmatter.keys() {
        if declared.iter().any(|d| d == key) {
            continue;
        }
        findings.push(TixIssueFinding {
            severity: Severity::Warning,
            file: rel.to_string(),
            line: locate_key_line(text, key),
            column: 1,
            value: key.clone(),
            kind: "unknown_key".to_string(),
            message: format!(
                "unknown top-level frontmatter key `{key}`: it is preserved on write but \
                 nothing reads it. If that is deliberate, declare it in \
                 `[custom_attributes]` in tix.toml; if not, it is probably a typo."
            ),
        });
    }
}

/// The 1-based line of `key:` in the file's frontmatter, or 0 when it cannot be
/// found. `0` renders as a file-level finding, which is the honest answer when the
/// key is present in the model but not locatable in the text.
fn locate_key_line(text: &str, key: &str) -> usize {
    let frontmatter = match text.strip_prefix("---\n") {
        Some(rest) => rest,
        None => return 0,
    };
    for (offset, line) in frontmatter.lines().enumerate() {
        if line == key
            || line.starts_with(&format!("{key}:"))
            || line.starts_with(&format!("\"{key}\":"))
        {
            return offset + 2; // +1 for the opening `---`, +1 for 1-based lines.
        }
        if line == "---" {
            break;
        }
    }
    0
}

/// Value validation over the parsed issue.
///
/// Used to say "unknown keys are never flagged", which was the charter's stated
/// policy and is now **reversed** by #1361: unknown keys warn. See
/// [`unknown_frontmatter_keys`].
/// Planning fields (WP03) add three reference checks that all route through
/// the WP01 registry and WP02 date-pair validator — no copied rules:
///   - every `cycles` name must be a configured `[cycles.*]` entry,
///   - every `milestones` name must be a registry file under `milestones_dir`,
///   - `start_date`/`due_date` must parse and satisfy `due_date >= start_date`.
fn validate_issue(
    config: &Config,
    planner: &Planner,
    milestone_names: &[String],
    rel: &str,
    text: &str,
    issue: &Issue,
    findings: &mut Vec<TixIssueFinding>,
) {
    let status = issue.status.display();
    let columns = &config.default_config().columns;
    if !columns.iter().any(|c| c == status) {
        findings.push(value_finding(
            rel,
            text,
            "status",
            status,
            format!("valid options: {}", columns.join(", ")),
        ));
    }
    if issue.title.trim().is_empty() {
        findings.push(value_finding(
            rel,
            text,
            "title",
            &issue.title,
            "title is required".to_string(),
        ));
    }
    // An unset priority — `priority: ''` — is a legitimate state, not an unknown
    // value: the author said "no priority" rather than failing to choose one. It
    // is exempted, and exempted *only* as the empty string, so a genuine typo
    // (`priority: 'hih'`, `priority: 'urgent'`) is still reported. Silently
    // coercing it to the default `medium` instead would lose the author's
    // statement, which is the one thing the typed model is not supposed to do.
    if let Priority::Other(unknown) = &issue.priority
        && !issue.priority.is_undefined()
    {
        findings.push(value_finding(
            rel,
            text,
            "priority",
            unknown,
            "valid options: '' (unset), low, medium, high".to_string(),
        ));
    }
    // The issue type, always checked against the configured vocabulary. An
    // unknown value is a **warning** with the remedy in the message: the reader
    // can accept it by adding it to `custom_types` in `tix.toml`.
    if let Some(t) = issue
        .type_
        .as_deref()
        .map(str::trim)
        .filter(|t| !t.is_empty())
    {
        let allowed = config.custom_types();
        if !allowed.iter().any(|a| a.eq_ignore_ascii_case(t)) {
            let located = locate_value(text, "type");
            findings.push(TixIssueFinding {
                severity: Severity::Warning,
                file: rel.to_string(),
                line: located.as_ref().map_or(0, |l| l.line),
                column: located.as_ref().map_or(0, |l| l.column),
                value: t.to_string(),
                kind: "unknown_type".to_string(),
                message: format!(
                    "unknown issue type `{t}`; valid types: {}. Add it to \
                     `types` in tix.toml to accept it.",
                    allowed.join(", ")
                ),
            });
        }
    }
    if !is_iso_date(&issue.created) {
        findings.push(value_finding(
            rel,
            text,
            "created",
            &issue.created,
            "expected a date in `YYYY-MM-DD` format".to_string(),
        ));
    }
    if !is_iso_date(&issue.updated) {
        findings.push(value_finding(
            rel,
            text,
            "updated",
            &issue.updated,
            "expected a date in `YYYY-MM-DD` format".to_string(),
        ));
    }
    if let Some(estimate) = &issue.estimate
        && estimate.parse::<f64>().is_err()
    {
        findings.push(value_finding(
            rel,
            text,
            "estimate",
            estimate,
            "estimate must be numeric".to_string(),
        ));
    }
    // The retired `links:` key is a validation result, not a parse fallback:
    // the field is reported and its value is never read as typed data (C-001).
    // A hand-edited `## History` line is a per-issue diagnostic, never a
    // parse failure: the line is preserved verbatim and the rest of the file
    // stays readable. Reported once per offending line so the author can see
    // exactly which lines the tool could not read.
    // A `## History` section that is **not at the front of the body**.
    //
    // `data-model.md` §8 pins the section to the front: "Only a section at the
    // *front* of the body is recognised — the position pinned above — so a
    // `## History` heading appearing later in an issue's prose is ordinary body
    // text." The parser implements that correctly, which is exactly why the
    // violation is dangerous: the section and its events are not rejected, they
    // are reclassified as body prose.
    //
    // The two consequences are both silent and both bad. `tix view` shows the
    // heading as text, so the issue appears to have no audit trail at all; and
    // `tix edit --body` replaces the body wholesale, which destroys the section
    // and every event in it. A plain write preserves it, so the loss arrives
    // later and looks unrelated to whatever hand-edit caused it.
    //
    // Reported once per occurrence rather than once per issue, because a file
    // can carry a misplaced section *and* a real one, and the difference matters.
    //
    // Fenced code blocks are skipped. A ``` console sample that prints a
    // `## History` heading, or a ``` markdown example of the format, is
    // **literal text**, not a section — and reporting it flagged every issue
    // that documents the History format, including this repository's own
    // mission findings. A heading inside a fence is not a misplaced section,
    // so the scan has to know where the fences are.
    //
    // The finding also carries the **absolute** line of the offending heading,
    // derived from the body's offset in the file (`body_start_line`). The old
    // form went through `value_finding`, which locates a *frontmatter key* —
    // and `## History` is not one, so the diagnostic always landed at 1:1 and
    // underlined the opening `---` instead of the heading it was about.
    let fenced = fenced_lines(&issue.body);
    let body_line = body_start_line(text, &issue.body);
    for (index, line) in issue.body.lines().enumerate() {
        if fenced.get(index).copied().unwrap_or(false) {
            continue;
        }
        if crate::model::is_history_heading(line) {
            findings.push(finding_at_line(
                rel,
                body_line + index,
                1,
                "history",
                line.trim(),
                "a `## History` section must be the first thing in the body; \
                 this one is treated as body text, so its entries are not in the \
                 audit trail and the next `tix edit --body` deletes them"
                    .to_string(),
            ));
        }
    }
    // A body line shaped like a History entry but not recognised as one. The
    // section has no end-of-section delimiter, so an entry hand-typed with ASCII
    // hyphens instead of U+2014 cannot be told apart from prose by shape alone
    // and is left in the body rather than absorbed. Reporting it means the
    // author finds out, instead of the entry silently disappearing from the
    // audit trail. Fenced lines are skipped here too, for the same reason.
    for (index, line) in issue.body.lines().enumerate() {
        if fenced.get(index).copied().unwrap_or(false) {
            continue;
        }
        if crate::model::looks_like_an_unrecognised_entry(line) {
            findings.push(finding_at_line(
                rel,
                body_line + index,
                1,
                "history",
                line.trim(),
                "looks like a `## History` entry but was not recognised \u{2014} \
                 entries need U+2014 em-dash separators, not ASCII hyphens"
                    .to_string(),
            ));
        }
    }
    for line in issue.history.unparsed() {
        let finding = value_finding(
            rel,
            text,
            "history",
            line,
            "unparseable `## History` entry \u{2014} kept verbatim, never repaired; \
             expected `- <timestamp> \u{2014} <actor> \u{2014} <kind> \u{2014} <detail>` \
             with U+2014 em-dash separators"
                .to_string(),
        );
        findings.push(finding);
    }
    if let Some(cycles) = &issue.cycles {
        for name in cycles {
            if planner.cycle(name).is_none() {
                let known = planner.cycle_names();
                let list = if known.is_empty() {
                    "none configured".to_string()
                } else {
                    known.join(", ")
                };
                findings.push(value_finding(
                    rel,
                    text,
                    "cycles",
                    name,
                    format!("unknown cycle — valid cycles: {list}"),
                ));
            }
        }
    }
    if let Some(milestones) = &issue.milestones {
        for name in milestones {
            if planner.milestone(name).is_err() || !milestone_names.iter().any(|m| m == name) {
                let list = if milestone_names.is_empty() {
                    "none configured".to_string()
                } else {
                    milestone_names.join(", ")
                };
                findings.push(value_finding(
                    rel,
                    text,
                    "milestones",
                    name,
                    format!("unknown milestone — valid milestones: {list}"),
                ));
            }
        }
    }
    // The date-pair rule surfaces as one finding named after the offending
    // member; the message reuses the WP02 validator verbatim.
    if let Err(TixError::Config(msg)) =
        crate::model::validate_issue_dates(issue.start_date.as_deref(), issue.due_date.as_deref())
    {
        let (kind, value) = if msg.starts_with("start_date") {
            ("start_date", issue.start_date.as_deref().unwrap_or(""))
        } else {
            ("due_date", issue.due_date.as_deref().unwrap_or(""))
        };
        findings.push(value_finding(rel, text, kind, value, msg));
    }
}

/// A value finding, located by scanning the raw frontmatter (serde_yaml
/// loses spans, so the line/column come from the source text itself).
fn value_finding(
    rel: &str,
    text: &str,
    kind: &str,
    value: &str,
    message: String,
) -> TixIssueFinding {
    finding_at(rel, text, kind, kind, value, message)
}

/// As [`value_finding`], but the finding's `kind` and the frontmatter key used to
/// locate the value are named separately.
///
/// They are the same string for every per-file check, which is why one parameter
/// used to do both jobs. They are **not** the same for a relationship diagnostic:
/// the finding's kind is the contract's name (`retired_links_key`,
/// `conflicting_edge`, …) while the key to look up is whatever the diagnostic
/// names (`links`, `kind`, `to`). Passing the contract name to `locate_value`
/// looks for a frontmatter key that does not exist, so every relationship finding
/// came out at line 0, column 0 — a file-level finding that points at the first
/// line of the file rather than at the offending value.
fn finding_at(
    rel: &str,
    text: &str,
    kind: &str,
    locate_key: &str,
    value: &str,
    message: String,
) -> TixIssueFinding {
    let (line, column) = locate_value(text, locate_key)
        .map(|l| (l.line, l.column))
        .unwrap_or((0, 0));
    TixIssueFinding {
        severity: Severity::Error,
        file: rel.to_string(),
        line,
        column,
        value: value.to_string(),
        kind: kind.to_string(),
        message,
    }
}

/// A finding located by an **absolute** line/column in the file.
///
/// For a body-level subject there is no frontmatter key to search for, so
/// [`finding_at`] cannot place it: `## History` is a heading, not a key, and a
/// search for it misses and falls back to line 0 — a diagnostic that underlines
/// the opening `---` while talking about a heading fifty lines down. The caller
/// has already computed the line (`body_start_line` + the body offset), so the
/// value is carried straight through.
fn finding_at_line(
    rel: &str,
    line: usize,
    column: usize,
    kind: &str,
    value: &str,
    message: String,
) -> TixIssueFinding {
    TixIssueFinding {
        severity: Severity::Error,
        file: rel.to_string(),
        line,
        column,
        value: value.to_string(),
        kind: kind.to_string(),
        message,
    }
}

/// Mark each line of `body` as inside (`true`) or outside (`false`) a fenced
/// code block.
///
/// A fence opens on a line whose first non-space run is three or more backticks
/// or tildes (```` ``` ````, `~~~`) and closes on a matching run of the same
/// character at least as long, with nothing after it. Only fenced *content*
/// matters here; the fence lines themselves are marked outside, which is
/// harmless because a `## History` heading never looks like a fence.
///
/// This exists so the History scan can tell a **section heading** from a
/// **documented example** of one. Without it, an issue that quotes the History
/// format inside a code block — the natural way to describe this very defect —
/// was reported as if its prose carried a misplaced section.
fn fenced_lines(body: &str) -> Vec<bool> {
    let mut flags = Vec::new();
    // The open fence's marker character and the length of its run.
    let mut fence: Option<(char, usize)> = None;
    for line in body.lines() {
        let trimmed = line.trim_start();
        let marker = trimmed.chars().next().filter(|c| *c == '`' || *c == '~');
        let run = marker.map_or(0, |c| trimmed.chars().take_while(|x| *x == c).count());
        flags.push(fence.is_some());
        match fence {
            // A closing fence: same character, at least as long, and the rest of
            // the line blank — the same rule the CommonMark spec uses, so an
            // inline `` ` `` run inside a fence does not close it early.
            Some((c, len)) => {
                if marker == Some(c) && run >= len && trimmed[run..].trim().is_empty() {
                    fence = None;
                }
            }
            None if run >= 3 => fence = marker.map(|c| (c, run)),
            None => {}
        }
    }
    flags
}

/// The 1-based line of `body`'s first line within the whole file `text`.
///
/// `Issue::parse` leaves `body` as a **suffix** of the file — the frontmatter
/// and any leading `## History` section are stripped as a prefix — so the
/// offset is just the length difference, and counting the newlines before it
/// gives the line. A `body` that is somehow not a suffix degrades to line 1
/// rather than panicking (`saturating_sub`).
fn body_start_line(text: &str, body: &str) -> usize {
    let start = text.len().saturating_sub(body.len());
    text[..start].matches('\n').count() + 1
}

/// The 1-based line/column of the raw `key:` value inside the frontmatter
/// block (before the closing `---` delimiter), if present.
fn locate_value(text: &str, key: &str) -> Option<Located> {
    let prefix = format!("{key}:");
    let fm_end = text[4..].find("\n---").map(|i| i + 4).unwrap_or(text.len());
    let fm = &text[..fm_end];
    let mut line_start = 0usize;
    for (idx, line) in fm.split_inclusive('\n').enumerate() {
        let content = line.trim_end_matches(['\n', '\r']);
        if let Some(rest) = content.trim_start().strip_prefix(&prefix) {
            let pad = content.len() - content.trim_start().len();
            let value_skip = rest.len() - rest.trim_start().len();
            let start = line_start + pad + prefix.len() + value_skip;
            return Some(Located {
                line: idx + 1,
                column: start - line_start + 1,
            });
        }
        line_start += line.len();
    }
    None
}

/// Where a frontmatter value sits, in 1-based line/column terms.
///
/// Only the line and column are carried. Byte offsets used to be here as well, and
/// `span_for` read them — but it re-derived them from a search keyed on the
/// finding's *kind*, which is a diagnostic name rather than a frontmatter key, so
/// it never found anything and the offsets were dead in practice. `span_for` now
/// derives the span from the line and column directly.
struct Located {
    line: usize,
    column: usize,
}

/// A strict `YYYY-MM-DD` calendar-date check (shape + month 1..=12, day
/// 1..=31), so `2026-13-99` is flagged as malformed.
fn is_iso_date(s: &str) -> bool {
    let b = s.as_bytes();
    if b.len() != 10 || b[4] != b'-' || b[7] != b'-' {
        return false;
    }
    for &c in &b[0..4] {
        if !c.is_ascii_digit() {
            return false;
        }
    }
    for &c in &[b[5], b[6], b[8], b[9]] {
        if !c.is_ascii_digit() {
            return false;
        }
    }
    let month = u32::from(b[5] - b'0') * 10 + u32::from(b[6] - b'0');
    let day = u32::from(b[8] - b'0') * 10 + u32::from(b[9] - b'0');
    (1..=12).contains(&month) && (1..=31).contains(&day)
}

/// Render one finding as a rustc-style diagnostic (C-004) and return it as a
/// `String`; [`run_with`] prints it to stderr.
///
/// The header names the field and the offending value; the snippet points at the
/// exact line; the annotation label carries the explanation; and a `help:` footer
/// names the remedy for every kind the linter can produce. **Every** finding —
/// error *and* warning — runs through this one renderer, so the severity is
/// expressed by the level, the id prefix, and the colour together, and the output
/// always agrees with the exit code (#1361).
///
/// Returning the string, rather than printing here, keeps the formatting
/// unit-testable without a stderr harness. The `help:` line is what makes the
/// output *actionable*: the fix for most findings is a one-liner the reader
/// cannot infer from the message alone ("valid options: …" says what is wrong,
/// not what to type).
fn render_diagnostic(issues_dir: &Path, f: &TixIssueFinding) -> String {
    let title = match f.kind.as_str() {
        "missing" => "missing frontmatter".to_string(),
        "parse" => "parse error".to_string(),
        // The WIP family (WP07): a column over its limit, or an estimate that cannot
        // be counted. Named rather than run through the generic `invalid X value`
        // form, which reads as a frontmatter problem and is not one.
        "over_limit" => "work-in-progress limit exceeded".to_string(),
        "malformed_estimate" | "missing_conversion" | "ambiguous_conversion" => {
            "estimate cannot be counted".to_string()
        }
        _ => format!("invalid {} value '{}'", f.kind, f.value),
    };
    // A warning must not be **rendered** as an error either (#1361). The level, the
    // id prefix, and the colour all follow the severity, so the output agrees with
    // the exit code.
    let id = match f.severity {
        Severity::Error => "E-tix-lint",
        Severity::Warning => "W-tix-lint",
    };
    let mut diagnostic = match f.severity {
        Severity::Error => Diagnostic::error(id, title),
        Severity::Warning => Diagnostic::warning(id, title),
    };
    let path = issues_dir.join(&f.file);
    match fs::read_to_string(&path) {
        Ok(text) if !f.file.is_empty() => {
            diagnostic = diagnostic
                .at(&path, span_for(f, &text))
                .labelled(&f.message);
        }
        // No source to point at — a configuration finding, or a diagnostic with no
        // single subject, or a file that vanished between the walk and the render.
        // The explanation rides as a `note:` so it is never dropped: folding it into
        // the title (what this branch used to do) lost the sentence for exactly the
        // finding the reader needed it for.
        _ if !f.message.is_empty() => {
            let note = if f.file.is_empty() {
                f.message.clone()
            } else {
                format!("{}: {}", f.file, f.message)
            };
            diagnostic = diagnostic.with_note(note);
        }
        _ => {}
    }
    if let Some(help) = diagnostic_help(&f.kind) {
        diagnostic = diagnostic.with_help(help);
    }
    diagnostic.render_auto()
}

/// The one-line remedy for a finding kind, rendered as a `help:` footer.
///
/// Every kind the linter can emit has an entry here, so no diagnostic ever ends
/// without telling the reader what to do about it. The annotation already says
/// *what* is wrong and, where relevant, *which values are valid*; this says which
/// edit makes it right. `None` is reserved for a kind added without a remedy,
/// which is a bug the totality test (`every_finding_kind_has_a_help_line`) makes
/// visible.
fn diagnostic_help(kind: &str) -> Option<&'static str> {
    Some(match kind {
        // Per-issue frontmatter values.
        "status" => "set `status:` to one of the board's configured columns",
        "priority" => "use `''` (unset), `low`, `medium`, or `high`",
        "title" => "give the issue a non-empty `title:`",
        "unknown_type" => "add the type to `types` in tix.toml, or fix the spelling",
        "created" | "updated" => "use `YYYY-MM-DD`, e.g. `2026-10-06`",
        "estimate" => "use a number, e.g. `estimate: 3` or `estimate: 0.5`",
        "unknown_key" => "declare it in `[custom_attributes]` in tix.toml, or remove it",
        "cycles" => "create it with `tix cycle create`, or fix the name",
        "milestones" => "create it with `tix milestone create`, or fix the name",
        "start_date" | "due_date" => {
            "use `YYYY-MM-DD`, and keep `due_date` on or after `start_date`"
        }
        "history" => {
            "keep one `## History` section as the first block of the body, with U+2014 \
             em-dash separators between the fields"
        }
        // Related-issue references.
        "related_issue_empty" => "remove the empty entry, or name an issue",
        "related_issue_unresolved" => {
            "point at an issue in this store; paths and URLs are accepted verbatim"
        }
        // Whole-file findings.
        "missing" => "add frontmatter delimited by `---` at the top of the file",
        "parse" => "fix the parse error named on the annotation",
        "layout_mismatch" => "run `tix layout migrate`",
        "foreign_repo_not_linted" => "check the path in `related_issues`",
        // Typed relationships.
        "retired_links_key" => "recreate the links with `tix link`, then delete the `links:` key",
        "self_link" => "a relationship cannot point at its own issue",
        "missing_target" => "name an issue that exists",
        "conflicting_edge" => "keep one edge per pair, or use `related` for a plain link",
        "unknown_kind" => "use one of: `blocks`, `duplicates`, `related`",
        "unknown_relation" => "run `tix link list` to see the stored relationship ids",
        // Parent hierarchy.
        "self_parent" => "clear `parent:`, or point it at a different issue",
        "unresolved_parent" | "unknown_parent" => {
            "point `parent:` at an issue that exists, or clear it with `tix edit --parent ''`"
        }
        // Project associations and an epic record's project references (WP09 item A).
        // One kind, two holders: an issue association or an epic's `projects` field.
        "missing_project_reference" => {
            "point the association at a live project, or remove the dangling id with \
             `tix project remove <id> <issue>` / `tix epic edit`"
        }
        // The epic reference family (WP09 item A). One kind per failing reference
        // kind the service diagnoses.
        "missing_epic_reference" => {
            "recreate the epic, drop the reference with `tix epic remove <id> <sel>`, \
             or clear the parent with `tix epic edit <id> --parent ''`"
        }
        "missing_issue_reference" => {
            "remove the malformed entry from `epics:`, or replace it with an epic id"
        }
        "missing_milestone_reference" => {
            "create it with `tix milestone create`, or clear the epic's milestone"
        }
        "missing_cycle_reference" => "create it with `tix cycle create`, or clear the epic's cycle",
        // Workflow configuration and policy.
        "unmapped_column_default" => {
            "optional: add a `[kanban.<board>.category_map]` table to silence this"
        }
        "direct_edit_policy_violation" => {
            "resolve the contradiction yourself — `tix lint` never edits the file"
        }
        "unknown_column" => "use a column the board configures",
        "invalid_transition" => "run `tix workflow show` to see the permitted transitions",
        "missing_completion_category" => {
            "map a column to `completed` in the board's `category_map`"
        }
        "unknown_category" => "use a category from `tix workflow categories`",
        "mapping_for_unknown_column" => "map only columns the board configures",
        // `cycle` is shared by the relationship and hierarchy passes.
        "cycle" => "break the loop by removing one edge",
        _ => return None,
    })
}

/// The byte span (into the full file text) the diagnostic points at.
fn span_for(f: &TixIssueFinding, text: &str) -> Range<usize> {
    if f.line > 0 {
        // Derived from the finding's **own** line and column, not by searching the
        // text again for a key.
        //
        // The previous version re-ran `locate_value(text, &f.kind)`, which is
        // wrong twice over for a relationship finding: the kind is a *diagnostic*
        // name (`self_link`, `retired_links_key`, `conflicting_edge`) and not a
        // frontmatter key, so the search always missed and fell through to
        // `0..text.len()` — the whole file underlined from line 1. The finding's
        // `line`/`column` were already correct; they were simply being ignored in
        // favour of a second, wrong answer.
        //
        // A finding could also point at a *body* line, which a frontmatter search
        // could never find, so deriving the span is also the only version that
        // works for a body-level diagnostic.
        let mut line_start = 0usize;
        for (idx, line) in text.split_inclusive('\n').enumerate() {
            if idx + 1 == f.line {
                let start = line_start + (f.column - 1).min(line.len());
                let end = line_start + line.trim_end_matches(['\n', '\r']).len();
                return start..end.max(start);
            }
            line_start += line.len();
        }
        // The recorded line is not in the text — a stale finding, or a file that
        // changed under us. Annotate the first line rather than the whole file,
        // which is what the file-level branch below does and is a better guess
        // than underlining everything.
        let first = text.lines().next().map(str::len).unwrap_or(0);
        return 0..if first > 0 { first } else { text.len() };
    }
    // File-level findings (missing/parse) annotate the first line.
    let first = text.lines().next().map(str::len).unwrap_or(0);
    0..if first > 0 { first } else { text.len() }
}

#[cfg(test)]
mod tests {
    use super::*;
    use std::fs;
    use tempfile::TempDir;

    fn make_store() -> (TempDir, Store) {
        let tmp = TempDir::new().unwrap();
        let dir = tmp.path().join("issues");
        fs::create_dir_all(&dir).unwrap();
        let config_path = tmp.path().join("tix.toml");
        let config = crate::config::Config {
            issues_dir: dir.clone(),
            milestones_dir: Some(tmp.path().join("docs/milestones")),
            ..crate::config::Config::default()
        };
        config.save(&config_path).unwrap();
        let config = crate::config::Config::load_with_warning(&config_path, |_| {}).unwrap();
        (
            tmp,
            Store {
                issues_dir: dir,
                config,
            },
        )
    }

    /// Seed one milestone into the store's configured milestones dir.
    fn seed_milestone(store: &Store, name: &str) {
        crate::planning::Planner::from_config(store.config.clone(), std::path::PathBuf::new())
            .create_milestone_file(name, None, None)
            .unwrap();
    }

    fn write(store: &Store, name: &str, contents: &str) {
        if let Some(dir) = name.rsplit_once('/').map(|(d, _)| d) {
            fs::create_dir_all(store.issues_dir.join(dir)).unwrap();
        }
        fs::write(store.issues_dir.join(name), contents).unwrap();
    }

    const CLEAN: &str = "---\nnumber: 1\ntitle: \"A clean issue\"\nstatus: todo\npriority: medium\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\nBody.\n";

    /// Findings that set the exit code.
    fn errors(findings: &[TixIssueFinding]) -> Vec<&TixIssueFinding> {
        findings
            .iter()
            .filter(|f| f.severity == Severity::Error)
            .collect()
    }

    #[test]
    fn collect_reports_no_error_on_a_clean_store() {
        // WP06: this test used to assert `findings.is_empty()`, and that premise is
        // now false **by design**. T026 step 3 requires `unmapped_column_default` for
        // every column relying on the safe default, and the default board has four,
        // so a repository that has never configured a `category_map` — which is
        // every repository, including this one — now reports four warnings.
        //
        // "Clean" therefore means "no finding that sets the exit code". The warnings
        // are asserted separately, by
        // `the_safe_default_is_reported_as_a_warning_not_an_error`.
        let (tmp, store) = make_store();
        let _ = tmp;
        write(&store, "alice/2026-01-01-clean.md", CLEAN);
        let findings = collect(&store.config, &store).unwrap();
        assert_eq!(errors(&findings).len(), 0, "errors: {findings:#?}");
    }

    #[test]
    fn an_unset_priority_is_accepted_but_a_typo_is_still_reported() {
        // `priority: ''` means "no priority", which is a statement the author
        // made, not a value they failed to supply. It is exempt from the
        // unknown-priority check. The exemption is for the empty string *only*,
        // so this test asserts both halves: the empty string is clean, and every
        // neighbouring near-miss is still reported. An exemption written as
        // "ignore priorities we do not recognise" would silence all of them.
        let (tmp, store) = make_store();
        let _ = tmp;
        for (i, value) in ["''", "'hih'", "'urgent'", "'Medium '", "'0'"]
            .iter()
            .enumerate()
        {
            write(
                &store,
                &format!("alice/2026-01-01-p{i}.md"),
                &format!(
                    "---\nnumber: {}\ntitle: \"P{i}\"\nstatus: todo\npriority: {value}\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\nBody.\n",
                    i as u32 + 10
                ),
            );
        }
        let findings = collect(&store.config, &store).unwrap();
        let priority_findings: Vec<&str> = findings
            .iter()
            .filter(|f| f.kind == "priority")
            .map(|f| f.value.as_str())
            .collect();

        assert!(
            !priority_findings.contains(&""),
            "an unset priority is not an error: {findings:?}"
        );
        for typo in ["hih", "urgent", "Medium ", "0"] {
            assert!(
                priority_findings.contains(&typo),
                "{typo:?} must still be reported, got {priority_findings:?}"
            );
        }
    }

    #[test]
    fn collect_flags_invalid_status_and_priority() {
        let (tmp, store) = make_store();
        let _ = tmp;
        write(
            &store,
            "alice/2026-01-01-broken.md",
            "---\nnumber: 2\ntitle: \"Broken\"\nstatus: shipped\npriority: critical\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\nBody.\n",
        );
        let findings = collect(&store.config, &store).unwrap();
        let kinds: Vec<&str> = findings.iter().map(|f| f.kind.as_str()).collect();
        assert!(
            kinds.contains(&"status"),
            "unknown status must be flagged, got {kinds:?}"
        );
        assert!(
            kinds.contains(&"priority"),
            "unknown priority must be flagged, got {kinds:?}"
        );
        assert!(
            findings.iter().any(|f| f.kind == "status"
                && f.message
                    .contains("valid options: todo, in progress, in review, done")),
            "status finding must name the valid set, got {findings:?}"
        );
    }

    #[test]
    fn collect_flags_an_unrecognised_history_entry_left_in_the_body() {
        // The body-scan that reports these is only reachable when the line really
        // is in the body. The previous fixture had a `## History` section whose
        // sole line was ASCII-hyphen, which the parser does not recognise — so
        // the section was empty, the line landed in the body, and the finding
        // came from the `unparsed()` loop instead. Removing the entire body-scan
        // left the test green: it never exercised the code it was named for.
        //
        // The case where the line is unambiguously body text: there is no
        // `## History` section at all, so nothing can put it inside one. The
        // companion case — a trailing bullet after the last real event — is
        // *retained* in the section rather than reaching the body, and is covered
        // by `a_trailing_bullet_after_the_last_event_is_retained_and_reported`.
        let (tmp, store) = make_store();
        let _ = tmp;
        write(
            &store,
            "alice/2026-01-01-no-history-section.md",
            concat!(
                "---\nnumber: 3\ntitle: \"H\"\nstatus: todo\npriority: medium\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\n",
                "## Notes\n",
                "\n",
                "- 2026-01-01T00:00:00Z - alice - relationship_added - ascii hyphens\n",
            ),
        );
        let issue = store.get(crate::storage::IssueSelector::Number(3)).unwrap();
        // Explicit precondition, because this test used to pass for the wrong
        // reason: its old fixture's finding came from the `unparsed()` loop, so
        // deleting the whole body-scan left it green.
        assert!(
            issue
                .body
                .contains("- 2026-01-01T00:00:00Z - alice - relationship_added"),
            "precondition — the line must really be body text, got {:?}",
            issue.body
        );

        let findings = collect(&store.config, &store).unwrap();
        let flagged: Vec<_> = findings.iter().filter(|f| f.kind == "history").collect();
        assert_eq!(flagged.len(), 1, "exactly one finding, {findings:?}");
        assert!(
            flagged[0].message.contains("em-dash"),
            "and it says what is wrong: {}",
            flagged[0].message
        );
    }

    #[test]
    fn lint_does_not_flag_ordinary_body_bullets() {
        // The mirror case, so the diagnostic cannot become noise: prose bullets
        // must stay silent.
        //
        // Cycle-4 F5 widened this list considerably. The first version of the
        // reporting predicate accepted any 10-character run of digits, dashes
        // and punctuation, so `tix lint` — which runs on every issue, on every
        // invocation — told authors to use em-dash separators on schedule
        // windows, phone numbers, version strings and bare ticket numbers. The
        // two bullets this test originally used (`- Reproduction: run it`,
        // `- Expected: it works`) cannot trip that heuristic, so the test was
        // green throughout and the noise was invisible to it.
        let (tmp, store) = make_store();
        let _ = tmp;
        let bullets = [
            "- Reproduction: run it",
            "- Expected: it works",
            "- 10:00-11:00 sync with infra",
            "- 10:00:00.000 elapsed before the timeout",
            "- 1234567890 is the upstream ticket",
            "- +49-30-12345678 is the hotline",
            "- 1.2.3.4.5.6 shipped",
            "- ----------",
            "- 2026-02-02: review the migration",
            "- 2026-01-01..2026-01-05 was the freeze",
            "- 2026-02-02 review the migration",
            // Cycle-5 Blocker 3: three hyphen-separated fields after a date is a
            // changelog line, not a mistyped entry. A version that required only
            // three total fields flagged every one of these, and `tix lint` exits
            // non-zero — so this is a failing lint on ordinary prose, not a
            // cosmetic nit. Four fields is what the format specifies.
            "- 2026-01-01 - owner - status",
            "- 2026-01-01 - Bob - Done",
            "- 2026-01-02 - alice - reviewed",
            "- 2026-01-01 - start - end",
            "- 2026-01-01 - v1.2 - released",
        ];
        for (i, bullet) in bullets.iter().enumerate() {
            write(
                &store,
                &format!("alice/2026-01-01-b{i}.md"),
                &format!(
                    "---\nnumber: {}\ntitle: \"B{i}\"\nstatus: todo\npriority: medium\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\n## Notes\n\n{bullet}\n",
                    i as u32 + 4
                ),
            );
        }
        let findings = collect(&store.config, &store).unwrap();
        assert!(
            !findings.iter().any(|f| f.kind == "history"),
            "ordinary prose must not be reported: {findings:?}"
        );
    }

    #[test]
    fn a_trailing_bullet_after_the_last_event_is_retained_and_reported() {
        // The other side of cycle-4 F1's fix, and the trade it makes explicitly.
        //
        // A `- ` bullet with no em dash at the *end* of a History section is
        // ambiguous: a hand-edited entry, or body prose. The old rule always
        // called it prose, which meant an ordinary command could delete the
        // `## History` heading, demote the line into the body, and render the
        // next append *above* it — with lint silent, because a demoted line is
        // body prose and the unparsed loop no longer sees it.
        //
        // The new rule retains it, and retention is *reported*, so the failure
        // mode is disclosed rather than silent. N1's actual trigger — several
        // body bullets in a row — is unaffected: the second one disqualifies the
        // first, so the section ends where it should and both stay body text.
        for (name, body) in [
            (
                "dated-typo",
                concat!(
                    "## History\n",
                    "\n",
                    "- 2026-01-01T00:00:00Z \u{2014} alice \u{2014} relationship_added \u{2014} real\n",
                    "\n",
                    "- 2026-02-02T00:00:00Z - alice - relationship_added - ascii\n",
                ),
            ),
            (
                "after-a-real-event",
                concat!(
                    "## History\n",
                    "\n",
                    "- 2026-01-01T00:00:00Z \u{2014} alice \u{2014} relationship_added \u{2014} real\n",
                    "\n",
                    "- 2026-02-02 review the migration\n",
                ),
            ),
        ] {
            let (tmp, store) = make_store();
            let _ = tmp;
            write(
                &store,
                &format!("alice/2026-01-01-{name}.md"),
                &format!(
                    "---\nnumber: 3\ntitle: \"H\"\nstatus: todo\npriority: medium\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\n{body}"
                ),
            );
            let issue = store.get(crate::storage::IssueSelector::Number(3)).unwrap();
            assert_eq!(
                issue.history.unparsed().len(),
                1,
                "{name}: retained in the section, not demoted — {:?}",
                issue.history.unparsed()
            );
            assert!(
                issue.body.is_empty(),
                "{name}: and nothing moved into the body, got {:?}",
                issue.body
            );
            let findings = collect(&store.config, &store).unwrap();
            assert!(
                findings.iter().any(|f| f.kind == "history"),
                "{name}: and the retention is reported, not silent — {findings:?}"
            );
        }
    }

    #[test]
    fn lint_flags_the_hand_typing_errors_it_exists_to_catch() {
        // The mirror of the test above, and the reason its earlier version was
        // worth little: a negative test alone cannot tell you the diagnostic
        // still works. Every case here is a real way a hand mistypes an entry
        // with ASCII hyphens — including the two forms the previous
        // implementation missed, because it required the literal `" - "` and a
        // bare `- ` marker while the recognition path accepted both.
        for line in [
            // The canonical typo.
            "- 2026-01-01T00:00:00Z - alice - relationship_added - detail",
            // No spaces around the separators: the same mistake without them.
            "- 2026-01-01T00:00:00Z-alice-relationship_added-detail",
            // A date-only timestamp, and a hand-typed zone offset.
            "- 2026-01-01 - alice - relationship_added - detail",
            "- 2026-01-01T00:00:00+02:00 - alice - relationship_added - detail",
            // Extra whitespace after the marker. `HistoryEvent::parse` strips
            // `- ` and then trims each field, so with em-dash separators this is
            // recognised as a real entry — the asymmetry that made its hyphen
            // equivalent silently unreported.
            //
            // A *tab* marker is deliberately absent from this list:
            // `HistoryEvent::parse` requires `- ` too, so `-\t…` is outside the
            // format on the recognition path as well. That is consistent, not
            // asymmetric, and widening it would be a format change.
            "-  2026-01-01T00:00:00Z - alice - relationship_added - detail",
            "   - 2026-01-01T00:00:00Z - alice - relationship_added - detail",
            // Every accepted list marker, not just `- `.
            "* 2026-01-01T00:00:00Z - alice - relationship_added - detail",
            "+ 2026-01-01T00:00:00Z - alice - relationship_added - detail",
            "1. 2026-01-01T00:00:00Z - alice - relationship_added - detail",
        ] {
            assert!(
                crate::model::looks_like_an_unrecognised_entry(line),
                "{line:?} must be reported — it is a mistyped entry"
            );
        }

        // And end to end, so this is not only a predicate test.
        let (tmp, store) = make_store();
        let _ = tmp;
        write(
            &store,
            "alice/2026-01-01-typo.md",
            concat!(
                "---\nnumber: 9\ntitle: \"T\"\nstatus: todo\npriority: medium\n",
                "created: 2026-01-01\nupdated: 2026-01-01\n---\n\n## Notes\n\n",
                "- 2026-01-01T00:00:00Z-alice-relationship_added-detail\n",
            ),
        );
        let findings = collect(&store.config, &store).unwrap();
        assert!(
            findings.iter().any(|f| f.kind == "history"),
            "the no-space form reaches the user: {findings:?}"
        );
    }

    // ---- WP06 T025/T026: the relationship sweep ----

    /// An empty `links` is not a finding — only a *populated* one has data the
    /// typed model can no longer read.
    ///
    /// Reporting mere presence made `tix lint` reject every issue file this
    /// repository has ever written (they all carry a `links: []` from the old
    /// model), with no way to make them valid short of hand-editing every file,
    /// because the mission provides no migration for the key.
    /// Write one issue file, verbatim, so a test can seed a relationship the
    /// write path would refuse to create. That is the whole point: lint observes
    /// on-disk data, and the interesting on-disk data is exactly what the command
    /// layer rejects.
    fn seed(dir: &Store, number: u32, relationships: &str) {
        let block = if relationships.is_empty() {
            String::new()
        } else {
            format!("relationships:\n{relationships}")
        };
        let text = format!(
            "---\nnumber: {number}\ntitle: T{number}\nstatus: todo\npriority: medium\n\
             labels: []\n{block}created: 2026-01-01\nupdated: 2026-01-01\n---\n\nBody {number}.\n"
        );
        fs::write(
            dir.issues_dir.join(format!("2026-01-01-t{number}.md")),
            text,
        )
        .unwrap();
    }

    fn rel_findings(findings: &[TixIssueFinding]) -> Vec<&TixIssueFinding> {
        findings
            .iter()
            .filter(|f| {
                matches!(
                    f.kind.as_str(),
                    "self_link"
                        | "missing_target"
                        | "conflicting_edge"
                        | "cycle"
                        | "unknown_kind"
                        | "unknown_relation"
                        | "retired_links_key"
                )
            })
            .collect()
    }

    /// One seeded case per diagnostic kind that can exist in stored data.
    ///
    /// **Five of the seven, not seven** — and the reason is structural, not an
    /// omission. `unknown_kind` and `unknown_relation` are *write-time*
    /// diagnostics: a stored edge's kind is deserialised into the closed
    /// `RelationKind` vocabulary, so an unknown kind makes the file fail to parse
    /// (reported as `parse`, before any graph exists), and a stored relationship
    /// has its id by construction so an unresolvable id cannot be stored either.
    /// Both are reachable and correctly named on the write path —
    /// `tix link 0 1 --type sideways` and `tix link edit rel_NOPE` — and pinning
    /// them here would mean asserting something unreachable. T025 step 3 asks for
    /// all seven to "surface here"; that is not satisfiable, and saying so is
    /// more useful than a test that seeds a fixture the parser cannot load.
    #[test]
    fn every_storable_relationship_diagnostic_surfaces() {
        let cases: Vec<(&str, &str, &str)> = vec![
            (
                "self_link",
                "  - id: r1\n    from: 1\n    to: 1\n    kind: blocks\n    display: \"x\"\n",
                "cannot block itself",
            ),
            (
                "missing_target",
                "  - id: r1\n    from: 1\n    to: 99\n    kind: blocks\n    display: \"x\"\n",
                "resolves to no issue",
            ),
            (
                "conflicting_edge",
                "  - id: r1\n    from: 1\n    to: 2\n    kind: blocks\n    display: \"x\"\n  \
                 - id: r2\n    from: 1\n    to: 2\n    kind: related\n    display: \"y\"\n",
                "refusing to store it as",
            ),
            ("retired_links_key", "", "retired `links:` key"),
        ];
        for (kind, relationships, phrase) in cases.iter() {
            let (tmp, store) = make_store();
            let _ = tmp;
            seed(&store, 1, relationships);
            // Issue 2 must exist, or every edge to it is a *missing target* and
            // the case under test never gets to be the thing that is reported.
            seed(&store, 2, "");
            if *kind == "retired_links_key" {
                let f = store.issues_dir.join("2026-01-01-t1.md");
                let text = fs::read_to_string(&f).unwrap();
                fs::write(
                    &f,
                    text.replacen("created: ", "links:\n- '2'\ncreated: ", 1),
                )
                .unwrap();
            }
            let findings = collect(&store.config, &store).unwrap();
            let rels = rel_findings(&findings);
            assert!(
                rels.iter().any(|f| f.kind == *kind),
                "{kind} must surface; got {:?}",
                rels.iter().map(|f| f.kind.clone()).collect::<Vec<_>>()
            );
            let hit = rels.iter().find(|f| f.kind == *kind).unwrap();
            assert!(
                hit.message.contains(phrase),
                "{kind}: the message must explain the problem: {}",
                hit.message
            );
            // And the finding points at the file, not at the store root.
            assert_eq!(hit.file, "2026-01-01-t1.md", "{kind} must name its file");
            // …and at the offending **key**, not at the first line of the file.
            // A finding whose `line` is 0 renders as a file-level block with a
            // caret under line 1, which points at nothing in particular — and it
            // is what happens when the contract name (`retired_links_key`) is
            // passed as the key to search for instead of the key the diagnostic
            // names (`links`). The distinction is invisible unless asserted.
            assert!(
                hit.line > 0,
                "{kind} must point at the offending key, not at line 1: {hit:?}"
            );
        }
    }

    /// A cycle, which cannot be created through the command layer at all —
    /// `tix link` refuses it — and so exists only on disk.
    #[test]
    fn a_cycle_on_disk_is_reported_once() {
        let (tmp, store) = make_store();
        let _ = tmp;
        seed(
            &store,
            1,
            "  - id: r1\n    from: 1\n    to: 2\n    kind: blocks\n    display: \"x\"\n",
        );
        seed(
            &store,
            2,
            "  - id: r2\n    from: 2\n    to: 1\n    kind: blocks\n    display: \"y\"\n",
        );
        let findings = collect(&store.config, &store).unwrap();
        let cycles: Vec<&TixIssueFinding> = rel_findings(&findings)
            .into_iter()
            .filter(|f| f.kind == "cycle")
            .collect();
        assert_eq!(
            cycles.len(),
            1,
            "one cycle, one finding \u{2014} not one per blocked issue: {:?}",
            rel_findings(&findings)
                .iter()
                .map(|f| (f.kind.clone(), f.message.clone()))
                .collect::<Vec<_>>()
        );
        // The message names the whole cycle, so the finding is not misleading
        // about its scope even though it is attributed to one file.
        assert!(
            cycles[0].message.contains("->"),
            "the cycle must be rendered: {}",
            cycles[0].message
        );
    }

    /// A clean tree says nothing (T026 step 4).
    #[test]
    fn a_clean_tree_reports_no_relationship_findings() {
        let (tmp, store) = make_store();
        let _ = tmp;
        // `1 blocks 2` and `2 blocks 3`: a legitimate chain, no cycle. Note that
        // `2 related 1` would *not* be a clean fixture — `related` is
        // order-normalised, so it canonicalises to `1 related 2` and collides with
        // the blocks edge, which is a real `conflicting_edge` and not a mistake.
        seed(
            &store,
            1,
            "  - id: r1\n    from: 1\n    to: 2\n    kind: blocks\n    display: \"x\"\n",
        );
        seed(
            &store,
            2,
            "  - id: r2\n    from: 2\n    to: 3\n    kind: blocks\n    display: \"y\"\n",
        );
        seed(&store, 3, "");
        let findings = collect(&store.config, &store).unwrap();
        assert!(
            rel_findings(&findings).is_empty(),
            "a valid graph is silent: {:?}",
            rel_findings(&findings)
        );
    }

    /// Three distinct problems, three findings — not one, not five (T026 steps 3
    /// and 5).
    #[test]
    fn three_seeded_problems_report_exactly_three_findings() {
        let (tmp, store) = make_store();
        let _ = tmp;
        seed(
            &store,
            1,
            "  - id: r1\n    from: 1\n    to: 1\n    kind: blocks\n    display: \"x\"\n",
        );
        seed(
            &store,
            2,
            "  - id: r2\n    from: 2\n    to: 98\n    kind: blocks\n    display: \"y\"\n",
        );
        seed(
            &store,
            3,
            "  - id: r3\n    from: 3\n    to: 4\n    kind: blocks\n    display: \"z\"\n",
        );
        seed(
            &store,
            4,
            "  - id: r4\n    from: 4\n    to: 3\n    kind: blocks\n    display: \"w\"\n",
        );
        // And a retired key, so the count is not just the three graph rules.
        let f = store.issues_dir.join("2026-01-01-t1.md");
        let text = fs::read_to_string(&f).unwrap();
        fs::write(
            &f,
            text.replacen("created: ", "links:\n- '2'\ncreated: ", 1),
        )
        .unwrap();

        let findings = collect(&store.config, &store).unwrap();
        let rels = rel_findings(&findings);
        let mut kinds: Vec<&str> = rels.iter().map(|f| f.kind.as_str()).collect();
        kinds.sort_unstable();
        assert_eq!(
            kinds,
            vec!["cycle", "missing_target", "retired_links_key", "self_link"],
            "each root cause once, and nothing implied: {kinds:?}"
        );
    }

    /// The sweep is **one** pass, not one per issue (T026 step 1).
    ///
    /// Asserted structurally rather than by timing: `validate_snapshot` is the
    /// only source of relationship findings, it is called exactly once, and the
    /// graph it is called on is built from the walk's own output rather than from
    /// `Store::list` — which is the thing that would have made this quadratic and
    /// would have aborted on a malformed file.
    #[test]
    fn the_sweep_does_not_re_read_the_tree_through_store_list() {
        let source = include_str!("lint.rs");
        let body = &source[source.find("fn collect_relationships").unwrap()
            ..source.find("fn collect_for_file").unwrap()];
        assert!(
            !body.contains("store.list()") && !body.contains("graph_source("),
            "the sweep must graph the tolerant walk's output, not re-read the tree"
        );
    }

    /// A malformed file must not stop the sweep (FR-018).
    ///
    /// This is the regression test for a defect introduced and caught inside this
    /// work package: the first version of the sweep called
    /// `store.graph_source()`, whose `list()` aborts on the first malformed file,
    /// so one broken file made `tix lint` exit 2 with an internal error and
    /// reported **no** relationship problem anywhere in the tree.
    #[test]
    fn a_malformed_file_does_not_stop_the_relationship_sweep() {
        let (tmp, store) = make_store();
        let _ = tmp;
        seed(
            &store,
            1,
            "  - id: r1\n    from: 1\n    to: 1\n    kind: blocks\n    display: \"x\"\n",
        );
        // A file that cannot be parsed at all.
        fs::write(
            store.issues_dir.join("2026-01-01-broken.md"),
            "---\nnumber: 99\ntitle: Broken\nstatus: [not, a, string]\n---\n\nBody.\n",
        )
        .unwrap();

        let findings = collect(&store.config, &store).unwrap();
        assert!(
            findings.iter().any(|f| f.kind == "parse"),
            "the malformed file is still reported as `parse`: {:?}",
            findings.iter().map(|f| f.kind.clone()).collect::<Vec<_>>()
        );
        assert!(
            rel_findings(&findings)
                .iter()
                .any(|f| f.kind == "self_link"),
            "and the sweep still runs, finding the self-link in another file: {:?}",
            rel_findings(&findings)
        );
    }

    /// Two runs, byte-identical output (T026 step 2, and the acceptance gate).
    #[test]
    fn two_runs_over_the_same_tree_agree_exactly() {
        let (tmp, store) = make_store();
        let _ = tmp;
        seed(
            &store,
            1,
            "  - id: r1\n    from: 1\n    to: 1\n    kind: blocks\n    display: \"x\"\n",
        );
        seed(
            &store,
            2,
            "  - id: r2\n    from: 2\n    to: 97\n    kind: blocks\n    display: \"y\"\n",
        );
        seed(
            &store,
            3,
            "  - id: r3\n    from: 3\n    to: 4\n    kind: blocks\n    display: \"z\"\n",
        );
        seed(
            &store,
            4,
            "  - id: r4\n    from: 4\n    to: 3\n    kind: blocks\n    display: \"w\"\n",
        );
        let render = |findings: &[TixIssueFinding]| {
            findings
                .iter()
                .map(|f| format!("{}|{}|{}|{}", f.file, f.kind, f.value, f.message))
                .collect::<Vec<_>>()
        };
        let first = render(&collect(&store.config, &store).unwrap());
        let second = render(&collect(&store.config, &store).unwrap());
        assert_eq!(first, second, "two runs must produce identical output");
        assert!(
            !first.is_empty(),
            "and the fixture must actually find something"
        );
    }

    /// The **rendered** diagnostic points at the offending line.
    ///
    /// The struct fields were already correct, and asserting them proved nothing
    /// about what the user sees: `span_for` re-searched the text for a key named
    /// after the finding's *kind* — `self_link`, `retired_links_key` — which is
    /// not a frontmatter key, so every relationship finding fell through to
    /// `0..text.len()` and the CLI underlined the whole file from line 1. The MCP
    /// `lint_issues` output was right the whole time, so only the CLI surface was
    /// wrong, and only the rendered form showed it.
    #[test]
    fn a_relationship_finding_underlines_the_offending_line_not_the_file() {
        let (tmp, store) = make_store();
        let _ = tmp;
        seed(
            &store,
            1,
            "  - id: r1\n    from: 1\n    to: 1\n    kind: blocks\n    display: \"x\"\n",
        );
        seed(&store, 2, "");
        let findings = collect(&store.config, &store).unwrap();
        let finding = rel_findings(&findings)
            .into_iter()
            .find(|f| f.kind == "self_link")
            .expect("a self_link finding");
        let text = fs::read_to_string(store.issues_dir.join(&finding.file)).unwrap();
        let span = span_for(finding, &text);
        let line_no = text[..span.start].lines().count().max(1);
        assert_eq!(
            line_no, finding.line,
            "the span must start on the line the finding recorded"
        );
        assert!(
            span.end <= text.len() && span.end > span.start,
            "and must be a real range inside the file: {span:?} of {} bytes",
            text.len()
        );
        // And it must not have degenerated into the whole file.
        assert!(
            span.end < text.len(),
            "the span must not cover the whole file: {span:?}"
        );
        // The span sits on the line that declares the field the finding names.
        // `TixIssueFinding::value` is the *field* name for a relationship
        // diagnostic — that is what the title reads
        // (`invalid self_link value 'to'`) — while the span covers that field's
        // value. So the check is that the underlined line belongs to the named
        // field, not that the two strings are equal.
        let line_start = text[..span.start].rfind('\n').map(|i| i + 1).unwrap_or(0);
        let line = &text[line_start
            ..text[line_start..]
                .find('\n')
                .map(|i| line_start + i)
                .unwrap_or(text.len())];
        assert!(
            line.trim_start()
                .starts_with(&format!("{}:", finding.value)),
            "the underline must be on the line declaring `{}`, got {line:?}",
            finding.value
        );
        assert!(
            !text[span.clone()].trim().is_empty(),
            "and it must cover that field's value: {span:?}"
        );
    }

    #[test]
    fn an_empty_links_key_is_not_a_finding() {
        let (tmp, store) = make_store();
        let _ = tmp;
        for (i, links) in ["[]", "~", "null", ""].iter().enumerate() {
            let n = 10 + i as u32;
            write(
                &store,
                &format!("alice/2026-01-01-empty-{i}.md"),
                &format!(
                    "---\nnumber: {n}\ntitle: \"Empty {i}\"\nstatus: todo\npriority: medium\n\
                     links: {links}\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\nBody.\n"
                ),
            );
        }
        let findings = collect(&store.config, &store).unwrap();
        let retired: Vec<&TixIssueFinding> = findings
            .iter()
            .filter(|f| f.kind == "retired_links_key")
            .collect();
        assert!(
            retired.is_empty(),
            "an empty `links` loses nothing, so it is not a finding: {findings:?}"
        );
    }

    /// A populated `links` is still a finding, in every shape the old model and a
    /// hand-edit can produce.
    #[test]
    fn a_populated_links_key_is_still_a_finding() {
        let (tmp, store) = make_store();
        let _ = tmp;
        for (i, links) in ["\n- '12'", "\n- 12\n- 13", " ['12']", " [12]", "\n- task-7"]
            .iter()
            .enumerate()
        {
            let n = 20 + i as u32;
            write(
                &store,
                &format!("alice/2026-01-01-full-{i}.md"),
                &format!(
                    "---\nnumber: {n}\ntitle: \"Full {i}\"\nstatus: todo\npriority: medium\n\
                     links:{links}\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\nBody.\n"
                ),
            );
        }
        let findings = collect(&store.config, &store).unwrap();
        let retired: Vec<&TixIssueFinding> = findings
            .iter()
            .filter(|f| f.kind == "retired_links_key")
            .collect();
        assert_eq!(
            retired.len(),
            5,
            "every populated shape is reported: {findings:?}"
        );
    }

    #[test]
    fn collect_flags_a_retired_links_key() {
        // The retired `links:` key is reported once, as a whole-field finding.
        // Per-entry value checking is gone with the field: the value is never
        // read as typed data (C-001), so there is nothing to validate entry by
        // entry.
        let (tmp, store) = make_store();
        let _ = tmp;
        write(
            &store,
            "alice/2026-01-01-linked.md",
            "---\nnumber: 3\ntitle: \"Linked\"\nstatus: todo\npriority: medium\nlinks:\n- '12'\n- task-7\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\nBody.\n",
        );
        let findings = collect(&store.config, &store).unwrap();
        let retired: Vec<&TixIssueFinding> = findings
            .iter()
            .filter(|f| f.kind == "retired_links_key")
            .collect();
        assert_eq!(
            retired.len(),
            1,
            "one finding for the retired key: {findings:?}"
        );
        assert_eq!(retired[0].value, "links", "the finding names the field");
        assert!(
            retired[0].message.contains("retired"),
            "the message explains why: {}",
            retired[0].message
        );
        assert!(retired[0].line > 0, "the finding is located in the file");
    }

    #[test]
    fn collect_reports_unparseable_file_as_missing_finding() {
        let (tmp, store) = make_store();
        let _ = tmp;
        write(
            &store,
            "alice/2026-01-01-not-frontmatter.md",
            "Just prose, no frontmatter.\n",
        );
        let findings = collect(&store.config, &store).unwrap();
        // Counted over **errors**, not over all findings: WP06 added the safe-default
        // warnings, so `findings.len()` is now 5 rather than 1 and the count was
        // measuring the default board's column count. An earlier version of this
        // assertion failed with `left: 4, right: 1` — the extra findings were the
        // board's four columns, not anything about the unparseable file.
        let errors = errors(&findings);
        assert_eq!(errors.len(), 1, "{findings:#?}");
        assert_eq!(errors[0].kind, "missing");
        assert_eq!(errors[0].file, "alice/2026-01-01-not-frontmatter.md");
    }

    #[test]
    fn collect_flags_unknown_cycle_and_milestone_with_valid_lists() {
        let (tmp, mut store) = make_store();
        let _ = tmp;
        store.config.cycles.insert(
            "Cycle 1".to_string(),
            crate::config::PlanningCycle::default(),
        );
        seed_milestone(&store, "v1.0");
        write(
            &store,
            "alice/2026-01-01-planning.md",
            "---\nnumber: 4\ntitle: \"Planning\"\nstatus: todo\npriority: medium\ncycles:\n- Nope\n- Cycle 1\nmilestones:\n- v2.0\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\nBody.\n",
        );
        let findings = collect(&store.config, &store).unwrap();
        assert!(
            findings.iter().any(|f| f.kind == "cycles"
                && f.value == "Nope"
                && f.message.contains("valid cycles: Cycle 1")),
            "unknown cycle finding must name the valid set, got {findings:?}"
        );
        assert!(
            findings.iter().any(|f| f.kind == "milestones"
                && f.value == "v2.0"
                && f.message.contains("valid milestones: v1.0")),
            "unknown milestone finding must name the valid set, got {findings:?}"
        );
        assert!(
            findings
                .iter()
                .all(|f| f.kind != "cycles" || f.value != "Cycle 1"),
            "a configured cycle must not be flagged, got {findings:?}"
        );
        assert!(
            findings
                .iter()
                .all(|f| f.kind != "milestones" || f.value != "v1.0"),
            "a registered milestone must not be flagged, got {findings:?}"
        );
    }

    #[test]
    fn collect_flags_unknown_milestone_when_registry_empty() {
        let (tmp, store) = make_store();
        let _ = tmp;
        write(
            &store,
            "alice/2026-01-01-orphan.md",
            "---\nnumber: 5\ntitle: \"Orphan\"\nstatus: todo\npriority: medium\nmilestones:\n- nowhere\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\nBody.\n",
        );
        let findings = collect(&store.config, &store).unwrap();
        assert!(
            findings.iter().any(|f| f.kind == "milestones"
                && f.message.contains("valid milestones: none configured")),
            "an empty registry must render as none configured, got {findings:?}"
        );
    }

    #[test]
    fn collect_flags_due_before_start_with_frozen_message() {
        let (tmp, store) = make_store();
        let _ = tmp;
        write(
            &store,
            "alice/2026-01-01-dates.md",
            "---\nnumber: 6\ntitle: \"Dates\"\nstatus: todo\npriority: medium\nstart_date: 2026-09-18\ndue_date: 2026-09-07\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\nBody.\n",
        );
        let findings = collect(&store.config, &store).unwrap();
        assert!(
            findings.iter().any(|f| f.kind == "due_date"
                && f.value == "2026-09-07"
                && f.message == "due_date \"2026-09-07\" must be `>= start_date` \"2026-09-18\""),
            "due-before-start must be flagged with the frozen message, got {findings:?}"
        );
        assert!(
            findings.iter().any(|f| f.kind == "due_date" && f.line > 0),
            "the date finding must be located at the due_date line"
        );
    }

    #[test]
    fn collect_flags_malformed_calendar_date() {
        let (tmp, store) = make_store();
        let _ = tmp;
        write(
            &store,
            "alice/2026-01-01-bad-date.md",
            "---\nnumber: 7\ntitle: \"Bad date\"\nstatus: todo\npriority: medium\nstart_date: 2026-13-99\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\nBody.\n",
        );
        let findings = collect(&store.config, &store).unwrap();
        assert!(
            findings.iter().any(|f| f.kind == "start_date"
                && f.value == "2026-13-99"
                && f.message == "start_date \"2026-13-99\" must be a `YYYY-MM-DD` calendar date"),
            "a non-calendar date must be flagged, got {findings:?}"
        );
    }

    /// #1308: a `## History` section that is not at the front of the body is
    /// reclassified as body text, silently.
    ///
    /// The parser is right — `data-model.md` §8 pins the section to the front —
    /// and that is what makes the violation dangerous: the section is not
    /// rejected, it is demoted. `tix view` then shows the issue with no audit
    /// trail, and the next `tix edit --body` replaces the body wholesale, which
    /// deletes the section and every event in it. A plain write preserves it, so
    /// the loss arrives later and looks unrelated to the hand-edit that caused
    /// it.
    ///
    /// Measured, not inferred: each variant below was driven through the real
    /// binary and the whole section was destroyed in every case — not only the
    /// 3-field entry this was first filed against.
    #[test]
    fn a_history_section_that_is_not_at_the_front_of_the_body_is_reported() {
        let (tmp, store) = make_store();
        let _ = tmp;
        write(
            &store,
            "2026-01-01-misplaced.md",
            "---\nnumber: 1\ntitle: \"Misplaced\"\nstatus: todo\npriority: medium\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\nSome ordinary paragraph.\n\n## History\n\n* 2026-01-01 \u{2014} alice \u{2014} relationship_added\n",
        );
        let findings = collect(&store.config, &store).unwrap();
        let message = findings
            .iter()
            .find(|f| f.file == "2026-01-01-misplaced.md")
            .map(|f| f.message.clone())
            .expect("a misplaced `## History` section must be reported");
        assert!(
            message.contains("must be the first thing in the body"),
            "names the rule: {message}"
        );
        assert!(
            message.contains("tix edit --body"),
            "names the consequence, so the finding is actionable: {message}"
        );
    }

    /// The diagnostic must not fire on a correctly placed section, or `tix lint`
    /// would report every healthy issue.
    #[test]
    fn a_history_section_at_the_front_of_the_body_is_not_reported() {
        let (tmp, store) = make_store();
        let _ = tmp;
        write(
            &store,
            "2026-01-01-placed.md",
            "---\nnumber: 1\ntitle: \"Placed\"\nstatus: todo\npriority: medium\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\n## History\n\n- 2026-01-01T00:00:00Z \u{2014} alice \u{2014} relationship_added \u{2014} d\n\nSome ordinary paragraph.\n",
        );
        let findings = collect(&store.config, &store).unwrap();
        assert!(
            !findings
                .iter()
                .any(|f| f.message.contains("first thing in the body")),
            "a well-placed section is not a violation: {findings:?}"
        );
    }

    /// The lint rule and the parser must agree on which lines are headings, or
    /// lint reports violations that do not exist while missing ones that do.
    /// `## History   ` is a heading; `   ## History` and `## History\r` are not.
    #[test]
    fn the_heading_test_matches_the_parser_exactly() {
        for (line, expected) in [
            ("## History", true),
            ("## History ", true),
            ("## History\t", true),
            ("## History \t ", true),
            ("   ## History", false),
            ("## History\r", false),
            ("## History extra", false),
            ("## History:", false),
        ] {
            assert_eq!(crate::model::is_history_heading(line), expected, "{line:?}");
        }
    }

    /// A `## History` inside a fenced code block is a *documented example*, not a
    /// misplaced section. Reporting it flagged every issue that describes the
    /// History format — this repository's own mission findings did — so the scan
    /// is fence-aware.
    #[test]
    fn a_fenced_history_heading_is_not_a_misplaced_section() {
        let (tmp, store) = make_store();
        let _ = tmp;
        write(
            &store,
            "2026-01-01-fenced.md",
            "---\nnumber: 1\ntitle: \"Fenced\"\nstatus: todo\npriority: medium\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\nAn example file:\n\n```markdown\n## History\n\n- 2026-01-01 - alice - relationship_added\n```\n\nThat is how the section looks.\n",
        );
        let findings = collect(&store.config, &store).unwrap();
        assert!(
            !findings.iter().any(|f| f.file == "2026-01-01-fenced.md"),
            "nothing inside a fence is a section or an entry: {findings:?}"
        );
    }

    /// The diagnostic points at the misplaced heading, not at line 1. The old
    /// `value_finding` path searched for a frontmatter key `history`, missed, and
    /// underlined the opening `---` — a span that named the wrong line.
    #[test]
    fn a_misplaced_history_finding_points_at_the_heading() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let text = "---\nnumber: 1\ntitle: \"Misplaced\"\nstatus: todo\npriority: medium\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\nSome ordinary paragraph.\n\n## History\n\n* 2026-01-01 \u{2014} alice \u{2014} relationship_added\n";
        write(&store, "2026-01-01-misplaced.md", text);
        let findings = collect(&store.config, &store).unwrap();
        let finding = findings
            .iter()
            .find(|f| f.message.contains("first thing in the body"))
            .expect("the misplaced heading is reported");
        let heading_line = text
            .lines()
            .position(|line| line == "## History")
            .map(|index| index + 1)
            .unwrap();
        assert_eq!(
            finding.line, heading_line,
            "the span names the heading line, not line 1"
        );
        assert!(finding.column >= 1, "column is 1-based: {}", finding.column);
    }

    /// Every kind the linter can emit carries a `help:` remedy, so no diagnostic
    /// dead-ends without telling the reader what to do. The list is the union of
    /// the literal kinds and the three diagnostic enums; it is exhaustive on
    /// purpose, so adding a check without a remedy fails here.
    #[test]
    fn every_finding_kind_has_a_help_line() {
        let literals = [
            "status",
            "title",
            "priority",
            "unknown_type",
            "created",
            "updated",
            "estimate",
            "unknown_key",
            "cycles",
            "milestones",
            "start_date",
            "due_date",
            "history",
            "related_issue_empty",
            "related_issue_unresolved",
            "missing",
            "parse",
            "layout_mismatch",
            "foreign_repo_not_linted",
            "missing_project_reference",
            "missing_epic_reference",
            "missing_issue_reference",
            "missing_milestone_reference",
            "missing_cycle_reference",
        ];
        let relationships = [
            "self_link",
            "missing_target",
            "conflicting_edge",
            "cycle",
            "unknown_kind",
            "unknown_relation",
            "retired_links_key",
        ];
        let hierarchy = [
            "self_parent",
            "unresolved_parent",
            "cycle",
            "unknown_parent",
        ];
        let workflow = crate::workflow::DiagnosticKind::ALL.map(|kind| kind.as_str());
        for kind in literals
            .iter()
            .copied()
            .chain(workflow)
            .chain(relationships)
            .chain(hierarchy)
        {
            assert!(
                diagnostic_help(kind).is_some(),
                "`{kind}` has no `help:` remedy"
            );
        }
    }

    /// The renderer is rustc-style `annotate-snippets` output: a severity header
    /// with the id, and a `help:` footer naming the fix.
    #[test]
    fn a_rendered_diagnostic_carries_the_id_and_a_help_footer() {
        let finding = TixIssueFinding {
            severity: Severity::Error,
            file: "issues/x.md".into(),
            line: 0,
            column: 0,
            value: "wip".into(),
            kind: "status".into(),
            message: "valid options: todo, in progress, done".into(),
        };
        let rendered = render_diagnostic(std::path::Path::new("/nonexistent-tix-lint"), &finding);
        assert!(rendered.contains("E-tix-lint"), "{rendered}");
        assert!(
            rendered.contains("invalid status value 'wip'"),
            "{rendered}"
        );
        assert!(
            rendered.contains("set `status:` to one of the board's configured columns"),
            "the remedy is attached: {rendered}"
        );

        let warning = TixIssueFinding {
            severity: Severity::Warning,
            ..finding
        };
        let rendered = render_diagnostic(std::path::Path::new("/nonexistent-tix-lint"), &warning);
        assert!(rendered.contains("W-tix-lint"), "{rendered}");
    }
}