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