tix/triage/

decision.rs

//! The triage decision service: accept, decline, duplicate, defer.
//!
//! Mission `triage-inbox-review-flow-01M3C600`, WP03 (T012–T016). One typed
//! request in, either one append-only `History` event intent out or a refusal —
//! and **nothing else**. The service never opens, reads or writes a file:
//! [`DecisionService::apply`] takes `&Issue` and returns a *clone* carrying the
//! decision, so a rejected or repeated decision cannot have side effects that
//! someone would have to undo (data-model §3, contract `decision-history.md`).
//!
//! What it deliberately does **not** do:
//!
//! - **No second workflow.** Every status target goes through
//!   [`crate::workflow::validate_transition`]; the completion and terminal rules
//!   are #183's and are not restated here (C-002).
//! - **No second History log.** A decision is one line in the existing
//!   `## History` section, rendered by [`crate::triage::history`] (C-004).
//! - **No destructive outcome.** Decline moves to the configured `canceled`
//!   column; duplicate returns a typed relation and never merges, deletes or
//!   moves a file (C-003, C-006). Merging is issue 189's.
//!
//! ## Order of checks
//!
//! The refusal order is fixed and each step is observable, because two
//! refusals that both apply must always produce the *same* one:
//!
//! 1. required fields — [`Refusal::missing_reason`], [`Refusal::missing_actor`];
//! 2. the requested target — [`Refusal::missing_target`],
//!    [`Refusal::invalid_duplicate_target`], [`Refusal::unknown_issue`];
//! 3. idempotence — [`DecisionOutcome::NoOp`];
//! 4. queue membership — [`Refusal::not_in_triage`] or
//!    [`Refusal::already_decided`];
//! 5. the shared transition policy — [`Refusal::from_workflow`] (which surfaces
//!    #183's `unknown_column` / `invalid_transition` unchanged).
//!
//! Step 3 before step 4 is load-bearing: declining twice is a no-op **even
//! though the first decline took the issue out of the inbox**, so the
//! idempotence question has to be asked before "is it still queued?". Step 2
//! before both means a malformed request is refused identically whatever the
//! issue happens to look like — a typo is not a state-dependent error.

use std::fmt;
use std::str::FromStr;

use crate::config::Config;
use crate::error::{Result, TixError};
use crate::model::{HistoryEvent, HistoryEventKind, Issue, Status};
use crate::relationships::{CanonicalEdge, RelationKind, Relationship};
use crate::triage::history::{self, TriageDecisionRecord};
use crate::triage::inbox::InboxScope;
use crate::triage::record_entry;
use crate::workflow::{self, DiagnosticKind, TransitionOutcome, WorkflowDiagnostic};

/// This service's fallible type: a [`Refusal`] is the only failure it produces.
///
/// Deliberately not [`crate::error::Result`]. A refusal carries a *contract*
/// diagnostic name and an issue identity, and collapsing it into a bare
/// [`TixError`] at the boundary where it is raised would mean the CLI, TUI and
/// MCP surfaces (NFR-003) each had to re-derive which condition was hit from
/// prose. [`Refusal`] converts with `?`/`Into` when a surface wants the crate's
/// error type.
type DecisionResult<T> = std::result::Result<T, Refusal>;

/// The four decisions (data-model §3, `kind`).
///
/// A closed set with no "other": there is no automatic decision path (C-008),
/// so every decision is one of these four, named explicitly by the caller.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum DecisionKind {
    /// Move into the normal workflow: `--to`, or `[triage].accepted_status`.
    Accept,
    /// Move to the configured declined column. Never delete, never archive.
    Decline,
    /// Record a typed `duplicates` relation against another issue.
    Duplicate,
    /// Stay in the inbox with a refreshed entry time.
    Defer,
}

impl DecisionKind {
    /// The canonical lowercase spelling used on the CLI and in messages.
    #[must_use]
    pub fn as_str(self) -> &'static str {
        match self {
            DecisionKind::Accept => "accept",
            DecisionKind::Decline => "decline",
            DecisionKind::Duplicate => "duplicate",
            DecisionKind::Defer => "defer",
        }
    }

    /// The `## History` kind an applied decision of this kind appends.
    ///
    /// The mapping lives here rather than at each call site so that "which
    /// history entry does this decision write?" has one answer — the service
    /// asks it to compare against the last recorded decision, and a test asks it
    /// to assert the event that landed.
    #[must_use]
    pub fn event_kind(self) -> HistoryEventKind {
        match self {
            DecisionKind::Accept => HistoryEventKind::TriageAccepted,
            DecisionKind::Decline => HistoryEventKind::TriageDeclined,
            DecisionKind::Duplicate => HistoryEventKind::TriageDuplicated,
            DecisionKind::Defer => HistoryEventKind::TriageDeferred,
        }
    }

    /// Whether this decision refuses to proceed without a reason.
    ///
    /// **Decline, duplicate and defer require one; accept does not.** The
    /// contract says decline/duplicate/defer "require a reason" and that accept
    /// "records a reason when supplied", and data-model §3 repeats the split —
    /// so an accept with no reason produces an event whose `reason` field is
    /// absent rather than a refusal. Whether *the CLI* insists on `--reason` for
    /// accept is the surface's own spelling question (contract `cli-surface.md`
    /// defers the reason rules to `decision-history.md`), not a rule about what
    /// a decision is.
    #[must_use]
    pub fn requires_reason(self) -> bool {
        !matches!(self, DecisionKind::Accept)
    }
}

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

impl FromStr for DecisionKind {
    type Err = TixError;

    /// Parse a decision name. Unknown names are rejected rather than coerced:
    /// guessing between `decline` and `defer` on a batch would apply a decision
    /// nobody chose.
    fn from_str(s: &str) -> Result<Self> {
        match s.trim().to_ascii_lowercase().as_str() {
            "accept" => Ok(DecisionKind::Accept),
            "decline" => Ok(DecisionKind::Decline),
            "duplicate" => Ok(DecisionKind::Duplicate),
            "defer" => Ok(DecisionKind::Defer),
            other => Err(TixError::InvalidInput(format!(
                "unknown triage decision `{other}` — expected accept, decline, \
                 duplicate, or defer"
            ))),
        }
    }
}

/// A typed decision request (data-model §3).
///
/// Everything a decision needs to be judged **without** touching the filesystem:
/// the decision itself, who made it, when, and the optional targets. The issue
/// under decision is not a field — it is the `issue` argument of
/// [`DecisionService::apply`], because one batch decision is applied to many
/// issues and a request that embedded its subject would have to be rebuilt per
/// item (contract `decision-history.md`, §Batch behavior).
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct TriageDecision {
    /// Which decision this is.
    pub kind: DecisionKind,
    /// The recorded reason, verbatim. Required for decline, duplicate and
    /// defer; see [`DecisionKind::requires_reason`].
    pub reason: String,
    /// Who decided. Required: an audit entry with no actor is worse than no
    /// entry (contract §Required reason and actor).
    pub actor: String,
    /// The duplicate target's issue number, for a duplicate decision.
    pub target_issue: Option<u32>,
    /// The accept target column (`--to`). Absent means "the configured
    /// `[triage].accepted_status`".
    pub target_status: Option<String>,
    /// The decision time, RFC 3339. Supplied rather than read from the clock so
    /// a batch stamps every event identically and a test is deterministic.
    pub timestamp: String,
}

impl TriageDecision {
    /// A decision with no optional target and an explicit timestamp.
    ///
    /// The timestamp is a parameter rather than a default for the same reason
    /// [`crate::triage::inbox::collect`] takes `today`: a service that reads the
    /// wall clock cannot be tested against a pinned time, and a batch would
    /// otherwise hand `now()` to a loop that runs it once per item.
    #[must_use]
    pub fn new(
        kind: DecisionKind,
        timestamp: impl Into<String>,
        actor: impl Into<String>,
        reason: impl Into<String>,
    ) -> Self {
        Self {
            kind,
            reason: reason.into(),
            actor: actor.into(),
            target_issue: None,
            target_status: None,
            timestamp: timestamp.into(),
        }
    }
}

/// How the caller resolved the duplicate target's identity.
///
/// The service validates *identities*; resolving a selector (`--of some title`)
/// against the store is the caller's job, because that is the part that needs
/// the files. This enum carries the outcome of that resolution so the service
/// can still name the failure itself — a duplicate decision's diagnostics are
/// the contract's, not the caller's, and "matched three issues" is one of them.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum DuplicateTarget<'a> {
    /// The selector resolved to exactly one issue.
    Resolved(&'a Issue),
    /// The selector resolved to no issue at all.
    Unknown,
    /// The selector matched more than one issue, so no single identity exists.
    Ambiguous,
}

/// What a decision plans to do, decided before any state is read.
///
/// The intermediate between "what was asked for" and "what will change" exists
/// so that steps 2 and 3 of the order can be answered *before* step 5 validates
/// the move: idempotence has to be judged on the resolved target (an accept
/// whose `--to` equals the current column is a no-op), and a duplicate's
/// relation needs its target's title, which only the resolution has.
#[derive(Debug, Clone, PartialEq, Eq)]
enum Plan {
    /// Move to this column, once #183 accepts the transition.
    Move(String),
    /// Create this typed relation; the issue's column does not change.
    Duplicate {
        target_number: u32,
        target_title: String,
    },
}

/// Why a decision was refused, and for which issue (contract §Diagnostics).
///
/// One struct rather than an enum of refusals: the contract fixes a *stable set
/// of names* (`missing_reason`, `unknown_issue`, `unknown_column`,
/// `invalid_transition`, `invalid_duplicate_target`, `not_in_triage`,
/// `already_decided`, `write_failed` — "include", so the list is a vocabulary
/// and not a closed set) and a message, and a surface that matches on eight
/// variants gains nothing over one that matches on the name string it has to
/// render anyway. The names are pinned by tests, which is what makes them
/// stable.
///
/// Item identity reaches a surface through the refusal's own message (or the
/// batch line that prefixes it with `#<n>`), not through a stored field: the
/// three accessors that once exposed it had no caller, and NFR-003's "same kind
/// **and item identity**" is checked where a user sees it (tix #1495).
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Refusal {
    kind: &'static str,
    message: String,
}

impl Refusal {
    /// Build a refusal directly. Public so a caller that resolves the *input*
    /// (a selector that matched nothing, a malformed batch flag) reports the
    /// same contract name the service would.
    #[must_use]
    pub fn new(kind: &'static str, message: impl Into<String>) -> Self {
        Self {
            kind,
            message: message.into(),
        }
    }

    /// A required reason is absent (`missing_reason`).
    fn missing_reason(issue: u32, kind: DecisionKind) -> Self {
        Self::new(
            "missing_reason",
            format!("`#{issue}` {kind} requires a reason"),
        )
    }

    /// No actor was supplied (`missing_actor`).
    fn missing_actor(issue: u32) -> Self {
        Self::new(
            "missing_actor",
            format!(
                "`#{issue}` needs an actor: a triage decision is never recorded \
                 anonymously"
            ),
        )
    }

    /// Accept was given neither `--to` nor `[triage].accepted_status`
    /// (`missing_target`).
    ///
    /// The contract's stable list does not spell this name out, and no other
    /// name fits: `unknown_column` would claim a column was named when none was,
    /// and `invalid_transition` would claim #183 refused a move it was never
    /// asked about. "Stable kinds *include*" makes the list non-exhaustive, so
    /// the vocabulary grows by one rather than borrowing a name that lies.
    fn missing_target(issue: u32) -> Self {
        Self::new(
            "missing_target",
            format!(
                "`#{issue}` accept has no target column: pass `--to`, or set \
                 `[triage].accepted_status`"
            ),
        )
    }

    /// The duplicate target is structurally wrong: absent, a self link, or
    /// ambiguous (`invalid_duplicate_target`).
    fn invalid_duplicate_target(message: impl Into<String>) -> Self {
        Self::new("invalid_duplicate_target", message)
    }

    /// The requested identity resolves to no issue (`unknown_issue`).
    fn unknown_issue(message: impl Into<String>) -> Self {
        Self::new("unknown_issue", message)
    }

    /// `--of` matched more than one issue.
    fn ambiguous() -> Self {
        Self::invalid_duplicate_target(
            "`--of` matched more than one issue; name the duplicate target by \
             its number",
        )
    }

    /// The issue is not in the triage inbox (`not_in_triage`).
    fn not_in_triage(issue: u32) -> Self {
        Self::new(
            "not_in_triage",
            format!(
                "`#{issue}` is not in the triage inbox: decisions are made from \
                 the queue, and this issue is in a normal column"
            ),
        )
    }

    /// The issue already carries a triage decision (`already_decided`).
    ///
    /// Told apart from [`Self::not_in_triage`] by whether the file records a
    /// decision at all: an issue that never was triaged and sits in a normal
    /// column was simply never queued, while one that left the queue *with* a
    /// decision has used its triage review. Changing it again is a normal status
    /// change (`tix move`), which is where that belongs.
    fn already_decided(issue: u32) -> Self {
        Self::new(
            "already_decided",
            format!(
                "`#{issue}` already carries a triage decision and has left the \
                 inbox; change it with a normal status command instead"
            ),
        )
    }

    /// A refusal from #183's transition service, passed through with its own
    /// name (`unknown_column`, `invalid_transition`).
    ///
    /// The workflow diagnostic is **not** re-interpreted: #183 owns what a legal
    /// transition is (C-002), so the only thing done here is translating its
    /// [`DiagnosticKind`] into this contract's spelling — and two of the seven
    /// workflow kinds are the only ones `validate_transition` can produce. Every
    /// other refusal it could ever be handed — the terminal lock, adjacency, the
    /// completion guard — is one thing to a triage caller: the move is against
    /// policy, which is the contract's `invalid_transition`.
    fn from_workflow(diagnostic: &WorkflowDiagnostic) -> Self {
        let kind = match diagnostic.kind {
            DiagnosticKind::UnknownColumn => "unknown_column",
            _ => "invalid_transition",
        };
        Self::new(
            kind,
            format!("{}: {}", diagnostic.subject, diagnostic.message),
        )
    }
}

impl fmt::Display for Refusal {
    /// `kind: message`, rendered by the crate's one refusal renderer so a
    /// message that already leads with its kind is not prefixed twice.
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        // The **value** form, plain: `kind: message`, and no prefix when the message
        // already leads with its kind. This is a stored message the batch embeds in
        // an item outcome, not a terminal block — the rustc-style rendering happens
        // once, at the boundary, in `From<Refusal> for TixError`.
        if self.message.starts_with(self.kind) {
            f.write_str(&self.message)
        } else {
            write!(f, "{}: {}", self.kind, self.message)
        }
    }
}

impl From<Refusal> for TixError {
    /// A refusal becomes a rendered block: the one place a refusal is turned into
    /// something a surface prints, so its rustc-style header is produced exactly
    /// once, here, rather than by every caller that happens to `to_string()` it.
    fn from(refusal: Refusal) -> Self {
        TixError::Refused(crate::error::render_diagnostics(
            "E-tix-triage",
            [(refusal.kind, refusal.message.as_str())],
        ))
    }
}

/// The result of one applied decision: the updated issue and the event intent
/// (data-model §4).
///
/// The issue is a **clone the caller still has to write** — the service hands
/// back what *would* be on disk, and storage performs the write. That is the
/// whole filesystem-free claim: every rejection and every no-op returns before
/// this variant exists, so "did anything change?" is answerable from the return
/// value alone.
///
/// `Applied` is far larger than `NoOp`, which trips `clippy::large_enum_variant`
/// — and boxing would add an allocation to a value that is *moved*, not copied,
/// on the only path that constructs it. The size comes from `Issue`, which the
/// service clones by necessity (see [`DecisionService::apply`]): the caller must
/// be able to discard the result without having given anything up.
#[allow(clippy::large_enum_variant)]
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum DecisionOutcome {
    /// One decision applied: exactly one event, appended to the issue's own
    /// History (NFR-002).
    Applied {
        /// The issue as the decision leaves it: status moved, entry metadata
        /// updated, and `event` appended to its History.
        issue: Issue,
        /// The event intent. Always the *last* entry of `issue.history` —
        /// one applied decision produces exactly one event, and there is no
        /// second copy of it anywhere.
        event: HistoryEvent,
        /// The typed relation a duplicate decision creates. `None` for the
        /// other three: only a duplicate adds an edge, and it adds no file
        /// change at all (C-006).
        relation: Option<Relationship>,
    },
    /// Already satisfied: no mutation, no event, no write (contract
    /// §Idempotence and append-only behavior).
    NoOp,
}

impl DecisionOutcome {
    /// Whether this outcome changed nothing.
    #[must_use]
    pub fn is_noop(&self) -> bool {
        matches!(self, DecisionOutcome::NoOp)
    }
}

/// The decision service: one validated inbox configuration plus the caller's
/// duplicate-target resolution.
///
/// Built once per command and reused across a batch, so a selection of fifty
/// issues is validated against one scope rather than fifty.
pub struct DecisionService<'a> {
    /// The config `validate_transition` needs; also the source of
    /// `[triage].accepted_status` and `[triage].declined_status`.
    config: &'a Config,
    /// The inbox: membership, and the board's category resolution.
    scope: InboxScope,
    /// The optional accept target from `[triage]`.
    accepted_status: Option<String>,
    /// The required decline target from `[triage]`.
    declined_status: String,
    /// How the caller resolved `--of`.
    target: DuplicateTarget<'a>,
}

impl<'a> DecisionService<'a> {
    /// Build a service over an already-resolved inbox scope.
    ///
    /// `scope` comes from the caller rather than from `config` because the
    /// caller needs the same `Option<InboxScope>` answer for every `tix triage`
    /// subcommand — "does this project have an inbox at all?" is the first thing
    /// `list` asks too, and resolving it twice would give two chances to
    /// disagree.
    ///
    /// The one thing checked here is the decline column, and it is checked once
    /// rather than on every decision: a service that cannot name the column a
    /// decline would move to has no business accepting a decline, and failing at
    /// construction means no issue in a batch is decided before the defect shows.
    pub fn new(
        config: &'a Config,
        scope: InboxScope,
        target: DuplicateTarget<'a>,
    ) -> DecisionResult<Self> {
        let declined_status = config
            .triage
            .as_ref()
            .and_then(|triage| triage.declined_status.clone())
            .ok_or_else(|| {
                Refusal::new(
                    "unknown_column",
                    "`[triage].declined_status` names no column, so a decline \
                     would have nowhere to go",
                )
            })?;
        let accepted_status = config
            .triage
            .as_ref()
            .and_then(|t| t.accepted_status.clone());
        Ok(Self {
            config,
            scope,
            accepted_status,
            declined_status,
            target,
        })
    }

    /// Apply one decision to one issue.
    ///
    /// Returns the issue *as it would be written*, plus the event intent — or a
    /// refusal, or [`DecisionOutcome::NoOp`]. The input is borrowed, so a
    /// rejected decision provably cannot have modified it, and the caller decides
    /// whether and when to write (contract §Diagnostics: the service produces a
    /// refusal, the surface reports it).
    pub fn apply(
        &self,
        issue: &Issue,
        request: &TriageDecision,
    ) -> DecisionResult<DecisionOutcome> {
        // 1 — required fields, before the issue is looked at: a missing reason
        // is refused the same way whatever the queue happens to contain.
        check_required(issue, request)?;

        // 2 — the target, resolved and validated against the issue's identity.
        let plan = self.plan(issue, request)?;

        // 3 — idempotence, before membership: declining twice is a no-op even
        // though the first decline left the inbox.
        if is_noop(issue, request, &plan) {
            return Ok(DecisionOutcome::NoOp);
        }

        // 4 — still in the queue?
        if !self.scope.matches(issue) {
            return Err(if history::has_triage_decision(issue) {
                Refusal::already_decided(issue.number)
            } else {
                Refusal::not_in_triage(issue.number)
            });
        }

        let prior_state = issue.status.display().to_string();
        let mut updated = issue.clone();

        // 5 — the one transition evaluation. Nothing below re-derives a rule.
        let (result, relation) = match plan {
            Plan::Move(column) => {
                let outcome = workflow::validate_transition(
                    issue,
                    &column,
                    &self.scope.resolution,
                    self.config,
                    None,
                );
                let to = match outcome {
                    TransitionOutcome::Accepted { to, .. } => to,
                    TransitionOutcome::Rejected(diagnostic) => {
                        return Err(Refusal::from_workflow(&diagnostic));
                    }
                };
                updated.status = Status::new(&to);
                (to, None)
            }
            Plan::Duplicate {
                target_number,
                target_title,
            } => {
                // The relation, not a file operation: the intent is returned and
                // storage writes it. No merge, no delete, no move (C-006).
                let relation = Relationship::new(
                    issue.number,
                    target_number,
                    RelationKind::Duplicates,
                    &target_title,
                );
                let result = format!("{} #{}", relation.kind, relation.to);
                (result, Some(relation))
            }
        };

        // Entry metadata (data-model §2). A defer re-enters, so the wait
        // restarts; a decision that *takes the issue out of the queue* leaves no
        // stamp behind, because a stamp that measures a queue the issue is no
        // longer in would age it wrongly the moment it came back — and the
        // honest answer to "when did it enter?" is then "unknown", not a date
        // that measures something else (research R4). The test is against the
        // updated issue, so a decline that is configured to land in an inbox
        // column keeps its stamp: it never left.
        match request.kind {
            DecisionKind::Defer => {
                record_entry(&mut updated, &request.timestamp, Some("defer"));
            }
            DecisionKind::Accept | DecisionKind::Decline if !self.scope.matches(&updated) => {
                updated.entered_at = None;
                updated.source = None;
            }
            _ => {}
        }

        let record = TriageDecisionRecord::new(
            &request.timestamp,
            &request.actor,
            request.kind.event_kind(),
            prior_state,
            result,
            Some(request.reason.clone()),
            request.target_issue,
            crate::triage::entry_source(&updated).map(str::to_string),
        );
        let event = record.to_event();
        updated.history.append(event.clone());

        Ok(DecisionOutcome::Applied {
            issue: updated,
            event,
            relation,
        })
    }

    /// Resolve what this decision would change, and refuse it if it cannot
    /// (step 2 of the order).
    fn plan(&self, issue: &Issue, request: &TriageDecision) -> DecisionResult<Plan> {
        match request.kind {
            DecisionKind::Accept => {
                // `--to` wins when given; `[triage].accepted_status` is the
                // configured fallback; neither is a refusal rather than a guess.
                // A blank `--to` is treated as absent, because a flag that
                // carried only whitespace did not name a column.
                let named = request
                    .target_status
                    .as_deref()
                    .map(str::trim)
                    .filter(|target| !target.is_empty())
                    .map(str::to_string);
                named
                    .or_else(|| self.accepted_status.clone())
                    .map(Plan::Move)
                    .ok_or_else(|| Refusal::missing_target(issue.number))
            }
            DecisionKind::Decline => Ok(Plan::Move(self.declined_status.clone())),
            DecisionKind::Defer => Ok(Plan::Move(self.scope.deferred_status.clone())),
            DecisionKind::Duplicate => {
                let target = self.resolve_duplicate(issue, request)?;
                Ok(Plan::Duplicate {
                    target_number: target.number,
                    target_title: target.title.clone(),
                })
            }
        }
    }

    /// Validate `--of` and return the issue it names (T014).
    ///
    /// The self-link test goes through [`CanonicalEdge`] rather than comparing
    /// two numbers, so "is this a self link?" is the same question the
    /// relationship service asks of an authored edge and has one answer. The
    /// target itself is only ever *read*: this service creates a relation
    /// intent, never a file operation.
    fn resolve_duplicate(
        &self,
        issue: &Issue,
        request: &TriageDecision,
    ) -> DecisionResult<&'a Issue> {
        // Ambiguity first: it is the only refusal about the *selector* rather
        // than the target, and reporting "you gave no target" to someone who
        // supplied one that matched three issues sends them looking for a flag
        // they already used.
        if let DuplicateTarget::Ambiguous = self.target {
            return Err(Refusal::ambiguous());
        }
        let asked = request.target_issue.ok_or_else(|| {
            Refusal::invalid_duplicate_target(format!(
                "`#{}` is a duplicate decision with no target (`--of`)",
                issue.number
            ))
        })?;
        if CanonicalEdge::new(issue.number, asked, RelationKind::Duplicates).is_self_link() {
            return Err(Refusal::invalid_duplicate_target(format!(
                "`#{asked}` cannot be a duplicate of itself"
            )));
        }
        match self.target {
            DuplicateTarget::Resolved(target) if target.number == asked => Ok(target),
            DuplicateTarget::Resolved(target) => Err(Refusal::unknown_issue(format!(
                "`--of` resolved to `#{}`, but `#{asked}` was asked for",
                target.number
            ))),
            DuplicateTarget::Unknown => {
                Err(Refusal::unknown_issue(format!("`#{asked}` does not exist")))
            }
            // Unreachable: `Ambiguous` returns above. Written as a real arm
            // rather than `unreachable!()` so the match stays total and a
            // future reordering cannot introduce a panic.
            DuplicateTarget::Ambiguous => Err(Refusal::ambiguous()),
        }
    }
}

/// Step 3: whether this decision is already satisfied, so nothing is changed
/// and no event is appended.
///
/// Two different questions with one answer each, because "already done" means
/// different things for a decision that moves an issue and one that records an
/// edge:
///
/// - **duplicate**: the canonical `duplicates` edge already exists. The reason
///   is deliberately ignored — an edge is an edge, and re-declaring the same
///   duplicate with a sharper justification is a comment, not a second relation.
/// - **move**: the issue is already in the target column **and** the most recent
///   decision recorded is this kind with this reason. The History half matters
///   for `defer`, where the column does not change at all: a first defer must
///   refresh the entry and append, and an identical repeat must do neither. It
///   also means a *changed* reason on an unchanged column is a new decision,
///   which is quickstart §7's rule.
///
/// The consequence worth naming: an issue moved into the target column by a
/// plain `tix move`, whose last triage decision happens to be this kind and
/// reason, reads as a no-op. The requested final state and reason both hold, so
/// nothing would change — and inventing an event to mark the difference would be
/// the log recording a decision the user did not make.
///
/// A free function for the same reason as [`check_required`]: it reads nothing
/// from the service, and the plan it is handed already carries the resolved
/// target.
fn is_noop(issue: &Issue, request: &TriageDecision, plan: &Plan) -> bool {
    match plan {
        Plan::Duplicate { target_number, .. } => {
            let wanted = CanonicalEdge::new(issue.number, *target_number, RelationKind::Duplicates);
            issue
                .relationships
                .iter()
                .any(|relation| relation.canonical() == wanted)
        }
        Plan::Move(column) => {
            if !issue.status.is(column) {
                return false;
            }
            history::last_decision(issue).is_some_and(|record| {
                record.is_same_decision(request.kind.event_kind(), Some(request.reason.as_str()))
            })
        }
    }
}

/// Step 1: the fields every decision must carry (contract §Required reason and
/// actor).
///
/// A free function rather than a method because it reads nothing from the
/// service — and a check that ignores `self` but takes `&self` says the opposite
/// about where the rule lives.
fn check_required(issue: &Issue, request: &TriageDecision) -> DecisionResult<()> {
    if request.kind.requires_reason() && request.reason.trim().is_empty() {
        return Err(Refusal::missing_reason(issue.number, request.kind));
    }
    if request.actor.trim().is_empty() {
        return Err(Refusal::missing_actor(issue.number));
    }
    // One decision is one `## History` **line**, and the grammar is line-based: a
    // newline in the reason or the actor ends the line and starts another, which the
    // next reader parses as a second event. That is a forged audit entry, written by
    // the decision the user is making:
    //
    //     $ tix triage decline 1 --reason $'no budget\n- 2026-10-01T09:00:00Z — root — triage_accepted — triage -> done'
    //     # two events in ## History; the second one is a lie
    //
    // Refused rather than stripped: a reason silently altered on the way to the
    // audit trail is the same class of problem in a quieter colour — the file would
    // no longer record what the user decided.
    //
    // Checked here, at the one boundary every surface crosses (the CLI's two
    // spellings, the TUI's modal, the MCP tools), because this is the rule about what
    // a decision *is*. The pre-existing `--reason` on `tix move`/`tix edit` shares
    // this predicate now (tix #1493); a multi-line **comment body** is deliberately
    // not guarded, because a comment lives in its own document section rather than
    // on a `## History` line.
    if request.reason.chars().any(is_line_break) {
        return Err(Refusal::new(
            "invalid_reason",
            format!(
                "`#{}` {} has a line break in its reason; a reason is written to one \
                 History line, so a line break would look like a second decision",
                issue.number, request.kind
            ),
        ));
    }
    if request.actor.chars().any(is_line_break) {
        return Err(Refusal::new(
            "invalid_actor",
            format!(
                "`#{}` has a line break in its actor; the actor is written to one History \
                 line, so a line break would look like a second decision",
                issue.number
            ),
        ));
    }
    Ok(())
}

/// A character that ends a `## History` line, or would move the cursor within one.
///
/// `\n` and `\r` are the forging vector; `\u{85}` (NEL), `\u{2028}` (line separator)
/// and `\u{2029}` (paragraph separator) are the same thing in other encodings, and a
/// bare `\t` is left alone because it cannot start a new entry.
///
/// **Shared with the other line-writing seam.** `tix move --reason` and
/// `tix edit --status … --reason` render their override reason into the same
/// line-based grammar, so they refuse through this predicate too rather than
/// carrying a second copy of the character set (tix #1493). A multi-line
/// **comment body** is deliberately not guarded: a comment is written to its own
/// document section, not to a `## History` line, so many lines there are the
/// point rather than a forgery.
pub fn is_line_break(c: char) -> bool {
    matches!(c, '\n' | '\r' | '\u{85}' | '\u{2028}' | '\u{2029}')
}