differential_engine/llmio.rs
1//! The model adapter: an agent CLI on the path, prompt on stdin, completion on
2//! stdout (ADR 0016; one argv per agent, ADR 0033).
3//!
4//! `llm` is the port; this is its one implementation, on the pattern of
5//! `forge`/`forgeio`. Nothing else in the engine may reach into subprocess
6//! machinery: grouping and the pipeline consume `LlmBackend` from `llm`, the
7//! application layer builds a `CommandBackend` here and hands it over, and the
8//! layering test names this file as an adapter so that the port's file is
9//! checked as domain.
10//!
11//! There are five agents, one constructor each, and the trait did not move to
12//! make room for them (ADR 0033). What differs between them is the argv, and
13//! the part of the argv that matters is how each one is stopped from writing:
14//!
15//! - **An allowlist** — `claude_cli`, `copilot_cli`. The agent may run the
16//! named tools and nothing else, and the fetch command is one of them.
17//! - **An OS sandbox** — `codex_cli`, `droid_cli`. The agent may run anything
18//! and the kernel refuses the writes, so there is no allowlist to derive and
19//! `fetch` does not appear in the argv.
20//! - **Nothing** — `pi_cli`. Pi ships no sandbox and no per-command allowlist,
21//! and the shell tool it needs to fetch is the one that also lets it write.
22//! That is a decision with a reason, recorded in ADR 0033 and in the
23//! constructor, and `config::Agent::read_only` is how a caller
24//! tells a user about it.
25//!
26//! Adding a sixth means a constructor here, a variant in `config::Agent`, and
27//! an arm in the application layer's `backend_from`. The compiler asks for the
28//! last two; this comment is the only thing that asks for the boundary.
29
30use std::path::{Path, PathBuf};
31use std::sync::Arc;
32use std::sync::atomic::AtomicBool;
33use std::time::Duration;
34
35use crate::config::DEFAULT_TIMEOUT_SECS;
36use crate::llm::{LlmBackend, LlmError};
37use crate::subprocess;
38
39/// A subprocess backend: prompt on stdin, completion on stdout.
40pub struct CommandBackend {
41 argv: Vec<String>,
42 timeout: Duration,
43 /// What a reviewer is shown: see [`LlmBackend::name`].
44 name: String,
45 /// The argv as it will actually run, for error text only. A spawn failure
46 /// is debugged with the whole command, and neither `name` nor `identity`
47 /// is that: one is a product name, the other stands a placeholder where
48 /// the executable's path was.
49 command: String,
50 /// See [`LlmBackend::identity`].
51 identity: String,
52 /// Where the child runs.
53 ///
54 /// The prompt hands the model `git diff <base> <head> -- <path>` with paths
55 /// as the document records them, which is relative to the repository root.
56 /// Git resolves a bare pathspec against the **current directory**, not the
57 /// root, so a child inheriting `dfr`'s cwd matches nothing whenever `dfr`
58 /// was run from a subdirectory — and matching nothing is an empty diff and
59 /// exit 0, not an error. The model would then rate a class having seen no
60 /// diff at all, and nothing anywhere would say so.
61 ///
62 /// `None` means inherit, which is right for a child that reads no repository
63 /// (the tests here, and any future backend that takes its whole input on
64 /// stdin).
65 working_dir: Option<PathBuf>,
66 /// Set from another thread to kill an in-flight child (a reviewer
67 /// abandoning the wait). Without this the subprocess would outlive the
68 /// process that asked for it, up to the whole timeout.
69 cancel: Option<Arc<AtomicBool>>,
70}
71
72impl CommandBackend {
73 /// A backend named by its own command line.
74 ///
75 /// The named constructors below are the production path; this is for a
76 /// backend with nothing better to call itself, which in practice means a
77 /// test double.
78 pub fn new(argv: Vec<String>, timeout: Duration) -> Self {
79 assert!(!argv.is_empty(), "CommandBackend needs a program to run");
80 let command = argv.join(" ");
81 CommandBackend {
82 argv,
83 timeout,
84 name: command.clone(),
85 identity: command.clone(),
86 command,
87 working_dir: None,
88 cancel: None,
89 }
90 }
91
92 /// Run the child in `dir`.
93 ///
94 /// The repository root, for any backend whose prompt names repo-relative
95 /// paths — which the default one does. See the field for what goes wrong
96 /// without it, and why it goes wrong silently.
97 pub fn with_working_dir(mut self, dir: &Path) -> Self {
98 self.working_dir = Some(dir.to_path_buf());
99 self
100 }
101
102 /// Kill the child as soon as `flag` is set.
103 pub fn with_cancel(mut self, flag: Arc<AtomicBool>) -> Self {
104 self.cancel = Some(flag);
105 self
106 }
107
108 pub fn with_timeout(mut self, timeout: Duration) -> Self {
109 self.timeout = timeout;
110 self
111 }
112
113 /// The default: headless, text output, and read-only tools (ADR 0022).
114 ///
115 /// ADR 0010 denied tools outright, because the evaluated grouping tool kept
116 /// exiting 1 on `stop_reason: "tool_use"`. Denying them cured it by sending
117 /// no tool definitions at all, so the model could not ask. An allowlist is
118 /// the other cure: it can ask, and the answer is yes.
119 ///
120 /// `fetch` is the executable the prompt tells the model to run — normally
121 /// this process. The allowlist is derived from it, so the two cannot
122 /// disagree about what the model is allowed to invoke.
123 ///
124 /// Nothing here can write. The fetch command reads the document the engine
125 /// just wrote; the rest read the repository. `git log` and `git show` are
126 /// what reach the *reason* a change was made, which no prompt can carry.
127 ///
128 /// **`git diff` is advertised; the rest are not.** The prompt names the
129 /// fetch command and `git diff`, and nothing else.
130 ///
131 /// That is a change of rule, and it is worth saying why. `git diff` is
132 /// advertised because it is now the only way to see what a hunk says: the
133 /// fetch command's `diff` query is gone, having duplicated `class` except
134 /// for the text. A tool the model must use and is not told about is a tool
135 /// it will not use.
136 ///
137 /// It costs an invitation to read the whole repository, and the prompt is
138 /// what pays for that: it says to read what decides a label and then stop.
139 ///
140 /// It no longer costs a route around the generated content this stage folds
141 /// away, though it did when it was written. `generated` is part of the
142 /// shape-class key now (ADR 0004), so no class the model is given contains
143 /// a generated file and there is nothing folded left for it to ask
144 /// `git diff` about by accident. The prompt still says not to go looking.
145 ///
146 /// `Read`, `Grep`, `Glob`, `git log` and `git show` stay unadvertised for
147 /// the original reason: a model that needs the code around a hunk can go
148 /// and read it, but it is not sent looking. If you add a tool here, do not
149 /// add a line about it to the prompt.
150 ///
151 /// The allowlist is this function's business, not the user's, and there is
152 /// no config that replaces it. `[grouping].agent` picks between agents by
153 /// name; it used to take a free argv, which handed a stranger's process the
154 /// prompt and none of the allowlist, fetch command or read path the prompt
155 /// is written for.
156 ///
157 /// `fetch` is where a binary lives, so it is the one part of this argv that
158 /// says nothing about what the model will do. The cache identity stands a
159 /// placeholder in its place: change the allowlist and every cached grouping
160 /// is rightly invalidated, move the binary and none of them are.
161 ///
162 /// **`--permission-mode default` is what makes the allowlist mean anything,
163 /// and it was missing for two releases** (ADR 0033). `--allowed-tools` ADDS
164 /// permissions; it does not cap them. A user whose own settings set
165 /// `defaultMode` to `auto`, `acceptEdits` or `bypassPermissions` was
166 /// handing this call an agent that could write, commit and push, and
167 /// nothing anywhere said so. `default` means ask, and a headless call has
168 /// nobody to ask, so the answer is no.
169 ///
170 /// It was found by `dfr agents --probe`, on the first run, against the
171 /// agent that had shipped as the only option. That is the whole argument
172 /// for the probe existing.
173 pub fn claude_cli(fetch: &str) -> Self {
174 let mut b = Self::new(
175 Self::claude_argv(fetch),
176 Duration::from_secs(DEFAULT_TIMEOUT_SECS),
177 );
178 b.name = "Claude Code".to_string();
179 b.identity = Self::claude_argv("<fetch>").join(" ");
180 b
181 }
182
183 fn claude_argv(fetch: &str) -> Vec<String> {
184 vec![
185 "claude".to_string(),
186 "-p".to_string(),
187 "--output-format".to_string(),
188 "text".to_string(),
189 "--permission-mode".to_string(),
190 "default".to_string(),
191 "--allowed-tools".to_string(),
192 format!(
193 "Bash({fetch} agent:*),Bash(git diff:*),Read,Grep,Glob,\
194 Bash(git log:*),Bash(git show:*)"
195 ),
196 ]
197 }
198
199 /// Headless `codex exec`, read-only by OS sandbox (ADR 0033).
200 ///
201 /// Codex has no tool allowlist and needs none: `--sandbox read-only` is
202 /// enforced by the kernel — Seatbelt on macOS, bubblewrap on Linux — so the
203 /// model may run any command it likes and the writes are refused beneath
204 /// it. That is a different boundary from Claude Code's and an equally real
205 /// one, which is why `fetch` does not appear in this argv at all. The
206 /// prompt still names the fetch command; nothing has to permit it.
207 ///
208 /// `-c approval_policy="never"` is the headless half. Without it a command
209 /// the sandbox refuses escalates to a human who is not there, and the call
210 /// sits until the deadline kills it. With it the refusal returns to the
211 /// model as a tool failure, which is what we want it to see.
212 ///
213 /// It is a config override rather than the `--ask-for-approval` flag the
214 /// docs name, because **that flag does not exist on `codex exec`** — it is
215 /// on the interactive top-level command only, and `codex exec` rejects it
216 /// outright. Checked against 0.154.0, where passing it is
217 /// `error: unexpected argument`, which is a failure to spawn rather than a
218 /// bad grouping.
219 ///
220 /// `codex exec` already defaults to never asking, so this says out loud
221 /// what is currently true anyway. That is the point: a boundary resting on
222 /// another program's default is one release away from being no boundary,
223 /// and `--ignore-user-config` means nothing on disk can move it back.
224 ///
225 /// `--color never` keeps stdout clean. The response parser takes the text
226 /// between the first `{` and the last `}`, and an escape sequence inside
227 /// that span is a parse error with a sample nobody can read.
228 ///
229 /// The trailing `-` makes stdin the whole prompt. Codex will otherwise
230 /// treat stdin as context for an argv instruction, and there is no argv
231 /// instruction here.
232 ///
233 /// Never pass `--full-auto`, `--yolo` or
234 /// `--dangerously-bypass-approvals-and-sandbox`: each removes the boundary.
235 ///
236 /// `--ignore-user-config` and `--ignore-rules` are the same lesson Claude
237 /// Code taught (ADR 0033): the sandbox a flag asks for is not the sandbox
238 /// that runs if the user's own `config.toml` or execpolicy rules say
239 /// otherwise. An argv that can be widened by a file this crate never reads
240 /// is not a boundary, it is a request.
241 pub fn codex_cli() -> Self {
242 let mut b = Self::new(
243 Self::codex_argv(),
244 Duration::from_secs(DEFAULT_TIMEOUT_SECS),
245 );
246 b.name = "Codex".to_string();
247 b.identity = Self::codex_argv().join(" ");
248 b
249 }
250
251 fn codex_argv() -> Vec<String> {
252 vec![
253 "codex".to_string(),
254 "exec".to_string(),
255 "--ignore-user-config".to_string(),
256 "--ignore-rules".to_string(),
257 "-c".to_string(),
258 "approval_policy=\"never\"".to_string(),
259 "--sandbox".to_string(),
260 "read-only".to_string(),
261 "--color".to_string(),
262 "never".to_string(),
263 "-".to_string(),
264 ]
265 }
266
267 /// Headless `droid exec`, read-only by default (ADR 0033).
268 ///
269 /// Droid is the one agent whose boundary is what this function does NOT
270 /// pass. Its documented default is read-only file inspection plus git read
271 /// operations, with file edits, package installs and git writes blocked,
272 /// and a blocked action fails rather than asking — so a bare `droid exec`
273 /// neither writes nor stalls.
274 ///
275 /// Never pass `--auto` at any level, and never
276 /// `--skip-permissions-unsafe`. Each is the whole boundary, given away.
277 ///
278 /// `-o text` prints the final message only. `-` makes stdin the prompt.
279 pub fn droid_cli() -> Self {
280 let mut b = Self::new(
281 Self::droid_argv(),
282 Duration::from_secs(DEFAULT_TIMEOUT_SECS),
283 );
284 b.name = "Droid".to_string();
285 b.identity = Self::droid_argv().join(" ");
286 b
287 }
288
289 fn droid_argv() -> Vec<String> {
290 vec![
291 "droid".to_string(),
292 "exec".to_string(),
293 "-o".to_string(),
294 "text".to_string(),
295 "-".to_string(),
296 ]
297 }
298
299 /// Headless `copilot`, read-only by allowlist and an explicit deny
300 /// (ADR 0033).
301 ///
302 /// The closest of the five to Claude Code: an allowlist derived from
303 /// `fetch`, so the prompt can never name a command the model may not run.
304 ///
305 /// **There is deliberately no `-p`.** Copilot reads the prompt from stdin,
306 /// and its own documentation says piped input is ignored when `-p` is
307 /// given. Passing both would send an empty prompt and waste a call.
308 ///
309 /// `-s` suppresses the session decoration around the reply, for the same
310 /// reason Codex gets `--color never`. `--no-ask-user` stops the agent
311 /// pausing for a human who is not there.
312 ///
313 /// `--deny-tool write` is belt and braces: `write` is already absent from
314 /// the allowlist, and a deny takes precedence over any allow, so the two
315 /// cannot be talked out of agreeing.
316 ///
317 /// Never pass `--allow-all-tools` or `--allow-all-paths`.
318 pub fn copilot_cli(fetch: &str) -> Self {
319 let mut b = Self::new(
320 Self::copilot_argv(fetch),
321 Duration::from_secs(DEFAULT_TIMEOUT_SECS),
322 );
323 b.name = "GitHub Copilot".to_string();
324 b.identity = Self::copilot_argv("<fetch>").join(" ");
325 b
326 }
327
328 fn copilot_argv(fetch: &str) -> Vec<String> {
329 vec![
330 "copilot".to_string(),
331 "-s".to_string(),
332 "--no-ask-user".to_string(),
333 "--deny-tool".to_string(),
334 "write".to_string(),
335 "--allow-tool".to_string(),
336 format!("read,shell(git:*),shell({fetch}:*)"),
337 ]
338 }
339
340 /// Headless `pi`. **Read-only is NOT enforced here** (ADR 0033).
341 ///
342 /// Every other constructor in this file hands the model a boundary. This
343 /// one cannot, and the reason is Pi's design rather than an oversight in
344 /// this argv.
345 ///
346 /// Pi ships no sandbox, no per-command allowlist and no approval prompts.
347 /// Its `-t` flag toggles whole tools, and `bash` is one tool: the model
348 /// needs it to run the fetch command and `git diff`, and the same tool lets
349 /// it write a file, commit or push. Nothing but the prompt asks it not to.
350 ///
351 /// Dropping `bash` would restore the boundary and take the change with it.
352 /// The model would be back to grouping from class ids alone, which is the
353 /// truncated payload ADR 0022 was written to end — a worse grouping, every
354 /// time, in exchange for a risk the prompt never asks anyone to take.
355 ///
356 /// So the author chose this knowingly, and the duty that comes with it is
357 /// disclosure: `Agent::read_only` answers `NotEnforced` for Pi, and
358 /// every place that offers the name says so.
359 ///
360 /// The rest of the argv is hermetic sealing, and it is not decoration.
361 /// `-nc` drops `AGENTS.md` and `CLAUDE.md`, `-na` drops the repository's
362 /// own `.pi/` config, and `--no-extensions --no-skills` drop the user's.
363 /// Each is a file outside the cache key that could otherwise change a
364 /// grouping, which is the hole ADR 0022 names and cannot close.
365 /// `--no-session` stops Pi writing a session file for a call nobody
366 /// resumes.
367 pub fn pi_cli() -> Self {
368 let mut b = Self::new(Self::pi_argv(), Duration::from_secs(DEFAULT_TIMEOUT_SECS));
369 b.name = "Pi".to_string();
370 b.identity = Self::pi_argv().join(" ");
371 b
372 }
373
374 fn pi_argv() -> Vec<String> {
375 vec![
376 "pi".to_string(),
377 "-p".to_string(),
378 "--mode".to_string(),
379 "text".to_string(),
380 "--no-session".to_string(),
381 "-nc".to_string(),
382 "-na".to_string(),
383 "--no-extensions".to_string(),
384 "--no-skills".to_string(),
385 "-t".to_string(),
386 "read,grep,find,ls,bash".to_string(),
387 ]
388 }
389
390 /// The program this backend spawns, for a caller checking `PATH`.
391 ///
392 /// `dfr agents` says whether each agent is installed, and the answer has to
393 /// come from the argv that will actually run rather than from a second list
394 /// of executable names that could disagree with it.
395 pub fn program(&self) -> &str {
396 &self.argv[0]
397 }
398
399 /// The whole command line, for a caller showing what will run.
400 ///
401 /// Not [`LlmBackend::name`], which is a product name, and not
402 /// [`LlmBackend::identity`], which stands a placeholder where the binary
403 /// path is. This is the argv itself, and the two callers that want it are a
404 /// spawn failure and `dfr agents`.
405 pub fn command(&self) -> &str {
406 &self.command
407 }
408}
409
410impl LlmBackend for CommandBackend {
411 fn name(&self) -> &str {
412 &self.name
413 }
414
415 fn identity(&self) -> &str {
416 &self.identity
417 }
418
419 fn complete(&self, prompt: &str) -> Result<String, LlmError> {
420 let command = || self.command.clone();
421 let out = subprocess::run(&subprocess::Run {
422 argv: &self.argv,
423 stdin: Some(prompt.as_bytes()),
424 working_dir: self.working_dir.as_deref(),
425 timeout: self.timeout,
426 cancel: self.cancel.as_ref(),
427 })
428 .map_err(|f| match f {
429 subprocess::Failure::Spawn(source) => LlmError::Spawn {
430 command: command(),
431 source,
432 },
433 subprocess::Failure::Io(source) => LlmError::Io {
434 command: command(),
435 source,
436 },
437 subprocess::Failure::Timeout => LlmError::Timeout {
438 command: command(),
439 timeout: self.timeout,
440 },
441 subprocess::Failure::Cancelled => LlmError::Cancelled { command: command() },
442 })?;
443
444 if !out.status.success() {
445 return Err(LlmError::Failed {
446 command: command(),
447 code: out.status.code(),
448 stderr: subprocess::stderr_excerpt(&out.stderr, 600),
449 });
450 }
451 let text = String::from_utf8_lossy(&out.stdout).into_owned();
452 if text.trim().is_empty() {
453 return Err(LlmError::Empty { command: command() });
454 }
455 Ok(text)
456 }
457}
458
459#[cfg(test)]
460mod tests {
461 use std::sync::atomic::Ordering;
462
463 use super::*;
464
465 #[test]
466 fn cat_echoes_the_prompt() {
467 let b = CommandBackend::new(vec!["cat".into()], Duration::from_secs(10));
468 let out = b.complete("hello prompt\n").unwrap();
469 assert_eq!(out, "hello prompt\n");
470 }
471
472 #[test]
473 fn nonzero_exit_is_failed() {
474 let b = CommandBackend::new(vec!["false".into()], Duration::from_secs(10));
475 match b.complete("x") {
476 Err(LlmError::Failed { code, .. }) => assert_eq!(code, Some(1)),
477 other => panic!("expected Failed, got {other:?}"),
478 }
479 }
480
481 #[test]
482 fn empty_output_is_an_error() {
483 let b = CommandBackend::new(vec!["true".into()], Duration::from_secs(10));
484 match b.complete("x") {
485 Err(LlmError::Empty { .. }) => {}
486 other => panic!("expected Empty, got {other:?}"),
487 }
488 }
489
490 #[test]
491 fn cancel_kills_the_child() {
492 // A long sleep with a generous deadline: only the cancel flag can end
493 // this, and it must do so promptly rather than leaving the child to
494 // outlive the caller.
495 let flag = Arc::new(AtomicBool::new(false));
496 let backend =
497 CommandBackend::new(vec!["sleep".into(), "600".into()], Duration::from_secs(600))
498 .with_cancel(Arc::clone(&flag));
499 let started = std::time::Instant::now();
500 std::thread::spawn(move || {
501 std::thread::sleep(Duration::from_millis(100));
502 flag.store(true, Ordering::Relaxed);
503 });
504 let err = backend.complete("hello").unwrap_err();
505 assert!(
506 matches!(err, LlmError::Cancelled { .. }),
507 "expected cancellation, got {err:?}"
508 );
509 assert!(
510 started.elapsed() < Duration::from_secs(5),
511 "child was not killed promptly"
512 );
513 }
514
515 #[test]
516 fn deadline_kills_the_child() {
517 let b = CommandBackend::new(
518 vec!["sleep".into(), "30".into()],
519 Duration::from_millis(200),
520 );
521 let started = std::time::Instant::now();
522 match b.complete("x") {
523 Err(LlmError::Timeout { .. }) => {}
524 other => panic!("expected Timeout, got {other:?}"),
525 }
526 assert!(
527 started.elapsed() < Duration::from_secs(5),
528 "child was not killed promptly"
529 );
530 }
531
532 #[test]
533 fn large_prompt_does_not_deadlock() {
534 // A prompt bigger than the pipe buffer, against a child that echoes
535 // while reading: the writer thread prevents the classic deadlock.
536 let b = CommandBackend::new(vec!["cat".into()], Duration::from_secs(30));
537 let big = "line of prompt text\n".repeat(60_000); // ~1.2 MB
538 let out = b.complete(&big).unwrap();
539 assert_eq!(out.len(), big.len());
540 }
541
542 #[test]
543 fn where_the_binary_lives_is_not_part_of_the_cache_identity() {
544 // The grouping cache key hashes `identity`. If it hashed the argv the
545 // absolute path would be in the key, and a debug build, a release build
546 // and a second checkout of the same commit would each re-run a
547 // four-hundred-second call over an identical class partition.
548 let a = CommandBackend::claude_cli("/Users/someone/.cargo/bin/dfr");
549 let b = CommandBackend::claude_cli("/srv/ci/target/release/dfr");
550 assert_eq!(a.identity(), b.identity());
551 assert!(!a.identity().contains(".cargo"), "{}", a.identity());
552
553 // A backend with nothing better to call itself is its own identity, and
554 // two different agents must never share a cache entry.
555 let one = CommandBackend::new(vec!["agent-one".into()], Duration::from_secs(1));
556 let two = CommandBackend::new(vec!["agent-two".into()], Duration::from_secs(1));
557 assert_eq!(one.identity(), one.name());
558 assert_ne!(one.identity(), two.identity());
559 }
560
561 #[test]
562 fn the_child_runs_where_it_was_told_to() {
563 // The prompt hands the model repo-root-relative paths for `git diff`.
564 // Git resolves a bare pathspec against the CURRENT DIRECTORY, so a child
565 // inheriting this process's cwd matches nothing whenever `dfr` ran from
566 // a subdirectory — and matching nothing is an empty diff and exit 0, not
567 // an error. The model would rate a class having seen no diff, and
568 // nothing would say so. Hence a test on the cwd itself.
569 let dir = tempfile::TempDir::new().unwrap();
570 // The temp dir may be a symlink (/var -> /private/var on macOS), so
571 // compare what the child reports against the canonical form.
572 let want = dir.path().canonicalize().unwrap();
573 let b = CommandBackend::new(vec!["pwd".into()], Duration::from_secs(10))
574 .with_working_dir(dir.path());
575 let got = b.complete("x").unwrap();
576 assert_eq!(
577 std::path::Path::new(got.trim()).canonicalize().unwrap(),
578 want,
579 "the child must run in the directory it was given"
580 );
581
582 // Without it, the child inherits — which is right for a backend that
583 // reads no repository, and wrong for one whose prompt names paths.
584 let inherit = CommandBackend::new(vec!["pwd".into()], Duration::from_secs(10));
585 assert_ne!(
586 std::path::Path::new(inherit.complete("x").unwrap().trim())
587 .canonicalize()
588 .unwrap(),
589 want
590 );
591 }
592
593 #[test]
594 fn the_reviewer_sees_a_product_name_and_an_error_sees_the_command() {
595 // The splash prints `name` on one line. The argv is four times the
596 // width and answers a different question, so it lives where it is the
597 // answer: a spawn failure.
598 let b = CommandBackend::claude_cli("/opt/bin/dfr");
599 assert_eq!(b.name(), "Claude Code");
600
601 let missing = CommandBackend::new(
602 vec!["definitely-not-a-real-program".into()],
603 Duration::from_secs(1),
604 );
605 match missing.complete("x") {
606 Err(LlmError::Spawn { command, .. }) => {
607 assert_eq!(command, "definitely-not-a-real-program");
608 }
609 other => panic!("expected Spawn, got {other:?}"),
610 }
611 }
612
613 #[test]
614 fn changing_the_allowlist_does_change_the_cache_identity() {
615 // The other half of the rule: the allowlist shapes what the model can
616 // see, so it must stay in the key even though the path does not.
617 let b = CommandBackend::claude_cli("/opt/bin/dfr");
618 assert!(b.identity().contains("Read,Grep,Glob"), "{}", b.identity());
619 assert!(!b.identity().contains("/opt/bin"), "{}", b.identity());
620 }
621
622 #[test]
623 fn claude_cli_default_allows_reading_and_nothing_else() {
624 let b = CommandBackend::claude_cli("/opt/bin/dfr");
625 let argv = &b.command;
626 assert!(
627 argv.contains("Bash(/opt/bin/dfr agent:*)"),
628 "the allowlist names the same executable the prompt does"
629 );
630 assert!(
631 argv.contains("Bash(git diff:*)"),
632 "the prompt tells the model to run git diff, so it must be permitted"
633 );
634 // The whole list, exactly. The argv is built with a line continuation,
635 // and a stray space inside one would produce an allowlist that parses
636 // as something else. This is the security boundary, and a broken fetch
637 // costs minutes of a model working around it, so it fails here loudly
638 // rather than there silently.
639 assert!(
640 argv.ends_with(
641 "--allowed-tools Bash(/opt/bin/dfr agent:*),Bash(git diff:*),Read,Grep,Glob,Bash(git log:*),Bash(git show:*)"
642 ),
643 "{argv}"
644 );
645 // Without this the allowlist is advisory: `--allowed-tools` adds
646 // permissions and does not cap them, so a user whose settings set
647 // `defaultMode` to `auto` got an agent that could write. `dfr agents
648 // --probe` caught it; this line is what stops it coming back.
649 assert!(
650 argv.contains("--permission-mode default"),
651 "the allowlist only binds under the default permission mode: {argv}"
652 );
653 // The allowlist is the security boundary, so the test states what must
654 // stay OUT of it, not merely what is in it.
655 for forbidden in [
656 "Write",
657 "Edit",
658 "Bash(git commit",
659 "Bash(git push",
660 "WebFetch",
661 ] {
662 assert!(!argv.contains(forbidden), "{forbidden} must not be allowed");
663 }
664 }
665
666 /// Every backend this crate builds, for the tests that must hold across all
667 /// of them. A new agent belongs here, and two of the tests below fail until
668 /// it is.
669 fn every_backend() -> Vec<(&'static str, CommandBackend)> {
670 vec![
671 ("claude-code", CommandBackend::claude_cli("/opt/bin/dfr")),
672 ("codex", CommandBackend::codex_cli()),
673 ("droid", CommandBackend::droid_cli()),
674 ("copilot", CommandBackend::copilot_cli("/opt/bin/dfr")),
675 ("pi", CommandBackend::pi_cli()),
676 ]
677 }
678
679 #[test]
680 fn codex_runs_sandboxed_and_never_stops_to_ask() {
681 let b = CommandBackend::codex_cli();
682 assert_eq!(b.name(), "Codex");
683 assert_eq!(b.program(), "codex");
684 // The whole argv, exactly. Codex has no allowlist to get wrong, so the
685 // boundary IS these two flag pairs and nothing else says so.
686 assert_eq!(
687 b.command(),
688 "codex exec --ignore-user-config --ignore-rules -c approval_policy=\"never\" \
689 --sandbox read-only --color never -"
690 );
691 // A sandbox the user's own config can widen is not a sandbox. Same
692 // lesson as `--permission-mode default` on Claude Code (ADR 0033).
693 assert!(
694 b.command().contains("--ignore-user-config"),
695 "{}",
696 b.command()
697 );
698 // `-` is what makes stdin the whole prompt rather than context for an
699 // argv instruction that does not exist here. Without it Codex waits for
700 // an instruction and the call is wasted.
701 assert!(b.command().ends_with(" -"), "{}", b.command());
702 }
703
704 #[test]
705 fn droid_is_read_only_because_of_what_it_does_not_pass() {
706 let b = CommandBackend::droid_cli();
707 assert_eq!(b.name(), "Droid");
708 assert_eq!(b.program(), "droid");
709 assert_eq!(b.command(), "droid exec -o text -");
710 // Droid's default is read-only, so its boundary is an absence. A test
711 // on presence would pass while the boundary was being given away, which
712 // is why this one is written the other way round.
713 assert!(!b.command().contains("--auto"), "{}", b.command());
714 }
715
716 #[test]
717 fn copilot_allows_reading_and_the_fetch_command_and_nothing_else() {
718 let b = CommandBackend::copilot_cli("/opt/bin/dfr");
719 assert_eq!(b.name(), "GitHub Copilot");
720 assert_eq!(b.program(), "copilot");
721 assert!(
722 b.command().contains("shell(/opt/bin/dfr:*)"),
723 "the allowlist names the same executable the prompt does: {}",
724 b.command()
725 );
726 assert!(
727 b.command().contains("shell(git:*)"),
728 "the prompt tells the model to run git diff, so it must be permitted"
729 );
730 // The whole list, exactly, for the reason the Claude one is pinned: a
731 // stray character inside it produces an allowlist that parses as
732 // something else, and it fails here loudly rather than there silently.
733 assert!(
734 b.command().ends_with(
735 "--deny-tool write --allow-tool read,shell(git:*),shell(/opt/bin/dfr:*)"
736 ),
737 "{}",
738 b.command()
739 );
740 // Copilot ignores piped input when `-p` is given, and the prompt only
741 // ever arrives on stdin. A `-p` here would send an empty prompt.
742 assert!(
743 !b.command().contains(" -p"),
744 "the prompt comes from stdin: {}",
745 b.command()
746 );
747 assert!(b.command().contains("--no-ask-user"), "{}", b.command());
748 }
749
750 #[test]
751 fn pi_is_the_one_agent_that_can_write_and_says_so() {
752 // This test states an exception, not a requirement. Pi ships no sandbox
753 // and no per-command allowlist, so the shell tool it needs to run the
754 // fetch command is the same tool that lets it write (ADR 0033).
755 //
756 // `bash` being present is therefore the decision, and pinning it here is
757 // what stops a later reader "fixing" it and silently taking the fetch
758 // command away — which does not fail, it just groups worse.
759 let b = CommandBackend::pi_cli();
760 assert_eq!(b.name(), "Pi");
761 assert_eq!(b.program(), "pi");
762 assert!(
763 b.command().contains("-t read,grep,find,ls,bash"),
764 "pi needs bash to fetch; removing it removes the change, not the risk: {}",
765 b.command()
766 );
767 assert!(
768 !b.command().contains("edit") && !b.command().contains("write"),
769 "the write tools stay off even though bash makes that a courtesy: {}",
770 b.command()
771 );
772 // The hermetic flags are not decoration. Each one is a file outside the
773 // cache key that could otherwise change a grouping.
774 for flag in [
775 "-nc",
776 "-na",
777 "--no-extensions",
778 "--no-skills",
779 "--no-session",
780 ] {
781 assert!(
782 b.command().contains(flag),
783 "{flag} missing: {}",
784 b.command()
785 );
786 }
787 }
788
789 #[test]
790 fn no_agent_is_given_a_flag_that_removes_its_boundary() {
791 // One list, every agent. These are the flags each CLI offers for
792 // turning its own protection off, and none of them may ever appear in
793 // an argv this crate writes. A new agent is covered the moment it joins
794 // `every_backend`.
795 const FORBIDDEN: [&str; 8] = [
796 "--yolo",
797 "--full-auto",
798 "--dangerously-bypass-approvals-and-sandbox",
799 "--dangerously-allow-all",
800 "--skip-permissions-unsafe",
801 "--allow-all-tools",
802 "--allow-all-paths",
803 "--auto",
804 ];
805 for (key, b) in every_backend() {
806 for flag in FORBIDDEN {
807 assert!(
808 !b.command().contains(flag),
809 "{key} must never be given {flag}: {}",
810 b.command()
811 );
812 }
813 }
814 }
815
816 #[test]
817 fn no_agent_takes_its_prompt_or_its_working_directory_in_the_argv() {
818 // Two rules that hold across all five.
819 //
820 // The prompt goes on stdin, always: `complete` writes it there and
821 // passes no argument. An agent given an argv prompt would read an empty
822 // one.
823 //
824 // The working directory comes from `with_working_dir`, never from a
825 // flag. A path in the argv lands in the cache identity, and then a
826 // debug build, a release build and a second checkout each re-run a
827 // four-hundred-second call over an identical class partition.
828 for (key, b) in every_backend() {
829 for flag in ["--cwd", "--workspace", "--dir", "-C "] {
830 assert!(
831 !b.command().contains(flag),
832 "{key} must take its directory from with_working_dir, not {flag}"
833 );
834 }
835 assert!(
836 !b.identity().contains("/opt/bin"),
837 "{key} put a binary path in its cache identity: {}",
838 b.identity()
839 );
840 }
841 }
842
843 #[test]
844 fn no_two_agents_share_a_cache_identity() {
845 // Two agents sharing an identity share a cache entry, so one would
846 // serve the other's grouping under a name it never ran.
847 let all = every_backend();
848 for (i, (key_a, a)) in all.iter().enumerate() {
849 for (key_b, b) in all.iter().skip(i + 1) {
850 assert_ne!(
851 a.identity(),
852 b.identity(),
853 "{key_a} and {key_b} share a cache identity"
854 );
855 assert_ne!(a.name(), b.name(), "{key_a} and {key_b} share a name");
856 }
857 }
858 }
859}