tix/commands/

mv.rs

use std::io::IsTerminal;

use crate::commands::parse_selector;
use crate::config::Config;
use crate::editor::read_tty_line;
use crate::error::{Result, TixError};
use crate::model::{HistoryEventKind, Issue};
use crate::relationships::BlockedState;
use crate::storage::Store;

/// The board's configured completion column, never the literal `"done"`.
///
/// Research R7, and the reason this is a function rather than an inline
/// `== "done"`: a board may name its completion column anything, and a move guard
/// that hard-codes the string would silently stop guarding those boards — or, far
/// worse, would guard the wrong column on a board that has no column named `done`
/// at all.
///
/// **Workflow-*category* semantics are deliberately not handled here.** Category
/// mapping and transition policy belong to source issue #183
/// (`workflow-status-categories`). Adding a category concept to this guard would
/// make completion mean two different things on two boards, which is the failure
/// this function exists to prevent.
pub fn completion_column(config: &Config) -> Option<String> {
    // WP09: the **service's** completion column, not `done_column`. The two differ on
    // a board whose `category_map` places `completed` somewhere other than
    // `done_column`, and the difference disabled the blocked-move guard on exactly
    // those boards: the guard early-returns for any column that is not `done_column`,
    // so a blocked issue moved into the *real* completion column got no warning, no
    // `--yes` requirement and no History entry, while a move into `done_column` —
    // not complete at all — demanded confirmation. See
    // `workflow::completion_column`, which records the reproduction.
    crate::workflow::completion_column(config.default_config(), config)
}

/// What the shared move policy decided about one issue (FR-004).
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum MoveGuard {
    /// Apply the move as-is. Either the move does not target the completion
    /// column, or the issue has no open blockers, or the issue is **already** in
    /// the target column.
    Clear,
    /// The issue has open blockers and the move targets the completion column.
    /// The move needs an explicit acknowledgement before it may be applied.
    Blocked(BlockedState),
}

impl MoveGuard {
    /// Whether this move must be acknowledged before it can be applied.
    pub fn needs_acknowledgement(&self) -> bool {
        matches!(self, MoveGuard::Blocked(_))
    }

    /// The blocked reason to show, or `None` when the move is clear.
    pub fn warning(&self) -> Option<String> {
        match self {
            MoveGuard::Clear => None,
            MoveGuard::Blocked(state) => crate::commands::blocked_note(state),
        }
    }
}

/// The one blocked-move policy, shared by the CLI and the TUI (FR-004, C-003).
///
/// This is the single place the "moving blocked work toward completion warns,
/// confirms, and records the decision" rule exists. The CLI and the TUI both call
/// it, which is what makes their outcomes identical rather than merely similar —
/// a TUI path with its own copy of this rule is the highest-risk defect in the
/// mission, and factoring it here is the only thing that prevents one.
///
/// Three cases return [`MoveGuard::Clear`], and the third is the subtle one:
///
/// 1. the target column is not the board's completion column — blockers do not
///    block ordinary progress;
/// 2. the issue has no open blockers;
/// 3. **the issue is already in the target column.** Re-running an applied move
///    is a no-op, so it must not warn a second time and must not be the thing
///    that appends a second acknowledgement. Making idempotency a property of the
///    *guard* rather than of the caller's bookkeeping means a caller that forgets
///    to check still cannot produce a duplicate event.
///
/// Blocked state is read from the service, never computed here (C-003): a local
/// re-derivation is exactly the drift the parity contract's bypass rule exists to
/// catch.
/// The WIP decision for a move, from the shared policy (WP04 T018).
///
/// Returns the one History intent an `override` carries (`None` for `allow` and
/// `warn`) and refuses with the policy's own rendered decision for a `block` or a
/// `rejected` one — having written nothing, because nothing here can write. A `warn`
/// is reported on **stderr** so stdout stays parseable (T021), and the move
/// proceeds: a warning is not a refusal.
fn wip_gate(
    config: &Config,
    store: &Store,
    issue: &Issue,
    column: &str,
    override_reason: Option<&str>,
    override_wip: bool,
    confirmed: bool,
) -> Result<Option<crate::wip::HistoryIntent>> {
    let issues = store.list()?;
    let context = crate::wip::MoveContext::new(config, &issues);
    let mut request = crate::wip::MoveRequest::new(issue, column);
    if let Some(reason) = override_reason {
        request = request.because(reason);
    }
    // The bound limit is the one in force **now**: that is the limit the caller is
    // being asked to authorize. A limit that changes between the question and the
    // write makes the override stale rather than silently authoritative, which is
    // what `--override` being a request rather than an authorization means (T020).
    let decision = match config
        .default_config()
        .wip
        .effective_limit(column)
        .cloned()
        .filter(|_| override_wip)
    {
        Some(limit) => {
            // `confirmed` is the CLI's `--yes`, and it is **not** implied by
            // `--override`: a flag that authorizes is not a flag that confirms the
            // authorization, and the policy refuses the difference by name
            // (`unconfirmed_override`) rather than assuming a caller meant it (T020).
            let over = crate::wip::OverrideRequest {
                confirmed,
                ..crate::wip::OverrideRequest::new(
                    crate::storage::actor_identity(),
                    crate::model::utc_now_rfc3339(),
                    override_reason.unwrap_or_default(),
                    crate::wip::BoundLimit::of(column, &limit),
                )
            };
            crate::wip::evaluate(&context, &request.overriding(&over))
        }
        None => crate::wip::evaluate(&context, &request),
    };
    match decision.outcome {
        crate::wip::WipOutcome::Allow => Ok(None),
        crate::wip::WipOutcome::Warn => {
            eprintln!("{}", crate::wip::render_decision(&decision));
            Ok(None)
        }
        crate::wip::WipOutcome::Override => Ok(decision.history.clone()),
        // The policy asks #183 first (it must, to decide), so a refused *transition*
        // arrives here too. It is deliberately **not** re-reported: the seam is about
        // to make the same call, and its `TixError::Workflow` names the condition the
        // way this command always has (`unknown_column`, `invalid_transition`), with
        // the exit code that already had a contract. One refusal, one wording, one
        // code — the existing conventions survive (T018).
        crate::wip::WipOutcome::Rejected
            if decision.diagnostics.first().is_some_and(|diagnostic| {
                diagnostic.kind == crate::wip::WipDiagnosticKind::RejectedTransition
            }) =>
        {
            Ok(None)
        }
        crate::wip::WipOutcome::Block | crate::wip::WipOutcome::Rejected => {
            Err(TixError::Refused(crate::wip::render_decision(&decision)))
        }
    }
}

pub fn blocked_move_guard(
    config: &Config,
    store: &Store,
    issue: &Issue,
    column: &str,
) -> Result<MoveGuard> {
    if completion_column(config).as_deref() != Some(column) {
        return Ok(MoveGuard::Clear);
    }
    if issue.status.is(column) {
        // Already applied. See case 3 above.
        return Ok(MoveGuard::Clear);
    }
    let (issues, done_column) = store.graph_source()?;
    let graph = crate::commands::graph_over(&issues, &done_column);
    let state = graph.blocked_state(issue.number);
    Ok(if state.is_blocked {
        MoveGuard::Blocked(state)
    } else {
        MoveGuard::Clear
    })
}

/// What one guarded move did.
#[derive(Debug, Clone)]
pub struct GuardedMove {
    /// The issue as stored after the move.
    pub issue: Issue,
    /// The blocked warning, when the guard raised one.
    pub warning: Option<String>,
    /// Whether a `blocked_move_acknowledged` History event was appended.
    pub acknowledged: bool,
}

/// Apply a move through the shared guard, recording the acknowledgement when one
/// was given and required.
///
/// `acknowledged` is the caller's answer to the guard: a CLI `--yes`, a TUI
/// confirmation key, or a bulk/import path that has already made the decision. A
/// caller that passes `false` for a move the guard flagged is **refused**, and
/// refused before any file is written (SC-005) — not warned and then applied.
///
/// The History event is appended **here** and not by the TUI. Every surface
/// recording the decision through one function is what makes the audit trail
/// complete: a TUI that wrote its own event would be free to word it differently,
/// or skip it, and the History would no longer be a record of decisions.
///
/// ## WP03: two writes, and why that is not a second write *path*
///
/// An acknowledged move writes twice: the seam's [`Store::set_status`], then this
/// function's acknowledgement event. That is pre-existing behaviour and it is kept
/// deliberately, because the two entries record two different decisions — a
/// `transition_accepted` says the move was permitted, a
/// `blocked_move_acknowledged` says the user chose to proceed past open blockers.
/// Collapsing them would lose one of the two facts the audit trail exists for.
///
/// The seam is still the only *path* status is written on: this second
/// `store.update` re-serialises an issue the seam has already validated, and adds
/// a History line. It does not judge a transition, so it holds no policy. The test
/// that would catch a future leak is in the WP's Test Strategy: one
/// `transition_accepted` per accepted move, and nothing added by a refusal.
// The move's inputs are the guard's (config, store, selector, target), the write's
// (commit, override reason), the acknowledgement, and now the WIP override's event:
// eight independent facts, each of which a caller supplies or does not. A struct
// would only move the list one line up; `run` above carries the same allow.
#[allow(clippy::too_many_arguments)]
pub fn apply_guarded(
    config: &Config,
    store: &Store,
    raw: &str,
    column: &str,
    commit: Option<&str>,
    override_reason: Option<&str>,
    acknowledged: bool,
    wip: Option<&crate::wip::HistoryIntent>,
) -> Result<GuardedMove> {
    let issue = store.get(parse_selector(store, raw)?)?;
    let guard = blocked_move_guard(config, store, &issue, column)?;
    let warning = guard.warning();

    if guard.needs_acknowledgement() && !acknowledged {
        return Err(TixError::InvalidInput(format!(
            "{} — moving to '{column}' with open blockers requires an explicit \
             confirmation; re-run with --yes to record the decision",
            warning.unwrap_or_else(|| "blocked".to_string())
        )));
    }

    let acknowledgement = guard.needs_acknowledgement();
    // One transaction for the whole move (#1580). An acknowledged move writes
    // twice — the status seam's `set_status`, then this function's
    // acknowledgement event — and `apply` may itself write a second time (the
    // inbox entry stamp), so the sequence is one unit: a failure on the later
    // write must not leave the status change committed without its record.
    let moved = store.transaction(|| -> Result<crate::model::Issue> {
        let mut moved = apply(store, raw, column, commit, override_reason, wip)?;
        if acknowledgement {
            // Appended only here, only on the accepted mutation, so a re-run — which
            // the guard reports as `Clear` because the issue is already in the
            // column — cannot append a second entry. Existing entries are never
            // touched: `append_now` is the only mutation, so prior History is
            // byte-identical afterwards.
            //
            // The **service's** `reason`, not the surfaces' `blocked_note` wrapper.
            // `blocked_note` prepends `blocked: ` for the CLI warning, the TUI prompt
            // and the detail view, so using it here produced
            // `moved to done: blocked: Blocked by #0 Issue A` — the prefix twice.
            // The History line is not a surface; it is the record, and it reads
            // better naming the blockers than saying `blocked:` before naming them.
            let reason = match &guard {
                MoveGuard::Blocked(state) => state.reason.clone(),
                MoveGuard::Clear => String::new(),
            };
            moved.history.append_now(
                &crate::storage::actor_identity(),
                HistoryEventKind::BlockedMoveAcknowledged,
                &format!("moved to {column}: {reason}"),
            );
            moved.bump_updated();
            moved.filename = store.update(&moved.filename, &moved)?;
        }
        Ok(moved)
    })?;

    Ok(GuardedMove {
        issue: moved,
        warning,
        acknowledged: acknowledgement,
    })
}

/// `tix move <issue> <column>` (FR-005): validate the column, and when the
/// target is the `done_column`, prompt for a commit hash on a TTY or accept
/// `--commit <hash>`; moving *out* of done clears the recorded commit.
///
/// The interactive commit prompt (`read_line` on a TTY) is CLI glue
/// (C-005); the done-column rule itself lives in [`apply`], which the MCP
/// `move_issue` tool (WP04) calls directly.
#[allow(clippy::too_many_arguments)]
pub fn run(
    config: &Config,
    store: &Store,
    raw: &str,
    column: &str,
    commit: Option<&str>,
    override_reason: Option<&str>,
    yes: bool,
    override_wip: bool,
) -> Result<()> {
    // WP04 T016 step 2: the commit-prompt trigger used to be
    // `board.effective_done_column() == Some(column)` — a completion check that
    // lives in the command and knows nothing about categories (C-004). It is now
    // the **service's** decision on the target column, so the prompt fires for
    // exactly the moves the seam will record as reaching completion.
    //
    // These two could genuinely disagree, and the disagreement is user-visible
    // rather than academic. A board that maps `shipped` to `completed` while
    // keeping `done_column = "done"` has a completion column that is *not* the
    // column the service calls complete: the old trigger prompted for a commit
    // on a move to `done` that the seam would not record as complete, and stayed
    // silent on a move to `shipped` that it would. One of the two answers was a
    // lie, and which one depended on the board's configuration.
    let (board_name, resolution) = crate::workflow::active_resolution(config);
    let issue = store.get(parse_selector(store, raw)?)?;

    // The guard runs **before** the commit prompt: a move that will be refused
    // for open blockers should not first ask the user for a commit hash, and
    // auto-capturing `HEAD` for a move that is then refused records nothing but
    // wastes the one clean chance the user had to notice.
    let guard = blocked_move_guard(config, store, &issue, column)?;
    if let Some(warning) = guard.warning() {
        eprintln!(
            "{}",
            crate::diagnostics::Diagnostic::warning("W-tix-move", warning.clone()).render_auto()
        );
        if !yes {
            return Err(TixError::InvalidInput(format!(
                "{warning} — moving to '{column}' with open blockers requires an \
                 explicit confirmation; re-run with --yes to record the decision"
            )));
        }
    }

    // WP04 T018 (mission `configurable-wip-limits-01M3C6WX`): the **one** WIP
    // decision, from the shared policy service, before anything is written. It runs
    // after the blocker guard (a move refused for open blockers should not first be
    // judged against a limit) and before the commit prompt (a move that will be
    // refused must not ask the user for a hash). A `warn` proceeds with a warning on
    // stderr; an `override` proceeds and carries the one History entry the move
    // append writes; a `block` or a `rejected` decision refuses here, having written
    // nothing.
    let wip = wip_gate(
        config,
        store,
        &issue,
        column,
        override_reason,
        override_wip,
        yes,
    )?;

    // WP04 T016 steps 3-4: the transition is judged by the seam, on the way to the
    // write, and the reason supplied here is what lands in the `TransitionDecision`.
    //
    // **This function does not pre-judge the transition, and that is a correction.**
    // It used to call `validate_transition` here first, on the stated reasoning that
    // a refusal must be explained before the commit prompt is shown. WP04's own
    // verification showed that reasoning was wrong: the pre-judgement returned
    // `TixError::Workflow(diagnostic)` — the *same* diagnostic the seam returns — so
    // it produced an identical message and changed nothing observable. Reverting it
    // left every WP04 test green, which is what proved it was a duplicate
    // evaluation rather than a second safety net.
    //
    // It is also safe to omit, which is not obvious. A move that reaches completion
    // can never be refused: `unknown_column` cannot apply to a configured column, the
    // terminal lock only refuses leaving a terminal category, and a same-column
    // request is accepted. So no refused move ever reaches the prompt below, and
    // dropping the pre-judgement cannot make a user answer a prompt and *then* be
    // refused.
    let reaches_completion = resolution
        .resolve(column)
        .is_some_and(crate::workflow::WorkflowCategory::is_complete);

    let commit = if reaches_completion && commit.is_none() {
        // Auto-capture `HEAD` (the work commit the user just made) before
        // falling back to the interactive prompt / hard error, so the common
        // "commit, then close" flow records the hash without a second pass.
        if let Some(head) = std::env::current_dir()
            .ok()
            .as_deref()
            .and_then(crate::git::current_head)
        {
            Some(head)
        } else if std::io::stdin().is_terminal() {
            let trimmed = read_tty_line("Commit hash (or leave empty): ")?;
            if trimmed.is_empty() {
                None
            } else {
                Some(trimmed)
            }
        } else {
            return Err(TixError::InvalidInput(format!(
                "moving to '{column}' requires a commit hash — pass --commit <hash>"
            )));
        }
    } else {
        commit.map(str::to_string)
    };
    let moved = apply_guarded(
        config,
        store,
        raw,
        column,
        commit.as_deref(),
        override_reason,
        yes,
        wip.as_ref(),
    )?;
    let issue = moved.issue;
    match issue.commit.as_deref() {
        Some(hash) => println!("Moved {} to {column} (commit {hash})", issue.filename),
        None => println!("Moved {} to {column}", issue.filename),
    }
    if moved.acknowledged {
        // Said on stdout as well as stderr so a non-interactive run's log records
        // that the decision was taken deliberately rather than slipping through.
        println!("Acknowledged blocked move to {column}");
    }
    // T016 step 5: report the resolved category, not just the column. The column
    // name is what a user typed; the category is what the board considers it, and
    // on a board with a `category_map` those are different things — reporting only
    // the column leaves a user unable to tell whether a move landed in the category
    // they meant.
    if let Some(category) = crate::workflow::category_of(&issue, &resolution) {
        println!(
            "  {column} is `{}` ({}) on board {board_name}",
            category.key(),
            category.label()
        );
    }
    Ok(())
}

/// `tix move <ISSUES…> <COLUMN>` — the batch form.
///
/// A selection of one delegates to [`run`], so the single-issue surface (the
/// commit prompt, the resolved-category report) is byte-compatible. A larger
/// selection prints a plan and, with `--yes`, applies each item through the same
/// [`apply_guarded`] the single command uses: the guards, the transition seam and
/// the WIP policy are not re-implemented here. A refused item does not roll back
/// the items already applied, and the command exits `1` when any item was
/// refused, matching `tix bulk`.
#[allow(clippy::too_many_arguments)]
#[allow(clippy::too_many_lines)]
pub fn run_batch(
    config: &Config,
    store: &Store,
    args: &[String],
    select: Option<&str>,
    filter: Option<&str>,
    search: Option<&str>,
    commit: Option<&str>,
    override_reason: Option<&str>,
    yes: bool,
    override_wip: bool,
    dry_run: bool,
) -> Result<crate::cli::ExitCode> {
    use crate::cli::ExitCode;

    let Some(column) = args.last().cloned() else {
        return Err(TixError::InvalidInput(
            "`tix move` needs a target column as its last value".to_string(),
        ));
    };
    let mut selectors: Vec<String> = args[..args.len() - 1].to_vec();
    if let Some(select) = select {
        selectors.push(select.to_string());
    }
    let request = crate::commands::bulk::SelectionRequest::new(
        &selectors,
        filter,
        search,
        crate::bulk::SelectionScope::Active,
    )?;
    let selection = crate::commands::bulk::resolve(store, &request)?;
    if selection.is_empty() {
        return Err(TixError::NotFound(
            "no issues match the move selection".to_string(),
        ));
    }

    // `--dry-run` is read-only for any size, including a single selector.
    if dry_run {
        print_batch_move_plan(&selection, &column, true);
        return Ok(ExitCode::Success);
    }

    // A selection of one is the single-issue command, unchanged.
    if selection.len() == 1 {
        let raw = selection.issues()[0].number.to_string();
        run(
            config,
            store,
            &raw,
            &column,
            commit,
            override_reason,
            yes,
            override_wip,
        )?;
        return Ok(ExitCode::Success);
    }

    if !yes {
        print_batch_move_plan(&selection, &column, false);
        return Ok(ExitCode::UsageConfig);
    }

    // One commit answer for the batch. `auto` stays `auto` so each issue resolves
    // its own closing commit; an explicit hash is recorded verbatim; absent both,
    // HEAD is captured once, and a non-TTY batch refuses before writing anything.
    let (_, resolution) = crate::workflow::active_resolution(config);
    let reaches_completion = resolution
        .resolve(&column)
        .is_some_and(crate::workflow::WorkflowCategory::is_complete);
    let commit: Option<String> = if reaches_completion {
        match commit {
            Some(value) => Some(value.to_string()),
            None => {
                if let Some(head) = std::env::current_dir()
                    .ok()
                    .as_deref()
                    .and_then(crate::git::current_head)
                {
                    Some(head)
                } else if std::io::stdin().is_terminal() {
                    let answer = read_tty_line("Commit hash (or leave empty): ")?;
                    let answer = answer.trim();
                    (!answer.is_empty()).then(|| answer.to_string())
                } else {
                    return Err(TixError::InvalidInput(format!(
                        "moving to '{column}' requires a commit hash — pass --commit <hash>"
                    )));
                }
            }
        }
    } else {
        commit.map(str::to_string)
    };

    let mut moved = 0usize;
    let mut refused: Vec<(u32, String)> = Vec::new();
    for issue in selection.issues() {
        let wip = match wip_gate(
            config,
            store,
            issue,
            &column,
            override_reason,
            override_wip,
            yes,
        ) {
            Ok(intent) => intent,
            Err(error) => {
                refused.push((issue.number, error.to_string()));
                continue;
            }
        };
        match apply_guarded(
            config,
            store,
            &issue.filename,
            &column,
            commit.as_deref(),
            override_reason,
            yes,
            wip.as_ref(),
        ) {
            Ok(outcome) => {
                moved += 1;
                match outcome.issue.commit.as_deref() {
                    Some(hash) => println!("#{} moved to {column} (commit {hash})", issue.number),
                    None => println!("#{} moved to {column}", issue.number),
                }
            }
            Err(error) => refused.push((issue.number, error.to_string())),
        }
    }
    for (number, reason) in &refused {
        println!("#{number} refused: {reason}");
    }
    println!("{} moved, {} refused", moved, refused.len());
    Ok(if refused.is_empty() {
        ExitCode::Success
    } else {
        ExitCode::UsageConfig
    })
}

/// The batch plan: the selection, the target and the fact that nothing was
/// written. One line per issue, so `--dry-run` shows exactly what `--yes` would
/// act on.
fn print_batch_move_plan(selection: &crate::bulk::Selection, column: &str, dry_run: bool) {
    println!(
        "selection: {} issue{} ({}, {} scope)",
        selection.len(),
        if selection.len() == 1 { "" } else { "s" },
        selection.source().as_str(),
        selection.scope().as_str()
    );
    for issue in selection.issues() {
        println!("    #{} {}", issue.number, issue.title);
    }
    println!("would move {} issue(s) to {column}", selection.len());
    if dry_run {
        println!("dry run: nothing written");
    } else {
        println!("nothing written; re-run with --yes to apply");
    }
}

///
/// Side-effect policy: never reads `stdin`/writes `stdout` — the done-column
/// commit *prompt* stays in [`run`]; this core only applies the rule:
/// - moving into `done_column` records the passed `commit` (or `None`),
/// - `commit` of `"auto"` resolves the closing commit from git history,
/// - moving out of `done_column` clears `commit` (set to `None`).
///
/// Selector ambiguity/missing resolution is unchanged from the CLI path.
///
/// ## WP03: this is a thin adapter now
///
/// Every line of policy that used to live here — the unknown-column check, the
/// `done_column` coupling, the status assignment, the `updated` bump, the write —
/// moved into [`Store::set_status`], the storage seam. What remains is resolving
/// the selector and `"auto"`, which are command-layer concerns.
///
/// The consequence worth stating: this function can no longer be *the* move
/// implementation, so the TUI, MCP, and `tix edit --status` cannot bypass policy
/// by going around it. Research R8 found MCP could change a status with no check
/// at all, precisely because the checked path was a command-layer one and MCP had
/// a route that skipped it.
///
/// **`config` is gone from the signature.** It became unused once the policy
/// moved, because the seam reads the configuration from the `Store` that owns it.
/// A parameter left in place would tell the next reader that this function
/// consults a configuration it does not, and that is the kind of thing that
/// survives for years. The only non-test caller is [`apply_guarded`], which still
/// needs `config` for the blocked-move guard.
#[allow(clippy::too_many_arguments)]
pub fn apply(
    store: &Store,
    raw: &str,
    column: &str,
    commit: Option<&str>,
    override_reason: Option<&str>,
    wip: Option<&crate::wip::HistoryIntent>,
) -> Result<Issue> {
    let selector = parse_selector(store, raw)?;
    let issue = store.get(selector)?;

    // The override reason is written into the **same line-based `## History`
    // grammar** every other move uses, so a line break in it would end the event's
    // line and start another the next reader parses as a real event. Refused, not
    // stripped, and through the shared predicate the triage service uses — one
    // decision about the grammar rather than a fourth local copy (tix #1493). The
    // value is named by *meaning* so a surface whose flag is spelled differently
    // (MCP's `reason`, the TUI's prompt) still reads correctly.
    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 commit: Option<String> = match commit {
        Some("auto") => Some(crate::commands::resolve_auto_commit(store, &issue)?),
        Some(other) => Some(other.to_string()),
        None => None,
    };

    // The seam: validates against the transition service, applies the done-column
    // commit coupling, appends exactly one `TransitionDecision`, and writes once.
    // A refusal arrives as `Err(TixError::Workflow(_))` having touched nothing.
    let outcome = store.set_status(
        &issue.filename,
        crate::storage::StatusChange::to(column)
            .with_commit(commit.as_deref())
            .with_override(override_reason),
    )?;

    // A move into the inbox is an **entry**: stamp the queue age it is measured
    // from, with the `moved` origin (tix #1489). The status seam wrote the move
    // first — the landing column is what decides whether the stamp is due — so the
    // stamp rides a second write, and only when the move actually wrote (a
    // same-column re-apply must not restart the wait).
    let mut moved = outcome.issue;
    if outcome.written && crate::triage::record_move_entry(&store.config, &mut moved)? {
        moved.filename = store.update(&moved.filename, &moved)?;
    }
    // The override's one History entry (WP04 T020): appended **only** when the move
    // actually wrote, through the same `## History` grammar every other event uses,
    // and with the actor and timestamp the override was authorized with rather than
    // a fresh clock read. A refused or same-column move writes nothing here, so a
    // repeated request cannot append a second entry.
    if let Some(intent) = wip
        && outcome.written
    {
        moved.history.append(crate::model::HistoryEvent::new(
            intent.timestamp.clone(),
            &intent.actor,
            intent.kind,
            &intent.detail,
        ));
        moved.bump_updated();
        moved.filename = store.update(&moved.filename, &moved)?;
    }
    Ok(moved)
}

#[cfg(test)]
mod tests {
    #[allow(clippy::wildcard_imports)]
    use super::*;
    use crate::commands::create::create;
    use crate::model::Issue;
    use crate::model::Status;
    use crate::storage::IssueSelector;
    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(),
            ..crate::config::Config::default()
        };
        config.save(&config_path).unwrap();
        let config = crate::config::Config::load_with_warning(&config_path, |_| {}).unwrap();
        (
            tmp,
            Store {
                issues_dir: dir,
                config,
            },
        )
    }

    /// Seed an issue with a **distinct** title.
    ///
    /// The title has to vary per call: the filename is derived from it, so two
    /// issues with the same title resolve to the same path and the second
    /// silently overwrites the first. That was invisible while every test seeded
    /// exactly one issue — `create` returns the in-memory record, so a clobbered
    /// issue still *looks* created while `store.get` cannot find it. Seeding more
    /// than one is what exposed it.
    fn seed_named(store: &Store, title: &str) -> Issue {
        create(
            &store.config,
            store,
            title,
            Some("body"),
            None,
            None,
            Some(""),
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
            None,
        )
        .unwrap()
    }

    /// Seed an issue with a title unique to this call.
    fn seed(store: &Store) -> Issue {
        thread_local! {
            static COUNTER: std::cell::Cell<u32> = const { std::cell::Cell::new(0) };
        }
        let n = COUNTER.with(|c| {
            let n = c.get();
            c.set(n + 1);
            n
        });
        seed_named(store, &format!("Move me {n}"))
    }

    #[test]
    fn apply_into_done_records_commit() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let seeded = seed(&store);
        let moved = apply(
            &store,
            &seeded.number.to_string(),
            "done",
            Some("abc123"),
            None,
            None,
        )
        .unwrap();
        assert_eq!(moved.status.display(), "done");
        assert_eq!(moved.commit.as_deref(), Some("abc123"));
        assert_eq!(moved.number, seeded.number);
        let reloaded = store
            .get(crate::storage::IssueSelector::Filename(
                moved.filename.clone(),
            ))
            .unwrap();
        assert_eq!(reloaded.commit.as_deref(), Some("abc123"));
    }

    #[test]
    fn apply_out_of_done_clears_commit() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let seeded = seed(&store);
        apply(
            &store,
            &seeded.number.to_string(),
            "done",
            Some("abc123"),
            /* override_reason */ None,
            None,
        )
        .unwrap();
        // WP03: on a board with **no** `category_map` — which is every board in
        // the wild before this mission — the move out of `done` is permitted, so
        // the commit is cleared exactly as before. This is the half of the
        // behaviour that must not change: the terminal lock and adjacency are
        // opt-in, and this is the proof that a legacy board does not acquire a
        // policy it never asked for.
        let moved_out = apply(
            &store,
            &seeded.number.to_string(),
            "in progress",
            None,
            None,
            None,
        )
        .unwrap();
        assert_eq!(moved_out.status.display(), "in progress");
        assert_eq!(
            moved_out.commit, None,
            "moving out of done clears the commit"
        );
    }

    #[test]
    fn apply_out_of_done_is_refused_on_a_board_that_declared_a_category_map() {
        // The other half of the test above, and the reason the opt-in is keyed on
        // `category_map` rather than on `terminal_locked`. The same move, on a
        // board whose author *did* describe their categories, is refused by name
        // and writes nothing.
        let (tmp, store) = store_with_category_map();
        let _ = tmp;
        let seeded = seed(&store);
        let moved = apply(
            &store,
            &seeded.number.to_string(),
            "done",
            Some("abc123"),
            None,
            None,
        )
        .unwrap();
        let before = std::fs::read_to_string(store.issues_dir.join(&moved.filename)).unwrap();

        let refused =
            apply(&store, &seeded.number.to_string(), "todo", None, None, None).unwrap_err();
        assert!(
            matches!(&refused, TixError::Workflow(d) if d.kind.as_str() == "invalid_transition"),
            "expected the named `invalid_transition`, got: {refused:?}"
        );
        let text = refused.to_string();
        assert!(
            text.contains("needs an explicit override with a reason"),
            "and the message must say how to proceed: {text}"
        );
        assert_eq!(
            std::fs::read_to_string(store.issues_dir.join(&moved.filename)).unwrap(),
            before,
            "a refusal must be byte-identical, not merely unsuccessful"
        );

        // With a reason, the same move applies, and the reason is recorded so the
        // audit trail explains itself.
        let moved_out = apply(
            &store,
            &seeded.number.to_string(),
            "todo",
            None,
            Some("reopening: filed against the wrong epic"),
            None,
        )
        .unwrap();
        assert_eq!(moved_out.status.display(), "todo");
        assert_eq!(moved_out.commit, None, "and the commit is cleared");
        let events = moved_out.history.events();
        let decision = events
            .iter()
            .rev()
            .find(|e| e.kind == HistoryEventKind::TransitionAccepted)
            .expect("the accepted move records a decision");
        let parsed = crate::model::TransitionDecision::from_event(decision).unwrap();
        assert_eq!(parsed.from, "done");
        assert_eq!(parsed.to, "todo");
        assert_eq!(parsed.from_category, "completed");
        assert_eq!(parsed.to_category, "unstarted");
        assert_eq!(
            parsed.override_reason.as_deref(),
            Some("reopening: filed against the wrong epic"),
            "the reason is recorded, so the audit trail explains the override"
        );
    }

    #[test]
    fn apply_into_done_without_commit_leaves_none() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let seeded = seed(&store);
        let moved = apply(&store, &seeded.number.to_string(), "done", None, None, None).unwrap();
        assert_eq!(moved.status.display(), "done");
        assert_eq!(moved.commit, None);
    }

    #[test]
    fn apply_rejects_unknown_column() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let seeded = seed(&store);
        let err = apply(
            &store,
            &seeded.number.to_string(),
            "bogus",
            None,
            None,
            None,
        )
        .unwrap_err();
        // WP03: the service owns this message now, and it names the *kind* before
        // the prose — so this asserts the contract-relevant part (the shared
        // diagnostic name, contract §6) as well as the full text. Before WP03 this
        // was `TixError::InvalidInput`, a string identical in shape to a typo
        // caught by argument parsing, so a caller could not tell "no such column"
        // from "that move is against policy". That distinction is the reason the
        // variant exists, so it is asserted rather than assumed.
        assert!(
            matches!(&err, TixError::Workflow(d) if d.kind.as_str() == "unknown_column"),
            "expected the named `unknown_column` diagnostic, got: {err:?}"
        );
        assert_eq!(
            err.to_string(),
            "transition rejected (unknown_column) for todo -> bogus: `bogus` is not a \
             column on board `default`; valid columns: todo, in progress, in review, done"
        );
    }

    #[test]
    fn apply_invalid_selector_is_not_found() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let _ = seed(&store);
        let err = apply(&store, "999", "todo", None, None, None).unwrap_err();
        assert!(matches!(err, TixError::NotFound(_)));
    }

    // ---- T016/T017: the blocked-move guard ---------------------------

    /// A store whose board **declares a `category_map`**, i.e. one that has opted
    /// into category policy.
    ///
    /// WP03. The lock and adjacency are gated on exactly this (see
    /// `CategoryResolution::explicit_policy`), so a test that wants policy needs
    /// this and a test that wants the *absence* of policy needs [`make_store`],
    /// which has no map. The mapping is chosen so a move out of `done` is
    /// unambiguous: `done` is `completed`, and `todo`/`doing` are `unstarted` —
    /// which is backwards in the declared order, so the lock fires first and the
    /// diagnostic is the lock's rather than adjacency's.
    fn store_with_category_map() -> (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(),
            kanban: crate::config::KanbanSection {
                configurations: indexmap::IndexMap::from([(
                    "default".to_string(),
                    crate::config::BoardConfiguration {
                        columns: vec!["todo".into(), "doing".into(), "done".into()],
                        done_column: Some("done".to_string()),
                        category_map: indexmap::IndexMap::from([
                            ("todo".to_string(), "unstarted".to_string()),
                            ("doing".to_string(), "active".to_string()),
                            ("done".to_string(), "completed".to_string()),
                        ]),
                        ..Default::default()
                    },
                )]),
            },
            ..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,
            },
        )
    }

    /// A store whose board names its completion column `done_column`, default
    /// `done`.
    fn store_with_done_column(done_column: &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 config = crate::config::Config {
            issues_dir: dir.clone(),
            kanban: crate::config::KanbanSection {
                configurations: indexmap::IndexMap::from([(
                    "default".to_string(),
                    crate::config::BoardConfiguration {
                        columns: vec!["todo".into(), "doing".into(), done_column.into()],
                        done_column: Some(done_column.to_string()),
                        // Derived, not listed: naming every field here would make
                        // the test fail to compile the next time one is added, for
                        // no extra assurance about the guard.
                        ..Default::default()
                    },
                )]),
            },
            ..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,
            },
        )
    }

    /// A blocker and a blocked issue joined by an open `blocks` edge, plus a
    /// third issue that is only `related` to the blocked one.
    fn seed_blocked_pair(store: &Store) -> (Issue, Issue, Issue) {
        let blocker = seed(store);
        let subject = seed(store);
        let friend = seed(store);
        store
            .link(
                &blocker.number.to_string(),
                &subject.number.to_string(),
                crate::relationships::RelationKind::Blocks,
            )
            .unwrap();
        store
            .link(
                &friend.number.to_string(),
                &subject.number.to_string(),
                crate::relationships::RelationKind::Related,
            )
            .unwrap();
        (blocker, subject, friend)
    }

    /// Every `blocked_move_acknowledged` events in an issue, in file order.
    fn acknowledged_events(issue: &Issue) -> Vec<String> {
        issue
            .history
            .events()
            .iter()
            .filter(|e| e.kind == HistoryEventKind::BlockedMoveAcknowledged)
            .map(|e| e.detail.clone())
            .collect()
    }

    #[test]
    fn t016_a_blocked_move_toward_completion_is_flagged_and_names_the_blocker() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let (blocker, subject, _) = seed_blocked_pair(&store);
        let guard = blocked_move_guard(
            &store.config,
            &store,
            &store.get(IssueSelector::Number(subject.number)).unwrap(),
            "done",
        )
        .unwrap();
        assert!(guard.needs_acknowledgement(), "{guard:?}");
        let warning = guard.warning().unwrap();
        assert!(
            warning.contains(&blocker.number.to_string()),
            "the warning names the blocking issue: {warning}"
        );
        assert!(
            warning.contains("blocked:"),
            "and uses the shared wording every surface renders: {warning}"
        );
    }

    #[test]
    fn t016_the_warning_is_the_service_reason_verbatim() {
        // T016's note: warning content is part of the contract, and it must be
        // derived from `BlockedState.reason` rather than composed per surface.
        let (tmp, store) = make_store();
        let _ = tmp;
        let (_, subject, _) = seed_blocked_pair(&store);
        let issue = store.get(IssueSelector::Number(subject.number)).unwrap();
        let (issues, done_column) = store.graph_source().unwrap();
        let graph = crate::commands::graph_over(&issues, &done_column);
        let expected = graph.blocked_state(subject.number).reason;
        let guard = blocked_move_guard(&store.config, &store, &issue, "done").unwrap();
        assert_eq!(
            guard.warning().unwrap(),
            format!("blocked: {expected}"),
            "the guard must not re-word the service"
        );
    }

    #[test]
    fn t016_without_acknowledgement_the_move_is_refused_and_the_file_is_unchanged() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let (_, subject, _) = seed_blocked_pair(&store);
        let sel = subject.number.to_string();
        let before = fs::read_to_string(store.issues_dir.join(&subject.filename)).unwrap();

        let err = apply_guarded(
            &store.config,
            &store,
            &sel,
            "done",
            Some("abc123"),
            None,
            false,
            None,
        )
        .unwrap_err();
        let text = err.to_string();
        assert!(
            text.contains("blocked"),
            "the refusal names the reason: {text}"
        );
        assert!(text.contains("--yes"), "and says how to proceed: {text}");
        assert_eq!(
            fs::read_to_string(store.issues_dir.join(&subject.filename)).unwrap(),
            before,
            "a refused move must not write (SC-005)"
        );
    }

    #[test]
    fn t016_with_acknowledgement_the_move_applies_and_records_exactly_one_event() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let (_, subject, _) = seed_blocked_pair(&store);
        let moved = apply_guarded(
            &store.config,
            &store,
            &subject.number.to_string(),
            "done",
            Some("abc123"),
            None,
            true,
            None,
        )
        .unwrap();
        assert!(moved.acknowledged);
        assert_eq!(moved.issue.status.display(), "done");

        let reread = store.get(IssueSelector::Number(subject.number)).unwrap();
        let events = acknowledged_events(&reread);
        assert_eq!(events.len(), 1, "exactly one event, got {events:?}");
        assert!(
            events[0].contains("done"),
            "the event records the decision: {events:?}"
        );
    }

    #[test]
    fn t016_rerunning_the_move_is_idempotent() {
        // "no second warning storm, no duplicate History entry". Made a property
        // of the *guard* — an issue already in the target column is `Clear` — so
        // a caller that forgets to check still cannot duplicate the event.
        let (tmp, store) = make_store();
        let _ = tmp;
        let (_, subject, _) = seed_blocked_pair(&store);
        let sel = subject.number.to_string();

        let first = apply_guarded(
            &store.config,
            &store,
            &sel,
            "done",
            Some("abc"),
            None,
            true,
            None,
        )
        .unwrap();
        assert!(first.acknowledged);
        let after_first = store.get(IssueSelector::Number(subject.number)).unwrap();

        let second = apply_guarded(
            &store.config,
            &store,
            &sel,
            "done",
            Some("abc"),
            None,
            true,
            None,
        )
        .unwrap();
        assert!(!second.acknowledged, "a re-run acknowledges nothing");
        assert!(
            second.warning.is_none(),
            "and must not warn again: {:?}",
            second.warning
        );
        let after_second = store.get(IssueSelector::Number(subject.number)).unwrap();
        assert_eq!(
            acknowledged_events(&after_second).len(),
            1,
            "no second acknowledgement event"
        );
        assert_eq!(
            after_second.history.events().len(),
            after_first.history.events().len(),
            "no History growth at all on a re-run"
        );
    }

    #[test]
    fn t016_history_is_append_only_across_an_acknowledged_move() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let (blocker, subject, _) = seed_blocked_pair(&store);
        let blocker = store.get(IssueSelector::Number(blocker.number)).unwrap();
        let before = store.get(IssueSelector::Number(subject.number)).unwrap();
        let prior: Vec<String> = before
            .history
            .events()
            .iter()
            .map(|e| format!("{e:?}"))
            .collect();
        assert!(
            !blocker.history.events().is_empty(),
            "precondition: the edge recorded an event on its canonical owner"
        );

        apply_guarded(
            &store.config,
            &store,
            &subject.number.to_string(),
            "done",
            Some("abc"),
            None,
            true,
            None,
        )
        .unwrap();

        let after = store.get(IssueSelector::Number(subject.number)).unwrap();
        let now: Vec<String> = after
            .history
            .events()
            .iter()
            .map(|e| format!("{e:?}"))
            .collect();
        assert_eq!(
            &now[..prior.len()],
            &prior[..],
            "prior entries must be byte-identical and in the same order"
        );
        // WP03: an acknowledged move appends **two** entries, not one — a
        // `transition_accepted` from the seam and a `blocked_move_acknowledged`
        // from the guard. They record two different decisions (the move was
        // permitted; the user chose to proceed past open blockers) and collapsing
        // them would lose one. The kinds are asserted, not just the count, because
        // a count of 2 would also be satisfied by a duplicated
        // acknowledgement and by nothing else being recorded at all.
        assert_eq!(
            now.len(),
            prior.len() + 2,
            "a transition decision and an acknowledgement, nothing else"
        );
        let appended: Vec<HistoryEventKind> = after.history.events()[prior.len()..]
            .iter()
            .map(|e| e.kind)
            .collect();
        assert_eq!(
            appended,
            vec![
                HistoryEventKind::TransitionAccepted,
                HistoryEventKind::BlockedMoveAcknowledged,
            ],
            "in that order: the seam writes first, then the guard appends its own"
        );
    }

    #[test]
    fn t016_moving_to_a_non_completion_column_does_not_warn() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let (_, subject, _) = seed_blocked_pair(&store);
        let issue = store.get(IssueSelector::Number(subject.number)).unwrap();
        // The default board's own columns, not invented ones — a test that names
        // a column the board does not have would be asserting the "unknown
        // column" error, not the guard.
        for column in ["todo", "in progress", "in review"] {
            assert!(
                store
                    .config
                    .default_config()
                    .columns
                    .iter()
                    .any(|c| c == column),
                "precondition: `{column}` is a real column"
            );
            let guard = blocked_move_guard(&store.config, &store, &issue, column).unwrap();
            assert!(
                !guard.needs_acknowledgement(),
                "blockers do not block a move to {column}"
            );
        }
        // And the move applies without an acknowledgement.
        let moved = apply_guarded(
            &store.config,
            &store,
            &subject.number.to_string(),
            "in progress",
            None,
            None,
            false,
            None,
        )
        .unwrap();
        assert!(!moved.acknowledged);
        assert_eq!(moved.issue.status.display(), "in progress");
    }

    #[test]
    fn t016_a_custom_done_column_is_resolved_through_config() {
        // The test that catches a hard-coded `"done"` (T017's own note). A board
        // whose completion column is `shipped` has no `done` column at all, so an
        // implementation matching the literal would never guard `shipped`.
        let (tmp, store) = store_with_done_column("shipped");
        let _ = tmp;
        let (blocker, subject, _) = seed_blocked_pair(&store);
        let issue = store.get(IssueSelector::Number(subject.number)).unwrap();

        assert_eq!(
            completion_column(&store.config).as_deref(),
            Some("shipped"),
            "precondition: the configured completion column"
        );
        let guard = blocked_move_guard(&store.config, &store, &issue, "shipped").unwrap();
        assert!(
            guard.needs_acknowledgement(),
            "moving to the configured completion column must be guarded"
        );
        assert!(
            guard
                .warning()
                .unwrap()
                .contains(&blocker.number.to_string())
        );

        // And `done` is not a column on this board, so it cannot be the guard's
        // trigger by accident.
        let err = apply_guarded(
            &store.config,
            &store,
            &subject.number.to_string(),
            "shipped",
            None,
            None,
            false,
            None,
        )
        .unwrap_err();
        assert!(err.to_string().contains("blocked"), "got: {err}");

        // An unblocked issue moves to `shipped` freely.
        let mut done_blocker = store.get(IssueSelector::Number(blocker.number)).unwrap();
        done_blocker.status = Status::new("shipped".to_string());
        store.update(&done_blocker.filename, &done_blocker).unwrap();
        let moved = apply_guarded(
            &store.config,
            &store,
            &subject.number.to_string(),
            "shipped",
            None,
            None,
            false,
            None,
        )
        .unwrap();
        assert!(
            !moved.acknowledged,
            "a blocker that reached the completion column does not block"
        );
    }

    #[test]
    fn t016_an_unblocked_issue_moves_freely_and_records_nothing() {
        let (tmp, store) = make_store();
        let _ = tmp;
        let a = seed(&store);
        seed(&store);
        let moved = apply_guarded(
            &store.config,
            &store,
            &a.number.to_string(),
            "done",
            Some("abc"),
            None,
            false,
            None,
        )
        .unwrap();
        assert!(!moved.acknowledged);
        assert!(moved.warning.is_none());
        assert_eq!(
            acknowledged_events(&moved.issue),
            [] as [std::string::String; 0]
        );
    }
}