Skip to main content

magi/
talk.rs

1//! The standing conversation: a place to think out loud with an agent between
2//! tasks, reachable from a phone.
3//!
4//! [`crate::chat`] is an interview with one purpose - arrive at a task file
5//! and file it - and it ends the moment that happens. This module is the other
6//! kind of conversation an operator wants: one that stays open. Ask a
7//! question, have the agent read a file or run a command to check something,
8//! talk through an idea, and when it is time to act, tell it to file the work
9//! rather than do it here. The conversation does not end; it is what the
10//! operator opens the next time something comes up.
11//!
12//! # Talking is not implementing
13//!
14//! Every turn here runs with `allow_write: false` - the same restriction
15//! [`crate::chat`] puts on its own interview, for the same reason. An agent
16//! that can edit files while the operator is mid-thought can leave the
17//! checkout in a state neither of them chose. When the operator wants a
18//! change made, the agent is told to run `magi task add --solo`
19//! ([`briefing`]) rather than reach for an editor: the change goes through
20//! magi's own queue, on the repository's own terms, and the operator can
21//! watch it happen instead of trusting that it did.
22//!
23//! `--solo` rather than a plain `magi task add` is the point of pairing this
24//! module with [`crate::queue::Task::solo`]. A task that came out of a
25//! conversation the operator just had is a decision already made, not a
26//! design question worth three independent takes - so it runs through one
27//! implementer and straight into review, the way [`crate::graph::Runner`]
28//! already degrades a single-candidate run.
29//!
30//! # Shape
31//!
32//! The same split [`crate::chat`] and [`crate::queue`] use: [`Talk`] is data
33//! plus pure helpers, [`Talks`] owns the I/O and is constructed with its root,
34//! so every test here drives a real store in a temp directory rather than the
35//! operator's own home.
36
37use std::path::{Path, PathBuf};
38use std::sync::{Arc, Mutex, MutexGuard, PoisonError};
39use std::time::Duration;
40
41use anyhow::{Context, Result, bail};
42use jiff::Timestamp;
43use serde::{Deserialize, Serialize};
44
45use crate::agent::{self, Invocation, SeatState};
46use crate::config::Config;
47use crate::plan;
48use crate::queue::{Queue, Source, Task};
49
50/// On-disk format for a conversation. Bumped when a field's meaning changes.
51pub const SCHEMA: u32 = 1;
52
53/// Wall-clock limit for one agent turn.
54///
55/// Fifteen minutes, three times [`crate::chat::TURN_TIMEOUT`]. A planning turn
56/// answers a question about intent; a turn here is expected to run several
57/// shell commands and read their output before answering one - "what does
58/// this function do", "is this still true", "run the tests and tell me" - and
59/// a five-minute budget cuts that off mid-investigation on exactly the
60/// conversation meant to support it.
61const TURN_TIMEOUT: Duration = Duration::from_secs(900);
62
63/// Seat name for the conversation's agent, scoping its CLI-side session away
64/// from every other seat magi ever opens - the same rule [`crate::chat`]
65/// applies to its own interviewer.
66const SEAT: &str = "talk";
67
68/// Prefix on a turn magi wrote rather than an agent. See
69/// [`crate::chat::MAGI_NOTE`], which this mirrors.
70const MAGI_NOTE: &str = "magi: ";
71
72/// Who said something.
73#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
74#[serde(rename_all = "lowercase")]
75pub enum Who {
76    /// The operator.
77    Operator,
78    /// The conversation's agent - or magi itself, reporting that a turn
79    /// failed. See [`MAGI_NOTE`].
80    Agent,
81}
82
83/// One message in the conversation.
84#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
85#[serde(deny_unknown_fields)]
86pub struct Turn {
87    /// Who wrote it.
88    pub who: Who,
89    /// What they said.
90    pub body: String,
91    /// When it was said.
92    pub at: Timestamp,
93}
94
95/// Where a conversation is in its life. Unlike [`crate::chat::ChatStatus`]
96/// there is no `filed`: this conversation can file any number of tasks
97/// without ending, so it only ever moves once, from open to closed.
98#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
99#[serde(rename_all = "lowercase")]
100pub enum TalkStatus {
101    /// Still open; the operator may say more, and may have already filed work
102    /// out of it.
103    Open,
104    /// Closed by hand. Kept on disk as a record.
105    Closed,
106}
107
108impl TalkStatus {
109    /// Is this conversation still live?
110    pub fn open(self) -> bool {
111        matches!(self, Self::Open)
112    }
113
114    /// Wire form, for the phone and for logs.
115    pub fn as_str(self) -> &'static str {
116        match self {
117            Self::Open => "open",
118            Self::Closed => "closed",
119        }
120    }
121}
122
123/// One standing conversation.
124#[derive(Debug, Clone, Serialize, Deserialize)]
125#[serde(deny_unknown_fields)]
126pub struct Talk {
127    /// On-disk format version.
128    pub schema: u32,
129    /// Conversation id, e.g. `20260904-014455-ab12`.
130    pub id: String,
131    /// Repository this conversation is about.
132    pub repo: PathBuf,
133    /// Roster agent id holding the conversation.
134    pub agent: String,
135    /// Current state.
136    pub status: TalkStatus,
137    /// Everything said, oldest first.
138    pub turns: Vec<Turn>,
139    /// When the conversation was opened.
140    pub created_at: Timestamp,
141    /// Last change to this file.
142    pub updated_at: Timestamp,
143    /// The CLI-side conversation, so a turn after the first costs one
144    /// sentence instead of the whole transcript. Not `pub` for the same
145    /// reason [`crate::chat::Chat`]'s is not: it is magi's bookkeeping, and a
146    /// caller that edited it would detach the record from the conversation
147    /// the model actually holds.
148    seat: SeatState,
149}
150
151impl Talk {
152    /// Short form used in lists and notifications, matching a run's short id.
153    pub fn short(&self) -> &str {
154        short(&self.id)
155    }
156}
157
158/// A conversation store on disk.
159#[derive(Debug, Clone)]
160pub struct Talks {
161    root: PathBuf,
162    /// Serializes the read-modify-write cycle that reads a talk, decides
163    /// something from its `status`, and writes the whole record back.
164    /// [`close`], [`record`] and the tail of [`turn`] all take this before
165    /// that cycle rather than after just the read: a re-read narrows the
166    /// window another writer can land in, but does not close it, since
167    /// nothing stopped that other writer's own put from landing between this
168    /// call's re-read and its own put. Shared across every clone, since every
169    /// clone is a handle onto the same files.
170    lock: Arc<Mutex<()>>,
171}
172
173impl Talks {
174    /// The operator's conversations, `<home>/talks`.
175    pub fn open() -> Self {
176        Self::at(crate::run::home().join("talks"))
177    }
178
179    /// A store at an explicit root. Tests use this, which is why none of them
180    /// need the operator's real home.
181    pub fn at(root: PathBuf) -> Self {
182        Self {
183            root,
184            lock: Arc::new(Mutex::new(())),
185        }
186    }
187
188    /// Claim the right to read-modify-write a talk's `status`. A plain
189    /// `std::sync::Mutex`, not an async one: every caller holds it across a
190    /// handful of small file operations and never across an `.await`, so
191    /// blocking the thread briefly is the right tool, not a reason to reach
192    /// for `tokio::sync::Mutex`. Poisoning recovers rather than propagates -
193    /// one panicking caller must not wedge every talk in the store the way it
194    /// would wedge the loop's own lock; see [`crate::web`]'s `lock_or_recover`,
195    /// which this mirrors.
196    fn guard(&self) -> MutexGuard<'_, ()> {
197        self.lock.lock().unwrap_or_else(PoisonError::into_inner)
198    }
199
200    /// Directory holding the conversation files.
201    pub fn root(&self) -> &Path {
202        &self.root
203    }
204
205    /// Path for one conversation id.
206    pub fn path_of(&self, id: &str) -> PathBuf {
207        self.root.join(format!("{id}.json"))
208    }
209
210    /// Where one conversation's prompts and CLI output are kept, beside the
211    /// record rather than inside it - see [`crate::chat::Chats::artifacts_of`].
212    pub fn artifacts_of(&self, id: &str) -> PathBuf {
213        self.root.join(format!("{id}.artifacts"))
214    }
215
216    /// Write a conversation, atomically, so a process killed mid-write leaves
217    /// the previous state readable rather than a truncated file.
218    pub fn put(&self, t: &mut Talk) -> Result<()> {
219        std::fs::create_dir_all(&self.root)
220            .with_context(|| format!("create {}", self.root.display()))?;
221        t.updated_at = Timestamp::now();
222        let body = serde_json::to_string_pretty(t).context("serialize talk")?;
223        let path = self.path_of(&t.id);
224        let tmp = path.with_extension("json.tmp");
225        std::fs::write(&tmp, &body).with_context(|| format!("write {}", tmp.display()))?;
226        std::fs::rename(&tmp, &path).with_context(|| format!("replace {}", path.display()))?;
227        Ok(())
228    }
229
230    /// Load a conversation by id or unambiguous id prefix.
231    pub fn get(&self, id: &str) -> Result<Talk> {
232        let resolved = self.resolve_id(id)?;
233        read_path(&self.path_of(&resolved))
234    }
235
236    /// Every conversation on disk: open first, then newest first - the same
237    /// ordering [`crate::chat::Chats::list`] uses, for the same reason: what
238    /// the operator is still using belongs above what they are done with.
239    pub fn list(&self) -> Vec<Talk> {
240        let mut all: Vec<Talk> = std::fs::read_dir(&self.root)
241            .into_iter()
242            .flatten()
243            .flatten()
244            .map(|e| e.path())
245            .filter(|p| p.extension().is_some_and(|x| x == "json"))
246            .filter_map(|p| read_path(&p).ok())
247            .collect();
248        all.sort_unstable_by(|a, b| {
249            let rank = |t: &Talk| u8::from(!t.status.open());
250            rank(a).cmp(&rank(b)).then_with(|| b.id.cmp(&a.id))
251        });
252        all
253    }
254
255    /// Expand an id prefix to exactly one conversation id.
256    pub fn resolve_id(&self, prefix: &str) -> Result<String> {
257        if self.path_of(prefix).is_file() {
258            return Ok(prefix.to_owned());
259        }
260        let hits: Vec<String> = self
261            .list()
262            .into_iter()
263            .map(|t| t.id)
264            .filter(|id| id.starts_with(prefix) || id.ends_with(prefix))
265            .collect();
266        match hits.len() {
267            1 => Ok(hits.into_iter().next().expect("exactly one hit")),
268            0 => bail!("no talk matches `{prefix}`"),
269            _ => bail!(
270                "`{prefix}` matches {} talks: {}",
271                hits.len(),
272                hits.join(", ")
273            ),
274        }
275    }
276
277    /// Change detection token, the same shape as
278    /// [`crate::chat::Chats::revision`]: the newest modification time in the
279    /// store, in milliseconds.
280    pub fn revision(&self) -> u64 {
281        std::fs::read_dir(&self.root)
282            .into_iter()
283            .flatten()
284            .flatten()
285            .filter_map(|e| e.metadata().ok())
286            .filter_map(|m| m.modified().ok())
287            .filter_map(|t| t.duration_since(std::time::UNIX_EPOCH).ok())
288            .map(|d| d.as_millis() as u64)
289            .max()
290            .unwrap_or(0)
291    }
292
293    /// How many conversations are still open.
294    pub fn count_open(&self) -> usize {
295        self.list().iter().filter(|t| t.status.open()).count()
296    }
297}
298
299/// Open a conversation. Unlike [`crate::chat::start`] this takes no agent
300/// turn: there is no idea to answer yet, and a conversation the operator has
301/// not said anything into yet is a normal, valid thing to have sitting on the
302/// phone.
303///
304/// `agent` is resolved the same way `magi plan` and [`crate::chat::start`]
305/// resolve their interviewer: [`plan::pick`] against `[roles] planner`, so
306/// this surface adds no configuration of its own.
307pub fn begin(store: &Talks, cfg: &Config, repo: PathBuf, agent: Option<&str>) -> Result<Talk> {
308    // Absolute, for the same reason `chat::start` canonicalizes: a relative
309    // path means the wrong repository once anything other than this process
310    // reads it back.
311    let repo = repo.canonicalize().unwrap_or(repo);
312    let want = agent.or(cfg.roles.planner.as_deref());
313    let spec = plan::pick(&cfg.agents, want, &plan::installed)?;
314
315    let now = Timestamp::now();
316    let mut talk = Talk {
317        schema: SCHEMA,
318        id: new_id(),
319        repo,
320        agent: spec.id.clone(),
321        status: TalkStatus::Open,
322        turns: Vec::new(),
323        created_at: now,
324        updated_at: now,
325        seat: SeatState::new(SEAT, &spec.id, crate::rng::entropy()),
326    };
327    store.put(&mut talk)?;
328    Ok(talk)
329}
330
331/// Append the operator's turn and flush it, without invoking anything.
332///
333/// Split out of [`say`] for the same reason [`crate::chat::record`] is split
334/// out of [`crate::chat::say`]: `POST /api/talks/{id}/say` answers once the
335/// message is safely on disk, and runs the agent's half in the background -
336/// see that function's doc for why holding the connection for a turn that can
337/// run fifteen minutes is the wrong shape for a phone.
338pub fn record(talk: &mut Talk, store: &Talks, text: &str) -> Result<String> {
339    // `web::talk_say` reads the talk, then awaits config discovery before
340    // calling this - a gap a concurrent `POST /api/talks/{id}/close` can land
341    // in. The guard held for the rest of this function is what actually closes
342    // that gap: re-reading status without it only shrinks the window a
343    // concurrent `close` could land in between this call's own read and its
344    // `put`, it does not remove it. See [`Talks::guard`] and the matching
345    // guard in `turn`, which this mirrors.
346    let _guard = store.guard();
347    if let Ok(fresh) = store.get(&talk.id) {
348        talk.status = fresh.status;
349    }
350    if !talk.status.open() {
351        bail!(
352            "talk {} is {} and takes no more turns",
353            talk.short(),
354            talk.status.as_str()
355        );
356    }
357    let text = text.trim();
358    if text.is_empty() {
359        bail!("nothing to say");
360    }
361    talk.turns.push(Turn {
362        who: Who::Operator,
363        body: text.to_owned(),
364        at: Timestamp::now(),
365    });
366    store.put(talk)?;
367    Ok(text.to_owned())
368}
369
370/// One operator turn and one agent turn, appended - the synchronous form, used
371/// by tests and by anything that is fine waiting out the turn itself.
372pub async fn say(talk: &mut Talk, store: &Talks, cfg: &Config, text: &str) -> Result<()> {
373    let text = record(talk, store, text)?;
374    turn(talk, store, cfg, &text).await
375}
376
377/// The agent's half of a turn: invoke, append, flush. Pairs with [`record`],
378/// the same way [`crate::chat::respond`] pairs with [`crate::chat::record`].
379pub async fn respond(talk: &mut Talk, store: &Talks, cfg: &Config, text: &str) -> Result<()> {
380    turn(talk, store, cfg, text).await
381}
382
383/// Close a conversation. Idempotent: closing an already-closed conversation is
384/// not an error, since the operator's intent - "I am done with this" - is
385/// already satisfied.
386///
387/// Re-reads the record under [`Talks::guard`] rather than trusting the
388/// caller's copy of `talk`, and writes that fresh copy back rather than the
389/// one passed in. `web::talk_close` loads `talk` and calls this right after
390/// with no gap of its own, but without the guard that load can still land
391/// between a `record` or `turn` elsewhere reading the file and writing it
392/// back - and a close built on the older snapshot would put it right back,
393/// silently dropping whatever turn the other call had just appended.
394pub fn close(talk: &mut Talk, store: &Talks) -> Result<()> {
395    let _guard = store.guard();
396    let mut fresh = store.get(&talk.id).unwrap_or_else(|_| talk.clone());
397    fresh.status = TalkStatus::Closed;
398    store.put(&mut fresh)?;
399    *talk = fresh;
400    Ok(())
401}
402
403/// Invoke the conversation's agent once and append what it said.
404///
405/// The first turn ever taken carries the full [`briefing`], because nothing
406/// else has told the agent what this conversation is or what it may do.
407/// Every turn after that behaves like [`crate::chat`]'s: resend nothing when
408/// the CLI can resume its own session, and fall back to [`transcript`] only
409/// when it cannot.
410async fn turn(talk: &mut Talk, store: &Talks, cfg: &Config, text: &str) -> Result<()> {
411    let spec = cfg
412        .agents
413        .iter()
414        .find(|a| a.id == talk.agent)
415        .with_context(|| {
416            format!(
417                "talk {} was opened with agent `{}`, which is no longer in \
418                 the roster; restore it in magi.toml or start a new \
419                 conversation",
420                talk.short(),
421                talk.agent
422            )
423        })?;
424
425    let resuming = agent::has_session(spec.kind, &talk.seat, cfg.graph.sessions);
426    let body = if talk.seat.turns == 0 {
427        format!(
428            "{}\n\n# Operator\n\n{text}",
429            briefing(&talk.repo, &cfg.graph.language)
430        )
431    } else if resuming {
432        text.to_owned()
433    } else {
434        format!("{}\n\n{text}", transcript(talk))
435    };
436
437    let artifacts = store.artifacts_of(&talk.id);
438    let stem = format!("turn-{}", talk.seat.turns + 1);
439    // The chat's build cache is the same shared one the graph's seats get, so
440    // a conversation that compiles does not mint another multi-GB target dir.
441    let cache_dir = cfg.cache_dir();
442    let inv = Invocation {
443        cwd: &talk.repo,
444        prompt: &body,
445        timeout: TURN_TIMEOUT,
446        // This conversation never writes to the repository: it tells the
447        // operator to run `magi task add --solo` instead, which is what keeps
448        // an implementer's diff attributable to a run rather than to a chat
449        // nobody reviewed.
450        allow_write: false,
451        sessions: cfg.graph.sessions,
452        artifacts: &artifacts,
453        stem: &stem,
454        // The conversation's own id, so `magi task add` run from inside it is
455        // attributed to this conversation - see `Source::Agent`.
456        run: &talk.id,
457        node: "chat",
458        cache_dir: cache_dir.as_deref(),
459    };
460
461    let outcome = agent::invoke(spec, &mut talk.seat, &inv).await;
462    let note = |why: String| Turn {
463        who: Who::Agent,
464        body: format!("{MAGI_NOTE}{why}"),
465        at: Timestamp::now(),
466    };
467    let (reply, failure) = match outcome {
468        Err(e) => (
469            note(format!("could not run agent `{}`: {e}", talk.agent)),
470            Some(format!("could not run agent `{}`: {e}", talk.agent)),
471        ),
472        Ok(out) if out.quota_exhausted() => {
473            let reset = out
474                .quota
475                .as_ref()
476                .and_then(|q| q.reset.clone())
477                .map_or_else(String::new, |r| format!(" (resets {r})"));
478            let why = format!(
479                "agent `{}` is out of quota{reset}; your message is saved, so \
480                 say it again when the window reopens",
481                talk.agent
482            );
483            (note(why.clone()), Some(why))
484        }
485        Ok(out) if out.timed_out => {
486            let why = format!(
487                "agent `{}` did not answer within {}s; your message is saved",
488                talk.agent,
489                TURN_TIMEOUT.as_secs()
490            );
491            (note(why.clone()), Some(why))
492        }
493        Ok(out) if !out.usable() => {
494            let why = format!(
495                "agent `{}` produced no answer (exit {}); your message is saved",
496                talk.agent,
497                out.exit_code
498                    .map_or_else(|| "unknown".to_owned(), |c| c.to_string())
499            );
500            (note(why.clone()), Some(why))
501        }
502        Ok(out) => (
503            Turn {
504                who: Who::Agent,
505                body: out.text.trim().to_owned(),
506                at: Timestamp::now(),
507            },
508            None,
509        ),
510    };
511
512    // A close landed on disk while this turn was in flight is read back here
513    // rather than trusted from the snapshot this call started with. `store`
514    // holds nothing else this function does not itself own - the turn guard
515    // in `web::Ui::begin_talk_turn` keeps `turns` and `seat` this call's
516    // alone to mutate - but `status` is not behind that guard, and an
517    // operator's close must stick: the whole point of ending a conversation
518    // is that an agent's answer to the last message before the close cannot
519    // silently reopen it. The guard is what makes that read-then-write
520    // section atomic with `close`'s own - taken only for this tail and not
521    // for the whole invocation above, so one talk's fifteen-minute turn does
522    // not block another talk's close from proceeding.
523    let _guard = store.guard();
524    if let Ok(fresh) = store.get(&talk.id) {
525        talk.status = fresh.status;
526    }
527    talk.turns.push(reply);
528    store.put(talk)?;
529
530    match failure {
531        Some(why) => bail!("{why}"),
532        None => Ok(()),
533    }
534}
535
536/// Everything said so far, as prose, for a CLI that cannot resume its own
537/// conversation. See [`crate::chat::transcript`], which this mirrors.
538fn transcript(talk: &Talk) -> String {
539    let mut out = String::from(
540        "This conversation cannot resume on the CLI's side, so here is \
541         everything said so far; answer only the last message.\n",
542    );
543    for t in &talk.turns {
544        let who = match t.who {
545            Who::Operator => "operator",
546            Who::Agent => "you",
547        };
548        out.push_str(&format!("\n## {who}\n\n{}\n", t.body.trim()));
549    }
550    out
551}
552
553/// The briefing the agent opens with, sent once as part of its first turn.
554///
555/// Pure, so the properties that matter can be asserted without an interview:
556/// it names `magi task add --solo` (the only route this conversation has to
557/// changing anything) and it does not carry
558/// [`crate::plan::TASK_FILE_SPEC`] - that spec describes a task *file*, which
559/// belongs to the planning interview and would tell this agent to write one
560/// here instead of filing through the queue.
561pub fn briefing(repo: &Path, language: &str) -> String {
562    let mut out = format!(
563        "You are magi's standing conversation partner for its operator, who \
564         usually has this open on a phone. Keep replies short: no preamble, \
565         no restating what they just said.\n\n\
566         # Repository\n\n{repo}\n\n\
567         You may look around: read files, run shell commands, search history, \
568         run tests - whatever answers the question. Do not write files. \
569         Implementing a change is not this conversation's job; a separate, \
570         blind competition of agents does that, and a repository this \
571         conversation has already edited would make their diffs unjudgeable.\n\n\
572         # When the operator wants something done\n\n\
573         Run:\n\n\
574         magi task add --solo --repo {repo} <instruction>\n\n\
575         and tell the operator the task id it prints, so they can follow it \
576         from the Queue. Write <instruction> so that an implementer who has \
577         never seen this conversation can act on it alone - it is everything \
578         they get. Use --solo: it runs the task through one implementer \
579         straight into review instead of the usual multi-agent competition, \
580         which is the right shape for a change this conversation has already \
581         settled, rather than one still worth several independent takes.\n",
582        repo = repo.display(),
583    );
584    out.push_str(&language_note(language));
585    out
586}
587
588/// The operator is talking, so their language matters here more than in most
589/// prompts magi sends - see [`crate::chat::language_note`], which this
590/// mirrors.
591fn language_note(language: &str) -> String {
592    if language.trim().is_empty() || language.eq_ignore_ascii_case("en") {
593        String::new()
594    } else {
595        format!("\nHold this conversation in {language}.\n")
596    }
597}
598
599/// Queue tasks this conversation has filed, oldest first.
600///
601/// A task is this conversation's when its [`Source::Agent`] names this
602/// conversation's id as `run` - which is exactly what happens when
603/// `magi task add` is run from inside a turn, because [`turn`] passes the
604/// conversation's own id as [`Invocation::run`].
605pub fn tasks_of(queue: &Queue, talk_id: &str) -> Vec<Task> {
606    let mut tasks: Vec<Task> = queue
607        .list()
608        .into_iter()
609        .filter(|t| matches!(&t.source, Source::Agent { run, .. } if run == talk_id))
610        .collect();
611    tasks.sort_unstable_by(|a, b| a.id.cmp(&b.id));
612    tasks
613}
614
615fn read_path(path: &Path) -> Result<Talk> {
616    let body = std::fs::read_to_string(path).with_context(|| format!("read {}", path.display()))?;
617    serde_json::from_str(&body).with_context(|| format!("parse {}", path.display()))
618}
619
620fn short(id: &str) -> &str {
621    id.split('-').next_back().unwrap_or(id)
622}
623
624fn new_id() -> String {
625    let stamp = jiff::Zoned::now().strftime("%Y%m%d-%H%M%S");
626    let seed = crate::rng::entropy();
627    format!("{stamp}-{:04x}", (seed ^ (seed >> 32)) & 0xffff)
628}
629
630#[cfg(test)]
631mod tests {
632    use std::collections::BTreeMap;
633
634    use crate::config::{AgentKind, AgentSpec, Graph};
635    use crate::queue::{Queue, Source, Task};
636
637    use super::*;
638
639    /// A store of its own, with no process-global state.
640    fn store() -> (tempfile::TempDir, Talks) {
641        let tmp = tempfile::tempdir().expect("tempdir");
642        let talks = Talks::at(tmp.path().join("talks"));
643        (tmp, talks)
644    }
645
646    /// A `kind = "command"` agent whose whole behaviour is a POSIX shell
647    /// script - see `chat`'s tests for why no test here may spawn a real
648    /// agent CLI.
649    fn mock_agent(dir: &Path, script: &str, env: BTreeMap<String, String>) -> AgentSpec {
650        let path = dir.join("mock-talk-agent.sh");
651        std::fs::write(&path, script).expect("write mock");
652        AgentSpec {
653            id: "mock".to_owned(),
654            kind: AgentKind::Command,
655            model: None,
656            command: vec!["sh".to_owned(), path.to_string_lossy().into_owned()],
657            extra_args: Vec::new(),
658            env,
659            prompt_delivery: None,
660        }
661    }
662
663    fn config(spec: AgentSpec) -> Config {
664        Config {
665            agents: vec![spec],
666            graph: Graph {
667                language: "en".to_owned(),
668                ..Graph::default()
669            },
670            ..Config::default()
671        }
672    }
673
674    /// Echo a canned reply, ignoring the prompt on stdin.
675    const REPLY: &str = "#!/bin/sh\ncat >/dev/null\nprintf '%s\\n' \"$MOCK_REPLY\"\n";
676
677    /// Say nothing and fail, the way a CLI that cannot start does.
678    const BROKEN: &str = "#!/bin/sh\ncat >/dev/null\nexit 3\n";
679
680    /// Reply with the prompt it was given, so a test can inspect exactly what
681    /// the agent received on stdin.
682    const ECHO: &str = "#!/bin/sh\ncat\n";
683
684    fn env(reply: &str) -> BTreeMap<String, String> {
685        BTreeMap::from([("MOCK_REPLY".to_owned(), reply.to_owned())])
686    }
687
688    #[test]
689    fn the_frozen_json_field_names_round_trip_through_disk() {
690        let (tmp, talks) = store();
691        let mut talk = Talk {
692            schema: SCHEMA,
693            id: "20260904-014455-ab12".to_owned(),
694            repo: tmp.path().to_owned(),
695            agent: "sonnet".to_owned(),
696            status: TalkStatus::Open,
697            turns: Vec::new(),
698            created_at: Timestamp::now(),
699            updated_at: Timestamp::now(),
700            seat: SeatState::new(SEAT, "sonnet", 7),
701        };
702        talks.put(&mut talk).expect("put");
703
704        let raw = std::fs::read_to_string(talks.path_of(&talk.id)).expect("read back");
705        let v: serde_json::Value = serde_json::from_str(&raw).expect("parse");
706        for field in [
707            "schema",
708            "id",
709            "repo",
710            "agent",
711            "status",
712            "turns",
713            "created_at",
714            "updated_at",
715        ] {
716            assert!(v.get(field).is_some(), "missing field `{field}`");
717        }
718        assert_eq!(v["schema"], 1);
719        assert_eq!(v["status"], "open");
720
721        let back = talks.get(&talk.id).expect("get");
722        assert_eq!(back.id, talk.id);
723        assert_eq!(back.status, TalkStatus::Open);
724    }
725
726    #[test]
727    fn opening_a_talk_takes_no_agent_turn() {
728        let (tmp, talks) = store();
729        // A script that would fail loudly if it were ever run: `begin` must
730        // not invoke anything, since there is nothing yet for an agent to
731        // answer.
732        let spec = mock_agent(tmp.path(), BROKEN, BTreeMap::new());
733        let cfg = config(spec);
734
735        let talk = begin(&talks, &cfg, tmp.path().to_owned(), None).expect("begin");
736        assert_eq!(talk.status, TalkStatus::Open);
737        assert!(talk.turns.is_empty(), "nothing has been said yet");
738
739        let on_disk = talks.get(&talk.id).expect("get");
740        assert_eq!(on_disk.turns.len(), 0);
741    }
742
743    #[tokio::test]
744    async fn the_first_turn_carries_the_briefing_and_later_turns_do_not() {
745        let (tmp, talks) = store();
746        let spec = mock_agent(tmp.path(), ECHO, BTreeMap::new());
747        let cfg = config(spec);
748        let mut talk = begin(&talks, &cfg, tmp.path().to_owned(), None).expect("begin");
749
750        say(&mut talk, &talks, &cfg, "what does the queue module do?")
751            .await
752            .expect("first turn");
753        let first_prompt = &talk.turns[1].body;
754        assert!(first_prompt.contains("magi task add --solo"));
755        assert!(first_prompt.contains("what does the queue module do?"));
756
757        say(&mut talk, &talks, &cfg, "and how is it locked?")
758            .await
759            .expect("second turn");
760        let second_prompt = &talk.turns[3].body;
761        assert!(
762            !second_prompt.contains("magi task add --solo"),
763            "the briefing is sent once, not on every turn: {second_prompt}"
764        );
765        assert!(second_prompt.contains("and how is it locked?"));
766    }
767
768    #[tokio::test]
769    async fn say_appends_the_operator_turn_then_the_agent_turn() {
770        let (tmp, talks) = store();
771        let spec = mock_agent(tmp.path(), REPLY, env("go ahead"));
772        let cfg = config(spec);
773        let mut talk = begin(&talks, &cfg, tmp.path().to_owned(), None).expect("begin");
774
775        say(&mut talk, &talks, &cfg, "can I rename this function?")
776            .await
777            .expect("say");
778
779        assert_eq!(talk.turns.len(), 2);
780        assert_eq!(talk.turns[0].who, Who::Operator);
781        assert_eq!(talk.turns[0].body, "can I rename this function?");
782        assert_eq!(talk.turns[1].who, Who::Agent);
783        assert_eq!(talk.turns[1].body, "go ahead");
784        assert_eq!(talks.get(&talk.id).expect("get").turns, talk.turns);
785    }
786
787    #[tokio::test]
788    async fn a_failed_turn_keeps_the_operator_message_and_says_what_happened() {
789        let (tmp, talks) = store();
790        let spec = mock_agent(tmp.path(), BROKEN, BTreeMap::new());
791        let cfg = config(spec);
792        let mut talk = begin(&talks, &cfg, tmp.path().to_owned(), None).expect("begin");
793
794        let err = say(&mut talk, &talks, &cfg, "check the tests")
795            .await
796            .expect_err("a turn with no answer is an error");
797        assert!(err.to_string().contains("no answer"), "{err}");
798
799        let on_disk = talks.get(&talk.id).expect("get");
800        assert_eq!(on_disk.turns.len(), 2);
801        assert_eq!(on_disk.turns[0].body, "check the tests");
802        let note = &on_disk.turns[1];
803        assert_eq!(note.who, Who::Agent);
804        assert!(note.body.starts_with(MAGI_NOTE), "{}", note.body);
805        assert!(note.body.contains("your message is saved"));
806    }
807
808    #[test]
809    fn closing_is_idempotent_and_a_closed_talk_takes_no_more_turns() {
810        let (tmp, talks) = store();
811        let spec = mock_agent(tmp.path(), REPLY, env("hi"));
812        let cfg = config(spec);
813        let mut talk = begin(&talks, &cfg, tmp.path().to_owned(), None).expect("begin");
814
815        close(&mut talk, &talks).expect("close");
816        assert_eq!(talk.status, TalkStatus::Closed);
817        close(&mut talk, &talks).expect("closing twice is not an error");
818
819        let err = record(&mut talk, &talks, "still there?").expect_err("closed talks refuse");
820        assert!(err.to_string().contains("closed"));
821        let _ = &cfg; // config kept only to build the agent above
822    }
823
824    #[tokio::test]
825    async fn a_close_that_lands_while_a_turn_is_in_flight_is_not_undone_by_the_reply() {
826        let (tmp, talks) = store();
827        let spec = mock_agent(tmp.path(), REPLY, env("here you go"));
828        let cfg = config(spec);
829        // The in-flight turn's own handle: loaded once, the way a spawned
830        // background task in `web::talk_say` holds one for the whole turn.
831        let mut in_flight = begin(&talks, &cfg, tmp.path().to_owned(), None).expect("begin");
832
833        // The operator closes the conversation through a *different* handle
834        // while the turn above is still running - exactly what a close typed
835        // on the phone while an agent is mid-answer looks like.
836        let mut closed_elsewhere = talks.get(&in_flight.id).expect("reread");
837        close(&mut closed_elsewhere, &talks).expect("close");
838        assert_eq!(
839            talks.get(&in_flight.id).expect("reread").status,
840            TalkStatus::Closed,
841            "the close landed on disk before the turn finished"
842        );
843
844        // The turn's own handle still says `open` - it was loaded before the
845        // close - and finishing it must not resurrect the conversation the
846        // operator already ended.
847        assert_eq!(in_flight.status, TalkStatus::Open);
848        respond(&mut in_flight, &talks, &cfg, "one more question")
849            .await
850            .expect("the turn itself still completes");
851
852        let on_disk = talks.get(&in_flight.id).expect("reread");
853        assert_eq!(
854            on_disk.status,
855            TalkStatus::Closed,
856            "a close must stick even when a turn that started before it finishes after it"
857        );
858        // The reply is not lost either: a turn already in flight when the
859        // operator closed still gets its answer recorded.
860        assert!(
861            on_disk.turns.iter().any(|t| t.body == "here you go"),
862            "the in-flight turn's own reply is still recorded: {:?}",
863            on_disk.turns
864        );
865    }
866
867    #[test]
868    fn a_close_that_lands_before_record_is_called_is_not_undone_by_it() {
869        let (tmp, talks) = store();
870        let spec = mock_agent(tmp.path(), REPLY, env("hi"));
871        let cfg = config(spec);
872        // The handle `web::talk_say` would have read before awaiting config
873        // discovery, then carried across that await into `record`.
874        let mut stale = begin(&talks, &cfg, tmp.path().to_owned(), None).expect("begin");
875
876        // The operator closes the conversation through a *different* handle
877        // in the gap between that read and the call to `record` below.
878        let mut closed_elsewhere = talks.get(&stale.id).expect("reread");
879        close(&mut closed_elsewhere, &talks).expect("close");
880        assert_eq!(
881            talks.get(&stale.id).expect("reread").status,
882            TalkStatus::Closed,
883            "the close landed on disk before record was called"
884        );
885
886        // The stale handle still says `open` - it was loaded before the
887        // close - so a `record` that trusted it would append a turn and
888        // write the conversation back open, undoing the close.
889        assert_eq!(stale.status, TalkStatus::Open);
890        let err = record(&mut stale, &talks, "still there?")
891            .expect_err("a close that landed first must be honored, not overwritten");
892        assert!(err.to_string().contains("closed"));
893
894        let on_disk = talks.get(&stale.id).expect("reread");
895        assert_eq!(
896            on_disk.status,
897            TalkStatus::Closed,
898            "record must not resurrect a conversation closed while its snapshot was stale"
899        );
900        assert!(
901            on_disk.turns.is_empty(),
902            "the rejected turn must not have been appended: {:?}",
903            on_disk.turns
904        );
905        let _ = &cfg; // config kept only to build the agent above
906    }
907
908    #[test]
909    fn close_blocks_on_records_guard_rather_than_interleaving_with_it() {
910        let (tmp, talks) = store();
911        let spec = mock_agent(tmp.path(), REPLY, env("hi"));
912        let cfg = config(spec);
913        let talk = begin(&talks, &cfg, tmp.path().to_owned(), None).expect("begin");
914
915        // Hold the same guard `record`'s read-modify-write section holds for
916        // the whole of its own read-then-write, standing in for `record`
917        // being paused between its read and its `put`.
918        let held = talks.guard();
919
920        let talks2 = talks.clone();
921        let id = talk.id.clone();
922        let closing = std::thread::spawn(move || {
923            let mut talk = talks2.get(&id).expect("get");
924            close(&mut talk, &talks2).expect("close");
925        });
926
927        std::thread::sleep(Duration::from_millis(50));
928        assert!(
929            !closing.is_finished(),
930            "close must wait for the guard, not read and write while it is held - \
931             a re-read alone narrows this window without closing it"
932        );
933
934        drop(held);
935        closing.join().expect("close thread panicked");
936
937        assert_eq!(
938            talks.get(&talk.id).expect("reread").status,
939            TalkStatus::Closed,
940            "once the guard is free, close still lands"
941        );
942        let _ = &cfg; // config kept only to build the agent above
943    }
944
945    #[test]
946    fn list_puts_open_talks_before_closed_ones() {
947        let (tmp, talks) = store();
948        let make = |id: &str, status: TalkStatus| {
949            let mut t = Talk {
950                schema: SCHEMA,
951                id: id.to_owned(),
952                repo: tmp.path().to_owned(),
953                agent: "mock".to_owned(),
954                status,
955                turns: Vec::new(),
956                created_at: Timestamp::now(),
957                updated_at: Timestamp::now(),
958                seat: SeatState::new(SEAT, "mock", 7),
959            };
960            talks.put(&mut t).expect("put");
961        };
962        make("20260901-000000-0001", TalkStatus::Open);
963        make("20260902-000000-0002", TalkStatus::Open);
964        make("20260903-000000-0003", TalkStatus::Closed);
965
966        let ids: Vec<String> = talks.list().into_iter().map(|t| t.id).collect();
967        assert_eq!(
968            ids,
969            [
970                "20260902-000000-0002",
971                "20260901-000000-0001",
972                "20260903-000000-0003"
973            ]
974        );
975        assert_eq!(talks.count_open(), 2);
976    }
977
978    #[test]
979    fn tasks_of_finds_only_this_talks_own_tasks() {
980        let dir = tempfile::tempdir().expect("tempdir");
981        let queue = Queue::at(dir.path().join("queue"));
982
983        let mut mine = Task::new(
984            "rework the loader".to_owned(),
985            "rework the loader".to_owned(),
986            PathBuf::from("/repo"),
987            Source::Agent {
988                run: "20260904-014455-ab12".to_owned(),
989                node: "chat".to_owned(),
990            },
991        );
992        queue.put(&mut mine).expect("put mine");
993
994        let mut theirs = Task::new(
995            "unrelated".to_owned(),
996            "unrelated".to_owned(),
997            PathBuf::from("/repo"),
998            Source::Agent {
999                run: "20260904-090000-zz99".to_owned(),
1000                node: "implement".to_owned(),
1001            },
1002        );
1003        queue.put(&mut theirs).expect("put theirs");
1004
1005        let mut human = Task::new(
1006            "typed by hand".to_owned(),
1007            "typed by hand".to_owned(),
1008            PathBuf::from("/repo"),
1009            Source::Human,
1010        );
1011        queue.put(&mut human).expect("put human");
1012
1013        let found = tasks_of(&queue, "20260904-014455-ab12");
1014        assert_eq!(found.len(), 1);
1015        assert_eq!(found[0].id, mine.id);
1016    }
1017
1018    #[test]
1019    fn the_briefing_names_solo_task_add_and_not_the_task_file_spec() {
1020        let brief = briefing(Path::new("/repo"), "en");
1021        assert!(brief.contains("magi task add --solo"));
1022        assert!(!brief.contains(plan::TASK_FILE_SPEC));
1023        assert!(brief.contains("/repo"));
1024        assert!(!brief.contains("Hold this conversation in"));
1025    }
1026
1027    #[test]
1028    fn the_briefing_names_the_language_when_it_is_not_english() {
1029        let brief = briefing(Path::new("/repo"), "Japanese");
1030        assert!(brief.contains("Hold this conversation in Japanese"));
1031    }
1032}