travelagent 1.11.1

use std::collections::{HashMap, HashSet, VecDeque};
use std::path::PathBuf;
use std::time::{Duration, Instant};

use ratatui::style::Color;

use crate::theme::Theme;
use crate::update::UpdateInfo;
use travelagent_core::engine::ReviewEngine;
use travelagent_core::model::{CommentType, DiffFile, DiffLine, FileStatus, LineRange, LineSide};
use travelagent_core::vcs::{CommitInfo, VcsBackend, VcsInfo};

mod agent_action;
pub mod ai_summary;
mod ai_summary_state;
mod annotations;
mod commit_select;
mod construct;
mod error_log;
mod layout;
mod live_mode_state;
mod mcp_listener_state;
pub mod mode;
mod modes;
mod navigation;
mod palette;
mod remote;
mod session;
mod tour;
mod tour_state;
mod ui_layout;
mod viewer_pane_state;

pub use agent_action::AgentActionState;
pub use ai_summary_state::AiSummaryState;
pub use error_log::ErrorLog;
pub use live_mode_state::LiveModeState;
pub use mcp_listener_state::{ListenerState, McpListenerState};

pub use mode::{AppMode, LocalState, RemoteSessionState};
pub use palette::PaletteState;
pub use tour_state::TourSessionState;
pub use ui_layout::UiLayoutState;
pub use viewer_pane_state::{ViewerPaneState, ViewerRender};

const VISIBLE_COMMIT_COUNT: usize = 10;
const COMMIT_PAGE_SIZE: usize = 10;
pub const STAGED_SELECTION_ID: &str = "__trv_staged__";
pub const UNSTAGED_SELECTION_ID: &str = "__trv_unstaged__";
pub const GAP_EXPAND_BATCH: usize = 20;

/// Count how many annotation lines a gap produces (expanders + hidden count).
fn gap_annotation_line_count(is_top_of_file: bool, remaining: usize) -> usize {
    if remaining == 0 {
        0
    } else if is_top_of_file {
        // ↑ expander, plus a HiddenLines line when remaining > batch
        if remaining > GAP_EXPAND_BATCH { 2 } else { 1 }
    } else {
        // Between hunks: ↓ + HiddenLines + ↑ when >= batch, else single ↕
        if remaining >= GAP_EXPAND_BATCH { 3 } else { 1 }
    }
}

#[derive(Debug, Clone)]
pub enum FileTreeItem {
    Directory {
        path: String,
        depth: usize,
        expanded: bool,
    },
    File {
        file_idx: usize,
        depth: usize,
    },
}

/// Identifies a gap between hunks in a file (for context expansion)
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct GapId {
    pub file_idx: usize,
    /// Index of the hunk that this gap precedes (0 = gap before first hunk)
    pub hunk_idx: usize,
}

/// Direction of gap expansion
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum ExpandDirection {
    /// ↓ Expand downward from upper boundary
    Down,
    /// ↑ Expand upward from lower boundary
    Up,
    /// ↕ Expand all remaining lines in both directions (merged expander)
    Both,
}

/// Result of checking what the cursor is on in a gap region
pub enum GapCursorHit {
    /// Cursor is on a directional expander
    Expander(GapId, ExpandDirection),
    /// Cursor is on the "N lines hidden" info line
    HiddenLines(GapId),
    /// Cursor is on already-expanded context
    ExpandedContent(GapId),
}

/// Describes what a rendered line represents - built once and used for O(1) cursor queries
#[derive(Debug, Clone)]
pub enum AnnotatedLine {
    /// Review comments section header line
    ReviewCommentsHeader,
    /// A review-level comment line (part of a multi-line comment box)
    ReviewComment { comment_idx: usize },
    /// File header line
    FileHeader { file_idx: usize },
    /// A file-level comment line (part of a multi-line comment box)
    FileComment { file_idx: usize, comment_idx: usize },
    /// Expander line showing hidden context with direction arrow
    Expander {
        gap_id: GapId,
        direction: ExpandDirection,
    },
    /// Informational line showing count of hidden lines between expanders
    HiddenLines { gap_id: GapId, count: usize },
    /// Expanded context line (muted text)
    ExpandedContext { gap_id: GapId, line_idx: usize },
    /// Hunk header (@@...@@)
    HunkHeader { file_idx: usize, hunk_idx: usize },
    /// Actual diff line with line numbers
    DiffLine {
        file_idx: usize,
        hunk_idx: usize,
        line_idx: usize,
        old_lineno: Option<u32>,
        new_lineno: Option<u32>,
    },
    /// Side-by-side paired diff line
    SideBySideLine {
        file_idx: usize,
        hunk_idx: usize,
        del_line_idx: Option<usize>,
        add_line_idx: Option<usize>,
        old_lineno: Option<u32>,
        new_lineno: Option<u32>,
    },
    /// A line comment (part of a multi-line comment box)
    LineComment {
        file_idx: usize,
        line: u32,
        side: LineSide,
        comment_idx: usize,
    },
    /// Binary or empty file indicator
    BinaryOrEmpty { file_idx: usize },
    /// Placeholder shown in place of a file's diff body when the file is
    /// collapsed (either auto-collapsed as a lockfile-like / oversized file,
    /// or explicitly collapsed by the user via the `z` keybinding).
    CollapsedFile { file_idx: usize },
    /// Header for the orphaned-comments section of a file. Renders as
    /// `── Orphaned comments (N) ──` before the normal file comments/hunks.
    /// `file_idx` is `Some(idx)` for a per-file section, or `None` for the
    /// session-level "removed files" bucket rendered above the file list.
    ///
    /// `file_idx` is currently unused by readers (rendering uses `file_path`
    /// on `OrphanedComment`), but kept on the header for future MCP/agent
    /// callers that want to resolve the section to a file by index without
    /// walking comments.
    OrphanedCommentsHeader {
        #[allow(dead_code)]
        file_idx: Option<usize>,
        count: usize,
    },
    /// One orphaned comment line (part of a multi-line orphan box). Points
    /// at the orphan by `(file_path, orphan_idx)`. `file_idx` is `Some` for
    /// orphans whose owning file is still in `diff_files`, and `None` for
    /// session-level orphans (the file disappeared entirely).
    OrphanedComment {
        #[allow(dead_code)]
        file_idx: Option<usize>,
        orphan_idx: usize,
        /// The path key in `session.files` for the `FileReview` that owns
        /// this orphan. Kept here because `file_idx` can be `None` for
        /// session-level orphans whose file is no longer in `diff_files`.
        file_path: std::path::PathBuf,
    },
    /// Spacing between files
    Spacing,
}

/// Result of searching for a source line number in annotations.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum FindSourceLineResult {
    /// Exact match found at the given annotation index.
    Exact(usize),
    /// No exact match; nearest line found at the given annotation index.
    Nearest(usize),
    /// No matching lines found in the current file at all.
    NotFound,
}

/// Search `line_annotations` for the annotation whose `new_lineno` best matches
/// `target_lineno` within the file identified by `current_file`.
pub fn find_source_line(
    annotations: &[AnnotatedLine],
    current_file: usize,
    target_lineno: u32,
) -> FindSourceLineResult {
    let mut best: Option<(usize, u32)> = None; // (index, distance)

    for (idx, annotation) in annotations.iter().enumerate() {
        let (file_idx, new_lineno) = match annotation {
            AnnotatedLine::DiffLine {
                file_idx,
                new_lineno,
                ..
            } => (*file_idx, *new_lineno),
            AnnotatedLine::SideBySideLine {
                file_idx,
                new_lineno,
                ..
            } => (*file_idx, *new_lineno),
            _ => continue,
        };
        if file_idx != current_file {
            continue;
        }
        if let Some(ln) = new_lineno {
            let dist = ln.abs_diff(target_lineno);
            if dist == 0 {
                return FindSourceLineResult::Exact(idx);
            }
            if best.is_none_or(|(_, b)| dist < b) {
                best = Some((idx, dist));
            }
        }
    }

    match best {
        Some((idx, _)) => FindSourceLineResult::Nearest(idx),
        None => FindSourceLineResult::NotFound,
    }
}

/// Resolve the diff annotation under the cursor to a `(repo-relative path,
/// source line)` pair suitable for opening in an external editor.
///
/// Prefers the `New` side (working-tree line); falls back to the `Old`
/// side only when no `New` lineno exists (e.g. a deleted line in SBS
/// view). Returns `None` when the cursor is not on a diff line (file
/// header, comment row, hunk header, gap expander, spacing, etc.), so
/// callers can surface a helpful message instead of silently doing
/// nothing.
///
/// The `diff_files` slice comes from `App::diff_files`; the path is taken
/// from `DiffFile::display_path()` (new_path, falling back to old_path)
/// so renames open the current filename on disk.
pub fn resolve_cursor_to_path_line(
    annotations: &[AnnotatedLine],
    cursor_line: usize,
    diff_files: &[travelagent_core::model::DiffFile],
) -> Option<(std::path::PathBuf, u32)> {
    let (file_idx, old_lineno, new_lineno) = match annotations.get(cursor_line)? {
        AnnotatedLine::DiffLine {
            file_idx,
            old_lineno,
            new_lineno,
            ..
        }
        | AnnotatedLine::SideBySideLine {
            file_idx,
            old_lineno,
            new_lineno,
            ..
        } => (*file_idx, *old_lineno, *new_lineno),
        _ => return None,
    };
    let line = new_lineno.or(old_lineno)?;
    let path = diff_files.get(file_idx)?.display_path()?.clone();
    Some((path, line))
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum InputMode {
    Normal,
    Comment,
    Command,
    Search,
    Help,
    Confirm,
    CommitSelect,
    VisualSelect,
    ReviewSubmit,
    CommandPalette,
    ReactionPicker,
    /// Picker popup shown while the user is composing a comment. Lists
    /// reusable templates defined in `[comment_templates]` in the user's
    /// config; selecting one inserts its expansion at the cursor and
    /// returns to [`InputMode::Comment`]. Opened via `Ctrl+T` in comment
    /// mode; cancelled with `Esc`.
    CommentTemplatePicker,
    /// Pre-diff reflection modal (Phase I2, Sparring Review). Four
    /// prompts — what the change *should* do, what it *shouldn't*, what
    /// could go wrong, and what assumptions underlie it — one focused at
    /// a time. Tab cycles the active field; Ctrl+S persists into
    /// `ReviewSession.mental_model`; Esc cancels without saving.
    /// Opened via the `m` chord from Normal mode.
    MentalModelEdit,
}

/// Draft state for the mental-model modal (Phase I2).
///
/// Seeded from `ReviewSession.mental_model` on modal-open and written
/// back on `Ctrl+S`. Cancelling via `Esc` discards the draft.
#[derive(Debug, Clone, Default)]
pub struct MentalModelEditState {
    /// Per-prompt text buffers. Indexed 0..=3 matching the fields on
    /// `travelagent_core::model::MentalModel` in struct order:
    /// `should_do`, `shouldnt_do`, `could_go_wrong`, `assumptions`.
    pub drafts: [String; 4],
    /// Which of the four fields has keyboard focus. Tab cycles forward,
    /// Shift+Tab cycles backward (both wrap mod 4).
    pub focused: usize,
}

/// Labels shown alongside each mental-model field. Same order as the
/// `drafts` array on [`MentalModelEditState`].
pub const MENTAL_MODEL_LABELS: [&str; 4] = [
    "What should this change do?",
    "What shouldn't it do?",
    "What could go wrong?",
    "What assumptions underlie it?",
];

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum DiffSource {
    WorkingTree,
    Staged,
    Unstaged,
    StagedAndUnstaged,
    CommitRange(Vec<String>),
    StagedUnstagedAndCommits(Vec<String>),
    Remote { pr_title: String, pr_number: u64 },
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ConfirmAction {
    CopyAndQuit,
    Merge,
    /// `:review-restart` — clear the reviewed flag on every file in the
    /// session. Destructive (loses all review progress), so gated behind
    /// this confirmation.
    RestartReview,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum FocusedPanel {
    FileList,
    Diff,
    CommitSelector,
}

/// Active panel in remote PR review mode
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum RemotePanel {
    Files,
    Description,
    Conversation,
    Commits,
    /// Phase I5: Sparring Review reconciliation panel. Only reachable
    /// while `App.spar_mode` is active — the `5` digit key is gated
    /// on that flag, and the tab bar hides the label otherwise.
    /// Scaffold today: lists captured spec comments with placeholder
    /// status rows. The real split-pane reconciliation flow
    /// (author-intent vs reviewer-spec, three-outcome resolution)
    /// lands in I5-2.
    Sparring,
}

impl RemotePanel {
    /// Map a digit key (1..=5) to a panel, gated on whether sparring is
    /// active. Used by the Normal-mode digit dispatcher in `main.rs` and
    /// unit-tested here so the gate logic stays in one place.
    ///
    /// Returns `None` for digits outside the panel range and for `5`
    /// when `spar_mode` is off (so `5` can still participate in `{N}G`
    /// line-count accumulation in non-sparring reviews).
    #[must_use]
    pub fn from_digit(digit: u8, spar_mode: bool) -> Option<RemotePanel> {
        match digit {
            1 => Some(RemotePanel::Files),
            2 => Some(RemotePanel::Description),
            3 => Some(RemotePanel::Conversation),
            4 => Some(RemotePanel::Commits),
            5 if spar_mode => Some(RemotePanel::Sparring),
            _ => None,
        }
    }
}

#[cfg(test)]
mod remote_panel_tests {
    use super::RemotePanel;

    #[test]
    fn digits_one_to_four_map_regardless_of_spar_mode() {
        for spar in [false, true] {
            assert_eq!(RemotePanel::from_digit(1, spar), Some(RemotePanel::Files));
            assert_eq!(
                RemotePanel::from_digit(2, spar),
                Some(RemotePanel::Description)
            );
            assert_eq!(
                RemotePanel::from_digit(3, spar),
                Some(RemotePanel::Conversation)
            );
            assert_eq!(RemotePanel::from_digit(4, spar), Some(RemotePanel::Commits));
        }
    }

    #[test]
    fn digit_five_requires_spar_mode() {
        assert_eq!(RemotePanel::from_digit(5, false), None);
        assert_eq!(
            RemotePanel::from_digit(5, true),
            Some(RemotePanel::Sparring)
        );
    }

    #[test]
    fn digits_out_of_range_return_none() {
        assert_eq!(RemotePanel::from_digit(0, true), None);
        assert_eq!(RemotePanel::from_digit(6, true), None);
        assert_eq!(RemotePanel::from_digit(9, true), None);
    }
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum DiffViewMode {
    Unified,
    SideBySide,
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum MessageType {
    Info,
    Warning,
    Error,
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Message {
    pub content: String,
    pub message_type: MessageType,
}

/// How long an agent-originated flash breadcrumb stays on the status bar
/// before expiring. Tuned to be long enough for a human to read "who just
/// moved my cursor" but short enough that it doesn't linger over the next
/// real status message.
pub const FLASH_MESSAGE_TTL: Duration = Duration::from_secs(2);

/// Upper bound on the `notify_queue` backlog. Sized to absorb a full
/// `git pull` worth of `file_changed` events with margin, while still
/// preventing unbounded growth when no MCP agent is attached to drain.
pub const MCP_NOTIFY_QUEUE_CAP: usize = 256;

/// Transient status-bar message shown when an MCP agent mutates viewport
/// state. Expires automatically — the status-bar renderer skips it once
/// `expires_at <= Instant::now()`.
#[derive(Debug, Clone)]
pub struct AgentFlash {
    pub text: String,
    pub expires_at: Instant,
}

/// Where the agent is currently pointing the "sidecar" cursor when the
/// human's viewport is pinned. Set by `trv_select_file` while `viewport_pinned`
/// is true (so the human's cursor stays put), and persists until the human
/// either toggles pin off or presses `Ctrl+G` to jump to the ghost.
#[derive(Debug, Clone)]
pub struct AgentGhost {
    /// Index into `diff_files`. Always valid when set; `record_agent_ghost`
    /// rejects out-of-range indices so we never render a stale path.
    pub file_idx: usize,
    /// Display path (lossy) of the ghosted file. Cached here so the status
    /// bar renderer doesn't have to walk `diff_files` every frame.
    pub path: String,
}

/// Lifecycle of an agent-proposed forge write awaiting human confirmation.
///
/// The bridge tool `trv_propose_forge_submit_review` creates a `Pending`
/// entry; the human answers `y`/`Enter` or `n`/`Esc` in the TUI modal to
/// transition to `Executing` / `Rejected`. Approved actions run on a
/// background tokio task through `Executing` into either `Succeeded` or
/// `Failed`; the agent polls `trv_get_confirmation_status(id)` (and/or
/// subscribes to `agent_action_decided` notifications) to observe the
/// terminal state.
///
/// There is no distinct `Approved` state: post-PR-#132 the approval path
/// transitions `Pending → Executing` directly (the old `Approved`
/// intermediate was a dead latch — observers serialize on the TUI thread
/// and never saw it).
#[derive(Debug, Clone)]
pub enum ConfirmationStatus {
    Pending,
    Rejected {
        reason: RejectReason,
    },
    /// Forge call in flight after approval. Observers polling during the
    /// spawned task see this state until the oneshot delivers the final
    /// `Succeeded` / `Failed` result.
    Executing,
    Succeeded {
        result_json: String,
    },
    Failed {
        error: String,
    },
}

/// Why a proposed forge action transitioned to `Rejected`.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum RejectReason {
    /// Human pressed `n` / `Esc` in the confirmation modal.
    User,
    /// `CONFIRMATION_TIMEOUT` elapsed while still `Pending`.
    Timeout,
    /// Agent called `trv_cancel_confirmation` to revoke the proposal.
    AgentCancelled,
    /// A second `trv_propose_*` arrived while another was still `Pending`;
    /// the new proposal is rejected with this reason without ever being
    /// stashed on the app.
    AlreadyPending,
}

impl RejectReason {
    /// Stable wire name for MCP notifications and status replies.
    pub fn as_str(self) -> &'static str {
        match self {
            RejectReason::User => "user",
            RejectReason::Timeout => "timeout",
            RejectReason::AgentCancelled => "agent_cancelled",
            RejectReason::AlreadyPending => "already_pending",
        }
    }
}

/// The agent-proposed action being proposed. Covers both remote
/// forge writes (`SubmitReview`) and local session mutations
/// (`SetMentalModel`) that need human confirmation. Kept as one
/// enum so the confirmation machinery (modal, timeout, archive,
/// notifications) stays unified rather than duplicated per kind.
///
/// **v1.4 ecosystem rename:** `ForgeActionKind` → `AgentActionKind`
/// (since `SetMentalModel` bypasses the forge entirely). Internal
/// Rust symbols were renamed to match (`agent_action.pending()`,
/// `agent_action.archive_with_decision(...)`, `agent_action.last_decision()`,
/// `McpNotify::AgentAction{Proposed,Decided}`). On the wire the
/// canonical notification names are `agent_action_{proposed,decided}`;
/// the legacy `forge_action_*` parallel emissions that shipped during
/// v1.4.x were removed in v1.5.0.
#[derive(Debug, Clone)]
pub enum AgentActionKind {
    SubmitReview {
        /// Already mapped to a valid `ReviewVerdict` at propose time.
        verdict: travelagent_core::forge::ReviewVerdict,
        body: String,
    },
    /// Phase I2: agent proposed overwriting the human's mental model.
    /// Unlike `SubmitReview`, this action commits synchronously (local
    /// session state — no network, no background task) and transitions
    /// `Pending → Succeeded` in one approval tick.
    SetMentalModel {
        mental_model: travelagent_core::model::MentalModel,
    },
    /// Phase I4c-2: agent proposed landing a generated test file on the
    /// sparring branch. `test_path` is resolved under
    /// `vcs_info.root_path`; path escapes (absolute, `..`) are refused
    /// at propose time. Writes to disk on approval (no VCS commit — the
    /// human authors the commit). Requires `spar_mode == true` — we
    /// don't silently drop generated tests onto the human's main working
    /// tree.
    AcceptGeneratedTest {
        /// Repo-relative, forward-slash path (e.g. `crates/foo/tests/bar.rs`).
        test_path: String,
        /// Full file content the agent generated client-side via
        /// `trv_write_test_from_spec`.
        test_body: String,
        /// The spec comment id the test addresses. Stored alongside the
        /// action for audit + modal rendering; no enforcement that it
        /// still exists — the human makes the call at approval time.
        spec_id: String,
    },
}

impl AgentActionKind {
    /// Stable wire name for MCP notifications (`kind` field).
    pub fn wire_name(&self) -> &'static str {
        match self {
            AgentActionKind::SubmitReview { .. } => "submit_review",
            AgentActionKind::SetMentalModel { .. } => "set_mental_model",
            AgentActionKind::AcceptGeneratedTest { .. } => "accept_generated_test",
        }
    }
}

/// One pending (or recently decided) forge write proposed by an agent.
///
/// Ephemeral — the app keeps at most one `Pending` at a time on
/// `App.agent_action.pending` so the human can only ever be asked about
/// one thing, and a ring of recently-decided entries keyed by id so
/// `trv_get_confirmation_status` keeps answering after the modal closes.
#[derive(Debug, Clone)]
pub struct PendingAgentAction {
    /// UUID-v4. Sent back to the agent as `confirmation_id`; the agent
    /// uses it to poll status and to cancel.
    pub id: String,
    pub kind: AgentActionKind,
    pub status: ConfirmationStatus,
    /// Wall-clock time the propose tool created this entry. Wire-visible
    /// via `trv_get_confirmation_status`.
    pub proposed_at: chrono::DateTime<chrono::Utc>,
    /// Monotonic counterpart to `proposed_at`, used exclusively by the
    /// timeout check so NTP skew / suspend-resume can't indefinitely
    /// delay (or prematurely fire) the `CONFIRMATION_TIMEOUT` expiry.
    /// Not serialized; internal bookkeeping only.
    pub proposed_at_monotonic: Instant,
    pub decided_at: Option<chrono::DateTime<chrono::Utc>>,
}

/// Outcome of a spawned forge `submit_review` call, delivered over a
/// oneshot channel from the background tokio task back to the TUI thread.
/// The TUI thread uses this to drive `Executing → Succeeded / Failed`
/// without blocking the event loop (WARNING 1).
#[derive(Debug)]
pub enum ForgeSubmitResult {
    Ok,
    Err(String),
}

/// Most-recently-decided agent-action confirmation summary, exposed via
/// `review://status.agent_action.last_decision` so resource subscribers
/// can reconstruct the last event without racing against the decided-id
/// they didn't have when `agent_action_proposed` fired.
/// Populated by every `approve_pending_agent_action` / rejection /
/// timeout / cancellation path that drives a terminal transition.
#[derive(Debug, Clone)]
pub struct LastAgentDecision {
    pub id: String,
    /// `"approved"` (succeeded or failed at forge level — see the archived
    /// action's status for that distinction), `"rejected"`, `"succeeded"`,
    /// or `"failed"`. Matches the `decision` wire token on
    /// `agent_action_decided`.
    pub decision: &'static str,
    /// Set for rejections (`"user"` / `"timeout"` / `"agent_cancelled"` /
    /// `"already_pending"`). `None` for approvals / succeeded / failed.
    pub reason: Option<&'static str>,
    pub decided_at: chrono::DateTime<chrono::Utc>,
}

/// How long a `Pending` confirmation lives before timing out. Tuned for
/// the human-in-the-loop review workflow: long enough that a reviewer can
/// read the proposal and scan the diff, short enough that a forgotten
/// modal doesn't block the agent forever.
pub const CONFIRMATION_TIMEOUT: Duration = Duration::from_secs(300);

/// Cap on the recently-decided confirmations ring. `trv_get_confirmation_status`
/// consults this so an agent that polls after a decision still gets the
/// terminal status; drop-oldest once the ring is full.
pub const CONFIRMATION_HISTORY_CAP: usize = 16;

/// Push-notification event the TUI thread wants to raise to an attached MCP
/// agent. Drained each tick by the main event loop into the bridge's notify
/// sink, which forwards as an rmcp `CustomNotification` with the method
/// `notifications/trv/<snake_case_variant>`.
///
/// Variants are intentionally small and JSON-friendly; the bridge serializes
/// them verbatim into the notification params. If this enum grows into
/// something heavier, wrap `notify_queue` in bounded semantics to match the
/// live-watcher channels — for now the volume is too low to matter.
#[derive(Debug, Clone)]
pub enum McpNotify {
    /// Live-mode watcher picked up one or more path changes and a rescan
    /// completed. `files` is the display-path list, basename-preserved. An
    /// empty vec means "we don't know which files" (fallback when the
    /// watcher surfaced a directory-level event).
    FileChanged { files: Vec<String> },
    /// A comment was added to the session. `author` is `"human"` or
    /// `"agent"` so the receiving agent can filter out its own echoes
    /// without body-sniffing the MCP marker.
    CommentAdded {
        file: String,
        line: Option<u32>,
        author: &'static str,
    },
    /// A review submission completed — either a forge submit (with a
    /// verdict) or an export (no verdict). Includes RFC3339 timestamp so
    /// the agent can correlate with its own action log.
    ReviewSubmitted { verdict: Option<String>, at: String },
    /// An agent proposed a forge write and the TUI has stashed it as a
    /// pending confirmation. Subscribed agents can notice so a second
    /// actor knows a proposal is already in flight.
    AgentActionProposed { id: String, kind: &'static str },
    /// The pending confirmation transitioned to a terminal state. `decision`
    /// is `"approved"` or `"rejected"`; `reason` is set for rejections
    /// (`"user"`/`"timeout"`/`"agent_cancelled"`/`"already_pending"`).
    AgentActionDecided {
        id: String,
        decision: &'static str,
        reason: Option<&'static str>,
    },
    /// The TUI is shutting down its MCP socket listener (`:mcp-off`). A
    /// server-initiated soft-drain signal: peers have `deadline_ms` to finish
    /// in-flight tool calls before the socket closes. Purely an event, NOT a
    /// resource update — `resource_uris_for` returns `&[]` so no
    /// `resources/updated` is fanned alongside it.
    Hangup { deadline_ms: u64, reason: String },
    /// The human initiated an agent-driven tour from the commit picker
    /// (`:tour`). `commit_ids` is the scope: the toggled-commit selection
    /// when non-empty, otherwise the full picker revset. Purely an event,
    /// not a resource mutation — `resource_uris_for` returns `&[]`.
    TourRequest { commit_ids: Vec<String> },
}

impl McpNotify {
    /// Notification method suffix (after the `notifications/trv/` prefix).
    /// Lives on the enum so the stable wire name is a property of the
    /// variant, not a bridge-side lookup table that could drift.
    pub fn method_suffix(&self) -> &'static str {
        match self {
            Self::FileChanged { .. } => "file_changed",
            Self::CommentAdded { .. } => "comment_added",
            Self::ReviewSubmitted { .. } => "review_submitted",
            Self::AgentActionProposed { .. } => "agent_action_proposed",
            Self::AgentActionDecided { .. } => "agent_action_decided",
            Self::Hangup { .. } => "hangup",
            Self::TourRequest { .. } => "tour_request",
        }
    }
}

/// Grouped state for the comment editing workflow.
pub struct CommentEditState {
    pub buffer: String,
    pub cursor: usize,
    pub comment_type: CommentType,
    pub types: Vec<CommentTypeDefinition>,
    pub is_review_level: bool,
    pub is_file_level: bool,
    pub line: Option<(u32, LineSide)>,
    pub editing_id: Option<String>,
    /// Visual selection anchor point (starting line, side)
    pub visual_anchor: Option<(u32, LineSide)>,
    /// Line range for range comments (used when creating comments from visual selection)
    pub line_range: Option<(LineRange, LineSide)>,
    /// Calculated screen position for comment input cursor (col, row) for IME positioning.
    /// Set during render when in Comment mode, None otherwise.
    pub cursor_screen_pos: Option<(u16, u16)>,
}

/// Grouped state for the commit selection workflow.
pub struct CommitSelectionState {
    pub list: Vec<CommitInfo>,
    pub cursor: usize,
    pub scroll_offset: usize,
    pub viewport_height: usize,
    /// Selected commit range as (start_idx, end_idx) inclusive, where start <= end.
    /// Indices refer to positions in list.
    pub selection_range: Option<(usize, usize)>,
    /// State describing how many commits are currently shown and how pagination behaves.
    pub visible_count: usize,
    pub page_size: usize,
    pub has_more: bool,
}

/// Grouped state for gap context expansion (expanded lines between hunks).
pub struct GapExpansionState {
    /// Stores lines expanded downward from the upper boundary of each gap
    pub expanded_top: HashMap<GapId, Vec<DiffLine>>,
    /// Stores lines expanded upward from the lower boundary of each gap (in ascending line order)
    pub expanded_bottom: HashMap<GapId, Vec<DiffLine>>,
}

// Tour types live in `travelagent_core::model`; call sites import them
// directly from there. The old `pub use` re-exports here were retained
// during the H2 AppMode split for backwards-compatibility and were
// finally dropped in Phase B once the TUI call sites converged.

/// Directional hint the human sends to the agent via `=` / `-` keys.
/// Consumed once by the agent through `trv_tour_take_granularity_hint`.
/// Ephemeral — not persisted in the session.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum GranularityHint {
    /// User wants larger stops — batch more commits together.
    Coarser,
    /// User wants smaller stops — split batched stops into individual commits.
    Finer,
}

impl GranularityHint {
    pub fn id(&self) -> &'static str {
        match self {
            Self::Coarser => "coarser",
            Self::Finer => "finer",
        }
    }
}

#[cfg(test)]
mod resolve_cursor_to_path_line_tests {
    //! Unit tests for `resolve_cursor_to_path_line` — the pure helper that
    //! turns a cursor position + annotation vector into a `(path, line)`
    //! pair for the `gf` editor-jump feature.
    use super::*;
    use std::path::PathBuf;
    use travelagent_core::model::{DiffFile, FileStatus};

    fn make_file(new_path: &str) -> DiffFile {
        DiffFile {
            old_path: None,
            new_path: Some(PathBuf::from(new_path)),
            status: FileStatus::Modified,
            hunks: vec![],
            is_binary: false,
            is_too_large: false,
            is_commit_message: false,
        }
    }

    #[test]
    fn cursor_resolve_to_path_line_unified() {
        // Unified-mode DiffLine with both old+new linenos: prefer the New
        // side (working-tree line) per the editor-jump contract.
        let files = vec![make_file("src/foo.rs")];
        let annotations = vec![
            AnnotatedLine::FileHeader { file_idx: 0 },
            AnnotatedLine::HunkHeader {
                file_idx: 0,
                hunk_idx: 0,
            },
            AnnotatedLine::DiffLine {
                file_idx: 0,
                hunk_idx: 0,
                line_idx: 0,
                old_lineno: Some(10),
                new_lineno: Some(12),
            },
        ];

        let resolved = resolve_cursor_to_path_line(&annotations, 2, &files);
        assert_eq!(resolved, Some((PathBuf::from("src/foo.rs"), 12)));
    }

    #[test]
    fn cursor_resolve_to_path_line_sbs() {
        // Side-by-side mode: the New side resolves to new_lineno, and a
        // deletion-only row (no new_lineno) falls back to old_lineno.
        let files = vec![make_file("lib/bar.rs")];
        let annotations = vec![
            AnnotatedLine::FileHeader { file_idx: 0 },
            // Added/context row with both linenos — pick New.
            AnnotatedLine::SideBySideLine {
                file_idx: 0,
                hunk_idx: 0,
                del_line_idx: None,
                add_line_idx: Some(0),
                old_lineno: Some(7),
                new_lineno: Some(8),
            },
            // Deletion-only row — new is None, so fall back to Old.
            AnnotatedLine::SideBySideLine {
                file_idx: 0,
                hunk_idx: 0,
                del_line_idx: Some(1),
                add_line_idx: None,
                old_lineno: Some(9),
                new_lineno: None,
            },
        ];

        let new_side = resolve_cursor_to_path_line(&annotations, 1, &files);
        assert_eq!(new_side, Some((PathBuf::from("lib/bar.rs"), 8)));

        let old_side = resolve_cursor_to_path_line(&annotations, 2, &files);
        assert_eq!(old_side, Some((PathBuf::from("lib/bar.rs"), 9)));
    }

    #[test]
    fn cursor_resolve_to_path_line_on_header_returns_none() {
        // Cursor on anything that isn't a diff row — file header, hunk
        // header, spacing, or the review-comments header — yields None so
        // the handler can show a helpful message instead of blindly
        // opening nothing.
        let files = vec![make_file("src/foo.rs")];
        let annotations = vec![
            AnnotatedLine::ReviewCommentsHeader,
            AnnotatedLine::FileHeader { file_idx: 0 },
            AnnotatedLine::HunkHeader {
                file_idx: 0,
                hunk_idx: 0,
            },
            AnnotatedLine::Spacing,
        ];

        for idx in 0..annotations.len() {
            assert_eq!(
                resolve_cursor_to_path_line(&annotations, idx, &files),
                None,
                "annotation index {idx} should resolve to None",
            );
        }

        // Out-of-bounds cursor is also None.
        assert_eq!(resolve_cursor_to_path_line(&annotations, 999, &files), None,);
    }
}

#[cfg(test)]
mod agent_flash_tests {
    use super::*;

    #[test]
    fn set_flash_message_stores_text_and_expiry() {
        // Minimal struct-only check: we can't build a full App here without a
        // VCS, so exercise the AgentFlash carrier directly to confirm the
        // "fresh flash is visible, expired flash is hidden" semantics that
        // `current_flash_text` relies on.
        let fresh = AgentFlash {
            text: "\u{1f916} agent: jumped to foo.rs".to_string(),
            expires_at: Instant::now() + Duration::from_secs(10),
        };
        assert!(fresh.expires_at > Instant::now());
        assert!(fresh.text.contains("jumped to foo.rs"));
    }

    #[test]
    fn expired_flash_is_in_the_past() {
        // An already-expired flash (expires_at < now) is what
        // `current_flash_text` treats as "no active flash".
        let expired = AgentFlash {
            text: "old".to_string(),
            expires_at: Instant::now() - Duration::from_millis(1),
        };
        assert!(expired.expires_at <= Instant::now());
    }
}

#[cfg(test)]
mod granularity_hint_tests {
    use super::*;

    #[test]
    fn granularity_hint_has_stable_ids() {
        assert_eq!(GranularityHint::Coarser.id(), "coarser");
        assert_eq!(GranularityHint::Finer.id(), "finer");
    }
}

/// Grouped state for the inline commit selector panel (multi-commit reviews).
pub struct InlineCommitSelectorState {
    /// CommitInfo for commits in the current review (display order: newest first)
    pub commits: Vec<CommitInfo>,
    /// Whether the inline commit selector panel is visible
    pub visible: bool,
    /// Cached individual/subrange diffs keyed by (start_idx, end_idx) into commits
    pub diff_cache: HashMap<(usize, usize), Vec<DiffFile>>,
    /// The combined "all selected" diff, cached for quick restoration
    pub range_diff_files: Option<Vec<DiffFile>>,
    /// Saved inline selection range when entering full commit select mode via :commits
    pub saved_selection: Option<(usize, usize)>,
}

/// Navigation + input-surface state, extracted from `App` in task
/// #51 PR A. Fields stay `pub` — the encapsulation pass that adds
/// mutation methods + invariants is a separate PR. The grouping
/// makes "where is the user and what are they looking at" legible
/// without surfing past 90-odd unrelated App fields.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct NavigationState {
    /// Which input mode the TUI is in (Normal, Comment, Command, …).
    /// Every handler dispatch gates on this; `set_input_mode` would
    /// live here in a future encapsulation pass.
    pub input_mode: InputMode,
    /// Which panel owns keyboard focus within Normal mode.
    pub focused_panel: FocusedPanel,
    /// Unified vs side-by-side diff rendering — toggled by `:diff`.
    pub diff_view_mode: DiffViewMode,
    /// The mode command mode was entered from. Command mode overwrites
    /// `input_mode` with `Command`, losing the originating context; this
    /// remembers it so mode-gated commands (e.g. `:tour`, valid only from
    /// the full-screen commit picker) can check where they were launched.
    /// Set by `enter_command_mode`; defaults to `Normal`.
    pub command_origin: InputMode,
}

impl Default for NavigationState {
    fn default() -> Self {
        Self {
            input_mode: InputMode::Normal,
            focused_panel: FocusedPanel::Diff,
            diff_view_mode: DiffViewMode::Unified,
            command_origin: InputMode::Normal,
        }
    }
}

pub struct App {
    pub theme: Theme,
    pub vcs: Box<dyn VcsBackend>,
    pub vcs_info: VcsInfo,
    pub engine: ReviewEngine,
    pub diff_files: Vec<DiffFile>,
    pub diff_source: DiffSource,

    /// Navigation + input-surface state. Phase H extraction (task #51
    /// PR A). Regroups the three fields that answer "where is the user
    /// right now and how are they looking at the diff": input mode,
    /// which panel owns keyboard focus, and whether the diff renders
    /// unified or side-by-side. Kept as a flat sub-struct with `pub`
    /// fields for now — the encapsulation pass that adds mutation
    /// methods is PR C of the same task.
    pub nav: NavigationState,

    pub file_list_state: FileListState,
    pub diff_state: DiffState,
    pub help_state: HelpState,
    /// Command / command-palette input state. Replaces the pre-v1.6
    /// pair of `command_buffer: String` (here) and
    /// `ui_layout.palette_cursor: usize` (there) — they co-moved
    /// enough that factoring into a sub-struct with invariant-
    /// preserving mutators was clearer than ~11 paired mutations
    /// across `handler::handle_command_palette_action`.
    pub palette: PaletteState,
    pub search_buffer: String,
    pub last_search_pattern: Option<String>,
    pub comment: CommentEditState,
    /// Draft state for the mental-model modal (Phase I2). Populated
    /// when the user opens the modal via `m`; reset to `Default` on
    /// cancel; consumed into `session.mental_model` on Ctrl+S save.
    pub mental_model_edit: MentalModelEditState,
    /// Per-field byte cap enforced at ingress into the modal. Read
    /// from `[mental_model].byte_limit` in the user's config (default
    /// 2048). Captured here at App construction so the modal handler
    /// doesn't need to rethread the config outcome. Named `byte_limit`
    /// to match the enforcement semantics (bytes, not Unicode scalars).
    pub mental_model_byte_limit: usize,

    /// Phase I3: blind-tests mode state. When `true`, diff files
    /// matching any pattern in `blind_patterns` are filtered out of
    /// `diff_files`. Toggled at runtime by the `:unblind` slash
    /// command; initial value comes from the `--blind-tests` CLI
    /// flag. `blind_patterns` is the snapshot of
    /// `hidden_from_reviewer` from `.travelagent/review.toml` at
    /// startup — reloading the config mid-session isn't supported
    /// yet.
    pub blind_mode: bool,
    pub blind_patterns: Vec<String>,

    /// Phase I1 (scaffold): Sparring Review mode. Initial value
    /// comes from the `--spar` CLI flag. Today this only drives the
    /// `[spar]` status-bar glyph and gates future I4/I5/I6 work;
    /// the branch-management surface lands in a follow-up once the
    /// VCS trait exposes branch creation / checkout.
    pub spar_mode: bool,

    /// Phase I5-2b: spec_id → linkage state, populated by
    /// `refresh_spec_statuses()` which calls
    /// `travelagent_core::sparring::scan_spec_links` against the
    /// working tree. Missing entries default to `Unlinked` at
    /// render time. Rescanned on sparring-panel entry and after
    /// `AgentActionKind::AcceptGeneratedTest` approval — a full
    /// `:rescan-specs` command is deferred.
    pub spec_statuses:
        std::collections::HashMap<String, travelagent_core::sparring::SparringStatus>,
    /// Phase I5-2b: row index in the Sparring panel's active spec
    /// list. Clamped to `max(0, list_len - 1)` at render time so
    /// list mutations (accept/drop) can't leave the cursor past
    /// the last row.
    pub sparring_cursor: usize,

    pub commit_select: CommitSelectionState,

    pub should_quit: bool,
    /// When true, the on-exit autosave flush is skipped — the user asked
    /// to discard unsaved work (`:q!` / `quit!` / `ZQ`). Default `false`
    /// so graceful quits (`:q`, second-Ctrl+C after the dirty warning,
    /// etc.) still persist. `:w` / `:wq` / `ZZ` clear `dirty` themselves,
    /// so this flag doesn't affect them either way.
    pub discard_on_exit: bool,
    pub dirty: bool,
    pub quit_warned: bool,
    pub confirm_on_quit: bool,
    /// Reusable comment-body templates loaded from `[comment_templates]`
    /// in the user's config. Sorted alphabetically by name for a
    /// deterministic picker order. Empty when no section is configured.
    pub comment_templates: Vec<(String, String)>,
    pub message: Option<Message>,
    /// Ring buffer of the most recent error messages (H8). `set_error`
    /// pushes here in addition to clobbering `message`, so a burst of
    /// live-mode watcher errors doesn't hide a critical error (forge
    /// auth, save failure) below them. The `:errors` command cycles
    /// through the ring back into `message`.
    pub error_log: ErrorLog,
    /// Transient breadcrumb set when an MCP agent drains a viewport-mutating
    /// command. Expires after `FLASH_MESSAGE_TTL`. Takes precedence over
    /// normal info/warning messages in the status bar but yields to errors.
    pub agent_flash: Option<AgentFlash>,
    /// Cursor-ownership mode. `false` (default) = follow mode: agent
    /// navigation (`trv_select_file`) moves the human's viewport. `true`
    /// = pinned: agent navigation records an `AgentGhost` instead of
    /// mutating `diff_state`, leaving the human's cursor where they put
    /// it. Toggled by `Ctrl+P`.
    pub viewport_pinned: bool,
    /// Where the agent is currently pointing the sidecar cursor while the
    /// viewport is pinned. Cleared by `jump_to_agent_ghost` (`Ctrl+G`) and
    /// whenever pin is toggled off. `None` outside pin mode.
    pub agent_ghost: Option<AgentGhost>,
    /// Server-push notifications queued on the TUI thread, drained each tick
    /// by `main.rs` into the MCP bridge's notify sink. Populated by
    /// `push_notify` from live-mode rescans, comment saves, and review
    /// submissions. Ignored (unread) when no MCP bridge is attached.
    pub notify_queue: VecDeque<McpNotify>,
    /// Shared warning queue populated by the forge-client `warn_handler`
    /// callback. Drained each tick by `drain_forge_warnings` into the
    /// status bar + error-log ring. `Arc<Mutex<_>>` because the callback
    /// runs on whatever tokio worker the forge client is driven from,
    /// while the drain runs on the main event-loop thread.
    pub forge_warn_queue: std::sync::Arc<std::sync::Mutex<VecDeque<String>>>,
    pub pending_confirm: Option<ConfirmAction>,
    pub supports_keyboard_enhancement: bool,
    /// Frame-layout + picker state. Phase H extraction (task #51 PR
    /// B). Groups file-list/diff-pane geometry, expanded-dir tree
    /// state, and the two floating-picker cursors into one named
    /// bundle so "what does the current frame look like" is legible
    /// without surfing past the other 80-odd App fields. Fields stay
    /// `pub` — the encapsulation pass is a later PR.
    pub ui_layout: UiLayoutState,
    pub gaps: GapExpansionState,
    /// Cached annotations describing what each rendered line represents
    pub line_annotations: Vec<AnnotatedLine>,
    /// Output to stdout instead of clipboard when exporting
    pub output_to_stdout: bool,
    /// Pending output to print to stdout after TUI exits
    pub pending_stdout_output: Option<String>,
    /// Information about available updates (set by background check)
    pub update_info: Option<UpdateInfo>,
    /// Accumulated digit count for {N}G jump-to-line
    pub pending_count: Option<usize>,

    pub inline_selector: InlineCommitSelectorState,
    /// Path filter for scoping diff to a specific file or directory
    pub path_filter: Option<String>,
    /// Whether to include the "Comment types:" legend line in export
    pub export_legend: bool,

    /// Mode-specific state: `Local(LocalState)` in local-review mode, or
    /// `Remote(RemoteSessionState)` in PR/MR review mode. Phase H2 lifted
    /// all 20 remote-only fields (forge, pr_id, pr_metadata, pr_commits,
    /// remote_comments, review_threads, remote_panel, conversation/
    /// description/commits cursors, forge_host, replying_to_thread,
    /// review_verdict_cursor, review_body, review_body_editing,
    /// last_refreshed_at, rate_limit_remaining,
    /// reaction_picker_target_thread) into `RemoteSessionState` so local
    /// builds can't accidentally read them.
    pub mode: AppMode,

    /// Shared tokio runtime handle. Always present (built once in
    /// `main.rs`) so every `block_on` / async forge call routes through
    /// the single process runtime. Phase H1 replaced the old per-App
    /// `forge_runtime: Option<Runtime>` field.
    pub runtime_handle: tokio::runtime::Handle,

    /// Automatically `git add` files when marked as reviewed (local mode only, default false)
    pub auto_stage: bool,

    /// Tour-guide session state: plan, per-comment metadata, triage
    /// verdicts, granularity hint, and the score cache. Grouped out
    /// of `App` in v1.6 — see `app/tour_state.rs` for the per-field
    /// docs and rationale. `score_cache` lives here too even though
    /// it was sitting further down in the pre-v1.6 layout; the tour
    /// lifecycle owns it and grouping it with the rest makes
    /// `invalidate_tour_score_cache` a method on one struct.
    pub tour: TourSessionState,

    /// When `true`, the main loop will suspend the terminal and invoke the
    /// external editor against `comment.buffer` after the current event cycle.
    /// Set by `handle_comment_action` when `Action::OpenExternalEditor` fires;
    /// cleared by the main loop after the editor returns.
    pub pending_external_edit: bool,

    /// When `Some((path, line))`, the main loop will suspend the terminal and
    /// open `path` in `$VISUAL`/`$EDITOR` (fallback `vi`) at the given line
    /// after the current event cycle. Set by the `gf` (Action::OpenInEditor)
    /// handler in normal mode; cleared by the main loop after the editor
    /// returns. `path` is a repo-relative file path; the main-loop launcher
    /// resolves it against the repo root.
    pub pending_open_file_editor: Option<(std::path::PathBuf, u32)>,

    /// When `true`, paired deletion/addition lines in the diff view get
    /// word-level highlights around the changed tokens. When `false`, the
    /// legacy whole-line coloring is used.
    pub word_diff_enabled: bool,

    /// When `true`, PR descriptions and comment bodies are rendered as
    /// styled markdown. When `false`, bodies are rendered as plain text,
    /// one line per source line.
    pub markdown_rendering_enabled: bool,

    /// When `true`, file + hunk headers in the diff view are tinted by their
    /// per-block risk band (green/yellow/red). When `false`, the default
    /// monochrome hues are used — opt-out for colorblind users or anyone
    /// who prefers the plain UI.
    pub risk_border_colors: bool,

    /// AI-summary panel state bundle: markdown, freshness metadata,
    /// unread flag, panel open/scroll state. Grouped out of `App`
    /// in v1.6 — see `app/ai_summary_state.rs` for per-field docs.
    pub ai: AiSummaryState,

    /// Index of the highlighted reaction in the ReactionPicker popup (0..8).
    pub reaction_picker_cursor: usize,

    /// Tab width in columns used when expanding `\t` in rendered diff content.
    /// Configurable via `:set tabstop=N` where N is clamped to `1..=16`.
    pub tab_width: usize,

    /// Risk scoring configuration (`[risk]` section of the TOML config). Used
    /// by tour-guide batching, the `:set tour=<preset>` command, and the MCP
    /// commit-risk tools. Falls back to `RiskConfig::default()` when the
    /// config file doesn't have a `[risk]` section.
    pub risk_config: travelagent_core::risk::RiskConfig,

    /// Auto-collapse configuration (`[auto_collapse]` section of the TOML
    /// config). Controls which files start collapsed on session open. Falls
    /// back to `AutoCollapseConfig::default()` (enabled, threshold=500, empty
    /// user patterns) when the config file doesn't have the section.
    pub auto_collapse_cfg: travelagent_core::auto_collapse::AutoCollapseConfig,

    /// Live-review-mode state bundle: active flag, last refresh,
    /// deferred-rescan flag, cached file contents for re-anchor,
    /// last-selected orphan. Grouped out of `App` in v1.6 — see
    /// `app/live_mode_state.rs` for per-field docs. These five move
    /// together on every live-mode lifecycle event.
    pub live: LiveModeState,

    /// Agent-action confirmation state machine: pending proposal,
    /// recent-decision ring, in-flight forge oneshot receiver, and
    /// most-recent terminal decision summary. Grouped out of `App`
    /// in v1.6 because these four fields co-move on every lifecycle
    /// event (propose → approve/reject/timeout → archive → decision
    /// notification). See `app/agent_action.rs` for field-level docs.
    pub agent_action: AgentActionState,

    /// Runtime MCP-socket-listener intent (`:mcp-on` / `:mcp-off`). Holds
    /// only the user-intent bit; the actual socket guard lives in the main
    /// event loop, same split as `live`. See `app/mcp_listener_state.rs`.
    pub mcp_listener: McpListenerState,

    /// Full-file viewer pane (`:view` / `t`): when active, the main content
    /// area shows the whole current file (raw syntax-highlighted, or rendered
    /// for markdown) instead of the diff. See `app/viewer_pane_state.rs`.
    pub viewer: ViewerPaneState,

    /// Transient set of reviewed files the user is "peeking" — their diff
    /// body is rendered in place despite being marked reviewed (toggled with
    /// `Space` on a reviewed file). Per-session-run only; never persisted, so
    /// reopening a session starts with all reviewed files folded again.
    pub peeked_reviewed: HashSet<PathBuf>,

    /// Live count of connected MCP peers, refreshed roughly once per second
    /// by the main event loop from `hub.registry` (the hub lives in `main.rs`,
    /// not on `App`). Read by the `:tour` connection gate via
    /// [`App::mcp_peer_count`]. A cheap relaxed atomic — up to ~1s staleness
    /// is fine for a UX gate. Zero when no hub is attached.
    pub mcp_peer_count: std::sync::Arc<std::sync::atomic::AtomicUsize>,

    /// Set by the `:tour` command handler with the commit-id scope of an
    /// agent-driven tour request. Drained each tick by the main event loop
    /// and sent over `hub.notify_tx` as `McpNotify::TourRequest` — the same
    /// handler→main split as `live.pending_rescan`, because the handler is
    /// sync and the hub's notify sink lives in `main.rs`.
    pub pending_tour_request: Option<Vec<String>>,

    /// Polling counterpart to [`Self::pending_tour_request`]. Set at the same
    /// time, consumed by the agent via `trv_tour_take_pending_request` (one-
    /// shot, like `trv_tour_take_granularity_hint`).
    ///
    /// Why two fields: Claude Code's MCP integration doesn't surface
    /// server-pushed `notifications/trv/*` events to the assistant, so the
    /// notification-only path silently drops `:tour` requests for that client.
    /// This field gives the agent a polling fallback. Other clients that DO
    /// surface notifications get the live event AND can ignore polling.
    pub pending_tour_request_poll: Option<Vec<String>>,
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct CommentTypeDefinition {
    pub id: String,
    pub label: String,
    pub definition: Option<String>,
    pub color: Option<Color>,
}

#[derive(Default)]
pub struct FileListState {
    pub list_state: ratatui::widgets::ListState,
    pub scroll_x: usize,
    pub viewport_width: usize,    // Set during render
    pub viewport_height: usize,   // Set during render
    pub max_content_width: usize, // Set during render
}

impl FileListState {
    pub fn selected(&self) -> usize {
        self.list_state.selected().unwrap_or(0)
    }

    pub fn select(&mut self, index: usize) {
        self.list_state.select(Some(index));
    }

    pub fn scroll_left(&mut self, cols: usize) {
        self.scroll_x = self.scroll_x.saturating_sub(cols);
    }

    pub fn scroll_right(&mut self, cols: usize) {
        let max_scroll_x = self.max_content_width.saturating_sub(self.viewport_width);
        self.scroll_x = (self.scroll_x.saturating_add(cols)).min(max_scroll_x);
    }
}

#[derive(Debug)]
pub struct DiffState {
    pub scroll_offset: usize,
    pub scroll_x: usize,
    pub cursor_line: usize,
    pub current_file_idx: usize,
    pub viewport_height: usize,
    pub viewport_width: usize,
    pub max_content_width: usize,
    pub wrap_lines: bool,
    /// Number of logical lines that fit in the viewport (set during render).
    /// When wrapping is enabled, this accounts for lines expanding to multiple visual rows.
    pub visible_line_count: usize,
}

impl Default for DiffState {
    fn default() -> Self {
        Self {
            scroll_offset: 0,
            scroll_x: 0,
            cursor_line: 0,
            current_file_idx: 0,
            viewport_height: 0,
            viewport_width: 0,
            max_content_width: 0,
            wrap_lines: true,
            visible_line_count: 0,
        }
    }
}

#[derive(Debug, Default)]
pub struct HelpState {
    pub scroll_offset: usize,
    pub viewport_height: usize,
    pub total_lines: usize, // Set during render
}

impl App {
    /// Convenience accessor: `true` when the app is in remote PR/MR mode.
    /// Short-hand for `self.mode.is_remote()`. Retained as API surface
    /// even though current callers go through `remote()` directly.
    #[allow(dead_code)]
    pub fn is_remote(&self) -> bool {
        self.mode.is_remote()
    }

    /// Convenience accessor: borrow remote-session state when in remote
    /// mode, `None` otherwise. Short-hand for `self.mode.remote()`.
    pub fn remote(&self) -> Option<&RemoteSessionState> {
        self.mode.remote()
    }

    /// Convenience accessor: mutably borrow remote-session state when in
    /// remote mode, `None` otherwise. Short-hand for `self.mode.remote_mut()`.
    pub fn remote_mut(&mut self) -> Option<&mut RemoteSessionState> {
        self.mode.remote_mut()
    }

    /// Number of MCP peers currently connected, as last refreshed by the
    /// main event loop (≤ ~1s stale). Zero when no MCP hub is attached.
    /// Used by the `:tour` connection gate.
    pub fn mcp_peer_count(&self) -> usize {
        self.mcp_peer_count
            .load(std::sync::atomic::Ordering::Relaxed)
    }

    pub fn current_file(&self) -> Option<&DiffFile> {
        self.diff_files.get(self.diff_state.current_file_idx)
    }

    pub fn current_file_path(&self) -> Option<&PathBuf> {
        self.current_file()
            .map(travelagent_core::model::DiffFile::display_path_lossy)
    }

    /// Ensure the viewer pane's content cache holds the full text of the
    /// currently-selected file, reading it from the working tree if needed.
    ///
    /// Returns `Ok(())` when the cache is populated (or already current) and an
    /// error message string when the file can't be shown (binary, deleted,
    /// remote mode, or an I/O failure). Called by the render path before
    /// drawing the viewer so the on-disk content is always fresh for the
    /// selected file; the read is skipped when the cache already matches the
    /// path. The viewer is local-only (mirrors the live-mode constraint):
    /// remote PR/MR diffs have no local working-tree file to read.
    pub fn refresh_viewer_content(&mut self) -> Result<(), String> {
        if matches!(self.diff_source, DiffSource::Remote { .. }) {
            return Err("Viewer is only available for local diffs".to_string());
        }
        let Some(file) = self.current_file() else {
            return Err("No file selected".to_string());
        };
        if file.is_binary {
            return Err("Viewer can't display a binary file".to_string());
        }
        if file.status == travelagent_core::model::FileStatus::Deleted {
            return Err("Viewer can't display a deleted file".to_string());
        }
        let rel = file.display_path_lossy().clone();
        // Already cached for this path → nothing to do (the working tree is
        // re-read only on a file switch / explicit invalidation, not every
        // frame, to avoid a syscall per render).
        if self.viewer.has_content_for(&rel) {
            return Ok(());
        }
        // File switched (or first open): a Rendered sub-mode from a previous
        // markdown file must fall back to Raw if the new file isn't renderable.
        self.viewer.coerce_render_for(&rel);
        let abs = self.vcs_info.root_path.join(&rel);
        match std::fs::read_to_string(&abs) {
            Ok(text) => {
                let lines: Vec<String> = text.lines().map(str::to_string).collect();
                self.viewer.set_content(rel, lines);
                Ok(())
            }
            Err(e) => Err(format!("Couldn't read {}: {e}", rel.display())),
        }
    }

    /// Toggle the full-file viewer pane on/off (`:view` / `t`). Validates that
    /// a viewable file is available *before* flipping on, so the toggle fails
    /// loudly with a reason rather than activating into an empty pane. When the
    /// selected file changed since the cache was last filled, the cache is
    /// invalidated so the render path re-reads it.
    pub fn toggle_viewer(&mut self) {
        if self.viewer.is_active() {
            self.viewer.deactivate();
            self.set_message("Viewer: off (showing diff)");
            return;
        }
        // Probe viewability before activating.
        if let Err(msg) = self.refresh_viewer_content() {
            self.set_warning(msg);
            return;
        }
        // A markdown→code switch while the saved sub-mode is Rendered must fall
        // back to Raw for the now-current file.
        if let Some(path) = self.current_file_path().cloned() {
            self.viewer.coerce_render_for(&path);
        }
        self.viewer.activate();
        self.set_message("Viewer: on (t to close, T for raw/rendered)");
    }

    /// Toggle the viewer's raw↔rendered sub-mode (`:raw` / `:render` / `T`).
    /// No-op with a hint when the viewer isn't active or the current file isn't
    /// a renderable format (markdown only, for now).
    pub fn toggle_viewer_render(&mut self) {
        if !self.viewer.is_active() {
            self.set_message("Viewer not active (press t to open)");
            return;
        }
        let Some(path) = self.current_file_path().cloned() else {
            self.set_message("No file selected");
            return;
        };
        match self.viewer.toggle_render(&path) {
            Some(crate::app::ViewerRender::Rendered) => self.set_message("Viewer: rendered"),
            Some(crate::app::ViewerRender::Raw) => self.set_message("Viewer: raw"),
            None => self.set_warning("Rendered view: markdown only"),
        }
    }

    /// Half the viewer viewport height (for `Ctrl+D`/`Ctrl+U` in the viewer),
    /// at least 1 so a zero-height viewport still advances.
    pub fn viewer_viewport_half(&self) -> usize {
        (self.diff_state.viewport_height / 2).max(1)
    }

    /// A full viewer viewport page (for `PageDown`/`PageUp`), at least 1. The
    /// viewer writes its own height during render, but the diff viewport height
    /// is a safe proxy here (they share the same content rect).
    pub fn viewer_viewport_full(&self) -> usize {
        self.diff_state.viewport_height.max(1)
    }

    pub fn toggle_reviewed(&mut self) {
        let file_idx = self.diff_state.current_file_idx;
        self.toggle_reviewed_for_file_idx(file_idx, true);
    }

    /// Effective collapsed state for `file_idx`, folding in both the user's
    /// explicit override (if any) and the auto rule. Returns `false` for a
    /// missing index.
    pub fn is_file_collapsed(&self, file_idx: usize) -> bool {
        let Some(file) = self.diff_files.get(file_idx) else {
            return false;
        };
        let path = file.display_path_lossy();
        let explicit = self
            .engine
            .session()
            .files
            .get(path.as_path())
            .and_then(|r| r.collapsed);
        let (additions, deletions) = file.stat();
        travelagent_core::auto_collapse::effective_collapsed(
            explicit,
            path.as_path(),
            additions + deletions,
            &self.auto_collapse_cfg,
        )
    }

    /// Reason the file was auto-collapsed (for placeholder text). Returns
    /// `None` when the file is not auto-collapsed — e.g., small text file,
    /// or config has `enabled = false`. The UI still has to check
    /// [`Self::is_file_collapsed`] to decide whether to show a placeholder,
    /// since an explicit override can collapse a non-auto file.
    pub fn auto_collapse_reason_for(
        &self,
        file_idx: usize,
    ) -> Option<travelagent_core::auto_collapse::CollapseReason> {
        let file = self.diff_files.get(file_idx)?;
        let (additions, deletions) = file.stat();
        travelagent_core::auto_collapse::auto_collapse_reason(
            file.display_path_lossy().as_path(),
            additions + deletions,
            &self.auto_collapse_cfg,
        )
    }

    /// Toggle the auto-collapse override on the current file (the `z` binding).
    ///
    /// Semantics:
    /// * `None` → `Some(!auto)` — flip whatever auto said; if auto would have
    ///   collapsed this file, this expands it; otherwise it collapses it.
    /// * `Some(x)` → `Some(!x)` — flip the existing explicit choice.
    ///
    /// The explicit override survives session reloads, so collapsing a
    /// non-lockfile-like file once and closing the session keeps it collapsed
    /// next time.
    pub fn toggle_file_collapse(&mut self) {
        let file_idx = self.diff_state.current_file_idx;
        let Some(file) = self.diff_files.get(file_idx) else {
            return;
        };
        let path = file.display_path_lossy().clone();
        let (additions, deletions) = file.stat();
        let changed = additions + deletions;

        let auto_cfg = &self.auto_collapse_cfg;
        let auto = travelagent_core::auto_collapse::should_auto_collapse(
            path.as_path(),
            changed,
            auto_cfg,
        );

        // Ensure a FileReview entry exists so we can store the override.
        self.engine
            .session_mut()
            .add_file(path.clone(), file.status);
        if let Some(review) = self.engine.session_mut().get_file_mut(&path) {
            review.collapsed = Some(match review.collapsed {
                None => !auto,
                Some(x) => !x,
            });
            self.dirty = true;
        }
        self.rebuild_annotations();
    }

    pub fn toggle_reviewed_for_file_idx(&mut self, file_idx: usize, adjust_cursor: bool) {
        let Some(path) = self
            .diff_files
            .get(file_idx)
            .map(|file| file.display_path_lossy().clone())
        else {
            return;
        };

        if let Some(review) = self.engine.session_mut().get_file_mut(&path) {
            review.reviewed = !review.reviewed;
            let now_reviewed = review.reviewed;
            self.dirty = true;
            self.rebuild_annotations();

            // Auto-stage file when marked as reviewed in local mode
            if now_reviewed
                && self.auto_stage
                && matches!(
                    self.diff_source,
                    DiffSource::WorkingTree | DiffSource::StagedAndUnstaged
                )
                && self.vcs_info.vcs_type == travelagent_core::vcs::VcsType::Git
                && let Some(file) = self.diff_files.get(file_idx)
            {
                let file_path = file.display_path_lossy();
                match std::process::Command::new("git")
                    .arg("add")
                    .arg("--")
                    .arg(file_path.as_os_str())
                    .current_dir(&self.vcs_info.root_path)
                    .output()
                {
                    Ok(out) if out.status.success() => {}
                    _ => self.set_warning(format!("Failed to stage {}", file_path.display())),
                }
            }

            if adjust_cursor {
                self.diff_state.current_file_idx = file_idx;
                // Move cursor to the file header line
                let header_line = self.calculate_file_scroll_offset(file_idx);
                self.diff_state.cursor_line = header_line;
                self.ensure_cursor_visible();
            }
        }
    }

    pub fn file_count(&self) -> usize {
        self.diff_files.len()
    }

    pub fn reviewed_count(&self) -> usize {
        self.engine.session().reviewed_count()
    }

    /// Returns `(total_files, total_additions, total_deletions)` across all diff files.
    pub fn diff_stat(&self) -> (usize, usize, usize) {
        let mut additions = 0;
        let mut deletions = 0;
        for file in &self.diff_files {
            let (a, d) = file.stat();
            additions += a;
            deletions += d;
        }
        (self.diff_files.len(), additions, deletions)
    }

    pub fn set_message(&mut self, msg: impl Into<String>) {
        self.message = Some(Message {
            content: msg.into(),
            message_type: MessageType::Info,
        });
    }

    pub fn set_warning(&mut self, msg: impl Into<String>) {
        self.message = Some(Message {
            content: msg.into(),
            message_type: MessageType::Warning,
        });
    }

    pub fn set_error(&mut self, msg: impl Into<String>) {
        let msg = Message {
            content: msg.into(),
            message_type: MessageType::Error,
        };
        self.error_log.push(msg.clone());
        self.message = Some(msg);
    }

    /// Step the error-log recall cursor once, copying the pointed-to
    /// entry into `self.message` so the status bar shows it. Returns
    /// `false` (without mutating `message`) when the ring is empty.
    /// Backs the `:errors` command.
    pub fn recall_next_error(&mut self) -> bool {
        if let Some(entry) = self.error_log.next_recall() {
            self.message = Some(entry.clone());
            true
        } else {
            false
        }
    }

    /// Set a transient agent-originated breadcrumb that auto-expires after
    /// `ttl`. Used by the MCP bridge to surface agent-initiated viewport
    /// mutations so the human knows why their cursor just jumped.
    pub fn set_flash_message(&mut self, text: impl Into<String>, ttl: Duration) {
        self.agent_flash = Some(AgentFlash {
            text: text.into(),
            expires_at: Instant::now() + ttl,
        });
    }

    /// Return the active flash message text if one is set and hasn't yet
    /// expired. Returns `None` once `expires_at` is in the past. Used by the
    /// status bar and tests.
    pub fn current_flash_text(&self) -> Option<&str> {
        let flash = self.agent_flash.as_ref()?;
        if flash.expires_at > Instant::now() {
            Some(flash.text.as_str())
        } else {
            None
        }
    }

    /// Flip cursor-ownership mode. When pinning, we intentionally keep any
    /// stale `agent_ghost` around so the user has context for where the
    /// agent last pointed. When un-pinning, clear it — in follow mode the
    /// ghost is meaningless (agent navigation moves the real cursor).
    pub fn toggle_viewport_pin(&mut self) {
        self.viewport_pinned = !self.viewport_pinned;
        if !self.viewport_pinned {
            self.agent_ghost = None;
        }
        let msg = if self.viewport_pinned {
            "Viewport pinned — agent navigation won't move your cursor (Ctrl+G to jump to agent)"
        } else {
            "Viewport following agent navigation (Ctrl+P to pin)"
        };
        self.set_message(msg);
    }

    /// Mark every file in the current diff view reviewed. When touring, the
    /// diff view is already scoped to the current stop's commit(s) (see
    /// `tour_reload_current_stop`), so this is exactly "review all in the
    /// current commit". Returns the number of files newly marked reviewed
    /// (already-reviewed files aren't counted). Sets `dirty` + rebuilds
    /// annotations once at the end.
    pub fn mark_all_reviewed(&mut self) -> usize {
        let paths: Vec<(PathBuf, FileStatus)> = self
            .diff_files
            .iter()
            .map(|f| (f.display_path_lossy().clone(), f.status))
            .collect();
        let mut newly = 0;
        for (path, status) in paths {
            self.engine.session_mut().add_file(path.clone(), status);
            if let Some(review) = self.engine.session_mut().get_file_mut(&path)
                && !review.reviewed
            {
                review.reviewed = true;
                newly += 1;
            }
        }
        if newly > 0 {
            self.dirty = true;
            self.rebuild_annotations();
        }
        newly
    }

    /// Mark every file in the current diff view unreviewed. Returns the
    /// number of files that were reviewed and are now cleared. Recoverable
    /// (re-review with `r`), so no confirmation. Backs `:unreview-all`.
    pub fn unmark_all_reviewed(&mut self) -> usize {
        let paths: Vec<PathBuf> = self
            .diff_files
            .iter()
            .map(|f| f.display_path_lossy().clone())
            .collect();
        let mut cleared = 0;
        for path in paths {
            if let Some(review) = self.engine.session_mut().get_file_mut(&path)
                && review.reviewed
            {
                review.reviewed = false;
                cleared += 1;
            }
        }
        if cleared > 0 {
            self.dirty = true;
            self.rebuild_annotations();
        }
        cleared
    }

    /// Restart the review: mark **every** file in the whole session
    /// unreviewed (not just the current diff view), so a re-review starts
    /// from a clean slate. Destructive — the caller (`:review-restart`)
    /// gates this behind a confirmation modal. Returns the number of files
    /// cleared. Also clears any transient "peek" overrides so reviewed-fold
    /// state is consistent afterward.
    pub fn restart_review(&mut self) -> usize {
        let mut cleared = 0;
        for review in self.engine.session_mut().files.values_mut() {
            if review.reviewed {
                review.reviewed = false;
                cleared += 1;
            }
        }
        self.peeked_reviewed.clear();
        if cleared > 0 {
            self.dirty = true;
            self.rebuild_annotations();
        }
        cleared
    }

    /// Count of files currently marked reviewed across the whole session.
    /// Used to phrase the `:review-restart` confirmation prompt.
    pub fn reviewed_file_count(&self) -> usize {
        self.engine
            .session()
            .files
            .values()
            .filter(|r| r.reviewed)
            .count()
    }

    /// Toggle the transient "peek" override for the current file. A reviewed
    /// file is normally folded away in the diff; peeking unfolds its body in
    /// place **without** un-reviewing it (the `[✓]` stays). No-op message on a
    /// file that isn't reviewed (nothing to peek — it's already shown).
    /// Peek state is per-session-run only (not persisted).
    pub fn toggle_reviewed_peek(&mut self) {
        let Some(path) = self.current_file_path().cloned() else {
            return;
        };
        if !self.engine.session().is_file_reviewed(&path) {
            // Not reviewed → already expanded; defer to the normal collapse
            // toggle so `Space` still controls auto-collapsed files.
            self.toggle_file_collapse();
            return;
        }
        if self.peeked_reviewed.contains(&path) {
            self.peeked_reviewed.remove(&path);
            self.set_message("Reviewed file folded");
        } else {
            self.peeked_reviewed.insert(path);
            self.set_message("Peeking reviewed file (still marked reviewed)");
        }
        self.rebuild_annotations();
    }

    /// Whether the current cursor file's reviewed body is being peeked
    /// (expanded despite being reviewed). Read by the diff renderer.
    pub fn is_reviewed_peeked(&self, path: &std::path::Path) -> bool {
        self.peeked_reviewed.contains(path)
    }

    /// Queue an MCP server-push notification. The main loop drains
    /// `notify_queue` each tick into the bridge's notify sink; when no
    /// bridge is attached, notifications silently overflow the queue —
    /// bounded by `MCP_NOTIFY_QUEUE_CAP` so a long-lived TUI without an
    /// attached agent can't grow this unbounded. Oldest entries drop first.
    pub fn push_notify(&mut self, notify: McpNotify) {
        if self.notify_queue.len() >= MCP_NOTIFY_QUEUE_CAP {
            self.notify_queue.pop_front();
        }
        self.notify_queue.push_back(notify);
    }

    /// Lookup a forge-confirmation by id. Checks the currently-pending
    /// slot first, then the history ring. `None` when the id is unknown.
    /// Thin delegation to `AgentActionState::find`.
    pub fn find_forge_action(&self, id: &str) -> Option<&PendingAgentAction> {
        self.agent_action.find(id)
    }

    /// Returns `true` when the forge-write confirmation modal should
    /// intercept keystrokes. Gates on (a) a pending forge action in the
    /// `Pending` state and (b) the user being in `InputMode::Normal` so
    /// a literal `y` / `n` inside a comment / review-body / command /
    /// search buffer doesn't silently approve or reject the proposal
    /// (CRITICAL 1). Mirrors the gate inside `main.rs`'s key-handler;
    /// exposed here for direct unit coverage.
    pub fn forge_modal_should_capture(&self) -> bool {
        matches!(self.nav.input_mode, InputMode::Normal) && self.agent_action.has_waiting_pending()
    }

    /// Phase I5-2b: walk the working tree and refresh `spec_statuses`
    /// with the current `scan_spec_links` result. Active (non-resolved)
    /// spec comments get a `Linked` / `Unlinked` classification;
    /// resolved specs are intentionally omitted so the map stays small
    /// over the lifetime of a long session.
    ///
    /// Called on Sparring panel entry and after every
    /// `AcceptGeneratedTest` approval. Cheap in practice (the scanner
    /// skips the heavy build dirs and caps file size), but we don't
    /// call it per-frame — the map only mutates when one of those
    /// triggers fires.
    pub fn refresh_spec_statuses(&mut self) {
        use travelagent_core::model::CommentType;
        use travelagent_core::sparring::{SparringStatus, scan_spec_links};

        let mut active_ids: Vec<String> = Vec::new();
        let session = self.engine.session();
        for c in &session.review_comments {
            if matches!(c.comment_type, CommentType::Spec) && !c.resolved {
                active_ids.push(c.id.clone());
            }
        }
        for fr in session.files.values() {
            for c in &fr.file_comments {
                if matches!(c.comment_type, CommentType::Spec) && !c.resolved {
                    active_ids.push(c.id.clone());
                }
            }
            for cs in fr.line_comments.values() {
                for c in cs {
                    if matches!(c.comment_type, CommentType::Spec) && !c.resolved {
                        active_ids.push(c.id.clone());
                    }
                }
            }
            for c in &fr.orphaned_comments {
                if matches!(c.comment_type, CommentType::Spec) && !c.resolved {
                    active_ids.push(c.id.clone());
                }
            }
        }

        let links = scan_spec_links(&self.vcs_info.root_path, &active_ids);
        let mut statuses: std::collections::HashMap<String, SparringStatus> =
            std::collections::HashMap::new();
        for id in active_ids {
            let status = if links.contains_key(&id) {
                SparringStatus::Linked
            } else {
                SparringStatus::Unlinked
            };
            statuses.insert(id, status);
        }
        self.spec_statuses = statuses;
    }

    /// Phase I4c-2: resolve a proposed test path under the repo root
    /// and write the agent-generated body. Returns the absolute path
    /// that was written, or a human-readable error. Callers are
    /// responsible for preflighting `spar_mode`; this helper only
    /// enforces the path-safety invariants at write time so the
    /// approval path can't be tricked by a propose-time/approve-time
    /// TOCTOU gap.
    fn write_generated_test(
        &self,
        test_path: &str,
        test_body: &str,
    ) -> Result<std::path::PathBuf, String> {
        let rel = std::path::Path::new(test_path);
        if rel.is_absolute() {
            return Err(format!("test_path must be repo-relative: {test_path}"));
        }
        if rel.components().any(|c| {
            matches!(
                c,
                std::path::Component::ParentDir | std::path::Component::RootDir
            )
        }) {
            return Err(format!(
                "test_path must not traverse outside the repo: {test_path}"
            ));
        }
        let abs = self.vcs_info.root_path.join(rel);
        if let Some(parent) = abs.parent() {
            std::fs::create_dir_all(parent)
                .map_err(|e| format!("failed to create directory {}: {e}", parent.display()))?;
        }
        std::fs::write(&abs, test_body)
            .map_err(|e| format!("failed to write {}: {e}", abs.display()))?;
        Ok(abs)
    }

    /// Human approved the currently-pending forge action. Transitions
    /// `Pending → Executing` and spawns the forge `submit_review` call
    /// onto the shared tokio runtime; the main event loop then polls
    /// `poll_forge_completion` each tick to drive `Executing →
    /// Succeeded / Failed` once the background task reports back.
    ///
    /// Non-blocking by design (WARNING 1): the forge HTTP round trip
    /// must not sit on the TUI thread, because during a `block_on` no
    /// keystrokes process, no ticks run, no frames render, and pending
    /// `trv_cancel_confirmation` commands sit unread in the mpsc queue.
    ///
    /// No-op when nothing is pending.
    pub fn approve_pending_agent_action(&mut self) {
        use travelagent_core::forge::NewReview;
        let Some(mut action) = self.agent_action.take_pending() else {
            return;
        };
        if !matches!(action.status, ConfirmationStatus::Pending) {
            // Not in a state we can approve — put it back and bail.
            self.agent_action.put_back_pending(action);
            return;
        }
        // Mental-model write: commit synchronously and transition all
        // the way to Succeeded in a single tick. No forge call, no
        // background worker, no `agent_action.forge_completion` — just
        // overwrite the session's mental_model and archive.
        if let AgentActionKind::SetMentalModel { mental_model } = action.kind.clone() {
            let id = action.id.clone();
            let now = chrono::Utc::now();
            // Shared with the TUI `Ctrl+S` path in handler.rs: empty
            // collapses to `None`, `created_at` is preserved across
            // edits, `updated_at` is stamped now.
            self.engine
                .session_mut()
                .commit_mental_model(mental_model, now);
            self.dirty = true;
            let result_json = serde_json::to_string(&serde_json::json!({
                "ok": true,
                "kind": "set_mental_model",
                "at": now.to_rfc3339(),
            }))
            .unwrap_or_else(|_| r#"{"ok":true}"#.to_string());
            action.status = ConfirmationStatus::Succeeded { result_json };
            action.decided_at = Some(now);
            let decided_id = action.id.clone();
            self.agent_action.archive_with_decision(
                action,
                LastAgentDecision {
                    id: decided_id.clone(),
                    decision: "succeeded",
                    reason: None,
                    decided_at: now,
                },
            );
            self.push_notify(McpNotify::AgentActionDecided {
                id: decided_id,
                decision: "approved",
                reason: None,
            });
            self.set_message(format!("Mental model overwrite approved (id {id})"));
            return;
        }
        // AcceptGeneratedTest write: resolve the path under the repo
        // root, refuse path escapes, write the file, transition to
        // Succeeded. Like `SetMentalModel`, this commits synchronously
        // — no VCS commit, the human authors that.
        if let AgentActionKind::AcceptGeneratedTest {
            test_path,
            test_body,
            spec_id,
        } = action.kind.clone()
        {
            let id = action.id.clone();
            let now = chrono::Utc::now();
            match self.write_generated_test(&test_path, &test_body) {
                Ok(abs_path) => {
                    let result_json = serde_json::to_string(&serde_json::json!({
                        "ok": true,
                        "kind": "accept_generated_test",
                        "test_path": test_path,
                        "abs_path": abs_path.to_string_lossy(),
                        "spec_id": spec_id,
                        "at": now.to_rfc3339(),
                    }))
                    .unwrap_or_else(|_| r#"{"ok":true}"#.to_string());
                    action.status = ConfirmationStatus::Succeeded { result_json };
                    action.decided_at = Some(now);
                    let decided_id = action.id.clone();
                    self.agent_action.archive_with_decision(
                        action,
                        LastAgentDecision {
                            id: decided_id.clone(),
                            decision: "succeeded",
                            reason: None,
                            decided_at: now,
                        },
                    );
                    self.push_notify(McpNotify::AgentActionDecided {
                        id: decided_id,
                        decision: "approved",
                        reason: None,
                    });
                    self.set_message(format!("Generated test landed at {test_path} (id {id})"));
                    // Phase I5-2b: the new test may have flipped a spec
                    // from Unlinked → Linked. Refresh now so the
                    // Sparring panel reflects the link the next time
                    // it renders, without waiting for a manual rescan.
                    self.refresh_spec_statuses();
                }
                Err(err) => {
                    action.status = ConfirmationStatus::Failed { error: err.clone() };
                    action.decided_at = Some(now);
                    let decided_id = action.id.clone();
                    self.agent_action.archive_with_decision(
                        action,
                        LastAgentDecision {
                            id: decided_id.clone(),
                            decision: "failed",
                            reason: None,
                            decided_at: now,
                        },
                    );
                    self.push_notify(McpNotify::AgentActionDecided {
                        id: decided_id,
                        decision: "failed",
                        reason: None,
                    });
                    self.set_error(format!("Generated test write failed: {err}"));
                }
            }
            return;
        }
        // Snapshot the forge call's inputs before we consume `action`.
        // The `else` arm is unreachable — we've handled SetMentalModel
        // and AcceptGeneratedTest above, and SubmitReview is the only
        // remaining variant — but the explicit binding keeps the
        // compiler honest when a future variant is added.
        let AgentActionKind::SubmitReview { verdict, body } = action.kind.clone() else {
            self.agent_action.put_back_pending(action);
            return;
        };
        let id = action.id.clone();

        // Transition: Pending → Executing directly. The previous
        // `Approved` intermediate was dead (observers serialize on the
        // TUI thread and never saw it); dropping it simplifies the
        // state machine without losing any observable information.
        action.status = ConfirmationStatus::Executing;
        self.agent_action.put_back_pending(action);

        // Gate: the remote state could have been torn down between
        // propose time (where we checked `has_forge()`) and now. Treat
        // that as a synchronous failure rather than silently succeeding.
        // `Arc::clone` on the forge handle is cheap (atomic refcount)
        // and shippable across the await — see `RemoteSessionState.forge`.
        let (forge_arc, pr_id) = match self.remote() {
            Some(r) => match r.forge.as_ref() {
                Some(f) => (std::sync::Arc::clone(f), r.pr_id.clone()),
                None => {
                    let (tx, rx) = tokio::sync::oneshot::channel();
                    let _ = tx.send(ForgeSubmitResult::Err(
                        "no forge attached (demo mode?)".to_string(),
                    ));
                    self.agent_action.set_completion(rx);
                    return;
                }
            },
            None => {
                let (tx, rx) = tokio::sync::oneshot::channel();
                let _ = tx.send(ForgeSubmitResult::Err(
                    "no longer in remote mode".to_string(),
                ));
                self.agent_action.set_completion(rx);
                return;
            }
        };

        let (tx, rx) = tokio::sync::oneshot::channel::<ForgeSubmitResult>();
        self.agent_action.set_completion(rx);
        let review = NewReview {
            verdict,
            body,
            comments: vec![],
        };
        // Spawn the forge call on the shared tokio runtime so the TUI
        // thread stays responsive. The result lands on the oneshot;
        // `poll_forge_completion` (called each tick from main.rs)
        // finalizes.
        self.runtime_handle.spawn(async move {
            let result = match forge_arc.submit_review(&pr_id, review).await {
                Ok(()) => ForgeSubmitResult::Ok,
                Err(e) => ForgeSubmitResult::Err(format!("{e}")),
            };
            // Receiver may have been dropped if the user cancelled or
            // the TUI is shutting down; ignore the send error.
            let _ = tx.send(result);
        });
        self.set_message(format!(
            "Forge submit_review approved (id {id}): awaiting forge response"
        ));
    }

    /// Non-blocking check for an in-flight forge call spawned by
    /// `approve_pending_agent_action`. On completion, drives the final
    /// `Executing → Succeeded / Failed` transition, archives the action,
    /// emits the `agent_action_decided` + `ReviewSubmitted`
    /// notifications, and records manual-path session bookkeeping
    /// (`last_review_submitted_at` / `last_review_sha` / `dirty`) on
    /// success — WARNING 4.
    ///
    /// Called from the main event loop each tick alongside
    /// `tick_agent_action_timeout`. Idempotent when no completion is
    /// pending.
    pub fn poll_forge_completion(&mut self) {
        use travelagent_core::forge::ReviewVerdict;
        let Some(mut rx) = self.agent_action.take_completion() else {
            return;
        };
        let result = match rx.try_recv() {
            Ok(r) => r,
            Err(tokio::sync::oneshot::error::TryRecvError::Empty) => {
                // Still in flight — put the receiver back and bail.
                self.agent_action.put_back_completion(rx);
                return;
            }
            Err(tokio::sync::oneshot::error::TryRecvError::Closed) => {
                // Sender dropped without delivering — treat as a hard
                // failure so the action still reaches a terminal state.
                ForgeSubmitResult::Err("forge worker dropped without reply".to_string())
            }
        };
        let Some(mut action) = self.agent_action.take_pending() else {
            // No pending action to finalize (could have been cancelled
            // or timed out while the worker was still running). Drop
            // the result silently.
            return;
        };
        let id = action.id.clone();
        // Snapshot the verdict label for wire output + status messages.
        // `SetMentalModel` finalizes synchronously in
        // `approve_pending_agent_action` so it never reaches this poll —
        // guard with a defensive early-return so a misfire doesn't try
        // to label it as a forge verdict.
        let verdict_label: &'static str = match &action.kind {
            AgentActionKind::SubmitReview { verdict, .. } => match verdict {
                ReviewVerdict::Comment => "Comment",
                ReviewVerdict::Approve => "Approve",
                ReviewVerdict::RequestChanges => "Request Changes",
            },
            AgentActionKind::SetMentalModel { .. }
            | AgentActionKind::AcceptGeneratedTest { .. } => {
                // Both variants finalize synchronously in
                // `approve_pending_agent_action` — they never reach the
                // forge-completion poll. Put it back defensively if a
                // misfire ever routes one here.
                self.agent_action.put_back_pending(action);
                return;
            }
        };
        let now = chrono::Utc::now();
        action.decided_at = Some(now);
        match result {
            ForgeSubmitResult::Ok => {
                let result_json = serde_json::to_string(&serde_json::json!({
                    "ok": true,
                    "verdict": verdict_label,
                    "at": now.to_rfc3339(),
                }))
                .unwrap_or_else(|_| r#"{"ok":true}"#.to_string());
                action.status = ConfirmationStatus::Succeeded {
                    result_json: result_json.clone(),
                };
                // Forge mutated shared state — also emit the existing
                // ReviewSubmitted notification so subscribers to that
                // channel (not just `agent_action_decided`) see the
                // event, matching the manual :submit path.
                self.push_notify(McpNotify::ReviewSubmitted {
                    verdict: Some(verdict_label.to_string()),
                    at: now.to_rfc3339(),
                });
                // WARNING 4: mirror the manual `R` / `:submit` flow's
                // session bookkeeping (handler.rs:~1048) so the stale-
                // review marker in the status bar works identically
                // whether the review was submitted by hand or through
                // agent propose+approve.
                let head_sha = self
                    .remote()
                    .and_then(|r| r.pr_metadata.as_ref().map(|m| m.head_sha.clone()));
                self.engine.session_mut().last_review_submitted_at = Some(now);
                if let Some(sha) = head_sha {
                    self.engine.session_mut().last_review_sha = Some(sha);
                }
                self.dirty = true;
                self.set_message(format!(
                    "Forge submit_review approved (id {id}): {verdict_label}"
                ));
            }
            ForgeSubmitResult::Err(error) => {
                action.status = ConfirmationStatus::Failed {
                    error: error.clone(),
                };
                self.set_error(format!(
                    "Forge submit_review approved (id {id}) but failed: {error}"
                ));
            }
        }
        let decided_id = action.id.clone();
        let terminal_decision: &'static str = match &action.status {
            ConfirmationStatus::Succeeded { .. } => "succeeded",
            ConfirmationStatus::Failed { .. } => "failed",
            _ => "approved",
        };
        // WARNING 5: archive + record the terminal decision atomically so
        // status subscribers can reconstruct the most recent event.
        self.agent_action.archive_with_decision(
            action,
            LastAgentDecision {
                id: decided_id.clone(),
                decision: terminal_decision,
                reason: None,
                decided_at: now,
            },
        );
        // Decision notification — the legacy wire token is `"approved"`;
        // the richer `succeeded` / `failed` classification lives on
        // `agent_action.last_decision` (and on the archived action's status)
        // so existing agent subscribers don't break.
        self.push_notify(McpNotify::AgentActionDecided {
            id: decided_id,
            decision: "approved",
            reason: None,
        });
    }

    /// Human rejected the currently-pending forge action (pressed `n`
    /// or `Esc` in the modal). Transitions to `Rejected { User }` and
    /// archives.
    pub fn reject_pending_agent_action(&mut self) {
        self.reject_pending_agent_action_with_reason(RejectReason::User);
    }

    /// Reject the currently-pending forge action with a specific reason.
    /// Separate from `reject_pending_agent_action` so the timeout /
    /// agent-cancel paths can share the transition logic.
    fn reject_pending_agent_action_with_reason(&mut self, reason: RejectReason) {
        let Some(mut action) = self.agent_action.take_pending() else {
            return;
        };
        if !matches!(action.status, ConfirmationStatus::Pending) {
            self.agent_action.put_back_pending(action);
            return;
        }
        let now = chrono::Utc::now();
        action.status = ConfirmationStatus::Rejected { reason };
        action.decided_at = Some(now);
        let id = action.id.clone();
        self.agent_action.archive_with_decision(
            action,
            LastAgentDecision {
                id: id.clone(),
                decision: "rejected",
                reason: Some(reason.as_str()),
                decided_at: now,
            },
        );
        self.set_message(format!("Rejected agent's forge proposal (id {id})"));
        self.push_notify(McpNotify::AgentActionDecided {
            id,
            decision: "rejected",
            reason: Some(reason.as_str()),
        });
    }

    /// Check if the currently-pending forge action has timed out, and if
    /// so transition it to `Rejected { Timeout }`, archive it, and emit
    /// the `agent_action_decided` notification. Called from the main
    /// event-loop tick.
    ///
    /// Uses the monotonic `proposed_at_monotonic` (not wall-clock
    /// `proposed_at`) for the staleness check so NTP skew or
    /// suspend-resume can't indefinitely postpone the timeout.
    pub fn tick_agent_action_timeout(&mut self) {
        let is_stale = self.agent_action.pending().is_some_and(|p| {
            matches!(p.status, ConfirmationStatus::Pending)
                && p.proposed_at_monotonic.elapsed() > CONFIRMATION_TIMEOUT
        });
        if !is_stale {
            return;
        }
        if let Some(mut action) = self.agent_action.take_pending() {
            let now = chrono::Utc::now();
            action.status = ConfirmationStatus::Rejected {
                reason: RejectReason::Timeout,
            };
            action.decided_at = Some(now);
            let id = action.id.clone();
            self.agent_action.archive_with_decision(
                action,
                LastAgentDecision {
                    id: id.clone(),
                    decision: "rejected",
                    reason: Some(RejectReason::Timeout.as_str()),
                    decided_at: now,
                },
            );
            self.push_notify(McpNotify::AgentActionDecided {
                id,
                decision: "rejected",
                reason: Some(RejectReason::Timeout.as_str()),
            });
        }
    }

    /// Adopt an externally-constructed warning queue. Used by remote-mode
    /// setup paths (`remote::create_remote_app`) which build the queue
    /// before the App so they can hand a clone to the forge client's
    /// `warn_handler` callback at construction time. The default App
    /// already owns an empty queue — this swaps it for the one the forge
    /// is writing into.
    pub fn attach_forge_warn_queue(
        &mut self,
        queue: std::sync::Arc<std::sync::Mutex<VecDeque<String>>>,
    ) {
        self.forge_warn_queue = queue;
    }

    /// Drain any warnings the forge client queued via its warn callback
    /// into the status bar + error-log ring. Called each tick by the
    /// main event loop. No-op when the queue is empty. Mirrors the
    /// `notify_queue` drain shape but targets `set_error` instead of the
    /// MCP notify sink — forge pagination truncation is user-visible
    /// feedback, not an agent notification.
    pub fn drain_forge_warnings(&mut self) {
        let drained: Vec<String> = {
            let Ok(mut q) = self.forge_warn_queue.lock() else {
                return;
            };
            if q.is_empty() {
                return;
            }
            q.drain(..).collect()
        };
        for msg in drained {
            self.set_error(msg);
        }
    }

    /// Record where the agent tried to navigate. Called by `handle_select_file`
    /// in the MCP bridge when `viewport_pinned` is `true`. Rejects out-of-range
    /// `file_idx` silently so a racy rename can't wedge a stale ghost.
    pub fn record_agent_ghost(&mut self, file_idx: usize) {
        let Some(file) = self.diff_files.get(file_idx) else {
            return;
        };
        let path = file.display_path_lossy().to_string_lossy().to_string();
        self.agent_ghost = Some(AgentGhost { file_idx, path });
    }

    /// Jump to the agent's ghost position and clear pinned state — returning
    /// the human to follow mode at the agent's location. Returns `true` when
    /// a ghost was consumed, `false` when there was nothing to jump to.
    ///
    /// Borrow-checker note: we copy the ghost fields out before mutating the
    /// App so `self.jump_to_file` can reborrow `self`.
    pub fn jump_to_agent_ghost(&mut self) -> bool {
        let Some(ghost) = self.agent_ghost.take() else {
            // No ghost — leave pin state alone so the user can still toggle
            // follow-mode manually. Status message clarifies the no-op.
            self.set_message("No agent ghost to jump to");
            return false;
        };
        self.viewport_pinned = false;
        // Guard against stale ghost (e.g. rename/diff rescan dropped the
        // file): fall through to a soft error rather than panicking.
        if ghost.file_idx >= self.diff_files.len() {
            self.set_warning("Agent ghost pointed at a file that's no longer in the diff");
            return false;
        }
        self.jump_to_file(ghost.file_idx);
        true
    }

    pub fn jump_to_bottom(&mut self) {
        // Find the last non-spacing annotation line so the cursor lands on
        // actual content rather than the trailing blank line.
        let total = self.line_annotations.len();
        let mut target = total.saturating_sub(1);
        while target > 0 {
            if !matches!(
                self.line_annotations.get(target),
                Some(AnnotatedLine::Spacing)
            ) {
                break;
            }
            target -= 1;
        }
        self.diff_state.cursor_line = target;
        // Always position so the last line is at the bottom of the viewport
        let viewport = self.diff_state.viewport_height.max(1);
        self.diff_state.scroll_offset = total.saturating_sub(viewport);
        self.update_current_file_from_cursor();
    }

    pub(super) fn file_idx_to_tree_idx(&self, target_file_idx: usize) -> Option<usize> {
        let visible_items = self.build_visible_items();
        for (tree_idx, item) in visible_items.iter().enumerate() {
            if let FileTreeItem::File { file_idx, .. } = item
                && *file_idx == target_file_idx
            {
                return Some(tree_idx);
            }
        }
        None
    }

    /// Returns the source line number and side at the current cursor position, if on a diff line
    pub fn get_line_at_cursor(&self) -> Option<(u32, LineSide)> {
        let target = self.diff_state.cursor_line;
        match self.line_annotations.get(target) {
            Some(
                AnnotatedLine::DiffLine {
                    old_lineno,
                    new_lineno,
                    ..
                }
                | AnnotatedLine::SideBySideLine {
                    old_lineno,
                    new_lineno,
                    ..
                },
            ) => {
                // Prefer new line number (for added/context lines), fall back to old (for deleted)
                new_lineno
                    .map(|ln| (ln, LineSide::New))
                    .or_else(|| old_lineno.map(|ln| (ln, LineSide::Old)))
            }
            _ => None,
        }
    }

    /// Get the text of the diff line currently under the cursor, without the
    /// leading `+`/`-`/` ` diff marker. Returns `None` when the cursor is not
    /// on a diff line (e.g., on a comment, file header, or gap expander).
    pub fn current_line_content(&self) -> Option<String> {
        let target = self.diff_state.cursor_line;
        match self.line_annotations.get(target)? {
            AnnotatedLine::DiffLine {
                file_idx,
                hunk_idx,
                line_idx,
                ..
            } => {
                let file = self.diff_files.get(*file_idx)?;
                let hunk = file.hunks.get(*hunk_idx)?;
                let line = hunk.lines.get(*line_idx)?;
                Some(line.content.clone())
            }
            AnnotatedLine::SideBySideLine {
                file_idx,
                hunk_idx,
                add_line_idx,
                del_line_idx,
                ..
            } => {
                // Prefer the added line (new side); fall back to deleted.
                let file = self.diff_files.get(*file_idx)?;
                let hunk = file.hunks.get(*hunk_idx)?;
                let idx = add_line_idx.or(*del_line_idx)?;
                hunk.lines.get(idx).map(|l| l.content.clone())
            }
            _ => None,
        }
    }

    /// Get the content of a specific source line in the current file.
    /// Walks the current file's hunks and returns the first line whose
    /// `new_lineno` (when `side == New`) or `old_lineno` (when `side == Old`)
    /// matches `line`. Used for suggestion-block insertion at the anchor line.
    pub fn line_content_at(&self, line: u32, side: LineSide) -> Option<String> {
        let file = self.diff_files.get(self.diff_state.current_file_idx)?;
        for hunk in &file.hunks {
            for diff_line in &hunk.lines {
                let matches = match side {
                    LineSide::New => diff_line.new_lineno == Some(line),
                    LineSide::Old => diff_line.old_lineno == Some(line),
                };
                if matches {
                    return Some(diff_line.content.clone());
                }
            }
        }
        None
    }

    /// Get the content of the line range (inclusive) on the given side in the
    /// current file. Lines are joined with `\n`. Missing lines are skipped.
    /// Used for visual-selection suggestion blocks.
    pub fn line_content_range(&self, start: u32, end: u32, side: LineSide) -> Option<String> {
        let file = self.diff_files.get(self.diff_state.current_file_idx)?;
        let (lo, hi) = if start <= end {
            (start, end)
        } else {
            (end, start)
        };
        let mut parts: Vec<String> = Vec::new();
        for target in lo..=hi {
            for hunk in &file.hunks {
                for diff_line in &hunk.lines {
                    let matches = match side {
                        LineSide::New => diff_line.new_lineno == Some(target),
                        LineSide::Old => diff_line.old_lineno == Some(target),
                    };
                    if matches {
                        parts.push(diff_line.content.clone());
                        break;
                    }
                }
            }
        }
        if parts.is_empty() {
            None
        } else {
            Some(parts.join("\n"))
        }
    }
}

#[cfg(test)]
mod tree_tests {
    use super::*;
    use std::collections::HashSet;
    use travelagent_core::model::{DiffFile, FileStatus};

    fn make_file(path: &str) -> DiffFile {
        DiffFile {
            old_path: None,
            new_path: Some(PathBuf::from(path)),
            status: FileStatus::Modified,
            hunks: vec![],
            is_binary: false,
            is_too_large: false,
            is_commit_message: false,
        }
    }

    struct TreeTestHarness {
        diff_files: Vec<DiffFile>,
        expanded_dirs: HashSet<String>,
    }

    impl TreeTestHarness {
        fn new(paths: &[&str]) -> Self {
            Self {
                diff_files: paths.iter().map(|p| make_file(p)).collect(),
                expanded_dirs: HashSet::new(),
            }
        }

        fn expand_all(&mut self) {
            use std::path::Path;
            for file in &self.diff_files {
                let path = file.display_path_lossy();
                let mut current = path.parent();
                while let Some(parent) = current {
                    if parent != Path::new("") {
                        self.expanded_dirs
                            .insert(parent.to_string_lossy().to_string());
                    }
                    current = parent.parent();
                }
            }
        }

        fn collapse_all(&mut self) {
            self.expanded_dirs.clear();
        }

        fn toggle(&mut self, dir: &str) {
            if self.expanded_dirs.contains(dir) {
                self.expanded_dirs.remove(dir);
            } else {
                self.expanded_dirs.insert(dir.to_string());
            }
        }

        fn build_visible_items(&self) -> Vec<FileTreeItem> {
            use std::path::Path;
            let mut items = Vec::new();
            let mut seen_dirs: HashSet<String> = HashSet::new();

            for (file_idx, file) in self.diff_files.iter().enumerate() {
                let path = file.display_path_lossy();
                let mut ancestors: Vec<String> = Vec::new();
                let mut current = path.parent();
                while let Some(parent) = current {
                    if parent != Path::new("") {
                        ancestors.push(parent.to_string_lossy().to_string());
                    }
                    current = parent.parent();
                }
                ancestors.reverse();

                let mut visible = true;
                for (depth, dir) in ancestors.iter().enumerate() {
                    if !seen_dirs.contains(dir) && visible {
                        let expanded = self.expanded_dirs.contains(dir);
                        items.push(FileTreeItem::Directory {
                            path: dir.clone(),
                            depth,
                            expanded,
                        });
                        seen_dirs.insert(dir.clone());
                    }
                    if !self.expanded_dirs.contains(dir) {
                        visible = false;
                    }
                }

                if visible {
                    items.push(FileTreeItem::File {
                        file_idx,
                        depth: ancestors.len(),
                    });
                }
            }
            items
        }

        fn visible_file_count(&self) -> usize {
            self.build_visible_items()
                .iter()
                .filter(|i| matches!(i, FileTreeItem::File { .. }))
                .count()
        }

        fn visible_dir_count(&self) -> usize {
            self.build_visible_items()
                .iter()
                .filter(|i| matches!(i, FileTreeItem::Directory { .. }))
                .count()
        }
    }

    #[test]
    fn test_expand_all_shows_all_files() {
        let mut h = TreeTestHarness::new(&["src/ui/app.rs", "src/ui/help.rs", "src/main.rs"]);
        h.expand_all();

        assert_eq!(h.visible_file_count(), 3);
    }

    #[test]
    fn test_collapse_all_hides_all_files() {
        let mut h = TreeTestHarness::new(&["src/ui/app.rs", "src/main.rs"]);
        h.expand_all();
        h.collapse_all();

        assert_eq!(h.visible_file_count(), 0);
        assert_eq!(h.visible_dir_count(), 1); // only "src" visible
    }

    #[test]
    fn test_collapse_parent_hides_nested_dirs() {
        let mut h = TreeTestHarness::new(&["src/ui/components/button.rs"]);
        h.expand_all();
        assert_eq!(h.visible_dir_count(), 3); // src, src/ui, src/ui/components

        h.toggle("src");
        let items = h.build_visible_items();
        assert_eq!(items.len(), 1); // only collapsed "src" dir
        assert!(matches!(
            &items[0],
            FileTreeItem::Directory {
                expanded: false,
                ..
            }
        ));
    }

    #[test]
    fn test_root_files_always_visible() {
        let mut h = TreeTestHarness::new(&["README.md", "Cargo.toml"]);
        h.collapse_all();

        assert_eq!(h.visible_file_count(), 2);
    }

    #[test]
    fn test_tree_depth_correct() {
        let mut h = TreeTestHarness::new(&["a/b/c/file.rs"]);
        h.expand_all();

        let items = h.build_visible_items();
        assert!(matches!(&items[0], FileTreeItem::Directory { depth: 0, path, .. } if path == "a"));
        assert!(
            matches!(&items[1], FileTreeItem::Directory { depth: 1, path, .. } if path == "a/b")
        );
        assert!(
            matches!(&items[2], FileTreeItem::Directory { depth: 2, path, .. } if path == "a/b/c")
        );
        assert!(matches!(&items[3], FileTreeItem::File { depth: 3, .. }));
    }

    #[test]
    fn test_toggle_expands_collapsed_dir() {
        let mut h = TreeTestHarness::new(&["src/main.rs"]);
        h.collapse_all();
        assert_eq!(h.visible_file_count(), 0);

        h.toggle("src");
        assert_eq!(h.visible_file_count(), 1);
    }

    #[test]
    fn test_collapsed_dir_stays_in_visible_items() {
        // When collapsing a directory, the directory item itself should still be
        // present in the visible items at a stable position so the cursor can
        // remain on it (fix for #57).
        let mut h = TreeTestHarness::new(&["src/app.rs", "src/main.rs", "tests/test.rs"]);
        h.expand_all();

        // Find the position of "src" directory before collapsing
        let items_before = h.build_visible_items();
        let src_idx_before = items_before
            .iter()
            .position(|item| matches!(item, FileTreeItem::Directory { path, .. } if path == "src"))
            .expect("src dir should be visible");

        // Collapse "src"
        h.toggle("src");

        // "src" should still be visible at the same position
        let items_after = h.build_visible_items();
        let src_idx_after = items_after
            .iter()
            .position(|item| matches!(item, FileTreeItem::Directory { path, .. } if path == "src"))
            .expect("src dir should still be visible after collapse");

        assert_eq!(
            src_idx_before, src_idx_after,
            "collapsed directory should remain at the same position in the tree"
        );

        // And it should be marked as not expanded
        assert!(matches!(
            &items_after[src_idx_after],
            FileTreeItem::Directory {
                expanded: false,
                ..
            }
        ));
    }

    #[test]
    fn test_sibling_dirs_independent() {
        let mut h = TreeTestHarness::new(&["src/app.rs", "tests/test.rs"]);
        h.expand_all();
        h.toggle("src"); // collapse src

        assert_eq!(h.visible_file_count(), 1); // only tests/test.rs
    }
}

#[cfg(test)]
mod commit_selection_tests {
    use super::*;
    use chrono::Utc;
    use std::path::Path;
    use travelagent_core::error::{Result, TrvError};
    use travelagent_core::model::{FileStatus, SessionDiffSource};
    use travelagent_core::vcs::VcsType;

    struct DummyVcs {
        info: VcsInfo,
    }

    impl VcsBackend for DummyVcs {
        fn info(&self) -> &VcsInfo {
            &self.info
        }

        fn get_working_tree_diff(&self) -> Result<Vec<DiffFile>> {
            Err(TrvError::NoChanges)
        }

        fn fetch_context_lines(
            &self,
            _file_path: &Path,
            _file_status: FileStatus,
            _start_line: u32,
            _end_line: u32,
        ) -> Result<Vec<DiffLine>> {
            Ok(Vec::new())
        }
    }

    fn build_app(commit_list: Vec<CommitInfo>) -> App {
        let vcs_info = VcsInfo {
            root_path: PathBuf::from("/tmp"),
            head_commit: "head".to_string(),
            branch_name: Some("main".to_string()),
            vcs_type: VcsType::Git,
        };
        let session = travelagent_core::model::ReviewSession::new(
            vcs_info.root_path.clone(),
            vcs_info.head_commit.clone(),
            vcs_info.branch_name.clone(),
            SessionDiffSource::WorkingTree,
        );

        App::build(
            Box::new(DummyVcs {
                info: vcs_info.clone(),
            }),
            vcs_info,
            Theme::dark(),
            None,
            false,
            Vec::new(),
            session,
            DiffSource::WorkingTree,
            InputMode::CommitSelect,
            commit_list,
            None,
            crate::test_support::runtime_handle(),
            AppMode::Local(LocalState::default()),
        )
        .expect("failed to build test app")
    }

    fn normal_commit(id: &str) -> CommitInfo {
        CommitInfo {
            id: id.to_string(),
            short_id: id.to_string(),
            branch_name: None,
            summary: "Test commit".to_string(),
            body: None,
            author: "Test".to_string(),
            time: Utc::now(),
        }
    }

    #[test]
    fn special_commit_count_counts_leading_special_entries() {
        let app = build_app(vec![
            App::staged_commit_entry(),
            App::unstaged_commit_entry(),
            normal_commit("abc123"),
        ]);

        assert_eq!(app.special_commit_count(), 2);
    }

    #[test]
    fn special_commit_count_ignores_non_leading_special_entries() {
        let app = build_app(vec![normal_commit("abc123"), App::staged_commit_entry()]);

        assert_eq!(app.special_commit_count(), 0);
    }

    #[test]
    fn confirm_commit_selection_with_only_staged_does_not_panic() {
        // given: commit list contains only the staged "special" entry, selected
        let mut app = build_app(vec![App::staged_commit_entry()]);
        app.commit_select.selection_range = Some((0, 0));
        app.commit_select.cursor = 0;

        // when: confirming selection — with only staged selected, selected_ids
        //       (after filtering special commits) is empty. The pre-fix code
        //       would call `.last().unwrap()` on it and panic; the new code
        //       must fall back to the staged-only code path (`load_staged_selection`).
        //
        // `DummyVcs` doesn't override `get_staged_diff`, so the default trait
        // impl returns `UnsupportedOperation`. That error is the expected
        // outcome for this test VCS — the key point is we reach the staged
        // path rather than panicking on `.last().unwrap()`.
        let result = app.confirm_commit_selection();
        match result {
            Ok(()) => {} // staged-only path succeeded (unlikely with dummy VCS)
            Err(TrvError::UnsupportedOperation(msg)) => {
                assert!(
                    msg.contains("Staged"),
                    "expected staged-path error, got: {msg}"
                );
            }
            Err(other) => panic!("unexpected error (should not panic on unwrap): {other:?}"),
        }
    }
}

#[cfg(test)]
mod tour_unwrap_tests {
    use super::*;
    use std::path::Path;
    use travelagent_core::error::{Result, TrvError};
    use travelagent_core::model::{FileStatus, SessionDiffSource, TourStop};
    use travelagent_core::vcs::VcsType;

    struct TourDummyVcs {
        info: VcsInfo,
    }

    impl VcsBackend for TourDummyVcs {
        fn info(&self) -> &VcsInfo {
            &self.info
        }

        fn get_working_tree_diff(&self) -> Result<Vec<DiffFile>> {
            Err(TrvError::NoChanges)
        }

        fn get_commit_range_diff(&self, _commit_ids: &[String]) -> Result<Vec<DiffFile>> {
            // Produce a minimal, valid diff so tour_reload_current_stop succeeds.
            Ok(Vec::new())
        }

        fn fetch_context_lines(
            &self,
            _file_path: &Path,
            _file_status: FileStatus,
            _start_line: u32,
            _end_line: u32,
        ) -> Result<Vec<DiffLine>> {
            Ok(Vec::new())
        }
    }

    fn build_tour_app() -> App {
        let vcs_info = VcsInfo {
            root_path: PathBuf::from("/tmp"),
            head_commit: "head".to_string(),
            branch_name: Some("main".to_string()),
            vcs_type: VcsType::Git,
        };
        let session = travelagent_core::model::ReviewSession::new(
            vcs_info.root_path.clone(),
            vcs_info.head_commit.clone(),
            vcs_info.branch_name.clone(),
            SessionDiffSource::WorkingTree,
        );

        App::build(
            Box::new(TourDummyVcs {
                info: vcs_info.clone(),
            }),
            vcs_info,
            Theme::dark(),
            None,
            false,
            Vec::new(),
            session,
            DiffSource::WorkingTree,
            InputMode::Normal,
            Vec::new(),
            None,
            crate::test_support::runtime_handle(),
            AppMode::Local(LocalState::default()),
        )
        .expect("failed to build test app")
    }

    #[test]
    fn tour_start_with_stops_sets_tour_and_does_not_panic() {
        // Regression test for H4: tour_start previously used unwrap() to read
        // self.tour.plan after setting it. Any accidental clearing between the set
        // and the read would panic. The new code uses a local binding.
        let mut app = build_tour_app();
        let stops = vec![TourStop {
            commit_ids: vec!["abc".into()],
            summary: "only stop".into(),
            risk: travelagent_core::risk::RiskScore::MIN,
        }];

        let result = app.tour_start(stops);
        assert!(result.is_ok(), "tour_start should succeed: {result:?}");
        assert!(app.tour.plan.is_some());
        assert_eq!(app.tour.plan.as_ref().unwrap().index, 0);
    }

    #[test]
    fn tour_start_called_twice_does_not_panic() {
        // "tour_start already running" path — starting a second tour replaces
        // the first and must not panic on the prior unwrap.
        let mut app = build_tour_app();
        let stops_a = vec![TourStop {
            commit_ids: vec!["aaa".into()],
            summary: "first".into(),
            risk: travelagent_core::risk::RiskScore::MIN,
        }];
        let stops_b = vec![TourStop {
            commit_ids: vec!["bbb".into()],
            summary: "second".into(),
            risk: travelagent_core::risk::RiskScore::MIN,
        }];

        app.tour_start(stops_a).unwrap();
        let result = app.tour_start(stops_b);
        assert!(result.is_ok());
        let tour = app.tour.plan.as_ref().unwrap();
        assert_eq!(tour.stops.len(), 1);
        assert_eq!(tour.stops[0].summary, "second");
    }

    #[test]
    fn tour_goto_when_no_tour_returns_error_not_panic() {
        // Regression test: tour_goto used as_mut().unwrap() after confirming
        // tour was Some — but the new code uses let-else for the whole flow.
        let mut app = build_tour_app();
        let result = app.tour_goto(0);
        assert!(result.is_err(), "tour_goto without tour should error");
    }

    #[test]
    fn tour_goto_valid_index_succeeds() {
        let mut app = build_tour_app();
        let stops = vec![
            TourStop {
                commit_ids: vec!["aaa".into()],
                summary: "first".into(),
                risk: travelagent_core::risk::RiskScore::MIN,
            },
            TourStop {
                commit_ids: vec!["bbb".into()],
                summary: "second".into(),
                risk: travelagent_core::risk::RiskScore::MIN,
            },
        ];
        app.tour_start(stops).unwrap();

        let result = app.tour_goto(1);
        assert!(result.is_ok());
        assert_eq!(app.tour.plan.as_ref().unwrap().index, 1);
    }
}

#[cfg(test)]
mod merge_method_tests {
    use super::*;
    use travelagent_core::error::TrvError;
    use travelagent_core::forge::MergeMethod;

    #[test]
    fn pick_merge_method_prefers_squash_when_allowed() {
        let allowed = vec![MergeMethod::Squash, MergeMethod::Merge, MergeMethod::Rebase];
        assert_eq!(
            App::pick_merge_method(&allowed).unwrap(),
            MergeMethod::Squash
        );
    }

    #[test]
    fn pick_merge_method_falls_through_to_merge_when_no_squash() {
        let allowed = vec![MergeMethod::Merge, MergeMethod::Rebase];
        assert_eq!(
            App::pick_merge_method(&allowed).unwrap(),
            MergeMethod::Merge
        );
    }

    #[test]
    fn pick_merge_method_falls_through_to_rebase_as_last_resort() {
        let allowed = vec![MergeMethod::Rebase];
        assert_eq!(
            App::pick_merge_method(&allowed).unwrap(),
            MergeMethod::Rebase
        );
    }

    #[test]
    fn pick_merge_method_respects_squash_only_repos() {
        // Squash-only repo — most common modern convention.
        let allowed = vec![MergeMethod::Squash];
        assert_eq!(
            App::pick_merge_method(&allowed).unwrap(),
            MergeMethod::Squash
        );
    }

    #[test]
    fn pick_merge_method_errors_when_none_allowed() {
        // Repo allows nothing — must error, not panic.
        let allowed: Vec<MergeMethod> = vec![];
        let result = App::pick_merge_method(&allowed);
        assert!(matches!(result, Err(TrvError::UnsupportedOperation(_))));
    }
}

#[cfg(test)]
mod scroll_tests {
    use super::*;

    /// Test the max_scroll_offset calculation logic directly using DiffState
    /// This tests the core algorithm without needing full App setup
    fn calc_max_scroll(total_lines: usize, viewport_height: usize, wrap_lines: bool) -> usize {
        let viewport = viewport_height.max(1);
        if wrap_lines {
            // With wrapping, allow scrolling to show the last line at the top
            total_lines.saturating_sub(1)
        } else {
            // Without wrapping, stop when last line is at the bottom
            total_lines.saturating_sub(viewport)
        }
    }

    #[test]
    fn should_calculate_max_scroll_without_wrapping() {
        // Given 103 total lines and viewport of 20 (simulating header + 100 lines + spacing)
        let total = 103;
        let viewport = 20;

        // When we calculate max_scroll without wrapping
        let max_scroll = calc_max_scroll(total, viewport, false);

        // Then max_scroll should be total - viewport (allows last line at bottom)
        assert_eq!(max_scroll, 83); // 103 - 20
    }

    #[test]
    fn should_calculate_max_scroll_with_wrapping() {
        // Given 103 total lines and viewport of 20, with wrapping enabled
        let total = 103;
        let viewport = 20;

        // When we calculate max_scroll with wrapping
        let max_scroll = calc_max_scroll(total, viewport, true);

        // Then max_scroll should be total - 1 (allows last line at top)
        assert_eq!(max_scroll, 102); // 103 - 1
    }

    #[test]
    fn should_allow_scrolling_further_with_wrapping() {
        // Given identical content with and without wrapping
        let total = 103;
        let viewport = 20;

        // When we calculate max_scroll for both
        let max_no_wrap = calc_max_scroll(total, viewport, false);
        let max_with_wrap = calc_max_scroll(total, viewport, true);

        // Then wrapping should allow scrolling further
        assert!(
            max_with_wrap > max_no_wrap,
            "With wrapping, max_scroll ({max_with_wrap}) should be greater than without ({max_no_wrap})"
        );

        // The difference should be viewport - 1
        assert_eq!(max_with_wrap - max_no_wrap, viewport - 1);
    }

    #[test]
    fn should_handle_small_content_without_wrapping() {
        // Given content smaller than viewport (13 lines in viewport of 50)
        let total = 13;
        let viewport = 50;

        // When we calculate max_scroll
        let max_scroll = calc_max_scroll(total, viewport, false);

        // Then max_scroll should be 0 (no scrolling needed)
        assert_eq!(max_scroll, 0);
    }

    #[test]
    fn should_handle_small_content_with_wrapping() {
        // Given content smaller than viewport with wrapping
        let total = 13;
        let viewport = 50;

        // When we calculate max_scroll
        let max_scroll = calc_max_scroll(total, viewport, true);

        // Then max_scroll should still allow scrolling to the last line
        assert_eq!(max_scroll, 12); // total - 1
    }

    #[test]
    fn should_handle_empty_content() {
        // Given no content (0 lines)
        let total = 0;
        let viewport = 20;

        // When we calculate max_scroll
        let max_scroll_no_wrap = calc_max_scroll(total, viewport, false);
        let max_scroll_wrap = calc_max_scroll(total, viewport, true);

        // Then both should be 0
        assert_eq!(max_scroll_no_wrap, 0);
        assert_eq!(max_scroll_wrap, 0);
    }

    #[test]
    fn should_handle_zero_viewport() {
        // Given content with viewport of 0 (edge case)
        let total = 100;
        let viewport = 0;

        // When we calculate max_scroll (viewport.max(1) makes it 1)
        let max_scroll_no_wrap = calc_max_scroll(total, viewport, false);
        let max_scroll_wrap = calc_max_scroll(total, viewport, true);

        // Then no_wrap should be total - 1, wrap should be total - 1
        assert_eq!(max_scroll_no_wrap, 99); // total - 1 (since viewport becomes 1)
        assert_eq!(max_scroll_wrap, 99); // total - 1
    }

    #[test]
    fn should_match_max_scroll_offset_implementation() {
        // Verify calc_max_scroll matches the actual implementation
        let diff_state_no_wrap = DiffState {
            viewport_height: 20,
            wrap_lines: false,
            ..Default::default()
        };

        let diff_state_wrap = DiffState {
            viewport_height: 20,
            wrap_lines: true,
            ..Default::default()
        };

        // Test that DiffState defaults match our expectations
        assert!(!diff_state_no_wrap.wrap_lines);
        assert!(diff_state_wrap.wrap_lines);
        assert_eq!(diff_state_no_wrap.viewport_height, 20);
        assert_eq!(diff_state_wrap.viewport_height, 20);
    }
}

#[cfg(test)]
mod find_source_line_tests {
    use super::*;

    // hunk_idx and line_idx are set to 0 because find_source_line doesn't use them;
    // only file_idx and new_lineno matter for the search.
    fn make_diff_line(file_idx: usize, new_lineno: Option<u32>) -> AnnotatedLine {
        AnnotatedLine::DiffLine {
            file_idx,
            hunk_idx: 0,
            line_idx: 0,
            old_lineno: None,
            new_lineno,
        }
    }

    fn make_sbs_line(file_idx: usize, new_lineno: Option<u32>) -> AnnotatedLine {
        AnnotatedLine::SideBySideLine {
            file_idx,
            hunk_idx: 0,
            del_line_idx: None,
            add_line_idx: None,
            old_lineno: None,
            new_lineno,
        }
    }

    #[test]
    fn should_find_exact_match() {
        let annotations = vec![
            AnnotatedLine::FileHeader { file_idx: 0 },
            make_diff_line(0, Some(10)),
            make_diff_line(0, Some(11)),
            make_diff_line(0, Some(12)),
        ];

        let result = find_source_line(&annotations, 0, 11);
        assert_eq!(result, FindSourceLineResult::Exact(2));
    }

    #[test]
    fn should_find_nearest_when_no_exact_match() {
        let annotations = vec![
            make_diff_line(0, Some(10)),
            make_diff_line(0, Some(15)),
            make_diff_line(0, Some(20)),
        ];

        // Target 12 is closest to line 10 (dist=2) vs 15 (dist=3) vs 20 (dist=8)
        let result = find_source_line(&annotations, 0, 12);
        assert_eq!(result, FindSourceLineResult::Nearest(0));
    }

    #[test]
    fn should_find_nearest_above_target() {
        let annotations = vec![
            make_diff_line(0, Some(10)),
            make_diff_line(0, Some(15)),
            make_diff_line(0, Some(20)),
        ];

        // Target 18 is closest to line 20 (dist=2) vs 15 (dist=3) vs 10 (dist=8)
        let result = find_source_line(&annotations, 0, 18);
        assert_eq!(result, FindSourceLineResult::Nearest(2));
    }

    #[test]
    fn should_return_not_found_for_empty_annotations() {
        let annotations: Vec<AnnotatedLine> = vec![];
        let result = find_source_line(&annotations, 0, 42);
        assert_eq!(result, FindSourceLineResult::NotFound);
    }

    #[test]
    fn should_return_not_found_when_no_lines_in_current_file() {
        let annotations = vec![make_diff_line(1, Some(10)), make_diff_line(1, Some(20))];

        // File 0 has no lines
        let result = find_source_line(&annotations, 0, 10);
        assert_eq!(result, FindSourceLineResult::NotFound);
    }

    #[test]
    fn should_skip_lines_from_other_files() {
        let annotations = vec![
            make_diff_line(0, Some(100)), // file 0, line 100
            make_diff_line(1, Some(42)),  // file 1, exact match but wrong file
            make_diff_line(0, Some(50)),  // file 0, line 50
        ];

        // Searching file 0 for line 42 — should find nearest (50, dist=8) not file 1's exact match
        let result = find_source_line(&annotations, 0, 42);
        assert_eq!(result, FindSourceLineResult::Nearest(2));
    }

    #[test]
    fn should_skip_non_diff_line_annotations() {
        let annotations = vec![
            AnnotatedLine::FileHeader { file_idx: 0 },
            AnnotatedLine::HunkHeader {
                file_idx: 0,
                hunk_idx: 0,
            },
            AnnotatedLine::Spacing,
            make_diff_line(0, Some(42)),
        ];

        let result = find_source_line(&annotations, 0, 42);
        assert_eq!(result, FindSourceLineResult::Exact(3));
    }

    #[test]
    fn should_skip_diff_lines_with_no_new_lineno() {
        // Deletion-only lines have new_lineno = None
        let annotations = vec![make_diff_line(0, None), make_diff_line(0, Some(20))];

        let result = find_source_line(&annotations, 0, 5);
        assert_eq!(result, FindSourceLineResult::Nearest(1));
    }

    #[test]
    fn should_work_with_side_by_side_lines() {
        let annotations = vec![
            make_sbs_line(0, Some(10)),
            make_sbs_line(0, Some(20)),
            make_sbs_line(0, Some(30)),
        ];

        let result = find_source_line(&annotations, 0, 20);
        assert_eq!(result, FindSourceLineResult::Exact(1));
    }

    #[test]
    fn should_handle_mixed_diff_and_sbs_lines() {
        let annotations = vec![
            make_diff_line(0, Some(10)),
            make_sbs_line(0, Some(20)),
            make_diff_line(0, Some(30)),
        ];

        let result = find_source_line(&annotations, 0, 25);
        // Nearest is line 20 (dist=5) or line 30 (dist=5), first match wins
        assert_eq!(result, FindSourceLineResult::Nearest(1));
    }

    #[test]
    fn should_return_not_found_when_only_non_line_annotations() {
        let annotations = vec![
            AnnotatedLine::FileHeader { file_idx: 0 },
            AnnotatedLine::Spacing,
            AnnotatedLine::HunkHeader {
                file_idx: 0,
                hunk_idx: 0,
            },
        ];

        let result = find_source_line(&annotations, 0, 42);
        assert_eq!(result, FindSourceLineResult::NotFound);
    }

    #[test]
    fn should_prefer_exact_match_over_earlier_nearest() {
        let annotations = vec![
            make_diff_line(0, Some(41)), // dist=1 from target 42
            make_diff_line(0, Some(42)), // exact match
            make_diff_line(0, Some(43)), // dist=1 from target 42
        ];

        let result = find_source_line(&annotations, 0, 42);
        assert_eq!(result, FindSourceLineResult::Exact(1));
    }

    #[test]
    fn should_find_nearest_for_target_zero() {
        // target_lineno = 0 is out-of-range (lines are 1-indexed) but should
        // still return the nearest line rather than panicking.
        let annotations = vec![make_diff_line(0, Some(1)), make_diff_line(0, Some(5))];

        let result = find_source_line(&annotations, 0, 0);
        assert_eq!(result, FindSourceLineResult::Nearest(0));
    }

    #[test]
    fn should_tie_break_nearest_by_iteration_order() {
        // When two lines are equidistant, the first one encountered wins.
        // Here lines are in descending order; line 30 (idx 0) and line 10 (idx 2)
        // are both dist=10 from target 20, so idx 0 should win.
        let annotations = vec![
            make_diff_line(0, Some(30)),
            make_diff_line(0, Some(50)),
            make_diff_line(0, Some(10)),
        ];

        let result = find_source_line(&annotations, 0, 20);
        assert_eq!(result, FindSourceLineResult::Nearest(0));
    }
}

#[cfg(test)]
mod expand_gap_tests {
    use super::*;
    use std::path::Path;
    use travelagent_core::error::{Result, TrvError};
    use travelagent_core::model::{DiffHunk, DiffLine, FileStatus, LineOrigin, SessionDiffSource};
    use travelagent_core::vcs::VcsType;

    struct MockVcs {
        info: VcsInfo,
        /// Total lines available in the "file" (1-indexed)
        total_lines: u32,
    }

    impl VcsBackend for MockVcs {
        fn info(&self) -> &VcsInfo {
            &self.info
        }

        fn get_working_tree_diff(&self) -> Result<Vec<DiffFile>> {
            Err(TrvError::NoChanges)
        }

        fn fetch_context_lines(
            &self,
            _file_path: &Path,
            _file_status: FileStatus,
            start_line: u32,
            end_line: u32,
        ) -> Result<Vec<DiffLine>> {
            let mut result = Vec::new();
            for line_num in start_line..=end_line.min(self.total_lines) {
                result.push(DiffLine {
                    origin: LineOrigin::Context,
                    content: format!("line {line_num}"),
                    old_lineno: Some(line_num),
                    new_lineno: Some(line_num),
                    highlighted_spans: None,
                });
            }
            Ok(result)
        }
    }

    fn make_hunk(new_start: u32, new_count: u32) -> DiffHunk {
        let mut lines = Vec::new();
        for i in 0..new_count {
            lines.push(DiffLine {
                origin: LineOrigin::Context,
                content: format!("hunk line {}", new_start + i),
                old_lineno: Some(new_start + i),
                new_lineno: Some(new_start + i),
                highlighted_spans: None,
            });
        }
        DiffHunk {
            header: format!("@@ -{new_start},{new_count} +{new_start},{new_count} @@"),
            lines,
            old_start: new_start,
            old_count: new_count,
            new_start,
            new_count,
        }
    }

    fn build_app_with_files(files: Vec<DiffFile>, total_lines: u32) -> App {
        let vcs_info = VcsInfo {
            root_path: PathBuf::from("/tmp"),
            head_commit: "abc123".to_string(),
            branch_name: Some("main".to_string()),
            vcs_type: VcsType::Git,
        };
        let session = travelagent_core::model::ReviewSession::new(
            vcs_info.root_path.clone(),
            vcs_info.head_commit.clone(),
            vcs_info.branch_name.clone(),
            SessionDiffSource::WorkingTree,
        );

        App::build(
            Box::new(MockVcs {
                info: vcs_info.clone(),
                total_lines,
            }),
            vcs_info,
            Theme::dark(),
            None,
            false,
            files,
            session,
            DiffSource::WorkingTree,
            InputMode::Normal,
            Vec::new(),
            None,
            crate::test_support::runtime_handle(),
            AppMode::Local(LocalState::default()),
        )
        .expect("failed to build test app")
    }

    fn make_file_with_hunks(path: &str, hunks: Vec<DiffHunk>) -> DiffFile {
        DiffFile {
            old_path: None,
            new_path: Some(PathBuf::from(path)),
            status: FileStatus::Modified,
            hunks,
            is_binary: false,
            is_too_large: false,
            is_commit_message: false,
        }
    }

    #[test]
    fn should_expand_up_from_first_hunk() {
        // given: file with 50-line gap before first hunk (hunk starts at line 51)
        let file = make_file_with_hunks("test.rs", vec![make_hunk(51, 5)]);
        let mut app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 0,
        };

        // when: expand Up with limit 20 (reveals lines closest to hunk)
        app.expand_gap(gap_id.clone(), ExpandDirection::Up, Some(20))
            .unwrap();

        // then: 20 lines expanded from the bottom of the gap (lines 31-50)
        let content = app.gaps.expanded_bottom.get(&gap_id).unwrap();
        assert_eq!(content.len(), 20);
        assert_eq!(content[0].new_lineno, Some(31));
        assert_eq!(content[19].new_lineno, Some(50));
    }

    #[test]
    fn should_expand_all_lines_with_both_direction() {
        // given: file with 50-line gap before first hunk
        let file = make_file_with_hunks("test.rs", vec![make_hunk(51, 5)]);
        let mut app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 0,
        };

        // when: expand Both (all remaining)
        app.expand_gap(gap_id.clone(), ExpandDirection::Both, None)
            .unwrap();

        // then: all 50 lines in expanded_top
        let content = app.gaps.expanded_top.get(&gap_id).unwrap();
        assert_eq!(content.len(), 50);
        assert_eq!(content[0].new_lineno, Some(1));
        assert_eq!(content[49].new_lineno, Some(50));
    }

    #[test]
    fn should_expand_down_from_upper_hunk() {
        // given: file with two hunks, gap of 24 lines (6..29) between them
        let file = make_file_with_hunks("test.rs", vec![make_hunk(1, 5), make_hunk(30, 5)]);
        let mut app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 1,
        };

        // when: expand Down with limit 10
        app.expand_gap(gap_id.clone(), ExpandDirection::Down, Some(10))
            .unwrap();

        // then: 10 lines from top of gap (lines 6-15)
        let content = app.gaps.expanded_top.get(&gap_id).unwrap();
        assert_eq!(content.len(), 10);
        assert_eq!(content[0].new_lineno, Some(6));
        assert_eq!(content[9].new_lineno, Some(15));
    }

    #[test]
    fn should_expand_up_from_lower_hunk() {
        // given: file with two hunks, gap of 24 lines (6..29) between them
        let file = make_file_with_hunks("test.rs", vec![make_hunk(1, 5), make_hunk(30, 5)]);
        let mut app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 1,
        };

        // when: expand Up with limit 10
        app.expand_gap(gap_id.clone(), ExpandDirection::Up, Some(10))
            .unwrap();

        // then: 10 lines from bottom of gap (lines 20-29)
        let content = app.gaps.expanded_bottom.get(&gap_id).unwrap();
        assert_eq!(content.len(), 10);
        assert_eq!(content[0].new_lineno, Some(20));
        assert_eq!(content[9].new_lineno, Some(29));
    }

    #[test]
    fn should_append_on_subsequent_down_expand() {
        // given: already expanded 20 lines down
        let file = make_file_with_hunks("test.rs", vec![make_hunk(1, 5), make_hunk(50, 5)]);
        let mut app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 1,
        };
        app.expand_gap(gap_id.clone(), ExpandDirection::Down, Some(20))
            .unwrap();

        // when: expand Down 20 more
        app.expand_gap(gap_id.clone(), ExpandDirection::Down, Some(20))
            .unwrap();

        // then: 40 lines total in top
        let content = app.gaps.expanded_top.get(&gap_id).unwrap();
        assert_eq!(content.len(), 40);
        assert_eq!(content[0].new_lineno, Some(6));
        assert_eq!(content[39].new_lineno, Some(45));
    }

    #[test]
    fn should_prepend_on_subsequent_up_expand() {
        // given: already expanded 10 lines up from bottom
        let file = make_file_with_hunks("test.rs", vec![make_hunk(1, 5), make_hunk(50, 5)]);
        let mut app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 1,
        };
        app.expand_gap(gap_id.clone(), ExpandDirection::Up, Some(10))
            .unwrap();

        // when: expand Up 10 more
        app.expand_gap(gap_id.clone(), ExpandDirection::Up, Some(10))
            .unwrap();

        // then: 20 lines total in bottom, in ascending order
        let content = app.gaps.expanded_bottom.get(&gap_id).unwrap();
        assert_eq!(content.len(), 20);
        assert_eq!(content[0].new_lineno, Some(30));
        assert_eq!(content[19].new_lineno, Some(49));
    }

    #[test]
    fn should_cap_at_gap_boundaries() {
        // given: file with 50-line gap, already expanded 40 up
        let file = make_file_with_hunks("test.rs", vec![make_hunk(51, 5)]);
        let mut app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 0,
        };
        app.expand_gap(gap_id.clone(), ExpandDirection::Up, Some(40))
            .unwrap();

        // when: expand Up 20 more (only 10 remain)
        app.expand_gap(gap_id.clone(), ExpandDirection::Up, Some(20))
            .unwrap();

        // then: all 50 lines in bottom
        let content = app.gaps.expanded_bottom.get(&gap_id).unwrap();
        assert_eq!(content.len(), 50);
        assert_eq!(content[0].new_lineno, Some(1));
    }

    #[test]
    fn should_show_up_expander_for_top_of_file_partial() {
        // given: file with 50-line gap, expanded 20 lines up
        let file = make_file_with_hunks("test.rs", vec![make_hunk(51, 5)]);
        let mut app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 0,
        };
        app.expand_gap(gap_id.clone(), ExpandDirection::Up, Some(20))
            .unwrap();

        // then: should have ↑ expander + hidden lines annotation
        let expander_count = app
            .line_annotations
            .iter()
            .filter(|a| matches!(a, AnnotatedLine::Expander { gap_id: g, direction: ExpandDirection::Up } if *g == gap_id))
            .count();
        assert_eq!(expander_count, 1);

        let hidden_count = app
            .line_annotations
            .iter()
            .filter(|a| matches!(a, AnnotatedLine::HiddenLines { gap_id: g, .. } if *g == gap_id))
            .count();
        assert_eq!(hidden_count, 1, "should show hidden lines count");

        let expanded_count = app
            .line_annotations
            .iter()
            .filter(
                |a| matches!(a, AnnotatedLine::ExpandedContext { gap_id: g, .. } if *g == gap_id),
            )
            .count();
        assert_eq!(expanded_count, 20);
    }

    #[test]
    fn should_not_show_expander_when_fully_expanded() {
        // given: file with 50-line gap, fully expanded
        let file = make_file_with_hunks("test.rs", vec![make_hunk(51, 5)]);
        let mut app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 0,
        };
        app.expand_gap(gap_id.clone(), ExpandDirection::Both, None)
            .unwrap();

        // then: no expander or hidden lines
        let expander_count = app
            .line_annotations
            .iter()
            .filter(|a| matches!(a, AnnotatedLine::Expander { gap_id: g, .. } if *g == gap_id))
            .count();
        assert_eq!(expander_count, 0);
    }

    #[test]
    fn should_show_merged_expander_for_small_between_hunk_gap() {
        // given: file with two hunks and a 15-line gap between them
        let file = make_file_with_hunks("test.rs", vec![make_hunk(1, 5), make_hunk(21, 5)]);
        let app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 1,
        };

        // then: should show single ↕ expander (gap=15, < 20)
        let both_count = app
            .line_annotations
            .iter()
            .filter(|a| matches!(a, AnnotatedLine::Expander { gap_id: g, direction: ExpandDirection::Both } if *g == gap_id))
            .count();
        assert_eq!(both_count, 1, "small gap should show merged ↕ expander");
    }

    #[test]
    fn should_show_split_expanders_for_large_between_hunk_gap() {
        // given: file with two hunks and a 30-line gap between them
        let file = make_file_with_hunks("test.rs", vec![make_hunk(1, 5), make_hunk(36, 5)]);
        let app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 1,
        };

        // then: should show ↓ + hidden + ↑
        let down_count = app
            .line_annotations
            .iter()
            .filter(|a| matches!(a, AnnotatedLine::Expander { gap_id: g, direction: ExpandDirection::Down } if *g == gap_id))
            .count();
        let up_count = app
            .line_annotations
            .iter()
            .filter(|a| matches!(a, AnnotatedLine::Expander { gap_id: g, direction: ExpandDirection::Up } if *g == gap_id))
            .count();
        let hidden_count = app
            .line_annotations
            .iter()
            .filter(|a| matches!(a, AnnotatedLine::HiddenLines { gap_id: g, .. } if *g == gap_id))
            .count();
        assert_eq!(down_count, 1);
        assert_eq!(up_count, 1);
        assert_eq!(hidden_count, 1);
    }

    #[test]
    fn should_expand_gap_in_correct_file_not_adjacent_file() {
        // given: two files, each with a gap before the first hunk
        let file0 = make_file_with_hunks("a.rs", vec![make_hunk(31, 5)]);
        let file1 = make_file_with_hunks("b.rs", vec![make_hunk(21, 5)]);
        let mut app = build_app_with_files(vec![file0, file1], 100);

        let gap_id_file1 = GapId {
            file_idx: 1,
            hunk_idx: 0,
        };

        // when: expand gap in file1
        app.expand_gap(gap_id_file1.clone(), ExpandDirection::Up, Some(10))
            .unwrap();

        // then: expanded content is for file1's gap (10 lines from bottom)
        let content = app.gaps.expanded_bottom.get(&gap_id_file1).unwrap();
        assert_eq!(content.len(), 10);
        assert_eq!(content[9].new_lineno, Some(20));

        // and file0's gap should not be expanded
        let gap_id_file0 = GapId {
            file_idx: 0,
            hunk_idx: 0,
        };
        assert!(
            !app.gaps.expanded_top.contains_key(&gap_id_file0)
                && !app.gaps.expanded_bottom.contains_key(&gap_id_file0)
        );
    }

    #[test]
    fn should_noop_when_already_fully_expanded() {
        // given: file with 10-line gap, fully expanded
        let file = make_file_with_hunks("test.rs", vec![make_hunk(11, 5)]);
        let mut app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 0,
        };
        app.expand_gap(gap_id.clone(), ExpandDirection::Both, None)
            .unwrap();
        let len_before = app.gaps.expanded_top.get(&gap_id).unwrap().len();

        // when: try to expand again
        app.expand_gap(gap_id.clone(), ExpandDirection::Up, Some(20))
            .unwrap();

        // then: no change
        let len_after = app.gaps.expanded_top.get(&gap_id).unwrap().len();
        assert_eq!(len_before, len_after);
    }

    #[test]
    fn should_noop_when_limit_is_zero_up() {
        // given: file with 50-line gap, no prior expansion
        let file = make_file_with_hunks("test.rs", vec![make_hunk(51, 5)]);
        let mut app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 0,
        };

        // when: expand Up with limit Some(0)  — must NOT underflow/panic
        app.expand_gap(gap_id.clone(), ExpandDirection::Up, Some(0))
            .unwrap();

        // then: state unchanged (no expanded lines)
        assert!(!app.gaps.expanded_bottom.contains_key(&gap_id));
        assert!(!app.gaps.expanded_top.contains_key(&gap_id));
    }

    #[test]
    fn should_noop_when_limit_is_zero_down() {
        // given: file with gap between two hunks
        let file = make_file_with_hunks("test.rs", vec![make_hunk(1, 5), make_hunk(30, 5)]);
        let mut app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 1,
        };

        // when: expand Down with limit Some(0)  — must NOT underflow/panic
        app.expand_gap(gap_id.clone(), ExpandDirection::Down, Some(0))
            .unwrap();

        // then: state unchanged
        assert!(!app.gaps.expanded_top.contains_key(&gap_id));
        assert!(!app.gaps.expanded_bottom.contains_key(&gap_id));
    }

    #[test]
    fn should_expand_small_gap_fully_even_with_large_limit() {
        // given: file with 5-line gap
        let file = make_file_with_hunks("test.rs", vec![make_hunk(6, 5)]);
        let mut app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 0,
        };

        // when: expand Up with limit 20 (gap is only 5 lines)
        app.expand_gap(gap_id.clone(), ExpandDirection::Up, Some(20))
            .unwrap();

        // then: all 5 lines expanded, no expander remaining
        let content = app.gaps.expanded_bottom.get(&gap_id).unwrap();
        assert_eq!(content.len(), 5);

        let expander_count = app
            .line_annotations
            .iter()
            .filter(|a| matches!(a, AnnotatedLine::Expander { gap_id: g, .. } if *g == gap_id))
            .count();
        assert_eq!(expander_count, 0);
    }

    #[test]
    fn should_merge_to_both_when_remaining_drops_below_batch() {
        // given: 30-line between-hunk gap, expand 20 down => 10 remaining
        let file = make_file_with_hunks("test.rs", vec![make_hunk(1, 5), make_hunk(36, 5)]);
        let mut app = build_app_with_files(vec![file], 100);
        let gap_id = GapId {
            file_idx: 0,
            hunk_idx: 1,
        };
        app.expand_gap(gap_id.clone(), ExpandDirection::Down, Some(20))
            .unwrap();

        // then: remaining=10, should show ↕ merged expander
        let both_count = app
            .line_annotations
            .iter()
            .filter(|a| matches!(a, AnnotatedLine::Expander { gap_id: g, direction: ExpandDirection::Both } if *g == gap_id))
            .count();
        assert_eq!(both_count, 1, "should merge to ↕ when <20 remaining");
    }
}

#[cfg(test)]
mod refresh_remote_tests {
    use super::*;
    use async_trait::async_trait;
    use travelagent_core::error::Result as CoreResult;
    use travelagent_core::forge::{
        ForgeComments, ForgeMerge, ForgeReactions, ForgeRead, ForgeReview, ForgeType, MergeMethod,
        MergeableStatus, NewComment, NewReview, Permissions, PrId, PrListFilter, PrListItem,
        PrMetadata, PrState, ReactionContent, ReactionTarget, RemoteComment, ReviewThread, User,
    };
    use travelagent_core::model::DiffFile;
    use travelagent_core::vcs::CommitInfo;

    /// Minimal ForgeBackend stub that serves canned data. Only the methods
    /// invoked by `App::refresh_remote` return meaningful values; the rest
    /// panic since refresh never calls them.
    struct MockForge {
        metadata: PrMetadata,
        files: Vec<DiffFile>,
        commits: Vec<CommitInfo>,
        comments: Vec<RemoteComment>,
        threads: Vec<ReviewThread>,
    }

    #[async_trait]
    impl ForgeRead for MockForge {
        fn forge_type(&self) -> ForgeType {
            ForgeType::GitHub
        }
        async fn get_pr(&self, _id: &PrId) -> CoreResult<PrMetadata> {
            Ok(self.metadata.clone())
        }
        async fn get_pr_commits(&self, _id: &PrId) -> CoreResult<Vec<CommitInfo>> {
            Ok(self.commits.clone())
        }
        async fn get_pr_files(&self, _id: &PrId) -> CoreResult<Vec<DiffFile>> {
            Ok(self.files.clone())
        }
        async fn get_commit_diff(
            &self,
            _id: &PrId,
            _commit_sha: &str,
        ) -> CoreResult<Vec<DiffFile>> {
            unimplemented!("not used by refresh_remote")
        }
        async fn list_prs(
            &self,
            _owner: &str,
            _repo: &str,
            _filter: &PrListFilter,
        ) -> CoreResult<Vec<PrListItem>> {
            unimplemented!("not used by refresh_remote / forge_required tests")
        }
        async fn current_user(&self) -> CoreResult<User> {
            unimplemented!()
        }
        async fn check_permissions(&self, _id: &PrId) -> CoreResult<Permissions> {
            unimplemented!()
        }
    }

    #[async_trait]
    impl ForgeComments for MockForge {
        async fn get_comments(&self, _id: &PrId) -> CoreResult<Vec<RemoteComment>> {
            Ok(self.comments.clone())
        }
        async fn get_review_threads(&self, _id: &PrId) -> CoreResult<Vec<ReviewThread>> {
            Ok(self.threads.clone())
        }
        async fn post_comment(
            &self,
            _id: &PrId,
            _comment: NewComment,
        ) -> CoreResult<RemoteComment> {
            unimplemented!()
        }
        async fn post_reply(
            &self,
            _id: &PrId,
            _thread_id: &str,
            _body: &str,
        ) -> CoreResult<RemoteComment> {
            unimplemented!()
        }
        async fn edit_comment(
            &self,
            _id: &PrId,
            _comment_id: u64,
            _body: &str,
        ) -> CoreResult<RemoteComment> {
            unimplemented!()
        }
        async fn delete_comment(&self, _id: &PrId, _comment_id: u64) -> CoreResult<()> {
            unimplemented!()
        }
        async fn resolve_thread(&self, _thread_id: &str) -> CoreResult<()> {
            unimplemented!()
        }
        async fn unresolve_thread(&self, _thread_id: &str) -> CoreResult<()> {
            unimplemented!()
        }
    }

    #[async_trait]
    impl ForgeReview for MockForge {
        async fn submit_review(&self, _id: &PrId, _review: NewReview) -> CoreResult<()> {
            unimplemented!()
        }
    }

    #[async_trait]
    impl ForgeMerge for MockForge {
        async fn merge(&self, _id: &PrId, _method: MergeMethod) -> CoreResult<()> {
            unimplemented!()
        }
        async fn close(&self, _id: &PrId) -> CoreResult<()> {
            unimplemented!()
        }
        async fn reopen(&self, _id: &PrId) -> CoreResult<()> {
            unimplemented!()
        }
    }

    #[async_trait]
    impl ForgeReactions for MockForge {
        async fn add_reaction(
            &self,
            _target: &ReactionTarget,
            _content: ReactionContent,
        ) -> CoreResult<()> {
            unimplemented!()
        }
        async fn remove_reaction(
            &self,
            _target: &ReactionTarget,
            _content: ReactionContent,
        ) -> CoreResult<()> {
            unimplemented!()
        }
    }

    fn meta(title: &str, sha: &str) -> PrMetadata {
        PrMetadata {
            title: title.to_string(),
            body: String::new(),
            author: "tester".to_string(),
            state: PrState::Open,
            base_branch: "main".to_string(),
            head_branch: "feature".to_string(),
            head_sha: sha.to_string(),
            created_at: chrono::Utc::now(),
            mergeable: Some(MergeableStatus::Clean),
            is_draft: false,
        }
    }

    #[test]
    fn forge_required_returns_false_and_sets_warning_when_no_forge() {
        // This is the demo-mode guard: without a forge backend, mutating
        // actions (post comment, merge, etc.) must short-circuit and surface
        // a user-facing warning instead of erroring out.
        let mut app = App::new_remote(
            crate::theme::Theme::dark(),
            None,
            false,
            Vec::new(),
            "title".to_string(),
            7,
            "owner",
            "repo",
            crate::test_support::runtime_handle(),
            None,
            PrId {
                owner: "owner".to_string(),
                repo: "repo".to_string(),
                number: 7,
            },
        )
        .expect("new_remote");
        // no forge wired (mirrors demo mode).
        assert!(
            app.remote().expect("remote mode").forge.is_none(),
            "precondition: no forge wired"
        );

        let ok = app.forge_required("post a comment");
        assert!(!ok, "guard must return false when no forge is wired");

        let msg = app.message.as_ref().expect("warning message set");
        assert_eq!(msg.message_type, MessageType::Warning);
        assert!(
            msg.content.contains("post a comment"),
            "warning should include the action verb, got {:?}",
            msg.content
        );
    }

    #[test]
    fn forge_required_returns_true_and_preserves_message_when_forge_present() {
        // Counter-state: with a forge attached, the guard must return true and
        // *not* stomp any existing message on the status bar. Regression guard
        // for the `Some/None` branch in `forge_required`.
        let mut app = App::new_remote(
            crate::theme::Theme::dark(),
            None,
            false,
            Vec::new(),
            "title".to_string(),
            7,
            "owner",
            "repo",
            crate::test_support::runtime_handle(),
            Some(std::sync::Arc::new(MockForge {
                metadata: meta("title", "sha"),
                files: Vec::new(),
                commits: Vec::new(),
                comments: Vec::new(),
                threads: Vec::new(),
            })),
            PrId {
                owner: "owner".to_string(),
                repo: "repo".to_string(),
                number: 7,
            },
        )
        .expect("new_remote");
        app.set_message("pre-existing info");

        let ok = app.forge_required("anything");
        assert!(ok, "guard must return true when forge is attached");
        let msg = app
            .message
            .as_ref()
            .expect("existing info message preserved");
        assert_eq!(msg.message_type, MessageType::Info);
        assert_eq!(msg.content, "pre-existing info");
    }

    #[test]
    fn refresh_remote_replaces_metadata_and_stamps_time() {
        // Build an App without going through App::new (which requires a git
        // repo in the cwd). Use `new_remote` with pre-fetched data since that
        // path doesn't depend on VCS state.
        let theme = crate::theme::Theme::dark();
        let initial_meta = meta("Old title", "sha-initial");
        let mut app = App::new_remote(
            theme,
            None,
            false,
            Vec::new(),
            "Old title".to_string(),
            42,
            "owner",
            "repo",
            crate::test_support::runtime_handle(),
            Some(std::sync::Arc::new(MockForge {
                metadata: meta("New title", "sha-refreshed"),
                files: Vec::new(),
                commits: vec![CommitInfo {
                    id: "c1".to_string(),
                    short_id: "c1".to_string(),
                    branch_name: None,
                    summary: "Refreshed commit".to_string(),
                    body: None,
                    author: "tester".to_string(),
                    time: chrono::Utc::now(),
                }],
                comments: Vec::new(),
                threads: Vec::new(),
            })),
            PrId {
                owner: "owner".to_string(),
                repo: "repo".to_string(),
                number: 42,
            },
        )
        .expect("new_remote");
        {
            let r = app.remote_mut().expect("remote mode");
            r.pr_metadata = Some(initial_meta.clone());
            // Clear last_refreshed_at to verify it gets stamped.
            r.last_refreshed_at = None;
        }
        let before_commits = app.remote().unwrap().pr_commits.len();

        app.refresh_remote().expect("refresh_remote");

        let r = app.remote().expect("remote mode");
        let new_meta = r.pr_metadata.as_ref().expect("metadata present");
        assert_eq!(new_meta.title, "New title");
        assert_eq!(new_meta.head_sha, "sha-refreshed");
        assert_eq!(r.pr_commits.len(), before_commits + 1);
        assert!(r.last_refreshed_at.is_some());
    }

    #[test]
    fn drain_forge_warnings_pushes_into_error_log() {
        // The forge `warn_handler` callback appends into `forge_warn_queue`
        // from an async worker; `drain_forge_warnings` is the main-thread
        // pump that moves those messages into `error_log` so `:errors`
        // can cycle through them. Regression guard for the Phase A wiring.
        let mut app = App::new_remote(
            crate::theme::Theme::dark(),
            None,
            false,
            Vec::new(),
            "title".to_string(),
            7,
            "owner",
            "repo",
            crate::test_support::runtime_handle(),
            None,
            PrId {
                owner: "owner".to_string(),
                repo: "repo".to_string(),
                number: 7,
            },
        )
        .expect("new_remote");

        // Simulate the forge handler enqueueing two pagination warnings.
        app.forge_warn_queue
            .lock()
            .unwrap()
            .push_back("first warning".to_string());
        app.forge_warn_queue
            .lock()
            .unwrap()
            .push_back("second warning".to_string());

        app.drain_forge_warnings();

        // Queue should be empty after drain.
        assert!(app.forge_warn_queue.lock().unwrap().is_empty());
        // Most recent warning is the active status message (set_error
        // semantics — later calls overwrite `message`).
        let msg = app.message.as_ref().expect("status message set");
        assert_eq!(msg.message_type, MessageType::Error);
        assert_eq!(msg.content, "second warning");
        // Both messages landed in the error-log ring.
        let history: Vec<String> = app
            .error_log
            .iter_newest_first()
            .map(|m| m.content.clone())
            .collect();
        assert!(
            history.iter().any(|c| c == "first warning"),
            "first warning preserved in error_log ring, got {history:?}"
        );
        assert!(
            history.iter().any(|c| c == "second warning"),
            "second warning preserved in error_log ring, got {history:?}"
        );
    }

    #[test]
    fn drain_forge_warnings_is_noop_when_empty() {
        let mut app = App::new_remote(
            crate::theme::Theme::dark(),
            None,
            false,
            Vec::new(),
            "title".to_string(),
            7,
            "owner",
            "repo",
            crate::test_support::runtime_handle(),
            None,
            PrId {
                owner: "owner".to_string(),
                repo: "repo".to_string(),
                number: 7,
            },
        )
        .expect("new_remote");
        app.set_message("pre-existing info");

        app.drain_forge_warnings();

        // Empty queue must not clobber an existing info message.
        let msg = app.message.as_ref().expect("message preserved");
        assert_eq!(msg.content, "pre-existing info");
    }
}

#[cfg(test)]
mod jump_to_bottom_tests {
    use super::*;

    #[test]
    fn jump_to_bottom_lands_on_last_content_line_not_spacing() {
        // Build annotations that end with a Spacing line (like real diff output)
        let annotations = [
            AnnotatedLine::ReviewCommentsHeader,
            AnnotatedLine::FileHeader { file_idx: 0 },
            AnnotatedLine::HunkHeader {
                file_idx: 0,
                hunk_idx: 0,
            },
            AnnotatedLine::DiffLine {
                file_idx: 0,
                hunk_idx: 0,
                line_idx: 0,
                old_lineno: None,
                new_lineno: Some(1),
            },
            AnnotatedLine::DiffLine {
                file_idx: 0,
                hunk_idx: 0,
                line_idx: 1,
                old_lineno: None,
                new_lineno: Some(2),
            },
            AnnotatedLine::Spacing, // trailing spacing line
        ];

        // Simulate jump_to_bottom logic: find last non-spacing line
        let total = annotations.len();
        let mut target = total.saturating_sub(1);
        while target > 0 {
            if !matches!(annotations.get(target), Some(AnnotatedLine::Spacing)) {
                break;
            }
            target -= 1;
        }

        // Should land on the last DiffLine (index 4), not the Spacing (index 5)
        assert_eq!(target, 4);
        assert!(matches!(
            annotations[target],
            AnnotatedLine::DiffLine {
                new_lineno: Some(2),
                ..
            }
        ));
    }
}