tix/

model.rs

//! Issue data model and file-surface types.
//!
//! Mirrors `contracts/file-format.md` and `data-model.md` exactly. The
//! [`Issue`] struct is the Serde surface for the YAML frontmatter of an
//! issue file; the markdown body is carried in `body` and round-trips
//! verbatim. [`Status`], [`Priority`] and [`Labels`] are the value types
//! that appear in frontmatter.

use std::collections::{BTreeMap, HashSet};
use std::fmt;
use std::str::FromStr;
use std::time::{SystemTime, UNIX_EPOCH};

use serde::{Deserialize, Deserializer, Serialize, Serializer};

use crate::error::{Result, TixError};
use crate::relationships::{IssueView, Relationship};
use crate::slug::slugify;

/// A board column name. A newtype over the user-defined column strings from
/// `tix.toml`; unknown names survive deserialization because columns are
/// config-defined (a renamed column renders as a "Mismatched" pseudo-column,
/// never a crash).
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
pub struct Status(pub String);

impl Status {
    /// Create a status from a column name.
    pub fn new(name: impl Into<String>) -> Self {
        Status(name.into())
    }

    /// Whether this status equals `name` (exact, case-sensitive).
    pub fn is(&self, name: &str) -> bool {
        self.0 == name
    }

    /// The column name this status refers to.
    pub fn display(&self) -> &str {
        &self.0
    }
}

impl Default for Status {
    fn default() -> Self {
        Status("todo".to_string())
    }
}

impl fmt::Display for Status {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        f.write_str(&self.0)
    }
}

/// Issue priority.
///
/// The documented set is `low` / `medium` / `high` with `medium` as the
/// default (spec + data-model). Any other value is preserved as-is for
/// forward compatibility (data-model: "unknown values accepted as-is",
/// "preserved ... but not normalized"), so e.g. `urgent` round-trips
/// unchanged.
#[derive(Debug, Clone, PartialEq, Eq, Hash, Default)]
pub enum Priority {
    Low,
    #[default]
    Medium,
    High,
    /// An unknown priority string, preserved verbatim (forward-compatible).
    ///
    /// The empty string is *not* treated as unknown: it is an explicitly unset
    /// priority, which `tix lint` accepts and which round-trips as `''`. It stays
    /// in this variant rather than getting a variant of its own because
    /// `Priority` is public API on a published crate, and adding a variant to a
    /// non-`#[non_exhaustive]` enum is a semver break for every downstream
    /// `match`. Use [`Priority::is_undefined`] rather than comparing the string.
    Other(String),
}

impl Priority {
    fn as_str(&self) -> &str {
        match self {
            Priority::Low => "low",
            Priority::Medium => "medium",
            Priority::High => "high",
            Priority::Other(s) => s,
        }
    }

    /// Whether the priority is explicitly **unset** — written `priority: ''`.
    ///
    /// Distinct from [`Priority::default`], which is `Medium`. An issue can
    /// therefore say "no priority" without being read as "medium", and without
    /// the value being silently rewritten on the next write.
    ///
    /// `tix lint` accepts this state; see [`crate::commands::lint`] for the
    /// check it is exempt from.
    pub fn is_undefined(&self) -> bool {
        matches!(self, Priority::Other(s) if s.is_empty())
    }
}

impl FromStr for Priority {
    type Err = std::convert::Infallible;

    fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
        Ok(match s.to_ascii_lowercase().as_str() {
            "low" => Priority::Low,
            "medium" => Priority::Medium,
            "high" => Priority::High,
            _ => Priority::Other(s.to_string()),
        })
    }
}

impl fmt::Display for Priority {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        f.write_str(self.as_str())
    }
}

impl Serialize for Priority {
    fn serialize<S: Serializer>(&self, s: S) -> std::result::Result<S::Ok, S::Error> {
        s.serialize_str(self.as_str())
    }
}

impl<'de> Deserialize<'de> for Priority {
    fn deserialize<D: Deserializer<'de>>(d: D) -> std::result::Result<Self, D::Error> {
        let raw = String::deserialize(d)?;
        match raw.parse() {
            Ok(parsed) => Ok(parsed),
            Err(_) => Ok(Priority::Other(raw)),
        }
    }
}

/// Issue labels. Free-form strings kept exactly as authored (case and
/// spaces preserved); default `[]`.
pub type Labels = Vec<String>;

/// Assignee file-encoding rule (FR-027 / FR-014, `contracts/file-format.md`
/// rule 5).
///
/// - **In memory**: a manual handle is always bare (`alice`, never
///   `@alice`); a git-derived identity is the full `Name <email>` string,
///   verbatim.
/// - **Read** ([`de_assignee`]): a single leading `@` is stripped, so both
///   the legacy `@alice` bytes and the bare `alice` bytes normalize to the
///   same in-memory handle. A full identity / identity-like value never
///   starts with `@` and passes through untouched.
/// - **Write** ([`se_assignee`]): a value carrying an identity form (both `<`
///   and `>` present, e.g. `Name <email>`) is persisted verbatim — no forced
///   `@` and no mangling of the angle brackets. Any other value is a manual
///   handle and is persisted with a single leading `@`, byte-identical to the
///   legacy `@alice` files (C-010) and to what `contracts/cli.md` conformance
///   pins for `--assignee @alice`.
/// - `None` persists as `null` (omitted by `skip_serializing_if`).
///
/// The in-memory value stays display-neutral: the `@` cosmetic a CLI/TUI
/// renders around handles is their concern, not the model's.
fn de_assignee<'de, D: Deserializer<'de>>(d: D) -> std::result::Result<Option<String>, D::Error> {
    let opt = Option::<String>::deserialize(d)?;
    Ok(opt.map(|v| v.strip_prefix('@').unwrap_or(&v).to_string()))
}

/// Serialize an assignee per the encoding rule documented on [`de_assignee`]:
/// identity-like values verbatim, manual handles `@`-prefixed (legacy bytes).
fn se_assignee<S: Serializer>(opt: &Option<String>, s: S) -> std::result::Result<S::Ok, S::Error> {
    match opt {
        None => s.serialize_none(),
        Some(value) => {
            let is_identity = value.contains('<') && value.contains('>');
            let full = if is_identity || value.starts_with('@') {
                value.clone()
            } else {
                format!("@{value}")
            };
            s.serialize_str(&full)
        }
    }
}

/// Serialization rule for the optional planning lists `cycles`/`milestones`
/// (`contracts/file-format-planning.md`): the key is omitted when absent or
/// empty (only non-empty lists serialize), and duplicate entries are
/// collapsed on write preserving first-seen order.
fn skip_planning_list(v: &Option<Vec<String>>) -> bool {
    v.as_deref().is_none_or(|items| items.is_empty())
}

fn se_planning_list<S: Serializer>(
    v: &Option<Vec<String>>,
    s: S,
) -> std::result::Result<S::Ok, S::Error> {
    let items: Vec<&String> = match v {
        Some(items) => {
            items
                .iter()
                .fold((Vec::new(), HashSet::new()), |(mut out, mut seen), item| {
                    if seen.insert(item.as_str()) {
                        out.push(item);
                    }
                    (out, seen)
                })
                .0
        }
        None => Vec::new(),
    };
    items.serialize(s)
}

/// Today's date as a local-free calendar date in `YYYY-MM-DD`.
///
/// Computed from the Unix epoch in UTC; `crate`-wide reproducible and
/// dependency-free. Later WPs reuse it for `created`/`updated` and the
/// date prefix of filenames.
pub fn today() -> String {
    let secs = SystemTime::now()
        .duration_since(UNIX_EPOCH)
        .map(|d| d.as_secs())
        .unwrap_or(0);
    let days = (secs / 86_400) as i64;
    let (y, m, d) = civil_from_days(days);
    format!("{y:04}-{m:02}-{d:02}")
}

/// The current UTC instant as RFC 3339 (`2026-09-27T10:14:02Z`), used for
/// [`HistoryEvent`] timestamps.
///
/// Dependency-free like [`today`]: the epoch seconds are converted through
/// [`civil_from_days`] and the time-of-day is derived arithmetically.
pub fn utc_now_rfc3339() -> String {
    let secs = SystemTime::now()
        .duration_since(UNIX_EPOCH)
        .map(|d| d.as_secs())
        .unwrap_or(0);
    let days = (secs / 86_400) as i64;
    let rem = secs % 86_400;
    let (y, m, d) = civil_from_days(days);
    format!(
        "{y:04}-{m:02}-{d:02}T{:02}:{:02}:{:02}Z",
        rem / 3600,
        (rem % 3600) / 60,
        rem % 60
    )
}

/// Whether a frontmatter document still carries the retired `links:` key.
///
/// The key is not part of the model at all, so it cannot be detected after
/// deserialization, and it must never be *read* as typed data (C-001). The
/// frontmatter is therefore also parsed as a plain YAML mapping and checked for
/// a **top-level** `links` key.
///
/// A raw line scan was tried first and was wrong in both directions: it matched
/// an indented `links:` under some other key, and it missed a quoted `"links":`
/// key. Matching the parsed top-level key gets both right, and a line inside a
/// `|`/`>` block scalar is unambiguously not a key.
///
/// A frontmatter that will not parse as a mapping falls back to a line scan.
/// That fallback is **indentation-aware**, matching only column-0 keys.
///
/// It has to be. `serde_yaml` rejects duplicate keys when deserializing into a
/// `Mapping` but *tolerates* a duplicate unknown key when deserializing into
/// `Issue`, so a file can reach the fallback and still parse. An earlier
/// indentation-blind fallback then matched the *nested* `links:` in
/// `meta:\n  links: []` and reported a retired key on a file that has none —
/// the two code paths disagreeing about what "top-level" means, which is
/// precisely what the primary path was introduced to get right.
///
/// **An empty `links` is not a finding.** The key is reported because its *value*
/// is no longer readable, and an empty value has nothing to lose. Treating mere
/// presence as the defect 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. A tool that declares its entire existing
/// corpus invalid on the strength of a field that was empty is not reporting a
/// problem, it is refusing to read its own data.
/// Every top-level key this model knows, in `Issue`'s declaration order, plus the
/// retired `links:` at the end.
///
/// The trailing `links` is not a field — see below — so "declaration order" holds
/// for the modelled keys and the retired key is simply appended.
///
/// Used by [`capture_extra_frontmatter`] to decide what is "unknown". Kept as an
/// explicit list rather than derived from the struct because a macro cannot read a
/// struct's own field names, and a mistake here is silent data loss: a key wrongly
/// listed as known is dropped on write, which is the bug #1361 exists to remove.
///
/// A mistake in the other direction is equally silent and was equally real: when
/// `parent` was added to [`Issue`] and not here, [`capture_extra_frontmatter`] took it
/// for an unknown key and the writer emitted `parent: 7` **twice** in one file — once
/// from the field, once spliced back from the capture. The file still parsed, so
/// nothing failed until a test compared bytes.
/// `known_frontmatter_keys_cover_every_written_key` derives the key set from a
/// fully-populated [`Issue`] and is the guard for it: adding a frontmatter field
/// without registering its key here is now a compile-clean, test-red change.
///
/// The retired `links:` is here on purpose. It is **recognised but never captured**,
/// so it is reported by its own `retired_links_key` finding and then dropped (C-001).
/// Capturing it would round-trip it and quietly undo the typed-relationship
/// migration — the data would come back, but the model would still not understand it.
pub const KNOWN_FRONTMATTER_KEYS: &[&str] = &[
    "number",
    "title",
    "status",
    "labels",
    "priority",
    "assignee",
    "estimate",
    "commit",
    "gitlab_id",
    "linear_id",
    "mission",
    "task",
    "relationships",
    "parent",
    "files",
    "cycles",
    "milestones",
    "reviewer",
    "start_date",
    "due_date",
    "entered_at",
    "source",
    "created",
    "updated",
    "links",
];

/// Capture every top-level frontmatter key this model does not know, as
/// `key -> raw source block` (#1361).
///
/// **Raw block, not a parsed value.** Re-serialising a parsed value would re-quote
/// and re-indent it, so a key written one way would come back another way and every
/// write would produce a diff. Splicing the original bytes keeps the round trip
/// byte-stable for content this model has no opinion about — which is the whole
/// point of preserving it.
///
/// A block is the key's own line plus every following line that continues it: an
/// indented line, or a block-sequence item. The scan is textual on purpose. A YAML
/// parse would be tidier, but the raw text is what has to be reproduced, and
/// re-deriving block boundaries from a parse is how a nested list ends up half
/// captured.
///
/// `links` is skipped. See [`KNOWN_FRONTMATTER_KEYS`].
pub fn capture_extra_frontmatter(frontmatter: &str) -> BTreeMap<String, String> {
    let mut out = BTreeMap::new();
    let mut current: Option<(String, String)> = None;
    for line in frontmatter.lines() {
        match top_level_key(line) {
            Some(key) => {
                if let Some((name, block)) = current.take() {
                    out.insert(name, block);
                }
                if KNOWN_FRONTMATTER_KEYS.contains(&key.as_str()) {
                    continue;
                }
                current = Some((key, line.to_string()));
            }
            None => {
                // A continuation line: indented, or a block-sequence item. A blank
                // line ends the block, because YAML does too.
                if let Some((_, block)) = current.as_mut() {
                    if line.trim().is_empty() {
                        current = None;
                    } else {
                        block.push('\n');
                        block.push_str(line);
                    }
                }
            }
        }
    }
    if let Some((name, block)) = current.take() {
        out.insert(name, block);
    }
    out
}

/// The key of a top-level YAML mapping entry, or `None` for a continuation line.
///
/// Only column 0 counts. An indented `key:` is part of a nested mapping, and a
/// `- item` is a sequence entry; treating either as a top-level key would end the
/// previous block early and capture a fragment.
fn top_level_key(line: &str) -> Option<String> {
    if !line.starts_with(|c: char| c.is_ascii_alphabetic() || c == '_') {
        return None;
    }
    let colon = line.find(':')?;
    let key = &line[..colon];
    if key.is_empty()
        || !key
            .chars()
            .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
    {
        return None;
    }
    Some(key.to_string())
}

fn frontmatter_has_retired_links_key(frontmatter: &str) -> bool {
    if let Ok(mapping) = serde_yaml::from_str::<serde_yaml::Mapping>(frontmatter) {
        let key = serde_yaml::Value::String("links".into());
        return match mapping.get(&key) {
            Some(value) => !links_value_is_empty(value),
            None => false,
        };
    }
    frontmatter.lines().any(|line| {
        if line.starts_with(' ') || line.starts_with('\t') {
            return false;
        }
        line.split_once(':')
            .map(|(key, _)| key.trim() == "links")
            .unwrap_or(false)
    })
}

/// Whether a `links:` value carries no entries, in the shapes the old model
/// wrote and the shapes a hand-edit can produce: `[]`, `~`/`null`, an empty
/// scalar, and a block sequence with no items (`links:\n` followed by the next
/// key). Anything else is populated and therefore a finding.
fn links_value_is_empty(value: &serde_yaml::Value) -> bool {
    match value {
        serde_yaml::Value::Null => true,
        serde_yaml::Value::Sequence(items) => items.is_empty(),
        serde_yaml::Value::String(s) => s.trim().is_empty() || s.trim() == "[]",
        serde_yaml::Value::Mapping(m) => m.is_empty(),
        serde_yaml::Value::Tagged(t) => links_value_is_empty(&t.value),
        _ => false,
    }
}

/// A strict `YYYY-MM-DD` calendar-date check (shape + real month/day with
/// per-month day counts and leap years, `year >= 1970`), so `2026-13-99`,
/// `2026-02-30` and `1969-12-31` are all flagged as malformed. The shared
/// validation path for date-bearing config and frontmatter fields (FR-007).
pub fn validate_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 year = u32::from(b[0] - b'0') * 1000
        + u32::from(b[1] - b'0') * 100
        + u32::from(b[2] - b'0') * 10
        + u32::from(b[3] - b'0');
    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');
    if year < 1970 || !(1..=12).contains(&month) {
        return false;
    }
    day >= 1 && day <= days_in_month(year, month)
}

/// Real day count for a Gregorian month (leap-aware).
fn days_in_month(year: u32, month: u32) -> u32 {
    match month {
        1 | 3 | 5 | 7 | 8 | 10 | 12 => 31,
        4 | 6 | 9 | 11 => 30,
        2 => {
            let leap = (year % 4 == 0 && year % 100 != 0) || year % 400 == 0;
            if leap { 29 } else { 28 }
        }
        _ => 0,
    }
}

/// Message-bearing [`validate_date`] for surfaces that want to explain what
/// was wrong. The error names the offending value; callers add field context.
fn validate_date_msg(s: &str) -> std::result::Result<(), String> {
    if validate_date(s) {
        Ok(())
    } else {
        Err(format!(
            "`{s}` is not a `YYYY-MM-DD` calendar date (year >= 1970)"
        ))
    }
}

/// Validate a `(start_date, due_date)` pair for an issue (FR-007, C-002).
///
/// Each present date must parse as a calendar date; when both are present,
/// `due_date >= start_date` must hold. Returns a config-class error naming
/// both fields on an ordering violation, matching
/// `contracts/file-format-planning.md`.
pub fn validate_issue_dates(start_date: Option<&str>, due_date: Option<&str>) -> Result<()> {
    if let Some(start) = start_date
        && validate_date_msg(start).is_err()
    {
        return Err(TixError::Config(format!(
            "start_date {start:?} must be a `YYYY-MM-DD` calendar date"
        )));
    }
    if let Some(due) = due_date {
        if validate_date_msg(due).is_err() {
            return Err(TixError::Config(format!(
                "due_date {due:?} must be a `YYYY-MM-DD` calendar date"
            )));
        }
        if let Some(start) = start_date
            && !date_lte(start, due)
        {
            return Err(TixError::Config(format!(
                "due_date {due:?} must be `>= start_date` {start:?}"
            )));
        }
    }
    Ok(())
}

/// Chronological order for two validated `YYYY-MM-DD` strings.
///
/// Plain lexicographic comparison equals chronological order for the
/// formats [`validate_date`] accepts (fixed-width zero-padded fields), so
/// later WPs (overdue, filters, cascade) reuse identical ordering logic.
pub fn date_lte(a: &str, b: &str) -> bool {
    a <= b
}

/// Convert days since the Unix epoch to a `(year, month, day)` civil date
/// (Howard Hinnant's `civil_from_days`).
fn civil_from_days(z: i64) -> (i64, u32, u32) {
    let z = z + 719_468;
    let era = if z >= 0 { z } else { z - 146_096 } / 146_097;
    let doe = z - era * 146_097; // [0, 146096]
    let yoe = (doe - doe / 1_460 + doe / 36_524 - doe / 146_096) / 365; // [0, 399]
    let y = yoe + era * 400;
    let doy = doe - (365 * yoe + yoe / 4 - yoe / 100); // [0, 365]
    let mp = (5 * doy + 2) / 153; // [0, 11]
    let d = (doy - (153 * mp + 2) / 5 + 1) as u32; // [1, 31]
    let m = if mp < 10 { mp + 3 } else { mp - 9 } as u32; // [1, 12]
    (if m <= 2 { y + 1 } else { y }, m, d)
}

/// Parse a `### <author> (<YYYY-MM-DD>)` comment head into its parts.
///
/// The date is the trailing `(<YYYY-MM-DD>)` group; the author is everything
/// before it. A head without a well-formed trailing date still yields the
/// author (the date defaults to `""`), so malformed heads survive the parse
/// without crashing (`contracts/comments-format.md` rule "preserve verbatim").
fn parse_comment_head(after: &str) -> (String, String) {
    let (author, date) = match after.len() {
        len if len >= 12 && after.as_bytes()[len - 1] == b')' => {
            let close = len - 1;
            let open = after[..close].rfind(" (");
            match open {
                Some(open) => {
                    let maybe_date = &after[open + 2..close];
                    if maybe_date.len() == 10
                        && maybe_date.as_bytes()[4] == b'-'
                        && maybe_date.as_bytes()[7] == b'-'
                    {
                        (&after[..open], maybe_date)
                    } else {
                        (after, "")
                    }
                }
                None => (after, ""),
            }
        }
        _ => (after, ""),
    };
    (author.trim_end().to_string(), date.to_string())
}

/// A single issue comment, parsed from the `## Comments` body section.
///
/// Mirrors `contracts/comments-format.md`: each comment is a level-3 heading
/// `### <author> (<YYYY-MM-DD>)` followed by free text up to the next `###`
/// head or end of file. Blank lines inside `text` are preserved verbatim
/// (NFR-005); the model never normalizes hand-authored content.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Comment {
    pub author: String,
    pub date: String,
    pub text: String,
}

/// An issue: one markdown file with YAML frontmatter.
///
/// Field order and names are frozen by `contracts/file-format.md` FR-012.
/// `number` is a stored frontmatter key assigned by the storage layer at
/// create time by scanning the live issue set (issue #52; previously from
/// `tix.toml` `[state].next_number`, which is no longer persisted); this
/// module models and validates it but never allocates. `filename` and
/// `body` are memory-only (`#[serde(skip)]`).
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct Issue {
    /// Internal sequential id, never reused (C-007).
    pub number: u32,
    /// Human title; drives the slug in the filename.
    #[serde(default)]
    pub title: String,
    /// Column (board workflow stage) the issue sits in.
    #[serde(default)]
    pub status: Status,
    /// Free-form labels.
    #[serde(default)]
    pub labels: Labels,
    /// Priority; defaults to `medium`.
    #[serde(default)]
    pub priority: Priority,
    /// Assignee. In memory: bare handle (`alice`) or full `Name <email>`
    /// identity, verbatim. Files serialize per the rule documented on
    /// [`de_assignee`]/[`se_assignee`] (legacy `@alice` tolerated on read).
    #[serde(
        default,
        skip_serializing_if = "Option::is_none",
        deserialize_with = "de_assignee",
        serialize_with = "se_assignee"
    )]
    pub assignee: Option<String>,
    /// Free-form estimate (e.g. `2`, `S`, `3d`); default `null`.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub estimate: Option<String>,
    /// Git commit recorded when moved to `done` (FR-026).
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub commit: Option<String>,
    /// Remote GitLab issue id after export/import.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub gitlab_id: Option<String>,
    /// Remote Linear issue id after export/import.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub linear_id: Option<String>,
    /// Spec Kitty mission slug (e.g. `v3-archive-review`) this issue belongs to, if any.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub mission: Option<String>,
    /// Spec Kitty work package id (e.g. `WP01`) this issue links to, if any.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub task: Option<String>,
    /// Typed relationships to other issues, replacing the retired generic
    /// `links: Vec<String>` bag (C-001).
    ///
    /// Each entry is a [`Relationship`]: a typed, directed edge with a stable
    /// id and a human-readable display name. The vocabulary is the closed
    /// four-kind set — `blocks`, `blocked_by`, `duplicates`, `related` — and
    /// the kind that reaches disk is always the canonical one, so `blocked_by`
    /// is normalized into `blocks` before it is stored and a pair is never
    /// stored twice. See [`crate::relationships`] for the direction
    /// convention, which this field does not restate.
    ///
    /// Files without the key parse (`default`). An empty list is **omitted**
    /// rather than written as `[]`: the typed field is new, so there is no
    /// legacy expectation to preserve for it, and omitting it keeps an issue
    /// with no relationships free of a meaningless key.
    #[serde(default, skip_serializing_if = "Vec::is_empty")]
    pub relationships: Vec<Relationship>,
    /// This issue's **decomposition parent**, as an issue number.
    ///
    /// A parent is a decomposition — "this work is part of that" — and is a
    /// **distinct concept from a typed dependency edge**: `blocks` is a scheduling
    /// dependency, and conflating the two in one field is prohibited (C-007). It is
    /// also distinct from the `mission` and `task` Spec Kitty pointer fields, which
    /// are governance metadata and not a hierarchy (research R2).
    ///
    /// **At most one parent** (C-001). The type is a single `Option<u32>`, so two
    /// parents are not expressible; the field is not a list and no code path appends
    /// to it. Child membership is *derived* by inverting this pointer and is never
    /// stored on the parent (C-003) — see [`crate::hierarchy`], which owns that
    /// derivation.
    ///
    /// `#[serde(default)]` so every legacy file without the key parses unchanged,
    /// and skipped when absent so a file that never had a parent serialises
    /// byte-identically to its pre-mission form.
    ///
    /// The value is an issue **number**, not a selector: a selector may be a title
    /// fragment, and a title is not stable, so a stored title would silently break
    /// every child when the parent was retitled. Selectors are the *input* to
    /// `tix parent set`; the number is what lands in the file.
    ///
    /// A reference whose target no longer exists is **kept**, not cleared: it becomes
    /// the diagnosed `UnresolvedParentState` so it can be repaired.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub parent: Option<u32>,
    /// Repo-relative file paths of the governing specification (e.g.
    /// `openspec/changes/<name>/specs/<cap>/spec.md`). Default `[]`.
    #[serde(default)]
    pub files: Vec<String>,
    /// Planning cycle names this issue belongs to (FR-005). Each entry must
    /// name a `[cycles.<name>]` config entry. `Option` so legacy files lack
    /// the key entirely; serialized only when non-empty.
    #[serde(
        default,
        skip_serializing_if = "skip_planning_list",
        serialize_with = "se_planning_list"
    )]
    pub cycles: Option<Vec<String>>,
    /// Milestone names this issue belongs to (FR-005). Each entry must name a
    /// milestone file in `milestones_dir`. Serialized only when non-empty.
    #[serde(
        default,
        skip_serializing_if = "skip_planning_list",
        serialize_with = "se_planning_list"
    )]
    pub milestones: Option<Vec<String>>,
    /// Advisory reviewer identity (FR-006), free-form like `assignee`.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub reviewer: Option<String>,
    /// Planning start date, `YYYY-MM-DD` (FR-006).
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub start_date: Option<String>,
    /// Planning due date, `YYYY-MM-DD`; must be `>= start_date` (FR-007).
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub due_date: Option<String>,
    /// When the issue most recently entered the triage inbox, RFC 3339 UTC
    /// (`2026-09-30T09:12:44Z`), if it ever did.
    ///
    /// Mission `triage-inbox-review-flow-01M3C600`, data-model §2. Queue age is
    /// measured from here and from nothing else: `updated` moves on every edit and
    /// `created` predates the queue, so either would report a wait the issue did
    /// not have (research R4). Read it through [`crate::triage::entry_time`],
    /// which is the one documented fallback for an issue that has none.
    ///
    /// Optional and skipped when absent, so a repository that predates triage
    /// round-trips byte-identically instead of gaining a key on every file (C-001).
    ///
    /// Refreshed on re-entry, not merged — a defer restarts the wait (data-model
    /// §5) — and deliberately **not** stamped on every edit, which would turn it
    /// into a second `updated`.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub entered_at: Option<String>,
    /// Where the issue's current entry came from: `new`, `import`, `defer`, or
    /// any other origin the caller documents (data-model §2, `source`).
    ///
    /// Optional for the same reason as [`Self::entered_at`]. It describes *this*
    /// entry rather than the issue's permanent provenance, so it is rewritten
    /// whenever the issue re-enters.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub source: Option<String>,
    /// Creation date, `YYYY-MM-DD`, equals the filename date prefix.
    #[serde(default = "today")]
    pub created: String,
    /// Last-mutation date, `YYYY-MM-DD`.
    #[serde(default = "today")]
    pub updated: String,
    /// Issue file name — path relative to the issues dir (e.g. `alice/2026-05-10-fix-login.md` for grouped layout, or `2026-05-10-fix-login.md` for legacy flat files per C-010).
    #[serde(skip)]
    pub filename: String,
    /// Raw markdown body; memory-only, preserved verbatim.
    #[serde(skip)]
    pub body: String,
    /// The append-only `## History` section, parsed out of the body on read and
    /// re-rendered on write; memory-only like [`Issue::body`].
    ///
    /// The section is *stripped* from `body` during parsing, so `body` holds
    /// only the issue's own prose and the two never disagree. Rendering puts
    /// the section back immediately after the frontmatter, the position
    /// `data-model.md` §8 pins.
    #[serde(skip)]
    pub history: History,
    /// Whether the source frontmatter still carries a legacy `links:` key.
    ///
    /// The key is never read as typed data (C-001) and never migrated; it is
    /// recorded here so the service can report it as a
    /// `retired_links_key` validation result. Memory-only, like the rest of the
    /// file-surface state.
    #[serde(skip)]
    pub has_retired_links_key: bool,
    /// Top-level frontmatter keys this model does not know, as `key -> raw block`.
    ///
    /// #1361. Before this, an unknown key was dropped on every write: `Issue`
    /// deserialises a fixed set of keys and `frontmatter` re-emits exactly those, so
    /// a key `tix` does not recognise was deleted by the next unrelated `tix edit` —
    /// silently, because `tix lint` had no unknown-key rule at all.
    ///
    /// The value is the **raw source block**, not a parsed value, and that is the
    /// whole design. Re-serialising a parsed value would re-quote and re-indent it, so
    /// a key that was written one way would come back another way and every write
    /// would produce a spurious diff. Splicing the original bytes back in makes the
    /// round trip byte-stable for content this model has no opinion about.
    ///
    /// A `BTreeMap` rather than a `HashMap` so the render order is deterministic:
    /// `HashMap` iteration order would reshuffle these keys on every write and
    /// produce a diff in a file that had not changed.
    ///
    /// `Eq` is preserved (unlike `serde_yaml::Value`, which is not `Eq`), and the
    /// blocks are plain `String`s, so `Issue` keeps every one of its derives.
    ///
    /// The retired `links:` key is **excluded** — see [`Issue::parse`]. It is
    /// recognised, reported by its own finding, and dropped (C-001); round-tripping it
    /// would silently undo the typed-relationship migration.
    #[serde(skip)]
    pub extra_frontmatter: BTreeMap<String, String>,
    /// Whether the issue is archived (issue #1492).
    ///
    /// The **filename is the marker**, never a frontmatter field: an archived
    /// issue's markdown file is dot-prefixed in place
    /// (`docs/issues/<assignee>/.<file>.md`). This field is therefore derived
    /// state, not data — it mirrors the leading dot of [`Issue::filename`] and
    /// is set by [`crate::storage::Store::list_all`], never written to disk.
    /// `#[serde(skip)]` keeps it out of the frontmatter, and it defaults to
    /// `false` for every parse.
    #[serde(skip)]
    pub archived: bool,
}

// ---------------------------------------------------------------------------
// Append-only History section (WP02 T007)
// ---------------------------------------------------------------------------

/// The heading that introduces the History section, per `data-model.md` §8.
///
/// The section is recognised by a line whose trimmed content is exactly this —
/// nothing else introduces it.
pub const HISTORY_HEADING: &str = "## History";

/// The separator between the four fields of a rendered History entry
/// (U+2014 em dash).
const HISTORY_SEP: char = '\u{2014}';

/// What kind of mutation a [`HistoryEvent`] records.
///
/// Closed set per `data-model.md` §8. `blocked_move_acknowledged` is defined
/// here so WP04 consumes it without editing this file.
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub enum HistoryEventKind {
    /// A relationship was created.
    RelationshipAdded,
    /// A relationship was removed.
    RelationshipRemoved,
    /// A relationship was retargeted or re-kinded.
    RelationshipRetargeted,
    /// A move toward completion was confirmed while blockers were open.
    BlockedMoveAcknowledged,
    /// An issue gained its first parent.
    ParentSet,
    /// An existing parent reference was repointed.
    ParentChanged,
    /// A parent reference was removed.
    ParentCleared,
    /// A status change was accepted by the transition service and applied.
    ///
    /// WP03. The name says *accepted* because acceptance is the only outcome that
    /// reaches the log: a rejected transition is completely inert and appends
    /// nothing (contract §3.5), and a re-applied transition is idempotent and also
    /// appends nothing. So the presence of this kind in a file **is** the record
    /// that the change was accepted — a name like `transition_attempted` would
    /// imply rejections are logged here too, and they are not.
    TransitionAccepted,
    /// A triage inbox issue was accepted into the normal workflow.
    ///
    /// WP03 (mission `triage-inbox-review-flow-01M3C600`). Four triage kinds
    /// rather than one `triage_decided` for the reason the parent kinds are
    /// separate: the *decision* is the thing an audit reader is looking for, and
    /// `triage_decided — decline …` makes them read the detail to learn which of
    /// four mutually exclusive actions was taken. They extend the one #169 log
    /// (research R5) — there is no second log and no triage-specific section.
    ///
    /// Only **applied** decisions reach the log: a refusal appends nothing and a
    /// repeated identical decision is a no-op (contract `decision-history.md`),
    /// exactly as with [`HistoryEventKind::TransitionAccepted`].
    TriageAccepted,
    /// A triage inbox issue was declined: moved to the configured declined
    /// column, never deleted or archived (C-003).
    TriageDeclined,
    /// A triage inbox issue was marked a duplicate of another issue.
    ///
    /// The event records the decision; the typed `duplicates` edge is the
    /// [`HistoryEventKind::RelationshipAdded`] entry. One decision, two kinds
    /// *only* because the relation already has its own kind and inventing a
    /// second spelling for it would leave the graph and the log disagreeing.
    TriageDuplicated,
    /// A triage inbox issue was deferred: it stays in the inbox with a
    /// refreshed entry time (data-model §5).
    TriageDeferred,
    /// The issue file was archived: renamed in place to a dot-prefixed name
    /// (issue #1492).
    Archived,
    /// An archived issue was restored to its active name (issue #1492).
    Restored,
}

impl HistoryEventKind {
    /// The canonical snake_case spelling written to the file.
    pub fn as_str(self) -> &'static str {
        match self {
            HistoryEventKind::RelationshipAdded => "relationship_added",
            HistoryEventKind::RelationshipRemoved => "relationship_removed",
            HistoryEventKind::RelationshipRetargeted => "relationship_retargeted",
            HistoryEventKind::BlockedMoveAcknowledged => "blocked_move_acknowledged",
            // Three dedicated kinds rather than one generic `parent_changed` (research
            // open question 2, resolved in `data-model.md` §6.1): "set" and "cleared" are
            // not changes *of* a parent, they are its beginning and its end, and
            // collapsing them leaves the audit trail unable to distinguish "gained a
            // parent" from "moved to a different one" without reconstructing state from
            // the detail text.
            HistoryEventKind::ParentSet => "parent_set",
            HistoryEventKind::ParentChanged => "parent_changed",
            HistoryEventKind::ParentCleared => "parent_cleared",
            HistoryEventKind::TransitionAccepted => "transition_accepted",
            HistoryEventKind::TriageAccepted => "triage_accepted",
            HistoryEventKind::TriageDeclined => "triage_declined",
            HistoryEventKind::TriageDuplicated => "triage_duplicated",
            HistoryEventKind::TriageDeferred => "triage_deferred",
            HistoryEventKind::Archived => "archived",
            HistoryEventKind::Restored => "restored",
        }
    }

    /// Whether this kind records a triage decision (WP03's four).
    ///
    /// The membership test the decision service's idempotence check and
    /// `not_in_triage`/`already_decided` split both ask, so "is this a triage
    /// decision?" has one answer rather than a four-way `matches!` at every
    /// call site — where a fifth kind would be forgotten first.
    #[must_use]
    pub fn is_triage_decision(self) -> bool {
        matches!(
            self,
            HistoryEventKind::TriageAccepted
                | HistoryEventKind::TriageDeclined
                | HistoryEventKind::TriageDuplicated
                | HistoryEventKind::TriageDeferred
        )
    }
}

impl fmt::Display for HistoryEventKind {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        f.write_str(self.as_str())
    }
}

impl FromStr for HistoryEventKind {
    type Err = TixError;

    /// Parse a kind spelling. An unknown kind is **rejected**, never coerced
    /// to a default: a silently reclassified audit entry is worse than a
    /// parse failure, and the field is part of an append-only log.
    fn from_str(s: &str) -> Result<Self> {
        match s {
            "relationship_added" => Ok(HistoryEventKind::RelationshipAdded),
            "relationship_removed" => Ok(HistoryEventKind::RelationshipRemoved),
            "relationship_retargeted" => Ok(HistoryEventKind::RelationshipRetargeted),
            "blocked_move_acknowledged" => Ok(HistoryEventKind::BlockedMoveAcknowledged),
            "parent_set" => Ok(HistoryEventKind::ParentSet),
            "parent_changed" => Ok(HistoryEventKind::ParentChanged),
            "parent_cleared" => Ok(HistoryEventKind::ParentCleared),
            "transition_accepted" => Ok(HistoryEventKind::TransitionAccepted),
            "triage_accepted" => Ok(HistoryEventKind::TriageAccepted),
            "triage_declined" => Ok(HistoryEventKind::TriageDeclined),
            "triage_duplicated" => Ok(HistoryEventKind::TriageDuplicated),
            "triage_deferred" => Ok(HistoryEventKind::TriageDeferred),
            "archived" => Ok(HistoryEventKind::Archived),
            "restored" => Ok(HistoryEventKind::Restored),
            other => Err(TixError::Parse(format!(
                "unknown History event kind `{other}`"
            ))),
        }
    }
}

impl HistoryEventKind {
    /// The model kind for a hierarchy event.
    ///
    /// Vocabulary translation, not a rule: the service decides *which* transition
    /// happened, and this maps that decision onto the log's spelling. It lives on
    /// the type that owns the log rather than at each call site, because three
    /// callers need it — the storage write path, the `--parent` edit flag, and MCP's
    /// `update_issue` in WP07 — and a mapping repeated three times is one that will
    /// be completed three times and fixed once.
    pub fn from_parent(kind: crate::hierarchy::ParentEventKind) -> Self {
        match kind {
            crate::hierarchy::ParentEventKind::ParentSet => HistoryEventKind::ParentSet,
            crate::hierarchy::ParentEventKind::ParentChanged => HistoryEventKind::ParentChanged,
            crate::hierarchy::ParentEventKind::ParentCleared => HistoryEventKind::ParentCleared,
        }
    }
}

/// The auditable record of one **accepted** status transition (`data-model.md`
/// §5, WP03 T012).
///
/// ## Why this is not a `HistoryEventKind` alone
///
/// A [`HistoryEvent`] has exactly four fields — timestamp, actor, kind, detail —
/// and the on-disk line format is fixed at four by the U+2014 split in
/// [`HistoryEvent::parse`]. `data-model.md` §5 wants seven. The three surplus
/// fields therefore live in `detail`, and this struct is what converts between
/// the two: [`Self::to_event`] and [`Self::from_event`] are the only places that
/// encoding is written or read.
///
/// ## The `detail` grammar
///
/// ```text
/// detail := head [ "; override: " reason ]
/// head   := from " -> " to " (" from_category " -> " to_category ")"
/// ```
///
/// e.g. `todo -> in progress (unstarted -> active)`, or
/// `done -> in progress (completed -> active); override: mis-filed`.
///
/// **It is a grammar, not prose**, and that is a load-bearing choice rather than a
/// tidiness one. WP06's `lint_direct_edits` has to compare a file's *current*
/// `status` against the column its last accepted transition landed in; recovering
/// that column out of free-form English is guesswork, and a heuristic that
/// misreads it produces a lint result nobody can trust. A fixed grammar makes the
/// comparison exact, and makes "this detail is not a transition record"
/// detectable rather than a guess.
///
/// The parse is deliberately anchored rather than free-floating: the override is
/// split off at the first `") ; override: "`, and the categories are taken from
/// between the last `" ("` and the closing `")"`, so a **reason** may contain the
/// bare `"-> "` and the bare `"; override: "` without disturbing the fields. That
/// anchoring is the whole reason the marker includes the paren — see
/// [`OVERRIDE_MARKER`].
///
/// ## Known limit: a column name containing `" -> "`
///
/// The two column names are separated by the first `" -> "`, and nothing escapes
/// it, so a column literally named `a -> b` would render as
/// `a -> b -> in progress (unstarted -> active)` and parse back with `from == "a"`.
/// The categories still parse correctly, and the *categories* are what an audit
/// reader wants, but a WP06 that recovers `to` from a file would recover `b`.
///
/// This is inherent to a delimiter-based grammar, not a bug in this
/// implementation: any unescaped separator has a name it cannot represent, and
/// escaping would put backslashes into a line a human reads. A board column named
/// with an embedded arrow is not a realistic configuration, so the limit is
/// documented and pinned by a test rather than designed around. WP06 reads `to`
/// from these records, so it inherits the assumption and should not be surprised
/// by it.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct TransitionDecision {
    /// When the transition was accepted, RFC 3339 — the same shape and clock as
    /// every other History entry.
    pub timestamp: String,
    /// Who performed it.
    ///
    /// A plain `String`, carried through to [`HistoryEvent::actor`] untouched, so
    /// identity handling is the History log's existing one: a bare handle or a full
    /// `Name <email>`, with a legacy `@alice` tolerated on read. This struct
    /// re-implements none of that and cannot drift from it.
    pub actor: String,
    /// The column the issue was in, as stored.
    pub from: String,
    /// The column it was moved to, as stored.
    pub to: String,
    /// The category `from` resolved to, recorded for auditability.
    ///
    /// **Derived, never stored on the issue and never re-derived on read** — that
    /// is the point. If a board's `category_map` is later edited, the historical
    /// record still says what the category *was* when the move was accepted, which
    /// is the only way the audit trail stays true.
    pub from_category: String,
    /// The category `to` resolved to, recorded for the same reason.
    pub to_category: String,
    /// Why a policy rule was overridden, when one was.
    ///
    /// Required to be `Some` for an override to exist at all: the service refuses a
    /// bare "I did it anyway" (see `TransitionOverride`), so this is never `Some`
    /// with an empty or blank reason.
    pub override_reason: Option<String>,
}

/// The marker separating the transition record from its override reason.
///
/// **Includes the closing paren**, and that is load-bearing rather than
/// cosmetic. The obvious marker is `"; override: "`, split with `rsplit_once` so a
/// reason containing the marker survives — and it does not: the *last* occurrence
/// of a bare `"; override: "` inside a reason like `typo; override: actually…` is
/// the one that splits, which leaves the head ending in `; override: typo` and no
/// closing paren, so the parse fails outright. WP03's first version did exactly
/// that and its own test caught it.
///
/// Anchoring the marker to the `)` that closes the head makes the split
/// **unambiguous in both directions**: the head is the only `)` followed by the
/// marker, so `split_once` at the *first* occurrence always takes the right one.
/// A reason may therefore contain the bare marker, a bare `")"`, and even the full
/// anchored marker, and still round-trip.
///
/// That last case is worth spelling out because WP03's first two attempts both got
/// it wrong in opposite directions. `rsplit_once` on the *bare* marker took the
/// last occurrence, so a reason containing `"; override: "` truncated and the parse
/// failed outright. `rsplit_once` on the *anchored* marker would be no better: for
/// a reason containing `"); override: "` the last occurrence is the one inside the
/// reason. `split_once` on the anchored marker is the only combination of the two
/// that is correct, and `t012_a_reason_containing_the_anchored_marker_survives`
/// exists to keep it that way.
const OVERRIDE_MARKER: &str = "); override: ";
/// The marker opening the bracketed category pair.
const CATEGORY_OPEN: &str = " (";
/// The arrow between two names in the `detail` grammar.
const DETAIL_ARROW: &str = " -> ";

impl TransitionDecision {
    /// Build a decision from an accepted transition.
    pub fn new(
        timestamp: impl Into<String>,
        actor: impl Into<String>,
        from: impl Into<String>,
        to: impl Into<String>,
        from_category: impl Into<String>,
        to_category: impl Into<String>,
        override_reason: Option<String>,
    ) -> Self {
        Self {
            timestamp: timestamp.into(),
            actor: actor.into(),
            from: from.into(),
            to: to.into(),
            from_category: from_category.into(),
            to_category: to_category.into(),
            override_reason,
        }
    }

    /// Render to the single `detail` string the History line carries.
    pub fn detail(&self) -> String {
        let mut out = format!(
            "{from}{arrow}{to} ({fc}{arrow}{tc})",
            from = self.from,
            to = self.to,
            fc = self.from_category,
            tc = self.to_category,
            arrow = DETAIL_ARROW,
        );
        // A blank reason renders as no marker at all rather than as an empty field,
        // so `from_event` can never come back with `Some("")` — the same reasoning
        // that makes a blank actor render as an empty field rather than being
        // dropped, applied in the other direction.
        if let Some(reason) = self.override_reason.as_deref().map(str::trim) {
            if !reason.is_empty() {
                // The head already ends in the `)` the marker expects, so the
                // marker supplies the semicolon and the space only. Writing the
                // marker as a whole and *also* leaving the head's `)` is what
                // produced `...active)); override:` on the first pass.
                out.push_str(OVERRIDE_MARKER.trim_start_matches(')'));
                out.push_str(reason);
            }
        }
        out
    }

    /// Recover a decision from a History entry.
    ///
    /// `Err` when `event` is not a `transition_accepted` entry or its `detail` does
    /// not match the grammar — never a partially-filled struct. A caller can
    /// therefore treat a successful parse as "this file records a transition to
    /// `to`", which is exactly the question WP06 asks.
    pub fn from_event(event: &HistoryEvent) -> Result<Self> {
        if event.kind != HistoryEventKind::TransitionAccepted {
            return Err(TixError::Parse(format!(
                "History entry is a `{}`, not a `transition_accepted`",
                event.kind
            )));
        }
        let malformed =
            |what: &str| TixError::Parse(format!("transition record {what}: `{}`", event.detail));
        // The reason is split off at the FIRST anchored marker: the head is the
        // only `)` followed by `; override: `, so a reason containing the bare
        // marker survives. See `OVERRIDE_MARKER`.
        let (head, override_reason) = match event.detail.split_once(OVERRIDE_MARKER) {
            Some((head, reason)) => (head, Some(reason.trim().to_string())),
            None => (event.detail.as_str(), None),
        };
        // The marker *includes* the head's closing paren, so put it back and let
        // the grammar below see one shape whether or not a reason was present. The
        // alternative — two separate shapes to parse — is how the two halves of a
        // grammar drift apart.
        let head = match &override_reason {
            Some(_) => format!("{head})"),
            None => head.to_string(),
        };
        let head = head.as_str();
        let (from, rest) = head
            .split_once(DETAIL_ARROW)
            .ok_or_else(|| malformed("has no `from -> to`"))?;
        let rest = rest
            .strip_suffix(')')
            .ok_or_else(|| malformed("has no closing `)` after the categories"))?;
        let (to, categories) = rest
            .split_once(CATEGORY_OPEN)
            .ok_or_else(|| malformed("has no ` (` before the categories"))?;
        let (from_category, to_category) = categories
            .split_once(DETAIL_ARROW)
            .ok_or_else(|| malformed("has no `from_category -> to_category`"))?;
        for (name, value) in [
            ("from", from),
            ("to", to),
            ("from_category", from_category),
            ("to_category", to_category),
        ] {
            if value.trim().is_empty() {
                return Err(malformed(name));
            }
        }
        Ok(Self {
            timestamp: event.timestamp.clone(),
            actor: event.actor.clone(),
            from: from.to_string(),
            to: to.to_string(),
            from_category: from_category.to_string(),
            to_category: to_category.to_string(),
            override_reason,
        })
    }

    /// The History entry to append for this decision.
    pub fn to_event(&self) -> HistoryEvent {
        HistoryEvent::new(
            self.timestamp.clone(),
            self.actor.clone(),
            HistoryEventKind::TransitionAccepted,
            self.detail(),
        )
    }

    /// Whether this decision is a same-column no-op record.
    ///
    /// The seam never appends one — idempotency is decided by
    /// `TransitionOutcome::Accepted { changed: false }` — but a **hand-edited** file
    /// can contain one, and WP06's `lint_direct_edits` needs to tell that apart
    /// from a real move rather than reading it as "status became todo".
    pub fn is_no_op(&self) -> bool {
        self.from == self.to
    }
}

/// One recorded mutation, and one line of the `## History` section.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct HistoryEvent {
    /// When the mutation happened, RFC 3339.
    pub timestamp: String,
    /// Who performed it. Handled like [`Issue::assignee`]: a bare handle or a
    /// full identity, with a legacy `@alice` tolerated on read.
    pub actor: String,
    /// What kind of mutation this was.
    pub kind: HistoryEventKind,
    /// The field-aware description shown in detail views.
    pub detail: String,
}

impl HistoryEvent {
    /// Build an event.
    pub fn new(
        timestamp: impl Into<String>,
        actor: impl Into<String>,
        kind: HistoryEventKind,
        detail: impl Into<String>,
    ) -> Self {
        Self {
            timestamp: timestamp.into(),
            actor: actor.into(),
            kind,
            detail: detail.into(),
        }
    }

    /// The one-line form written to the file:
    /// `- <timestamp> — <actor> — <kind> — <detail>`.
    ///
    /// Four fields, not the three an earlier draft of `data-model.md` §8
    /// specified: a three-field form cannot carry `actor`, so the struct field
    /// would not survive a round trip and the audit trail would silently lose
    /// who performed each mutation. `data-model.md` §8 is corrected to match on
    /// the base and mission branches — it is a protected path, so the fix is not
    /// in this lane. **Read §8 there, not this comment**, if the two ever
    /// disagree.
    ///
    /// A blank actor renders as an empty field rather than being dropped, so
    /// the field count stays fixed and parsing never has to guess.
    fn render(&self) -> String {
        format!(
            "- {t} {h} {a} {h} {k} {h} {d}",
            t = self.timestamp.trim(),
            h = HISTORY_SEP,
            a = self.actor.trim(),
            k = self.kind.as_str(),
            d = self.detail
        )
    }

    /// Parse one rendered list item.
    ///
    /// The four fields are split on the **first three** separators only, so a
    /// detail or actor containing an em dash survives intact.
    fn parse(line: &str) -> Result<Self> {
        let item = line
            .trim()
            .strip_prefix("- ")
            .ok_or_else(|| TixError::Parse(format!("History entry is not a list item: {line}")))?;
        // Split on the first three separators only, so a detail containing an
        // em dash survives intact. Fields are read **positionally** rather than
        // as "the next non-empty value": the actor may legitimately be blank,
        // which is exactly what `render` writes when no identity was recorded,
        // so demanding a non-empty value there would make the writer's own
        // output unparseable.
        let parts: Vec<&str> = item.splitn(4, HISTORY_SEP).collect();
        let missing =
            |what: &str| TixError::Parse(format!("History entry is missing `{what}`: {line}"));
        if parts.len() < 3 {
            return Err(missing("a field separator"));
        }
        let timestamp = parts[0].trim();
        if timestamp.is_empty() {
            return Err(missing("timestamp"));
        }
        let actor = parts[1].trim().to_string();
        let kind_text = parts[2].trim();
        if kind_text.is_empty() {
            return Err(missing("kind"));
        }
        let kind = kind_text.parse::<HistoryEventKind>()?;
        let detail = parts.get(3).copied().unwrap_or_default().trim().to_string();
        Ok(Self {
            timestamp: timestamp.to_string(),
            actor,
            kind,
            detail,
        })
    }
}

/// Whether a line is a **recognised** History entry attempt.
/// Whether a line is **shaped like a markdown list item**, regardless of whether
/// it is a valid entry.
///
/// Used to decide where an unrecognised line belongs: a list item inside a
/// History section is far more likely a hand-edited entry than prose, and the two
/// are not told apart by anything else, because the section has no terminator.
///
/// All four common markers count, not just `- `. This is not a widening of the
/// entry grammar — recognition still requires U+2014, so nothing here can be read
/// as a relationship. With `- ` alone, an entry written
/// `* 2026-01-01T00:00:00Z — a — kind — d` was treated as prose, so the section
/// ended, the heading was deleted, and the next append was rendered above it.
fn is_list_item(item: &str) -> bool {
    for marker in ["- ", "* ", "+ "] {
        if item.starts_with(marker) {
            return true;
        }
    }
    // An ordered marker: digits then `.` or `)` then a space. Bounded to two
    // digits, or `2026. Q1 goals` reads as an ordered-list item and a paragraph
    // of ordinary body prose gets retained in the audit log.
    let Some(dot) = item.find(['.', ')']) else {
        return false;
    };
    dot > 0
        && dot <= 2
        && item[..dot].bytes().all(|b| b.is_ascii_digit())
        && item[dot + 1..].starts_with(' ')
}

/// Whether a line is a **recognised** History entry attempt.
///
/// Recognition requires the U+2014 separator, and nothing else. That is
/// deliberately stricter than "looks like" one: a looser test here is not a
/// nicety, it decides whether a line is *moved into the audit log* or left in
/// the body. A timestamp-shaped heuristic accepted a body to-do item
/// (`- 2026-02-02 review the migration`) as an entry attempt, which emptied
/// `body` and made `tix lint` blame the author — the original M3 defect arriving
/// through a different door, while the documentation claimed the opposite.
///
/// A line that is genuinely an entry but was typed with ASCII hyphens is
/// therefore *not* recognised, and never becomes an event. It may still be
/// *retained* in the section; see [`looks_like_an_unrecognised_entry`].
pub fn looks_like_history_entry(line: &str) -> bool {
    let item = line.trim();
    let Some(rest) = item.strip_prefix("- ") else {
        return false;
    };
    rest.contains(HISTORY_SEP)
}

/// Whether a line looks like a History entry that was **not** recognised — an
/// entry hand-typed with ASCII hyphens instead of U+2014.
///
/// This is the shape test behind two decisions, and they use it differently:
///
/// - `parse_front` asks whether an ambiguous line is a hand-edited *entry* rather
///   than body prose. Being wrong in that direction costs the author's bytes,
///   because a demoted line is body text and a later `tix edit --body` destroys
///   it with no trace.
/// - `tix lint` asks whether to *report* a line, where a false positive costs a
///   user-visible diagnostic on every run and a false negative costs a silent
///   typo.
///
/// A mistyped entry has the whole shape of one: a date, then three further
/// hyphen-separated fields — `<timestamp> - <actor> - <kind> - <detail>`. Both
/// halves are required. An earlier version accepted any 10-character run of
/// digits, dashes and punctuation, which flagged `- 10:00-11:00 sync with infra`,
/// `- 1234567890 is the upstream ticket`, `+49-30-12345678` and `1.2.3.4.5.6`; a
/// later one required only three fields, which flagged `- 2026-01-01 - Bob -
/// Done`. Four fields is what the format actually specifies, so that is what is
/// required.
pub fn looks_like_an_unrecognised_entry(line: &str) -> bool {
    let Some(rest) = strip_list_marker(line) else {
        return false;
    };
    if rest.contains(HISTORY_SEP) {
        return false; // genuinely recognised; not this function's business
    }
    entry_shaped_by_ascii_hyphens(rest, STRICT_TAIL_FIELDS)
}

/// Tail fields required to call a line a mistyped entry when **reporting** it.
///
/// Four fields in total, because that is the form carrying a detail; the shorter
/// three-field form is indistinguishable from a dated changelog line
/// (`- 2026-01-01 - Bob - Done`), and `tix lint` exits non-zero on a finding.
const STRICT_TAIL_FIELDS: usize = 3;

/// Tail fields required to call a line an entry **attempt** when deciding whether
/// to retain it.
///
/// Two, i.e. three fields in total, which is the shortest form `HistoryEvent::parse`
/// accepts — `detail` is optional there. A retention test stricter than the
/// recognition path is backwards: a valid three-field entry would be demoted to
/// body text and then destroyed by the next `tix edit --body`, while the
/// malformed four-field one survived. Cycle 6 found that inversion.
const RETENTION_TAIL_FIELDS: usize = 2;

/// Whether a line is an **entry attempt**: a list item whose content is shaped
/// like an entry, under any accepted marker.
///
/// This is the parser's question, and it is deliberately looser than
/// [`looks_like_an_unrecognised_entry`]'s, because the two have opposite costs.
/// Retaining a line that turns out to be prose moves it into the audit log *and
/// says so*; demoting a line that turns out to be an entry puts it in the body,
/// where the next `tix edit --body` deletes it with no trace. Retention is the
/// recoverable direction, so the parser errs towards retaining and the
/// diagnostic errs towards silence.
/// Whether a line introduces the `## History` section, under the same
/// tolerance the parser applies: trailing spaces or tabs are ignored, leading
/// whitespace is not.
///
/// Public so `tix lint` can report a **misplaced** section using the parser's own
/// rule. A second copy of this test could disagree with the parser about which
/// lines are headings, and then lint would report violations that do not exist
/// while missing ones that do.
pub fn is_history_heading(line: &str) -> bool {
    line.trim_end_matches([' ', '\t']) == HISTORY_HEADING
}

fn is_entry_attempt(item: &str) -> bool {
    let Some(rest) = strip_list_marker(item) else {
        return false;
    };
    if rest.contains(HISTORY_SEP) {
        // Correct separators. A `*`- or `1.`-marked entry is not *recognised* —
        // both `looks_like_history_entry` and `HistoryEvent::parse` require the
        // `- ` prefix — but it is unmistakably an entry attempt, and treating it
        // as prose cost a well-formed audit line. Cycle 6 found this.
        return true;
    }
    entry_shaped_by_ascii_hyphens(rest, RETENTION_TAIL_FIELDS)
}

/// Whether `rest` is a plausible entry body whose separators were typed as ASCII
/// hyphens: a date, then at least `min_tail` further hyphen-separated fields.
///
/// The tail splits on hyphens with optional surrounding spaces rather than on the
/// literal `" - "`, because an entry typed with ASCII hyphens is often typed
/// without the spaces too: `- 2026-01-01T00:00:00Z-alice-kind-d`. An earlier
/// version required `" - "` and so missed the case this tool exists to catch,
/// while the recognition path trims after the marker and accepted the same
/// whitespace — the identical line was "not an entry" and never mentioned.
fn entry_shaped_by_ascii_hyphens(rest: &str, min_tail: usize) -> bool {
    let Some(tail) = strip_leading_timestamp(rest) else {
        return false;
    };
    let parts: Vec<&str> = tail
        .split('-')
        .map(str::trim)
        .filter(|p| !p.is_empty())
        .collect();
    if parts.len() < min_tail {
        return false;
    }
    // The field right after the timestamp is an actor or a kind, and neither is a
    // date. Rejecting a numeric one is what keeps a date *range* out:
    // `- 2026-01-01..2026-01-05 was the freeze` otherwise splits into three
    // parts and reads as a mistyped entry.
    !starts_like_a_date(parts[0])
}

/// Whether a token opens with a run of four or more digits — a year, a serial
/// number, a date. Used only to reject prose that would otherwise split into
/// enough hyphen-separated fields to look like an entry.
fn starts_like_a_date(token: &str) -> bool {
    let digits = token.bytes().take_while(u8::is_ascii_digit).count();
    digits >= 4
}

/// The text after any accepted list marker, with whatever whitespace a hand put
/// after it removed.
///
/// `HistoryEvent::parse` trims after the prefix too, so this must as well:
/// otherwise `-  2026-…—…` and `-\t2026-…—…` are recognised as real entries while
/// their ASCII-hyphen equivalents are silently unreported.
fn strip_list_marker(line: &str) -> Option<&str> {
    let item = line.trim();
    if is_list_item(item) {
        let space = item.find(char::is_whitespace).unwrap_or(item.len());
        return Some(item[space..].trim_start());
    }
    None
}

/// The remainder of a line whose first field begins with a plausible ISO date, or
/// `None` if it does not.
///
/// A date is the only field shape that separates a mistyped entry from an
/// ordinary dated bullet, and requiring it is what keeps `tix lint` quiet on
/// prose. Plausibility is deliberately not checked: rejecting a hand-typed
/// offset like `+02:00` would create a false negative in the one tool whose job
/// is to catch exactly this mistake, so implausible shapes are reported rather
/// than filtered.
fn strip_leading_timestamp(rest: &str) -> Option<&str> {
    let b = rest.as_bytes();
    if b.len() < 10
        || !b[0..4].iter().all(u8::is_ascii_digit)
        || b[4] != b'-'
        || !b[5..7].iter().all(u8::is_ascii_digit)
        || b[7] != b'-'
        || !b[8..10].iter().all(u8::is_ascii_digit)
    {
        return None;
    }
    let mut i = 10;
    if b.get(i) == Some(&b'T') {
        i += 1;
        while i < b.len() && (b[i].is_ascii_digit() || b[i] == b':' || b[i] == b'.') {
            i += 1;
        }
        match b.get(i) {
            Some(b'Z') => i += 1,
            // A `-` is a zone separator only when digits follow it; otherwise it
            // is the first field separator and the timestamp ended at the time.
            Some(&b'-') => {
                let mut j = i + 1;
                while j < b.len() && b[j].is_ascii_digit() {
                    j += 1;
                }
                if j > i + 1 {
                    i = j;
                }
            }
            Some(&b'+') => {
                i += 1;
                while i < b.len() && (b[i].is_ascii_digit() || b[i] == b':') {
                    i += 1;
                }
            }
            _ => {}
        }
    }
    // \`.\` too: a date *range* continues with \`..\`, and leaving it glued to
    // the next token makes \`- 2026-01-01..2026-01-05 was the freeze\` split into
    // three fields.
    Some(rest[i.min(rest.len())..].trim_start_matches(['-', ' ', '\t', '.']))
}

/// One line of the `## History` section.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(untagged)]
pub enum HistoryEntry {
    /// A well-formed event.
    Event(HistoryEvent),
    /// A line that could not be parsed, preserved **verbatim**.
    ///
    /// The section is documented as human-editable, so a hand edit will
    /// eventually produce a line this parser cannot read — ASCII hyphens
    /// instead of the U+2014 separators is the obvious one. Such a line is
    /// never dropped, never coerced, and never auto-repaired: it is carried
    /// through a read/write cycle unchanged and reported by `tix lint` as a
    /// `history` finding on that issue only. It must never fail the
    /// parse of the whole file, because one mistyped line in one issue would
    /// otherwise make every issue in the repository unreadable.
    Unparsed(String),
}

/// The append-only event log of an issue (`data-model.md` §8).
///
/// Append-only is a hard guarantee of this type: the only mutating operation
/// is [`History::append`]. There is deliberately no remove, no reorder, no
/// trim, and no compaction — those are out of scope, but the container is
/// shaped so a future retention policy has a seam.
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
pub struct History {
    /// Every line of the section, in file order, which is append order.
    ///
    /// Held as an ordered list of entries rather than a flat `Vec<HistoryEvent>`
    /// so an **unparseable** line keeps its position and its exact text. That
    /// is what lets the section round-trip byte-stably without ever dropping or
    /// repairing a line a human hand-edited.
    pub entries: Vec<HistoryEntry>,
}

impl History {
    /// An empty log.
    pub fn new() -> Self {
        Self::default()
    }

    /// Append one event. Never rewrites, reorders, or removes an existing line.
    pub fn append(&mut self, event: HistoryEvent) {
        self.entries.push(HistoryEntry::Event(event));
    }

    /// Append with the current UTC timestamp.
    pub fn append_now(&mut self, actor: &str, kind: HistoryEventKind, detail: &str) {
        self.append(HistoryEvent::new(utc_now_rfc3339(), actor, kind, detail));
    }

    /// The parsed events, in file order.
    ///
    /// Unparseable lines are excluded here; see [`History::unparsed`].
    pub fn events(&self) -> Vec<&HistoryEvent> {
        self.entries
            .iter()
            .filter_map(|e| match e {
                HistoryEntry::Event(event) => Some(event),
                HistoryEntry::Unparsed(_) => None,
            })
            .collect()
    }

    /// Lines that could not be parsed, verbatim and in file order.
    ///
    /// Non-empty means the section was hand-edited into a shape this parser
    /// does not accept. `tix lint` reports each as a `history`
    /// finding; nothing else treats it as an error.
    pub fn unparsed(&self) -> Vec<&str> {
        self.entries
            .iter()
            .filter_map(|e| match e {
                HistoryEntry::Event(_) => None,
                HistoryEntry::Unparsed(line) => Some(line.as_str()),
            })
            .collect()
    }

    /// Whether the section has no lines at all.
    pub fn is_empty(&self) -> bool {
        self.entries.is_empty()
    }

    /// How many lines the section holds, parsed or not.
    pub fn len(&self) -> usize {
        self.entries.len()
    }

    /// Render the section, including its heading and the blank line after it.
    ///
    /// An empty log renders to the empty string, so the section is *omitted*
    /// from the file entirely rather than written as an empty heading. An
    /// unparseable line is re-emitted exactly as it was read, in place, so a
    /// read/write cycle is byte-stable whether or not the section is clean.
    pub fn render(&self) -> String {
        if self.is_empty() {
            return String::new();
        }
        let mut out = String::from(HISTORY_HEADING);
        out.push_str("\n\n");
        for entry in &self.entries {
            match entry {
                HistoryEntry::Event(event) => {
                    out.push_str(&event.render());
                    out.push('\n');
                }
                HistoryEntry::Unparsed(line) => {
                    out.push_str(line);
                    out.push('\n');
                }
            }
        }
        out.push('\n');
        out
    }

    /// Parse a leading `## History` section, returning the log and whatever body
    /// text followed it.
    ///
    /// Only a section at the *front* of the body is recognised, matching the
    /// position `data-model.md` §8 pins (immediately after the frontmatter). A
    /// `## History` heading appearing later in the prose is left alone as
    /// ordinary body text.
    ///
    /// The section ends at the first line that is neither blank nor a list item,
    /// so a **blank line between entries does not end it** — a blank line inside
    /// a list is the most ordinary thing a human or a Markdown formatter
    /// produces, and terminating there would silently demote audit entries to
    /// body prose and reorder the log. Unparseable list items are kept verbatim
    /// rather than failing the file.
    pub fn parse_front(body: &str) -> (Self, String) {
        let lead = body.len() - body.trim_start_matches('\n').len();
        let trimmed = &body[lead..];
        let Some(rest) = trimmed.strip_prefix(HISTORY_HEADING) else {
            return (History::new(), body.to_string());
        };
        // The heading must be alone on its line — but trailing spaces and tabs on
        // it are still that line. Requiring a bare `\n` meant `## History   ` was
        // not recognised at all, and since the heading is then neither the
        // section nor body, the *first* relationship write rendered a second
        // `## History` above the original. Cycle 6 executed that; it had been
        // filed against #1299 as a CRLF-only consequence, which understated it —
        // a trailing space is a far likelier trigger than CRLF.
        let rest = rest.trim_start_matches([' ', '\t']);
        let Some(after_heading) = rest.strip_prefix('\n') else {
            return (History::new(), body.to_string());
        };
        let after_heading = after_heading.trim_start_matches('\n');

        let mut history = History::new();
        let mut offset = 0usize;
        for line in after_heading.split_inclusive('\n') {
            // `item` is the trimmed form used for *shape* decisions; `raw_line`
            // is what a preserved `Unparsed` entry stores, so a hand edit keeps
            // its own bytes.
            let raw_line = line;
            let item = raw_line.trim();
            if item.is_empty() {
                // A blank line *inside* the section is skipped, not treated as
                // the end of it. It is consumed, so the body that follows does
                // not start with stray blank lines.
                offset += line.len();
                continue;
            }
            if !looks_like_history_entry(item) {
                // The section has no end-of-section delimiter, so this line is
                // ambiguous: either body prose, or a hand-edited line *inside*
                // the section. The two are told apart by what follows.
                //
                // If every remaining non-blank line is entry-shaped, the
                // ambiguous line was a hand edit inside the section, so the
                // section continues and the line is kept verbatim as
                // `Unparsed` — nothing is dropped and the real events after it
                // stay events. If anything after it is not entry-shaped, this
                // is body prose and the section genuinely ends here.
                //
                // The alternative — absorbing any `- ` line — moved body content
                // into the audit log, deleted the blank line separating them on
                // the first write, and then had `tix lint` blaming the author's
                // prose as unparseable entries. Trigger was ordinary:
                // `tix edit --body "- step one\n- step two"`.
                // Collected rather than consumed with `next()` + `all()`: that
                // skips judging the *first* following line, and `all()` over an
                // empty remainder is vacuously true — so a section followed by a
                // single non-entry line was absorbed. Both mistakes are the same
                // one: judging nothing and concluding "all of it is entries".
                let following: Vec<&str> = after_heading[offset + line.len()..]
                    .lines()
                    .filter(|l| !l.trim().is_empty())
                    .collect();
                // Two different questions, and conflating them is what broke this
                // twice.
                //
                // *Does anything after this disqualify it as body prose?* When
                // nothing follows, nothing can — `all()` is then vacuously true
                // and that is the **correct** answer, not a bug. A guard
                // requiring a non-empty remainder (added to stop an earlier
                // vacuous-truth defect) made the last line in a section always
                // terminate it, which deleted the `## History` heading, demoted
                // the author's line into the body, and rendered the next append
                // *above* it — file order stopped being append order, and
                // `tix lint` said nothing because a demoted line is body prose
                // and the unparsed loop no longer sees it.
                //
                // *Is this line shaped like a list item at all?* That still
                // decides, at end of section, whether keeping it is defensible.
                // `- step one` at EOF is far more likely a hand-edited entry than
                // the only body content of an issue, and keeping it *discloses*
                // the problem. A bare paragraph at EOF is prose, full stop, and
                // absorbing it would pollute the audit log.
                // Three inputs, and which ones matter depends on whether
                // anything follows. Cycle 5 found this half-fixed: consulting
                // only `is_list_item`, and only when nothing followed, left the
                // whole defect alive for any ambiguous line with body prose
                // after it.
                //
                // The asymmetry that decides the design: a *demoted* line is body
                // text, and `tix edit --body` replaces the body wholesale — so a
                // hand-edited entry that was demoted is destroyed by an ordinary
                // command, with lint silent. A *retained* line is kept verbatim
                // inside the section and reported. Given the choice, retaining is
                // the recoverable one, so the tie only goes to "this is prose"
                // when the line is not plausibly an entry at all.
                let plausible_entry = is_entry_attempt(item);
                let after_all_entries = following.iter().all(|l| looks_like_history_entry(l));
                let keep = if following.is_empty() {
                    // Nothing after to judge by, so shape alone decides. A bare
                    // paragraph at end of section is prose; anything list-shaped
                    // is kept, because there is no following content that could
                    // have been prose instead.
                    is_list_item(item)
                } else {
                    plausible_entry || after_all_entries
                };
                if !keep {
                    break;
                }
                // The **raw** line, not the trimmed one: `Unparsed` promises a
                // hand edit is carried through a read/write cycle unchanged, and
                // trimming silently repaired leading indentation and trailing
                // whitespace — exactly the auto-repair data-model §8 forbids.
                history.entries.push(HistoryEntry::Unparsed(
                    raw_line.trim_end_matches('\n').to_string(),
                ));
                offset += line.len();
                continue;
            }
            match HistoryEvent::parse(item) {
                Ok(event) => history.entries.push(HistoryEntry::Event(event)),
                // Kept verbatim. Diagnosed by `tix lint`, never repaired, never
                // dropped, and never fatal to the file.
                // Raw line, for the same reason as above.
                // Terminator only. `trim_end()` also eats trailing spaces and
                // tabs, which is the auto-repair data-model §8 forbids — and
                // `Err` is the arm a *recognised* entry lands in, so it is the
                // more likely one to carry real trailing whitespace.
                Err(_) => history.entries.push(HistoryEntry::Unparsed(
                    raw_line.trim_end_matches('\n').to_string(),
                )),
            }
            offset += line.len();
        }

        (history, after_heading[offset..].to_string())
    }
}

impl Issue {
    /// Parse a full issue file (`frontmatter` + `body`) from its text.
    ///
    /// The first line must be `---`; the closing `---` terminates the
    /// frontmatter. The body after it is preserved verbatim (the single
    /// structural separator blank line is consumed).
    pub fn parse(contents: &str) -> Result<Self> {
        let rest = contents
            .strip_prefix("---\n")
            .ok_or_else(|| TixError::Parse("file must start with a `---` delimiter".into()))?;
        let mut current = rest;
        let mut fm_len = 0usize;
        loop {
            let line_end = current.find('\n').map(|i| i + 1).unwrap_or(current.len());
            let content = current[..line_end]
                .strip_suffix('\n')
                .unwrap_or(&current[..line_end]);
            if content == "---" {
                break;
            }
            fm_len += line_end;
            current = &current[line_end..];
            if current.is_empty() {
                return Err(TixError::Parse(
                    "frontmatter is not terminated by a closing `---`".into(),
                ));
            }
        }
        let frontmatter = &rest[..fm_len];
        let body_raw = &rest[fm_len..];
        let mut body = body_raw.strip_prefix("---\n").unwrap_or(body_raw);
        if let Some(after) = body.strip_prefix('\n') {
            body = after;
        }
        let mut issue: Issue =
            serde_yaml::from_str(frontmatter).map_err(|e| TixError::Parse(e.to_string()))?;
        // A legacy `links:` key is recorded, never read. The typed model does
        // not migrate it and does not reinterpret it as typed edges; the
        // service reports it as a `retired_links_key` validation result (C-001).
        // `relationships` stays as serde left it — empty.
        issue.has_retired_links_key = frontmatter_has_retired_links_key(frontmatter);
        // Unknown top-level keys are captured so the next write can put them back
        // (#1361). `links` is excluded on purpose — see [`capture_extra_frontmatter`].
        issue.extra_frontmatter = capture_extra_frontmatter(frontmatter);
        let (history, remaining_body) = History::parse_front(body);
        issue.history = history;
        issue.body = remaining_body;
        Ok(issue)
    }

    /// Serialize just the frontmatter YAML (no delimiters), with `None`
    /// fields omitted except `created`/`updated` which are always present.
    pub fn frontmatter(&self) -> String {
        let fm =
            serde_yaml::to_string(self).expect("Issue frontmatter serialization is infallible");
        let mut fm = fm.trim_end_matches('\n').to_string();
        // Unknown keys are appended after the known ones, in `BTreeMap` order, so
        // the render is deterministic. A `HashMap` would reshuffle them on every
        // write and produce a diff in a file that had not changed (#1361).
        //
        // They go last rather than back in their original position because the
        // positions of the *known* keys move as optional fields come and go;
        // re-inserting at the original index would be a guess. `tix` has no
        // opinion about these keys, so where they sit carries no meaning.
        for (key, block) in &self.extra_frontmatter {
            fm.push('\n');
            fm.push_str(block);
            let _ = key;
        }
        fm.push('\n');
        fm
    }

    /// Render the full issue file: `---`, frontmatter, closing `---`,
    /// blank separator and the raw body.
    pub fn to_file_contents(&self) -> String {
        // Frontmatter, then the History section (omitted when empty), then the
        // issue's own body — the order `data-model.md` §8 pins.
        let history = self.history.render();
        let mut out = format!("---\n{}---\n\n", self.frontmatter());
        out.push_str(&history);
        out.push_str(&self.body);
        out
    }

    /// A borrowed [`IssueView`] of this issue, for the relationship service.
    ///
    /// Keeps the service off the full `Issue` surface: it needs only the
    /// number, title, status, relationships, and whether the legacy `links:`
    /// key is still present.
    pub fn issue_view(&self) -> IssueView<'_> {
        IssueView::new(
            self.number,
            &self.title,
            self.status.display(),
            &self.relationships,
        )
        .with_retired_links_key(self.has_retired_links_key)
        .with_parent(self.parent)
    }

    /// The date-prefixed filename stem: `{created}-{slug}` (C-008).
    pub fn filename_stem(&self) -> String {
        format!("{}-{}", self.created, slugify(&self.title))
    }

    /// Record a mutation by stamping today's date into `updated`.
    pub fn bump_updated(&mut self) {
        self.updated = today();
    }

    /// Set `start_date`/`due_date` together after atomic validation (FR-007):
    /// both must parse as calendar dates and `due_date >= start_date` when
    /// both are present (delegates to [`validate_issue_dates`]).
    pub fn set_dates(
        &mut self,
        start_date: Option<String>,
        due_date: Option<String>,
    ) -> Result<()> {
        validate_issue_dates(start_date.as_deref(), due_date.as_deref())?;
        self.start_date = start_date;
        self.due_date = due_date;
        Ok(())
    }

    /// The byte offset of the first top-level `## Comments` line in `body`,
    /// if any (C-001, `contracts/comments-format.md` rule 6).
    ///
    /// A line counts as the boundary only when it is *not* inside a fenced
    /// code block, so a literal `## Comments` in prose (or a code block) is
    /// never mistaken for the real section head.
    fn comments_boundary(body: &str) -> Option<usize> {
        let mut in_fence = false;
        let mut offset = 0usize;
        for raw in body.split_inclusive('\n') {
            let line = raw.strip_suffix('\n').unwrap_or(raw);
            if line.starts_with("```") || line.starts_with("~~~") {
                in_fence = !in_fence;
            } else if !in_fence && line == "## Comments" {
                return Some(offset);
            }
            offset += raw.len();
        }
        None
    }

    /// The description: everything above the first top-level `## Comments`
    /// heading, trimmed of trailing whitespace. With no section, the whole
    /// body is the description.
    pub fn description(&self) -> &str {
        match Self::comments_boundary(&self.body) {
            Some(idx) => self.body[..idx].trim_end(),
            None => self.body.trim_end(),
        }
    }

    /// The comments parsed from the `## Comments` section, in file order
    /// (NFR-005). With no section — or a section with no `###` heads —
    /// returns an empty vec.
    pub fn comments(&self) -> Vec<Comment> {
        let Some(boundary) = Self::comments_boundary(&self.body) else {
            return Vec::new();
        };
        let section = &self.body[boundary..];
        let mut comments = Vec::new();
        let mut offset = 0usize;
        for raw in section.split_inclusive('\n') {
            let line = raw.strip_suffix('\n').unwrap_or(raw);
            if let Some(after) = line.strip_prefix("### ") {
                let (author, date) = parse_comment_head(after);
                let start = offset + raw.len();
                let mut text = String::new();
                for next in section[start..].split_inclusive('\n') {
                    if next.strip_suffix('\n').unwrap_or(next).starts_with("### ") {
                        break;
                    }
                    text.push_str(next);
                }
                comments.push(Comment {
                    author,
                    date,
                    text: text.trim_end_matches('\n').to_string(),
                });
            }
            offset += raw.len();
        }
        comments
    }

    /// Append a comment to the `## Comments` section.
    ///
    /// Whitespace-only text is discarded and the body is left untouched
    /// (FR-004 case 4). Otherwise the section is created if absent and the
    /// comment block is appended after the last existing comment. The
    /// description and any existing comments are never rewritten.
    pub fn add_comment(&mut self, author: &str, text: &str) {
        if text.trim().is_empty() {
            return;
        }
        if Self::comments_boundary(&self.body).is_none() {
            if !self.body.trim().is_empty() {
                if !self.body.ends_with('\n') {
                    self.body.push('\n');
                }
                self.body.push('\n');
            }
            self.body.push_str("## Comments\n");
        }
        if !self.body.is_empty() && !self.body.ends_with('\n') {
            self.body.push('\n');
        }
        if !self.body.ends_with("\n\n") {
            self.body.push('\n');
        }
        self.body
            .push_str(&format!("### {author} ({})\n\n{text}\n", today()));
        self.bump_updated();
    }
}

// ---------------------------------------------------------------------------
// The hierarchy service's view of an `Issue`
// ---------------------------------------------------------------------------

/// Read in place, rather than through a borrowed projection.
///
/// Implemented here because `hierarchy` cannot import `model` without a cycle, and
/// because it is `Issue` — not [`IssueView`] — that every store-level caller already
/// holds. Without this each of them builds a `Vec<IssueView>` first, which is a
/// projection nobody asked for and one more place for two services to disagree about
/// the same snapshot.
impl crate::hierarchy::HierarchyNode for Issue {
    fn number(&self) -> u32 {
        self.number
    }
    fn title(&self) -> &str {
        &self.title
    }
    fn status(&self) -> &str {
        self.status.display()
    }
    fn parent_number(&self) -> Option<u32> {
        self.parent
    }
}

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

    #[test]
    fn status_helpers() {
        let s = Status::new("in progress");
        assert!(s.is("in progress"));
        assert!(!s.is("in_progress"));
        assert_eq!(s.display(), "in progress");
        assert_eq!(Status::default(), Status::new("todo"));
    }

    #[test]
    fn status_survives_unknown_column_name() {
        let parsed: Status = serde_yaml::from_str("\"renamed-column\"").unwrap();
        assert_eq!(parsed.display(), "renamed-column");
    }

    #[test]
    fn priority_roundtrips_lowercase_and_parses_case_insensitively() {
        // An unset priority round-trips as the empty string and is *not* the
        // default: `priority: ''` says "no priority", which is a different
        // statement from the `medium` a missing field yields.
        let undefined: Priority = "".parse().unwrap();
        assert!(undefined.is_undefined());
        assert_eq!(undefined.to_string(), "", "and it writes back as ''");
        assert!(
            !Priority::default().is_undefined(),
            "the default is medium, not unset"
        );
        assert!(
            !Priority::Other("urgent".into()).is_undefined(),
            "only the empty string is unset"
        );
        assert_eq!("low".parse::<Priority>().unwrap(), Priority::Low);
        assert_eq!("HIGH".parse::<Priority>().unwrap(), Priority::High);
        assert_eq!(Priority::default(), Priority::Medium);
        assert_eq!(Priority::Low.to_string(), "low");
    }

    #[test]
    fn unknown_priority_is_preserved_not_normalized() {
        let urgent: Priority = serde_yaml::from_str("\"urgent\"").unwrap();
        assert_eq!(urgent.to_string(), "urgent");
        let yaml = serde_yaml::to_string(&urgent).unwrap();
        assert!(yaml.contains("urgent"));
    }

    #[test]
    fn issue_roundtrip_preserves_every_field_including_number_and_body() {
        let mut issue = Issue {
            number: 7,
            title: "Add documentation for the feature".into(),
            status: Status::new("todo"),
            labels: vec!["docs".into(), "onboarding".into()],
            priority: Priority::High,
            assignee: Some("alice".into()),
            estimate: Some("2".into()),
            commit: Some("abc123".into()),
            gitlab_id: Some("gl-9".into()),
            linear_id: Some("LIN-9".into()),
            mission: None,
            task: None,
            relationships: vec![],
            history: crate::model::History::new(),
            has_retired_links_key: false,
            files: vec![],
            cycles: None,
            milestones: None,
            reviewer: None,
            start_date: None,
            due_date: None,
            created: "2026-05-10".into(),
            updated: "2026-05-10".into(),
            filename: String::new(),
            body: "Describe the issue here.\n\n![diagram](./assets/diagram.png)".into(),
            extra_frontmatter: std::collections::BTreeMap::new(),
            parent: None,
            archived: false,
            entered_at: None,
            source: None,
        };
        let contents = issue.to_file_contents();
        let parsed = Issue::parse(&contents).unwrap();
        assert_eq!(parsed, issue);
        issue.body.clear();
        assert_ne!(parsed, issue);
    }

    #[test]
    fn typed_relationships_round_trip_preserves_entries_verbatim() {
        let mut issue = Issue {
            number: 9,
            title: "T".into(),
            status: Status::default(),
            labels: vec![],
            priority: Priority::default(),
            assignee: None,
            estimate: None,
            commit: None,
            gitlab_id: None,
            linear_id: None,
            mission: None,
            task: None,
            relationships: vec![
                Relationship::new(9, 12, RelationKind::Blocks, "Twelve"),
                Relationship::new(9, 47, RelationKind::Related, "Forty seven"),
            ],
            history: History::new(),
            has_retired_links_key: false,
            files: vec!["docs/x.md".into(), "specs/y/spec.md".into()],
            cycles: None,
            milestones: None,
            reviewer: None,
            start_date: None,
            due_date: None,
            created: "2026-01-01".into(),
            updated: "2026-01-01".into(),
            filename: String::new(),
            body: String::new(),
            extra_frontmatter: std::collections::BTreeMap::new(),
            parent: None,
            archived: false,
            entered_at: None,
            source: None,
        };
        let contents = issue.to_file_contents();
        let parsed = Issue::parse(&contents).unwrap();
        assert_eq!(parsed.relationships.len(), 2);
        assert_eq!(parsed.relationships[0].kind, RelationKind::Blocks);
        assert_eq!(parsed.relationships[0].display, "blocks #12 Twelve");
        // The stable id survives the round trip, which is what makes an edit
        // addressable after a rewrite.
        assert_eq!(parsed.relationships[0].id, issue.relationships[0].id);
        assert_eq!(parsed.files, ["docs/x.md", "specs/y/spec.md"]);
        // Byte-stable through read -> write -> read.
        assert_eq!(parsed.to_file_contents(), contents);
        // A stored edge never carries the input-only `blocked_by` kind.
        assert!(
            parsed
                .relationships
                .iter()
                .all(|r| r.kind != RelationKind::BlockedBy)
        );
        issue.relationships.clear();
    }

    #[test]
    fn empty_relationships_is_omitted_while_files_keeps_its_inline_empty_list() {
        // The empty-list shape is decided once and pinned here: the typed field
        // is **omitted** when empty, because it is new and a meaningless
        // `relationships: []` would otherwise appear on every issue file. The
        // pre-existing `files: []` convention is untouched.
        let issue = Issue {
            number: 4,
            title: "T".into(),
            status: Status::default(),
            labels: vec![],
            priority: Priority::default(),
            assignee: None,
            estimate: None,
            commit: None,
            gitlab_id: None,
            linear_id: None,
            mission: None,
            task: None,
            relationships: vec![],
            history: History::new(),
            has_retired_links_key: false,
            files: vec![],
            cycles: None,
            milestones: None,
            reviewer: None,
            start_date: None,
            due_date: None,
            created: "2026-01-01".into(),
            updated: "2026-01-01".into(),
            filename: String::new(),
            body: String::new(),
            extra_frontmatter: std::collections::BTreeMap::new(),
            parent: None,
            archived: false,
            entered_at: None,
            source: None,
        };
        let fm = issue.frontmatter();
        assert!(!fm.contains("relationships"), "got:\n{fm}");
        assert!(fm.contains("files: []"), "got:\n{fm}");
    }

    #[test]
    fn an_unknown_relation_kind_in_frontmatter_is_rejected() {
        // The old field tolerated any string, so a non-numeric entry parsed
        // happily. The typed vocabulary is closed, so an unknown kind is a
        // parse error rather than a coerced default (C-001).
        let yaml = "\
number: 1
title: T
status: todo
relationships:
- id: rel_1
  from: 1
  to: 2
  kind: depends_on
created: '2026-01-01'
updated: '2026-01-01'
";
        let err = serde_yaml::from_str::<Issue>(yaml).unwrap_err();
        assert!(
            err.to_string().contains("depends_on"),
            "the unknown kind is named: {err}"
        );
    }

    #[test]
    fn a_legacy_links_key_parses_but_sets_the_retired_flag() {
        // Tolerant on read so existing repositories still load, and flagged so
        // the service can report it. The value is never read as typed data.
        let yaml = "\
number: 1
title: T
status: todo
links:
- '12'
- 47
created: '2026-01-01'
updated: '2026-01-01'
";
        let issue = Issue::parse(&format!("---\n{yaml}---\n\nbody\n")).unwrap();
        assert!(issue.has_retired_links_key);
        assert!(
            issue.relationships.is_empty(),
            "C-001: legacy values are never reinterpreted as typed edges"
        );
    }

    #[test]
    fn missing_dates_default_to_today() {
        let yaml = "number: 2\ntitle: Hi\nstatus: todo\n";
        let issue: Issue = serde_yaml::from_str(yaml).unwrap();
        assert_eq!(issue.created, today());
        assert_eq!(issue.updated, today());
    }

    #[test]
    fn body_is_preserved_verbatim() {
        let issue = Issue {
            number: 1,
            title: "T".into(),
            status: Status::default(),
            labels: vec![],
            priority: Priority::default(),
            assignee: None,
            estimate: None,
            commit: None,
            gitlab_id: None,
            linear_id: None,
            mission: None,
            task: None,
            relationships: vec![],
            history: crate::model::History::new(),
            has_retired_links_key: false,
            files: vec![],
            cycles: None,
            milestones: None,
            reviewer: None,
            start_date: None,
            due_date: None,
            created: "2026-01-02".into(),
            updated: "2026-01-02".into(),
            filename: String::new(),
            body: "line one\n\nline two\n```\ncode\n```".into(),
            extra_frontmatter: std::collections::BTreeMap::new(),
            parent: None,
            archived: false,
            entered_at: None,
            source: None,
        };
        let contents = issue.to_file_contents();
        let parsed = Issue::parse(&contents).unwrap();
        assert_eq!(parsed.body, issue.body);
    }

    #[test]
    fn assignee_at_prefix_is_stripped_on_read_and_restored_on_write() {
        let yaml = "number: 3\ntitle: T\nstatus: todo\nassignee: \"@alice\"\ncreated: \"2026-01-01\"\nupdated: \"2026-01-01\"\n";
        let issue: Issue = serde_yaml::from_str(yaml).unwrap();
        assert_eq!(issue.assignee.as_deref(), Some("alice"));
        let rerendered = issue.frontmatter();
        assert!(
            rerendered.contains("assignee: \"@alice\"")
                || rerendered.contains("assignee: '@alice'")
        );
    }

    #[test]
    fn full_identity_assignee_round_trips_verbatim() {
        let issue = Issue {
            number: 5,
            title: "T".into(),
            status: Status::default(),
            labels: vec![],
            priority: Priority::default(),
            assignee: Some("Herbert Maschke <thyseus@pm.me>".into()),
            estimate: None,
            commit: None,
            gitlab_id: None,
            linear_id: None,
            mission: None,
            task: None,
            relationships: vec![],
            files: vec![],
            cycles: None,
            milestones: None,
            reviewer: None,
            start_date: None,
            due_date: None,
            created: "2026-01-01".into(),
            updated: "2026-01-01".into(),
            filename: String::new(),
            body: String::new(),
            history: crate::model::History::new(),
            has_retired_links_key: false,
            extra_frontmatter: std::collections::BTreeMap::new(),
            parent: None,
            archived: false,
            entered_at: None,
            source: None,
        };
        let fm = issue.frontmatter();
        assert!(
            fm.contains("Herbert Maschke <thyseus@pm.me>"),
            "identity must be stored verbatim, got:\n{fm}"
        );
        assert!(
            !fm.contains("@Herbert Maschke"),
            "identity must not be @-prefixed, got:\n{fm}"
        );
        let parsed = Issue::parse(&issue.to_file_contents()).unwrap();
        assert_eq!(parsed, issue);
    }

    #[test]
    fn bare_handle_assignee_writes_legacy_at_prefix_and_reads_back_bare() {
        let issue = Issue {
            number: 6,
            title: "T".into(),
            status: Status::default(),
            labels: vec![],
            priority: Priority::default(),
            assignee: Some("alice".into()),
            estimate: None,
            commit: None,
            gitlab_id: None,
            linear_id: None,
            mission: None,
            task: None,
            relationships: vec![],
            files: vec![],
            cycles: None,
            milestones: None,
            reviewer: None,
            start_date: None,
            due_date: None,
            created: "2026-01-01".into(),
            updated: "2026-01-01".into(),
            filename: String::new(),
            body: String::new(),
            history: crate::model::History::new(),
            has_retired_links_key: false,
            extra_frontmatter: std::collections::BTreeMap::new(),
            parent: None,
            archived: false,
            entered_at: None,
            source: None,
        };
        let fm = issue.frontmatter();
        assert!(
            fm.contains("assignee: \"@alice\"") || fm.contains("assignee: '@alice'"),
            "legacy @-prefixed bytes expected, got:\n{fm}"
        );
        let parsed = Issue::parse(&issue.to_file_contents()).unwrap();
        assert_eq!(parsed.assignee.as_deref(), Some("alice"));
    }

    #[test]
    fn assignee_read_accepts_both_bare_and_at_prefixed_handles() {
        let legacy = "number: 7\ntitle: T\nstatus: todo\nassignee: \"@alice\"\ncreated: \"2026-01-01\"\nupdated: \"2026-01-01\"\n";
        let bare = "number: 7\ntitle: T\nstatus: todo\nassignee: \"alice\"\ncreated: \"2026-01-01\"\nupdated: \"2026-01-01\"\n";
        let from_legacy: Issue = serde_yaml::from_str(legacy).unwrap();
        let from_bare: Issue = serde_yaml::from_str(bare).unwrap();
        assert_eq!(from_legacy.assignee.as_deref(), Some("alice"));
        assert_eq!(from_bare.assignee.as_deref(), Some("alice"));
    }

    #[test]
    fn angle_bracket_identity_without_name_written_verbatim() {
        let issue = Issue {
            number: 8,
            title: "T".into(),
            status: Status::default(),
            labels: vec![],
            priority: Priority::default(),
            assignee: Some("<thyseus@pm.me>".into()),
            estimate: None,
            commit: None,
            gitlab_id: None,
            linear_id: None,
            mission: None,
            task: None,
            relationships: vec![],
            files: vec![],
            cycles: None,
            milestones: None,
            reviewer: None,
            start_date: None,
            due_date: None,
            created: "2026-01-01".into(),
            updated: "2026-01-01".into(),
            filename: String::new(),
            body: String::new(),
            history: crate::model::History::new(),
            has_retired_links_key: false,
            extra_frontmatter: std::collections::BTreeMap::new(),
            parent: None,
            archived: false,
            entered_at: None,
            source: None,
        };
        let fm = issue.frontmatter();
        assert!(
            fm.contains("<thyseus@pm.me>"),
            "identity-like value must be verbatim, got:\n{fm}"
        );
        assert!(
            !fm.contains("@<thyseus"),
            "identity-like value must not be @-prefixed, got:\n{fm}"
        );
    }

    #[test]
    fn all_none_frontmatter_only_contains_required_keys() {
        let issue = Issue {
            number: 4,
            title: "T".into(),
            status: Status::default(),
            labels: vec![],
            priority: Priority::default(),
            assignee: None,
            estimate: None,
            commit: None,
            gitlab_id: None,
            linear_id: None,
            mission: None,
            task: None,
            relationships: vec![],
            files: vec![],
            cycles: None,
            milestones: None,
            reviewer: None,
            start_date: None,
            due_date: None,
            created: "2026-01-01".into(),
            updated: "2026-01-01".into(),
            filename: String::new(),
            body: String::new(),
            history: crate::model::History::new(),
            has_retired_links_key: false,
            extra_frontmatter: std::collections::BTreeMap::new(),
            parent: None,
            archived: false,
            entered_at: None,
            source: None,
        };
        let fm = issue.frontmatter();
        assert!(fm.contains("number:"));
        assert!(fm.contains("title:"));
        assert!(fm.contains("status:"));
        assert!(fm.contains("labels:"));
        assert!(fm.contains("priority:"));
        assert!(fm.contains("created:"));
        assert!(fm.contains("updated:"));
        for key in ["assignee", "estimate", "commit", "gitlab_id", "linear_id"] {
            assert!(!fm.contains(key), "frontmatter must omit {key}, got:\n{fm}");
        }
    }

    #[test]
    fn filename_stem_matches_contract_format() {
        let issue = Issue {
            number: 1,
            title: "Fix !Login crash?".into(),
            status: Status::default(),
            labels: vec![],
            priority: Priority::default(),
            assignee: None,
            estimate: None,
            commit: None,
            gitlab_id: None,
            linear_id: None,
            mission: None,
            task: None,
            relationships: vec![],
            files: vec![],
            cycles: None,
            milestones: None,
            reviewer: None,
            start_date: None,
            due_date: None,
            created: "2026-05-10".into(),
            updated: "2026-05-10".into(),
            filename: String::new(),
            body: String::new(),
            history: crate::model::History::new(),
            has_retired_links_key: false,
            extra_frontmatter: std::collections::BTreeMap::new(),
            parent: None,
            archived: false,
            entered_at: None,
            source: None,
        };
        assert_eq!(issue.filename_stem(), "2026-05-10-fix-login-crash");
    }

    #[test]
    fn bump_updated_stamps_today() {
        let mut issue = Issue {
            number: 1,
            title: "T".into(),
            status: Status::default(),
            labels: vec![],
            priority: Priority::default(),
            assignee: None,
            estimate: None,
            commit: None,
            gitlab_id: None,
            linear_id: None,
            mission: None,
            task: None,
            relationships: vec![],
            files: vec![],
            cycles: None,
            milestones: None,
            reviewer: None,
            start_date: None,
            due_date: None,
            created: "2020-01-01".into(),
            updated: "2020-01-01".into(),
            filename: String::new(),
            body: String::new(),
            history: crate::model::History::new(),
            has_retired_links_key: false,
            extra_frontmatter: std::collections::BTreeMap::new(),
            parent: None,
            archived: false,
            entered_at: None,
            source: None,
        };
        issue.bump_updated();
        assert_eq!(issue.updated, today());
        assert_ne!(issue.updated, "2020-01-01");
    }

    #[test]
    fn parse_rejects_malformed_frontmatter() {
        assert!(Issue::parse("no delimiter\n").is_err());
        assert!(Issue::parse("---\na: 1\n").is_err()); // unterminated
        assert!(Issue::parse("---\nbroken: [").is_err()); // bad yaml, unterminated
    }

    #[test]
    fn today_is_an_iso_calendar_date() {
        let t = today();
        assert_eq!(t.len(), 10);
        let parts: Vec<&str> = t.split('-').collect();
        assert_eq!(parts.len(), 3);
        assert_eq!(parts[0].len(), 4);
        assert_eq!(parts[1].len(), 2);
        assert_eq!(parts[2].len(), 2);
    }

    #[test]
    fn civil_from_days_epoch_is_1970_01_01() {
        assert_eq!(civil_from_days(0), (1970, 1, 1));
    }

    // ------------------------------------------------------------------
    // WP01 — Comment data model & body lifecycle
    // ------------------------------------------------------------------

    fn issue(body: &str) -> Issue {
        Issue {
            number: 1,
            title: "T".into(),
            status: Status::default(),
            labels: vec![],
            priority: Priority::default(),
            assignee: None,
            estimate: None,
            commit: None,
            gitlab_id: None,
            linear_id: None,
            mission: None,
            task: None,
            relationships: vec![],
            history: History::new(),
            has_retired_links_key: false,
            files: vec![],
            cycles: None,
            milestones: None,
            reviewer: None,
            start_date: None,
            due_date: None,
            created: "2026-01-01".into(),
            updated: "2026-01-01".into(),
            filename: String::new(),
            body: body.to_string(),
            extra_frontmatter: std::collections::BTreeMap::new(),
            parent: None,
            archived: false,
            entered_at: None,
            source: None,
        }
    }

    // --- T001: parse body → description + comments ---

    #[test]
    fn comments_empty_when_no_section() {
        let i = issue("Some description text.\n");
        assert_eq!(i.description(), "Some description text.");
        assert!(i.comments().is_empty());
    }

    #[test]
    fn comments_parsed_from_section() {
        let i = issue(
            "Hello\n\n## Comments\n\n\
             ### @alice (2026-09-13)\n\n\
             First comment.\n\n\
             ### bob (2026-09-14)\n\n\
             Second comment.\n",
        );
        assert_eq!(i.description(), "Hello");
        let cs = i.comments();
        assert_eq!(cs.len(), 2);
        assert_eq!(cs[0].author, "@alice");
        assert_eq!(cs[0].date, "2026-09-13");
        assert_eq!(cs[1].author, "bob");
        assert_eq!(cs[1].date, "2026-09-14");
    }

    #[test]
    fn code_fence_boundary_not_treated_as_boundary() {
        let body = "Description\n\n\
             ```\n\
             ## Comments\n\
             ```\n\n\
             ## Comments\n\n\
             ### eve (2026-10-01)\n\n\
             real comment\n";
        let i = issue(body);
        let cs = i.comments();
        assert_eq!(cs.len(), 1);
        assert_eq!(cs[0].author, "eve");
        assert_eq!(cs[0].date, "2026-10-01");
        assert_eq!(cs[0].text, "\nreal comment");
        assert!(
            i.description().contains("## Comments"),
            "code-blocked heading must be in description"
        );
    }

    #[test]
    fn blank_lines_preserved_in_comment_text() {
        let i = issue(
            "## Comments\n\n\
             ### @alice (2026-09-13)\n\n\
             line one\n\nline two\n\n\n\
             line three\n",
        );
        let cs = i.comments();
        assert_eq!(cs.len(), 1);
        let text = &cs[0].text;
        assert!(
            text.contains("\nline two"),
            "internal blank lines must be preserved: got {text:?}"
        );
        assert!(
            text.contains("line three"),
            "must contain all lines: got {text:?}"
        );
    }

    #[test]
    fn comments_empty_when_section_has_no_heads() {
        let i = issue("## Comments\n\n\n");
        assert!(i.comments().is_empty());
    }

    // --- T002: add_comment ---

    #[test]
    fn add_first_comment_to_empty_body() {
        let mut i = issue("");
        i.add_comment("@alice", "Hello world");
        assert_eq!(i.description(), "");
        let cs = i.comments();
        assert_eq!(cs.len(), 1);
        assert_eq!(cs[0].author, "@alice");
        assert_eq!(cs[0].text, "\nHello world");
        assert!(i.body.contains("## Comments"));
    }

    #[test]
    fn second_comment_appended_after_first() {
        let mut i = issue("");
        i.add_comment("@alice", "first");
        i.add_comment("bob", "second");
        let cs = i.comments();
        assert_eq!(cs.len(), 2);
        let pos_a = i.body.find("### @alice");
        let pos_b = i.body.find("### bob");
        assert!(pos_a < pos_b, "first comment must precede second");
    }

    #[test]
    fn whitespace_only_comment_discarded() {
        let mut i = issue("Hello\n");
        let before = i.body.clone();
        i.add_comment("@alice", "   \n  ");
        assert_eq!(i.body, before, "whitespace-only text must not change body");
    }

    #[test]
    fn description_unchanged_after_add_comment() {
        let mut i = issue("Description text.\n");
        i.add_comment("@alice", "a comment");
        assert_eq!(i.description(), "Description text.");
    }

    #[test]
    fn literal_comments_heading_in_code_block_not_touched() {
        let mut i = issue("```\n## Comments\n```\n");
        i.add_comment("@alice", "new");
        assert!(
            i.body.contains("```\n## Comments\n```"),
            "code-block heading must be preserved: {0:?}",
            i.body
        );
        assert!(i.body.contains("### @alice"), "must contain new comment");
    }

    // --- T003: roundtrip / NFR-005 ---

    #[test]
    fn roundtrip_preserves_comment_data() {
        let mut i = issue(
            "## Comments\n\n\
             ### @alice (2026-09-13)\n\n\
             Original comment.\n",
        );
        i.add_comment("bob", "Second comment");
        let file = i.to_file_contents();
        let parsed = Issue::parse(&file).unwrap();
        assert_eq!(parsed.description(), i.description());
        let cs = parsed.comments();
        assert_eq!(cs.len(), 2);
        assert_eq!(cs[0].author, "@alice");
        assert_eq!(cs[1].author, "bob");
        assert_eq!(cs[1].text, "\nSecond comment");
    }

    #[test]
    fn three_comments_in_file_order() {
        let mut i = issue("");
        i.add_comment("a", "one");
        i.add_comment("b", "two");
        i.add_comment("c", "three");
        let cs = i.comments();
        assert_eq!(
            cs.iter().map(|c| c.author.as_str()).collect::<Vec<_>>(),
            ["a", "b", "c"]
        );
        assert_eq!(
            i.body,
            format!(
                "## Comments\n\n### a ({0})\n\none\n\n### b ({0})\n\ntwo\n\n### c ({0})\n\nthree\n",
                today()
            )
        );
    }

    #[test]
    fn preserves_hand_authored_comment_markdown() {
        let body = "## Comments\n\n\
             ### @alice (2026-09-13)\n\n\
             **bold** and _italic_\n\n\
             - item 1\n- item 2\n\n";
        let i = issue(body);
        let cs = i.comments();
        assert_eq!(cs.len(), 1);
        assert!(cs[0].text.contains("**bold**"));
        assert!(cs[0].text.contains("- item 1"));
    }

    #[test]
    fn description_code_block_heading_not_boundary() {
        let mut i = issue("Intro text here\n\n```\n## Comments\n```\n");
        let before = i.body.clone();
        i.add_comment("eve", "real");
        assert_eq!(i.description(), "Intro text here\n\n```\n## Comments\n```");
        let cs = i.comments();
        assert_eq!(cs.len(), 1);
        assert_eq!(cs[0].author, "eve");
        assert!(
            i.body.starts_with(&before),
            "code-block heading must be preserved"
        );
    }

    // ------------------------------------------------------------------
    // WP02 — Planning fields, dates & file format
    // ------------------------------------------------------------------

    #[test]
    fn planning_fields_round_trip() {
        let issue = Issue {
            number: 12,
            title: "Ship cycle one".into(),
            status: Status::new("in progress"),
            labels: vec!["release".into()],
            priority: Priority::High,
            assignee: Some("alice".into()),
            estimate: None,
            commit: None,
            gitlab_id: None,
            linear_id: None,
            mission: None,
            task: None,
            relationships: vec![],
            history: crate::model::History::new(),
            has_retired_links_key: false,
            files: vec!["specs/p/spec.md".into()],
            cycles: Some(vec!["Cycle 1".into(), "Sprint hardening".into()]),
            milestones: Some(vec!["v1.0".into()]),
            reviewer: Some("Bob <bob@example.com>".into()),
            start_date: Some("2026-09-07".into()),
            due_date: Some("2026-09-18".into()),
            created: "2026-09-07".into(),
            updated: "2026-09-07".into(),
            filename: String::new(),
            body: "body".into(),
            extra_frontmatter: std::collections::BTreeMap::new(),
            parent: None,
            archived: false,
            entered_at: None,
            source: None,
        };
        let contents = issue.to_file_contents();
        let parsed = Issue::parse(&contents).unwrap();
        assert_eq!(parsed, issue);
        assert_eq!(
            parsed.cycles.clone().unwrap(),
            ["Cycle 1", "Sprint hardening"]
        );
        assert_eq!(parsed.milestones.clone().unwrap(), ["v1.0"]);
        assert_eq!(parsed.reviewer.as_deref(), Some("Bob <bob@example.com>"));
        assert_eq!(parsed.start_date.as_deref(), Some("2026-09-07"));
        assert_eq!(parsed.due_date.as_deref(), Some("2026-09-18"));
    }

    #[test]
    fn planning_fields_absent_on_legacy_serialize_nothing() {
        let issue = Issue {
            number: 3,
            title: "T".into(),
            status: Status::default(),
            labels: vec![],
            priority: Priority::default(),
            assignee: None,
            estimate: None,
            commit: None,
            gitlab_id: None,
            linear_id: None,
            mission: None,
            task: None,
            relationships: vec![],
            history: crate::model::History::new(),
            has_retired_links_key: false,
            files: vec![],
            cycles: None,
            milestones: None,
            reviewer: None,
            start_date: None,
            due_date: None,
            created: "2026-01-01".into(),
            updated: "2026-01-01".into(),
            filename: String::new(),
            body: "body".into(),
            extra_frontmatter: std::collections::BTreeMap::new(),
            parent: None,
            archived: false,
            entered_at: None,
            source: None,
        };
        let fm = issue.frontmatter();
        for needle in ["cycles", "milestones", "reviewer", "start_date", "due_date"] {
            assert!(!fm.contains(needle), "{needle} must be omitted, got:\n{fm}");
        }
        let parsed = Issue::parse(&issue.to_file_contents()).unwrap();
        assert!(parsed.cycles.is_none());
        assert!(parsed.milestones.is_none());
        assert!(parsed.reviewer.is_none());
        assert!(parsed.start_date.is_none());
        assert!(parsed.due_date.is_none());
    }

    #[test]
    fn empty_planning_lists_serialize_as_omitted() {
        let issue = Issue {
            number: 3,
            title: "T".into(),
            status: Status::default(),
            labels: vec![],
            priority: Priority::default(),
            assignee: None,
            estimate: None,
            commit: None,
            gitlab_id: None,
            linear_id: None,
            mission: None,
            task: None,
            relationships: vec![],
            history: crate::model::History::new(),
            has_retired_links_key: false,
            files: vec![],
            cycles: Some(vec![]),
            milestones: Some(vec![]),
            reviewer: None,
            start_date: None,
            due_date: None,
            created: "2026-01-01".into(),
            updated: "2026-01-01".into(),
            filename: String::new(),
            body: "body".into(),
            extra_frontmatter: std::collections::BTreeMap::new(),
            parent: None,
            archived: false,
            entered_at: None,
            source: None,
        };
        let fm = issue.frontmatter();
        assert!(!fm.contains("cycles"), "{fm}");
        assert!(!fm.contains("milestones"), "{fm}");
    }

    #[test]
    fn duplicate_planning_list_entries_collapse_on_write() {
        let issue = Issue {
            number: 3,
            title: "T".into(),
            status: Status::default(),
            labels: vec![],
            priority: Priority::default(),
            assignee: None,
            estimate: None,
            commit: None,
            gitlab_id: None,
            linear_id: None,
            mission: None,
            task: None,
            relationships: vec![],
            history: crate::model::History::new(),
            has_retired_links_key: false,
            files: vec![],
            cycles: Some(vec!["A".into(), "A".into(), "B".into()]),
            milestones: Some(vec!["m".into(), "m".into()]),
            reviewer: None,
            start_date: None,
            due_date: None,
            created: "2026-01-01".into(),
            updated: "2026-01-01".into(),
            filename: String::new(),
            body: "body".into(),
            extra_frontmatter: std::collections::BTreeMap::new(),
            parent: None,
            archived: false,
            entered_at: None,
            source: None,
        };
        let fm = issue.frontmatter();
        let parsed = Issue::parse(&issue.to_file_contents()).unwrap();
        assert_eq!(parsed.cycles.clone().unwrap(), ["A", "B"]);
        assert_eq!(parsed.milestones.clone().unwrap(), ["m"]);
        assert_eq!(fm.matches("- A").count(), 1, "{fm}");
        assert_eq!(fm.matches("- m").count(), 1, "{fm}");
    }

    #[test]
    fn legacy_fixture_round_trips_with_only_the_retired_key_removed() {
        // Canonical fixtures captured from the real corpus before WP02
        // (tests/fixtures/legacy-01.md, legacy-02.md).
        //
        // Each file carries one `links: []` line, and that line is the
        // **approved breaking change** (C-001): the typed model replaces the
        // generic bag and does not re-emit the retired key. So the round trip
        // for these particular fixtures is deliberately not byte-identical, and
        // must not pretend to be. The exact, intended delta is the one removed
        // line and nothing else: every other byte, including the
        // `## Mission Progress` and `## Comments` body sections, survives.
        //
        // The fixtures carry the key **empty**, so it is not *reported* — an
        // empty value has nothing to lose, and reporting it made `tix lint`
        // reject the whole corpus. The key is still never re-emitted, which is
        // what this test is about, and that is unchanged.
        for name in ["legacy-01.md", "legacy-02.md"] {
            let path = format!("{}/tests/fixtures/{name}", env!("CARGO_MANIFEST_DIR"));
            let original = std::fs::read_to_string(&path).unwrap();
            let issue = Issue::parse(&original).unwrap();
            assert!(
                !issue.has_retired_links_key,
                "{name} carries an *empty* retired key, so there is nothing to report"
            );
            assert!(
                issue.relationships.is_empty(),
                "C-001: the legacy value is never read as typed edges"
            );

            let kept: Vec<&str> = original
                .lines()
                .filter(|line| !line.starts_with("links:"))
                .collect();
            let expected = format!("{}\n", kept.join("\n"));

            let round = issue.to_file_contents();
            assert_eq!(
                round, expected,
                "{name} must differ from the legacy file by exactly the retired `links:` line"
            );
            assert!(
                !round.contains("links:"),
                "the retired key is never re-emitted"
            );
            // Byte-stable from here on: the new form round-trips exactly.
            assert_eq!(
                Issue::parse(&round).unwrap().to_file_contents(),
                round,
                "{name} must be byte-stable once in the new format"
            );
            assert!(issue.cycles.is_none() && issue.milestones.is_none());
            assert!(issue.reviewer.is_none());
            assert!(issue.start_date.is_none() && issue.due_date.is_none());
        }
    }

    #[test]
    fn a_file_in_the_current_format_round_trips_byte_identically() {
        // NFR-003: a file the current code writes must survive read -> write
        // -> read with no change at all. This is the guarantee the legacy
        // fixtures cannot make, because they predate the format change.
        let original = concat!(
            "---\n",
            "number: 7\n",
            "title: Current\n",
            "status: todo\n",
            "labels: []\n",
            "priority: medium\n",
            "created: '2026-01-01'\n",
            "updated: '2026-01-01'\n",
            "---\n",
            "\n",
            "## History\n",
            "\n",
            "- 2026-01-02T03:04:05Z \u{2014} alice \u{2014} relationship_added \u{2014} created blocks: blocks #9 Nine\n",
            "\n",
            "Prose that follows the section.\n",
        );
        let issue = Issue::parse(original).unwrap();
        assert!(!issue.has_retired_links_key);
        assert_eq!(issue.history.len(), 1, "the History section is parsed");
        assert_eq!(issue.history.events()[0].actor, "alice");
        assert_eq!(
            issue.history.events()[0].kind,
            HistoryEventKind::RelationshipAdded
        );
        assert!(
            issue.body.contains("Prose that follows"),
            "prose after the section is preserved in the body: {:?}",
            issue.body
        );

        let rendered = issue.to_file_contents();
        assert!(rendered.contains("## History"), "{rendered}");
        assert!(
            rendered.contains(" \u{2014} alice \u{2014} relationship_added \u{2014} "),
            "the rendered entry uses the documented spaced form: {rendered}"
        );
        // Idempotent: rendering the re-parsed form changes nothing. Asserting
        // idempotence rather than equality with the hand-written fixture keeps
        // the guarantee honest about what it covers.
        assert_eq!(
            Issue::parse(&rendered).unwrap().to_file_contents(),
            rendered,
            "a file in the current format round-trips byte-identically"
        );
    }

    #[test]
    fn validate_date_accepts_real_calendar_dates() {
        assert!(validate_date("2026-09-18"));
        assert!(validate_date("1970-01-01"));
        assert!(validate_date("2024-02-29")); // leap year
        assert!(!validate_date("2023-02-29")); // not leap
        assert!(!validate_date("2026-02-30"));
        assert!(!validate_date("2026-13-01"));
        assert!(!validate_date("2026-00-01"));
        assert!(!validate_date("2026-01-00"));
        assert!(!validate_date("2026-1-01"));
        assert!(!validate_date("2026-01-1"));
        assert!(!validate_date("1969-12-31")); // pre-1970 rejected
        assert!(!validate_date("2026/01/01"));
        assert!(!validate_date("today"));
        assert_eq!(days_in_month(2024, 2), 29);
        assert_eq!(days_in_month(2023, 2), 28);
        assert_eq!(days_in_month(2026, 4), 30);
        assert_eq!(days_in_month(2026, 1), 31);
    }

    #[test]
    fn lexicographic_order_equals_chronological_order() {
        assert!(date_lte("2026-01-05", "2026-01-12"));
        assert!(date_lte("2026-01-05", "2026-01-05"));
        assert!(!date_lte("2026-01-12", "2026-01-05"));
        assert!(date_lte("2025-12-31", "2026-01-01"));
    }

    #[test]
    fn validate_issue_dates_enforces_format_and_ordering() {
        assert!(validate_issue_dates(None, None).is_ok());
        assert!(validate_issue_dates(Some("2026-09-07"), None).is_ok());
        assert!(validate_issue_dates(None, Some("2026-09-18")).is_ok());
        assert!(validate_issue_dates(Some("2026-09-07"), Some("2026-09-18")).is_ok());
        assert!(validate_issue_dates(Some("2026-09-18"), Some("2026-09-18")).is_ok());
        let e = validate_issue_dates(Some("2026-09-18"), Some("2026-09-07")).unwrap_err();
        assert!(e.to_string().contains("due_date"), "naming both: {e}");
        assert!(e.to_string().contains("start_date"), "naming both: {e}");
        assert!(validate_issue_dates(Some("2026-13-01"), None).is_err());
        assert!(validate_issue_dates(None, Some("2026-02-30")).is_err());
    }

    #[test]
    fn malformed_planning_fields_load_without_panic() {
        // Storage is tolerant (T007): garbage in the new optional fields must
        // parse without panic and be preserved as-is on the parsed value;
        // validation is a separate, explicit concern.
        let invalid = "---\nnumber: 1\ntitle: T\nstatus: todo\nlabels: []\npriority: medium\nstart_date: not-a-date\ndue_date: \"2026-02-30\"\nreviewer: nobody\ncycles:\n- \"\"\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\nbody\n";
        let issue = Issue::parse(invalid).unwrap();
        assert_eq!(issue.start_date.as_deref(), Some("not-a-date"));
        assert_eq!(issue.due_date.as_deref(), Some("2026-02-30"));
        assert_eq!(issue.reviewer.as_deref(), Some("nobody"));
        assert_eq!(issue.cycles.clone().unwrap(), [""]);
    }

    #[test]
    fn set_dates_validates_before_mutating() {
        let mut issue = issue("body");
        issue
            .set_dates(Some("2026-09-07".into()), Some("2026-09-18".into()))
            .unwrap();
        assert_eq!(issue.start_date.as_deref(), Some("2026-09-07"));
        assert_eq!(issue.due_date.as_deref(), Some("2026-09-18"));
        let before = issue.clone();
        let err = issue.set_dates(Some("2026-09-18".into()), Some("2026-09-07".into()));
        assert!(err.is_err());
        assert_eq!(issue, before, "failed set_dates must not partially mutate");
    }
    // ------------------------------------------------------------------
    // Final-review probes, promoted to tests where they pin real behavior
    // ------------------------------------------------------------------

    /// Frontmatter with one extra line inserted before the closing delimiter.
    fn frontmatter_with(extra: &str) -> String {
        format!("---\nnumber: 1\ntitle: T\nstatus: todo\n{extra}---\n\nBody.\n")
    }

    #[test]
    fn retired_key_detection_matches_only_the_exact_key_name() {
        // The detector parses the frontmatter and looks for a top-level `links`
        // key, because the key is not part of the model and so cannot be seen
        // after deserialization. A key that merely *contains* `links` is not the
        // retired key, and a line inside a block scalar is content, not a key.
        for (extra, expect) in [
            // Populated: reported, because the value is no longer readable.
            ("links:\n- '12'\n", true),
            ("related_links: []\n", false),
            ("links_count: 3\n", false),
            ("# links: []\n", false),
            // A quoted key is still the same key, so it IS the retired key —
            // the earlier line scan missed these entirely.
            ("\"links\":\n- '12'\n", true),
            // Empty: present, but nothing to lose, so not reported. Reporting it
            // rejected every issue file this repository has ever written.
            ("links: []\n", false),
            ("links: ~\n", false),
            ("priority: medium\n", false),
            // A nested `links:` is not the retired top-level key.
            ("meta:\n  links: []\n", false),
            // A line inside a block scalar is content, not a key.
            ("notes: |\n  links: see the docs\n", false),
        ] {
            let issue = Issue::parse(&frontmatter_with(extra)).unwrap();
            assert_eq!(
                issue.has_retired_links_key, expect,
                "frontmatter line {extra:?}"
            );
        }
    }

    #[test]
    fn retired_key_detection_agrees_with_itself_on_the_fallback_path() {
        // The line-scan fallback is reachable: `serde_yaml` rejects duplicate
        // keys when building a `Mapping` but `Issue` tolerates a duplicate
        // *unknown* key, so such a file parses and lands on the fallback. Both
        // paths must agree that a nested `links:` is not the retired key — the
        // fallback used to match it at any indentation and disagree with the
        // primary path, so the same file was reported or not depending on which
        // path ran.
        for (extra, expect) in [
            ("meta:\n  links: []\n", false),
            ("meta:\n  links: []\nzzz: 1\nzzz: 2\n", false),
            ("links: []\nzzz: 1\nzzz: 2\n", true),
        ] {
            let issue = Issue::parse(&frontmatter_with(extra)).unwrap();
            assert_eq!(
                issue.has_retired_links_key, expect,
                "frontmatter {extra:?} must be judged the same way on both paths"
            );
        }
    }

    #[test]
    fn a_history_heading_inside_prose_stays_prose() {
        // Only a section at the *front* of the body is the History section, the
        // position data-model §8 pins. A heading further down is ordinary prose
        // and must survive a read/write round trip untouched.
        let raw = "---\nnumber: 1\ntitle: T\nstatus: todo\n---\n\nProse first.\n\n## History\n\n- 2026-01-01T00:00:00Z \u{2014} a \u{2014} relationship_added \u{2014} d\n";
        let issue = Issue::parse(raw).unwrap();
        assert!(
            issue.history.is_empty(),
            "a mid-body heading is not History"
        );
        assert!(
            issue.body.contains("## History"),
            "and it stays in the body: {:?}",
            issue.body
        );
        // And it round-trips as prose, not as a section.
        let rendered = issue.to_file_contents();
        assert!(rendered.contains("## History"));
        assert_eq!(
            Issue::parse(&rendered).unwrap().to_file_contents(),
            rendered
        );
    }

    /// The three heading cases the cycle-7 review found **unpinned**: a trailing
    /// tab, leading whitespace, and extra words after the heading. All three
    /// behave correctly — they are now pinned so the next change to the heading
    /// rule has to move a test rather than a comment.
    ///
    /// The pair that matter is the tab against the leading space, because they sit
    /// on opposite sides of the stated rule ("trailing whitespace is ignored;
    /// leading whitespace is not") and nothing tested it.
    #[test]
    fn the_heading_rule_ignores_trailing_whitespace_and_nothing_else() {
        let event = "2026-01-01T00:00:00Z \u{2014} a \u{2014} relationship_added \u{2014} d";
        let with = |heading: &str| {
            format!("---\nnumber: 1\ntitle: T\nstatus: todo\n---\n\n{heading}\n\n- {event}\n")
        };

        for (heading, recognised, why) in [
            ("## History", true, "the exact heading"),
            (
                "## History\t",
                true,
                "a trailing tab is trailing whitespace",
            ),
            ("## History  ", true, "so are trailing spaces"),
            (
                "   ## History",
                false,
                "leading whitespace is explicitly not ignored",
            ),
            (
                "## History extra words",
                false,
                "nothing else may introduce the section",
            ),
        ] {
            let issue = Issue::parse(&with(heading)).unwrap();
            assert_eq!(issue.history.is_empty(), !recognised, "{heading:?} — {why}");
            if recognised {
                assert_eq!(issue.history.len(), 1, "{heading:?} holds one event");
            } else {
                assert!(
                    issue.body.contains("History"),
                    "{heading:?} stays body prose: {:?}",
                    issue.body
                );
            }
        }
    }

    /// The two non-recognised heading shapes above are **undiagnosed**: `tix lint`
    /// reports a mispositioned `## History` only when the heading matches exactly,
    /// so `   ## History` and `## History extra words` fall through both checks
    /// silently.
    ///
    /// That is defensible rather than a defect — neither is a section, so there is
    /// no audit trail to lose, and §8 already says a heading appearing in an
    /// issue's prose is ordinary text. It is pinned here so the silence is a
    /// recorded decision: if a future change makes either shape *recognised*, this
    /// test is where that has to be noticed, because `tix lint` will not notice.
    #[test]
    fn a_non_section_history_heading_is_undiagnosed() {
        for heading in ["   ## History", "## History extra words"] {
            let raw =
                format!("---\nnumber: 1\ntitle: T\nstatus: todo\n---\n\n{heading}\n\nBody.\n");
            let issue = Issue::parse(&raw).unwrap();
            assert!(issue.history.is_empty(), "{heading:?} is not a section");
            // The misposition diagnostic keys on the exact heading, so it does not
            // fire for either of these.
            assert!(
                !is_history_heading(&format!(
                    "{heading}
"
                )),
                "{heading:?} is not the History heading at all"
            );
        }
    }

    /// A front-position section whose first list-shaped line is followed by prose
    /// is **emptied**: the heading and every line after it become body text, and
    /// nothing reports it.
    ///
    /// This is #1336, found while applying the §8 corrections for #1309. It is the
    /// same deferred damage §8's own asymmetry describes — the demoted text
    /// survives a plain write and is destroyed by the next `tix edit --body` — and
    /// it needs nothing more exotic than a hand-edited to-do bullet above real
    /// prose. Pinned here so the behaviour is recorded rather than discovered.
    #[test]
    fn a_front_position_section_can_be_emptied_by_an_earlier_line() {
        let raw = "---\nnumber: 9\ntitle: T\nstatus: todo\n---\n\n## History\n\n- step one\n\nNow the real body text, with no entry shape at all.\n\n- 2026-01-01 \u{2014} Bob \u{2014} Done\n";
        let issue = Issue::parse(raw).unwrap();
        // The section is recognised, then emptied: `is_history_heading` says the
        // heading is at the front, so the misposition diagnostic cannot fire, and
        // an empty section renders as nothing.
        assert!(
            issue.history.is_empty(),
            "the section is emptied: {:?}",
            issue.history
        );
        assert!(
            !issue.to_file_contents().contains("## History"),
            "and the heading is gone from the rendered file:\n{}",
            issue.to_file_contents()
        );
        // The demoted text is still in the body — the loss is deferred, not
        // immediate, which is what makes it look unrelated to the bullet.
        assert!(
            issue.body.contains("step one") && issue.body.contains("Done"),
            "the text survives as prose until the next `tix edit --body`: {:?}",
            issue.body
        );
    }

    #[test]
    fn a_blank_actor_round_trips_because_its_position_is_fixed() {
        // `render` writes an empty actor field when no identity was recorded. If
        // `parse` demanded a non-empty field it would reject the writer's own
        // output — and before the parse became non-fatal, that made the whole
        // file, and therefore every issue in the repository, unreadable.
        let event = HistoryEvent::new(
            "2026-01-01T00:00:00Z",
            "",
            HistoryEventKind::RelationshipAdded,
            "d",
        );
        let line = event.render();
        assert!(
            line.contains("\u{2014}  \u{2014}"),
            "render writes a blank, fixed-position actor field: {line}"
        );
        assert_eq!(
            HistoryEvent::parse(&line).expect("the writer's own output must parse"),
            event
        );
    }

    #[test]
    fn a_blank_line_inside_the_section_does_not_end_it() {
        // A blank line between list entries is the most ordinary thing a human
        // or a Markdown formatter produces. Terminating the section there would
        // silently demote audit entries to body prose — they would vanish from
        // `history`, reappear as the issue's description, and the next append
        // would write *before* them, so file order would stop being append
        // order.
        let raw = concat!(
            "---\n",
            "number: 1\n",
            "title: T\n",
            "status: todo\n",
            "---\n",
            "\n",
            "## History\n",
            "\n",
            "- 2026-01-01T00:00:00Z \u{2014} a \u{2014} relationship_added \u{2014} first\n",
            "\n",
            "- 2026-01-01T00:00:01Z \u{2014} a \u{2014} relationship_added \u{2014} second\n",
            "\n",
            "Tail prose.\n",
        );
        let issue = Issue::parse(raw).unwrap();
        assert_eq!(
            issue.history.events().len(),
            2,
            "both entries are History events, not one event plus prose"
        );
        assert_eq!(issue.history.unparsed().len(), 0);
        assert_eq!(
            issue.body, "Tail prose.\n",
            "the prose after the section is still the body"
        );
        // Append order is preserved: a new event lands last, not first.
        let mut issue = issue;
        issue
            .history
            .append_now("a", HistoryEventKind::RelationshipRemoved, "third");
        let rendered = issue.to_file_contents();
        let first = rendered.find("first").unwrap();
        let second = rendered.find("second").unwrap();
        let third = rendered.find("third").unwrap();
        assert!(
            first < second && second < third,
            "file order is append order"
        );
        assert_eq!(
            Issue::parse(&rendered).unwrap().to_file_contents(),
            rendered
        );
    }

    #[test]
    fn m3_body_bullets_after_the_section_stay_body() {
        // M3. The section has no end-of-section delimiter, and an earlier rule
        // accepted *any* `- ` line as an entry attempt. So ordinary prose
        // bullets were absorbed into the audit log: `body` became empty, the
        // blank line separating the section from the body was deleted on the
        // first write, and `tix lint` then blamed the author for "unparseable
        // entries" they never wrote. Trigger was a plain
        // `tix edit --body "- step one\n- step two"`.
        let raw = concat!(
            "---\n",
            "number: 1\n",
            "title: T\n",
            "status: todo\n",
            "---\n",
            "\n",
            "## History\n",
            "\n",
            "- 2026-01-01T00:00:00Z \u{2014} a \u{2014} relationship_added \u{2014} d1\n",
            "\n",
            "- Reproduction: run it\n",
            "- Expected: it works\n",
        );
        let issue = Issue::parse(raw).unwrap();
        assert_eq!(issue.history.events().len(), 1, "only the real event");
        assert!(
            issue.history.unparsed().is_empty(),
            "prose bullets are not mistaken for entries: {:?}",
            issue.history.unparsed()
        );
        assert_eq!(
            issue.body, "- Reproduction: run it\n- Expected: it works\n",
            "the bullets stay body prose"
        );
        // And the write does not restructure the file.
        let rendered = issue.to_file_contents();
        assert!(rendered.contains("- Reproduction: run it"));
        assert_eq!(
            Issue::parse(&rendered).unwrap().to_file_contents(),
            rendered
        );
    }

    #[test]
    fn m3_a_stray_line_inside_the_section_keeps_the_events_after_it() {
        // The mirror image of the case above, and the reason the boundary is
        // resolved by looking ahead rather than by shape alone. A hand-edited
        // non-entry line *inside* the section must not demote the real events
        // after it: they would vanish from the audit trail, reappear as the
        // issue's description, and the next append would be written *before*
        // them, so file order would stop being append order.
        let raw = concat!(
            "---\n",
            "number: 1\n",
            "title: T\n",
            "status: todo\n",
            "---\n",
            "\n",
            "## History\n",
            "\n",
            "- 2026-01-01T00:00:00Z \u{2014} a \u{2014} relationship_added \u{2014} d1\n",
            "  garbage hand edit   \n",
            "- 2026-01-01T00:00:01Z \u{2014} a \u{2014} relationship_added \u{2014} d2\n",
        );
        let mut issue = Issue::parse(raw).unwrap();
        assert_eq!(
            issue.history.events().len(),
            2,
            "the event after the stray line is still an event"
        );
        let unparsed = issue.history.unparsed();
        assert_eq!(unparsed.len(), 1, "and the stray line is kept, not dropped");
        // Compared **raw**. The earlier version trimmed before comparing, which
        // is why it could not see either the injected blank line or the loss of
        // the leading indentation and trailing spaces — the exact properties it
        // claimed to check. The fixture carries both on purpose.
        assert_eq!(
            unparsed[0], "  garbage hand edit   ",
            "preserved byte-for-byte, including indentation and trailing space"
        );
        // And the render must not have inserted a blank line of its own.
        let rendered = issue.to_file_contents();
        assert!(
            rendered.contains("  garbage hand edit   \n"),
            "re-emitted exactly: {:?}",
            rendered
        );
        assert!(
            !rendered.contains("garbage hand edit   \n\n"),
            "and no blank line is injected after it: {:?}",
            rendered
        );
        issue
            .history
            .append_now("bob", HistoryEventKind::RelationshipAdded, "d3");
        let rendered = issue.to_file_contents();
        let d1 = rendered.find("d1").unwrap();
        let d2 = rendered.find("d2").unwrap();
        let d3 = rendered.find("d3").unwrap();
        assert!(d1 < d2 && d2 < d3, "file order is still append order");
    }

    #[test]
    fn m3_a_bullet_that_looks_like_an_entry_is_recognised_for_diagnosis() {
        // An entry hand-typed with ASCII hyphens has no U+2014 separator, so it
        // cannot be recognised as an entry and must not be absorbed into the
        // body silently. `looks_like_history_entry` is what lets `tix lint`
        // report it.
        assert!(looks_like_history_entry(
            "- 2026-01-01T00:00:00Z \u{2014} a \u{2014} relationship_added \u{2014} d"
        ));
        // Recognition requires the separator. An entry typed with ASCII hyphens
        // is **not** recognised, deliberately: this predicate decides whether a
        // line is moved into the audit log, and the looser timestamp heuristic
        // absorbed ordinary body to-do items.
        assert!(
            !looks_like_history_entry("- 2026-01-01T00:00:00Z - a - relationship_added - d"),
            "an ASCII-hyphen line is not recognised, so it stays in the body"
        );
        assert!(
            !looks_like_history_entry("- Reproduction: run it"),
            "ordinary prose is not an entry"
        );
        assert!(!looks_like_history_entry("just a paragraph"));
        assert!(!looks_like_history_entry("-no space after the dash"));

        // The *reporting* predicate is separate and deliberately looser: it only
        // makes `tix lint` say something, it never moves a line.
        assert!(looks_like_an_unrecognised_entry(
            "- 2026-01-01T00:00:00Z - a - relationship_added - d"
        ));
        assert!(!looks_like_an_unrecognised_entry(
            "- 2026-01-01T00:00:00Z \u{2014} a \u{2014} relationship_added \u{2014} d"
        ));
        assert!(
            !looks_like_an_unrecognised_entry("- Reproduction: run it"),
            "and ordinary prose is still not accused"
        );
    }

    #[test]
    fn f5_every_list_marker_is_recognised_and_reported() {
        // Cycle-4, on the `*`/`1.` accept that cycle 3 recorded as "undiagnosed".
        // It was the *same* defect as F1 through a different marker: with only
        // `- ` treated as a list item, an entry written as
        // `* 2026-01-01T00:00:00Z — a — kind — d` was called prose, the section
        // ended, the heading was deleted, and the next append was rendered above
        // it. Silent, because a demoted line is body prose.
        //
        // This is not a widening of the entry grammar: `looks_like_history_entry`
        // still requires U+2014, so none of these can be read as a relationship.
        // It is about which lines get a fair hearing at end-of-section.
        // With the separator present but a non-`-` marker, the line is retained
        // verbatim and reported rather than parsed. Parsing it would re-emit it
        // with a `- ` marker, and rewriting the author's bytes is exactly the
        // auto-repair data-model §8 forbids. Retained-and-reported is the third
        // option between "parse and rewrite" and "demote and say nothing", and
        // it is the one that loses nothing.
        for marker in ["* ", "+ ", "1. ", "2) "] {
            let line = format!(
                "{marker}2026-01-01T00:00:00Z \u{2014} alice \u{2014} relationship_added \u{2014} d"
            );
            assert!(
                !looks_like_history_entry(&line),
                "{marker:?} is not the canonical form, so it is not an event"
            );
            let raw =
                format!("---\nnumber: 1\ntitle: T\nstatus: todo\n---\n\n## History\n\n{line}\n");
            let issue = Issue::parse(&raw).unwrap();
            assert!(
                issue.history.events().is_empty(),
                "{marker:?}: no event is fabricated"
            );
            assert_eq!(
                issue.history.unparsed(),
                vec![line.clone()],
                "{marker:?}: retained byte-for-byte, marker intact"
            );
            assert_eq!(issue.body, "", "{marker:?}: and not demoted to the body");
        }

        // The canonical marker with the separator is a real event, as before.
        let canonical =
            "- 2026-01-01T00:00:00Z \u{2014} alice \u{2014} relationship_added \u{2014} d";
        assert!(looks_like_history_entry(canonical));
        let canonical_raw =
            format!("---\nnumber: 1\ntitle: T\nstatus: todo\n---\n\n## History\n\n{canonical}\n");
        assert_eq!(
            Issue::parse(&canonical_raw).unwrap().history.events().len(),
            1,
            "the canonical form is an event"
        );

        // And every marker *without* the separator still gets a fair hearing
        // rather than being silently demoted.
        for marker in ["- ", "* ", "+ ", "1. "] {
            let line = format!("{marker}hand edited note");
            let raw =
                format!("---\nnumber: 1\ntitle: T\nstatus: todo\n---\n\n## History\n\n{line}\n");
            let issue = Issue::parse(&raw).unwrap();
            assert!(
                issue.history.events().is_empty(),
                "{marker:?}: no event is fabricated"
            );
            assert_eq!(
                issue.history.unparsed(),
                vec![line.clone()],
                "{marker:?}: retained verbatim in the section"
            );
            assert!(
                issue.body.is_empty(),
                "{marker:?}: and not demoted into the body, got {:?}",
                issue.body
            );
        }

        // The "and reported" half of this test's name, which the retention
        // assertions above did not check. Reporting for a line *inside* a section
        // goes through the unparsed loop; reporting for one in the *body* goes
        // through `looks_like_an_unrecognised_entry`. Both are asserted here so
        // the name is earned — cycle 5 found the retention half asserted and the
        // reporting half assumed.
        for marker in ["- ", "* ", "+ ", "1. "] {
            let line = format!("{marker}2026-01-01T00:00:00Z - alice - relationship_added - d");
            assert!(
                looks_like_an_unrecognised_entry(&line),
                "{marker:?}: reported as a mistyped entry"
            );
            // And the same line with a *different* marker shape is not, so the
            // assertion above is discriminating rather than trivially true.
            assert!(
                !looks_like_an_unrecognised_entry(&format!("2026. {line}")),
                "a bare sentence is never an entry"
            );
        }

        // A bare paragraph at end of section is still prose. That is the other
        // half of the rule and the reason `is_list_item` is not simply "true".
        for prose in [
            "just a closing paragraph",
            "Reproduction: run it",
            "see the thread for context",
        ] {
            let raw =
                format!("---\nnumber: 1\ntitle: T\nstatus: todo\n---\n\n## History\n\n{prose}\n");
            let issue = Issue::parse(&raw).unwrap();
            assert!(issue.history.is_empty(), "{prose:?}: no section content");
            assert_eq!(issue.body.trim(), prose, "{prose:?}: it is body prose");
        }
    }

    #[test]
    fn an_unparseable_entry_never_fails_the_parse() {
        // The single most important property of the History section: it is
        // advertised as human-editable, so a hand edit will eventually produce a
        // line this parser cannot read. That must cost the *author* a lint
        // finding on their own issue, not the whole repository its readability.
        // A recognised entry attempt whose kind is unknown: kept verbatim inside
        // the section, so `tix lint` can report it.
        let bad_kind = "- 2026-01-01T00:00:00Z \u{2014} alice \u{2014} bogus_kind \u{2014} d";
        let raw =
            format!("---\nnumber: 1\ntitle: T\nstatus: todo\n---\n\n## History\n\n{bad_kind}\n");
        let issue = Issue::parse(&raw).expect("one bad line must not fail the file");
        assert!(issue.history.events().is_empty(), "no event was fabricated");
        assert_eq!(issue.history.unparsed(), vec![bad_kind], "kept verbatim");
        assert_eq!(issue.body, "", "nothing leaked into the body");

        // An entry typed with ASCII hyphens is not *recognised*, but it is still a
        // list item at the end of the section, so it is retained verbatim and
        // reported rather than demoted. The earlier version of this test
        // asserted it stayed body text; cycle-4 F1 showed the alternative deletes
        // the `## History` heading and reorders the log around it, silently.
        // Retention is the disclosed failure mode, so it is the one chosen — and
        // `looks_like_history_entry` still returns false for it, so no event is
        // ever fabricated from it either.
        let ascii = "- 2026-01-01T00:00:00Z - alice - relationship_added - ascii";
        let raw2 =
            format!("---\nnumber: 1\ntitle: T\nstatus: todo\n---\n\n## History\n\n{ascii}\n");
        let issue2 = Issue::parse(&raw2).expect("and it must not fail the file either");
        assert!(issue2.history.events().is_empty(), "no event is fabricated");
        assert_eq!(issue2.history.unparsed(), vec![ascii], "kept verbatim");
        assert_eq!(issue2.body, "", "and not demoted into the body");

        let rendered = issue.to_file_contents();
        assert_eq!(
            Issue::parse(&rendered).unwrap().to_file_contents(),
            rendered,
            "and the file round-trips unchanged"
        );
    }

    #[test]
    fn a_history_entry_keeps_an_em_dash_inside_its_detail() {
        // The line is split on the first three separators only, so a detail
        // containing an em dash survives rather than being truncated.
        let raw = "---\nnumber: 1\ntitle: T\nstatus: todo\n---\n\n## History\n\n- 2026-01-01T00:00:00Z \u{2014} alice \u{2014} relationship_added \u{2014} note \u{2014} dash inside\n\nTail.\n";
        let issue = Issue::parse(raw).unwrap();
        assert_eq!(issue.history.len(), 1);
        let event = &issue.history.events()[0];
        assert_eq!(event.timestamp, "2026-01-01T00:00:00Z");
        assert_eq!(event.actor, "alice");
        assert_eq!(event.kind, HistoryEventKind::RelationshipAdded);
        assert_eq!(event.detail, "note \u{2014} dash inside");
        // Idempotence rather than equality with the hand-written input: the
        // input omits keys serde always emits, so only the rendered form is a
        // meaningful byte-stability assertion.
        let rendered = issue.to_file_contents();
        assert_eq!(
            Issue::parse(&rendered).unwrap().to_file_contents(),
            rendered,
            "the entry round-trips byte-identically"
        );
        assert_eq!(
            Issue::parse(&rendered).unwrap().history.events()[0].detail,
            "note \u{2014} dash inside",
            "and the detail is stable across the round trip"
        );
    }

    // -----------------------------------------------------------------------
    // WP02 / T006 + T007 — the `parent` field and its three History kinds
    // -----------------------------------------------------------------------
    //
    // Requirement ids in the test names: C-001 (at most one parent), C-003 (no
    // child data on the parent), C-004 (every change audited), C-007 (a parent is
    // not a dependency edge), SC-004 (a rejected change writes nothing).

    /// A file body, so a test's only variable is the frontmatter under test.
    fn wp02_file(fm: &str) -> String {
        format!("---\n{fm}---\n\nBody text.")
    }

    /// The frontmatter every legacy file in the repository carries: no `parent:`
    /// key at all. This is the pinned pre-mission byte form.
    ///
    /// In the **canonical rendered form**, not a merely plausible one. The writer
    /// unquotes strings and always emits `files: []`, so a hand-written fixture
    /// with quotes in it is not byte-stable for reasons that have nothing to do
    /// with `parent` — the first version of these tests failed for exactly that
    /// reason and was measuring the renderer rather than the field.
    const WP02_LEGACY_FM: &str = "number: 3\ntitle: Legacy issue\nstatus: todo\nlabels: []\npriority: medium\nfiles: []\ncreated: 2026-01-01\nupdated: 2026-01-01\n";

    /// The same file, with a parent, canonical.
    const WP02_PARENT_FM: &str = "number: 3\ntitle: Child\nstatus: todo\nlabels: []\npriority: medium\nparent: 7\nfiles: []\ncreated: 2026-01-01\nupdated: 2026-01-01\n";

    #[test]
    fn t006_a_file_with_a_parent_round_trips_byte_stably() {
        let original = wp02_file(WP02_PARENT_FM);
        let issue = Issue::parse(&original).expect("a parent parses");
        assert_eq!(issue.parent, Some(7), "the parent number is read as data");

        // load -> save -> load, byte-identical at every hop.
        let once = issue.to_file_contents();
        assert_eq!(
            once, original,
            "the parent key is written where it was read"
        );
        let twice = Issue::parse(&once).unwrap().to_file_contents();
        assert_eq!(twice, once, "a second round trip changes nothing");
    }

    #[test]
    fn t006_a_legacy_file_with_no_parent_key_serialises_to_its_pre_mission_bytes() {
        // The pinned legacy gate. If this fails, the field is leaking into files
        // that never had one.
        let original = wp02_file(WP02_LEGACY_FM);
        let issue = Issue::parse(&original).unwrap();
        assert_eq!(issue.parent, None, "an absent key is no parent");
        let rendered = issue.to_file_contents();
        assert_eq!(rendered, original);
        assert!(
            !rendered.contains("parent:"),
            "a file that never had a parent must not grow a `parent:` key"
        );
        assert_eq!(
            Issue::parse(&rendered).unwrap().to_file_contents(),
            rendered
        );
    }

    #[test]
    fn t006_the_parent_key_is_written_exactly_once() {
        // Regression: `parent` was added to [`Issue`] but not to
        // [`KNOWN_FRONTMATTER_KEYS`], so `capture_extra_frontmatter` also took it
        // for an unknown key and the writer emitted the key twice in one file. The
        // file still parsed. A count assertion is what notices.
        let rendered = Issue::parse(&wp02_file(WP02_PARENT_FM))
            .unwrap()
            .to_file_contents();
        assert_eq!(
            rendered.matches("\nparent:").count(),
            1,
            "`parent:` must be written once, got:\n{rendered}"
        );
        let reparsed = Issue::parse(&rendered).unwrap();
        assert_eq!(reparsed.to_file_contents(), rendered);
    }

    #[test]
    fn known_frontmatter_keys_cover_every_written_key() {
        // The guard for `KNOWN_FRONTMATTER_KEYS`, which is a hand-maintained mirror
        // of the frontmatter field set — no macro can read a struct's own field
        // names. Both of its failure modes are silent, and in opposite directions:
        //
        // * a key **listed but not modelled** is dropped on write (data loss, the
        //   bug #1361 exists to remove);
        // * a key **modelled but not listed** is also captured as unknown and emitted
        //   a *second* time, so the file carries the same key twice. That is what
        //   `parent` did.
        //
        // The first version of this test was a one-directional subset check with a
        // fixture that left `relationships`, `cycles` and `milestones` unpopulated.
        // Deleting `"relationships"` from the list left it **green** while the
        // double-write defect was live, and so did adding a brand-new unregistered
        // field — both measured and confirmed. A test that cannot go red in the
        // states it claims to rule out is worse than none, because it is read as
        // proof.
        //
        // So this asserts **both directions**, populates every optional field so
        // the key set is complete, and adds two direct checks that do not consult
        // the list at all — the symptom, not the cause.
        let mut issue = Issue::parse(&wp02_file(
            "number: 1\ntitle: T\nstatus: todo\nlabels: []\npriority: medium\nfiles: []\ncreated: 2026-01-01\nupdated: 2026-01-01\n",
        ))
        .unwrap();
        issue.assignee = Some("@alice".into());
        issue.estimate = Some("1h".into());
        issue.commit = Some("abc1234".into());
        issue.gitlab_id = Some("7".into());
        issue.linear_id = Some("9".into());
        issue.mission = Some("m".into());
        issue.task = Some("T001".into());
        issue.reviewer = Some("@bob".into());
        issue.start_date = Some("2026-01-01".into());
        issue.due_date = Some("2026-02-01".into());
        issue.entered_at = Some("2026-03-01T10:00:00Z".into());
        issue.source = Some("import".into());
        issue.parent = Some(2);
        issue.cycles = Some(vec!["A".into()]);
        issue.milestones = Some(vec!["m".into()]);
        issue.files = vec!["docs/a.md".into()];
        issue.labels = vec!["one".into(), "two".into()];
        issue.relationships = vec![Relationship::new(1, 2, RelationKind::Blocks, "T")];

        let frontmatter = issue.frontmatter();
        let written: Vec<String> = frontmatter.lines().filter_map(top_level_key).collect();

        // -- The direct checks first ------------------------------------------
        //
        // These do not consult `KNOWN_FRONTMATTER_KEYS` at all, so they hold
        // whatever the list says, and they are what actually catches the defect:
        // the symptom rather than the cause. Ordered first so a regression reports
        // the real problem instead of a completeness count.
        //
        // A fully-populated canonical file must have **no unknown keys**. A key the
        // model owns but the list omits is captured as unknown and will be emitted
        // a second time.
        assert_eq!(
            capture_extra_frontmatter(&frontmatter),
            BTreeMap::new(),
            "a fully-populated canonical file must have NO unknown keys: a key the \
             model owns is being captured as unknown and will be emitted twice"
        );

        // And no top-level key may appear twice in a rendered file. This is the
        // defect stated as an assertion, with nothing to maintain but the writer.
        let mut sorted = written.clone();
        sorted.sort();
        let before = sorted.len();
        sorted.dedup();
        assert_eq!(
            before,
            sorted.len(),
            "a key is written more than once in:\n{frontmatter}"
        );

        // -- Direction 1: every written key is registered -----------------------
        for key in &written {
            assert!(
                KNOWN_FRONTMATTER_KEYS.contains(&key.as_str()),
                "`{key}` is written by the model but absent from KNOWN_FRONTMATTER_KEYS, \
                 so capture_extra_frontmatter treats it as unknown and the writer emits \
                 it twice. Add it to the list."
            );
        }

        // -- Direction 2: every registered key is one the model can write -------
        //
        // The opposite silent failure: a key listed but not modelled is dropped on
        // write, which is data loss.
        for key in KNOWN_FRONTMATTER_KEYS {
            if *key == "links" {
                continue; // Recognised but never re-emitted, by design (that C-001).
            }
            assert!(
                written.iter().any(|k| k == key),
                "`{key}` is listed as known but no field writes it, so the entry is \
                 wrong in the other direction: a key wrongly listed as known is \
                 dropped on write. Either model it or remove it from the list."
            );
        }

        // -- Completeness floor -------------------------------------------------
        //
        // Last, because it is a statement about the *fixture*, not about the code:
        // if a field is added and this test stops populating it, the two assertions
        // above silently cover less. Asserted so that cannot pass unnoticed.
        assert_eq!(
            written.len(),
            KNOWN_FRONTMATTER_KEYS.len() - 1,
            "the fixture must exercise every known key except the retired one; \
             got {written:?} against {KNOWN_FRONTMATTER_KEYS:?}"
        );

        for key in KNOWN_FRONTMATTER_KEYS {
            assert!(
                !key.is_empty() && !key.contains(char::is_whitespace),
                "`{key}` is not a usable top-level key"
            );
        }
    }

    #[test]
    fn t006_an_explicitly_null_parent_is_the_absent_state() {
        // `parent:` with no value is the "no parent" state. It is dropped on
        // write, matching every other optional field in this frontmatter
        // (`assignee:`, `commit:`). Pinned rather than assumed, because the
        // legacy gate above is what would catch the blast radius if it changed.
        let issue =
            Issue::parse(&wp02_file("number: 3\ntitle: T\nstatus: todo\nparent:\n")).unwrap();
        assert_eq!(issue.parent, None);
        assert!(!issue.to_file_contents().contains("parent:"));
    }

    #[test]
    fn t006_an_empty_string_parent_is_rejected_rather_than_read_as_a_reference() {
        // A YAML `""` is a *string*, not a number. Coercing it to `None` would
        // erase a real reference that a template or an editor emptied by
        // mistake, and the file would then read as a deliberate "no parent".
        let err = Issue::parse(&wp02_file("number: 3\ntitle: T\nparent: \"\"\n"))
            .expect_err("an empty-string parent is malformed, not a reference");
        assert!(
            err.to_string().contains("parent"),
            "the error must name the offending field, got: {err}"
        );
    }

    #[test]
    fn t006_a_malformed_parent_value_is_rejected() {
        // The spellings that are genuinely rejected. Every one of these would be a
        // reference the user did not write if the parser were lenient about them.
        for (label, value) in [
            ("an empty string", "\"\""),
            ("a single-quoted number", "'7'"),
            ("a zero-padded number", "007"),
            ("an underscore-separated number", "1_000"),
            ("a float", "7.5"),
            ("a float written as an integer", "7.0"),
            ("scientific notation", "1e2"),
            ("a list", "[7]"),
            ("a map", "{a: 1}"),
            ("a bool", "true"),
            ("a negative", "-1"),
            ("a value beyond u32", "4294967296"),
        ] {
            let parsed = Issue::parse(&wp02_file(&format!(
                "number: 3\ntitle: T\nparent: {value}\n"
            )));
            assert!(
                parsed.is_err(),
                "{label} (`parent: {value}`) must be rejected, got {parsed:?}"
            );
        }
    }

    #[test]
    fn t006_the_accepted_scalar_forms_are_pinned_because_they_are_surprising() {
        // An earlier version of the malformed-value test claimed "none may become
        // `Some(_)`" and listed seven spellings. It was **false**: measured, five
        // more scalar forms are read as numbers.
        //
        // Pinned here rather than left unstated, so the behaviour is a recorded
        // decision instead of a YAML accident. Whether these should be *accepted*
        // is an open question (filed as #1372): tightening the accepted set is a
        // parse-semantics change and belongs in a contract clause, not in a test
        // edit made quietly inside WP02.
        //
        // `!!str 7` is the sharp one. A value explicitly tagged as a *string* is read
        // as a number — the same class the empty-string test refuses. `0x1F`, `0o17`
        // and `0b111` are alternative radices, and `+7` is a leading sign.
        //
        // `parent: 0` deserves a note, because an earlier version of this comment
        // claimed "`0` is accepted although issue numbers start at 1" and a later
        // one said the reverse. The history: the claim was *wrong* when written —
        // `Store::resolve_next_number` returned `Ok(0)` for an empty directory, so
        // the first issue in every fresh repository was `#0`. Found by running the
        // binary, not by reading the code (tix #1376).
        //
        // `resolve_next_number` now starts at `1`, so the original comment is
        // finally accurate. **`0` still parses, and must.** The parser accepts any
        // `u32`; rejecting `0` here would contradict it, and an existing repository
        // can already contain an issue numbered `0` (this one does — it was written
        // before the fix, and renumbering it is a migration). Rejecting the value
        // would also refuse to *read* such a file, turning a numbering convention
        // into data loss.
        //
        // The general rule this test pins: the parser is not the allocator. What
        // `resolve_next_number` may *produce* is a separate question with a
        // separate answer, and this list asserts only that both `0` and `7` are
        // numbers.
        for (value, expected) in [
            ("0x1F", 31),
            ("0o17", 15),
            ("0b111", 7),
            ("+7", 7),
            ("!!str 7", 7),
            ("0", 0),
        ] {
            let issue = Issue::parse(&wp02_file(&format!(
                "number: 3\ntitle: T\nparent: {value}\n"
            )))
            .unwrap_or_else(|e| panic!("`parent: {value}` is expected to parse: {e}"));
            assert_eq!(
                issue.parent,
                Some(expected),
                "`parent: {value}` is pinned to {expected}"
            );
        }

        // And the two spellings of "no parent" that are not a null literal.
        for value in ["null", "~"] {
            let issue = Issue::parse(&wp02_file(&format!(
                "number: 3\ntitle: T\nparent: {value}\n"
            )))
            .unwrap();
            assert_eq!(issue.parent, None, "`parent: {value}` is the absent state");
        }
    }

    #[test]
    fn t006_at_most_one_parent_is_expressible_because_the_field_is_not_a_list() {
        // C-001 is a *shape* guarantee, so it is asserted as a shape: a second
        // `parent:` key in the file is a duplicate-mapping error, not a silent
        // last-one-wins.
        let err = Issue::parse(&wp02_file("number: 3\ntitle: T\nparent: 7\nparent: 8\n"))
            .expect_err("two parents are not representable");
        assert!(err.to_string().contains("parent"), "got: {err}");
    }

    #[test]
    fn t006_the_parent_key_is_distinct_from_mission_and_task() {
        // C-007 / research R2: a decomposition parent is not the Spec Kitty
        // governance pointer, and not a typed dependency edge. Asserted on the
        // *rendered keys*, not the struct fields, because only the keys are the
        // contract.
        let mut issue = Issue::parse(&wp02_file(
            "number: 3\ntitle: T\nstatus: todo\nlabels: []\npriority: medium\nmission: some-mission\ntask: T003\nparent: 7\nfiles: []\ncreated: 2026-01-01\nupdated: 2026-01-01\n",
        ))
        .unwrap();
        // Set after parsing, because a typed relationship carries a generated id
        // that a hand-written fixture cannot supply. Without this the rendered file
        // has no `relationships:` key at all — an empty list is skipped — and the
        // assertion below would pass for the wrong reason.
        issue.relationships = vec![Relationship::new(3, 8, RelationKind::Blocks, "Eight")];
        assert!(
            issue.relationships.len() == 1,
            "the fixture must carry a relationship under test"
        );
        let fm = issue.frontmatter();
        // `relationships:` is in this list on purpose. WP02's own review guidance
        // asks whether the parent key is distinct from `mission`, `task` **and the
        // typed relationship key**; the first version of this test named only the
        // first two.
        for key in ["mission:", "task:", "relationships:", "parent:"] {
            assert_eq!(
                fm.matches(key).count(),
                1,
                "`{key}` must appear exactly once, got:\n{fm}"
            );
        }
        assert_eq!(issue.mission.as_deref(), Some("some-mission"));
        assert_eq!(issue.task.as_deref(), Some("T003"));
        assert_eq!(issue.parent, Some(7));

        // And clearing one leaves the others alone: they are not aliases.
        issue.parent = None;
        let fm = issue.frontmatter();
        assert!(!fm.contains("parent:"));
        assert!(fm.contains("mission: some-mission"));
        assert!(fm.contains("task: T003"));
    }

    #[test]
    fn t007_the_three_parent_event_kinds_round_trip_through_the_file() {
        // C-004: the audit trail is only an audit trail if the kinds survive it.
        for (kind, spelling) in [
            (HistoryEventKind::ParentSet, "parent_set"),
            (HistoryEventKind::ParentChanged, "parent_changed"),
            (HistoryEventKind::ParentCleared, "parent_cleared"),
        ] {
            assert_eq!(kind.as_str(), spelling);
            assert_eq!(
                HistoryEventKind::from_str(spelling).unwrap(),
                kind,
                "`{spelling}` must parse back to the same kind"
            );
        }
        // An unknown kind is rejected rather than coerced (this module's own
        // rule), so a typo in a hand-edited log cannot be quietly reclassified.
        assert!(HistoryEventKind::from_str("parent_changed_to").is_err());

        // Pin the *serde* spelling too. The enum derives `Serialize`/`Deserialize`
        // under `rename_all = "snake_case"` **and** carries `as_str`/`from_str`, so
        // there are two independent sources of the wire spelling and nothing tied
        // them together. `History` is `#[serde(skip)]` on `Issue`, so the derive is
        // currently unused on the file path — which is exactly why it could drift
        // unnoticed if the history ever became serialised.
        for (kind, spelling) in [
            (HistoryEventKind::ParentSet, "parent_set"),
            (HistoryEventKind::ParentChanged, "parent_changed"),
            (HistoryEventKind::ParentCleared, "parent_cleared"),
        ] {
            let yaml = serde_yaml::to_string(&kind).unwrap();
            assert_eq!(
                yaml.trim(),
                spelling,
                "the derived serde spelling must match `as_str`"
            );
            let back: HistoryEventKind = serde_yaml::from_str(&yaml).unwrap();
            assert_eq!(back, kind, "`{spelling}` must round trip through serde");
        }
    }

    #[test]
    fn t007_parent_events_survive_a_history_section_round_trip() {
        let original = "---\nnumber: 3\ntitle: Child\nstatus: todo\nlabels: []\npriority: medium\nparent: 7\nfiles: []\ncreated: 2026-01-01\nupdated: 2026-01-01\n---\n\n## History\n\n- 2026-01-02T03:04:05Z \u{2014} alice \u{2014} parent_set \u{2014} parent none -> #7\n\nProse after the section.\n";
        let issue = Issue::parse(original).unwrap();
        assert_eq!(issue.history.len(), 1);
        assert_eq!(issue.history.events()[0].kind, HistoryEventKind::ParentSet);
        assert_eq!(issue.to_file_contents(), original, "byte-stable");
    }

    #[test]
    fn t007_issue_view_carries_the_parent_so_both_services_read_one_projection() {
        // The hierarchy service and the relationship service must not each build
        // their own view of an issue, or they can disagree about the same
        // snapshot.
        let issue = Issue::parse(&wp02_file(WP02_PARENT_FM)).unwrap();
        let view = issue.issue_view();
        assert_eq!(view.parent, Some(7));
        assert_eq!(
            crate::hierarchy::HierarchyNode::parent_number(&view),
            Some(7)
        );
    }

    // ------------------------------------------------------------------
    // WP03 T012 — the TransitionDecision record and its detail grammar
    // ------------------------------------------------------------------

    fn a_decision() -> TransitionDecision {
        TransitionDecision::new(
            "2026-09-29T12:00:00Z",
            "Herbert Maschke <thyseus@pm.me>",
            "todo",
            "in progress",
            "unstarted",
            "active",
            None,
        )
    }

    #[test]
    fn t012_the_kind_spells_parses_and_renders() {
        assert_eq!(
            HistoryEventKind::TransitionAccepted.as_str(),
            "transition_accepted"
        );
        assert_eq!(
            "transition_accepted".parse::<HistoryEventKind>().unwrap(),
            HistoryEventKind::TransitionAccepted
        );
        assert_eq!(
            HistoryEventKind::TransitionAccepted.to_string(),
            "transition_accepted"
        );
    }

    #[test]
    fn t012_an_unknown_kind_is_rejected_rather_than_coerced() {
        // T012 step 6. A silently reclassified audit entry is worse than a parse
        // failure, so this is the same contract the other kinds have — pinned
        // because the new kind is the newest addition to a closed set and the set
        // is exactly where a `catch-all => Ok(..)` would be introduced.
        let err = "transition_attempted"
            .parse::<HistoryEventKind>()
            .unwrap_err();
        assert!(
            err.to_string().contains("unknown History event kind"),
            "{err}"
        );
    }

    #[test]
    fn t012_the_detail_grammar_renders_and_recovers_every_field() {
        let decision = a_decision();
        assert_eq!(
            decision.detail(),
            "todo -> in progress (unstarted -> active)",
            "the rendered form is the contract, so it is asserted literally"
        );
        let recovered = TransitionDecision::from_event(&decision.to_event()).unwrap();
        assert_eq!(recovered, decision, "a lossless round trip");
    }

    #[test]
    fn t012_an_override_reason_round_trips() {
        let mut decision = a_decision();
        decision.override_reason = Some("reopening: filed against the wrong epic".into());
        assert_eq!(
            decision.detail(),
            "todo -> in progress (unstarted -> active); override: reopening: filed \
             against the wrong epic"
        );
        let recovered = TransitionDecision::from_event(&decision.to_event()).unwrap();
        assert_eq!(
            recovered.override_reason.as_deref(),
            Some("reopening: filed against the wrong epic")
        );
    }

    #[test]
    fn t012_a_reason_containing_the_bare_marker_survives() {
        // The reason is split off at the first `") ; override: "`, so a reason may
        // contain the bare `"; override: "` without being truncated. This is the
        // test that caught the first implementation, which used `rsplit_once` on
        // the *bare* marker: the last occurrence inside the reason is the one that
        // split, leaving a head with no closing paren and failing to parse at all.
        let mut decision = a_decision();
        decision.override_reason = Some("typo; override: actually the right one".into());
        let recovered = TransitionDecision::from_event(&decision.to_event()).unwrap();
        assert_eq!(
            recovered.override_reason.as_deref(),
            Some("typo; override: actually the right one")
        );
        assert_eq!(recovered.to, "in progress", "and the fields still line up");
    }

    #[test]
    fn t012_a_reason_containing_the_anchored_marker_survives() {
        // The discriminating test. `split_once` and `rsplit_once` agree on every
        // other input — the anchored marker appears exactly once unless the reason
        // contains it — so nothing else in this module can tell them apart, and the
        // first version of `t012_a_reason_containing_the_bare_marker_survives`
        // passed against both. This one cannot.
        let mut decision = a_decision();
        decision.override_reason = Some("quoting it: ); override: and again".into());
        let recovered = TransitionDecision::from_event(&decision.to_event()).unwrap();
        assert_eq!(
            recovered.override_reason.as_deref(),
            Some("quoting it: ); override: and again"),
            "the FIRST anchored marker is the split point, and the reason is the rest"
        );
        assert_eq!(recovered.from, "todo");
        assert_eq!(recovered.to, "in progress");
    }

    #[test]
    fn t012_a_reason_containing_an_arrow_survives() {
        let mut decision = a_decision();
        decision.override_reason = Some("note: a -> b was meant to be a -> c".into());
        let recovered = TransitionDecision::from_event(&decision.to_event()).unwrap();
        assert_eq!(
            recovered.override_reason.as_deref(),
            Some("note: a -> b was meant to be a -> c")
        );
        assert_eq!(
            recovered.from, "todo",
            "the head is unaffected by the reason"
        );
        assert_eq!(recovered.to, "in progress");
    }

    #[test]
    fn t012_a_blank_reason_renders_as_no_marker_at_all() {
        // A blank reason must not come back as `Some("")`, which would make
        // `from_event` and `detail()` disagree about whether an override exists.
        let mut decision = a_decision();
        decision.override_reason = Some("   ".into());
        assert_eq!(
            decision.detail(),
            "todo -> in progress (unstarted -> active)"
        );
        let recovered = TransitionDecision::from_event(&decision.to_event()).unwrap();
        assert!(recovered.override_reason.is_none());
    }

    #[test]
    fn t012_a_column_named_with_an_arrow_is_a_documented_limit_not_a_silent_mis_split() {
        // WP03's doc comment originally claimed "a column named `a -> b` cannot
        // desynchronise the fields". It **can**, and this test is why that claim
        // was struck: the grammar is delimiter-based with no escaping, so `from`
        // parses as `"a"`. The limit is inherent — any unescaped separator has a
        // name it cannot represent — and is now documented on the type.
        //
        // What matters downstream is pinned here: the **categories** still parse
        // correctly, because they come from the bracketed tail rather than from
        // the arrow-separated head.
        let mut decision = a_decision();
        decision.from = "a -> b".into();
        decision.to = "in progress".into();
        let recovered = TransitionDecision::from_event(&decision.to_event()).unwrap();
        assert_eq!(recovered.from, "a", "the documented mis-split, pinned");
        assert_eq!(recovered.to_category, "active", "categories stay intact");
        assert_eq!(recovered.from_category, "unstarted");
    }

    #[test]
    fn t012_a_malformed_detail_is_an_error_not_a_partial_record() {
        for bad in [
            "no arrows here",
            "todo -> in progress",             // no bracketed categories
            "todo -> in progress (unstarted)", // no category arrow
            "todo -> in progress (unstarted -> active", // no closing paren
            " -> in progress (unstarted -> active)", // empty from
        ] {
            let event = HistoryEvent::new(
                "2026-09-29T12:00:00Z",
                "alice",
                HistoryEventKind::TransitionAccepted,
                bad,
            );
            assert!(
                TransitionDecision::from_event(&event).is_err(),
                "`{bad}` must not parse into a partial record"
            );
        }
    }

    #[test]
    fn t012_from_event_refuses_a_different_kind() {
        let event = HistoryEvent::new(
            "2026-09-29T12:00:00Z",
            "alice",
            HistoryEventKind::ParentSet,
            "todo -> in progress (unstarted -> active)",
        );
        let err = TransitionDecision::from_event(&event).unwrap_err();
        assert!(
            err.to_string().contains("not a `transition_accepted`"),
            "a parent event is not a transition, however well-formed its detail: {err}"
        );
    }

    #[test]
    fn t012_the_decision_survives_a_full_file_round_trip_byte_for_byte() {
        let mut decision = a_decision();
        decision.override_reason = Some("reopening".into());
        const FILE: &str = "---\nnumber: 1\ntitle: \"T\"\nstatus: \"in progress\"\nlabels: []\npriority: \"medium\"\nfiles: []\ncreated: \"2026-01-01\"\nupdated: \"2026-01-01\"\n---\n\nBody.";
        let mut issue = Issue::parse(FILE).unwrap();
        issue.history.append(decision.to_event());
        let rendered = issue.to_file_contents();
        let reparsed = Issue::parse(&rendered).unwrap();
        assert_eq!(
            reparsed.to_file_contents(),
            rendered,
            "byte-stable through the file, like every other History kind"
        );
        let events = reparsed.history.events();
        assert_eq!(events.len(), 1);
        assert_eq!(
            TransitionDecision::from_event(events[0]).unwrap(),
            decision,
            "and every field survives the file, not just the rendered text"
        );
    }

    #[test]
    fn t012_a_same_column_record_is_recognisable_as_a_no_op() {
        // A hand-edited file can contain one of these even though the seam never
        // writes one; WP06 must be able to tell that apart from a real move.
        let mut decision = a_decision();
        decision.to = "todo".into();
        assert!(decision.is_no_op());
        let mut real = decision.clone();
        real.to = "in progress".into();
        assert!(!real.is_no_op());
    }
}