Skip to main content

magi/
agent.rs

1//! Driving agent CLIs.
2//!
3//! Every agent in magi is a subscription CLI (`claude`, `opencode`, `agy`) or
4//! an arbitrary command, invoked headless in a working directory. There is no
5//! API-key path on purpose: the CLIs carry the operator's own plan, and they
6//! are the only interface that exposes an agent's whole tool loop rather than a
7//! single completion.
8//!
9//! # Seats, not agents
10//!
11//! Conversations are keyed by *seat* ([`SeatState::key`]), never by agent id. A
12//! model that implements candidate B and also sits as judge 3 gets two
13//! unrelated conversations, so the judge cannot recognise its own work from
14//! having written it. Sessions are what make deliberation affordable — a judge
15//! remembers its own argument instead of being re-fed the entire candidate set
16//! — and seat scoping is what keeps that from destroying blindness.
17//!
18//! # Session mechanics per CLI
19//!
20//! | CLI | open | resume |
21//! |-----|------|--------|
22//! | `claude` | `--session-id <uuid>` (magi mints it) | `--resume <uuid>` |
23//! | `opencode` | `--format json` reports `sessionID` | `-s <id>` |
24//! | `agy` | `--output-format json` reports `conversation_id` | `--conversation <id>` |
25//! | `codex` | `exec --json` reports `thread.started.thread_id` | `exec … resume <id>` |
26//! | `omp` | `-p --mode=json` reports `id` on its `"type":"session"` line | `--resume <id>` |
27//!
28//! Claude is the only one magi can address before the first turn; the others
29//! report an id back, so [`SeatState::captured_session`] stays `None` until a
30//! turn has completed and [`has_session`] answers honestly instead of
31//! optimistically.
32use std::collections::BTreeMap;
33use std::path::{Path, PathBuf};
34use std::process::Stdio;
35use std::time::{Duration, Instant};
36
37use anyhow::{Context as _, Result, bail};
38use serde::{Deserialize, Serialize};
39use std::sync::{Arc, Mutex};
40
41use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _};
42use tokio::process::Command;
43
44use crate::config::{AgentKind, AgentSpec, Delivery};
45use crate::proc::Quiet as _;
46use crate::rng::SplitMix64;
47
48/// Conversation state for one seat, persisted with the run so `magi run
49/// --resume` continues the same CLI conversations.
50#[derive(Debug, Clone, Serialize, Deserialize)]
51pub struct SeatState {
52    /// Stable seat name, e.g. `impl-A`, `judge-2`, `review-1`, `fix`.
53    pub key: String,
54    /// Agent id occupying the seat.
55    pub agent: String,
56    /// Turns already taken in this seat.
57    pub turns: usize,
58    /// Claude session uuid, minted up front so the first turn and every resume
59    /// agree on it without parsing anything back.
60    pub claude_session: Option<String>,
61    /// Session id reported by a CLI that mints its own (`opencode`, `agy`).
62    pub captured_session: Option<String>,
63}
64
65impl SeatState {
66    /// New seat. `run_seed` scopes the minted Claude uuid to this run.
67    pub fn new(key: &str, agent: &str, run_seed: u64) -> Self {
68        let mut rng = SplitMix64::new(run_seed ^ crate::rng::fnv1a(key));
69        Self {
70            key: key.to_owned(),
71            agent: agent.to_owned(),
72            turns: 0,
73            claude_session: Some(rng.uuid_v4()),
74            captured_session: None,
75        }
76    }
77}
78
79/// Can a follow-up prompt rely on this seat remembering the conversation?
80pub fn has_session(kind: AgentKind, seat: &SeatState, sessions_enabled: bool) -> bool {
81    if !sessions_enabled || seat.turns == 0 {
82        return false;
83    }
84    match kind {
85        AgentKind::Claude => seat.claude_session.is_some(),
86        AgentKind::Opencode | AgentKind::Antigravity | AgentKind::Codex | AgentKind::Omp => {
87            seat.captured_session.is_some()
88        }
89        AgentKind::Command => true,
90    }
91}
92
93/// One agent invocation.
94#[derive(Debug)]
95pub struct Invocation<'a> {
96    /// Working directory. Always a real checkout, so every CLI can read the
97    /// repository without per-vendor "extra directory" flags.
98    pub cwd: &'a Path,
99    /// The full prompt.
100    pub prompt: &'a str,
101    /// Wall-clock limit; the process tree is killed when it elapses.
102    pub timeout: Duration,
103    /// May the agent modify files? Judges and reviewers may not.
104    pub allow_write: bool,
105    /// Continue this seat's conversation when the CLI supports it.
106    pub sessions: bool,
107    /// Directory for prompt / stdout / stderr artifacts.
108    pub artifacts: &'a Path,
109    /// Artifact filename stem.
110    pub stem: &'a str,
111    /// Run this invocation belongs to. Exported as `MAGI_RUN` so an agent that
112    /// files a task with `magi task add` is attributed to the run that was
113    /// paying for it, rather than looking like a human wandered by.
114    pub run: &'a str,
115    /// Graph node being executed, e.g. `implement` or `review`. Exported as
116    /// `MAGI_NODE` for the same reason: "who asked for this" is the first
117    /// question about an autonomously created task.
118    pub node: &'a str,
119    /// Shared build cache the seat should build into, from the rendered
120    /// `CARGO_TARGET_DIR=` in the verify commands. Exported as
121    /// `CARGO_TARGET_DIR` so the implementer's compile lands inside the same
122    /// directory `verify` reads back from - one cache, one prune, and the
123    /// build the agent just paid for is the build the gate reuses.
124    pub cache_dir: Option<&'a Path>,
125    /// Absolute paths of images the operator attached to this conversation,
126    /// outside `cwd` - see `chat`/`talk`'s `attachments_dir`. Empty for every
127    /// invocation that is not a chat or talk turn. [`build_command`] uses
128    /// this only to decide whether a CLI's sandbox needs widening to read
129    /// them; the prompt text naming each path and its mime is built by the
130    /// caller, not here.
131    pub attachments: &'a [PathBuf],
132}
133
134/// Evidence that a CLI ran out of its rate limit / quota, distinct from an
135/// ordinary failure.
136///
137/// `reset` is free text: CLIs render the reset time in their own locale, and
138/// parsing it exactly would be a bug factory. When it is not readable we say
139/// nothing rather than invent a format.
140#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
141pub struct Quota {
142    /// Human-readable reset time, when the CLI printed one.
143    #[serde(default)]
144    pub reset: Option<String>,
145}
146
147/// A CLI hung up on its own stream while the agent was working.
148///
149/// Separate from a failure because the work was done and billed, and separate
150/// from a [`Quota`] because it is worth asking again: the answer is in the
151/// conversation, not lost to a limit that has to reset first. See
152/// [`dropped_stream`].
153#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
154pub struct Dropped {
155    /// What the CLI said as it hung up, verbatim.
156    pub why: String,
157    /// Output tokens the CLI reported before it did - the evidence that this
158    /// was a delivery failure and not an agent that produced nothing.
159    pub output_tokens: u64,
160}
161
162/// One command a CLI's own structured event stream reported running, with
163/// the result it reported for it.
164///
165/// This is evidence the CLI chose to report about its own tool loop — never
166/// something magi polled, supervised, or inferred from a process list. Only
167/// the Codex arm of [`extract`] currently populates it (its `item.completed`
168/// / `command_execution` events name `id`, `command`, `exit_code` and
169/// `aggregated_output` directly); every other backend's CLI does not expose
170/// this in what magi currently captures, so its seats simply never produce
171/// any. A command a CLI never reported finishing (still running when the
172/// turn ended, or the event stream never named it) has no entry here either
173/// — there is no event to build one from, and this type must never be used
174/// to *guess* that a command is still in flight.
175#[derive(Debug, Clone, Serialize, Deserialize)]
176pub struct CommandEvidence {
177    /// The CLI's own id for this command.
178    pub id: String,
179    /// The command itself, as the CLI reported it.
180    pub description: String,
181    /// Exit code the CLI reported for it.
182    pub exit_code: Option<i32>,
183    /// Tail of the command's own output, when the CLI reported one.
184    pub result_summary: String,
185    /// Which CLI/event stream this came from, e.g. `"codex"`.
186    pub source: String,
187}
188
189/// Result of an agent invocation.
190#[derive(Debug, Clone, Serialize, Deserialize)]
191pub struct AgentOutput {
192    /// The agent's final message, extracted from whatever the CLI printed.
193    pub text: String,
194    /// Exit status code.
195    pub exit_code: Option<i32>,
196    /// Did the invocation hit its timeout?
197    pub timed_out: bool,
198    /// Wall-clock duration.
199    pub duration_ms: u64,
200    /// Artifact file names, relative to the run's `artifacts/` directory.
201    pub artifacts: Vec<String>,
202    /// Rate-limit / quota exhaustion, when it can be told apart from a normal
203    /// failure. `None` for a normal failure, a timeout, or a CLI we cannot
204    /// read — the conservative default.
205    #[serde(default)]
206    pub quota: Option<Quota>,
207    /// The CLI hung up on its own stream after the agent had done billed
208    /// work. `None` unless that exact shape was recognised — see
209    /// [`dropped_stream`].
210    #[serde(default)]
211    pub dropped: Option<Dropped>,
212    /// Commands the CLI's own event stream reported running, see
213    /// [`CommandEvidence`]. Always empty for a backend this crate does not
214    /// currently read structured job events from.
215    #[serde(default)]
216    pub commands: Vec<CommandEvidence>,
217}
218
219impl AgentOutput {
220    /// Did the CLI exit cleanly with something to say?
221    pub fn usable(&self) -> bool {
222        !self.timed_out && self.exit_code == Some(0) && !self.text.trim().is_empty()
223    }
224
225    /// Did this invocation run out of the CLI's rate limit / quota?
226    pub fn quota_exhausted(&self) -> bool {
227        self.quota.is_some()
228    }
229
230    /// Did the agent work and the CLI fail to deliver it?
231    ///
232    /// Worth re-asking, unlike [`AgentOutput::quota_exhausted`]: the answer is
233    /// in a conversation this process can resume.
234    pub fn work_undelivered(&self) -> bool {
235        self.dropped.is_some()
236    }
237}
238
239/// How long to keep reading a pipe after the child is gone.
240///
241/// Bounded on purpose: a surviving grandchild can hold the write end open
242/// forever, and the graph must not hang on a process it has already killed.
243const PIPE_GRACE: Duration = Duration::from_secs(3);
244
245/// Bytes a pipe reader has accumulated so far, shared with whoever spawned it.
246type Captured = Arc<Mutex<Vec<u8>>>;
247
248/// Read `pipe` to end in its own task, appending into a buffer the caller can
249/// inspect at any time.
250///
251/// The buffer is shared rather than returned because the interesting moment is
252/// exactly the one where the reader has *not* finished: a killed agent's pipe
253/// may still be held open by a surviving grandchild, and the bytes that did
254/// arrive are the only evidence of what it was doing. An earlier version
255/// returned the buffer from the task and dropped it on timeout, which is how
256/// `<stem>.out` came to be empty on every timeout.
257fn drain<R>(pipe: Option<R>) -> (Captured, Option<tokio::task::JoinHandle<()>>)
258where
259    R: tokio::io::AsyncRead + Unpin + Send + 'static,
260{
261    let buf: Captured = Arc::new(Mutex::new(Vec::new()));
262    let Some(mut pipe) = pipe else {
263        return (buf, None);
264    };
265    let sink = Arc::clone(&buf);
266    let handle = tokio::spawn(async move {
267        let mut chunk = [0u8; 8192];
268        loop {
269            match pipe.read(&mut chunk).await {
270                Ok(0) | Err(_) => break,
271                Ok(n) => {
272                    if let Ok(mut guard) = sink.lock() {
273                        guard.extend_from_slice(&chunk[..n]);
274                    }
275                }
276            }
277        }
278    });
279    (buf, Some(handle))
280}
281
282/// Take whatever a reader has captured, giving it at most `grace` to finish.
283///
284/// A reader still blocked after that is abandoned, not awaited — but its bytes
285/// come back either way, which is the whole point.
286async fn collect(
287    buf: &Captured,
288    handle: Option<tokio::task::JoinHandle<()>>,
289    grace: Duration,
290) -> String {
291    if let Some(handle) = handle {
292        if tokio::time::timeout(grace, handle).await.is_err() {
293            tracing::debug!("a pipe is still held open after the child exited");
294        }
295    }
296    let bytes = buf.lock().map(|g| g.clone()).unwrap_or_default();
297    String::from_utf8_lossy(&bytes).into_owned()
298}
299
300/// Invoke `spec` for `seat`, updating the seat's conversation state.
301pub async fn invoke(
302    spec: &AgentSpec,
303    seat: &mut SeatState,
304    inv: &Invocation<'_>,
305) -> Result<AgentOutput> {
306    tokio::fs::create_dir_all(inv.artifacts)
307        .await
308        .with_context(|| format!("create {}", inv.artifacts.display()))?;
309    let prompt_path = inv.artifacts.join(format!("{}.prompt.md", inv.stem));
310    tokio::fs::write(&prompt_path, inv.prompt)
311        .await
312        .with_context(|| format!("write {}", prompt_path.display()))?;
313
314    let plan = build_command(spec, seat, inv, &prompt_path)?;
315    tracing::debug!(seat = %seat.key, agent = %spec.id, argv = ?plan.argv, "spawning agent");
316
317    let started = Instant::now();
318    let mut cmd = Command::new(&plan.argv[0]);
319    cmd.args(&plan.argv[1..])
320        .current_dir(inv.cwd)
321        .envs(&spec.env)
322        .env("MAGI_SEAT", &seat.key)
323        .env("MAGI_TURN", seat.turns.to_string())
324        .env("MAGI_RUN", inv.run)
325        .env("MAGI_NODE", inv.node)
326        .env("MAGI_PROMPT_FILE", &prompt_path)
327        .env("MAGI_ALLOW_WRITE", if inv.allow_write { "1" } else { "0" })
328        .env("GIT_TERMINAL_PROMPT", "0")
329        .stdin(if plan.stdin.is_some() {
330            Stdio::piped()
331        } else {
332            Stdio::null()
333        })
334        .stdout(Stdio::piped())
335        .stderr(Stdio::piped())
336        .kill_on_drop(true)
337        // No console window. `magi web` has no console of its own, so Windows
338        // would give each agent a fresh one - and draw it. See `crate::proc`.
339        .quiet();
340    if let Some(cache) = inv.cache_dir {
341        // Same directory the verify commands build into: one cache to prune,
342        // and the compile the seat pays for is the compile the gate reuses.
343        cmd.env("CARGO_TARGET_DIR", cache);
344    } else {
345        // `Command` inherits this process's environment by default, so
346        // simply not setting the variable here is not the same as the seat
347        // not seeing it: if the magi process itself is running under a
348        // shared `CARGO_TARGET_DIR` (the ordinary case), a read-only seat
349        // would otherwise inherit that exact path and try to build there
350        // anyway - the write refusal this is meant to prevent in the first
351        // place. Strip it explicitly.
352        cmd.env_remove("CARGO_TARGET_DIR");
353    }
354
355    let mut child = cmd
356        .spawn()
357        .with_context(|| format!("spawn `{}` for seat {}", plan.argv[0], seat.key))?;
358    // Feed stdin from a task rather than inline: a `command` agent that never
359    // reads its stdin, or a prompt larger than the pipe buffer, would
360    // otherwise deadlock here before the process is ever waited on.
361    if let (Some(body), Some(mut sink)) = (plan.stdin.clone(), child.stdin.take()) {
362        tokio::spawn(async move {
363            sink.write_all(body.as_bytes()).await.ok();
364            sink.shutdown().await.ok();
365        });
366    }
367
368    // Drain the pipes in their own tasks, and wait on the *process*, not on
369    // end-of-file. Two failures come out of conflating those:
370    //
371    // 1. `wait_with_output` returns when both pipes reach EOF, which is not
372    //    when the child exits. A CLI that leaves a helper process holding the
373    //    inherited stdout handle - normal on Windows, where a `.cmd` shim and
374    //    its grandchildren share handles - never closes the pipe, so a seat
375    //    that answered in five minutes was billed the full hour and then
376    //    recorded as a timeout. The answer was thrown away with it.
377    // 2. Cancelling `wait_with_output` at the timeout drops the buffers it
378    //    owned, so `<stem>.out` and `<stem>.err` were written empty exactly
379    //    when an operator needs them most. "It printed nothing" and "we
380    //    discarded what it printed" looked identical on disk.
381    //
382    // Now the readers own the bytes, so a timeout keeps whatever arrived, and
383    // the wait ends at exit even if a stray handle stays open.
384    let (out_buf, out_reader) = drain(child.stdout.take());
385    let (err_buf, err_reader) = drain(child.stderr.take());
386
387    let (code, timed_out) = match tokio::time::timeout(inv.timeout, child.wait()).await {
388        Ok(res) => {
389            let status = res.with_context(|| format!("wait for seat {}", seat.key))?;
390            (status.code(), false)
391        }
392        Err(_) => {
393            tracing::warn!(seat = %seat.key, secs = inv.timeout.as_secs(), "agent timed out");
394            // Kill the tree so the readers see EOF instead of hanging with it.
395            child.start_kill().ok();
396            (None, true)
397        }
398    };
399
400    // The child is gone either way, so the readers are bounded now. A grace
401    // window rather than an unbounded await: a surviving grandchild can still
402    // hold the write end open, and losing a few trailing bytes beats hanging
403    // the graph on a process we no longer control.
404    let stdout = collect(&out_buf, out_reader, PIPE_GRACE).await;
405    let stderr = collect(&err_buf, err_reader, PIPE_GRACE).await;
406
407    let out_path = inv.artifacts.join(format!("{}.out", inv.stem));
408    let err_path = inv.artifacts.join(format!("{}.err", inv.stem));
409    tokio::fs::write(&out_path, &stdout).await.ok();
410    tokio::fs::write(&err_path, &stderr).await.ok();
411
412    let extracted = extract(spec.kind, &stdout);
413    if let Some(session) = extracted.session {
414        match spec.kind {
415            AgentKind::Claude => seat.claude_session = Some(session),
416            AgentKind::Opencode | AgentKind::Antigravity | AgentKind::Codex | AgentKind::Omp => {
417                seat.captured_session = Some(session);
418            }
419            AgentKind::Command => {}
420        }
421    }
422    if let Some(status) = &extracted.status
423        && !status.eq_ignore_ascii_case("success")
424    {
425        tracing::warn!(seat = %seat.key, status = %status, "agent reported a non-success status");
426    }
427    let text = if extracted.text.trim().is_empty() {
428        // A CLI that printed only to stderr still told us something.
429        if stdout.trim().is_empty() {
430            stderr.trim().to_owned()
431        } else {
432            stdout.trim().to_owned()
433        }
434    } else {
435        extracted.text
436    };
437    seat.turns += 1;
438
439    Ok(AgentOutput {
440        text,
441        exit_code: code,
442        timed_out,
443        duration_ms: started.elapsed().as_millis() as u64,
444        artifacts: vec![
445            file_name(&prompt_path),
446            file_name(&out_path),
447            file_name(&err_path),
448        ],
449        quota: extracted.quota,
450        dropped: extracted.dropped,
451        commands: extracted.commands,
452    })
453}
454
455fn file_name(p: &Path) -> String {
456    p.file_name()
457        .unwrap_or_default()
458        .to_string_lossy()
459        .into_owned()
460}
461
462/// The argv plus optional stdin body for one invocation.
463#[derive(Debug)]
464struct Plan {
465    argv: Vec<String>,
466    stdin: Option<String>,
467}
468
469/// How a file-delivered prompt is pointed at, per CLI.
470///
471/// `agy` has a native file-context syntax, `@<path>`, and it is measurably the
472/// better contract: on the same trivial task it finished in 17s against 73s for
473/// the prose form, because prose makes the model spend a tool round-trip
474/// deciding to read the file. It is also the form yukimemi/rvpm proved out.
475///
476/// opencode has no equivalent, so it gets the prose. That is not a fallback
477/// worth apologising for — it works, and it is what the winning opencode
478/// candidates on this repository have been driven by all along.
479fn pointer(kind: AgentKind, prompt_path: &Path) -> String {
480    if matches!(kind, AgentKind::Antigravity) {
481        return format!("@{}", prompt_path.display());
482    }
483    format!(
484        "Read the file at {} and follow every instruction in it exactly. That \
485         file is your complete task description; this message contains nothing \
486         else.",
487        prompt_path.display()
488    )
489}
490
491fn build_command(
492    spec: &AgentSpec,
493    seat: &SeatState,
494    inv: &Invocation<'_>,
495    prompt_path: &Path,
496) -> Result<Plan> {
497    let mut argv: Vec<String> = Vec::new();
498    let mut stdin: Option<String> = None;
499    let delivery = spec.delivery();
500    let resuming = has_session(spec.kind, seat, inv.sessions);
501
502    match spec.kind {
503        AgentKind::Claude => {
504            // Claude's own tools have no cwd-confined sandbox - the CLI can
505            // already `Read` any absolute path magi hands it, an attachment
506            // outside the repository included - so no extra flag is needed
507            // here.
508            argv.push("claude".to_owned());
509            argv.push("-p".to_owned());
510            argv.push("--output-format".to_owned());
511            argv.push("json".to_owned());
512            if let Some(m) = &spec.model {
513                argv.push("--model".to_owned());
514                argv.push(m.clone());
515            }
516            if inv.sessions {
517                let uuid = seat
518                    .claude_session
519                    .as_deref()
520                    .context("claude seat is missing its session uuid")?;
521                argv.push(if resuming { "--resume" } else { "--session-id" }.to_owned());
522                argv.push(uuid.to_owned());
523            }
524            argv.push("--permission-mode".to_owned());
525            argv.push("bypassPermissions".to_owned());
526            if !inv.allow_write {
527                argv.push("--disallowed-tools".to_owned());
528                argv.push("Edit,Write,MultiEdit,NotebookEdit".to_owned());
529            }
530        }
531        AgentKind::Opencode => {
532            // `--auto` below already bypasses every permission, reads of a
533            // path outside `--dir` included, so an attachment elsewhere
534            // needs no extra flag.
535            argv.push("opencode".to_owned());
536            argv.push("run".to_owned());
537            argv.push("--format".to_owned());
538            argv.push("json".to_owned());
539            argv.push("--dir".to_owned());
540            argv.push(inv.cwd.to_string_lossy().into_owned());
541            // `--auto` gates *every* permission, reads included: without it a
542            // non-interactive opencode cannot even open the prompt file, and
543            // the seat drops out of the panel with "the user rejected
544            // permission to use this specific tool call". opencode has no
545            // read-only mode, so read-only seats rely on the prompt plus the
546            // fact that judge and reviewer worktrees are disposable — judges'
547            // are deleted after the tally, reviewers' are reset to the commit
548            // under review every round.
549            argv.push("--auto".to_owned());
550            if let Some(m) = &spec.model {
551                argv.push("-m".to_owned());
552                argv.push(m.clone());
553            }
554            if resuming {
555                argv.push("-s".to_owned());
556                argv.push(
557                    seat.captured_session
558                        .clone()
559                        .expect("has_session checked the id is present"),
560                );
561            }
562        }
563        AgentKind::Antigravity => {
564            argv.push("agy".to_owned());
565            argv.push("--output-format".to_owned());
566            argv.push("json".to_owned());
567            // agy's print mode gives up after 5 minutes by default, which is
568            // far below an implementation node's budget.
569            argv.push("--print-timeout".to_owned());
570            argv.push(format!("{}s", inv.timeout.as_secs()));
571            argv.push("--mode".to_owned());
572            argv.push(
573                if inv.allow_write {
574                    "accept-edits"
575                } else {
576                    "plan"
577                }
578                .to_owned(),
579            );
580            if inv.allow_write {
581                argv.push("--dangerously-skip-permissions".to_owned());
582            }
583            if let Some(m) = &spec.model {
584                argv.push("--model".to_owned());
585                argv.push(m.clone());
586            }
587            if resuming {
588                argv.push("--conversation".to_owned());
589                argv.push(
590                    seat.captured_session
591                        .clone()
592                        .expect("has_session checked the id is present"),
593                );
594            }
595            // The prompt file lives outside the worktree, so the workspace has
596            // to be widened to reach it - and so does an attachment's own
597            // directory, which usually lives right beside it under the
598            // conversation's `artifacts_dir` (see `chat`/`talk`). "Usually":
599            // a chat derived from another one (`chat::derived_background`)
600            // can carry attachment paths that live under the *source*
601            // conversation's own artifacts dir instead, so each attachment
602            // outside `inv.artifacts` gets its own `--add-dir` rather than
603            // assuming one directory covers all of `inv.attachments`.
604            let mut add_dirs: Vec<String> = Vec::new();
605            if delivery == Delivery::File || !inv.attachments.is_empty() {
606                add_dirs.push(inv.artifacts.to_string_lossy().into_owned());
607            }
608            for path in inv.attachments {
609                let Some(parent) = path.parent() else {
610                    continue;
611                };
612                if parent.starts_with(inv.artifacts) {
613                    continue;
614                }
615                let dir = parent.to_string_lossy().into_owned();
616                if !add_dirs.contains(&dir) {
617                    add_dirs.push(dir);
618                }
619            }
620            for dir in add_dirs {
621                argv.push("--add-dir".to_owned());
622                argv.push(dir);
623            }
624        }
625        AgentKind::Codex => {
626            // `--sandbox` below governs writes, not reads (see the module
627            // doc: it is what makes codex the one kind whose *read-only*
628            // mode is enforced, by refusing edits) - both presets can read
629            // anywhere the OS lets the process, so an attachment outside
630            // `cwd` is already reachable without an extra flag.
631            argv.push("codex".to_owned());
632            argv.push("exec".to_owned());
633            argv.push("--json".to_owned());
634            // The worktrees magi hands out are real checkouts, but a judge's
635            // is detached and a fixture's may be no repository at all.
636            argv.push("--skip-git-repo-check".to_owned());
637            argv.push("-C".to_owned());
638            argv.push(inv.cwd.to_string_lossy().into_owned());
639            // Codex is the only kind whose read-only-ness is enforced by the
640            // CLI rather than by the prompt: a judge or reviewer seat cannot
641            // write even if it decides to try. Implementers get the workspace,
642            // and nothing ever gets `--dangerously-bypass-approvals-and-sandbox`.
643            argv.push("--sandbox".to_owned());
644            argv.push(
645                if inv.allow_write {
646                    "workspace-write"
647                } else {
648                    "read-only"
649                }
650                .to_owned(),
651            );
652            // Nothing is watching to approve anything: an unattended seat that
653            // asks blocks until its timeout kills it.
654            argv.push("-c".to_owned());
655            argv.push("approval_policy=\"never\"".to_owned());
656            if let Some(m) = &spec.model {
657                argv.push("-m".to_owned());
658                argv.push(m.clone());
659            }
660            // `resume` is a subcommand of `exec`, and it rejects the flags
661            // above when they follow it - so every option is emitted first and
662            // the subcommand last. Established by hand against codex-cli
663            // 0.153.4: with the order reversed the CLI exits on
664            // `unexpected argument '--sandbox'`.
665            if resuming {
666                argv.push("resume".to_owned());
667                argv.push(
668                    seat.captured_session
669                        .clone()
670                        .expect("has_session checked the id is present"),
671                );
672            }
673        }
674        AgentKind::Omp => {
675            // `omp` reads the prompt from stdin in print mode (see
676            // `AgentSpec::delivery`), so the whole instruction arrives without
677            // an argv length limit - the same reason codex gets stdin.
678            argv.push("omp".to_owned());
679            argv.push("-p".to_owned());
680            argv.push("--mode=json".to_owned());
681            // `--auto-approve` is required, and is the same trade opencode's
682            // `--auto` makes: it gates *every* permission, reads included, so
683            // without it a non-interactive seat cannot even open the prompt
684            // file magi wrote and drops out of the panel on a permission
685            // rejection. `omp` has no read-only mode of its own, so a judge or
686            // reviewer seat rests on the prompt plus the worktree discipline
687            // (judge worktrees are deleted after the tally, reviewer worktrees
688            // are reset to the commit under review every round) - never on this
689            // flag, and never on a bypass flag.
690            argv.push("--auto-approve".to_owned());
691            if let Some(m) = &spec.model {
692                argv.push("--model".to_owned());
693                argv.push(m.clone());
694            }
695            // Established by hand against omp 18.1.19: `-p --mode=json` reports
696            // the session id on its `"type":"session"` line, and
697            // `--resume <id>` continues that conversation. `--continue` is
698            // deliberately not used - it opens a *new* session rather than the
699            // stored one, which silently loses the seat's memory.
700            if resuming {
701                argv.push("--resume".to_owned());
702                argv.push(
703                    seat.captured_session
704                        .clone()
705                        .expect("has_session checked the id is present"),
706                );
707            }
708        }
709        AgentKind::Command => {
710            // The operator's own command line, not one of the roster CLIs -
711            // there is no flag this function could add on its behalf, so an
712            // attachment's path has to reach it the same way the prompt
713            // does, through the substitutions below.
714            if spec.command.is_empty() {
715                bail!("agent `{}` has kind = \"command\" but no command", spec.id);
716            }
717            let vars: BTreeMap<&str, String> = BTreeMap::from([
718                ("{prompt_file}", prompt_path.to_string_lossy().into_owned()),
719                ("{cwd}", inv.cwd.to_string_lossy().into_owned()),
720                ("{label}", seat.key.clone()),
721                ("{session}", seat.claude_session.clone().unwrap_or_default()),
722            ]);
723            for raw in &spec.command {
724                let mut arg = raw.clone();
725                for (k, v) in &vars {
726                    if arg.contains(k) {
727                        arg = arg.replace(k, v);
728                    }
729                }
730                argv.push(arg);
731            }
732        }
733    }
734
735    argv.extend(spec.extra_args.iter().cloned());
736
737    // `agy` takes the prompt as the value of `-p`, so the flag has to be
738    // emitted right before whatever the delivery mode produces.
739    if spec.kind == AgentKind::Antigravity {
740        argv.push("-p".to_owned());
741    }
742    // `codex exec` reads stdin only when its prompt argument is `-`; without
743    // it the CLI waits on a prompt it will never be given.
744    if spec.kind == AgentKind::Codex && delivery == Delivery::Stdin {
745        argv.push("-".to_owned());
746    }
747    match delivery {
748        Delivery::Stdin if spec.kind == AgentKind::Antigravity => {
749            // agy has no text stdin path; fall back to the pointer file.
750            argv.push(pointer(spec.kind, prompt_path));
751        }
752        Delivery::Stdin => stdin = Some(inv.prompt.to_owned()),
753        Delivery::Argv => argv.push(inv.prompt.to_owned()),
754        Delivery::File => argv.push(pointer(spec.kind, prompt_path)),
755    }
756
757    Ok(Plan { argv, stdin })
758}
759
760/// What a CLI's stdout yielded.
761#[derive(Debug, Default)]
762struct Extracted {
763    text: String,
764    session: Option<String>,
765    status: Option<String>,
766    quota: Option<Quota>,
767    dropped: Option<Dropped>,
768    commands: Vec<CommandEvidence>,
769}
770
771/// Pull the agent's message (and any session id) out of a CLI's stdout.
772fn extract(kind: AgentKind, stdout: &str) -> Extracted {
773    match kind {
774        AgentKind::Claude => {
775            let Ok(v) = serde_json::from_str::<serde_json::Value>(stdout.trim()) else {
776                return Extracted {
777                    text: stdout.trim().to_owned(),
778                    ..Extracted::default()
779                };
780            };
781            Extracted {
782                text: v
783                    .get("result")
784                    .and_then(|r| r.as_str())
785                    .unwrap_or_default()
786                    .to_owned(),
787                session: v
788                    .get("session_id")
789                    .and_then(|s| s.as_str())
790                    .map(str::to_owned),
791                status: v.get("is_error").and_then(|e| e.as_bool()).map(|e| {
792                    if e {
793                        "error".to_owned()
794                    } else {
795                        "success".to_owned()
796                    }
797                }),
798                quota: claude_quota(&v),
799                // Claude reports a truncated stream as an ordinary error; the
800                // shape `dropped_stream` keys on is agy's.
801                dropped: None,
802                commands: Vec::new(),
803            }
804        }
805        AgentKind::Opencode => {
806            // A JSONL event stream: text parts concatenated in arrival order.
807            let mut text = String::new();
808            let mut session = None;
809            for line in stdout.lines() {
810                let Ok(v) = serde_json::from_str::<serde_json::Value>(line.trim()) else {
811                    continue;
812                };
813                if session.is_none() {
814                    session = v
815                        .get("sessionID")
816                        .and_then(|s| s.as_str())
817                        .map(str::to_owned);
818                }
819                let part = v.get("part").unwrap_or(&serde_json::Value::Null);
820                if part.get("type").and_then(|t| t.as_str()) == Some("text")
821                    && let Some(t) = part.get("text").and_then(|t| t.as_str())
822                {
823                    if !text.is_empty() {
824                        text.push('\n');
825                    }
826                    text.push_str(t);
827                }
828            }
829            Extracted {
830                text,
831                session,
832                status: None,
833                quota: None,
834                dropped: None,
835                commands: Vec::new(),
836            }
837        }
838        AgentKind::Antigravity => {
839            // agy prints warnings before the JSON object, so parse the last
840            // line that is one rather than the whole stream.
841            let obj = stdout
842                .lines()
843                .rev()
844                .find_map(|l| serde_json::from_str::<serde_json::Value>(l.trim()).ok());
845            let Some(v) = obj else {
846                return Extracted {
847                    text: stdout.trim().to_owned(),
848                    ..Extracted::default()
849                };
850            };
851            Extracted {
852                text: v
853                    .get("response")
854                    .and_then(|r| r.as_str())
855                    .unwrap_or_default()
856                    .trim()
857                    .to_owned(),
858                session: v
859                    .get("conversation_id")
860                    .and_then(|s| s.as_str())
861                    .map(str::to_owned),
862                status: v.get("status").and_then(|s| s.as_str()).map(str::to_owned),
863                quota: None,
864                dropped: dropped_stream(&v),
865                commands: Vec::new(),
866            }
867        }
868        AgentKind::Codex => {
869            // A JSONL event stream, prefixed on a real machine by tracing
870            // lines the CLI writes about its own config and skills - so
871            // non-JSON lines are skipped rather than treated as the answer.
872            //
873            // The thread id arrives once, in `thread.started`, and a resumed
874            // turn reports the same one. The answer is the last
875            // `item.completed` carrying an `agent_message`: earlier ones are
876            // the model narrating its way through the tool loop, and taking
877            // the first would hand the caller a progress note instead of a
878            // verdict.
879            //
880            // A `command_execution` item is different evidence entirely: the
881            // CLI's own record that it ran a command and what that command
882            // reported back, kept as `CommandEvidence` — see run
883            // 20260912-214939-b3bb's artifacts, where these very fields
884            // (`id`/`command`/`exit_code`/`aggregated_output`) were what
885            // caught a paired test result magi's own agent prose had missed.
886            // Only what `item.completed` actually reports: a command still
887            // running when the turn ended emits no such event at all, and is
888            // not something this can detect — see [`CommandEvidence`]'s own
889            // doc for why that must not be guessed at instead.
890            let mut text = String::new();
891            let mut session = None;
892            let mut status = None;
893            let mut commands = Vec::new();
894            for line in stdout.lines() {
895                let Ok(v) = serde_json::from_str::<serde_json::Value>(line.trim()) else {
896                    continue;
897                };
898                match v.get("type").and_then(|t| t.as_str()) {
899                    Some("thread.started") => {
900                        session = v
901                            .get("thread_id")
902                            .and_then(|s| s.as_str())
903                            .map(str::to_owned);
904                    }
905                    Some("item.completed") => {
906                        let item = v.get("item").unwrap_or(&serde_json::Value::Null);
907                        match item.get("type").and_then(|t| t.as_str()) {
908                            Some("agent_message") => {
909                                if let Some(t) = item.get("text").and_then(|t| t.as_str()) {
910                                    text = t.trim().to_owned();
911                                }
912                            }
913                            Some("command_execution") => {
914                                commands.push(command_evidence(item));
915                            }
916                            _ => {}
917                        }
918                    }
919                    Some("turn.completed") => status = Some("success".to_owned()),
920                    Some("turn.failed") => status = Some("error".to_owned()),
921                    _ => {}
922                }
923            }
924            Extracted {
925                text,
926                session,
927                status,
928                quota: None,
929                dropped: None,
930                commands,
931            }
932        }
933        AgentKind::Omp => {
934            // A JSONL event stream. The session id arrives once, on the
935            // `"type":"session"` line that opens the run.
936            //
937            // The answer is the *last* non-empty assistant text block anywhere
938            // in the stream, and neither of the two obvious shortcuts works:
939            //
940            // 1. Do not key on `agent_end`. `omp` emits it only for a run that
941            //    quiesces on a message turn; a turn that ends on a tool call
942            //    (`stopReason: "toolUse"`) ends the run with **no `agent_end`
943            //    line at all**, and the answer is in `message_end` / `turn_end`
944            //    instead. Reading only `agent_end` silently discards a complete
945            //    review - which is exactly what the first hand-written wrapper
946            //    did, three times, before this arm existed.
947            // 2. Do not take the first assistant text. Earlier ones narrate the
948            //    tool loop (sometimes with a single `.`), so the last non-empty
949            //    block is the answer and the one before it is a progress note.
950            //
951            // Every line is parsed independently: a non-JSON line (a CLI
952            // warning, a truncated write) is skipped rather than treated as the
953            // answer, the same way the codex arm treats its tracing prefix.
954            let mut text = String::new();
955            let mut session = None;
956            for line in stdout.lines() {
957                let Ok(v) = serde_json::from_str::<serde_json::Value>(line.trim()) else {
958                    continue;
959                };
960                if v.get("type").and_then(|t| t.as_str()) == Some("session") {
961                    session = v.get("id").and_then(|s| s.as_str()).map(str::to_owned);
962                    continue;
963                }
964                // `agent_end` carries the whole thread; `turn_end` and
965                // `message_end` each carry one message. Whichever appears, the
966                // messages are walked the same way.
967                let messages: Vec<&serde_json::Value> = match v.get("type").and_then(|t| t.as_str())
968                {
969                    Some("agent_end") => v
970                        .get("messages")
971                        .and_then(|m| m.as_array())
972                        .map(|m| m.iter().collect())
973                        .unwrap_or_default(),
974                    Some("turn_end") | Some("message_end") => {
975                        v.get("message").into_iter().collect()
976                    }
977                    _ => continue,
978                };
979                for message in messages {
980                    if message.get("role").and_then(|r| r.as_str()) != Some("assistant") {
981                        continue;
982                    }
983                    let Some(parts) = message.get("content").and_then(|c| c.as_array()) else {
984                        continue;
985                    };
986                    for part in parts {
987                        if part.get("type").and_then(|t| t.as_str()) != Some("text") {
988                            continue;
989                        }
990                        if let Some(t) = part.get("text").and_then(|t| t.as_str())
991                            && !t.trim().is_empty()
992                        {
993                            text = t.trim().to_owned();
994                        }
995                    }
996                }
997            }
998            Extracted {
999                text,
1000                session,
1001                status: None,
1002                quota: None,
1003                dropped: None,
1004                commands: Vec::new(),
1005            }
1006        }
1007        AgentKind::Command => {
1008            // A `command` agent may wrap a subscription CLI (a fixture, or a
1009            // thin shim around `claude`). If its output is the claude error
1010            // shape we recognise the quota the same way, so tests and wrappers
1011            // do not need their own detection; anything else is just text.
1012            let parsed = serde_json::from_str::<serde_json::Value>(stdout.trim()).ok();
1013            let quota = parsed.as_ref().and_then(claude_quota);
1014            // A `command` fixture may also stand in for a CLI that hangs up on
1015            // its own stream, which is how that path is tested.
1016            let dropped = parsed.as_ref().and_then(dropped_stream);
1017            Extracted {
1018                text: stdout.trim().to_owned(),
1019                session: None,
1020                status: None,
1021                quota,
1022                dropped,
1023                commands: Vec::new(),
1024            }
1025        }
1026    }
1027}
1028
1029/// Build one [`CommandEvidence`] from a Codex `command_execution` item.
1030///
1031/// `command` arrives either as a single string or as an argv array,
1032/// depending on how the CLI shaped the call; both are read rather than
1033/// assuming one. Missing fields are left at their honest defaults (an empty
1034/// id/description, `exit_code: None`) rather than guessed at.
1035fn command_evidence(item: &serde_json::Value) -> CommandEvidence {
1036    let description = match item.get("command") {
1037        Some(serde_json::Value::String(s)) => s.clone(),
1038        Some(serde_json::Value::Array(parts)) => parts
1039            .iter()
1040            .filter_map(|p| p.as_str())
1041            .collect::<Vec<_>>()
1042            .join(" "),
1043        _ => String::new(),
1044    };
1045    let result_summary = item
1046        .get("aggregated_output")
1047        .and_then(|o| o.as_str())
1048        .map(|s| tail_chars(s.trim(), 400))
1049        .unwrap_or_default();
1050    CommandEvidence {
1051        id: item
1052            .get("id")
1053            .and_then(|s| s.as_str())
1054            .unwrap_or_default()
1055            .to_owned(),
1056        description,
1057        exit_code: item
1058            .get("exit_code")
1059            .and_then(serde_json::Value::as_i64)
1060            .map(|e| e as i32),
1061        result_summary,
1062        source: "codex".to_owned(),
1063    }
1064}
1065
1066/// The last `max` characters of `s`, cut on a char boundary.
1067fn tail_chars(s: &str, max: usize) -> String {
1068    let count = s.chars().count();
1069    if count <= max {
1070        return s.to_owned();
1071    }
1072    s.chars().skip(count - max).collect()
1073}
1074
1075/// Recognise claude's rate-limit error shape, when it is present.
1076///
1077/// The only output we have observed is the JSON object carrying `is_error:
1078/// true` and a `result` mentioning the session limit. We key on exactly that;
1079/// every other CLI (and any future shape) returns `None` and is treated as an
1080/// ordinary failure — the conservative side.
1081fn claude_quota(v: &serde_json::Value) -> Option<Quota> {
1082    let is_err = v.get("is_error").and_then(|e| e.as_bool()).unwrap_or(false);
1083    if !is_err {
1084        return None;
1085    }
1086    let result = v.get("result").and_then(|r| r.as_str()).unwrap_or("");
1087    if !result.to_lowercase().contains("session limit") {
1088        return None;
1089    }
1090    // "…session limit · resets 4:50am (Asia/Tokyo)". The timezone read is not
1091    // worth parsing exactly; keep the whole phrase after "resets" as free text.
1092    let reset = result
1093        .split("resets ")
1094        .nth(1)
1095        .map(str::trim)
1096        .filter(|s| !s.is_empty())
1097        .map(str::to_owned);
1098    Some(Quota { reset })
1099}
1100
1101/// Recognise a CLI that gave up on its own stream while the agent was working.
1102///
1103/// Observed once, verbatim, from `agy` on a candidate that produced nothing:
1104///
1105/// ```text
1106/// {"conversation_id":"36743d06-…","status":"ERROR","response":"",
1107///  "error":"the connection to the agent was interrupted before the response
1108///           finished: subscriber fell behind updates, stalled for 5s",
1109///  "duration_seconds":431.19,"num_turns":1,
1110///  "usage":{"input_tokens":260113,"output_tokens":14267,
1111///           "thinking_tokens":9695,"cache_read_tokens":2200925}}
1112/// ```
1113///
1114/// Seven minutes of work and fourteen thousand output tokens, billed, with an
1115/// empty `response`: the agent did the job and the CLI's own subscriber fell
1116/// behind and hung up. That is **not** an agent that failed to implement, and
1117/// counting it as one is how `agy` came to read as 0 wins in 4 entries with
1118/// five empty candidates - a number that has twice been used to argue the seat
1119/// out of the roster, and twice been wrong (see `cb6b830`, which reverted the
1120/// first removal: *"agy does not fail to implement, it fails to report"*).
1121///
1122/// The distinction that matters is **billed work with nothing delivered**, so
1123/// that is what this keys on: an error status, an empty response, and a usage
1124/// report showing output tokens. Everything else - including an error with no
1125/// usage at all - returns `None` and stays an ordinary failure, the
1126/// conservative side, exactly as [`claude_quota`] treats shapes it does not
1127/// recognise.
1128///
1129/// Unlike a quota, this **is** worth re-asking: the work exists in the
1130/// conversation the CLI just abandoned, and `conversation_id` is right there.
1131fn dropped_stream(v: &serde_json::Value) -> Option<Dropped> {
1132    let status = v.get("status").and_then(|s| s.as_str()).unwrap_or("");
1133    if !status.eq_ignore_ascii_case("error") {
1134        return None;
1135    }
1136    let response = v.get("response").and_then(|r| r.as_str()).unwrap_or("");
1137    if !response.trim().is_empty() {
1138        // It answered. Whatever the status says, there is something to read.
1139        return None;
1140    }
1141    let produced = v
1142        .get("usage")
1143        .and_then(|u| u.get("output_tokens"))
1144        .and_then(serde_json::Value::as_u64)
1145        .unwrap_or(0);
1146    if produced == 0 {
1147        // An error with nothing produced is just an error.
1148        return None;
1149    }
1150    Some(Dropped {
1151        why: v
1152            .get("error")
1153            .and_then(|e| e.as_str())
1154            .unwrap_or("the CLI ended the stream without delivering its answer")
1155            .trim()
1156            .to_owned(),
1157        output_tokens: produced,
1158    })
1159}
1160
1161/// Preflight: which configured agents are not runnable here?
1162pub fn missing_programs(specs: &[AgentSpec]) -> Vec<String> {
1163    let mut missing = Vec::new();
1164    for s in specs {
1165        let program = match s.kind {
1166            AgentKind::Command => s.command.first().map(String::as_str),
1167            other => other.program(),
1168        };
1169        if let Some(p) = program
1170            && !crate::config::which(p)
1171            && !Path::new(p).is_file()
1172            && !missing.iter().any(|m: &String| m == p)
1173        {
1174            missing.push(p.to_owned());
1175        }
1176    }
1177    missing
1178}
1179
1180/// Absolute path of a run's artifact directory.
1181pub fn artifacts_dir(run_dir: &Path) -> PathBuf {
1182    run_dir.join("artifacts")
1183}
1184
1185/// Can this agent's CLI actually be run on this machine?
1186pub fn installed(spec: &AgentSpec) -> bool {
1187    // A `command` agent has no program of its own to look for - its argv is the
1188    // operator's, and they are the authority on whether it runs.
1189    spec.kind.program().is_none_or(crate::config::which)
1190}
1191
1192/// Choose the agent for a seat that stands alone rather than rotating through
1193/// the roster: [`crate::talk`]'s standing conversation, [`crate::bump`]'s
1194/// release-bump decision, or anything else that needs one agent picked once
1195/// rather than a panel filled in.
1196///
1197/// `available` is a parameter rather than a call to [`installed`] so the order
1198/// below is assertable on a machine with none of these CLIs installed, which is
1199/// every CI runner.
1200///
1201/// The order, and why:
1202///
1203/// 1. An explicit id always wins, and is an error rather than a fallback when
1204///    it is unusable. Naming a seat has a reason, and silently substituting a
1205///    different model would waste whatever that reason was.
1206/// 2. Otherwise a [`AgentKind::Claude`] seat, ahead of the roster order: it is
1207///    the only one of the three CLIs magi can address before the first turn
1208///    (see this module's own doc on session mechanics), which matters most for
1209///    a conversation that opens with nothing typed yet.
1210/// 3. Otherwise the first runnable agent in roster order, because the roster
1211///    order is the operator's own stated preference and magi has nothing
1212///    better to go on.
1213pub fn pick(
1214    agents: &[AgentSpec],
1215    want: Option<&str>,
1216    available: &dyn Fn(&AgentSpec) -> bool,
1217) -> Result<AgentSpec> {
1218    if let Some(id) = want {
1219        let spec = agents
1220            .iter()
1221            .find(|a| a.id == id)
1222            .with_context(|| format!("no agent `{id}` in the roster; it has {}", ids(agents)))?;
1223        if !available(spec) {
1224            bail!(
1225                "agent `{}` needs `{}` on PATH; install it or pass a different \
1226                 --agent",
1227                spec.id,
1228                spec.kind.program().unwrap_or("its command")
1229            );
1230        }
1231        return Ok(spec.clone());
1232    }
1233
1234    if agents.is_empty() {
1235        bail!(
1236            "the agent roster is empty, so there is nobody to ask: install one \
1237             of claude, opencode or agy - magi derives a roster from what is on \
1238             PATH - or add an [[agents]] entry to magi.toml."
1239        );
1240    }
1241
1242    if let Some(spec) = agents
1243        .iter()
1244        .find(|a| a.kind == AgentKind::Claude && available(a))
1245    {
1246        return Ok(spec.clone());
1247    }
1248
1249    agents
1250        .iter()
1251        .find(|a| available(a))
1252        .cloned()
1253        .with_context(|| {
1254            let missing = agents
1255                .iter()
1256                .filter_map(|a| a.kind.program())
1257                .collect::<Vec<_>>()
1258                .join(", ");
1259            format!(
1260                "no agent in the roster can be run here: install one of \
1261                 {missing}, or add an [[agents]] entry to magi.toml for a CLI \
1262                 you do have"
1263            )
1264        })
1265}
1266
1267fn ids(agents: &[AgentSpec]) -> String {
1268    if agents.is_empty() {
1269        return "no agents at all".to_owned();
1270    }
1271    agents
1272        .iter()
1273        .map(|a| a.id.clone())
1274        .collect::<Vec<_>>()
1275        .join(", ")
1276}
1277
1278#[cfg(test)]
1279mod tests {
1280    use super::*;
1281
1282    const COMMAND_HELPER_MODE: &str = "MAGI_TEST_COMMAND_HELPER_MODE";
1283
1284    /// Test-only command agent implemented by this test binary itself. Unlike
1285    /// `echo` and `sleep`, it is available wherever the Rust tests run.
1286    fn command_helper(mode: &str) -> AgentSpec {
1287        AgentSpec {
1288            id: "helper".to_owned(),
1289            kind: AgentKind::Command,
1290            model: None,
1291            command: vec![
1292                std::env::current_exe()
1293                    .expect("locate test helper")
1294                    .to_string_lossy()
1295                    .into_owned(),
1296                "--exact".to_owned(),
1297                "agent::tests::command_agent_test_helper".to_owned(),
1298                "--nocapture".to_owned(),
1299            ],
1300            extra_args: Vec::new(),
1301            env: BTreeMap::from([(COMMAND_HELPER_MODE.to_owned(), mode.to_owned())]),
1302            prompt_delivery: None,
1303        }
1304    }
1305
1306    #[test]
1307    fn command_agent_test_helper() {
1308        match std::env::var(COMMAND_HELPER_MODE).as_deref() {
1309            Ok("reply") => println!("hello {}", std::env::var("MAGI_SEAT").unwrap()),
1310            Ok("cache") => println!("{}", std::env::var("CARGO_TARGET_DIR").unwrap()),
1311            Ok("no-cache") => println!(
1312                "{}",
1313                std::env::var("CARGO_TARGET_DIR").unwrap_or_else(|_| "ABSENT".to_owned())
1314            ),
1315            Ok("ignore-stdin") => println!("done"),
1316            Ok("chatty-sleep") => {
1317                println!("i-said-something");
1318                std::thread::sleep(Duration::from_secs(30));
1319            }
1320            Ok("sleep") => std::thread::sleep(Duration::from_secs(30)),
1321            Ok(other) => panic!("unknown command helper mode {other}"),
1322            Err(_) => {}
1323        }
1324    }
1325
1326    fn spec(kind: AgentKind, model: Option<&str>) -> AgentSpec {
1327        AgentSpec {
1328            id: "a".to_owned(),
1329            kind,
1330            model: model.map(str::to_owned),
1331            command: vec!["echo".to_owned(), "{label}".to_owned()],
1332            extra_args: Vec::new(),
1333            env: BTreeMap::new(),
1334            prompt_delivery: None,
1335        }
1336    }
1337
1338    fn inv<'a>(cwd: &'a Path, art: &'a Path, allow_write: bool) -> Invocation<'a> {
1339        Invocation {
1340            cwd,
1341            prompt: "do the thing",
1342            timeout: Duration::from_secs(900),
1343            allow_write,
1344            sessions: true,
1345            artifacts: art,
1346            stem: "t",
1347            run: "test-run",
1348            node: "test",
1349            cache_dir: None,
1350            attachments: &[],
1351        }
1352    }
1353
1354    fn plan_for(kind: AgentKind, seat: &SeatState, allow_write: bool) -> Plan {
1355        build_command(
1356            &spec(kind, None),
1357            seat,
1358            &inv(Path::new("."), Path::new("/art"), allow_write),
1359            Path::new("/art/p.md"),
1360        )
1361        .unwrap()
1362    }
1363
1364    #[test]
1365    fn claude_mints_then_resumes_the_same_uuid() {
1366        let mut seat = SeatState::new("judge-1", "a", 7);
1367        let uuid = seat.claude_session.clone().unwrap();
1368        let first = plan_for(AgentKind::Claude, &seat, true);
1369        assert!(first.argv.windows(2).any(|w| w == ["--session-id", &uuid]));
1370        assert!(!first.argv.iter().any(|a| a == "--resume"));
1371
1372        seat.turns = 1;
1373        let second = plan_for(AgentKind::Claude, &seat, true);
1374        assert!(second.argv.windows(2).any(|w| w == ["--resume", &uuid]));
1375        assert!(!second.argv.iter().any(|a| a == "--session-id"));
1376    }
1377
1378    #[test]
1379    fn read_only_seats_cannot_edit() {
1380        let seat = SeatState::new("judge-1", "a", 7);
1381        let claude = plan_for(AgentKind::Claude, &seat, false);
1382        assert!(claude.argv.iter().any(|a| a == "--disallowed-tools"));
1383        assert!(
1384            !plan_for(AgentKind::Claude, &seat, true)
1385                .argv
1386                .iter()
1387                .any(|a| a == "--disallowed-tools")
1388        );
1389
1390        let agy = plan_for(AgentKind::Antigravity, &seat, false);
1391        assert!(agy.argv.windows(2).any(|w| w == ["--mode", "plan"]));
1392        assert!(
1393            !agy.argv
1394                .iter()
1395                .any(|a| a == "--dangerously-skip-permissions")
1396        );
1397        let agy_rw = plan_for(AgentKind::Antigravity, &seat, true);
1398        assert!(
1399            agy_rw
1400                .argv
1401                .windows(2)
1402                .any(|w| w == ["--mode", "accept-edits"])
1403        );
1404        assert!(
1405            agy_rw
1406                .argv
1407                .iter()
1408                .any(|a| a == "--dangerously-skip-permissions")
1409        );
1410        // agy is pointed at its prompt with its own `@<path>` syntax, not with
1411        // prose asking it to read a file. Measured on one trivial task: 17s
1412        // against 73s, because prose costs a tool round-trip before the model
1413        // has even seen its instructions. It is also the form rvpm proved.
1414        let agy_prompt = agy_rw
1415            .argv
1416            .iter()
1417            .position(|a| a == "-p")
1418            .map(|i| agy_rw.argv[i + 1].clone())
1419            .expect("agy takes its prompt with -p");
1420        assert!(
1421            agy_prompt.starts_with('@'),
1422            "agy must get a file reference, got {agy_prompt:?}"
1423        );
1424        assert!(
1425            !agy_prompt.contains("Read the file at"),
1426            "the prose pointer is for CLIs with no file syntax"
1427        );
1428
1429        // opencode is the exception: `--auto` also gates reads, so withholding
1430        // it silently drops the seat out of the panel. Verified against the CLI
1431        // — a read-only judge failed with "the user rejected permission to use
1432        // this specific tool call" while trying to open its own prompt.
1433        for allow_write in [false, true] {
1434            assert!(
1435                plan_for(AgentKind::Opencode, &seat, allow_write)
1436                    .argv
1437                    .iter()
1438                    .any(|a| a == "--auto"),
1439                "opencode needs --auto even to read (allow_write = {allow_write})"
1440            );
1441        }
1442    }
1443
1444    /// The three things about `codex exec` that were established by hand and
1445    /// that a rewrite would silently get wrong.
1446    #[test]
1447    fn codex_is_sandboxed_reads_stdin_and_puts_resume_last() {
1448        let mut seat = SeatState::new("judge-1", "a", 7);
1449
1450        // 1. Read-only is enforced by the CLI, not by the prompt - the only
1451        //    roster member for which that is true - and nothing ever asks for
1452        //    the bypass.
1453        let ro = plan_for(AgentKind::Codex, &seat, false);
1454        assert!(ro.argv.windows(2).any(|w| w == ["--sandbox", "read-only"]));
1455        let rw = plan_for(AgentKind::Codex, &seat, true);
1456        assert!(
1457            rw.argv
1458                .windows(2)
1459                .any(|w| w == ["--sandbox", "workspace-write"])
1460        );
1461        for p in [&ro, &rw] {
1462            assert!(
1463                !p.argv
1464                    .iter()
1465                    .any(|a| a == "--dangerously-bypass-approvals-and-sandbox"),
1466                "the bypass defeats the only enforced read-only mode we have"
1467            );
1468            // Nobody is watching to approve anything.
1469            assert!(
1470                p.argv
1471                    .windows(2)
1472                    .any(|w| w == ["-c", "approval_policy=\"never\""]),
1473                "an unattended seat that asks for approval blocks until timeout"
1474            );
1475        }
1476
1477        // 2. The prompt arrives on stdin, and `-` is what makes codex read it.
1478        assert_eq!(ro.stdin.as_deref(), Some("do the thing"));
1479        assert_eq!(
1480            ro.argv.last().map(String::as_str),
1481            Some("-"),
1482            "without the `-` argument codex waits for a prompt it never gets"
1483        );
1484
1485        // 3. `resume` is a subcommand and rejects the options above when they
1486        //    follow it, so it has to be emitted after all of them - and only
1487        //    once the CLI has reported a thread id.
1488        seat.turns = 1;
1489        assert!(!has_session(AgentKind::Codex, &seat, true));
1490        assert!(
1491            !plan_for(AgentKind::Codex, &seat, true)
1492                .argv
1493                .iter()
1494                .any(|a| a == "resume")
1495        );
1496        seat.captured_session = Some("01a07440-4545-7492-85c1-024e3259a90a".to_owned());
1497        let resumed = plan_for(AgentKind::Codex, &seat, true);
1498        let at = resumed
1499            .argv
1500            .iter()
1501            .position(|a| a == "resume")
1502            .expect("resumes by subcommand");
1503        assert_eq!(resumed.argv[at + 1], "01a07440-4545-7492-85c1-024e3259a90a");
1504        assert!(
1505            resumed.argv[..at].iter().any(|a| a == "--sandbox"),
1506            "every option precedes the subcommand"
1507        );
1508        assert_eq!(resumed.argv.last().map(String::as_str), Some("-"));
1509    }
1510
1511    /// The three things about `omp -p --mode=json` that were established by
1512    /// hand against omp 18.1.19 and that a rewrite would silently get wrong.
1513    #[test]
1514    fn omp_reads_stdin_auto_approves_and_resumes_by_id() {
1515        let mut seat = SeatState::new("review-1", "a", 7);
1516
1517        // 1. Print mode plus JSON, and the prompt on stdin: a judging prompt
1518        //    carrying three patches is past the Windows argv cap, so argv
1519        //    delivery is not an option for every node.
1520        let first = plan_for(AgentKind::Omp, &seat, false);
1521        assert!(first.argv.iter().any(|a| a == "-p"));
1522        assert!(first.argv.iter().any(|a| a == "--mode=json"));
1523        assert_eq!(first.stdin.as_deref(), Some("do the thing"));
1524        assert!(
1525            !first.argv.iter().any(|a| a == "do the thing"),
1526            "the prompt reached argv, where Windows caps it"
1527        );
1528
1529        // 2. `--auto-approve` is required (an unattended seat that stops to ask
1530        //    blocks until its node timeout kills it), and it is the *only*
1531        //    permission flag: omp has no read-only mode, so the bypass flag
1532        //    that would throw away codex's one enforced guarantee must never
1533        //    appear here either.
1534        for allow_write in [false, true] {
1535            let p = plan_for(AgentKind::Omp, &seat, allow_write);
1536            assert!(
1537                p.argv.iter().any(|a| a == "--auto-approve"),
1538                "omp needs --auto-approve even to read (allow_write = {allow_write})"
1539            );
1540            assert!(
1541                !p.argv
1542                    .iter()
1543                    .any(|a| a == "--dangerously-bypass-approvals-and-sandbox"),
1544                "nothing ever asks for the bypass"
1545            );
1546        }
1547
1548        // 3. The id omp reports is the only resume token - magi cannot mint it
1549        //    up front, so a seat resumes only once a turn has reported one.
1550        seat.turns = 1;
1551        assert!(!has_session(AgentKind::Omp, &seat, true));
1552        assert!(
1553            !plan_for(AgentKind::Omp, &seat, true)
1554                .argv
1555                .iter()
1556                .any(|a| a == "--resume")
1557        );
1558        seat.captured_session = Some("01a09fe9-4e31-7226-85b3-fda6f46689d5".to_owned());
1559        let resumed = plan_for(AgentKind::Omp, &seat, true);
1560        assert!(
1561            resumed
1562                .argv
1563                .windows(2)
1564                .any(|w| w == ["--resume", "01a09fe9-4e31-7226-85b3-fda6f46689d5"]),
1565            "a captured id is what makes the next turn a resume"
1566        );
1567        // `--continue` opens a *new* session instead of the stored one, which
1568        // would silently drop the seat's memory.
1569        assert!(!resumed.argv.iter().any(|a| a == "--continue"));
1570        // stdin still carries the prompt on a resumed turn.
1571        assert_eq!(resumed.stdin.as_deref(), Some("do the thing"));
1572    }
1573
1574    /// The extraction trap that cost three complete reviews when it was done by
1575    /// hand: a turn that ends on a tool call emits **no** `agent_end` line, so
1576    /// keying on `agent_end` finds nothing and the seat reads as one that
1577    /// produced no answer at all.
1578    #[test]
1579    fn omp_takes_the_answer_without_an_agent_end_line() {
1580        let stream = concat!(
1581            r#"{"type":"session","version":3,"id":"01a09fe9-4e31-7226-85b3-fda6f46689d5","cwd":"C:\\w"}"#,
1582            "\n",
1583            r#"{"type":"agent_start"}"#,
1584            "\n",
1585            r#"{"type":"turn_start"}"#,
1586            "\n",
1587            r#"{"type":"message_update","assistantMessageEvent":{"type":"text_delta","contentIndex":1,"delta":"."}}"#,
1588            "\n",
1589            r#"{"type":"message_end","message":{"role":"assistant","content":[{"type":"thinking","thinking":"checking"},{"type":"text","text":"."}]}}"#,
1590            "\n",
1591            r#"{"type":"turn_end","message":{"role":"assistant","content":[{"type":"thinking","thinking":"done"},{"type":"text","text":"{\"vote\":\"approve\"}"}]}}"#,
1592            "\n",
1593        );
1594        let out = extract(AgentKind::Omp, stream);
1595        assert_eq!(
1596            out.text, "{\"vote\":\"approve\"}",
1597            "the last assistant text block is the answer even with no agent_end"
1598        );
1599        assert_eq!(
1600            out.session.as_deref(),
1601            Some("01a09fe9-4e31-7226-85b3-fda6f46689d5")
1602        );
1603    }
1604
1605    /// A stream that *does* carry `agent_end` walks the whole thread, and the
1606    /// last non-empty assistant text still wins over the tool-loop narration
1607    /// that came before it.
1608    #[test]
1609    fn omp_walks_agent_end_and_ignores_tool_loop_narration() {
1610        let stream = concat!(
1611            r#"{"type":"session","version":3,"id":"s1"}"#,
1612            "\n",
1613            "{\"type\":\"agent_end\",\"messages\":[{\"role\":\"user\",\"content\":[{\"type\":\"text\",\"text\":\"review this\"}]},{\"role\":\"assistant\",\"content\":[{\"type\":\"text\",\"text\":\"Looking at the diff…\"}]},{\"role\":\"assistant\",\"content\":[{\"type\":\"thinking\",\"thinking\":\"…\"},{\"type\":\"text\",\"text\":\"## 判定\\n\\n問題ありません。\"}]}]}",
1614            "\n",
1615        );
1616        let out = extract(AgentKind::Omp, stream);
1617        assert_eq!(
1618            out.text, "## 判定\n\n問題ありません。",
1619            "the narration is not the answer, and non-ASCII survives intact"
1620        );
1621        assert_eq!(out.session.as_deref(), Some("s1"));
1622    }
1623
1624    /// A line that is not JSON - a CLI warning, a half-written line - is
1625    /// skipped rather than becoming the answer.
1626    #[test]
1627    fn omp_skips_non_json_lines() {
1628        let stream = concat!(
1629            "Warning: some omp notice\n",
1630            r#"{"type":"session","version":3,"id":"s2"}"#,
1631            "\n",
1632            r#"{"type":"message_end","message":{"role":"assistant","content":[{"type":"text","text":"the answer"}]}}"#,
1633            "\n",
1634            "trailing junk",
1635            "\n",
1636        );
1637        let out = extract(AgentKind::Omp, stream);
1638        assert_eq!(out.text, "the answer");
1639        assert_eq!(out.session.as_deref(), Some("s2"));
1640    }
1641
1642    /// A real `codex exec --json` stream, tracing prefix included.
1643    #[test]
1644    fn codex_takes_the_last_agent_message_and_the_thread_id() {
1645        let stream = concat!(
1646            "2026-09-06T01:05:49.394445Z ERROR codex_models_manager: failed to load models cache\n",
1647            r#"{"type":"thread.started","thread_id":"01a07440-4545-7492-85c1-024e3259a90a"}"#,
1648            "\n",
1649            r#"{"type":"turn.started"}"#,
1650            "\n",
1651            r#"{"type":"item.completed","item":{"id":"item_0","type":"agent_message","text":"Looking into it."}}"#,
1652            "\n",
1653            r#"{"type":"item.completed","item":{"id":"item_1","type":"command_execution","text":"cargo test"}}"#,
1654            "\n",
1655            r#"{"type":"item.completed","item":{"id":"item_2","type":"agent_message","text":"{\"verdict\": \"ok\"}"}}"#,
1656            "\n",
1657            r#"{"type":"turn.completed","usage":{"input_tokens":17137}}"#,
1658            "\n",
1659        );
1660        let out = extract(AgentKind::Codex, stream);
1661        assert_eq!(
1662            out.text, "{\"verdict\": \"ok\"}",
1663            "the last agent message is the answer; earlier ones narrate"
1664        );
1665        assert_eq!(
1666            out.session.as_deref(),
1667            Some("01a07440-4545-7492-85c1-024e3259a90a")
1668        );
1669        assert_eq!(out.status.as_deref(), Some("success"));
1670
1671        let failed = concat!(
1672            r#"{"type":"thread.started","thread_id":"t1"}"#,
1673            "\n",
1674            r#"{"type":"turn.failed","error":{"message":"nope"}}"#,
1675            "\n",
1676        );
1677        assert_eq!(
1678            extract(AgentKind::Codex, failed).status.as_deref(),
1679            Some("error")
1680        );
1681    }
1682
1683    #[test]
1684    fn captured_sessions_resume_only_once_reported() {
1685        let mut seat = SeatState::new("impl-A", "a", 7);
1686        seat.turns = 1;
1687        for kind in [AgentKind::Opencode, AgentKind::Antigravity] {
1688            assert!(!has_session(kind, &seat, true));
1689            let p = plan_for(kind, &seat, true);
1690            assert!(!p.argv.iter().any(|a| a == "-s" || a == "--conversation"));
1691        }
1692
1693        seat.captured_session = Some("sid".to_owned());
1694        assert!(has_session(AgentKind::Opencode, &seat, true));
1695        assert!(
1696            plan_for(AgentKind::Opencode, &seat, true)
1697                .argv
1698                .windows(2)
1699                .any(|w| w == ["-s", "sid"])
1700        );
1701        assert!(
1702            plan_for(AgentKind::Antigravity, &seat, true)
1703                .argv
1704                .windows(2)
1705                .any(|w| w == ["--conversation", "sid"])
1706        );
1707    }
1708
1709    #[test]
1710    fn sessions_disabled_never_resumes() {
1711        let mut seat = SeatState::new("impl-A", "a", 7);
1712        seat.turns = 3;
1713        seat.captured_session = Some("sid".to_owned());
1714        for kind in [
1715            AgentKind::Claude,
1716            AgentKind::Opencode,
1717            AgentKind::Antigravity,
1718        ] {
1719            assert!(!has_session(kind, &seat, false));
1720        }
1721    }
1722
1723    #[test]
1724    fn long_prompts_never_reach_argv_for_file_delivery_clis() {
1725        let seat = SeatState::new("judge-1", "a", 7);
1726        for kind in [AgentKind::Opencode, AgentKind::Antigravity] {
1727            let p = plan_for(kind, &seat, false);
1728            assert!(
1729                p.argv.iter().all(|a| a != "do the thing"),
1730                "{kind:?} put the prompt on the command line"
1731            );
1732            assert!(p.argv.iter().any(|a| a.contains("/art/p.md")));
1733        }
1734        // agy has no text stdin, so its `-p` must always carry something.
1735        let p = plan_for(AgentKind::Antigravity, &seat, false);
1736        let at = p.argv.iter().position(|a| a == "-p").unwrap();
1737        assert!(p.argv.get(at + 1).is_some_and(|v| v.contains("p.md")));
1738        assert!(p.stdin.is_none());
1739    }
1740
1741    #[test]
1742    fn agy_print_timeout_tracks_the_node_budget() {
1743        let seat = SeatState::new("impl-A", "a", 7);
1744        let p = build_command(
1745            &spec(AgentKind::Antigravity, None),
1746            &seat,
1747            &Invocation {
1748                cwd: Path::new("."),
1749                prompt: "p",
1750                timeout: Duration::from_secs(3600),
1751                allow_write: true,
1752                sessions: true,
1753                artifacts: Path::new("/art"),
1754                stem: "t",
1755                run: "test-run",
1756                node: "test",
1757                cache_dir: None,
1758                attachments: &[],
1759            },
1760            Path::new("/art/p.md"),
1761        )
1762        .unwrap();
1763        assert!(p.argv.windows(2).any(|w| w == ["--print-timeout", "3600s"]));
1764    }
1765
1766    /// `--add-dir` is what lets antigravity open a file outside the
1767    /// worktree at all. Today that only happens when the delivery mode is
1768    /// already `File`, but an attachment can arrive on a seat whose delivery
1769    /// is `Stdin` or `Argv` (an explicit `prompt_delivery` override), and the
1770    /// image still lives outside `cwd` - so the flag has to widen for that
1771    /// reason too, independent of how the prompt itself is delivered.
1772    #[test]
1773    fn attachments_widen_antigravitys_add_dir_even_off_file_delivery() {
1774        let mut s = spec(AgentKind::Antigravity, None);
1775        s.prompt_delivery = Some(Delivery::Argv);
1776        let seat = SeatState::new("talk", "a", 7);
1777        let atts = [PathBuf::from("/art/attachments/abc.png")];
1778
1779        let without = build_command(
1780            &s,
1781            &seat,
1782            &Invocation {
1783                attachments: &[],
1784                ..inv(Path::new("."), Path::new("/art"), true)
1785            },
1786            Path::new("/art/p.md"),
1787        )
1788        .unwrap();
1789        assert!(
1790            !without.argv.iter().any(|a| a == "--add-dir"),
1791            "no attachment, no reason to widen the sandbox: {without:?}"
1792        );
1793
1794        let with = build_command(
1795            &s,
1796            &seat,
1797            &Invocation {
1798                attachments: &atts,
1799                ..inv(Path::new("."), Path::new("/art"), true)
1800            },
1801            Path::new("/art/p.md"),
1802        )
1803        .unwrap();
1804        assert!(
1805            with.argv.windows(2).any(|w| w == ["--add-dir", "/art"]),
1806            "an attachment outside cwd must widen the sandbox even off File delivery: {with:?}"
1807        );
1808    }
1809
1810    /// A chat derived from another one (`chat::derived_background`) can pass
1811    /// `turn` attachment paths that live under the *source* conversation's
1812    /// own artifacts dir, not this invocation's `artifacts`. A single
1813    /// `--add-dir` for `inv.artifacts` alone would leave those unreadable, so
1814    /// each attachment directory outside it must get its own grant.
1815    #[test]
1816    fn an_inherited_attachment_outside_this_conversations_artifacts_dir_gets_its_own_add_dir() {
1817        let seat = SeatState::new("plan", "a", 7);
1818        let atts = [
1819            PathBuf::from("/art/attachments/own.png"),
1820            PathBuf::from("/other-chat/attachments/inherited.png"),
1821        ];
1822
1823        let p = build_command(
1824            &spec(AgentKind::Antigravity, None),
1825            &seat,
1826            &Invocation {
1827                attachments: &atts,
1828                ..inv(Path::new("."), Path::new("/art"), true)
1829            },
1830            Path::new("/art/p.md"),
1831        )
1832        .unwrap();
1833
1834        assert!(
1835            p.argv.windows(2).any(|w| w == ["--add-dir", "/art"]),
1836            "this conversation's own artifacts dir must still be granted: {p:?}"
1837        );
1838        assert!(
1839            p.argv
1840                .windows(2)
1841                .any(|w| w == ["--add-dir", "/other-chat/attachments"]),
1842            "the inherited attachment's own directory must be granted too: {p:?}"
1843        );
1844    }
1845
1846    #[test]
1847    fn command_agents_get_placeholders_substituted() {
1848        let seat = SeatState::new("impl-A", "a", 7);
1849        let p = plan_for(AgentKind::Command, &seat, true);
1850        assert_eq!(p.argv[0], "echo");
1851        assert_eq!(p.argv[1], "impl-A");
1852        assert_eq!(p.stdin.as_deref(), Some("do the thing"));
1853    }
1854
1855    #[test]
1856    fn claude_rate_limit_is_detected_and_reset_read_when_present() {
1857        // The exact shape observed in the wild (run 20260831-031005-ae94).
1858        let stdout = r#"{"is_error": true, "terminal_reason": "api_error",
1859                        "result": "You've hit your session limit · resets 4:50am (Asia/Tokyo)",
1860                        "session_id": "b8e928f1-754e-4bd3-86c5-0567763654e3"}"#;
1861        let out = extract(AgentKind::Claude, stdout);
1862        let quota = out.quota.as_ref().expect("rate limit must be detected");
1863        assert_eq!(
1864            quota.reset.as_deref(),
1865            Some("4:50am (Asia/Tokyo)"),
1866            "reset time read from the body"
1867        );
1868    }
1869
1870    #[test]
1871    fn claude_rate_limit_without_a_readable_reset_is_still_detected() {
1872        let out = extract(
1873            AgentKind::Claude,
1874            r#"{"is_error":true,"result":"session limit reached"}"#,
1875        );
1876        let quota = out.quota.expect("rate limit detected without a reset");
1877        assert!(quota.reset.is_none(), "unknown reset is kept as unknown");
1878    }
1879
1880    #[test]
1881    fn ordinary_failures_are_never_quota() {
1882        // A normal failed claude call (is_error with a different message).
1883        let claude_fail = extract(
1884            AgentKind::Claude,
1885            r#"{"is_error":true,"result":"account does not exist"}"#,
1886        );
1887        assert!(claude_fail.quota.is_none());
1888
1889        // A command agent that exits 1 with plain text.
1890        let cmd_fail = extract(AgentKind::Command, "boom");
1891        assert!(cmd_fail.quota.is_none());
1892
1893        // A successful call is not quota even if it mentions the phrase.
1894        let success = extract(
1895            AgentKind::Command,
1896            r#"{"is_error":false,"result":"session limit is fine"}"#,
1897        );
1898        assert!(success.quota.is_none());
1899    }
1900
1901    /// Minimal, anonymised shape of run 20260912-214939-b3bb's
1902    /// artifacts/impl-A.out: `command_execution` items reporting a paired
1903    /// test run as `1 passed, 1 failed` twice, while the final
1904    /// `agent_message` nevertheless claimed the target passed. The point of
1905    /// `CommandEvidence` is that this claim and the CLI's own structured
1906    /// record of what actually ran are now two separate things a caller can
1907    /// compare, rather than the prose being the only account available.
1908    #[test]
1909    fn codex_command_execution_events_are_captured_alongside_the_final_message() {
1910        let stream = concat!(
1911            r#"{"type":"thread.started","thread_id":"t1"}"#,
1912            "\n",
1913            r#"{"type":"item.completed","item":{"id":"item49","type":"command_execution","command":["bash","-lc","cargo test --test graph_cached_gate"],"exit_code":1,"aggregated_output":"test result: 1 passed; 1 failed"}}"#,
1914            "\n",
1915            r#"{"type":"item.completed","item":{"id":"item52","type":"command_execution","command":["bash","-lc","cargo test --test graph_cached_gate a_single_test"],"exit_code":0,"aggregated_output":"test result: 1 passed; 0 failed"}}"#,
1916            "\n",
1917            r#"{"type":"item.completed","item":{"id":"item99","type":"agent_message","text":"Both tests in the target pass."}}"#,
1918            "\n",
1919            r#"{"type":"turn.completed"}"#,
1920            "\n",
1921        );
1922        let out = extract(AgentKind::Codex, stream);
1923        assert_eq!(out.text, "Both tests in the target pass.");
1924        assert_eq!(out.commands.len(), 2, "{:?}", out.commands);
1925
1926        let paired = &out.commands[0];
1927        assert_eq!(paired.id, "item49");
1928        assert_eq!(paired.exit_code, Some(1));
1929        assert!(paired.description.contains("graph_cached_gate"));
1930        assert!(paired.result_summary.contains("1 failed"));
1931
1932        let solo = &out.commands[1];
1933        assert_eq!(solo.exit_code, Some(0));
1934
1935        // The structured evidence disagrees with the final prose - exactly
1936        // what a caller must be able to see instead of trusting the message
1937        // alone: the full target never passed in one command.
1938        assert!(
1939            out.commands
1940                .iter()
1941                .any(|c| c.exit_code != Some(0) && c.description.contains("graph_cached_gate")),
1942            "a failed run of the actual target must still be visible: {:?}",
1943            out.commands
1944        );
1945    }
1946
1947    #[test]
1948    fn command_agent_can_carry_the_claude_quota_shape() {
1949        let out = extract(
1950            AgentKind::Command,
1951            r#"{"is_error":true,"result":"You've hit your session limit · resets 1:00am (UTC)"}"#,
1952        );
1953        assert!(
1954            out.quota.is_some(),
1955            "a wrapper emitting the claude shape counts as quota"
1956        );
1957    }
1958
1959    #[test]
1960    fn claude_json_result_is_extracted() {
1961        let out = extract(
1962            AgentKind::Claude,
1963            r#"{"result":"all done","session_id":"abc","is_error":false}"#,
1964        );
1965        assert_eq!(out.text, "all done");
1966        assert_eq!(out.session.as_deref(), Some("abc"));
1967        assert_eq!(out.status.as_deref(), Some("success"));
1968    }
1969
1970    /// The shape behind fix-2 in run 20260912-114326-d3b8, minimal and
1971    /// anonymised: a Claude CLI turn that ended `subtype: success`,
1972    /// `is_error: false`, `terminal_reason: completed`, `stop_reason:
1973    /// end_turn` — every signal this crate reads as a clean CLI turn — while
1974    /// `result` is a progress update, not the report the fixer node needed,
1975    /// and no `FixReport` JSON is anywhere in it.
1976    ///
1977    /// `AgentOutput::usable()` (a CLI fact: exit 0, not timed out, non-empty
1978    /// text) must stay true here — that is the honest reading of what the
1979    /// CLI reported — while `verdict::extract_json` on the same text must
1980    /// fail. Conflating the two is exactly the bug this fixture reproduces:
1981    /// `run.json` recorded `fix.failed = "unparsable fix report: the reply
1982    /// contained no JSON object"` and moved straight to the next review round
1983    /// with no report ever recovered from that seat.
1984    #[test]
1985    fn a_clean_cli_turn_is_not_the_same_fact_as_the_nodes_own_work_being_done() {
1986        let stdout = r#"{"type":"result","subtype":"success","is_error":false,"terminal_reason":"completed","stop_reason":"end_turn","result":"I'll pause here until the `cargo make check` background run reports back.","session_id":"11111111-1111-1111-1111-111111111111"}"#;
1987        let out = extract(AgentKind::Claude, stdout);
1988        assert_eq!(out.status.as_deref(), Some("success"));
1989        assert!(out.quota.is_none());
1990        assert!(!out.text.trim().is_empty());
1991
1992        let agent_out = AgentOutput {
1993            text: out.text.clone(),
1994            exit_code: Some(0),
1995            timed_out: false,
1996            duration_ms: 500,
1997            artifacts: Vec::new(),
1998            quota: out.quota,
1999            dropped: out.dropped,
2000            commands: out.commands,
2001        };
2002        assert!(
2003            agent_out.usable(),
2004            "the CLI turn itself ended cleanly and must read as usable"
2005        );
2006        assert!(
2007            crate::verdict::extract_json::<crate::verdict::FixReport>(&agent_out.text).is_err(),
2008            "a clean CLI turn is not proof the node's own report ever arrived"
2009        );
2010    }
2011
2012    #[test]
2013    fn opencode_event_stream_is_concatenated() {
2014        let stream = concat!(
2015            r#"{"type":"step_start","sessionID":"ses_1","part":{"type":"step-start"}}"#,
2016            "\n",
2017            r#"{"type":"text","sessionID":"ses_1","part":{"type":"text","text":"first"}}"#,
2018            "\n",
2019            "garbage line\n",
2020            r#"{"type":"text","sessionID":"ses_1","part":{"type":"text","text":"second"}}"#,
2021            "\n"
2022        );
2023        let out = extract(AgentKind::Opencode, stream);
2024        assert_eq!(out.text, "first\nsecond");
2025        assert_eq!(out.session.as_deref(), Some("ses_1"));
2026    }
2027
2028    #[test]
2029    fn agy_json_survives_a_leading_warning_line() {
2030        let stdout = concat!(
2031            "warning: --mode plan has no effect while slash commands are disabled.\n",
2032            r#"{"conversation_id":"eaf2d00a","status":"SUCCESS","response":"persimmon\n"}"#,
2033            "\n"
2034        );
2035        let out = extract(AgentKind::Antigravity, stdout);
2036        assert_eq!(out.text, "persimmon");
2037        assert_eq!(out.session.as_deref(), Some("eaf2d00a"));
2038        assert_eq!(out.status.as_deref(), Some("SUCCESS"));
2039    }
2040
2041    /// Run 26c7's candidate B, verbatim from `artifacts/impl-B.out`.
2042    ///
2043    /// The seat read as an empty candidate. It was seven minutes of work and
2044    /// 14,267 output tokens, billed, that the CLI then declined to hand over.
2045    /// Five such candidates are why `agy` reads as 0 wins in 4 entries, and
2046    /// that number has twice been used to argue the seat out of the roster.
2047    const AGY_DROPPED: &str = concat!(
2048        r#"{"conversation_id":"36743d06-c0b3-4b79-9fa2-23869289d7b6","status":"ERROR","#,
2049        r#""response":"","error":"the connection to the agent was interrupted before "#,
2050        r#"the response finished: subscriber fell behind updates, stalled for 5s","#,
2051        r#""duration_seconds":431.1941803,"num_turns":1,"usage":{"input_tokens":260113,"#,
2052        r#""output_tokens":14267,"thinking_tokens":9695,"cache_read_tokens":2200925,"#,
2053        r#""total_tokens":274380}}"#
2054    );
2055
2056    #[test]
2057    fn a_cli_that_hangs_up_on_billed_work_is_not_an_agent_that_produced_nothing() {
2058        let out = extract(AgentKind::Antigravity, AGY_DROPPED);
2059        let dropped = out.dropped.expect("recognised as undelivered work");
2060        assert_eq!(dropped.output_tokens, 14267);
2061        assert!(
2062            dropped.why.contains("subscriber fell behind"),
2063            "the CLI's own words are kept for the record: {}",
2064            dropped.why
2065        );
2066        // And the conversation is still there to resume, which is the whole
2067        // reason this is worth re-asking where a quota is not.
2068        assert_eq!(
2069            out.session.as_deref(),
2070            Some("36743d06-c0b3-4b79-9fa2-23869289d7b6")
2071        );
2072        assert!(out.quota.is_none(), "a dropped stream is not a rate limit");
2073    }
2074
2075    #[test]
2076    fn an_error_with_nothing_produced_stays_an_ordinary_failure() {
2077        // No usage at all: the agent never got going, so there is nothing in
2078        // the conversation to resume and nothing was billed. Treating this as
2079        // undelivered work would buy a second call for no reason.
2080        let bare = r#"{"conversation_id":"c1","status":"ERROR","response":"","error":"boom"}"#;
2081        assert!(extract(AgentKind::Antigravity, bare).dropped.is_none());
2082
2083        // Produced tokens, but it did answer - so there is something to read
2084        // and the status is not our business.
2085        let answered = concat!(
2086            r#"{"conversation_id":"c2","status":"ERROR","response":"here it is","#,
2087            r#""usage":{"output_tokens":10}}"#
2088        );
2089        assert!(extract(AgentKind::Antigravity, answered).dropped.is_none());
2090
2091        // A success is a success.
2092        let ok = concat!(
2093            r#"{"conversation_id":"c3","status":"SUCCESS","response":"done","#,
2094            r#""usage":{"output_tokens":10}}"#
2095        );
2096        assert!(extract(AgentKind::Antigravity, ok).dropped.is_none());
2097    }
2098
2099    #[test]
2100    fn an_undelivered_output_is_not_usable_but_is_worth_asking_again() {
2101        let out = AgentOutput {
2102            text: String::new(),
2103            exit_code: Some(1),
2104            timed_out: false,
2105            duration_ms: 431_194,
2106            artifacts: Vec::new(),
2107            quota: None,
2108            dropped: Some(Dropped {
2109                why: "subscriber fell behind updates".to_owned(),
2110                output_tokens: 14267,
2111            }),
2112            commands: Vec::new(),
2113        };
2114        assert!(!out.usable());
2115        assert!(out.work_undelivered());
2116        // The distinction the retry policy rests on: a quota fails the same way
2117        // until it resets, an abandoned conversation can be picked up.
2118        assert!(!out.quota_exhausted());
2119    }
2120
2121    #[test]
2122    fn non_json_stdout_falls_back_to_raw_text() {
2123        let out = extract(AgentKind::Antigravity, "plain answer\n");
2124        assert_eq!(out.text, "plain answer");
2125        assert!(out.session.is_none());
2126    }
2127
2128    #[tokio::test]
2129    async fn command_agent_round_trip_writes_artifacts() {
2130        let dir = tempfile::tempdir().unwrap();
2131        let art = dir.path().join("artifacts");
2132        let mut seat = SeatState::new("impl-A", "a", 7);
2133        let s = command_helper("reply");
2134        let out = invoke(
2135            &s,
2136            &mut seat,
2137            &Invocation {
2138                cwd: dir.path(),
2139                prompt: "unused",
2140                timeout: Duration::from_secs(30),
2141                allow_write: true,
2142                sessions: true,
2143                artifacts: &art,
2144                stem: "impl-A",
2145                run: "test-run",
2146                node: "test",
2147                cache_dir: None,
2148                attachments: &[],
2149            },
2150        )
2151        .await
2152        .unwrap();
2153        assert!(out.usable(), "{out:?}");
2154        assert!(out.text.contains("hello impl-A"), "{}", out.text);
2155        assert_eq!(seat.turns, 1);
2156        assert!(art.join("impl-A.prompt.md").is_file());
2157        assert!(art.join("impl-A.out").is_file());
2158    }
2159
2160    #[tokio::test]
2161    async fn the_invocation_cache_dir_reaches_the_seat_as_cargo_target_dir() {
2162        // The whole point of threading the cache path through `Invocation`:
2163        // the compile the agent pays for lands in the directory `verify` reads
2164        // back out of its rendered commands, so one cache has one prune.
2165        let dir = tempfile::tempdir().unwrap();
2166        let cache = dir.path().join("magi-cache");
2167        let mut seat = SeatState::new("impl-A", "a", 7);
2168        let s = command_helper("cache");
2169        let out = invoke(
2170            &s,
2171            &mut seat,
2172            &Invocation {
2173                cwd: dir.path(),
2174                prompt: "unused",
2175                timeout: Duration::from_secs(30),
2176                allow_write: true,
2177                sessions: true,
2178                artifacts: &dir.path().join("artifacts"),
2179                stem: "cache",
2180                run: "test-run",
2181                node: "test",
2182                cache_dir: Some(&cache),
2183                attachments: &[],
2184            },
2185        )
2186        .await
2187        .unwrap();
2188        assert!(out.usable(), "{out:?}");
2189        assert!(
2190            out.text.contains(cache.to_string_lossy().as_ref()),
2191            "the seat must see CARGO_TARGET_DIR = the shared cache"
2192        );
2193    }
2194
2195    #[tokio::test]
2196    async fn cache_dir_none_strips_a_cargo_target_dir_inherited_from_this_process() {
2197        // `Command` inherits the parent's environment by default, so
2198        // `cache_dir: None` alone is not the same as a seat never seeing
2199        // `CARGO_TARGET_DIR` - it also has to be true when *this* process
2200        // (standing in for the real magi process, which normally does have
2201        // one set, from its own `[verify]` config) already has the variable
2202        // set. Simulating that is the only way to exercise the inheritance
2203        // path at all.
2204        let previous = std::env::var("CARGO_TARGET_DIR").ok();
2205        // SAFETY: this crate's tests run single-threaded
2206        // (`RUST_TEST_THREADS=1`); see
2207        // `updater::tests::env_kill_switch_semantics` for the same
2208        // reasoning applied to another process-global env var.
2209        unsafe {
2210            std::env::set_var("CARGO_TARGET_DIR", "/should/never/reach/a/read-only/seat");
2211        }
2212        let dir = tempfile::tempdir().unwrap();
2213        let mut seat = SeatState::new("review-1", "a", 7);
2214        let s = command_helper("no-cache");
2215        let result = invoke(
2216            &s,
2217            &mut seat,
2218            &Invocation {
2219                cwd: dir.path(),
2220                prompt: "unused",
2221                timeout: Duration::from_secs(30),
2222                allow_write: false,
2223                sessions: true,
2224                artifacts: &dir.path().join("artifacts"),
2225                stem: "no-cache",
2226                run: "test-run",
2227                node: "test",
2228                cache_dir: None,
2229                attachments: &[],
2230            },
2231        )
2232        .await;
2233        // Restored before any assertion that could panic, so a failure here
2234        // never leaks a bogus `CARGO_TARGET_DIR` into whichever test runs
2235        // next in this same process.
2236        // SAFETY: see above.
2237        unsafe {
2238            match &previous {
2239                Some(v) => std::env::set_var("CARGO_TARGET_DIR", v),
2240                None => std::env::remove_var("CARGO_TARGET_DIR"),
2241            }
2242        }
2243        let out = result.unwrap();
2244        assert!(out.usable(), "{out:?}");
2245        assert!(
2246            out.text.contains("ABSENT"),
2247            "a read-only seat must never inherit the process's own CARGO_TARGET_DIR: {}",
2248            out.text
2249        );
2250    }
2251
2252    #[tokio::test]
2253    async fn a_prompt_larger_than_the_pipe_buffer_does_not_deadlock() {
2254        let dir = tempfile::tempdir().unwrap();
2255        let mut seat = SeatState::new("impl-A", "a", 7);
2256        // The helper never reads stdin, so an inline write_all would block once
2257        // the OS pipe buffer filled — long before the process could be waited on.
2258        let s = command_helper("ignore-stdin");
2259        let big = "x".repeat(1_000_000);
2260        let out = invoke(
2261            &s,
2262            &mut seat,
2263            &Invocation {
2264                cwd: dir.path(),
2265                prompt: &big,
2266                timeout: Duration::from_secs(60),
2267                allow_write: true,
2268                sessions: true,
2269                artifacts: &dir.path().join("artifacts"),
2270                stem: "big",
2271                run: "test-run",
2272                node: "test",
2273                cache_dir: None,
2274                attachments: &[],
2275            },
2276        )
2277        .await
2278        .unwrap();
2279        assert!(out.usable(), "{out:?}");
2280        assert!(out.text.contains("done"), "{}", out.text);
2281    }
2282
2283    #[tokio::test]
2284    async fn timeout_is_reported_not_hung() {
2285        let dir = tempfile::tempdir().unwrap();
2286        let mut seat = SeatState::new("impl-A", "a", 7);
2287        let s = command_helper("sleep");
2288        let out = invoke(
2289            &s,
2290            &mut seat,
2291            &Invocation {
2292                cwd: dir.path(),
2293                prompt: "unused",
2294                timeout: Duration::from_millis(300),
2295                allow_write: true,
2296                sessions: true,
2297                artifacts: &dir.path().join("artifacts"),
2298                stem: "slow",
2299                run: "test-run",
2300                node: "test",
2301                cache_dir: None,
2302                attachments: &[],
2303            },
2304        )
2305        .await
2306        .unwrap();
2307        assert!(out.timed_out);
2308        assert!(!out.usable());
2309    }
2310
2311    #[tokio::test]
2312    async fn a_timeout_keeps_what_the_agent_had_already_printed() {
2313        // The old implementation cancelled `wait_with_output`, which dropped
2314        // the buffers it owned, so `<stem>.out` was written empty on every
2315        // timeout. "It printed nothing" and "we discarded what it printed"
2316        // looked identical on disk — and one real hour-long stall was
2317        // diagnosed wrongly twice because of it.
2318        let dir = tempfile::tempdir().unwrap();
2319        let artifacts = dir.path().join("artifacts");
2320        let mut seat = SeatState::new("impl-A", "a", 7);
2321        let s = command_helper("chatty-sleep");
2322        let out = invoke(
2323            &s,
2324            &mut seat,
2325            &Invocation {
2326                cwd: dir.path(),
2327                prompt: "unused",
2328                // Wide enough to cover process-spawn latency inside a loaded
2329                // parallel test run, not merely the helper's first write. At
2330                // two seconds this passed alone and failed in the full suite,
2331                // which is a dice roll rather than a test.
2332                timeout: Duration::from_secs(10),
2333                allow_write: true,
2334                sessions: true,
2335                artifacts: &artifacts,
2336                stem: "chatty",
2337                run: "test-run",
2338                node: "test",
2339                cache_dir: None,
2340                attachments: &[],
2341            },
2342        )
2343        .await
2344        .unwrap();
2345
2346        assert!(out.timed_out, "{out:?}");
2347        assert!(!out.usable(), "a cut-off answer is still not an answer");
2348        let recorded = std::fs::read_to_string(artifacts.join("chatty.out")).unwrap();
2349        assert!(
2350            recorded.contains("i-said-something"),
2351            "the artifact must keep what arrived before the kill, got {recorded:?}"
2352        );
2353        assert!(
2354            out.text.contains("i-said-something"),
2355            "and the graph must be able to see it too, got {:?}",
2356            out.text
2357        );
2358    }
2359
2360    #[test]
2361    fn missing_programs_reports_command_binaries() {
2362        let mut s = spec(AgentKind::Command, None);
2363        s.command = vec!["definitely-not-a-real-binary-xyz".to_owned()];
2364        assert_eq!(
2365            missing_programs(&[s]),
2366            ["definitely-not-a-real-binary-xyz".to_owned()]
2367        );
2368    }
2369
2370    fn pick_spec(id: &str, kind: AgentKind) -> AgentSpec {
2371        AgentSpec {
2372            id: id.to_owned(),
2373            kind,
2374            model: None,
2375            command: Vec::new(),
2376            extra_args: Vec::new(),
2377            env: BTreeMap::new(),
2378            prompt_delivery: None,
2379        }
2380    }
2381
2382    /// Availability stub: an agent is runnable unless its id was listed as
2383    /// missing. Keeps the selection tests off `PATH` entirely.
2384    fn without<'a>(missing: &'a [&'a str]) -> impl Fn(&AgentSpec) -> bool + 'a {
2385        move |a: &AgentSpec| !missing.contains(&a.id.as_str())
2386    }
2387
2388    #[test]
2389    fn pick_prefers_the_claude_seat_even_when_it_is_not_first_in_the_roster() {
2390        let agents = [
2391            pick_spec("oc", AgentKind::Opencode),
2392            pick_spec("opus", AgentKind::Claude),
2393            pick_spec("agy", AgentKind::Antigravity),
2394        ];
2395        let got = pick(&agents, None, &without(&[])).expect("a pick");
2396        assert_eq!(got.id, "opus");
2397    }
2398
2399    #[test]
2400    fn pick_falls_back_to_the_first_installed_agent_in_roster_order() {
2401        let agents = [
2402            pick_spec("opus", AgentKind::Claude),
2403            pick_spec("oc", AgentKind::Opencode),
2404            pick_spec("agy", AgentKind::Antigravity),
2405        ];
2406        let got = pick(&agents, None, &without(&["opus", "oc"])).expect("a pick");
2407        assert_eq!(got.id, "agy");
2408    }
2409
2410    #[test]
2411    fn pick_on_an_empty_roster_says_what_to_install() {
2412        let msg = pick(&[], None, &without(&[]))
2413            .expect_err("nobody to ask")
2414            .to_string();
2415        assert!(msg.contains("roster is empty"), "{msg}");
2416        assert!(msg.contains("claude"), "{msg}");
2417        assert!(msg.contains("magi.toml"), "{msg}");
2418    }
2419
2420    #[test]
2421    fn pick_on_a_roster_with_nothing_installed_names_the_programs_that_are_missing() {
2422        let agents = [
2423            pick_spec("opus", AgentKind::Claude),
2424            pick_spec("oc", AgentKind::Opencode),
2425        ];
2426        let err = pick(&agents, None, &without(&["opus", "oc"])).expect_err("nothing runnable");
2427        let msg = format!("{err:#}");
2428        assert!(msg.contains("claude"), "{msg}");
2429        assert!(msg.contains("opencode"), "{msg}");
2430    }
2431
2432    #[test]
2433    fn an_explicitly_named_agent_wins_over_the_claude_preference() {
2434        let agents = [
2435            pick_spec("opus", AgentKind::Claude),
2436            pick_spec("oc", AgentKind::Opencode),
2437        ];
2438        let got = pick(&agents, Some("oc"), &without(&[])).expect("a pick");
2439        assert_eq!(got.id, "oc");
2440    }
2441
2442    #[test]
2443    fn an_unknown_agent_id_lists_the_ids_that_do_exist() {
2444        let agents = [
2445            pick_spec("opus", AgentKind::Claude),
2446            pick_spec("oc", AgentKind::Opencode),
2447        ];
2448        let msg = pick(&agents, Some("gemini"), &without(&[]))
2449            .expect_err("no such agent")
2450            .to_string();
2451        assert!(msg.contains("gemini"), "{msg}");
2452        assert!(msg.contains("opus, oc"), "{msg}");
2453    }
2454
2455    #[test]
2456    fn an_explicitly_named_agent_that_is_not_installed_is_an_error_not_a_fallback() {
2457        let agents = [
2458            pick_spec("opus", AgentKind::Claude),
2459            pick_spec("oc", AgentKind::Opencode),
2460        ];
2461        let msg = pick(&agents, Some("oc"), &without(&["oc"]))
2462            .expect_err("must not silently substitute another model")
2463            .to_string();
2464        assert!(msg.contains("opencode"), "{msg}");
2465        assert!(msg.contains("--agent"), "{msg}");
2466    }
2467}