Skip to main content

magi/
direct.rs

1//! Runs started by hand are tasks.
2//!
3//! `magi run` and `magi review` used to mint a run no task knew about: no
4//! detail page, no attempt history, invisible to duplicate detection, and an
5//! orphan the moment anything cleaned up after it. They now file a task first,
6//! through the queue like `magi task add`, and then either hand it to a live
7//! loop or execute it here through the daemon's own attempt
8//! ([`crate::daemon::run_claimed`]). There is no second dispatcher in this
9//! module: it decides *who* runs the task and watches it, nothing more.
10//!
11//! * A live loop in another process (a fresh `daemon.json` heartbeat, see
12//!   [`crate::daemon::foreign_loop`]) gets the task as `urgent`, runs it in
13//!   its own urgent slot, and the caller [`follow`]s it. The caller never
14//!   claims or executes it: two processes must not drive one task.
15//! * Otherwise the caller claims the task **before it is written**, so a loop
16//!   that starts mid-run finds the claim and leaves it alone, and executes it.
17
18use std::path::{Path, PathBuf};
19use std::time::{Duration, Instant};
20
21use anyhow::{Context, Result, bail};
22use jiff::Timestamp;
23
24use crate::config::Config;
25use crate::queue::{Claim, Queue, RunOverrides, Source, Task, TaskStatus};
26
27/// What to file. Everything `magi run` / `magi review` take that changes what
28/// the run does rides on the task, so a loop that executes it behaves the same
29/// as this process would have.
30#[derive(Debug, Clone)]
31pub struct Filing {
32    /// The task text; for a review, a line saying which branch.
33    pub instruction: String,
34    /// The task's title.
35    pub title: String,
36    /// Absolute repository path.
37    pub repo: PathBuf,
38    /// Who asked: the operator, or the agent seat named by `MAGI_RUN` / `MAGI_NODE`.
39    pub source: Source,
40    /// `--solo`.
41    pub solo: bool,
42    /// Command-line choices carried onto the task.
43    pub overrides: RunOverrides,
44    /// `magi review <branch>`: the branch to review.
45    pub review_of: Option<String>,
46}
47
48impl Filing {
49    fn into_task(self) -> Task {
50        let mut task = Task::new(self.title, self.instruction, self.repo, self.source);
51        task.solo = self.solo;
52        task.review_of = self.review_of;
53        let mut overrides = self.overrides;
54        overrides.inherit_daemon_options = false;
55        task.overrides = Some(overrides);
56        task
57    }
58}
59
60/// Who will execute a filed task.
61#[derive(Debug)]
62pub enum Filed {
63    /// A loop in another process owns the queue; the task is `urgent` and
64    /// unclaimed. `pid` is the loop's, when it published one.
65    Daemon {
66        /// The filed task.
67        task: Task,
68        /// The loop's pid.
69        pid: Option<u32>,
70    },
71    /// Nobody else is serving: the task is claimed for this process, and the
72    /// claim lives as long as this value.
73    Standalone {
74        /// The filed task.
75        task: Task,
76        /// Held until dropped, which is what keeps a later loop away.
77        claim: Claim,
78    },
79}
80
81/// File `filing` and decide who runs it. `home` is the magi home whose
82/// `daemon.json` is judged; `own_pid` is this process (a loop of our own is
83/// not "another" one). `force_local` (`--dry-run`) never hands over: a dry run
84/// spends no agent call and a loop would not honour that.
85pub fn file(
86    queue: &Queue,
87    home: &Path,
88    now: Timestamp,
89    own_pid: u32,
90    filing: Filing,
91    force_local: bool,
92) -> Result<Filed> {
93    let mut task = filing.into_task();
94    let owner = if force_local {
95        None
96    } else {
97        crate::daemon::foreign_loop(crate::daemon::read_status(home).as_ref(), now, own_pid)
98    };
99    match owner {
100        Some(pid) => {
101            task.urgent = true;
102            queue.put(&mut task).context("file the task")?;
103            Ok(Filed::Daemon { task, pid })
104        }
105        None => {
106            // Claimed first: between `put` and the claim a loop could take it.
107            let claim = queue.claim(&task.id)?;
108            queue.put(&mut task).context("file the task")?;
109            Ok(Filed::Standalone { task, claim })
110        }
111    }
112}
113
114/// The config a task's run is built with: the task's `--config` layer, then
115/// its overrides and `solo`. What the loop's `attempt` does, for the dry run.
116pub fn config_for(task: &Task, repo: &Path) -> Result<Config> {
117    let path = task.overrides.as_ref().and_then(|o| o.config.as_deref());
118    let (mut cfg, _) = Config::discover(repo, path)?;
119    if let Some(o) = &task.overrides {
120        o.apply(&mut cfg);
121    }
122    if task.solo {
123        cfg.graph.implementers = 1;
124    }
125    Ok(cfg)
126}
127
128/// How a followed task ended up.
129#[derive(Debug)]
130pub struct Followed {
131    /// The task as last read.
132    pub task: Task,
133    /// False when `max_wait` ran out first: the loop still owns the task and
134    /// nothing is known of its outcome yet.
135    pub finished: bool,
136}
137
138impl Followed {
139    /// The error a caller must exit with when the wait ran out before the
140    /// task finished: that is not a success, and the outcome is not known.
141    /// `None` once the task finished (whatever its outcome).
142    pub fn unfinished(&self) -> Option<anyhow::Error> {
143        if self.finished {
144            return None;
145        }
146        Some(anyhow::anyhow!(
147            "task {id} has not finished: it is still running under the loop. \
148             This is not a success; the outcome is not known yet. Read it with \
149             `magi task show {id}`. The loop keeps the task, so do not run it again.",
150            id = self.task.short()
151        ))
152    }
153}
154
155/// Watch `id` until the loop that owns it is done with it, calling `seen` with
156/// each run id the first time it appears on the task, and `progress` with a
157/// line whenever the newest run's status changes.
158///
159/// Finished is `done`, `held` or `blocked`: a `failed` or `queued` task is the
160/// loop's to retry. A task that stays unfinished while no loop is alive, or
161/// past `max_wait`, is an error - never taken over here, because the loop may
162/// merely be slow to heartbeat and a second driver would race it. Recovery is
163/// the loop's own claim reclaim and `Runner::resume`. Running out of
164/// `max_wait` is not an error: it returns with `finished: false`.
165pub async fn follow(
166    queue: &Queue,
167    home: &Path,
168    id: &str,
169    poll: Duration,
170    max_wait: Option<Duration>,
171    mut seen: impl FnMut(&str),
172    mut progress: impl FnMut(&str),
173) -> Result<Followed> {
174    let began = Instant::now();
175    let mut announced = 0usize;
176    let mut last_status = String::new();
177    loop {
178        let task = queue.get(id)?;
179        for run in task.runs.iter().skip(announced) {
180            seen(run);
181        }
182        announced = task.runs.len();
183        if let Some(run) = task.runs.last()
184            && crate::run::try_home().is_some()
185            && let Ok(state) = crate::run::RunState::load(run)
186        {
187            let line = format!("run {}: {}", state.short(), state.status.as_str());
188            if line != last_status {
189                progress(&line);
190                last_status = line;
191            }
192        }
193        if matches!(
194            task.status,
195            TaskStatus::Done | TaskStatus::Held | TaskStatus::Blocked
196        ) {
197            return Ok(Followed {
198                task,
199                finished: true,
200            });
201        }
202        let reading = crate::daemon::read_status(home);
203        if crate::daemon::foreign_loop(reading.as_ref(), Timestamp::now(), std::process::id())
204            .is_none()
205        {
206            bail!(
207                "no magi loop is serving the queue any more, and task {} is still {}; it was \
208                 not taken over here (the loop may only be slow to heartbeat). Start `magi \
209                 serve` to carry on, or follow it with `magi task show {}`",
210                task.short(),
211                task.status.as_str(),
212                task.short()
213            );
214        }
215        if max_wait.is_some_and(|m| began.elapsed() >= m) {
216            return Ok(Followed {
217                task,
218                finished: false,
219            });
220        }
221        tokio::time::sleep(poll).await;
222    }
223}
224
225/// A run opened inside this process (the follow-up review of `magi fix`) and
226/// the task that owns it. See [`adopt`].
227#[derive(Debug)]
228pub struct Adopted {
229    queue: Queue,
230    /// The task this process claimed and started for the run; `None` when the
231    /// claim could not be taken and the run was only linked.
232    task: Option<Task>,
233    claim: Option<Claim>,
234    quota_before: Vec<crate::run::QuotaLoss>,
235}
236
237/// Give a run this process is about to execute an owning task. A parent task
238/// that exists and is queued, failed or held is claimed, started with the
239/// run and settled by [`Adopted::finish`] exactly like an ownerless run's; if
240/// somebody else holds its claim, or the task is Done, blocked or running, the run is only linked and nothing is settled. With no such
241/// parent, a task is filed and claimed for the duration. `Ok(None)` when no
242/// magi home is pinned, or when the run was only linked to its parent. An
243/// error means the run has no owner and must not be executed: filing or
244/// claiming the new task failed.
245pub fn adopt(
246    state: &crate::run::RunState,
247    parent_task: Option<&str>,
248) -> anyhow::Result<Option<Adopted>> {
249    if crate::run::try_home().is_none() {
250        return Ok(None);
251    }
252    adopt_in(Queue::open(), state, parent_task)
253}
254
255fn adopt_in(
256    queue: Queue,
257    state: &crate::run::RunState,
258    parent_task: Option<&str>,
259) -> anyhow::Result<Option<Adopted>> {
260    use anyhow::Context as _;
261    if let Some(parent) = parent_task
262        && let Ok(id) = queue.resolve_id(parent)
263    {
264        // The parent exists: never file a second owner for this run.
265        return Ok(match queue.claim(&id) {
266            Ok(claim) => {
267                let started = queue.get(&id).and_then(|mut task| {
268                    // A task the daemon could pick up, or one a failed
269                    // hand-started run left held (that is what a requested
270                    // follow-up review re-verifies), is this run's to settle.
271                    // A Done, blocked or running one is settled already or
272                    // owned by somebody else: restarting it would let a
273                    // follow-up review overwrite its outcome, so it only
274                    // gains the run.
275                    if !(task.status.runnable() || task.status == TaskStatus::Held) {
276                        return Ok(None);
277                    }
278                    task.start(state.id.clone());
279                    queue.put(&mut task)?;
280                    Ok(Some(task))
281                });
282                match started {
283                    Ok(None) => {
284                        drop(claim);
285                        let _ = queue.link_run(&id, &state.id);
286                        None
287                    }
288                    Ok(Some(task)) => Some(Adopted {
289                        queue,
290                        task: Some(task),
291                        claim: Some(claim),
292                        quota_before: state.quota.clone(),
293                    }),
294                    Err(e) => {
295                        tracing::warn!("could not start task {id} for run {}: {e:#}", state.id);
296                        drop(claim);
297                        let _ = queue.link_run(&id, &state.id);
298                        None
299                    }
300                }
301            }
302            Err(e) => {
303                // Another driver owns the task; do not settle over its result.
304                tracing::warn!("task {id} is claimed elsewhere ({e:#}); linking run only");
305                let _ = queue.link_run(&id, &state.id);
306                None
307            }
308        });
309    }
310    let mut task = Task::new(
311        crate::queue::title_from(&state.instruction, 72),
312        state.instruction.clone(),
313        state.repo.clone(),
314        Source::Human,
315    );
316    let claim = queue
317        .claim(&task.id)
318        .with_context(|| format!("could not claim a new task for run {}", state.id))?;
319    task.start(state.id.clone());
320    queue
321        .put(&mut task)
322        .with_context(|| format!("could not file a new task for run {}", state.id))?;
323    Ok(Some(Adopted {
324        queue,
325        task: Some(task),
326        claim: Some(claim),
327        quota_before: state.quota.clone(),
328    }))
329}
330
331impl Adopted {
332    /// Settle the task of an adopted ownerless run after it executed.
333    pub fn finish(mut self, state: &crate::run::RunState, result: Result<()>) {
334        if let Some(task) = &mut self.task {
335            crate::daemon::finish_attempt(
336                crate::daemon::Opts::default().max_attempts,
337                &self.queue,
338                task,
339                state,
340                &self.quota_before,
341                result,
342            );
343            crate::daemon::hold_if_runnable(&self.queue, task);
344        }
345        drop(self.claim.take());
346    }
347}
348
349#[cfg(test)]
350mod tests {
351    use super::*;
352    use crate::daemon::Status;
353
354    fn filing(repo: &Path) -> Filing {
355        Filing {
356            instruction: "review the branch".to_owned(),
357            title: "review x".to_owned(),
358            repo: repo.to_path_buf(),
359            source: Source::Human,
360            solo: false,
361            overrides: RunOverrides {
362                merge: Some("none".to_owned()),
363                ..RunOverrides::default()
364            },
365            review_of: Some("feat/x".to_owned()),
366        }
367    }
368
369    fn heartbeat(home: &Path, pid: u32, age_secs: i64) {
370        let mut status = Status::new();
371        status.pid = pid;
372        status.updated_at = Timestamp::now()
373            .checked_sub(jiff::SignedDuration::from_secs(age_secs))
374            .unwrap();
375        crate::daemon::write_status_to(&home.join("daemon.json"), &status).unwrap();
376    }
377
378    #[test]
379    fn task_implementers_direct_filing_keeps_options_isolated() {
380        let dir = tempfile::tempdir().unwrap();
381        let mut direct = filing(dir.path());
382        direct.overrides.inherit_daemon_options = true;
383        let task = direct.into_task();
384        assert!(!task.overrides.unwrap().inherit_daemon_options);
385    }
386
387    #[test]
388    fn without_a_loop_the_task_is_claimed_before_anyone_else_can_take_it() {
389        let dir = tempfile::tempdir().unwrap();
390        let q = Queue::at(dir.path().join("queue"));
391        let filed = file(
392            &q,
393            dir.path(),
394            Timestamp::now(),
395            1,
396            filing(dir.path()),
397            false,
398        )
399        .unwrap();
400        let Filed::Standalone { task, claim } = filed else {
401            panic!("no loop is alive");
402        };
403        assert!(!task.urgent);
404        assert_eq!(task.review_of.as_deref(), Some("feat/x"));
405        assert_eq!(
406            task.overrides.as_ref().unwrap().merge.as_deref(),
407            Some("none")
408        );
409        assert!(
410            q.claim(&task.id).is_err(),
411            "a second process must not be able to claim a standalone run's task"
412        );
413        drop(claim);
414        assert!(q.claim(&task.id).is_ok(), "released with the claim");
415    }
416
417    #[test]
418    fn a_live_foreign_loop_gets_an_urgent_unclaimed_task() {
419        let dir = tempfile::tempdir().unwrap();
420        let q = Queue::at(dir.path().join("queue"));
421        heartbeat(dir.path(), 4242, 0);
422        let filed = file(
423            &q,
424            dir.path(),
425            Timestamp::now(),
426            1,
427            filing(dir.path()),
428            false,
429        )
430        .unwrap();
431        let Filed::Daemon { task, pid } = filed else {
432            panic!("a loop is alive");
433        };
434        assert_eq!(pid, Some(4242));
435        assert!(task.urgent);
436        let stored = q.get(&task.id).unwrap();
437        assert_eq!(stored.status, TaskStatus::Queued);
438        assert!(stored.runs.is_empty(), "nothing ran in this process");
439        assert!(q.claim(&task.id).is_ok(), "and this process holds no claim");
440    }
441
442    #[test]
443    fn a_stale_heartbeat_our_own_pid_and_a_dry_run_do_not_count_as_a_loop() {
444        let dir = tempfile::tempdir().unwrap();
445        let q = Queue::at(dir.path().join("queue"));
446        heartbeat(dir.path(), 4242, 3600);
447        assert!(matches!(
448            file(
449                &q,
450                dir.path(),
451                Timestamp::now(),
452                1,
453                filing(dir.path()),
454                false
455            )
456            .unwrap(),
457            Filed::Standalone { .. }
458        ));
459        heartbeat(dir.path(), 7, 0);
460        assert!(matches!(
461            file(
462                &q,
463                dir.path(),
464                Timestamp::now(),
465                7,
466                filing(dir.path()),
467                false
468            )
469            .unwrap(),
470            Filed::Standalone { .. }
471        ));
472        heartbeat(dir.path(), 4242, 0);
473        assert!(matches!(
474            file(
475                &q,
476                dir.path(),
477                Timestamp::now(),
478                1,
479                filing(dir.path()),
480                true
481            )
482            .unwrap(),
483            Filed::Standalone { .. }
484        ));
485    }
486
487    #[tokio::test]
488    async fn following_gives_up_on_a_task_nobody_is_serving() {
489        let dir = tempfile::tempdir().unwrap();
490        let q = Queue::at(dir.path().join("queue"));
491        let mut t = filing(dir.path()).into_task();
492        q.put(&mut t).unwrap();
493        let err = follow(
494            &q,
495            dir.path(),
496            &t.id,
497            Duration::from_millis(5),
498            None,
499            |_| {},
500            |_| {},
501        )
502        .await
503        .unwrap_err();
504        assert!(format!("{err}").contains("no magi loop"), "{err}");
505        assert!(q.claim(&t.id).is_ok(), "never taken over");
506    }
507
508    #[tokio::test]
509    async fn following_returns_when_the_loop_finishes_the_task_and_times_out_otherwise() {
510        let dir = tempfile::tempdir().unwrap();
511        let q = Queue::at(dir.path().join("queue"));
512        heartbeat(dir.path(), 4242, 0);
513        let mut t = filing(dir.path()).into_task();
514        q.put(&mut t).unwrap();
515        let waited = follow(
516            &q,
517            dir.path(),
518            &t.id,
519            Duration::from_millis(5),
520            Some(Duration::from_millis(30)),
521            |_| {},
522            |_| {},
523        )
524        .await
525        .unwrap();
526        assert!(!waited.finished, "the wait ran out, the loop still owns it");
527        let err = waited
528            .unfinished()
529            .expect("an unfinished wait is an error")
530            .to_string();
531        assert!(err.contains(t.short()), "{err}");
532        assert!(
533            err.contains("not finished") || err.contains("not a success"),
534            "{err}"
535        );
536        assert!(err.contains("magi task show"), "{err}");
537        assert_eq!(
538            q.get(&t.id).unwrap().status,
539            TaskStatus::Queued,
540            "the queue is untouched"
541        );
542        t.link_run("20260101-000000-abcd");
543        t.succeed();
544        q.put(&mut t).unwrap();
545        let mut runs = Vec::new();
546        let done = follow(
547            &q,
548            dir.path(),
549            &t.id,
550            Duration::from_millis(5),
551            Some(Duration::from_secs(5)),
552            |r| runs.push(r.to_owned()),
553            |_| {},
554        )
555        .await
556        .unwrap();
557        assert_eq!(done.task.status, TaskStatus::Done);
558        assert!(done.unfinished().is_none());
559        assert_eq!(runs, ["20260101-000000-abcd"]);
560    }
561
562    fn parent_in(q: &Queue, dir: &Path) -> Task {
563        let mut t = filing(dir).into_task();
564        q.put(&mut t).unwrap();
565        t
566    }
567
568    fn follow_up_state(dir: &Path) -> crate::run::RunState {
569        crate::run::RunState::new(
570            dir.to_path_buf(),
571            "main".to_owned(),
572            "abc1234".to_owned(),
573            "review the branch".to_owned(),
574            Config::default(),
575        )
576    }
577
578    #[test]
579    fn an_adopted_review_claims_starts_and_settles_a_runnable_task() {
580        let dir = tempfile::tempdir().unwrap();
581        let q = Queue::at(dir.path().join("queue"));
582        let parent = parent_in(&q, dir.path());
583        let state = follow_up_state(dir.path());
584        let adopted = adopt_in(q.clone(), &state, Some(&parent.id))
585            .unwrap()
586            .expect("adopted");
587        let running = q.get(&parent.id).unwrap();
588        assert_eq!(running.status, TaskStatus::Running);
589        assert_eq!(running.attempts, parent.attempts + 1);
590        assert_eq!(running.runs, std::slice::from_ref(&state.id));
591        assert!(q.claim(&parent.id).is_err(), "exclusive while it runs");
592        adopted.finish(&state, Err(anyhow::anyhow!("boom")));
593        let settled = q.get(&parent.id).unwrap();
594        assert_eq!(settled.status, TaskStatus::Held, "{settled:?}");
595        assert_eq!(
596            settled.runs,
597            std::slice::from_ref(&state.id),
598            "no duplicate run"
599        );
600        assert!(q.claim(&parent.id).is_ok(), "claim released");
601        assert_eq!(q.list().len(), 1, "no second owner was filed");
602    }
603
604    #[test]
605    fn an_ownerless_run_whose_task_cannot_be_filed_is_an_error() {
606        let dir = tempfile::tempdir().unwrap();
607        // The queue root sits under a regular file, so claiming cannot create it.
608        let blocker = dir.path().join("blocker");
609        std::fs::write(&blocker, "x").unwrap();
610        let q = Queue::at(blocker.join("q"));
611        let state = follow_up_state(dir.path());
612        let err = adopt_in(q, &state, None).expect_err("must not be swallowed");
613        assert!(format!("{err:#}").contains(&state.id), "{err:#}");
614    }
615
616    #[test]
617    fn a_task_claimed_elsewhere_only_gains_the_run() {
618        let dir = tempfile::tempdir().unwrap();
619        let q = Queue::at(dir.path().join("queue"));
620        let parent = parent_in(&q, dir.path());
621        let _theirs = q.claim(&parent.id).unwrap();
622        let state = follow_up_state(dir.path());
623        assert!(
624            adopt_in(q.clone(), &state, Some(&parent.id))
625                .unwrap()
626                .is_none()
627        );
628        let after = q.get(&parent.id).unwrap();
629        assert_eq!(after.status, parent.status);
630        assert_eq!(after.attempts, parent.attempts);
631        assert_eq!(after.runs, std::slice::from_ref(&state.id));
632        assert_eq!(q.list().len(), 1, "no second owner was filed");
633    }
634
635    #[test]
636    fn a_done_task_only_gains_the_run() {
637        let dir = tempfile::tempdir().unwrap();
638        let q = Queue::at(dir.path().join("queue"));
639        let mut parent = parent_in(&q, dir.path());
640        parent.succeed();
641        q.put(&mut parent).unwrap();
642        let state = follow_up_state(dir.path());
643        assert!(
644            adopt_in(q.clone(), &state, Some(&parent.id))
645                .unwrap()
646                .is_none()
647        );
648        let after = q.get(&parent.id).unwrap();
649        assert_eq!(after.status, TaskStatus::Done);
650        assert_eq!(after.attempts, parent.attempts);
651        assert_eq!(after.runs, std::slice::from_ref(&state.id));
652        assert!(q.claim(&parent.id).is_ok(), "claim released");
653    }
654
655    #[test]
656    fn a_held_task_is_claimed_and_settled_by_its_follow_up_review() {
657        let dir = tempfile::tempdir().unwrap();
658        let q = Queue::at(dir.path().join("queue"));
659        let mut parent = parent_in(&q, dir.path());
660        parent.hold_manual(Some("the run did not finish: stale".to_owned()));
661        q.put(&mut parent).unwrap();
662        let state = follow_up_state(dir.path());
663        let adopted = adopt_in(q.clone(), &state, Some(&parent.id))
664            .unwrap()
665            .expect("adopted");
666        assert_eq!(q.get(&parent.id).unwrap().status, TaskStatus::Running);
667        assert!(q.claim(&parent.id).is_err(), "claimed while it runs");
668        adopted.finish(&state, Err(anyhow::anyhow!("fresh failure")));
669        let after = q.get(&parent.id).unwrap();
670        assert_eq!(after.status, TaskStatus::Held);
671        assert!(
672            after
673                .hold_reason
674                .as_deref()
675                .unwrap()
676                .contains("fresh failure"),
677            "{after:?}"
678        );
679        assert_eq!(after.runs, std::slice::from_ref(&state.id));
680        assert!(q.claim(&parent.id).is_ok(), "claim released");
681    }
682}