Skip to main content

magi/
handover.rs

1//! Taking a branch over from an earlier attempt at the same task.
2//!
3//! A retried task can reopen a branch (`magi/f82f/A`) that the *previous*
4//! run still has checked out in its own worktree. Git holds a branch in one
5//! worktree at a time, so the new run's review cannot start, and the only way
6//! forward used to be an agent asking the operator whether the old worktree
7//! may be deleted. Auto-fold cannot help: it only folds terminal runs, and an
8//! unfinished run keeps its worktrees on purpose so it can be resumed.
9//!
10//! When the run holding the branch is an earlier attempt at the same task
11//! ([`crate::queue::Task::earlier_attempts`]) - superseded or not: blocked,
12//! failed, stale or parked alike - and nothing is driving it, its worktree
13//! (and only its worktree) is released before the review starts. The
14//! branch, every commit and any pull request stay exactly where they were.
15//!
16//! A branch held by a run of *another* task is released on the same terms when
17//! magi can prove the worktree is its own: the path is a candidate worktree
18//! one run's record names, laid under that record's own worktree root, and the
19//! run is not being driven. Anything else - a path magi did not make, a live
20//! run, uncommitted files - is left alone and the refusal names which.
21//!
22//! [`decide`] is pure; [`release`] is the only function here that touches git
23//! or disk. A hand-run `magi review` has no task, so it carries a [`Takeover`]
24//! with no earlier attempts and can only release a worktree of a run magi
25//! itself recorded.
26
27use std::path::{Path, PathBuf};
28
29use anyhow::Result;
30
31use crate::git;
32use crate::run::{Liveness, RunState, RunStatus};
33
34/// The earlier attempts at a task a new run may take a branch over from.
35#[derive(Debug, Clone)]
36pub struct Takeover {
37    /// Ids of runs the new attempt supersedes, from
38    /// [`crate::queue::Task::earlier_attempts`].
39    pub earlier: Vec<String>,
40    /// The magi home their records live under.
41    pub home: PathBuf,
42    /// The owner's answer to an earlier divergence question about the branch,
43    /// applied once the earlier worktree is released (git refuses to move a
44    /// branch that is checked out).
45    pub choice: Option<crate::reconcile::Choice>,
46}
47
48/// The operator has to decide: the takeover was refused, and the text says why.
49///
50/// A distinct type so the queue loop can hold the task for a person instead
51/// of spending an attempt on it (see `daemon::attempt`).
52///
53/// Structured so the operator-facing hold reason can be rendered in the run's
54/// language (`daemon::refused_text`); `Display` is the English text, unchanged.
55#[derive(Debug, Clone, PartialEq, Eq)]
56pub enum Refused {
57    /// The branch is held by a path magi cannot prove it made.
58    Foreign {
59        /// The contested branch.
60        branch: String,
61        /// The worktree that holds it.
62        path: String,
63        /// Machine detail of why it was refused (English).
64        why: String,
65    },
66    /// A magi worktree holds the branch, and releasing it is not safe.
67    Unsafe {
68        /// The contested branch.
69        branch: String,
70        /// The worktree that holds it.
71        path: String,
72        /// Machine detail of why it was refused (English).
73        why: String,
74    },
75    /// Releasing the earlier run's worktree failed and was undone.
76    ReleaseFailed {
77        /// The contested branch.
78        branch: String,
79        /// The worktree that holds it.
80        path: String,
81        /// Short id of the run that owned the worktree.
82        run: String,
83    },
84}
85
86impl std::fmt::Display for Refused {
87    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
88        match self {
89            Refused::Foreign { branch, path, why } => write!(
90                f,
91                "branch `{branch}` is checked out in {path}, which magi will not remove by \
92                 itself: {why}. Remove that worktree (`git worktree remove`) if it is not \
93                 needed, and try again."
94            ),
95            Refused::Unsafe { branch, path, why } => write!(
96                f,
97                "branch `{branch}` is checked out in {path}: {why}. Commit or discard the work \
98                 there and remove that worktree (`git worktree remove`), or say the run may be \
99                 discarded, and try again."
100            ),
101            Refused::ReleaseFailed { branch, path, run } => write!(
102                f,
103                "branch `{branch}` is checked out in {path} by run {run}, and releasing that \
104                 worktree failed or found it changed (git refuses to remove a worktree with \
105                 uncommitted changes); it was left as it was"
106            ),
107        }
108    }
109}
110
111impl std::error::Error for Refused {}
112
113/// A worktree that was released, with what is needed to put it back if the
114/// review that took the branch over never starts.
115#[derive(Debug, Clone)]
116pub struct Released {
117    /// The run whose worktree was released.
118    pub old_id: String,
119    index: usize,
120    path: PathBuf,
121    home: PathBuf,
122    /// The branch tip when the worktree was released; the branch sync may
123    /// have moved it since.
124    tip: String,
125    /// What was released and why it was safe, for the new run's events.
126    pub audit: String,
127}
128
129impl Released {
130    /// Undo the release after the new run failed to start: check the branch
131    /// out again at the old path and unmark the old run. Best-effort - if the
132    /// worktree cannot be re-added the old run stays marked, which is true.
133    pub async fn restore(&self, repo: &Path, branch: &str) {
134        let path = self.path.to_string_lossy().to_string();
135        // Nothing holds the branch now, so it can be put back where the old
136        // run left it before its worktree is recreated at that commit.
137        let refname = format!("refs/heads/{branch}");
138        if git::rev_parse(repo, &refname).await.ok().as_deref() != Some(self.tip.as_str())
139            && let Err(e) = git::git(repo, &["branch", "-f", branch, &self.tip]).await
140        {
141            tracing::warn!("could not put `{branch}` back at {}: {e:#}", self.tip);
142            return;
143        }
144        if let Err(e) = git::git(repo, &["worktree", "add", &path, branch]).await {
145            tracing::warn!(
146                "could not put run {}'s worktree back at {path}: {e:#}",
147                self.old_id
148            );
149            return;
150        }
151        let put_back = RunState::load_under(&self.old_id, &self.home).and_then(|mut s| {
152            if let Some(c) = s.candidates.get_mut(self.index) {
153                c.folded = false;
154            }
155            s.released_to = None;
156            s.released_branches.retain(|b| b != branch);
157            s.events.pop();
158            s.save_under(&self.home)
159        });
160        if let Err(e) = put_back {
161            tracing::warn!("could not unmark run {}: {e:#}", self.old_id);
162        }
163    }
164}
165
166/// What is known about the run and worktree holding the branch.
167#[derive(Debug, Clone)]
168pub struct Holder {
169    /// The run's id.
170    pub run: String,
171    /// Where the run got to.
172    pub status: RunStatus,
173    /// Whether anything is driving it.
174    pub liveness: Liveness,
175    /// A driver pid is recorded but could not be shown dead: it may well be
176    /// running, so "unknown" must not read as "gone".
177    pub driver_unproven: bool,
178    /// Any uncommitted change, untracked files included.
179    pub dirty: bool,
180    /// The first few changed paths, for the refusal text.
181    pub dirty_files: Vec<String>,
182    /// The recorded driver pid, for the refusal text.
183    pub driver_pid: Option<u32>,
184    /// HEAD of the worktree.
185    pub head: String,
186    /// The branch tip in the repository.
187    pub tip: String,
188}
189
190/// Whose worktree holds the branch, as far as magi can prove.
191#[derive(Debug, Clone, PartialEq, Eq)]
192pub enum Owner {
193    /// A candidate worktree of an earlier attempt at the same task.
194    EarlierAttempt,
195    /// A candidate worktree some run's record names, laid under that run's
196    /// own worktree root: magi made it.
197    MagiRun,
198    /// Not provably magi's; the text says why.
199    Foreign(String),
200}
201
202/// The verdict of [`decide`].
203#[derive(Debug, Clone, PartialEq, Eq)]
204pub enum Decision {
205    /// Safe to release the worktree.
206    Release,
207    /// Not safe to release; the text says exactly why, for the operator.
208    Refuse(String),
209}
210
211fn short(sha: &str) -> String {
212    sha.chars().take(7).collect()
213}
214
215/// Whether `holder`'s worktree may be released to a new run.
216///
217/// [`Liveness::Unknown`] releases only when no driver pid was ever recorded
218/// (nothing that could still be running); a recorded pid that could not be
219/// shown dead is refused like a live one. A run of another task that is in
220/// `landing` (possibly waiting on the owner's approval) is refused as well.
221pub fn decide(owner: &Owner, holder: &Holder) -> Decision {
222    if let Owner::Foreign(why) = owner {
223        return Decision::Refuse(why.clone());
224    }
225    let mut why = Vec::new();
226    if holder.liveness == Liveness::Live {
227        why.push(format!(
228            "run {} is being worked on right now{}",
229            crate::run::short_of(&holder.run),
230            holder
231                .driver_pid
232                .map(|p| format!(" (driver pid {p})"))
233                .unwrap_or_default()
234        ));
235    } else if holder.driver_unproven {
236        why.push("its driver process could not be shown to be gone".to_owned());
237    }
238    if *owner == Owner::MagiRun && holder.status == RunStatus::Landing {
239        why.push("that run is in `landing`, possibly waiting on an approval".to_owned());
240    }
241    if holder.dirty {
242        let files = holder.dirty_files.join(", ");
243        why.push(format!("its worktree has uncommitted changes ({files})"));
244    }
245    if holder.head != holder.tip {
246        why.push("its HEAD is not at the branch tip".to_owned());
247    }
248    if why.is_empty() {
249        return Decision::Release;
250    }
251    let kind = match owner {
252        Owner::EarlierAttempt => "an earlier attempt at this task",
253        _ => "a magi run of another task",
254    };
255    Decision::Refuse(format!(
256        "run {} (status `{}`, worktree {}, HEAD {}, branch tip {}) is {kind} and still has the \
257         branch checked out, so it was not released automatically: {}",
258        crate::run::short_of(&holder.run),
259        holder.status.as_str(),
260        if holder.dirty { "dirty" } else { "clean" },
261        short(&holder.head),
262        short(&holder.tip),
263        why.join("; ")
264    ))
265}
266
267/// Read what [`decide`] needs from disk and git.
268async fn inspect(
269    repo: &Path,
270    branch: &str,
271    path: &Path,
272    state: &RunState,
273    home: &Path,
274) -> Result<Holder> {
275    let claimed = crate::daemon::is_working_on(home, &state.id, jiff::Timestamp::now());
276    let liveness = state.liveness(claimed);
277    // Spelled out: `status.showUntrackedFiles=no` in a user's config would
278    // otherwise hide new files from `git status` and let them be deleted.
279    let porcelain = git::git(path, &["status", "--porcelain", "--untracked-files=normal"]).await?;
280    Ok(Holder {
281        run: state.id.clone(),
282        status: state.status,
283        liveness,
284        driver_unproven: liveness == Liveness::Unknown && state.driver_pid.is_some(),
285        dirty: !porcelain.trim().is_empty(),
286        dirty_files: porcelain
287            .lines()
288            .take(5)
289            .map(|l| l.get(3..).unwrap_or(l).trim().to_owned())
290            .collect(),
291        driver_pid: state.driver_pid,
292        head: git::rev_parse(path, "HEAD").await?,
293        tip: git::rev_parse(repo, &format!("refs/heads/{branch}")).await?,
294    })
295}
296
297fn same_path(a: &Path, b: &Path) -> bool {
298    let canon = |p: &Path| std::fs::canonicalize(p).unwrap_or_else(|_| p.to_path_buf());
299    canon(a) == canon(b)
300}
301
302fn foreign(branch: &str, path: &Path, why: &str) -> anyhow::Error {
303    Refused::Foreign {
304        branch: branch.to_owned(),
305        path: path.display().to_string(),
306        why: why.to_owned(),
307    }
308    .into()
309}
310
311/// The run and candidate whose recorded worktree is `path`, when magi
312/// provably made it: the record names the path, the path sits directly in the
313/// record's own worktree root (`<root>/<short>/<dir>`), and exactly one run
314/// says so. `Err` is the reason a path under a root could not be proven.
315fn find_magi_owner(
316    path: &Path,
317    home: &Path,
318) -> std::result::Result<Option<(RunState, usize)>, String> {
319    let Some(bay) = path.parent() else {
320        return Ok(None);
321    };
322    let Some(bay_name) = bay.file_name().and_then(|n| n.to_str()) else {
323        return Ok(None);
324    };
325    let mut found = Vec::new();
326    for id in crate::run::list_ids_in(&home.join("runs")) {
327        if crate::run::short_of(&id) != bay_name {
328            continue;
329        }
330        let Ok(state) = RunState::load_under(&id, home) else {
331            return Err(format!("run {bay_name}'s record could not be read"));
332        };
333        if !same_path(&state.worktree_root(), bay) {
334            continue;
335        }
336        if let Some(i) = state
337            .candidates
338            .iter()
339            .position(|c| !c.folded && same_path(&c.worktree, path))
340        {
341            found.push((state, i));
342        }
343    }
344    match found.len() {
345        0 => Ok(None),
346        1 => Ok(found.pop()),
347        _ => Err(format!("more than one run record claims it ({bay_name})")),
348    }
349}
350
351/// Release the worktree holding `branch` to `new_run`, when an earlier attempt
352/// at the same task, or a run magi itself recorded, holds it and it is safe. Returns the run whose worktree
353/// was released (see [`Released`]), or `None` when nothing needed (or was allowed) to happen.
354///
355/// An `Err` is a refusal or a failed removal; the message is what the
356/// operator reads. Nothing is left half-done: the old run's record is written
357/// before the removal and rolled back if git refuses it.
358pub async fn release(
359    repo: &Path,
360    branch: &str,
361    new_run: &str,
362    takeover: &Takeover,
363) -> Result<Option<Released>> {
364    let Some(path) = git::worktree_holding(repo, branch).await? else {
365        return Ok(None);
366    };
367    // The holder has to be an unfolded candidate worktree of a run on record.
368    // One nobody recorded (made by hand, say) is not ours to touch.
369    let mut owner = None;
370    for id in &takeover.earlier {
371        let Ok(state) = RunState::load_under(id, &takeover.home) else {
372            continue;
373        };
374        if let Some(i) = state
375            .candidates
376            .iter()
377            .position(|c| !c.folded && same_path(&c.worktree, &path))
378        {
379            // Naming the path is not enough: magi must have laid it too.
380            if !path
381                .parent()
382                .is_some_and(|b| same_path(&state.worktree_root(), b))
383            {
384                return Err(foreign(
385                    branch,
386                    &path,
387                    &format!(
388                        "run {} records it, but it is outside that run's worktree root, so \
389                         magi did not make it",
390                        crate::run::short_of(id)
391                    ),
392                ));
393            }
394            owner = Some((state, i, Owner::EarlierAttempt));
395            break;
396        }
397    }
398    if owner.is_none() {
399        owner = match find_magi_owner(&path, &takeover.home) {
400            Ok(found) => found.map(|(s, i)| (s, i, Owner::MagiRun)),
401            Err(why) => return Err(foreign(branch, &path, &why)),
402        };
403    }
404    let Some((mut state, index, kind)) = owner else {
405        return Err(foreign(
406            branch,
407            &path,
408            &format!(
409                "{} is not a candidate worktree any magi run recorded (made by hand, or by \
410                 something other than magi)",
411                path.display()
412            ),
413        ));
414    };
415
416    let holder = inspect(repo, branch, &path, &state, &takeover.home).await?;
417    if let Decision::Refuse(why) = decide(&kind, &holder) {
418        return Err(Refused::Unsafe {
419            branch: branch.to_owned(),
420            path: path.display().to_string(),
421            why,
422        }
423        .into());
424    }
425    let audit = format!(
426        "run {} (status `{}`, no driver, clean, HEAD {} = branch tip, branch `{branch}` kept)",
427        crate::run::short_of(&holder.run),
428        holder.status.as_str(),
429        short(&holder.head)
430    );
431
432    let old_id = state.id.clone();
433    state.candidates[index].folded = true;
434    state.released_to = Some(new_run.to_owned());
435    if !state.released_branches.iter().any(|b| b == branch) {
436        state.released_branches.push(branch.to_owned());
437    }
438    state.event(
439        "release",
440        format!(
441            "worktree of `{branch}` released to run {}: {audit}; this run can no longer be \
442             resumed from here",
443            crate::run::short_of(new_run)
444        ),
445    );
446    state.save_under(&takeover.home)?;
447
448    // Re-read right before the removal for the driver and the tip, and let git
449    // itself refuse a dirty worktree: the removal is *not* `--force`, so an
450    // edit made after the first look is refused rather than thrown away.
451    // Ignored files such as `target/` go with the worktree by design.
452    // Against the record as it is on disk now, not the copy read earlier: a
453    // resume that started in between has saved its driver there. (The resume
454    // side refuses a released run too - see `Runner::execute_graph`.)
455    let again = match RunState::load_under(&old_id, &takeover.home) {
456        Ok(fresh) => inspect(repo, branch, &path, &fresh, &takeover.home).await,
457        Err(e) => Err(e),
458    };
459    let safe = matches!(&again, Ok(h) if decide(&kind, h) == Decision::Release);
460    let removed = safe
461        && git::worktree_remove_clean(repo, &path)
462            .await
463            .unwrap_or(false);
464    if !removed || path.exists() {
465        state = RunState::load_under(&old_id, &takeover.home)?;
466        state.candidates[index].folded = false;
467        state.released_to = None;
468        state.released_branches.retain(|b| b != branch);
469        state.events.pop();
470        state.event(
471            "release",
472            format!(
473                "release of `{branch}` to run {} was undone: {audit}",
474                crate::run::short_of(new_run)
475            ),
476        );
477        state.save_under(&takeover.home)?;
478        return Err(Refused::ReleaseFailed {
479            branch: branch.to_owned(),
480            path: path.display().to_string(),
481            run: crate::run::short_of(&old_id).to_owned(),
482        }
483        .into());
484    }
485    Ok(Some(Released {
486        old_id,
487        index,
488        path,
489        home: takeover.home.clone(),
490        tip: holder.tip,
491        audit,
492    }))
493}
494
495#[cfg(test)]
496mod tests {
497    #[test]
498    fn refused_display_is_the_english_text() {
499        let r = Refused::Foreign {
500            branch: "b".into(),
501            path: "/w".into(),
502            why: "w".into(),
503        };
504        assert_eq!(
505            r.to_string(),
506            "branch `b` is checked out in /w, which magi will not remove by itself: w. Remove \
507             that worktree (`git worktree remove`) if it is not needed, and try again."
508        );
509    }
510
511    use super::*;
512
513    fn holder() -> Holder {
514        Holder {
515            run: "20260901-000000-f82f".to_owned(),
516            status: RunStatus::Gating,
517            liveness: Liveness::Unknown,
518            driver_unproven: false,
519            dirty: false,
520            dirty_files: Vec::new(),
521            driver_pid: None,
522            head: "a".repeat(40),
523            tip: "a".repeat(40),
524        }
525    }
526
527    #[test]
528    fn a_clean_superseded_stale_run_is_released() {
529        assert_eq!(decide(&Owner::EarlierAttempt, &holder()), Decision::Release);
530        let dead = Holder {
531            liveness: Liveness::Dead,
532            ..holder()
533        };
534        assert_eq!(decide(&Owner::EarlierAttempt, &dead), Decision::Release);
535    }
536
537    #[test]
538    fn a_foreign_worktree_is_refused_with_its_reason() {
539        let owner = Owner::Foreign("it is a foreign path".to_owned());
540        assert_eq!(
541            decide(&owner, &holder()),
542            Decision::Refuse("it is a foreign path".to_owned())
543        );
544    }
545
546    #[test]
547    fn a_landing_run_of_another_task_is_refused_but_an_earlier_attempt_is_not() {
548        let landing = Holder {
549            status: RunStatus::Landing,
550            ..holder()
551        };
552        assert!(matches!(
553            decide(&Owner::MagiRun, &landing),
554            Decision::Refuse(_)
555        ));
556        assert_eq!(decide(&Owner::EarlierAttempt, &landing), Decision::Release);
557    }
558
559    #[test]
560    fn a_dirty_worktree_is_refused_and_says_why() {
561        let dirty = Holder {
562            dirty: true,
563            dirty_files: vec!["scratch.txt".to_owned()],
564            ..holder()
565        };
566        let Decision::Refuse(why) = decide(&Owner::EarlierAttempt, &dirty) else {
567            panic!("dirty must be refused");
568        };
569        assert!(
570            why.contains("uncommitted") && why.contains("scratch.txt"),
571            "{why}"
572        );
573        assert!(why.contains("f82f") && why.contains("gating") && why.contains("dirty"));
574    }
575
576    #[test]
577    fn a_live_run_is_refused() {
578        let live = Holder {
579            liveness: Liveness::Live,
580            driver_pid: Some(4242),
581            ..holder()
582        };
583        let Decision::Refuse(why) = decide(&Owner::EarlierAttempt, &live) else {
584            panic!("live must be refused");
585        };
586        assert!(
587            why.contains("right now") && why.contains("f82f") && why.contains("4242"),
588            "{why}"
589        );
590    }
591
592    #[test]
593    fn a_driver_that_could_not_be_shown_dead_is_refused() {
594        let unproven = Holder {
595            driver_unproven: true,
596            ..holder()
597        };
598        let Decision::Refuse(why) = decide(&Owner::EarlierAttempt, &unproven) else {
599            panic!("an unproven driver must be refused");
600        };
601        assert!(why.contains("driver"), "{why}");
602    }
603
604    #[test]
605    fn a_head_off_the_tip_is_refused() {
606        let off = Holder {
607            head: "b".repeat(40),
608            ..holder()
609        };
610        assert!(matches!(
611            decide(&Owner::EarlierAttempt, &off),
612            Decision::Refuse(_)
613        ));
614    }
615}