devflow_core/agents/
pi.rs

1//! Pi coding-agent harness adapter.
2//!
3//! Launches `pi -p "<prompt>"` in non-interactive print mode. The prompt is
4//! POSITIONAL — `-p` is a boolean flag (not the prompt carrier, and not stdin
5//! transport like Claude's stream-json). `--no-approve` is always passed:
6//! `--approve` trusts project-local extensions/skills/settings that execute
7//! UNSANDBOXED (Pi ships no sandbox), and a fresh per-phase worktree establishes
8//! no trust decision — that is a security boundary, not a convenience.
9//!
10//! No `--model`/`--provider` wiring here: model/provider selection is the
11//! `AgentDriver` contract's job (Phase 37), and `AgentAdapter` has no config
12//! surface to source it from. Pi uses its own defaults (provider `google`).
13//!
14//! Note: Pi has NO `--` end-of-options convention — passing `--` is rejected as
15//! an unknown option, so the prompt is passed raw. DevFlow's own stage prompts
16//! never begin with `-`, so the leading-dash hazard (a markdown `- [ ]` list) is
17//! a Phase 37 concern, not something a `--` can guard here.
18
19use super::{AgentAdapter, AgentDriver};
20use crate::phase_id::PhaseId;
21use std::path::PathBuf;
22
23/// The modular driver for Pi (37-03): print-mode `-p` launch, `pi auth check`
24/// health, and the de-Claude-ified workflow-reference prompt. NO JSON unwrapper
25/// or monitor/`CloseRule` integration here — that is 37.1/38 (CONTEXT D-04).
26pub struct PiDriver;
27
28impl AgentDriver for PiDriver {
29    fn name(&self) -> &'static str {
30        "Pi"
31    }
32
33    fn render_prompt(&self, intent: &crate::prompt::StageIntent) -> String {
34        crate::prompt::render_workflow_style(intent, &self.workflow_root())
35    }
36
37    /// Pi installs its GSD workflows under `~/.pi/agent/gsd-core/workflows/`,
38    /// NOT the Codex install the default points at (code-review finding #5).
39    fn workflow_root(&self) -> String {
40        "$HOME/.pi/agent/gsd-core/workflows".to_string()
41    }
42
43    fn build_command(
44        &self,
45        _phase: PhaseId,
46        prompt: &str,
47        _extra_writable_roots: &[PathBuf],
48    ) -> (&'static str, Vec<String>) {
49        (
50            "pi",
51            vec!["-p".into(), "--no-approve".into(), prompt.to_string()],
52        )
53    }
54
55    fn health(&self, _state: &crate::state::State) -> Result<(), String> {
56        // Credential readiness via `pi auth check` — Pi's own verb — rather
57        // than env-var sniffing (see `classify_auth_check`).
58        // `--no-refresh` prevents a stalled OAuth token refresh from hanging
59        // preflight (code-review finding #8: `pi auth check` refreshes expired
60        // credentials by default, and `.output()` has no timeout).
61        let output = std::process::Command::new("pi")
62            .args([
63                "auth",
64                "check",
65                "--json",
66                "--provider",
67                "google",
68                "--no-refresh",
69            ])
70            .output()
71            .map_err(|e| format!("could not run `pi auth check`: {e}"))?;
72        classify_auth_check(
73            &String::from_utf8_lossy(&output.stdout),
74            output.status.success(),
75        )
76    }
77}
78
79/// Legacy `AgentAdapter` face for Pi (D-11 removal point). Delegates to
80/// [`PiDriver`].
81pub struct PiAgent;
82
83impl AgentAdapter for PiAgent {
84    fn name(&self) -> &'static str {
85        PiDriver.name()
86    }
87
88    fn exec_command(
89        &self,
90        phase: PhaseId,
91        prompt: &str,
92        extra_writable_roots: &[PathBuf],
93    ) -> (&'static str, Vec<String>) {
94        PiDriver.build_command(phase, prompt, extra_writable_roots)
95    }
96
97    fn completion_signal_detected(&self, _output: &str) -> bool {
98        // `pi -p` exits cleanly when done; the monitor detects exit via kill -0.
99        false
100    }
101
102    fn preflight(&self, state: &crate::state::State) -> Result<(), String> {
103        PiDriver.health(state)
104    }
105
106    fn render_prompt(&self, intent: &crate::prompt::StageIntent) -> String {
107        PiDriver.render_prompt(intent)
108    }
109}
110
111/// Map `pi auth check --json` output to a readiness verdict. Split out so the
112/// classification is unit-testable without spawning a process. Parses the JSON
113/// rather than substring-matching, so whitespace formatting can't defeat it.
114fn classify_auth_check(stdout: &str, success: bool) -> Result<(), String> {
115    // A successful exit code alone is not enough: a credentialless check still
116    // prints `{"status":"not_ready",...}`. Require the `ready` status AND exit 0.
117    let ready = success
118        && serde_json::from_str::<serde_json::Value>(stdout)
119            .ok()
120            .and_then(|v| v.get("status").and_then(|s| s.as_str()).map(str::to_owned))
121            .is_some_and(|s| s == "ready");
122    if ready {
123        Ok(())
124    } else {
125        Err("no provider credential resolves — run `pi auth check` for details".to_string())
126    }
127}
128
129#[cfg(test)]
130mod tests {
131    use super::*;
132    use crate::mode::Mode;
133    use crate::state::{AgentKind, State};
134    use std::sync::Mutex;
135
136    /// Serializes tests that mutate the process-global `PATH` (`set_var` is
137    /// process-wide; `cargo test` runs tests in parallel).
138    static ENV_MUTEX: Mutex<()> = Mutex::new(());
139
140    #[test]
141    fn exec_command_shape() {
142        let (program, args) = PiAgent.exec_command(PhaseId::new(1), "do the thing", &[]);
143        assert_eq!(program, "pi");
144        assert_eq!(args, vec!["-p", "--no-approve", "do the thing"]);
145    }
146
147    #[test]
148    fn classify_auth_check_rejects_not_ready() {
149        assert!(classify_auth_check(
150            r#"{"status":"not_ready","provider":"google","reason":"credentials_not_configured"}"#,
151            false,
152        )
153        .is_err());
154    }
155
156    #[test]
157    fn classify_auth_check_accepts_ready() {
158        assert!(
159            classify_auth_check(
160                r#"{"status":"ready","provider":"google","authType":"api_key"}"#,
161                true,
162            )
163            .is_ok()
164        );
165    }
166
167    #[test]
168    fn classify_auth_check_tolerates_formatted_json() {
169        // Pretty-printed / whitespace-padded JSON must not defeat the parse.
170        assert!(classify_auth_check("{\n  \"status\": \"ready\"\n}", true).is_ok());
171    }
172
173    #[test]
174    fn classify_auth_check_rejects_ready_text_with_failed_exit() {
175        // A failed exit must not be read as ready even if the body says "ready".
176        assert!(classify_auth_check(r#"{"status":"ready"}"#, false).is_err());
177    }
178
179    /// A `State` value for `preflight`, which ignores it (`_state`) —
180    /// constructed only to satisfy the trait signature.
181    fn test_state() -> State {
182        State::new(
183            PhaseId::new(36),
184            AgentKind::Pi,
185            Mode::Auto,
186            std::path::PathBuf::from("/tmp"),
187        )
188    }
189
190    /// Writes an executable `pi` stub into a fresh tempdir. The stub records its
191    /// arguments (`"$@"`, one per line) to `args.txt` in the same dir, prints
192    /// `body` to stdout, and exits with `exit_code`. The returned tempdir is the
193    /// only entry the test puts on `PATH`, so the operator's live `pi` is never
194    /// consulted.
195    fn stub_pi_on_path(body: &str, exit_code: i32) -> tempfile::TempDir {
196        let dir = tempfile::tempdir().expect("create stub dir");
197        let stub = dir.path().join("pi");
198        let script = format!(
199            "#!/bin/sh\nprintf '%s\\n' \"$@\" > '{args}'\necho '{body}'\nexit {exit_code}\n",
200            args = dir.path().join("args.txt").display(),
201            body = body,
202            exit_code = exit_code,
203        );
204        std::fs::write(&stub, script).expect("write pi stub");
205        #[cfg(unix)]
206        {
207            use std::os::unix::fs::PermissionsExt;
208            let mut perms = std::fs::metadata(&stub).expect("stat stub").permissions();
209            perms.set_mode(0o755);
210            std::fs::set_permissions(&stub, perms).expect("chmod +x stub");
211        }
212        dir
213    }
214
215    /// RAII guard that replaces `PATH` with `path` and restores the previous
216    /// value on `Drop` — including the panic path, so a failing test never
217    /// hands the next test a mutated `PATH`.
218    struct PathGuard {
219        original: Option<std::ffi::OsString>,
220    }
221
222    impl PathGuard {
223        fn set(path: &std::path::Path) -> Self {
224            let original = std::env::var_os("PATH");
225            // SAFETY: held under ENV_MUTEX; no other thread reads/writes PATH.
226            unsafe { std::env::set_var("PATH", path) };
227            Self { original }
228        }
229    }
230
231    impl Drop for PathGuard {
232        fn drop(&mut self) {
233            match &self.original {
234                Some(prev) => unsafe { std::env::set_var("PATH", prev) },
235                None => unsafe { std::env::remove_var("PATH") },
236            }
237        }
238    }
239
240    /// The shell-out must actually spawn `pi auth check --json --provider
241    /// google` — not just classify a pre-parsed string. The stub records its
242    /// argv, so this proves the wiring end to end.
243    #[test]
244    fn preflight_invokes_pi_auth_check_and_accepts_ready() {
245        let _guard = ENV_MUTEX.lock().unwrap();
246        let stub_dir = stub_pi_on_path(r#"{"status":"ready"}"#, 0);
247        let _path = PathGuard::set(stub_dir.path());
248
249        PiAgent
250            .preflight(&test_state())
251            .expect("a `ready` stub should pass preflight");
252
253        let argv = std::fs::read_to_string(stub_dir.path().join("args.txt")).unwrap();
254        assert_eq!(
255            argv,
256            "auth\ncheck\n--json\n--provider\ngoogle\n--no-refresh\n"
257        );
258    }
259
260    /// The negative control AC #1 requires: a `pi` binary that reports
261    /// `not_ready` must yield the credentialless `Err`, proving the predicate
262    /// tests credential readiness, not env-var presence.
263    #[test]
264    fn preflight_reports_credentialless_when_auth_check_says_not_ready() {
265        let _guard = ENV_MUTEX.lock().unwrap();
266        let stub_dir = stub_pi_on_path(
267            r#"{"status":"not_ready","reason":"credentials_not_configured"}"#,
268            0,
269        );
270        let _path = PathGuard::set(stub_dir.path());
271
272        let err = PiAgent
273            .preflight(&test_state())
274            .expect_err("a `not_ready` stub should fail preflight");
275        assert!(
276            err.contains("no provider credential resolves"),
277            "unexpected error: {err}"
278        );
279    }
280
281    /// The exit code must be honored through the shell-out path, not just the
282    /// pure classifier: a `ready` body with a failed exit is still a failure.
283    #[test]
284    fn preflight_rejects_ready_body_with_failed_exit() {
285        let _guard = ENV_MUTEX.lock().unwrap();
286        let stub_dir = stub_pi_on_path(r#"{"status":"ready"}"#, 1);
287        let _path = PathGuard::set(stub_dir.path());
288
289        assert!(
290            PiAgent.preflight(&test_state()).is_err(),
291            "a failed exit must not be read as ready even when the body says ready"
292        );
293    }
294}
devflow_core/agents/pi.rs

devflow_core/agents/
pi.rs
