tix-tracker 0.7.0

use std::time::{SystemTime, UNIX_EPOCH};

use crate::assignee::parse_assignee;
use crate::commands::{parse_priority, parse_selector};
use crate::config::Config;
use crate::editor::{FieldEdits, merge_fields, resolve_editor, run_editor};
use crate::error::{Result, TixError};
use crate::hierarchy;
use crate::model::HistoryEventKind;
use crate::model::Issue;
use crate::storage::Store;

/// `tix edit <issue>`: with flags, merge them into the frontmatter via
/// `merge_fields` and rewrite the file; with no flags, open `$EDITOR` on a
/// temporary copy, parse the result and commit it back. A parse failure is
/// reported with the parser's own diagnostic, writes nothing, and keeps the
/// copy it names (tix #1506).
///
/// `--assignee` is special (FR-031): the merged field is run through
/// `store.reassign`, which *moves* the file into the new assignee segment
/// (or `unassigned/` for `--assignee ""`). The success line always prints the
/// relative path the issue lives at afterwards.
#[allow(clippy::too_many_arguments)]
pub fn run(
    config: &Config,
    store: &Store,
    raw: &str,
    title: Option<&str>,
    body: Option<&str>,
    status: Option<&str>,
    priority: Option<&str>,
    assignee: Option<&str>,
    labels: Option<&str>,
    files: Option<&str>,
    estimate: Option<&str>,
    cycles: Option<&str>,
    milestones: Option<&str>,
    reviewer: Option<&str>,
    start_date: Option<&str>,
    due_date: Option<&str>,
    commit: Option<&str>,
    parent: Option<&str>,
    override_reason: Option<&str>,
    related_issues: Option<&str>,
    type_: Option<&str>,
) -> Result<()> {
    if title.is_some()
        || body.is_some()
        || status.is_some()
        || priority.is_some()
        || assignee.is_some()
        || labels.is_some()
        || files.is_some()
        || estimate.is_some()
        || cycles.is_some()
        || milestones.is_some()
        || reviewer.is_some()
        || start_date.is_some()
        || due_date.is_some()
        || commit.is_some()
        || parent.is_some()
        || related_issues.is_some()
        || type_.is_some()
    {
        let issue = apply(
            config,
            store,
            raw,
            title,
            body,
            status,
            priority,
            assignee,
            labels,
            files,
            estimate,
            cycles,
            milestones,
            reviewer,
            start_date,
            due_date,
            commit,
            parent,
            override_reason,
            related_issues,
            type_,
        )?;
        println!("Updated {}", issue.filename);
    } else {
        let selector = parse_selector(store, raw)?;
        let issue = store.get(selector)?;
        edit_with_editor(config, store, &issue)?;
    }
    Ok(())
}

/// The edit core (WP02), holding the **flag path**: apply `merge_fields`
/// to the issue selected by `raw`, rewriting the file and returning the
/// updated [`Issue`] (with `filename` set to the relative path it lives at
/// afterwards, including after a reassign move).
///
/// This is what MCP's `update_issue` tool (WP04) calls — never the
/// `$EDITOR` flow, which is CLI-only (C-005). Side-effect policy: this core
/// never reads `stdin`/writes `stdout` and never launches an editor.
/// Validation order is byte-identical to the pre-extraction flag path:
/// selector resolution first, then priority validation (Directive 024), then
/// the planning-reference and date-pair rules (WP03, all before any store
/// mutation), then the reassign rule and `commit` clearing interplay.
///
/// WP03 parity: `--cycles`/`--milestones` reject unknown names via the same
/// `create::validate_planning_inputs` path (and thus the identical message)
/// the CLI create and MCP tools share; an empty value clears the field
/// (`Some(None)`), mirroring create's blank-means-none rule. `updated` is
/// bumped only when the merge actually changed the issue.
#[allow(clippy::too_many_arguments)]
pub fn apply(
    config: &Config,
    store: &Store,
    raw: &str,
    title: Option<&str>,
    body: Option<&str>,
    status: Option<&str>,
    priority: Option<&str>,
    assignee: Option<&str>,
    labels: Option<&str>,
    files: Option<&str>,
    estimate: Option<&str>,
    cycles: Option<&str>,
    milestones: Option<&str>,
    reviewer: Option<&str>,
    start_date: Option<&str>,
    due_date: Option<&str>,
    commit: Option<&str>,
    parent: Option<&str>,
    override_reason: Option<&str>,
    related_issues: Option<&str>,
    type_: Option<&str>,
) -> Result<Issue> {
    let selector = parse_selector(store, raw)?;
    let issue = store.get(selector)?;

    // `--parent` is resolved and validated through the hierarchy service **before**
    // anything is built or written, so a refusal leaves the file byte-identical
    // (contract §4, SC-004). The `Option<Option<u32>>` follows the `--assignee ""`
    // convention: an absent flag is `None` (untouched), `--parent ""` is
    // `Some(None)` (clear), and anything else is a selector resolved to its number.
    let (parent_edit, parent_event) = match parent.map(str::trim) {
        None => (None, None),
        Some("") => {
            let (index, _) = store.hierarchy_index()?;
            let change = hierarchy::clear_parent(&index, issue.number)?;
            debug_assert_eq!(change.event.is_some(), issue.parent.is_some());
            (Some(None), change.event)
        }
        Some(raw) => {
            let target = store.resolve_issue(raw)?;
            let (index, _) = store.hierarchy_index()?;
            let change = hierarchy::set_parent(&index, issue.number, target.number)?;
            (Some(Some(target.number)), change.event)
        }
    };

    // `--commit auto` back-fills the closing commit from git history; a plain
    // value is recorded verbatim and `clear` removes the field.
    let resolved_commit: Option<String> = match commit {
        Some("auto") => Some(crate::commands::resolve_auto_commit(store, &issue)?),
        Some(other) => Some(other.to_string()),
        None => None,
    };

    if let Some(p) = priority {
        parse_priority(p)?;
    }
    let parsed_cycles = cycles.map(|raw| crate::commands::create::parse_planning(Some(raw)));
    let parsed_milestones =
        milestones.map(|raw| crate::commands::create::parse_planning(Some(raw)));
    let parsed_reviewer = reviewer.map(|r| {
        let r = r.trim();
        if r.is_empty() {
            None
        } else {
            Some(r.to_string())
        }
    });
    let parsed_start = start_date.map(|d| crate::commands::create::parse_date(Some(d)));
    let parsed_due = due_date.map(|d| crate::commands::create::parse_date(Some(d)));
    // Planning references are validated only when a non-empty list is
    // supplied (`Some(Some(_))`); a cleared list (`Some(None)`) or an absent
    // flag never consults the registry. Both checks run before any store
    // mutation so a rejected edit leaves every file untouched.
    if let Some(Some(items)) = &parsed_cycles {
        crate::commands::create::validate_planning_inputs(config, Some(items), None)?;
    }
    if let Some(Some(items)) = &parsed_milestones {
        crate::commands::create::validate_planning_inputs(config, None, Some(items))?;
    }
    // The date-pair rule compares the *effective* pair: a provided date wins,
    // otherwise the issue's stored value applies — so clearing one side still
    // re-validates the surviving member against the other.
    if parsed_start.is_some() || parsed_due.is_some() {
        let effective_start = parsed_start
            .clone()
            .flatten()
            .or_else(|| issue.start_date.clone());
        let effective_due = parsed_due
            .clone()
            .flatten()
            .or_else(|| issue.due_date.clone());
        crate::model::validate_issue_dates(effective_start.as_deref(), effective_due.as_deref())?;
    }
    // WP04 T017: `--status` is **withheld from the merge** and applied through the
    // storage seam instead.
    //
    // Leaving it in `FieldEdits` and also routing it through the seam would look
    // like belt-and-braces and is actually a double write: `merge_fields` would set
    // the column, the file would be rewritten with the new status, and the seam
    // would then see a same-column transition, treat it as an idempotent no-op, and
    // append **no** `TransitionDecision`. The status would change and the audit
    // trail would say nothing happened — which is the exact failure `tix edit
    // --status` already had before this WP, just with the event now half-attempted.
    //
    // `--status ""` clears, matching every other field (`--assignee ""`,
    // `--commit clear`). A cleared status is *not* a transition: there is no target
    // column to judge, so it goes through the merge like a plain field write. That
    // is the existing convention, confirmed rather than assumed (T017 step 3), and
    // it is why the `filter` below is on emptiness rather than on the flag's
    // presence.
    let status_target = status.map(str::trim).filter(|s| !s.is_empty());

    let edits = FieldEdits {
        title: title.map(String::from),
        // Three distinct cases, and collapsing two of them is a silent data-loss
        // bug — the first version of this line did exactly that:
        //
        // - flag **absent** (`None`) → `None`. "Leave it alone."
        // - flag present but **blank** (`Some("")`) → `Some("")`. The clearing
        //   convention every other field here follows.
        // - flag present with a real **column** → `None`, because the seam sets it
        //   below after judging the transition.
        //
        // `(!status_is_transition).then(|| status.unwrap_or_default())` got the
        // first case wrong: an absent flag produced `Some("")`, which cleared the
        // status of every issue edited for any other reason — a `--title` change
        // wiped the column. Caught by `t014_clearing_an_issue_with_no_parent_is_a_no_op`,
        // whose byte snapshot showed the rewritten file. The two cases differ only
        // in whether a flag was typed, which is why nothing else noticed.
        status: match status {
            Some(value) if !value.trim().is_empty() => None,
            other => other.map(String::from),
        },
        priority: priority.map(String::from),
        labels: labels.map(|l| {
            l.split(',')
                .map(|s| s.trim().to_string())
                .filter(|s| !s.is_empty())
                .collect()
        }),
        files: files.map(|f| {
            f.split(',')
                .map(|s| s.trim().to_string())
                .filter(|s| !s.is_empty())
                .collect()
        }),
        related_issues: related_issues.map(|r| {
            r.split(',')
                .map(|s| s.trim().to_string())
                .filter(|s| !s.is_empty())
                .collect()
        }),
        type_: type_.map(|t| {
            let t = t.trim();
            if t.is_empty() {
                None
            } else {
                Some(t.to_string())
            }
        }),
        assignee: assignee.map(parse_assignee),
        estimate: estimate.map(|e| Some(e.to_string())),
        commit: resolved_commit.map(|c| if c == "clear" { None } else { Some(c) }),
        cycles: parsed_cycles,
        milestones: parsed_milestones,
        reviewer: parsed_reviewer,
        start_date: parsed_start,
        due_date: parsed_due,
        parent: parent_edit,
    };
    let mut merged = merge_fields(&issue, &edits);
    // `--status` is a **move**, so it goes through the same blocked-move policy
    // as `tix move` (`surface-parity.md` §2: same accept/reject decision on every
    // surface). Without this, `tix move <n> done` refused with a warning while
    // `tix edit <n> --status done` moved the same issue into the same column
    // silently, recorded nothing, and exited 0 -- so the guard was a suggestion
    // with a second door next to it.
    //
    // Refused rather than acknowledged: `tix edit` has no confirmation flag, and
    // `tix move --yes` is the spelling that records a deliberate decision. The
    // message names it, so the user has a route rather than only a wall.
    if let Some(target) = status_target {
        // `--reason` (MCP's `reason`) rides the `TransitionDecision` detail, which
        // is one line of the same line-based `## History` grammar. A line break in
        // it would forge a second parsed event, so it is refused through the shared
        // predicate `tix move` uses rather than carrying a local copy (tix #1493).
        // Checked first, so the refusal is about the value and not about whatever
        // the blocked-move guard would have said.
        if let Some(reason) = override_reason {
            if reason.chars().any(crate::triage::decision::is_line_break) {
                return Err(TixError::InvalidInput(
                    "invalid_reason: the override reason has a line break; it is written \
                     to one History line, so a line break would look like a second event"
                        .to_string(),
                ));
            }
        }
        let guard = crate::commands::mv::blocked_move_guard(config, store, &issue, target)?;
        if guard.needs_acknowledgement() {
            return Err(TixError::InvalidInput(format!(
                "{} — setting --status '{target}' with open blockers requires an \
                 explicit confirmation; use `tix move <issue> {target} --yes` to \
                 record the decision",
                guard.warning().unwrap_or_else(|| "blocked".to_string())
            )));
        }
        // WP04 T017: the **same** service call `tix mv` makes, and it is judged
        // here — before the merge is written — so a refusal leaves the file
        // byte-identical rather than writing every other `--status`-alongside field
        // and then failing. The seam re-judges on write; this copy is for the
        // message, so the diagnostic `tix mv` prints is the diagnostic this prints.
        //
        // A refusal is a hard error, not a warning: `tix edit --status` has no
        // `--yes`, and the reasoned override is spelled `--reason` (T016). A user
        // who needs the escape hatch has a route; a user who wants to bypass policy
        // silently does not have one.
        let (_, resolution) = crate::workflow::active_resolution(config);
        match crate::workflow::validate_transition(
            &issue,
            target,
            &resolution,
            config,
            override_reason,
        ) {
            crate::workflow::TransitionOutcome::Accepted { .. } => {}
            crate::workflow::TransitionOutcome::Rejected(diagnostic) => {
                return Err(TixError::Workflow(diagnostic));
            }
        }
    }
    if let Some(b) = body {
        merged.body = b.to_string();
    }
    // Exactly one History entry per **accepted** parent change (C-004), appended
    // after the blocked-move guard above so a refused `--status` cannot leave an
    // event behind for a change that never happened.
    //
    // `None` for a no-op, so `tix edit 2 --parent 1` run twice appends once. The
    // `if merged != issue` check below then also stays false and `updated` does not
    // move, which is the whole of T012's "no-op" contract: no entry, no date
    // change, zero exit, and the file's **bytes** unchanged.
    //
    // The write itself is unconditional, as it is for every other edit field, so
    // the no-op rewrites identical content. That is a deliberate consistency call
    // rather than an oversight: special-casing one field to skip the write would
    // make `tix edit 2 --parent 1` the only edit that does not rewrite the file,
    // and it would still leave the mtime moved for every other no-op edit. T014
    // pins the byte-level property, which is the one a user can observe.
    if let Some(event) = parent_event {
        let detail = hierarchy::change_detail(issue.parent, merged.parent);
        merged.history.append_now(
            &crate::storage::actor_identity(),
            HistoryEventKind::from_parent(event.kind),
            &detail,
        );
    }
    // `updated` bumps only when the merge actually changed the issue (the
    // editor path bumps on every edit, per `contracts/cli.md` §updated).
    if merged != issue {
        merged.bump_updated();
    }
    // Reassign first when the assignee segment may have changed, then
    // write the merged content at the (possibly new) relative path.
    let mut filename: String = issue.filename.clone();
    if assignee.is_some() {
        let moved = store.reassign(&issue.filename, merged.assignee.as_deref())?;
        filename = moved.to_string_lossy().to_string();
    }
    filename = store.update(&filename, &merged)?;
    merged.filename = filename.clone();

    // WP04 T017: the transition itself, through the seam — after the field write,
    // because a `--status` move and a `--title` edit are two different records and
    // the seam re-reads the file it writes. Doing it in this order means the
    // decision is appended to an issue whose other fields are already saved, so a
    // failure here cannot leave a half-applied edit behind.
    //
    // The merge above deliberately left `status` alone, so the seam sees the real
    // starting column and judges a real transition rather than a no-op.
    if let Some(target) = status_target {
        let outcome = store.set_status(
            &filename,
            crate::storage::StatusChange::to(target)
                .with_commit(merged.commit.as_deref())
                .with_override(override_reason),
        )?;
        merged = outcome.issue;
        // A `--status` that lands in the inbox is an **entry**, exactly as
        // `tix move` into it is (tix #1489): the queue needs the stamp its age is
        // measured from. Stamped after the seam has written the move, and only
        // when the move wrote something — a same-column re-apply must not restart
        // the wait.
        if outcome.written && crate::triage::record_move_entry(config, &mut merged)? {
            merged.filename = store.update(&merged.filename, &merged)?;
        }
    }
    Ok(merged)
}

/// Open the configured editor on a temporary copy of the issue file, then
/// apply whatever the editor produced.
///
/// The copy is parsed here, not by `run_editor` (tix #1506): `run_editor` runs
/// the editor and nothing else, so the real parse diagnostic — a duplicate key,
/// a bad fence — is what the CLI reports. Nothing is written on a parse failure,
/// and the copy is **kept** and named, because deleting it would destroy the very
/// edit the user is being told about.
fn edit_with_editor(config: &Config, store: &Store, issue: &Issue) -> Result<()> {
    let nanos = SystemTime::now()
        .duration_since(UNIX_EPOCH)
        .map(|d| d.as_nanos())
        .unwrap_or(0);
    let temp = std::env::temp_dir().join(format!("tix-edit-{}-{nanos}.md", std::process::id()));
    std::fs::write(&temp, issue.to_file_contents()).map_err(TixError::Io)?;

    let result = (|| -> Result<()> {
        let editor = resolve_editor(config.editor.as_deref())?;
        run_editor(&editor, &temp, config)?;
        let edited = std::fs::read_to_string(&temp).map_err(TixError::Io)?;
        let mut parsed = match Issue::parse(&edited) {
            Ok(parsed) => parsed,
            Err(e) => {
                let message = match e {
                    TixError::Parse(message) => message,
                    other => other.to_string(),
                };
                return Err(TixError::Parse(format!(
                    "{message} — nothing was written; your edit is kept at {}",
                    temp.display()
                )));
            }
        };
        if parsed.title.trim().is_empty() {
            return Err(TixError::Editor(format!(
                "title is required — nothing was written; your edit is kept at {}",
                temp.display()
            )));
        }
        parsed.bump_updated();
        store.update(&issue.filename, &parsed)?;
        Ok(())
    })();

    // The copy is removed only once the edit has actually been applied; a
    // failure keeps it, and the message above says where.
    if result.is_ok() {
        let _ = std::fs::remove_file(&temp);
    }
    result
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::commands::create::create;
    use crate::model::Priority;
    use std::fs;
    use tempfile::TempDir;

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

    /// Create a real committed issue to edit (via the create core, WP02).
    fn seed(store: &Store, assignee: &str) -> Issue {
        create(
            &store.config,
            store,
            "Seed issue",
            Some("body"),
            None,
            None,
            Some(assignee),
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
        )
        .unwrap()
    }

    #[test]
    fn apply_updates_fields_and_returns_updated_issue() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let seeded = seed(&store, "@alice");
        let updated = apply(
            &store.config,
            &store,
            &seeded.number.to_string(),
            Some("Renamed"),
            Some("new body"),
            Some("in progress"),
            Some("high"),
            None,
            Some("bug,urgent"),
            None,
            Some("2"),
            None,
            None,
            None,
            None,
            None,
            Some("abc123"),
            None,
            /* override_reason */ None,
            None,
            None,
        )
        .unwrap();
        assert_eq!(updated.title, "Renamed");
        assert_eq!(updated.body, "new body");
        assert_eq!(updated.status.display(), "in progress");
        assert_eq!(updated.priority, Priority::High);
        assert_eq!(
            updated.labels,
            vec!["bug".to_string(), "urgent".to_string()]
        );
        assert_eq!(updated.estimate.as_deref(), Some("2"));
        assert_eq!(updated.commit.as_deref(), Some("abc123"));
        // Move-on-change (the `[layout]` default): a status transition relocates
        // the file under the templated directory.
        assert_ne!(updated.filename, seeded.filename);
        assert!(
            updated.filename.contains("/in-progress/"),
            "relocated under the new status: {}",
            updated.filename
        );
        let reloaded = store
            .get(crate::storage::IssueSelector::Filename(
                updated.filename.clone(),
            ))
            .unwrap();
        assert_eq!(reloaded.title, "Renamed");
        assert_eq!(reloaded.body, "new body");
    }

    #[test]
    fn apply_reassign_moves_file_between_segments() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let seeded = seed(&store, "alice");
        let updated = apply(
            &store.config,
            &store,
            &seeded.number.to_string(),
            None,
            None,
            None,
            None,
            Some("@bob"),
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            /* override_reason */ None,
            None,
            None,
        )
        .unwrap();
        assert!(
            updated.filename.starts_with("bob/"),
            "file must move into the new segment, got {}",
            updated.filename
        );
        assert_eq!(updated.number, seeded.number, "number is stable");
        assert!(!store.issues_dir.join(&seeded.filename).exists());
        assert!(store.issues_dir.join(&updated.filename).is_file());
    }

    #[test]
    fn apply_commit_clear_resets_commit_field() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let seeded = seed(&store, "alice");
        let with_commit = apply(
            &store.config,
            &store,
            &seeded.number.to_string(),
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            Some("abc123"),
            None,
            /* override_reason */ None,
            None,
            None,
        )
        .unwrap();
        assert_eq!(with_commit.commit.as_deref(), Some("abc123"));
        let cleared = apply(
            &store.config,
            &store,
            &seeded.number.to_string(),
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            Some("clear"),
            None,
            /* override_reason */ None,
            None,
            None,
        )
        .unwrap();
        assert_eq!(cleared.commit, None);
    }

    #[test]
    fn apply_rejects_invalid_priority_with_frozen_message() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let seeded = seed(&store, "alice");
        let err = apply(
            &store.config,
            &store,
            &seeded.number.to_string(),
            None,
            None,
            None,
            Some("bogus"),
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            /* override_reason */ None,
            None,
            None,
        )
        .unwrap_err();
        assert_eq!(
            err.to_string(),
            "invalid input: invalid priority 'bogus' — use '' (unset), low, medium or high"
        );
    }

    #[test]
    fn apply_sets_files_list() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let a = seed(&store, "alice");
        let updated = apply(
            &store.config,
            &store,
            &a.number.to_string(),
            None,
            None,
            None,
            None,
            None,
            None,
            Some("specs/a.md, specs/b.md"),
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            /* override_reason */ None,
            None,
            None,
        )
        .unwrap();
        assert_eq!(updated.files, ["specs/a.md", "specs/b.md"]);
        let cleared = apply(
            &store.config,
            &store,
            &a.number.to_string(),
            None,
            None,
            None,
            None,
            None,
            None,
            Some(""),
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            /* override_reason */ None,
            None,
            None,
        )
        .unwrap();
        assert!(cleared.files.is_empty());
    }

    // -----------------------------------------------------------------------
    // WP03 / T011 + T012 — `--parent` on the shared edit core
    // -----------------------------------------------------------------------

    /// The edit core with every optional argument absent except `parent`.
    ///
    /// Eighteen positional arguments is too many to read at a call site, and one
    /// silently transposed is a test that passes for the wrong reason, so the
    /// shape is written out once here and every test goes through it.
    #[allow(clippy::too_many_arguments)]
    fn edit_parent(store: &Store, raw: &str, parent: Option<&str>) -> crate::error::Result<Issue> {
        apply(
            &store.config,
            store,
            raw,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            parent,
            /* override_reason */ None,
            None,
            None,
        )
    }

    /// Three issues in a store, with their numbers.
    ///
    /// Built through the `create` core with every optional argument absent, so a
    /// test that cares only about the hierarchy does not have to spell sixteen
    /// `None`s at every call site.
    fn trio() -> (TempDir, Store, Vec<Issue>) {
        let (tmp, store) = make_store();
        let made: Vec<Issue> = ["One", "Two", "Three"]
            .iter()
            .map(|t| create_bare(&store, t))
            .collect::<Result<Vec<_>>>()
            .unwrap();
        (tmp, store, made)
    }

    /// The create core with every optional argument absent.
    fn create_bare(store: &Store, title: &str) -> crate::error::Result<Issue> {
        create(
            &store.config,
            store,
            title,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
        )
    }

    /// Every issue file's bytes: the only honest proof that nothing was written.
    fn snapshot(store: &Store) -> Vec<(String, String)> {
        let mut out: Vec<(String, String)> = fs::read_dir(&store.issues_dir)
            .unwrap()
            .filter_map(|e| e.ok())
            .map(|e| (e.file_name().to_string_lossy().into_owned(), e.path()))
            .filter(|(_, p)| p.extension().is_some_and(|x| x == "md"))
            .map(|(n, p)| (n, fs::read_to_string(p).unwrap()))
            .collect();
        out.sort();
        out
    }

    fn reread(store: &Store, issue: &Issue) -> Issue {
        let path = store.issues_dir.join(&issue.filename);
        Issue::parse(&fs::read_to_string(path).unwrap()).unwrap()
    }

    #[test]
    fn t011_a_valid_change_writes_the_field_and_exactly_one_history_entry() {
        let (_tmp, store, made) = trio();
        let (a, b, _) = (&made[0], &made[1], &made[2]);
        let updated =
            edit_parent(&store, &a.number.to_string(), Some(&b.number.to_string())).unwrap();
        assert_eq!(updated.parent, Some(b.number));
        assert_eq!(
            updated.history.len(),
            1,
            "C-004: one entry per accepted change"
        );
        let events = updated.history.events();
        // `ParentSet`, not `ParentChanged`: this is the child's *first* parent.
        // Asserting `ParentChanged` here — which the first version of this test did
        // — would also have passed against a service emitting one generic kind for
        // all three transitions, which is the property data-model.md §6.1 records a
        // decision about.
        assert_eq!(events[0].kind, HistoryEventKind::ParentSet);
        assert_eq!(
            events[0].detail,
            format!("parent none -> #{}", b.number),
            "the detail names both sides"
        );
        assert_eq!(reread(&store, a).parent, Some(b.number), "and it persisted");
    }

    #[test]
    fn t011_repointing_appends_a_second_entry_naming_the_old_parent() {
        let (_tmp, store, made) = trio();
        let (a, b, c) = (&made[0], &made[1], &made[2]);
        edit_parent(&store, &a.number.to_string(), Some(&b.number.to_string())).unwrap();
        let updated =
            edit_parent(&store, &a.number.to_string(), Some(&c.number.to_string())).unwrap();
        assert_eq!(updated.parent, Some(c.number));
        assert_eq!(
            updated.history.len(),
            2,
            "one entry per change, not one per run"
        );
        let events = updated.history.events();
        let last = events.last().unwrap();
        assert_eq!(last.kind, HistoryEventKind::ParentChanged);
        assert_eq!(
            last.detail,
            format!("parent #{} -> #{}", b.number, c.number),
            "the old parent is named, so the trail reads without replaying state"
        );
    }

    #[test]
    fn t011_a_self_parent_is_refused_and_writes_nothing() {
        let (_tmp, store, made) = trio();
        let a = &made[0];
        let before = snapshot(&store);
        let err = edit_parent(&store, &a.number.to_string(), Some(&a.number.to_string()))
            .expect_err("an issue cannot be its own parent");
        assert!(err.to_string().contains("self_parent"), "got: {err}");
        assert_eq!(
            snapshot(&store),
            before,
            "SC-004: every file byte-identical"
        );
    }

    #[test]
    fn t011_an_unknown_parent_is_refused_and_writes_nothing() {
        let (_tmp, store, made) = trio();
        let a = &made[0];
        let before = snapshot(&store);
        assert!(edit_parent(&store, &a.number.to_string(), Some("no such issue")).is_err());
        assert_eq!(snapshot(&store), before, "SC-004");
    }

    #[test]
    fn t011_a_cycle_closing_change_is_refused_and_writes_nothing() {
        // 1 -> 2 -> 3, then #1 under #3. The case T011 singles out as "the one that
        // must not half-apply", so it asserts bytes rather than an error.
        let (_tmp, store, made) = trio();
        let (a, b, c) = (&made[0], &made[1], &made[2]);
        edit_parent(&store, &b.number.to_string(), Some(&a.number.to_string())).unwrap();
        edit_parent(&store, &c.number.to_string(), Some(&b.number.to_string())).unwrap();
        let before = snapshot(&store);

        let err = edit_parent(&store, &a.number.to_string(), Some(&c.number.to_string()))
            .expect_err("closing the loop must be refused");
        assert!(err.to_string().contains("cycle"), "got: {err}");
        assert_eq!(
            snapshot(&store),
            before,
            "SC-004: not one of the three files may change"
        );
    }

    #[test]
    fn t012_clearing_removes_the_field_and_appends_one_entry() {
        let (_tmp, store, made) = trio();
        let (a, b, _) = (&made[0], &made[1], &made[2]);
        edit_parent(&store, &a.number.to_string(), Some(&b.number.to_string())).unwrap();
        let updated = edit_parent(&store, &a.number.to_string(), Some("")).unwrap();
        assert_eq!(updated.parent, None, "`--parent \"\"` clears");
        assert_eq!(updated.history.len(), 2);
        let events = updated.history.events();
        let last = events.last().unwrap();
        assert_eq!(last.kind, HistoryEventKind::ParentCleared);
        assert_eq!(last.detail, format!("parent #{} -> none", b.number));
        assert_eq!(reread(&store, a).parent, None, "and it persisted");
    }

    #[test]
    fn t012_clearing_an_issue_with_no_parent_is_a_no_op() {
        let (_tmp, store, made) = trio();
        let a = &made[0];
        let before = snapshot(&store);
        let updated = edit_parent(&store, &a.number.to_string(), Some("")).unwrap();
        assert_eq!(updated.parent, None);
        assert_eq!(updated.history.len(), 0, "no entry for nothing");
        assert_eq!(
            updated.updated, a.updated,
            "`updated` must not move for a no-op"
        );
        assert_eq!(snapshot(&store), before, "and the bytes must not change");
    }

    #[test]
    fn t012_an_absent_flag_leaves_the_field_untouched() {
        let (_tmp, store, made) = trio();
        let (a, b, _) = (&made[0], &made[1], &made[2]);
        edit_parent(&store, &a.number.to_string(), Some(&b.number.to_string())).unwrap();
        let updated = edit_parent(&store, &a.number.to_string(), None).unwrap();
        assert_eq!(
            updated.parent,
            Some(b.number),
            "an absent flag is not a clear"
        );
        assert_eq!(updated.history.len(), 1, "and appends nothing");
    }

    #[test]
    fn t012_a_repeated_clear_is_a_no_op() {
        let (_tmp, store, made) = trio();
        let (a, b, _) = (&made[0], &made[1], &made[2]);
        edit_parent(&store, &a.number.to_string(), Some(&b.number.to_string())).unwrap();
        edit_parent(&store, &a.number.to_string(), Some("")).unwrap();
        let before = snapshot(&store);
        let again = edit_parent(&store, &a.number.to_string(), Some("")).unwrap();
        assert_eq!(
            again.history.len(),
            2,
            "a retry must not pad the audit trail"
        );
        assert_eq!(snapshot(&store), before, "SC-004 applies to a no-op too");
    }

    #[test]
    fn t012_the_updated_bump_is_not_observable_within_a_day_and_is_not_asserted() {
        // Recorded as a test that deliberately does NOT assert what its name
        // suggests, because the obvious assertion cannot fail.
        //
        // `bump_updated()` stamps `today()`. An issue created and edited on the same
        // day already has `updated == today()`, so `assert_ne!(updated.updated,
        // before.updated)` cannot distinguish a bump from no bump — it passes or
        // fails depending on nothing at all. The first version of this test asserted
        // exactly that, which is a test that measures nothing.
        //
        // Observing the bump needs a day boundary or an injected clock, and WP03
        // has neither. What *is* observable, and is pinned above, is the property
        // the bump hangs off: an accepted change rewrites the file, and a no-op
        // leaves every byte alone.
        let (_tmp, store, made) = trio();
        let (a, b, _) = (&made[0], &made[1], &made[2]);
        let updated =
            edit_parent(&store, &a.number.to_string(), Some(&b.number.to_string())).unwrap();
        assert_eq!(
            updated.updated, a.updated,
            "same-day, so this cannot differ — that is the point of this test"
        );
        // The part that is real: the change happened and persisted.
        let after = reread(&store, a);
        assert_eq!(after.parent, Some(b.number));
        assert_eq!(after.history.len(), 1);
    }

    #[test]
    fn t011_other_flags_still_apply_alongside_a_refused_parent() {
        // The parent is validated *before* anything is built, so a refusal cannot
        // have half-applied the other flags either.
        let (_tmp, store, made) = trio();
        let a = &made[0];
        let before = snapshot(&store);
        let err = apply(
            &store.config,
            &store,
            &a.number.to_string(),
            Some("A brand new title"),
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            Some(&a.number.to_string()),
            /* override_reason */ None,
            None,
            None,
        );
        assert!(err.is_err(), "the self-parent is refused");
        assert_eq!(snapshot(&store), before, "and the title was not written");
    }

    /// `tix edit` (the `$EDITOR` path) reports the **real** parse error, writes
    /// nothing, and keeps the copy it names (tix #1506).
    #[cfg(unix)]
    #[test]
    fn editor_path_surfaces_the_real_parse_error_and_keeps_the_copy() {
        use std::os::unix::fs::PermissionsExt;

        let (tmp, store) = make_store();
        let seeded = seed(&store, "@alice");
        let script = tmp.path().join("break.sh");
        fs::write(
            &script,
            "#!/bin/sh\nsed -i 's/priority: medium/priority: [broken/' \"$1\"\n",
        )
        .unwrap();
        fs::set_permissions(&script, fs::Permissions::from_mode(0o755)).unwrap();
        let mut config = store.config.clone();
        config.editor = Some(script.to_string_lossy().to_string());
        let disk = store.issues_dir.join(&seeded.filename);
        let before = fs::read_to_string(&disk).unwrap();

        let err = edit_with_editor(&config, &store, &seeded).unwrap_err();

        let message = err.to_string();
        assert!(matches!(err, TixError::Parse(_)), "{message}");
        assert!(
            !message.contains("file not valid after edit"),
            "the generic message is the bug: {message}"
        );
        assert!(
            message.contains("priority") && message.contains("invalid type"),
            "the real parse diagnostic must reach the user: {message}"
        );
        assert!(
            message.contains("kept at"),
            "the kept copy's path is named: {message}"
        );
        assert_eq!(
            fs::read_to_string(&disk).unwrap(),
            before,
            "a parse failure must write nothing"
        );
    }
}