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