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