marver 0.0.28

//! Reviewing and committing what an agent produced.

use std::path::PathBuf;

use chrono::{DateTime, Utc};

use crate::agent;
use crate::domain::{Task, TaskState};
use crate::git;
use crate::store::{Store, Transition};
use crate::tmux::{self, Tmux};

#[derive(Debug, thiserror::Error)]
pub enum Error {
    #[error(transparent)]
    Git(#[from] git::Error),
    #[error(transparent)]
    Store(#[from] crate::store::Error),
    #[error(transparent)]
    Tmux(#[from] tmux::Error),
    #[error("task {0} has no worktrees to review")]
    NotProvisioned(i64),
    #[error("nothing is staged")]
    NothingStaged,
    #[error("{path} has an unresolved conflict in {}", worktree.display())]
    Unmerged { worktree: PathBuf, path: String },
    #[error("a commit message is required")]
    EmptyMessage,
    #[error(
        "{} could not be committed ({source}); the rest were put back{}",
        worktree.display(),
        stranded_note(stranded)
    )]
    PartialCommit {
        worktree: PathBuf,
        #[source]
        source: git::Error,
        /// Worktrees whose commit would not undo, so they really are committed.
        stranded: Vec<PathBuf>,
    },
    #[error(
        "the task became {from} while it was being committed, so nothing was kept{}",
        stranded_note(stranded)
    )]
    MovedWhileCommitting {
        from: TaskState,
        /// Worktrees whose commit would not undo, so they really are committed.
        stranded: Vec<PathBuf>,
    },
    #[error("task {id} is {state}, not awaiting review")]
    NotAwaitingReview { id: i64, state: TaskState },
    #[error("follow-up instructions are required")]
    EmptyInstructions,
    #[error("instructions cannot contain control characters")]
    ControlCharacter,
    #[error("task {task} has no live session {session} to resume")]
    NoSession { task: i64, session: String },
    #[error(transparent)]
    Agent(#[from] agent::Error),
}

pub type Result<T> = std::result::Result<T, Error>;

/// Names the repos an unwind could not take back, so a partial commit is never
/// reported as a clean one.
fn stranded_note(stranded: &[PathBuf]) -> String {
    if stranded.is_empty() {
        return String::new();
    }
    let names: Vec<String> = stranded
        .iter()
        .map(|path| path.display().to_string())
        .collect();
    format!(", except {} which stayed committed", names.join(", "))
}

/// What happened to a file.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Change {
    Added,
    Modified,
    Deleted,
    Renamed,
    Copied,
    /// Not yet known to git at all.
    Untracked,
    /// A status this version does not model.
    Other,
}

impl Change {
    fn from_status(entry: &git::StatusEntry) -> Self {
        if entry.is_untracked() {
            return Self::Untracked;
        }
        // The index character wins when both are set: it is what a commit
        // would actually record.
        let code = if entry.index != ' ' {
            entry.index
        } else {
            entry.worktree
        };
        match code {
            'A' => Self::Added,
            'M' => Self::Modified,
            'D' => Self::Deleted,
            'R' => Self::Renamed,
            'C' => Self::Copied,
            _ => Self::Other,
        }
    }
}

/// One changed file within one of a task's worktrees.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct FileChange {
    pub repo_id: i64,
    /// Which worktree it lives in.
    pub worktree: PathBuf,
    /// Path relative to the worktree root.
    pub path: String,
    pub change: Change,
    pub staged: bool,
    pub unstaged: bool,
    /// Previous path, for renames.
    pub original: Option<String>,
}

impl FileChange {
    /// Absolute path on disk.
    pub fn absolute(&self) -> PathBuf {
        self.worktree.join(&self.path)
    }
}

/// Everything a task changed, across every repo it targets.
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct Review {
    pub files: Vec<FileChange>,
    /// How many of the task's repos actually had a worktree to look at.
    pub worktrees: usize,
}

impl Review {
    pub fn is_empty(&self) -> bool {
        self.files.is_empty()
    }

    /// Whether there is anything to review at all yet.
    pub fn is_provisioned(&self) -> bool {
        self.worktrees > 0
    }

    pub fn staged(&self) -> impl Iterator<Item = &FileChange> {
        self.files.iter().filter(|f| f.staged)
    }

    pub fn has_staged(&self) -> bool {
        self.files.iter().any(|f| f.staged)
    }
}

/// One line of a unified diff.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum DiffLine {
    Context(String),
    Added(String),
    Removed(String),
    /// `\ No newline at end of file` and similar annotations.
    Note(String),
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Hunk {
    /// The `@@ ... @@` line, verbatim.
    pub header: String,
    pub lines: Vec<DiffLine>,
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct FileDiff {
    pub path: String,
    pub hunks: Vec<Hunk>,
    /// Git reported the file as binary; there is nothing to show.
    pub binary: bool,
    /// An unmerged file, shown as a combined diff against both parents.
    pub conflicted: bool,
}

impl FileDiff {
    pub fn is_empty(&self) -> bool {
        self.hunks.is_empty() && !self.binary
    }
}

/// Parse unified diff text into per-file hunks.
pub fn parse_diff(text: &str) -> Vec<FileDiff> {
    let mut files: Vec<FileDiff> = Vec::new();
    // How many marker columns the current hunk's lines carry.
    let mut markers = 1usize;

    for line in text.lines() {
        if let Some(rest) = line.strip_prefix("diff --git ") {
            markers = 1;
            files.push(FileDiff {
                path: path_from_diff_header(rest),
                hunks: Vec::new(),
                binary: false,
                conflicted: false,
            });
            continue;
        }
        // What git emits for an unmerged file.
        if let Some(rest) = line
            .strip_prefix("diff --cc ")
            .or_else(|| line.strip_prefix("diff --combined "))
        {
            markers = 2;
            files.push(FileDiff {
                path: rest.trim().to_string(),
                hunks: Vec::new(),
                binary: false,
                conflicted: true,
            });
            continue;
        }

        let Some(file) = files.last_mut() else {
            continue;
        };

        if line.starts_with("Binary files ") || line.starts_with("GIT binary patch") {
            file.binary = true;
            continue;
        }
        if line.starts_with("@@") {
            // `@@ ...
            markers = line.chars().take_while(|&c| c == '@').count().max(2) - 1;
            file.hunks.push(Hunk {
                header: line.to_string(),
                lines: Vec::new(),
            });
            continue;
        }

        let Some(hunk) = file.hunks.last_mut() else {
            // Header noise between files: index, ---, +++, mode changes.
            continue;
        };
        if line.is_empty() {
            // A completely empty line inside a hunk is an empty context line.
            hunk.lines.push(DiffLine::Context(String::new()));
            continue;
        }
        if line.starts_with('\\') {
            hunk.lines.push(DiffLine::Note(line.to_string()));
            continue;
        }

        // By chars, not bytes.
        let prefix: String = line.chars().take(markers).collect();
        let body: String = line.chars().skip(markers).collect();
        hunk.lines.push(if prefix.contains('+') {
            DiffLine::Added(body)
        } else if prefix.contains('-') {
            DiffLine::Removed(body)
        } else if prefix.chars().all(|c| c == ' ') {
            DiffLine::Context(body)
        } else {
            DiffLine::Context(line.to_string())
        });
    }

    files
}

/// Pull the path out of `a/some/file b/some/file`.
fn path_from_diff_header(rest: &str) -> String {
    if let Some(index) = rest.find(" b/") {
        return rest[index + 3..].to_string();
    }
    rest.strip_prefix("a/").unwrap_or(rest).to_string()
}

/// What a commit produced, per repo.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct RepoCommit {
    pub repo_id: i64,
    pub worktree: PathBuf,
    /// Short hash of the new commit.
    pub commit: String,
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Committed {
    pub commits: Vec<RepoCommit>,
}

pub struct Reviewer;

impl Reviewer {
    /// Every change across a task's worktrees.
    pub fn review(store: &Store, task: &Task) -> Result<Review> {
        let mut files = Vec::new();
        let mut worktrees = 0;
        for link in store.list_task_repos(task.id)? {
            let Some(worktree) = link.worktree_path.clone() else {
                continue;
            };
            worktrees += 1;
            for entry in git::status(&worktree)? {
                files.push(FileChange {
                    repo_id: link.repo_id,
                    worktree: worktree.clone(),
                    change: Change::from_status(&entry),
                    staged: entry.is_staged(),
                    unstaged: entry.is_unstaged(),
                    original: entry.original.clone(),
                    path: entry.path,
                });
            }
        }
        // Stable ordering so a redraw does not shuffle the list under the
        // user's cursor.
        files.sort_by(|a, b| (&a.worktree, &a.path).cmp(&(&b.worktree, &b.path)));
        Ok(Review { files, worktrees })
    }

    /// The diff for one file, parsed for display.
    pub fn file_diff(file: &FileChange, staged: bool) -> Result<FileDiff> {
        let text = if file.change == Change::Untracked {
            git::diff_untracked(&file.worktree, &file.path)?
        } else {
            git::diff(&file.worktree, Some(&file.path), staged)?
        };
        Ok(parse_diff(&text)
            .into_iter()
            .next()
            .unwrap_or_else(|| FileDiff {
                path: file.path.clone(),
                hunks: Vec::new(),
                binary: false,
                conflicted: false,
            }))
    }

    pub fn stage(file: &FileChange) -> Result<()> {
        git::stage(&file.worktree, &file.path)?;
        Ok(())
    }

    pub fn unstage(file: &FileChange) -> Result<()> {
        git::unstage(&file.worktree, &file.path)?;
        Ok(())
    }

    /// Stage everything in every provisioned worktree.
    pub fn stage_all(store: &Store, task: &Task) -> Result<()> {
        for worktree in Self::worktrees(store, task)? {
            git::stage_all(&worktree)?;
        }
        Ok(())
    }

    /// Commit the staged changes in every worktree that has any, then move the
    /// task to `committed`.
    pub fn commit(
        store: &mut Store,
        task: &Task,
        message: &str,
        now: DateTime<Utc>,
    ) -> Result<Committed> {
        if message.trim().is_empty() {
            return Err(Error::EmptyMessage);
        }
        // Checked here, against a fresh read, rather than relying on the
        // transition at the end.
        Self::require_awaiting_review(store, task.id)?;

        let links = store.list_task_repos(task.id)?;
        let mut pending = Vec::new();
        for link in &links {
            let Some(worktree) = link.worktree_path.clone() else {
                continue;
            };
            // An unmerged file counts as staged to `has_staged_changes`, so
            // the commit was offered and then died on a raw `fatal: Exiting
            // because of an unresolved conflict`.
            if let Some(entry) = git::status(&worktree)?.iter().find(|e| e.is_unmerged()) {
                return Err(Error::Unmerged {
                    worktree,
                    path: entry.path.clone(),
                });
            }
            if git::has_staged_changes(&worktree)? {
                pending.push((link.repo_id, worktree));
            }
        }
        if pending.is_empty() {
            return Err(Error::NothingStaged);
        }

        // Git cannot commit across repositories in one go, so the next best
        // thing is that a failure leaves none of them committed — `--soft`
        // undoes a commit without touching the index or the working tree.
        let mut commits = Vec::new();
        for (repo_id, worktree) in pending {
            match git::commit(&worktree, message) {
                Ok(hash) => commits.push(RepoCommit {
                    repo_id,
                    worktree,
                    commit: hash,
                }),
                Err(err) => {
                    let stranded = Self::unwind(&commits);
                    return Err(Error::PartialCommit {
                        worktree,
                        source: err,
                        stranded,
                    });
                }
            }
        }

        // The same treatment a repo that would not commit gets. The state can
        // legally move while the commits are being made — the daemon reaping a
        // cancel, a hook landing — and writing them and then failing to record
        // it left the work committed to branches with the task in a state that
        // has no key to finish it. `--soft` puts every repo back exactly where
        // it was, so the refusal costs nothing but the message.
        if let Err(err) =
            store.transition_from(task.id, TaskState::AwaitingReview, TaskState::Committed, Transition::Plain, now)
        {
            let stranded = Self::unwind(&commits);
            return Err(match err {
                crate::store::Error::IllegalTransition { from, .. } => {
                    Error::MovedWhileCommitting { from, stranded }
                }
                other => other.into(),
            });
        }
        Ok(Committed { commits })
    }

    /// Send follow-up instructions to a task's agent and put it back to work.
    pub fn reject(
        store: &mut Store,
        tmux: &Tmux,
        task: &Task,
        instructions: &str,
        now: DateTime<Utc>,
    ) -> Result<()> {
        if instructions.trim().is_empty() {
            return Err(Error::EmptyInstructions);
        }
        if instructions.chars().any(char::is_control) {
            return Err(Error::ControlCharacter);
        }
        // Before typing, for the same reason as in `commit`: the keystrokes
        // cannot be taken back, and a cancelled task's session is about to be
        // killed by the daemon's next reap.
        Self::require_awaiting_review(store, task.id)?;

        // Finding the pane and typing into it is `agent`'s job, so that the
        // three places marver puts words in front of an agent — this, resuming
        // a paused task, sending it a todo — cannot disagree about how.
        agent::say(tmux, task, instructions).map_err(|err| match err {
            agent::Error::NoSession { task, session } => Error::NoSession { task, session },
            other => Error::Agent(other),
        })?;

        // Recorded after the send, and tolerant of already being there: typing
        // at the agent makes it emit `UserPromptSubmit`, so the daemon may have
        // moved the task first.
        Self::mark_running(store, task.id, now)
    }

    /// Put a task back to `running`, accepting that it may already be there.
    fn mark_running(store: &mut Store, id: i64, now: DateTime<Utc>) -> Result<()> {
        match store.transition(id, TaskState::Running, Transition::Plain, now) {
            Ok(_) => Ok(()),
            Err(crate::store::Error::IllegalTransition {
                from: TaskState::Running,
                ..
            }) => Ok(()),
            Err(err) => Err(err.into()),
        }
    }

    /// Refuse if the task has moved on since the screen read it.
    fn require_awaiting_review(store: &Store, id: i64) -> Result<()> {
        let state = store.get_task(id)?.state;
        if state != TaskState::AwaitingReview {
            return Err(Error::NotAwaitingReview { id, state });
        }
        Ok(())
    }

    /// Take back the commits already made, so a task commits all or nothing.
    /// Returns any that would not undo — those are left committed, and the
    /// caller has to say so.
    fn unwind(commits: &[RepoCommit]) -> Vec<PathBuf> {
        commits
            .iter()
            .filter(|made| git::uncommit(&made.worktree).is_err())
            .map(|made| made.worktree.clone())
            .collect()
    }

    fn worktrees(store: &Store, task: &Task) -> Result<Vec<PathBuf>> {
        let paths: Vec<PathBuf> = store
            .list_task_repos(task.id)?
            .into_iter()
            .filter_map(|link| link.worktree_path)
            .collect();
        if paths.is_empty() {
            return Err(Error::NotProvisioned(task.id));
        }
        Ok(paths)
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::domain::Repo;
    use crate::git::testing::init_repo;
    use crate::tmux::testing::TestServer;
    use std::ffi::OsString;
    use tempfile::TempDir;

    fn at(secs: i64) -> DateTime<Utc> {
        DateTime::from_timestamp(secs, 0).expect("valid timestamp")
    }

    struct Fixture {
        _tmp: TempDir,
        repos_dir: PathBuf,
        tasks_dir: PathBuf,
        store: Store,
    }

    impl Fixture {
        fn new() -> Self {
            let tmp = TempDir::new().unwrap();
            let repos_dir = tmp.path().join("repos");
            let tasks_dir = tmp.path().join("tasks");
            std::fs::create_dir_all(&repos_dir).unwrap();
            Self {
                repos_dir,
                tasks_dir,
                store: Store::open_in_memory().unwrap(),
                _tmp: tmp,
            }
        }

        fn repo(&self, name: &str) -> Repo {
            let path = self.repos_dir.join(name);
            init_repo(&path, "main");
            self.store.upsert_repo(&path, name, at(0)).unwrap()
        }

        /// A task with a worktree per repo, walked to `awaiting-review`.
        fn task_with_worktrees(&mut self, repos: &[Repo]) -> Task {
            let ids: Vec<i64> = repos.iter().map(|r| r.id).collect();
            let task = self
                .store
                .create_task("Fix it", "p", &self.tasks_dir, &ids, at(0))
                .unwrap();

            for repo in repos {
                let worktree = task.workspace_dir.join(&repo.name);
                std::fs::create_dir_all(&task.workspace_dir).unwrap();
                git::worktree_add(&repo.path, &worktree, &format!("t{}", task.id), "main").unwrap();
                self.store
                    .record_worktree(
                        task.id,
                        repo.id,
                        &worktree,
                        &format!("t{}", task.id),
                        "main",
                    )
                    .unwrap();
            }

            self.store
                .transition(task.id, TaskState::Running, Transition::Plain, at(1))
                .unwrap();
            self.store
                .transition(task.id, TaskState::AwaitingReview, Transition::Plain, at(2))
                .unwrap();
            self.store.get_task(task.id).unwrap()
        }

        fn worktree(&self, task: &Task, repo: &Repo) -> PathBuf {
            task.workspace_dir.join(&repo.name)
        }
    }

    // ---- diff parsing ----

    #[test]
    fn parses_a_simple_diff() {
        let text = "diff --git a/src/main.rs b/src/main.rs\n\
                    index 83db48f..bf269f4 100644\n\
                    --- a/src/main.rs\n\
                    +++ b/src/main.rs\n\
                    @@ -1,3 +1,4 @@\n\
                     fn main() {\n\
                    -    old();\n\
                    +    new();\n\
                    +    extra();\n\
                     }\n";
        let files = parse_diff(text);

        assert_eq!(files.len(), 1);
        assert_eq!(files[0].path, "src/main.rs");
        assert_eq!(files[0].hunks.len(), 1);
        assert_eq!(files[0].hunks[0].header, "@@ -1,3 +1,4 @@");
        assert_eq!(
            files[0].hunks[0].lines,
            [
                DiffLine::Context("fn main() {".into()),
                DiffLine::Removed("    old();".into()),
                DiffLine::Added("    new();".into()),
                DiffLine::Added("    extra();".into()),
                DiffLine::Context("}".into()),
            ]
        );
    }

    #[test]
    fn parses_several_files_and_hunks() {
        let text = "diff --git a/a.txt b/a.txt\n\
                    @@ -1 +1 @@\n\
                    -one\n\
                    +ONE\n\
                    @@ -10 +10 @@\n\
                    -ten\n\
                    +TEN\n\
                    diff --git a/b.txt b/b.txt\n\
                    @@ -1 +1 @@\n\
                    -two\n\
                    +TWO\n";
        let files = parse_diff(text);
        assert_eq!(files.len(), 2);
        assert_eq!(files[0].hunks.len(), 2);
        assert_eq!(files[1].path, "b.txt");
    }

    #[test]
    fn a_rename_reports_where_the_file_ended_up() {
        let files = parse_diff("diff --git a/old/name.rs b/new/name.rs\n@@ -1 +1 @@\n x\n");
        assert_eq!(files[0].path, "new/name.rs");
    }

    #[test]
    fn a_conflicted_file_parses_as_a_combined_diff() {
        // Verbatim from a real merge conflict.
        let text = "diff --cc c.txt\n\
                    index ba2906d,e45c9c2..0000000\n\
                    --- a/c.txt\n\
                    +++ b/c.txt\n\
                    @@@ -1,1 -1,1 +1,5 @@@\n\
                    ++<<<<<<< HEAD\n\
                    \x20+main\n\
                    ++=======\n\
                    + other\n\
                    ++>>>>>>> other\n";
        let files = parse_diff(text);

        assert_eq!(files.len(), 1);
        assert_eq!(files[0].path, "c.txt");
        assert!(files[0].conflicted);
        assert!(!files[0].is_empty(), "a conflict is not an empty diff");
        // Two marker columns, so the body starts after both.
        assert_eq!(
            files[0].hunks[0].lines,
            [
                DiffLine::Added("<<<<<<< HEAD".into()),
                DiffLine::Added("main".into()),
                DiffLine::Added("=======".into()),
                DiffLine::Added("other".into()),
                DiffLine::Added(">>>>>>> other".into()),
            ]
        );
    }

    #[test]
    fn a_multibyte_leading_character_does_not_panic() {
        // `line[1..]` was a byte slice, so any line beginning with a
        // multi-byte character crashed the whole TUI.
        let files = parse_diff("diff --git a/a.txt b/a.txt\n@@ -1 +1 @@\nédge\n");
        assert_eq!(files[0].hunks[0].lines.len(), 1);
    }

    #[test]
    fn binary_files_are_flagged_rather_than_parsed() {
        let text = "diff --git a/logo.png b/logo.png\n\
                    Binary files a/logo.png and b/logo.png differ\n";
        let files = parse_diff(text);
        assert!(files[0].binary);
        assert!(files[0].hunks.is_empty());
    }

    #[test]
    fn a_missing_trailing_newline_is_kept_as_a_note() {
        let text = "diff --git a/a b/a\n@@ -1 +1 @@\n-x\n\\ No newline at end of file\n+y\n";
        let lines = &parse_diff(text)[0].hunks[0].lines;
        assert!(matches!(lines[1], DiffLine::Note(_)));
    }

    #[test]
    fn an_empty_context_line_is_not_dropped() {
        // git emits a bare "" for a blank context line rather than " ".
        let text = "diff --git a/a b/a\n@@ -1,3 +1,3 @@\n one\n\n+two\n";
        let lines = &parse_diff(text)[0].hunks[0].lines;
        assert_eq!(lines[1], DiffLine::Context(String::new()));
        assert_eq!(lines[2], DiffLine::Added("two".into()));
    }

    #[test]
    fn header_noise_between_files_is_ignored() {
        let text = "diff --git a/a b/a\n\
                    old mode 100644\n\
                    new mode 100755\n\
                    similarity index 95%\n\
                    index 1234567..89abcde 100644\n\
                    --- a/a\n\
                    +++ b/a\n\
                    @@ -1 +1 @@\n\
                    -x\n\
                    +y\n";
        let files = parse_diff(text);
        assert_eq!(files[0].hunks.len(), 1);
        assert_eq!(files[0].hunks[0].lines.len(), 2, "only the +/- lines");
    }

    #[test]
    fn empty_input_yields_nothing() {
        assert!(parse_diff("").is_empty());
    }

    // ---- review against real repos ----

    #[test]
    fn an_untouched_worktree_has_nothing_to_review() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        assert!(Reviewer::review(&fx.store, &task).unwrap().is_empty());
    }

    #[test]
    fn modified_and_new_files_both_appear() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        let wt = fx.worktree(&task, &repo);

        std::fs::write(wt.join("README.md"), "changed\n").unwrap();
        std::fs::write(wt.join("new.rs"), "fn new() {}\n").unwrap();

        let review = Reviewer::review(&fx.store, &task).unwrap();
        let paths: Vec<&str> = review.files.iter().map(|f| f.path.as_str()).collect();
        assert_eq!(paths, ["README.md", "new.rs"]);
        assert_eq!(review.files[0].change, Change::Modified);
        assert_eq!(
            review.files[1].change,
            Change::Untracked,
            "a file git has never seen is most of what an agent produces"
        );
    }

    #[test]
    fn staging_moves_a_file_from_unstaged_to_staged() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        let wt = fx.worktree(&task, &repo);
        std::fs::write(wt.join("README.md"), "changed\n").unwrap();

        let before = Reviewer::review(&fx.store, &task).unwrap();
        assert!(!before.files[0].staged && before.files[0].unstaged);

        Reviewer::stage(&before.files[0]).unwrap();

        let after = Reviewer::review(&fx.store, &task).unwrap();
        assert!(after.files[0].staged);
        assert!(after.has_staged());
    }

    #[test]
    fn unstaging_puts_it_back() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        let wt = fx.worktree(&task, &repo);
        std::fs::write(wt.join("README.md"), "changed\n").unwrap();

        let review = Reviewer::review(&fx.store, &task).unwrap();
        Reviewer::stage(&review.files[0]).unwrap();
        let staged = Reviewer::review(&fx.store, &task).unwrap();
        Reviewer::unstage(&staged.files[0]).unwrap();

        let after = Reviewer::review(&fx.store, &task).unwrap();
        assert!(!after.files[0].staged);
        assert!(!after.has_staged());
    }

    #[test]
    fn an_untracked_file_still_has_a_viewable_diff() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        let wt = fx.worktree(&task, &repo);
        std::fs::write(wt.join("new.rs"), "fn added() {}\n").unwrap();

        let review = Reviewer::review(&fx.store, &task).unwrap();
        let diff = Reviewer::file_diff(&review.files[0], false).unwrap();

        assert!(
            diff.hunks
                .iter()
                .any(|h| h.lines.contains(&DiffLine::Added("fn added() {}".into()))),
            "git diff alone would show nothing here: {diff:?}"
        );
    }

    #[test]
    fn a_modified_file_diffs_against_the_index_or_the_tree() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        let wt = fx.worktree(&task, &repo);
        std::fs::write(wt.join("README.md"), "# changed\n").unwrap();

        let review = Reviewer::review(&fx.store, &task).unwrap();
        let unstaged = Reviewer::file_diff(&review.files[0], false).unwrap();
        assert!(!unstaged.hunks.is_empty(), "the working tree differs");

        let staged = Reviewer::file_diff(&review.files[0], true).unwrap();
        assert!(staged.is_empty(), "nothing is staged yet");
    }

    #[test]
    fn committing_records_a_commit_and_finishes_the_task() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        let wt = fx.worktree(&task, &repo);
        std::fs::write(wt.join("README.md"), "changed\n").unwrap();
        Reviewer::stage_all(&fx.store, &task).unwrap();

        let committed = Reviewer::commit(&mut fx.store, &task, "fix the thing", at(5)).unwrap();

        assert_eq!(committed.commits.len(), 1);
        assert!(!committed.commits[0].commit.is_empty());
        assert_eq!(
            fx.store.get_task(task.id).unwrap().state,
            TaskState::Committed
        );
        // The commit is real.
        let log = git::run(&wt, &["log", "-1", "--pretty=%s"]).unwrap();
        assert_eq!(log, "fix the thing");
        assert!(Reviewer::review(&fx.store, &task).unwrap().is_empty());
    }

    #[test]
    fn committing_a_conflicted_worktree_is_refused_before_anything_lands() {
        // An unmerged file counts as staged to `has_staged_changes`, so commit
        // was offered and then died on a raw `fatal: Exiting because of an
        // unresolved conflict` with no explanation the user could act on.
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        let wt = fx.worktree(&task, &repo);

        // Produce a real conflict in the worktree.
        std::fs::write(wt.join("c.txt"), "base\n").unwrap();
        git::stage_all(&wt).unwrap();
        git::commit(&wt, "seed").unwrap();
        git::run(&wt, &["checkout", "-q", "-b", "side"]).unwrap();
        std::fs::write(wt.join("c.txt"), "side\n").unwrap();
        git::stage_all(&wt).unwrap();
        git::commit(&wt, "side").unwrap();
        git::run(&wt, &["checkout", "-q", &format!("t{}", task.id)]).unwrap();
        std::fs::write(wt.join("c.txt"), "mine\n").unwrap();
        git::stage_all(&wt).unwrap();
        git::commit(&wt, "mine").unwrap();
        assert!(
            git::run(&wt, &["merge", "side"]).is_err(),
            "the merge should have conflicted"
        );

        let err = Reviewer::commit(&mut fx.store, &task, "resolve it", at(5)).unwrap_err();

        assert!(matches!(err, Error::Unmerged { .. }), "got {err:?}");
        assert_eq!(
            fx.store.get_task(task.id).unwrap().state,
            TaskState::AwaitingReview,
            "a refused commit must not move the task"
        );

        // And the conflict is visible rather than a blank diff.
        let review = Reviewer::review(&fx.store, &task).unwrap();
        let file = review.files.iter().find(|f| f.path == "c.txt").unwrap();
        let diff = Reviewer::file_diff(file, false).unwrap();
        assert!(diff.conflicted, "{diff:?}");
        assert!(!diff.is_empty(), "the conflict must be shown: {diff:?}");
    }

    #[test]
    fn committing_with_nothing_staged_is_refused() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        std::fs::write(fx.worktree(&task, &repo).join("README.md"), "changed\n").unwrap();

        assert!(matches!(
            Reviewer::commit(&mut fx.store, &task, "msg", at(5)),
            Err(Error::NothingStaged)
        ));
        assert_eq!(
            fx.store.get_task(task.id).unwrap().state,
            TaskState::AwaitingReview,
            "a refused commit must not finish the task"
        );
    }

    #[test]
    fn an_empty_message_is_refused() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        std::fs::write(fx.worktree(&task, &repo).join("README.md"), "x\n").unwrap();
        Reviewer::stage_all(&fx.store, &task).unwrap();

        assert!(matches!(
            Reviewer::commit(&mut fx.store, &task, "   ", at(5)),
            Err(Error::EmptyMessage)
        ));
    }

    #[test]
    fn a_multi_repo_task_commits_once_per_repo_with_one_message() {
        let mut fx = Fixture::new();
        let api = fx.repo("api");
        let web = fx.repo("web");
        let task = fx.task_with_worktrees(&[api.clone(), web.clone()]);

        std::fs::write(fx.worktree(&task, &api).join("README.md"), "api\n").unwrap();
        std::fs::write(fx.worktree(&task, &web).join("README.md"), "web\n").unwrap();
        Reviewer::stage_all(&fx.store, &task).unwrap();

        let committed = Reviewer::commit(&mut fx.store, &task, "cross-cutting", at(5)).unwrap();

        assert_eq!(committed.commits.len(), 2);
        for repo in [&api, &web] {
            let log = git::run(&fx.worktree(&task, repo), &["log", "-1", "--pretty=%s"]).unwrap();
            assert_eq!(log, "cross-cutting");
        }
    }

    #[test]
    fn one_repo_failing_to_commit_leaves_none_of_them_committed() {
        // Git cannot commit across repos in one go, so the guarantee is that
        // a failure is undone: half a cross-cutting change committed is worse
        // than none of it, because the second half is what says it is safe.
        let mut fx = Fixture::new();
        let api = fx.repo("api");
        let web = fx.repo("web");
        let task = fx.task_with_worktrees(&[api.clone(), web.clone()]);

        std::fs::write(fx.worktree(&task, &api).join("README.md"), "api\n").unwrap();
        std::fs::write(fx.worktree(&task, &web).join("README.md"), "web\n").unwrap();
        Reviewer::stage_all(&fx.store, &task).unwrap();

        // A pre-commit hook that refuses, in the second repo only. Repos are
        // visited in repo_id order, so `api` has already committed by then, and
        // a hook fails at commit rather than at the checks before it.
        let hooks = web.path.join(".git/hooks");
        std::fs::create_dir_all(&hooks).unwrap();
        let hook = hooks.join("pre-commit");
        std::fs::write(&hook, "#!/bin/sh\nexit 1\n").unwrap();
        use std::os::unix::fs::PermissionsExt;
        std::fs::set_permissions(&hook, std::fs::Permissions::from_mode(0o755)).unwrap();

        let err = Reviewer::commit(&mut fx.store, &task, "cross-cutting", at(5)).unwrap_err();

        assert!(matches!(err, Error::PartialCommit { .. }), "got {err:?}");
        let log = git::run(&fx.worktree(&task, &api), &["log", "--oneline"]).unwrap();
        assert_eq!(
            log.lines().count(),
            1,
            "the repo that worked must be back to its one starting commit: {log}"
        );
        assert!(
            git::has_staged_changes(&fx.worktree(&task, &api)).unwrap(),
            "and its work must still be staged, ready to try again"
        );
        assert_eq!(
            fx.store.get_task(task.id).unwrap().state,
            TaskState::AwaitingReview,
            "the task has not finished"
        );
    }

    #[test]
    fn a_repo_with_nothing_staged_gets_no_empty_commit() {
        let mut fx = Fixture::new();
        let api = fx.repo("api");
        let web = fx.repo("web");
        let task = fx.task_with_worktrees(&[api.clone(), web.clone()]);

        // Only one repo changed.
        std::fs::write(fx.worktree(&task, &api).join("README.md"), "api\n").unwrap();
        Reviewer::stage_all(&fx.store, &task).unwrap();

        let committed = Reviewer::commit(&mut fx.store, &task, "one repo only", at(5)).unwrap();

        assert_eq!(committed.commits.len(), 1);
        assert_eq!(committed.commits[0].repo_id, api.id);
        let web_log = git::run(&fx.worktree(&task, &web), &["log", "-1", "--pretty=%s"]).unwrap();
        assert_eq!(web_log, "initial", "the untouched repo gained no commit");
    }

    #[test]
    fn a_task_that_moved_on_gets_no_commit_at_all() {
        // The screen that offered the commit may have been open for minutes.
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        let wt = fx.worktree(&task, &repo);
        std::fs::write(wt.join("README.md"), "changed\n").unwrap();
        Reviewer::stage_all(&fx.store, &task).unwrap();

        // Somebody cancels while the review screen is still showing the diff.
        fx.store
            .transition(task.id, TaskState::Cancelled, Transition::Plain, at(4))
            .unwrap();

        let err = Reviewer::commit(&mut fx.store, &task, "too late", at(5)).unwrap_err();

        assert!(
            matches!(
                err,
                Error::NotAwaitingReview {
                    state: TaskState::Cancelled,
                    ..
                }
            ),
            "got {err:?}"
        );
        assert_eq!(
            git::run(&wt, &["log", "-1", "--pretty=%s"]).unwrap(),
            "initial",
            "the refusal must come before the commit, not after it"
        );
        assert!(
            git::has_staged_changes(&wt).unwrap(),
            "the staged work is left where the user can still see it"
        );
    }

    // ---- rejection ----

    /// Give `task` a live session running `cat`, and return its name.
    fn live_session(fx: &mut Fixture, task: &Task, server: &TestServer) -> Task {
        let session = tmux::session_name(None, task.id);
        std::fs::create_dir_all(&task.workspace_dir).unwrap();
        server
            .tmux
            .new_session_running(
                &session,
                &task.workspace_dir,
                tmux::DEFAULT_SIZE,
                &[OsString::from("cat")],
            )
            .unwrap();
        fx.store.set_session_name(task.id, &session, at(3)).unwrap();
        fx.store.get_task(task.id).unwrap()
    }

    /// Poll the pane until it shows `needle`, so the test does not race tmux.
    fn wait_for_pane(server: &TestServer, session: &str, needle: &str) -> String {
        let mut seen = String::new();
        for _ in 0..60 {
            if let Ok(panes) = server.tmux.list_panes(session)
                && let Some(pane) = panes.first()
                && let Ok(text) = server.tmux.capture_pane(pane)
            {
                seen = text;
                if seen.contains(needle) {
                    return seen;
                }
            }
            std::thread::sleep(std::time::Duration::from_millis(50));
        }
        seen
    }

    #[test]
    fn rejecting_types_the_instructions_and_puts_the_task_back_to_work() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        let server = TestServer::new();
        let task = live_session(&mut fx, &task, &server);

        Reviewer::reject(
            &mut fx.store,
            &server.tmux,
            &task,
            "MARVERREDO the error handling",
            at(6),
        )
        .unwrap();

        let session = task.session_name.clone().unwrap();
        let pane = wait_for_pane(&server, &session, "MARVERREDO");
        assert!(
            pane.contains("MARVERREDO the error handling"),
            "the whole instruction should arrive as typed: {pane:?}"
        );
        assert_eq!(
            fx.store.get_task(task.id).unwrap().state,
            TaskState::Running,
            "the task resumes in the session it never left"
        );
    }

    #[test]
    fn rejection_keeps_the_session_it_resumes_into() {
        // Not a new session, not a new task: the whole point of the edge is
        // that the agent is still sitting there with its context.
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        let server = TestServer::new();
        let task = live_session(&mut fx, &task, &server);
        let before = task.session_name.clone();

        Reviewer::reject(&mut fx.store, &server.tmux, &task, "again please", at(6)).unwrap();

        let after = fx.store.get_task(task.id).unwrap();
        assert_eq!(after.session_name, before);
        assert!(server.tmux.has_session(&before.unwrap()));
    }

    #[test]
    fn control_characters_are_refused_before_anything_is_typed() {
        // `send-keys -l` stops tmux reading these, but the agent still sees
        // the bytes: `\r` submits half a sentence and `\x03` interrupts the
        // very turn being asked for.
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));

        for text in ["do this\rand that", "stop\u{3}now", "line\nbreak"] {
            let err =
                Reviewer::reject(&mut fx.store, &Tmux::new(), &task, text, at(6)).unwrap_err();
            // Reaching the session lookup would mean the check runs too late —
            // and this task has no session, so that failure is
            // distinguishable.
            assert!(
                matches!(err, Error::ControlCharacter),
                "{text:?} gave {err:?}"
            );
        }
        assert_eq!(
            fx.store.get_task(task.id).unwrap().state,
            TaskState::AwaitingReview
        );
    }

    #[test]
    fn the_agents_own_hook_getting_there_first_is_not_a_failure() {
        // Typing at the agent makes it emit `UserPromptSubmit`, and the daemon
        // turns that into the very move `reject` is about to make. Landing
        // second means the task is already running, which is what was asked
        // for — reporting it as an error would put a red line under a
        // rejection that worked.
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        fx.store
            .transition(task.id, TaskState::Running, Transition::Plain, at(5))
            .unwrap();

        Reviewer::mark_running(&mut fx.store, task.id, at(6)).expect("already there is fine");

        assert_eq!(
            fx.store.get_task(task.id).unwrap().state,
            TaskState::Running
        );
    }

    #[test]
    fn a_task_that_cannot_be_put_back_to_work_still_says_so() {
        // The tolerance above is for one state only; a cancelled task must not
        // be quietly treated as resumed.
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        fx.store
            .transition(task.id, TaskState::Cancelled, Transition::Plain, at(5))
            .unwrap();

        assert!(Reviewer::mark_running(&mut fx.store, task.id, at(6)).is_err());
    }

    #[test]
    fn rejecting_a_task_that_moved_on_types_nothing() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        fx.store
            .transition(task.id, TaskState::Cancelled, Transition::Plain, at(4))
            .unwrap();

        // No session exists, so a `NoSession` error here would prove the state
        // check ran after tmux was already asked to type.
        let err =
            Reviewer::reject(&mut fx.store, &Tmux::new(), &task, "carry on", at(6)).unwrap_err();

        assert!(
            matches!(
                err,
                Error::NotAwaitingReview {
                    state: TaskState::Cancelled,
                    ..
                }
            ),
            "got {err:?}"
        );
    }

    #[test]
    fn rejecting_needs_instructions_and_a_session() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        let server = TestServer::new();

        let empty = Reviewer::reject(&mut fx.store, &server.tmux, &task, "   ", at(6)).unwrap_err();
        assert!(matches!(empty, Error::EmptyInstructions), "got {empty:?}");

        // Awaiting review, but the agent is gone — the user has nothing to
        // resume and needs to be told that rather than left waiting.
        let gone =
            Reviewer::reject(&mut fx.store, &server.tmux, &task, "again", at(6)).unwrap_err();
        assert!(matches!(gone, Error::NoSession { .. }), "got {gone:?}");
        assert_eq!(
            fx.store.get_task(task.id).unwrap().state,
            TaskState::AwaitingReview
        );
    }

    #[test]
    fn a_review_lists_every_worktree_a_task_spans() {
        let mut fx = Fixture::new();
        let api = fx.repo("api");
        let web = fx.repo("web");
        let task = fx.task_with_worktrees(&[api.clone(), web.clone()]);
        std::fs::write(fx.worktree(&task, &api).join("a.txt"), "a\n").unwrap();
        std::fs::write(fx.worktree(&task, &web).join("b.txt"), "b\n").unwrap();

        let review = Reviewer::review(&fx.store, &task).unwrap();
        assert_eq!(review.files.len(), 2);
        assert_ne!(
            review.files[0].worktree, review.files[1].worktree,
            "changes from different repos must stay distinguishable"
        );
    }

    #[test]
    fn deleted_files_are_reported() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        std::fs::remove_file(fx.worktree(&task, &repo).join("README.md")).unwrap();

        let review = Reviewer::review(&fx.store, &task).unwrap();
        assert_eq!(review.files[0].change, Change::Deleted);
    }

    #[test]
    fn a_path_with_spaces_survives_status_parsing() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        let wt = fx.worktree(&task, &repo);
        std::fs::write(wt.join("a file with spaces.txt"), "x\n").unwrap();

        let review = Reviewer::review(&fx.store, &task).unwrap();
        assert_eq!(
            review.files[0].path, "a file with spaces.txt",
            "the default status format would quote this"
        );
    }

    #[test]
    fn files_inside_a_new_directory_are_listed_individually() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx.task_with_worktrees(std::slice::from_ref(&repo));
        let wt = fx.worktree(&task, &repo);
        std::fs::create_dir_all(wt.join("src/deep")).unwrap();
        std::fs::write(wt.join("src/deep/one.rs"), "1\n").unwrap();
        std::fs::write(wt.join("src/deep/two.rs"), "2\n").unwrap();

        let review = Reviewer::review(&fx.store, &task).unwrap();
        let paths: Vec<&str> = review.files.iter().map(|f| f.path.as_str()).collect();
        assert_eq!(
            paths,
            ["src/deep/one.rs", "src/deep/two.rs"],
            "collapsing to the directory would hide the agent's work"
        );
    }

    #[test]
    fn an_unprovisioned_task_has_nothing_to_review() {
        let mut fx = Fixture::new();
        let repo = fx.repo("api");
        let task = fx
            .store
            .create_task("q", "p", &fx.tasks_dir, &[repo.id], at(0))
            .unwrap();
        assert!(Reviewer::review(&fx.store, &task).unwrap().is_empty());
        assert!(matches!(
            Reviewer::stage_all(&fx.store, &task),
            Err(Error::NotProvisioned(_))
        ));
    }
}