tix-tracker 0.12.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>,
    add_project: &[String],
    remove_project: &[String],
    projects: Option<&str>,
    add_epic: &[String],
    remove_epic: &[String],
    epics: 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<()> {
    // `--projects` on `tix edit` exists to **clear**: a blank value drops every
    // association, mirroring the empty-string-clears convention the other list
    // flags follow. A non-blank list is refused rather than guessed at, so setting
    // a list has exactly one spelling (`--add-project`) and a typo cannot silently
    // replace an association set.
    let clear_projects = match projects {
        None => false,
        Some(value) if crate::commands::edit_projects_clears(value) => true,
        Some(value) => return Err(crate::commands::edit_projects_must_be_clear(value)),
    };
    let has_projects = clear_projects || !add_project.is_empty() || !remove_project.is_empty();
    // `--epics` on `tix edit` mirrors `--projects`: it exists to **clear**.
    let clear_epics = match epics {
        None => false,
        Some(value) if crate::commands::edit_epics_clears(value) => true,
        Some(value) => return Err(crate::commands::edit_epics_must_be_clear(value)),
    };
    let has_epics = clear_epics || !add_epic.is_empty() || !remove_epic.is_empty();
    let has_fields = 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();
    if has_fields || has_projects || has_epics {
        // The association inputs are resolved and validated **before** the field
        // write, so `--add-project bogus` refuses the whole edit and leaves the file
        // byte-identical rather than applying the other flags and then failing
        // (T023 step 5). Addressing the issue by number keeps the mutations working
        // after a `--assignee` reassign moves the file.
        let number = store.resolve_issue(raw)?.number;
        let selector = number.to_string();
        let plan = if has_projects {
            Some(crate::commands::project::plan_edit_associations(
                config,
                store,
                &selector,
                add_project,
                remove_project,
                clear_projects,
            )?)
        } else {
            None
        };
        // The epic inputs are planned **before** any write too, so `--add-epic bogus`
        // refuses the edit with every file byte-identical (T025 step 5).
        let epic_plan = if has_epics {
            Some(crate::commands::epic::plan_edit_epics(
                config,
                store,
                &selector,
                add_epic,
                remove_epic,
                clear_epics,
            )?)
        } else {
            None
        };
        let issue = if has_fields {
            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_,
            )?
        } else {
            store.get(parse_selector(store, raw)?)?
        };
        let associations_changed = match &plan {
            Some(plan) => {
                crate::commands::project::apply_edit_associations(store, &selector, plan)?
            }
            None => false,
        };
        let epics_changed = match &epic_plan {
            Some(plan) => crate::commands::epic::apply_edit_epics(store, &selector, plan)?,
            None => false,
        };
        let associations_changed = associations_changed || epics_changed;
        // The success line must not claim a write that never happened. A field edit
        // rewrites the file unconditionally (the existing convention), so `Updated`
        // is right there; an association-only edit that changed nothing —
        // `--add-project` of an already-present association, `--projects ""` on an
        // issue with no associations — writes nothing and bumps no date, and says so.
        if has_fields || associations_changed {
            println!("Updated {}", issue.filename);
        } else {
            println!("Updated {} (no association change)", 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)]
#[allow(clippy::too_many_lines)]
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, parent_history) = match parent.map(str::trim) {
        None => (None, None, None),
        Some("") => {
            let (index, _) = store.hierarchy_index()?;
            let change = hierarchy::clear_parent_ref(&index, crate::id::canonical_ref(&issue))?;
            debug_assert_eq!(change.event.is_some(), issue.parent.is_some());
            let detail = change
                .event
                .as_ref()
                .map(|_| hierarchy::change_detail(change.previous, change.parent));
            (Some(None), change.event, detail)
        }
        Some(raw) => {
            let target = store.resolve_issue(raw)?;
            let (index, _) = store.hierarchy_index()?;
            let change = hierarchy::set_parent_ref(
                &index,
                crate::id::canonical_ref(&issue),
                crate::id::canonical_ref(&target),
            )?;
            let detail = change
                .event
                .as_ref()
                .map(|_| hierarchy::change_detail(change.previous, change.parent));
            (
                Some(Some(crate::id::canonical_ref(&target))),
                change.event,
                detail,
            )
        }
    };

    // `--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 {
        merged.history.append_now(
            &crate::storage::actor_identity(),
            HistoryEventKind::from_parent(event.kind),
            parent_history.as_deref().unwrap_or("parent changed"),
        );
    }
    // T047: the two producers FR-002 needs, at the same seam and under the same
    // rules as the parent event above — after the blocked-move guard, so a refused
    // `--status` cannot leave an event behind for a change that never happened, and
    // before the `updated` bump, so the record and the date agree.
    //
    // Both compare the **merged value**, never the flag: `merge_fields` leaves a
    // field alone when its flag was absent, so an unrelated edit differs in neither
    // and appends neither. An absent value and a blank one are the same "no estimate"
    // (the clearing convention `--estimate ""` follows), so re-clearing an estimate
    // that was never set is a no-op rather than an entry saying nothing changed —
    // and `parse_planning` already folds a blank `--cycles` to "no cycles".
    //
    // The detail carries the before/after values, so the prior value stays readable
    // in History after the change (C-004) and a reader never has to replay state to
    // learn what moved. There is no second log and no reconstruction path.
    //
    // This seam is what **records**: the analytics classes are only as wide as the
    // record, so a path that writes a field without appending anything (the bulk patch
    // service reaches `Store::update` directly) leaves no event and its change cannot
    // appear in the estimate-change or scope-addition class. That is a documented
    // coverage limit a surface reports as a marker — never a reason to read the
    // current frontmatter back into history.
    if estimate_value(merged.estimate.as_deref()) != estimate_value(issue.estimate.as_deref()) {
        let detail = estimate_change_detail(issue.estimate.as_deref(), merged.estimate.as_deref());
        merged.history.append_now(
            &crate::storage::actor_identity(),
            HistoryEventKind::EstimateChanged,
            &detail,
        );
    }
    if merged.cycles != issue.cycles {
        let detail = cycle_membership_detail(issue.cycles.as_deref(), merged.cycles.as_deref());
        merged.history.append_now(
            &crate::storage::actor_identity(),
            HistoryEventKind::CycleMembershipChanged,
            &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();
    }
    // The writes below are a **sequence** — reassign, the merged frontmatter, a
    // `--status` transition, a triage stamp — and each may touch a different path
    // (the reassign moves the file first). Wrapping the whole sequence in one
    // transaction makes it all-or-nothing: an I/O failure after the move restores
    // the file to its old path *and* bytes, instead of leaving the store half
    // mutated (tix #1302, decision #1541 option 2). The journal is recorded by the
    // store's write primitives, so no step here has to know it is inside one.
    store.transaction(|| {
        // 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(())
    })?;
    Ok(merged)
}

/// An estimate rendered for the record: an absent and a blank value are the same
/// "no estimate", which is the clearing convention `--estimate ""` follows.
fn estimate_value(raw: Option<&str>) -> Option<&str> {
    raw.map(str::trim).filter(|value| !value.is_empty())
}

/// The `detail` an `estimate_changed` entry carries: both sides, never just the new
/// value, so History says what moved without a reader reconstructing the old state.
fn estimate_change_detail(before: Option<&str>, after: Option<&str>) -> String {
    format!(
        "estimate: {} -> {}",
        estimate_value(before).unwrap_or("none"),
        estimate_value(after).unwrap_or("none")
    )
}

/// The `detail` a `cycle_membership_changed` entry carries: the membership lists in
/// stored order, `none` for an empty one.
///
/// The direction lives here rather than in a pair of kinds because one `--cycles`
/// edit routinely adds one cycle and removes another (see the kind's own note).
fn cycle_membership_detail(before: Option<&[String]>, after: Option<&[String]>) -> String {
    let render = |names: Option<&[String]>| match names {
        Some(names) if !names.is_empty() => names.join(", "),
        _ => "none".to_string(),
    };
    format!("cycles: {} -> {}", render(before), render(after))
}

/// 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 {
    #[allow(clippy::wildcard_imports)]
    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_eq!(cleared.files, [] as [std::string::String; 0]);
    }

    // -----------------------------------------------------------------------
    // 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,
        )
    }

    /// `tix edit <raw> --estimate <value>` (an empty value clears it).
    fn edit_estimate(store: &Store, raw: &str, estimate: &str) -> crate::error::Result<Issue> {
        apply(
            &store.config,
            store,
            raw,
            // title, body, status, priority, assignee, labels, files
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            Some(estimate),
            // cycles, milestones, reviewer, start_date, due_date, commit, parent,
            // override_reason, related_issues, type_
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
        )
    }

    /// `tix edit <raw> --cycles <value>` (an empty value clears the list).
    fn edit_cycles(store: &Store, raw: &str, cycles: &str) -> crate::error::Result<Issue> {
        apply(
            &store.config,
            store,
            raw,
            // title, body, status, priority, assignee, labels, files
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            // estimate
            None,
            Some(cycles),
            // milestones, reviewer, start_date, due_date, commit, parent,
            // override_reason, related_issues, type_
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
        )
    }

    /// An issue's file bytes, for byte-identical assertions on a created issue.
    fn bytes_of(store: &Store, issue: &Issue) -> String {
        fs::read_to_string(store.issues_dir.join(&issue.filename)).unwrap()
    }

    /// 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(crate::id::canonical_ref(b)));
        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(crate::id::canonical_ref(b)),
            "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(crate::id::canonical_ref(c)));
        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(crate::id::canonical_ref(b)),
            "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(crate::id::canonical_ref(b)));
        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"
        );
    }

    // -----------------------------------------------------------------------
    // T047 — the estimate-change and cycle-membership producers
    // -----------------------------------------------------------------------

    #[test]
    fn t047_an_applied_estimate_edit_appends_exactly_one_entry_naming_both_sides() {
        let (_tmp, store, made) = trio();
        let a = &made[0];
        let updated = edit_estimate(&store, &a.number.to_string(), "3").unwrap();
        assert_eq!(updated.estimate.as_deref(), Some("3"));
        assert_eq!(updated.history.len(), 1, "one entry per applied change");
        let events = updated.history.events();
        assert_eq!(events[0].kind, HistoryEventKind::EstimateChanged);
        assert_eq!(
            events[0].detail, "estimate: none -> 3",
            "the detail names both sides, so the prior value stays readable"
        );
        assert_eq!(
            reread(&store, a).history.events()[0].kind,
            HistoryEventKind::EstimateChanged,
            "and it persisted"
        );
    }

    #[test]
    fn t047_repeating_an_unchanged_estimate_appends_nothing() {
        let (_tmp, store, made) = trio();
        let a = &made[0];
        edit_estimate(&store, &a.number.to_string(), "3").unwrap();
        let before = bytes_of(&store, a);
        let updated = edit_estimate(&store, &a.number.to_string(), "3").unwrap();
        assert_eq!(updated.history.len(), 1, "no second entry for a no-op");
        assert_eq!(bytes_of(&store, a), before, "zero bytes changed");
    }

    #[test]
    fn t047_clearing_an_estimate_names_the_value_it_removed() {
        let (_tmp, store, made) = trio();
        let a = &made[0];
        edit_estimate(&store, &a.number.to_string(), "3").unwrap();
        let updated = edit_estimate(&store, &a.number.to_string(), "").unwrap();
        assert_eq!(updated.estimate.as_deref(), Some(""));
        assert_eq!(updated.history.len(), 2);
        assert_eq!(
            updated.history.events()[1].detail,
            "estimate: 3 -> none",
            "a clear is a change and says what it cleared"
        );
    }

    /// A store whose issues can be moved into a second, configured cycle.
    ///
    /// `create` puts a new issue in the **default** cycle, so an "add the cycle it is
    /// already in" edit is a no-op by construction — the add case has to move the
    /// issue somewhere else to be a change at all. The cycle is registered in the
    /// in-memory config, which is what `validate_planning_inputs` consults, so no
    /// `tix.toml` round trip is needed.
    fn trio_with_sprint() -> (TempDir, Store, Vec<Issue>) {
        let (tmp, mut store, made) = trio();
        store.config.cycles.insert(
            "Sprint 7".to_string(),
            crate::config::PlanningCycle {
                description: None,
                start_date: None,
                due_date: None,
            },
        );
        (tmp, store, made)
    }

    #[test]
    fn t047_an_applied_cycle_membership_edit_appends_one_entry_naming_both_sides() {
        let (_tmp, store, made) = trio_with_sprint();
        let a = &made[0];
        let updated = edit_cycles(&store, &a.number.to_string(), "Sprint 7").unwrap();
        assert_eq!(
            updated.cycles.as_deref(),
            Some(&["Sprint 7".to_string()][..])
        );
        assert_eq!(updated.history.len(), 1, "one entry per applied change");
        let events = updated.history.events();
        assert_eq!(events[0].kind, HistoryEventKind::CycleMembershipChanged);
        assert_eq!(
            events[0].detail, "cycles: Default -> Sprint 7",
            "the detail names both sides, so the cycle it left stays readable"
        );
        assert_eq!(
            reread(&store, a).history.events()[0].kind,
            HistoryEventKind::CycleMembershipChanged,
            "and it persisted"
        );
    }

    #[test]
    fn t047_a_cleared_membership_names_the_list_it_removed() {
        let (_tmp, store, made) = trio();
        let a = &made[0];
        let updated = edit_cycles(&store, &a.number.to_string(), "").unwrap();
        assert_eq!(updated.cycles, None);
        assert_eq!(updated.history.len(), 1);
        assert_eq!(
            updated.history.events()[0].detail,
            "cycles: Default -> none"
        );
    }

    #[test]
    fn t047_repeating_an_unchanged_membership_appends_nothing() {
        let (_tmp, store, made) = trio_with_sprint();
        let a = &made[0];
        edit_cycles(&store, &a.number.to_string(), "Sprint 7").unwrap();
        let before = bytes_of(&store, a);
        let updated = edit_cycles(&store, &a.number.to_string(), "Sprint 7").unwrap();
        assert_eq!(updated.history.len(), 1, "no second entry for a no-op");
        assert_eq!(bytes_of(&store, a), before, "zero bytes changed");
    }

    #[test]
    fn t047_a_refused_cycle_name_appends_no_entry_and_writes_nothing() {
        let (_tmp, store, made) = trio();
        let a = &made[0];
        let before = bytes_of(&store, a);
        let error = edit_cycles(&store, &a.number.to_string(), "No Such Cycle").unwrap_err();
        assert!(error.to_string().contains("unknown cycle"), "{error}");
        assert_eq!(bytes_of(&store, a), before, "the refusal wrote nothing");
        assert_eq!(reread(&store, a).history.len(), 0, "and appended nothing");
    }

    #[test]
    fn t047_one_command_changing_both_fields_appends_one_entry_each() {
        let (_tmp, store, made) = trio_with_sprint();
        let a = &made[0];
        let updated = apply(
            &store.config,
            &store,
            &a.number.to_string(),
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            Some("5"),
            Some("Sprint 7"),
            // milestones, reviewer, start_date, due_date, commit, parent,
            // override_reason, related_issues, type_
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
        )
        .unwrap();
        let kinds: Vec<HistoryEventKind> =
            updated.history.events().iter().map(|e| e.kind).collect();
        assert_eq!(
            kinds,
            vec![
                HistoryEventKind::EstimateChanged,
                HistoryEventKind::CycleMembershipChanged
            ],
            "one entry per changed field, in the order the seam applies them"
        );
    }
}

#[cfg(test)]
mod project_association_tests {
    #[allow(clippy::wildcard_imports)]
    use super::*;
    use crate::config::ProjectConfiguration;
    use std::fs;
    use tempfile::TempDir;

    /// A store whose registry declares `p-1`/`Platform` and `p-2`/`Infra`, holding
    /// one issue (#1) whose associations are `initial`.
    fn store_with(initial: &[&str]) -> (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 mut config = crate::config::Config {
            issues_dir: dir.clone(),
            milestones_dir: Some(tmp.path().join("docs/milestones")),
            ..crate::config::Config::default()
        };
        config.layout.directory = "{assignee}".to_string();
        config.layout.move_on_change = false;
        for (id, name) in [("p-1", "Platform"), ("p-2", "Infra")] {
            config.projects.insert(
                id.to_string(),
                ProjectConfiguration {
                    name: name.to_string(),
                    ..ProjectConfiguration::default()
                },
            );
        }
        config.save(&config_path).unwrap();
        let config = crate::config::Config::load_with_warning(&config_path, |_| {}).unwrap();
        let store = Store {
            issues_dir: dir.clone(),
            config,
        };
        let mut frontmatter =
            String::from("---\nnumber: 1\ntitle: \"Issue 1\"\nstatus: \"todo\"\nlabels: []\n");
        if !initial.is_empty() {
            frontmatter.push_str("projects:\n");
            for id in initial {
                frontmatter.push_str(&format!("- {id}\n"));
            }
        }
        frontmatter.push_str("---\n");
        fs::write(dir.join("2026-01-01-issue-1.md"), frontmatter).unwrap();
        (tmp, store)
    }

    /// The issue file's bytes, for byte-identical assertions.
    fn issue_bytes(store: &Store) -> String {
        fs::read_to_string(store.issues_dir.join("2026-01-01-issue-1.md")).unwrap()
    }

    /// `tix edit 1 [--add-project …] [--remove-project …] [--projects …]`.
    fn edit_1(
        store: &Store,
        add: &[String],
        remove: &[String],
        projects: Option<&str>,
    ) -> Result<()> {
        run(
            &store.config,
            store,
            "1",
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            add,
            remove,
            projects,
            &[],
            &[],
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
        )
    }

    #[test]
    fn add_project_associates_and_records_one_entry() {
        let (_tmp, store) = store_with(&[]);
        edit_1(&store, &["Infra".to_string()], &[], None).unwrap();
        let issue = store.resolve_issue("1").unwrap();
        assert_eq!(issue.projects, Some(vec!["p-2".to_string()]));
        assert_eq!(issue.history.events().len(), 1);
    }

    #[test]
    fn adding_an_existing_association_is_a_no_op_with_no_write() {
        let (_tmp, store) = store_with(&["p-1"]);
        let before = issue_bytes(&store);
        edit_1(&store, &["p-1".to_string()], &[], None).unwrap();
        let issue = store.resolve_issue("1").unwrap();
        assert_eq!(issue.projects, Some(vec!["p-1".to_string()]));
        assert_eq!(issue.history.events().len(), 0, "no event for a no-op");
        assert_eq!(issue_bytes(&store), before, "a no-op writes nothing");
    }

    #[test]
    fn remove_project_drops_only_the_association() {
        let (_tmp, store) = store_with(&["p-1", "p-2"]);
        edit_1(&store, &[], &["Platform".to_string()], None).unwrap();
        let issue = store.resolve_issue("1").unwrap();
        assert_eq!(issue.projects, Some(vec!["p-2".to_string()]));
        assert_eq!(issue.history.events().len(), 1);
    }

    #[test]
    fn removing_an_absent_association_is_a_no_op_with_no_write() {
        let (_tmp, store) = store_with(&["p-2"]);
        let before = issue_bytes(&store);
        edit_1(&store, &[], &["p-1".to_string()], None).unwrap();
        let issue = store.resolve_issue("1").unwrap();
        assert_eq!(issue.projects, Some(vec!["p-2".to_string()]));
        assert_eq!(issue.history.events().len(), 0);
        assert_eq!(issue_bytes(&store), before);
    }

    #[test]
    fn a_blank_projects_flag_clears_every_association() {
        let (_tmp, store) = store_with(&["p-1", "p-2"]);
        edit_1(&store, &[], &[], Some("")).unwrap();
        let issue = store.resolve_issue("1").unwrap();
        assert_eq!(issue.projects, None);
        assert_eq!(issue.history.events().len(), 2, "one entry per removal");
    }

    #[test]
    fn an_absent_projects_flag_leaves_the_associations_untouched() {
        let (_tmp, store) = store_with(&["p-1"]);
        let before = issue_bytes(&store);
        // A field edit with no project flag: the associations must be untouched.
        run(
            &store.config,
            &store,
            "1",
            Some("Renamed"),
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            &[],
            &[],
            None,
            &[],
            &[],
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
        )
        .unwrap();
        let issue = store.resolve_issue("1").unwrap();
        assert_eq!(issue.title, "Renamed");
        assert_eq!(issue.projects, Some(vec!["p-1".to_string()]));
        assert_ne!(issue_bytes(&store), before, "the rename did write");
    }

    #[test]
    fn an_unknown_project_refuses_the_edit_and_leaves_the_file_byte_identical() {
        let (_tmp, store) = store_with(&["p-1"]);
        let before = issue_bytes(&store);
        let error = edit_1(&store, &["bogus".to_string()], &[], None).unwrap_err();
        assert!(error.to_string().contains("unknown_project"), "{error}");
        assert_eq!(issue_bytes(&store), before, "nothing written");
    }

    #[test]
    fn an_unknown_remove_reference_refuses_the_edit_and_writes_nothing() {
        let (_tmp, store) = store_with(&["p-1"]);
        let before = issue_bytes(&store);
        let error = edit_1(&store, &[], &["bogus".to_string()], None).unwrap_err();
        assert!(error.to_string().contains("unknown_project"), "{error}");
        assert_eq!(issue_bytes(&store), before);
    }

    #[test]
    fn a_non_blank_projects_flag_is_refused() {
        let (_tmp, store) = store_with(&[]);
        let before = issue_bytes(&store);
        let error = edit_1(&store, &[], &[], Some("p-1")).unwrap_err();
        assert!(error.to_string().contains("--projects"), "{error}");
        assert_eq!(issue_bytes(&store), before);
    }
}