tix-tracker 0.6.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 annotate_snippets::{AnnotationKind, Group, Level, Renderer, Snippet};

use crate::commands::create::planner_for;
use crate::config::Config;
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 {
        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);
    // 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);
    Ok(findings)
}

/// 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 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 \
                     `custom_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.
    for (index, line) in issue.body.lines().enumerate() {
        if crate::model::is_history_heading(line) {
            findings.push(value_finding(
                rel,
                text,
                "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(),
            ));
            let _ = index;
        }
    }
    // 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.
    for line in issue.body.lines() {
        if crate::model::looks_like_an_unrecognised_entry(line) {
            findings.push(value_finding(
                rel,
                text,
                "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,
    }
}

/// 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 to stderr (C-004). The
/// header names the field/value; the origin and annotation label name the
/// file, line and valid set in a single reading (NFR-006).
fn render_diagnostic(issues_dir: &Path, f: &TixIssueFinding) {
    let renderer = Renderer::styled();
    let title = match f.kind.as_str() {
        "missing" => "missing frontmatter".to_string(),
        "parse" => "parse error".to_string(),
        _ => format!("invalid {} value '{}'", f.kind, f.value),
    };
    // A warning must not be **rendered** as an error either (#1361). `Level::ERROR`
    // in red with an `E-` id reads as "this file is broken" at a glance, and the
    // whole point of the severity split is that an unrecognised key is a valid file.
    // The level, the id prefix, and the colour all follow the severity, so the
    // output agrees with the exit code.
    let (level, id) = match f.severity {
        Severity::Error => (Level::ERROR, "E-tix-lint"),
        Severity::Warning => (Level::WARNING, "W-tix-lint"),
    };
    let text = fs::read_to_string(issues_dir.join(&f.file)).ok();
    let group = match text {
        Some(text) => level.primary_title(title).id(id).element(
            Snippet::source(text.clone())
                .line_start(1)
                .path(&f.file)
                .annotation(
                    AnnotationKind::Primary
                        .span(span_for(f, &text))
                        .label(&f.message),
                ),
        ),
        // No file to point at — a configuration finding, or a diagnostic with no
        // single subject. The message is appended to the title here, because this
        // branch otherwise rendered a **title only**: a finding that said which kind
        // fired and on what, and dropped the sentence explaining it. That is how
        // WP06's `unmapped_column_default` reported "invalid unmapped_column_default
        // value 'default'" and nothing else — the board, the columns, and the remedy
        // all reached the finding and never reached the user. The relationship path's
        // no-subject branch had the same hole since #1361 and its comment claimed
        // the finding "still has to surface".
        None => {
            let mut head = title;
            if !f.message.is_empty() {
                head.push_str(": ");
                head.push_str(&f.message);
            }
            if f.file.is_empty() {
                Group::with_title(level.primary_title(head).id(id))
            } else {
                Group::with_title(level.primary_title(format!("{}: {head}", f.file)).id(id))
            }
        }
    };
    eprintln!("{}", renderer.render(&[group]));
}

/// 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:?}");
        }
    }
}