magi/triage.rs
1//! Held-task triage: walking `held` tasks so a hold left by an accident does
2//! not sit unread forever next to one a human placed on purpose.
3//!
4//! [`crate::queue::HoldSource`] already distinguishes "the daemon or
5//! conductor held this during its own recovery" ([`HoldSource::Machine`],
6//! documented as recoverable) from "an operator held this on purpose"
7//! ([`HoldSource::Manual`]). What was missing was anything that actually acts
8//! on that distinction: nothing walked the held list and asked whether a
9//! machine hold's cause was still true, and a record written before
10//! `hold_source` existed (schema < 3, `None`) was silently protected forever
11//! by [`Task::operator_held`]'s conservative default - never wrong, but also
12//! never looked at again by anything.
13//!
14//! [`run_once`] is that walk. For every `held` task it finds:
15//!
16//! - [`HoldSource::Machine`]: if [`machine_cause_resolved`] can tell the
17//! cause is gone, the task goes straight back to `queued` - the same effect
18//! as `magi task release`, just automatic. When it cannot tell, a
19//! [`Question`] is filed once and the task stays held until answered.
20//! - `None` (a legacy record, or a hold nobody explained): always a question,
21//! exactly once - the whole point being that "protected forever" must not
22//! mean "never shown to anyone" either.
23//! - [`HoldSource::Manual`]: never touched automatically. Only once the hold
24//! has sat untouched past [`MANUAL_STALE_AFTER`] does it earn a question of
25//! its own, asking whether it is still wanted.
26//!
27//! Every question this module files carries [`NODE`] and uses the task id as
28//! [`Question::run`] - the same convention `crate::conduct`'s own questions
29//! use for a task rather than a run (see `conduct::apply_one`'s own comment
30//! on why the dedupe check there also filters on `node`, not `run` alone: an
31//! ordinary graph question's `run` is a real run id, and a coincidental
32//! equality with some task's id must not be read as "about this task").
33//! [`latest_triage_question`] follows the identical rule.
34//!
35//! # Why answers apply here rather than through `crate::queue::Task::block`
36//!
37//! `crate::conduct` blocks a task on its own question
38//! (`Task::block(vec![question_id], …)`), and `crate::daemon::resolve_blockers`
39//! unblocks it - unconditionally, back to `queued` - the moment that question
40//! is answered, whatever the answer actually said. That is correct for
41//! `conduct`: the *content* of the answer is meant for whoever reads
42//! `Task::answers` next, not for the resolver.
43//!
44//! A triage question's answer is different: "not yet" and "discard it" do two
45//! entirely different, non-resuming things, and only "resume it" may put the
46//! task back in line. Reusing the generic blocked/unblock path would resume
47//! every answer alike, so this module never calls [`Task::block`] and never
48//! leaves a triaged task anything but `held` while its question is open.
49//! [`interpret_answer`] reads [`Question::resolution`] itself and
50//! [`run_once`] acts on it directly: [`Task::release`] for an actual "resume
51//! it" choice, [`Queue::remove`] for "discard it" (捨ててよい really means
52//! "you may throw this away", not "leave it sitting held" - the English
53//! wording must say the same thing, not "leave it held"), and
54//! [`Task::hold_manual`] for anything else - which both keeps the task held
55//! and reclassifies it as a hold an operator has now actually seen, one
56//! `crate::conduct` and a later triage pass leave alone.
57//!
58//! The choice is read by its **position** in [`Question::choices`]
59//! ([`Wording::choices3`]/[`Wording::choices2`] always put "resume" first and
60//! "discard" third), never by comparing the answer text against [`Wording`]'s
61//! own strings picked from whatever config is in force *now* - the language a
62//! question was filed in and the language a later `run_once` call happens to
63//! read back are not guaranteed to be the same call's [`Config`], and a text
64//! comparison would silently misread a real "resume" answer as "keep held"
65//! the moment they disagree.
66//!
67//! # Idempotency, without a new field
68//!
69//! [`run_once`] runs on every daemon idle tick (see `crate::daemon::poll`)
70//! and on every `magi task triage`, so applying the *same* answered question
71//! twice has to be harmless - and, once a `HoldSource::Manual`/`None`
72//! question has been answered "not yet", finding a *fresh* one for the same
73//! task later (once it goes stale again) has to still be possible. Neither
74//! [`crate::queue::Task`] nor [`crate::ask::Question`] has a field for "this
75//! answer was already applied", so [`already_applied`] reads the same
76//! [`Question::short`] id back out of [`Task::hold_reason`] that
77//! [`keep_held_note`] appended to it - the same trick [`Question::abandon`]
78//! already uses to fold a fact into a text field that has no dedicated one.
79//! Appended, not written wholesale: the reason the hold happened in the first
80//! place is still worth reading in `magi task show` after an operator says
81//! "not yet". A "resume" or "discard" answer needs no marker at all: the task
82//! either leaves `held` entirely or stops existing, and either way it is
83//! never looked at by this module again.
84
85use std::path::{Path, PathBuf};
86use std::time::Duration;
87
88use jiff::Timestamp;
89
90use crate::ask::{Question, QuestionStatus, Questions};
91use crate::config::Config;
92use crate::disk;
93use crate::queue::{HoldSource, Queue, Task, TaskStatus};
94
95/// Node recorded on every question this module files - `crate::conduct::NODE`
96/// for the same idea applied to a `crate::conduct` decision instead.
97pub const NODE: &str = "triage";
98
99/// Seat name on a filed question. Not a real agent seat - there is no model
100/// call anywhere in this module - but every [`Question`] needs one, and every
101/// other deterministic filer (`crate::land`'s merge approval) names itself
102/// the same way.
103const SEAT: &str = "triage";
104
105/// How long a [`HoldSource::Manual`] hold sits untouched before triage asks
106/// whether it is still wanted.
107///
108/// A judgement call, not a `magi.toml` setting - the same reasoning
109/// `ask::REPLY_QUIET_WINDOW` documents for itself: there is no operator
110/// preference for "how long is too long to ignore my own hold" that a
111/// per-repository config could be *right* about. Seven days is long enough
112/// that an ordinary multi-day hold (waiting on a dependency, waiting on the
113/// operator's own schedule) never gets nagged, and short enough that a hold
114/// nobody has looked at in a week surfaces again rather than aging into the
115/// kind of silent backlog this feature exists to prevent.
116const MANUAL_STALE_AFTER: Duration = Duration::from_secs(7 * 24 * 60 * 60);
117
118/// Localised strings for a filed question, the same idea as `crate::land`'s
119/// own `Words`/`words()` - only the languages magi can actually check are
120/// translated, and anything else falls back to English.
121struct Wording {
122 lang: &'static str,
123 resume: &'static str,
124 wait: &'static str,
125 discard: &'static str,
126 resume_now: &'static str,
127 keep_held: &'static str,
128}
129
130const EN: Wording = Wording {
131 lang: "en",
132 resume: "resume it",
133 wait: "not yet",
134 discard: "discard it",
135 resume_now: "resume it",
136 keep_held: "keep it held",
137};
138
139const JA: Wording = Wording {
140 lang: "ja",
141 resume: "再開してよい",
142 wait: "まだ待って",
143 discard: "捨ててよい",
144 resume_now: "再開する",
145 keep_held: "まだ止めておく",
146};
147
148/// Pick the wording. Codes and names both, the same acceptance
149/// `crate::land::words` gives `[graph] language`.
150fn wording(language: &str) -> &'static Wording {
151 let l = language.trim();
152 if l.eq_ignore_ascii_case("ja")
153 || l.eq_ignore_ascii_case("jp")
154 || l.eq_ignore_ascii_case("japanese")
155 || l.eq_ignore_ascii_case("日本語")
156 {
157 &JA
158 } else {
159 &EN
160 }
161}
162
163impl Wording {
164 fn choices3(&self) -> Vec<String> {
165 vec![
166 self.resume.to_owned(),
167 self.wait.to_owned(),
168 self.discard.to_owned(),
169 ]
170 }
171
172 fn choices2(&self) -> Vec<String> {
173 vec![self.resume_now.to_owned(), self.keep_held.to_owned()]
174 }
175
176 fn source_label(&self, source: Option<HoldSource>) -> &'static str {
177 match (self.lang, source) {
178 ("ja", Some(HoldSource::Machine)) => "machine(機械による自動保留)",
179 ("ja", Some(HoldSource::Manual)) => "manual(操作者による手動保留)",
180 ("ja", None) => "unknown(schema 3 未満の旧レコード、または理由未記録)",
181 (_, Some(HoldSource::Machine)) => "machine (automatic recovery hold)",
182 (_, Some(HoldSource::Manual)) => "manual (an operator held this)",
183 (_, None) => "unknown (pre-schema-3 record, or never recorded)",
184 }
185 }
186
187 /// The body under the summary: everything an operator needs to judge this
188 /// without opening a terminal - id, title, hold reason, hold source.
189 ///
190 /// Falls back to [`Task::last_error`] when [`Task::hold_reason`] is empty:
191 /// the most common `HoldSource::Machine` hold of all - `Task::fail` once
192 /// attempts run out - only ever sets `last_error`, never `hold_reason`, so
193 /// reading `hold_reason` alone would leave the question blank for exactly
194 /// the case requirement 4 exists for.
195 fn detail(&self, task: &Task, why: &str) -> String {
196 let none = if self.lang == "ja" {
197 "(記録なし)"
198 } else {
199 "(none recorded)"
200 };
201 let reason = task
202 .hold_reason
203 .as_deref()
204 .or(task.last_error.as_deref())
205 .unwrap_or(none);
206 format!(
207 "task: {} ({})\ntitle: {}\nhold source: {}\nhold reason: {reason}\n\n{why}",
208 task.id,
209 task.short(),
210 task.title,
211 self.source_label(task.hold_source),
212 )
213 }
214
215 fn summary_machine_unknown(&self, task: &Task) -> String {
216 if self.lang == "ja" {
217 format!("保留タスク {} の再開可否を判断してください", task.short())
218 } else {
219 format!("decide whether to resume held task {}", task.short())
220 }
221 }
222
223 fn why_machine(&self) -> &'static str {
224 if self.lang == "ja" {
225 "機械的な保留(machine hold)ですが、原因がすでに解消しているかを自動では判断できませんでした。"
226 } else {
227 "This is a machine hold, but whether its cause has resolved could not be \
228 checked automatically."
229 }
230 }
231
232 fn summary_legacy(&self, task: &Task) -> String {
233 if self.lang == "ja" {
234 format!(
235 "hold_source が不明な保留タスク {} を確認してください",
236 task.short()
237 )
238 } else {
239 format!(
240 "held task {} has no recorded hold source - please take a look",
241 task.short()
242 )
243 }
244 }
245
246 fn why_legacy(&self) -> &'static str {
247 if self.lang == "ja" {
248 "hold_source が記録されていません。schema 3 より前のレコードか、理由が記録されなかった \
249 holdです。人が意図して止めたのか、クラッシュや強制再起動で宙に浮いただけなのか、\
250 このデータからは区別できません。"
251 } else {
252 "No hold_source was recorded - either a pre-schema-3 record, or a hold whose \
253 reason was never written down. Whether this was a deliberate hold or the \
254 leftover of a crash cannot be told from the data alone."
255 }
256 }
257
258 fn summary_manual_stale(&self, task: &Task, days: i64) -> String {
259 if self.lang == "ja" {
260 format!(
261 "{days}日間 保留されたままの手動保留タスク {} を確認してください",
262 task.short()
263 )
264 } else {
265 format!(
266 "held task {} has been on a manual hold for {days} day(s)",
267 task.short()
268 )
269 }
270 }
271
272 fn why_manual(&self) -> &'static str {
273 if self.lang == "ja" {
274 "操作者が明示的に止めた保留ですが、長期間そのままになっています。まだ止めておくか、\
275 再開するか教えてください。"
276 } else {
277 "An operator held this on purpose, but it has sat untouched for a while. Say \
278 whether to keep holding it or resume it."
279 }
280 }
281}
282
283/// Which of the three situations this module recognises a held task is in.
284#[derive(Debug, Clone, Copy, PartialEq, Eq)]
285enum Bucket {
286 /// `HoldSource::Machine`, cause not verifiably resolved.
287 MachineUnknown,
288 /// `hold_source` is `None`.
289 Legacy,
290 /// `HoldSource::Manual`, held past [`MANUAL_STALE_AFTER`].
291 ManualStale,
292}
293
294/// What one [`run_once`] pass did, task ids in each list.
295#[derive(Debug, Clone, Default)]
296pub struct Report {
297 /// A `HoldSource::Machine` hold whose cause was found resolved, put back
298 /// in line automatically.
299 pub resumed: Vec<String>,
300 /// A fresh question was filed this pass.
301 pub asked: Vec<String>,
302 /// An operator's answer to an earlier triage question was applied.
303 pub answered: Vec<String>,
304 /// A `blocked` task whose `blocked_by` named an id that no longer exists
305 /// was moved to a machine hold this pass - see
306 /// [`quarantine_orphaned_blocked`]. Distinct from `asked`: the question
307 /// about it, if any, is filed in the same pass and only counted there.
308 pub quarantined: Vec<String>,
309}
310
311impl Report {
312 /// Is there nothing to report? Callers use this to skip logging an empty
313 /// pass rather than repeating "triaged 0 held task(s)" on every idle tick.
314 pub fn is_empty(&self) -> bool {
315 self.resumed.is_empty()
316 && self.asked.is_empty()
317 && self.answered.is_empty()
318 && self.quarantined.is_empty()
319 }
320}
321
322/// The repository a task's config and disk check should read from. Mirrors
323/// `crate::conduct::repo_for`'s own fallback, duplicated rather than shared
324/// because that one is private to its module and the two are one `if` each.
325fn repo_for(task: &Task) -> PathBuf {
326 if task.repo.as_os_str().is_empty() {
327 PathBuf::from(".")
328 } else {
329 task.repo.clone()
330 }
331}
332
333/// Recognise a `HoldSource::Machine` hold caused by the free-space gate
334/// (`crate::disk::gate`'s message, or `daemon::disk_gate`'s "could not
335/// measure" fallback) from `hold_reason` text alone.
336///
337/// There is no field recording *why* a machine hold happened - `hold_machine`
338/// takes only a reason string - so this is the one signal available, and disk
339/// pressure is the one cause this module can safely re-measure without
340/// touching git, a run, or an agent CLI. Keep the two prefixes here in sync
341/// with `crate::disk::gate`'s formatted string and `daemon::disk_gate`'s own
342/// message if either changes; nothing else ties them together.
343fn is_disk_hold(task: &Task) -> bool {
344 task.hold_reason.as_deref().is_some_and(|r| {
345 r.starts_with("not enough free space to start a run:")
346 || r.starts_with("could not measure free space on ")
347 })
348}
349
350/// Has a `HoldSource::Machine` hold's cause resolved? `Some(true)` means yes -
351/// safe to requeue. `Some(false)` means the same cause was checked and is
352/// still in force. `None` means this hold's cause is not one this module
353/// knows how to re-check at all, and a human has to look.
354fn machine_cause_resolved(task: &Task, cfg: &Config) -> Option<bool> {
355 if !is_disk_hold(task) {
356 return None;
357 }
358 let min = cfg.disk.min_free_bytes;
359 if min == 0 {
360 // The operator turned the gate off since this hold was placed - its
361 // one possible cause is gone by construction, no measurement needed.
362 return Some(true);
363 }
364 let free = disk::free_bytes(&repo_for(task)).ok()?;
365 Some(disk::gate(free, min).is_none())
366}
367
368/// Is a `HoldSource::Manual` hold old enough to earn a "still wanted?"
369/// question? Same comparison `crate::clean::due` uses for a run's fold grace,
370/// against [`MANUAL_STALE_AFTER`] instead of a configured one.
371fn manual_is_stale(task: &Task, now: Timestamp) -> bool {
372 now.as_second() - task.updated_at.as_second() > MANUAL_STALE_AFTER.as_secs() as i64
373}
374
375/// The marker [`apply_answer`] writes into [`Task::hold_reason`] and
376/// [`already_applied`] reads back - see this module's own doc on why.
377fn marker_for(q: &Question) -> String {
378 format!("[triage:{}]", q.short())
379}
380
381/// Has `q`'s answer already been applied to `task`? See this module's doc.
382fn already_applied(task: &Task, q: &Question) -> bool {
383 let marker = marker_for(q);
384 task.hold_reason
385 .as_deref()
386 .is_some_and(|r| r.contains(marker.as_str()))
387}
388
389/// The most recent question this module filed for `task_id`, any status -
390/// open (still waiting), answered (may need applying), or abandoned (settled
391/// with nothing decided). Filters on both `node` and `run`, never `run`
392/// alone - see this module's doc on why a bare `run` match is not safe.
393fn latest_triage_question(questions: &Questions, task_id: &str) -> Option<Question> {
394 questions
395 .list()
396 .into_iter()
397 .filter(|q| q.node == NODE && q.run == task_id)
398 .max_by(|a, b| a.id.cmp(&b.id))
399}
400
401/// What an answered triage question's choice means, independent of which
402/// language it was filed in.
403#[derive(Debug, Clone, Copy, PartialEq, Eq)]
404enum AnswerAction {
405 /// The first choice, always "resume it" / "再開してよい" / "再開する" -
406 /// see [`Wording::choices3`] and [`Wording::choices2`], whose first entry
407 /// is always the resume choice.
408 Resume,
409 /// The third choice, present only in [`Wording::choices3`]: "discard it"
410 /// / "捨ててよい".
411 Discard,
412 /// Anything else: the second choice ("not yet" / "keep it held"), or an
413 /// answer that does not match any offered choice at all (should not
414 /// happen for a multiple-choice question, but the safe default is to
415 /// keep holding rather than guess at "resume").
416 KeepHeld,
417}
418
419/// Read `q`'s answer as an [`AnswerAction`].
420///
421/// Matched by **position in `q.choices`**, never by comparing the answer text
422/// against [`Wording`]'s own strings: [`Wording`] is picked from the task's
423/// *current* repository config, which can differ from whatever language was
424/// in force when the question was filed (a `--config` override on one `magi
425/// task triage` call and not the next, or the repository's config edited in
426/// between). Comparing text would then silently misread an actual "resume"
427/// answer as "keep held" - the choices themselves are fixed at filing time,
428/// in [`file_question`], and never change afterwards, so their position is
429/// the one thing that stays true regardless of which language reads them
430/// back.
431fn interpret_answer(q: &Question) -> AnswerAction {
432 let resolution = q.resolution().unwrap_or_default();
433 match q.choices.iter().position(|c| *c == resolution) {
434 Some(0) => AnswerAction::Resume,
435 Some(2) => AnswerAction::Discard,
436 _ => AnswerAction::KeepHeld,
437 }
438}
439
440/// The note [`already_applied`] looks for, appended to (never replacing)
441/// whatever [`Task::hold_reason`] already said - the original cause is still
442/// worth reading in `magi task show` after the operator answers "not yet",
443/// and [`Task::hold_manual`] would otherwise overwrite it outright.
444fn keep_held_note(task: &Task, q: &Question, resolution: &str) -> String {
445 let marker = format!("{} operator: {resolution}", marker_for(q));
446 match task.hold_reason.as_deref() {
447 Some(existing) if !existing.is_empty() => format!("{existing}\n{marker}"),
448 _ => marker,
449 }
450}
451
452/// File a fresh triage question for `task` and return it. The caller is
453/// responsible for having already established there is no open one - see
454/// [`latest_triage_question`] - so this never checks again.
455fn file_question(
456 questions: &Questions,
457 task: &Task,
458 bucket: Bucket,
459 w: &Wording,
460 now: Timestamp,
461) -> Option<Question> {
462 let (summary, why, choices) = match bucket {
463 Bucket::MachineUnknown => (
464 w.summary_machine_unknown(task),
465 w.why_machine(),
466 w.choices3(),
467 ),
468 Bucket::Legacy => (w.summary_legacy(task), w.why_legacy(), w.choices3()),
469 Bucket::ManualStale => {
470 let days = (now.as_second() - task.updated_at.as_second()) / (24 * 60 * 60);
471 (
472 w.summary_manual_stale(task, days),
473 w.why_manual(),
474 w.choices2(),
475 )
476 }
477 };
478 let mut q = Question::new(
479 task.id.clone(),
480 NODE.to_owned(),
481 SEAT.to_owned(),
482 summary,
483 w.detail(task, why),
484 choices,
485 );
486 questions.put(&mut q).ok()?;
487 Some(q)
488}
489
490/// Move every `blocked` task whose `blocked_by` names a task or question id
491/// that no longer exists to a machine hold, before the per-`held` walk
492/// [`run_once`] does gets a look at it.
493///
494/// `crate::daemon::resolve_blockers` already catches the same situation on
495/// every idle poll, and [`Queue::remove`] already catches it the moment a
496/// dependency is deleted through `magi task rm` - both call the same
497/// [`crate::queue::missing_blockers`]/[`crate::queue::missing_blocker_hold_reason`]
498/// this does. This third copy exists because a dependency can also be deleted
499/// by hand (the file just removed from disk, not through either of those
500/// paths), and because a queue can carry a `blocked` task with a
501/// long-since-deleted dependency from *before* either catch above ever
502/// existed - and such a task is `blocked`, never `held`, so it is invisible
503/// to the rest of this module without this pass. Running it here, first, is
504/// also what makes `magi task triage` alone - with no daemon running at all -
505/// enough to fix one: the task lands `held` in this same call, and the
506/// ordinary loop below files its question in the very same pass.
507fn quarantine_orphaned_blocked(queue: &Queue, questions: &Questions) -> Vec<String> {
508 let mut quarantined = Vec::new();
509 for listed in queue.list() {
510 if listed.status != TaskStatus::Blocked || listed.blocked_by.is_empty() {
511 continue;
512 }
513 let Ok(_claim) = queue.claim(&listed.id) else {
514 continue;
515 };
516 let Ok(mut task) = queue.get(&listed.id) else {
517 continue;
518 };
519 if task.status != TaskStatus::Blocked {
520 continue;
521 }
522 let missing = crate::queue::missing_blockers(queue, questions, &task.blocked_by);
523 if missing.is_empty() {
524 continue;
525 }
526 task.hold_machine(Some(crate::queue::missing_blocker_hold_reason(
527 &task.blocked_by,
528 &missing,
529 )));
530 if queue.put(&mut task).is_ok() {
531 quarantined.push(task.id.clone());
532 }
533 }
534 quarantined
535}
536
537/// Run one deterministic triage pass over every `held` task in `queue`. No
538/// model call anywhere in this function - see this module's own doc for what
539/// each `HoldSource` gets instead.
540///
541/// `config_override` is threaded straight to [`Config::discover`], the same
542/// role `daemon::Opts::config` plays for `daemon::prepare` - an explicit
543/// `--config` from the caller, or `None` to let each task's own repository
544/// pick its layers.
545///
546/// Safe to call on every daemon idle tick and from `magi task triage` alike:
547/// a task already answered and applied is left alone (see
548/// [`already_applied`]), and a task with an open question is left alone too,
549/// so repeated calls with nothing new to say do nothing.
550///
551/// Also runs [`quarantine_orphaned_blocked`] first, so a `blocked` task whose
552/// dependency no longer exists is caught and turned into a fresh `held`
553/// question in this same pass, not left for a later call to notice.
554pub fn run_once(
555 queue: &Queue,
556 questions: &Questions,
557 config_override: Option<&Path>,
558 now: Timestamp,
559) -> Report {
560 let mut report = Report {
561 quarantined: quarantine_orphaned_blocked(queue, questions),
562 ..Report::default()
563 };
564 for listed in queue.list() {
565 if listed.status != TaskStatus::Held {
566 continue;
567 }
568 let Ok(_claim) = queue.claim(&listed.id) else {
569 continue;
570 };
571 let Ok(mut task) = queue.get(&listed.id) else {
572 continue;
573 };
574 // Re-read under the claim: a release or a re-hold landed by a human
575 // between the listing above and the claim just taken must not be
576 // clobbered by a decision based on the stale copy.
577 if task.status != TaskStatus::Held {
578 continue;
579 }
580
581 let cfg = Config::discover(&repo_for(&task), config_override)
582 .ok()
583 .map(|(c, _)| c);
584 let w = wording(cfg.as_ref().map_or("en", |c| c.graph.language.as_str()));
585
586 if let Some(q) = latest_triage_question(questions, &task.id) {
587 if q.status.open() {
588 // Already asked, still waiting - nothing to do this pass.
589 continue;
590 }
591 if q.status == QuestionStatus::Answered && !already_applied(&task, &q) {
592 match interpret_answer(&q) {
593 AnswerAction::Resume => {
594 task.release();
595 if queue.put(&mut task).is_ok() {
596 report.answered.push(task.id.clone());
597 }
598 }
599 AnswerAction::Discard => {
600 if queue.remove(&task.id, false, questions).is_ok() {
601 report.answered.push(task.id.clone());
602 }
603 }
604 AnswerAction::KeepHeld => {
605 let resolution = q.resolution().unwrap_or_default();
606 let note = keep_held_note(&task, &q, &resolution);
607 task.hold_manual(Some(note));
608 if queue.put(&mut task).is_ok() {
609 report.answered.push(task.id.clone());
610 }
611 }
612 }
613 continue;
614 }
615 // Abandoned, or an already-applied answer: fall through to the
616 // ordinary per-source handling below, which is how a stale
617 // `HoldSource::Manual` re-ask - or a fresh machine/legacy
618 // question, once a prior one settled the task back into a hold -
619 // gets filed.
620 }
621
622 match task.hold_source {
623 Some(HoldSource::Machine) => {
624 if cfg.as_ref().and_then(|c| machine_cause_resolved(&task, c)) == Some(true) {
625 task.release();
626 if queue.put(&mut task).is_ok() {
627 report.resumed.push(task.id.clone());
628 }
629 } else if file_question(questions, &task, Bucket::MachineUnknown, w, now).is_some()
630 {
631 report.asked.push(task.id.clone());
632 }
633 }
634 None => {
635 if file_question(questions, &task, Bucket::Legacy, w, now).is_some() {
636 report.asked.push(task.id.clone());
637 }
638 }
639 Some(HoldSource::Manual) => {
640 if manual_is_stale(&task, now)
641 && file_question(questions, &task, Bucket::ManualStale, w, now).is_some()
642 {
643 report.asked.push(task.id.clone());
644 }
645 }
646 }
647 }
648 report
649}
650
651/// The open triage question about `task_id`, if any - what `magi task show`
652/// prints so a held task's card names the question waiting on it, not only
653/// its hold reason. `None` once it is answered or abandoned: nothing is
654/// waiting on it anymore.
655pub fn open_question_for(questions: &Questions, task_id: &str) -> Option<Question> {
656 latest_triage_question(questions, task_id).filter(|q| q.status.open())
657}
658
659/// Does this module still have unfinished business with `task`?
660///
661/// True while its latest triage question is still open (waiting on an
662/// answer), and true for a beat longer than [`open_question_for`] alone
663/// would say: once answered, the question sits [`QuestionStatus::Answered`]
664/// but unread until the next [`run_once`] pass actually applies it (see
665/// [`already_applied`]), and [`run_once`] only ever runs on a fully idle
666/// daemon tick - far less often than `crate::conduct` polls. A caller that
667/// only checked "is a question open" would walk straight through that gap
668/// the moment the operator answers, moving the task out of `held` before
669/// [`run_once`] gets a turn - orphaning the very answer it was about to
670/// apply, the same failure mode this function exists to keep `crate::conduct`
671/// out of. `crate::conduct::apply_one` is exactly that caller.
672pub fn pending_for(questions: &Questions, task: &Task) -> bool {
673 match latest_triage_question(questions, &task.id) {
674 Some(q) if q.status.open() => true,
675 Some(q) if q.status == QuestionStatus::Answered => !already_applied(task, &q),
676 _ => false,
677 }
678}
679
680/// Every task id with an open triage question right now - what `magi task
681/// list` uses to mark a held task that is already waiting on an operator
682/// decision, rather than have it read identically to one nobody has looked
683/// at yet.
684pub fn open_task_ids(questions: &Questions) -> std::collections::BTreeSet<String> {
685 questions
686 .list()
687 .into_iter()
688 .filter(|q| q.node == NODE && q.status.open())
689 .map(|q| q.run)
690 .collect()
691}
692
693#[cfg(test)]
694mod tests {
695 use super::*;
696 use crate::ask::Answer;
697 use crate::queue::Source;
698 use jiff::SignedDuration;
699
700 fn store() -> (tempfile::TempDir, Queue, Questions) {
701 let dir = tempfile::tempdir().unwrap();
702 let q = Queue::at(dir.path().join("queue"));
703 let s = Questions::at(dir.path().join("questions"));
704 (dir, q, s)
705 }
706
707 fn task(title: &str, repo: PathBuf) -> Task {
708 Task::new(title.to_owned(), format!("do {title}"), repo, Source::Human)
709 }
710
711 /// `[disk] min_free_bytes = 0` written next to a fictional repo, so
712 /// `machine_cause_resolved` never has to ask the real disk anything - the
713 /// same fixture pattern `daemon`'s own idle-loop tests use.
714 fn gate_disabled_config(dir: &std::path::Path) -> PathBuf {
715 let config = dir.join("magi.toml");
716 std::fs::write(&config, "[disk]\nmin_free_bytes = 0\n").unwrap();
717 config
718 }
719
720 #[test]
721 fn a_resolved_machine_hold_is_requeued_automatically() {
722 let (dir, q, questions) = store();
723 let config = gate_disabled_config(dir.path());
724 let mut t = task("disk pressure", dir.path().join("repo"));
725 t.hold_machine(Some(
726 "not enough free space to start a run: 10 bytes free, 100 required by \
727 `[disk] min_free_bytes`"
728 .to_owned(),
729 ));
730 q.put(&mut t).unwrap();
731
732 let report = run_once(&q, &questions, Some(&config), Timestamp::now());
733 assert_eq!(report.resumed, [t.id.clone()]);
734 assert!(report.asked.is_empty());
735
736 let back = q.get(&t.id).unwrap();
737 assert_eq!(back.status, TaskStatus::Queued);
738 assert!(back.hold_source.is_none());
739 assert!(questions.list().is_empty(), "nothing needed asking");
740 }
741
742 #[test]
743 fn a_machine_hold_with_no_recognised_cause_gets_one_question_not_two() {
744 let (dir, q, questions) = store();
745 let mut t = task("gate went red", dir.path().join("repo"));
746 t.hold_machine(Some("gate red".to_owned()));
747 q.put(&mut t).unwrap();
748
749 let first = run_once(&q, &questions, None, Timestamp::now());
750 assert_eq!(first.asked, [t.id.clone()]);
751 assert!(first.resumed.is_empty());
752
753 let open: Vec<_> = questions
754 .list()
755 .into_iter()
756 .filter(|q| q.status.open())
757 .collect();
758 assert_eq!(open.len(), 1);
759 assert_eq!(open[0].run, t.id);
760 assert_eq!(open[0].node, NODE);
761 assert_eq!(open[0].choices.len(), 3);
762
763 // A second pass with nothing new must not file a second question.
764 let second = run_once(&q, &questions, None, Timestamp::now());
765 assert!(second.asked.is_empty());
766 assert_eq!(
767 questions
768 .list()
769 .into_iter()
770 .filter(|q| q.status.open())
771 .count(),
772 1
773 );
774 }
775
776 #[test]
777 fn a_legacy_hold_with_no_recorded_source_gets_exactly_one_question() {
778 let (dir, q, questions) = store();
779 let mut t = task("schema 1 record", dir.path().join("repo"));
780 t.status = TaskStatus::Held;
781 assert!(t.hold_source.is_none(), "the case this test is about");
782 q.put(&mut t).unwrap();
783
784 let first = run_once(&q, &questions, None, Timestamp::now());
785 assert_eq!(first.asked, [t.id.clone()]);
786
787 let second = run_once(&q, &questions, None, Timestamp::now());
788 assert!(
789 second.asked.is_empty(),
790 "the same legacy hold must not be asked about twice"
791 );
792 assert_eq!(
793 questions
794 .list()
795 .into_iter()
796 .filter(|q| q.status.open())
797 .count(),
798 1
799 );
800 }
801
802 #[test]
803 fn a_manual_hold_is_never_auto_resumed() {
804 let (dir, q, questions) = store();
805 let mut t = task("operator stopped this", dir.path().join("repo"));
806 t.hold_manual(Some("waiting on a decision".to_owned()));
807 q.put(&mut t).unwrap();
808
809 let report = run_once(&q, &questions, None, Timestamp::now());
810 assert!(report.resumed.is_empty());
811 // Fresh, not stale yet - no question either.
812 assert!(report.asked.is_empty());
813
814 let back = q.get(&t.id).unwrap();
815 assert_eq!(back.status, TaskStatus::Held);
816 assert_eq!(back.hold_source, Some(HoldSource::Manual));
817 assert!(questions.list().is_empty());
818 }
819
820 #[test]
821 fn a_blocked_task_on_a_deleted_dependency_is_held_and_asked_about_in_one_pass() {
822 // The five real tasks this whole change exists for are `blocked`, not
823 // `held`, and no daemon has to be running for `magi task triage` alone
824 // to reach them - `run_once` must both quarantine and ask in the same
825 // call.
826 let (dir, q, questions) = store();
827 let mut still_going = task("still valid", dir.path().join("repo"));
828 q.put(&mut still_going).unwrap();
829
830 let mut t = task("orphaned", dir.path().join("repo"));
831 t.block(
832 vec!["20260101-000000-gone".to_owned(), still_going.id.clone()],
833 Some("waits on both".to_owned()),
834 );
835 q.put(&mut t).unwrap();
836
837 let report = run_once(&q, &questions, None, Timestamp::now());
838 assert_eq!(report.quarantined, [t.id.clone()]);
839 assert_eq!(
840 report.asked,
841 [t.id.clone()],
842 "the fresh machine hold must earn a question in the same pass"
843 );
844
845 let after = q.get(&t.id).unwrap();
846 assert_eq!(after.status, TaskStatus::Held);
847 assert_eq!(after.hold_source, Some(HoldSource::Machine));
848 assert!(after.blocked_by.is_empty());
849
850 // The reason is not disk-pressure wording, so this must not be read
851 // as a disk hold and silently auto-resumed.
852 assert!(!is_disk_hold(&after));
853
854 let open: Vec<_> = questions
855 .list()
856 .into_iter()
857 .filter(|q| q.status.open())
858 .collect();
859 assert_eq!(open.len(), 1);
860 assert_eq!(open[0].run, t.id);
861
862 // A second pass with nothing new files no second question.
863 let second = run_once(&q, &questions, None, Timestamp::now());
864 assert!(second.quarantined.is_empty());
865 assert!(second.asked.is_empty());
866 }
867
868 #[test]
869 fn a_stale_manual_hold_earns_a_two_choice_question() {
870 let (dir, q, questions) = store();
871 let mut t = task("been sitting a while", dir.path().join("repo"));
872 t.hold_manual(Some("waiting on a decision".to_owned()));
873 q.put(&mut t).unwrap();
874 // Back-date the hold past MANUAL_STALE_AFTER without waiting a week.
875 let mut back = q.get(&t.id).unwrap();
876 back.updated_at = Timestamp::now() - SignedDuration::new(8 * 24 * 60 * 60, 0);
877 std::fs::write(
878 q.path_of(&back.id),
879 serde_json::to_string_pretty(&back).unwrap(),
880 )
881 .unwrap();
882
883 let report = run_once(&q, &questions, None, Timestamp::now());
884 assert_eq!(report.asked, [t.id.clone()]);
885 let open: Vec<_> = questions
886 .list()
887 .into_iter()
888 .filter(|q| q.status.open())
889 .collect();
890 assert_eq!(open.len(), 1);
891 assert_eq!(open[0].choices.len(), 2);
892 }
893
894 #[test]
895 fn answering_resume_releases_the_task() {
896 let (dir, q, questions) = store();
897 let mut t = task("gate went red", dir.path().join("repo"));
898 t.hold_machine(Some("gate red".to_owned()));
899 q.put(&mut t).unwrap();
900 run_once(&q, &questions, None, Timestamp::now());
901
902 let mut asked = questions
903 .list()
904 .into_iter()
905 .find(|q| q.run == t.id)
906 .unwrap();
907 asked.answer(Answer::Choice(EN.resume.to_owned())).unwrap();
908 questions.put(&mut asked).unwrap();
909
910 let report = run_once(&q, &questions, None, Timestamp::now());
911 assert_eq!(report.answered, [t.id.clone()]);
912 let back = q.get(&t.id).unwrap();
913 assert_eq!(back.status, TaskStatus::Queued);
914 assert!(back.hold_source.is_none());
915 }
916
917 #[test]
918 fn answering_not_yet_keeps_it_held_as_a_manual_hold_and_does_not_reapply() {
919 let (dir, q, questions) = store();
920 let mut t = task("gate went red", dir.path().join("repo"));
921 t.hold_machine(Some("gate red".to_owned()));
922 q.put(&mut t).unwrap();
923 run_once(&q, &questions, None, Timestamp::now());
924
925 let mut asked = questions
926 .list()
927 .into_iter()
928 .find(|q| q.run == t.id)
929 .unwrap();
930 asked.answer(Answer::Choice(EN.wait.to_owned())).unwrap();
931 questions.put(&mut asked).unwrap();
932
933 let report = run_once(&q, &questions, None, Timestamp::now());
934 assert_eq!(report.answered, [t.id.clone()]);
935 let back = q.get(&t.id).unwrap();
936 assert_eq!(back.status, TaskStatus::Held);
937 assert_eq!(back.hold_source, Some(HoldSource::Manual));
938 assert!(
939 back.hold_reason
940 .as_deref()
941 .is_some_and(|r| r.contains("gate red")),
942 "the original cause must survive a \"not yet\" answer, not just the \
943 triage marker: {:?}",
944 back.hold_reason
945 );
946
947 // A third pass, same instant: the manual hold is fresh (just
948 // touched), so nothing more happens - in particular the already
949 // answered question is not re-applied.
950 let third = run_once(&q, &questions, None, Timestamp::now());
951 assert!(third.answered.is_empty());
952 assert!(third.asked.is_empty());
953 }
954
955 #[test]
956 fn answering_discard_removes_the_task_entirely() {
957 let (dir, q, questions) = store();
958 let mut t = task("gate went red", dir.path().join("repo"));
959 t.hold_machine(Some("gate red".to_owned()));
960 q.put(&mut t).unwrap();
961 run_once(&q, &questions, None, Timestamp::now());
962
963 let mut asked = questions
964 .list()
965 .into_iter()
966 .find(|q| q.run == t.id)
967 .unwrap();
968 asked.answer(Answer::Choice(EN.discard.to_owned())).unwrap();
969 questions.put(&mut asked).unwrap();
970
971 let report = run_once(&q, &questions, None, Timestamp::now());
972 assert_eq!(report.answered, [t.id.clone()]);
973 assert!(
974 q.get(&t.id).is_err(),
975 "\"discard it\" (捨ててよい) must actually discard the task, not \
976 just leave it sitting held forever"
977 );
978 }
979
980 #[test]
981 fn an_answer_is_read_by_its_position_in_choices_not_by_the_callers_current_language() {
982 // Filed while the repo's config reads Japanese...
983 let (dir, q, questions) = store();
984 let ja_config = dir.path().join("ja.toml");
985 std::fs::write(&ja_config, "[graph]\nlanguage = \"ja\"\n").unwrap();
986 let mut t = task("gate went red", dir.path().join("repo"));
987 t.hold_machine(Some("gate red".to_owned()));
988 q.put(&mut t).unwrap();
989 run_once(&q, &questions, Some(&ja_config), Timestamp::now());
990
991 let mut asked = questions
992 .list()
993 .into_iter()
994 .find(|q| q.run == t.id)
995 .unwrap();
996 assert_eq!(asked.choices[0], JA.resume, "filed in Japanese");
997 asked.answer(Answer::Choice(JA.resume.to_owned())).unwrap();
998 questions.put(&mut asked).unwrap();
999
1000 // ...but applied against an English config (a later `--config`, or an
1001 // edited repository config). The Japanese "再開してよい" answer must
1002 // still be read as a resume, not silently misread as "keep held"
1003 // because it fails a text comparison against the English wording.
1004 let en_config = dir.path().join("en.toml");
1005 std::fs::write(&en_config, "[graph]\nlanguage = \"en\"\n").unwrap();
1006 let report = run_once(&q, &questions, Some(&en_config), Timestamp::now());
1007 assert_eq!(report.answered, [t.id.clone()]);
1008 let back = q.get(&t.id).unwrap();
1009 assert_eq!(
1010 back.status,
1011 TaskStatus::Queued,
1012 "a resume answer must resume the task regardless of which \
1013 language it is read back in"
1014 );
1015 }
1016
1017 #[test]
1018 fn a_question_falls_back_to_last_error_when_hold_reason_was_never_set() {
1019 // `Task::fail` - the ordinary "out of attempts" machine hold - only
1020 // ever sets `last_error`, never `hold_reason`. The question detail
1021 // must still name a cause rather than reading "(none recorded)".
1022 let (dir, q, questions) = store();
1023 let mut t = task("kept failing the gate", dir.path().join("repo"));
1024 t.start("run-1".to_owned());
1025 t.fail("gate red three times running", 1);
1026 assert_eq!(t.status, TaskStatus::Held);
1027 assert!(t.hold_reason.is_none(), "the case this test is about");
1028 q.put(&mut t).unwrap();
1029
1030 run_once(&q, &questions, None, Timestamp::now());
1031 let asked = questions
1032 .list()
1033 .into_iter()
1034 .find(|q| q.run == t.id)
1035 .unwrap();
1036 assert!(
1037 asked.detail.contains("gate red three times running"),
1038 "the question must surface `last_error` when there is no \
1039 `hold_reason` to show instead: {}",
1040 asked.detail
1041 );
1042 }
1043
1044 #[test]
1045 fn open_question_for_and_open_task_ids_reflect_only_what_is_still_waiting() {
1046 let (dir, q, questions) = store();
1047 let mut t = task("gate went red", dir.path().join("repo"));
1048 t.hold_machine(Some("gate red".to_owned()));
1049 q.put(&mut t).unwrap();
1050
1051 assert!(open_question_for(&questions, &t.id).is_none());
1052 assert!(!open_task_ids(&questions).contains(&t.id));
1053
1054 run_once(&q, &questions, None, Timestamp::now());
1055 assert!(open_question_for(&questions, &t.id).is_some());
1056 assert!(open_task_ids(&questions).contains(&t.id));
1057
1058 let mut asked = questions
1059 .list()
1060 .into_iter()
1061 .find(|q| q.run == t.id)
1062 .unwrap();
1063 asked.answer(Answer::Choice(EN.resume.to_owned())).unwrap();
1064 questions.put(&mut asked).unwrap();
1065
1066 assert!(
1067 open_question_for(&questions, &t.id).is_none(),
1068 "an answered question is no longer open"
1069 );
1070 assert!(!open_task_ids(&questions).contains(&t.id));
1071 }
1072
1073 #[test]
1074 fn pending_for_stays_true_between_an_answer_and_the_next_run_once_pass() {
1075 // `open_question_for` alone goes `None` the instant the operator
1076 // answers, well before `run_once` - idle-tick only - gets a turn to
1077 // actually apply that answer (see `already_applied`). `pending_for`
1078 // exists so a caller polling far more often than `run_once` does -
1079 // `crate::conduct::apply_one` - does not walk through that gap.
1080 let (dir, q, questions) = store();
1081 let mut t = task("gate went red", dir.path().join("repo"));
1082 t.hold_machine(Some("gate red".to_owned()));
1083 q.put(&mut t).unwrap();
1084
1085 assert!(!pending_for(&questions, &t));
1086
1087 run_once(&q, &questions, None, Timestamp::now());
1088 let held = q.get(&t.id).unwrap();
1089 assert!(pending_for(&questions, &held), "still waiting on an answer");
1090
1091 let mut asked = questions
1092 .list()
1093 .into_iter()
1094 .find(|q| q.run == t.id)
1095 .unwrap();
1096 asked.answer(Answer::Choice(EN.wait.to_owned())).unwrap();
1097 questions.put(&mut asked).unwrap();
1098 assert!(!asked.status.open());
1099
1100 // The race window: answered, but `run_once` has not run again yet.
1101 let still_held = q.get(&t.id).unwrap();
1102 assert!(
1103 pending_for(&questions, &still_held),
1104 "answered but not yet applied is still pending"
1105 );
1106
1107 run_once(&q, &questions, None, Timestamp::now());
1108 let after = q.get(&t.id).unwrap();
1109 assert!(
1110 !pending_for(&questions, &after),
1111 "the answer is applied now, nothing left pending"
1112 );
1113 }
1114}