diff --git a/crates/flowproof-cli/src/agent_flow.rs b/crates/flowproof-cli/src/agent_flow.rs index 77081b0..c1058f0 100644 --- a/crates/flowproof-cli/src/agent_flow.rs +++ b/crates/flowproof-cli/src/agent_flow.rs @@ -1011,6 +1011,124 @@ fn require_progress(run: &AgentRun, cassette: &Cassette, plan: &Plan) -> Result< Ok(()) } +/// `flowproof doctor`: does this agent's model traffic reach the proxy? +/// +/// The dominant adoption failure is a client that never honours the injected +/// base URL - it talks to the real provider, flowproof captures nothing, and +/// today that is discovered only after a spec has been written and a key +/// spent on `record`. This answers the same question in ten seconds, with no +/// spec, no assertions and NO KEY: a synthetic one-turn cassette answers +/// whatever arrives. +/// +/// It deliberately does NOT print a verdict like "your wiring is correct". +/// An agent with two clients can reach the proxy with one and the real +/// provider with the other, so a request arriving proves that A client found +/// the proxy - not that all of them did. Reporting the observation and +/// letting the reader judge is the honest shape, and the same reason the +/// zero-capture guard names causes as possibilities rather than facts. +pub fn cmd_doctor(command: &str, timeout_secs: u64, prompt: &str) -> Result { + use flowproof_trace::cassette::{Cassette, Message, Turn, TurnRequest, TurnResponse}; + + // One canned turn. Anything the agent sends is answered from this, so no + // upstream and no key are involved. A request that DIVERGES from it still + // arrived, which is the fact being measured. + let probe = Cassette { + turns: vec![Turn { + protocol: "openai".to_string(), + request: TurnRequest { + model: "flowproof-doctor".to_string(), + messages: Vec::new(), + tools: Vec::new(), + }, + response: TurnResponse { + message: Message { + role: "assistant".to_string(), + content: Some( + "flowproof doctor: this reply came from the proxy, not a model." + .to_string(), + ), + tool_calls: Vec::new(), + tool_call_id: None, + }, + stop_reason: None, + }, + }], + }; + + let proxy = AgentProxy::start(probe, Mocks::new(), 0) + .map_err(|e| format!("starting the probe proxy: {e}"))?; + let base = proxy.base_url(); + println!("proxy listening on {base}"); + println!("running: {command}"); + + let mut env = BTreeMap::new(); + env.insert(PROMPT_VAR.to_string(), prompt.to_string()); + let run = run_against(&proxy, command, &env, Duration::from_secs(timeout_secs)) + .map_err(|e| e.to_string())?; + let log = proxy.log(); + // A divergence means the request did not match the canned turn, which is + // expected and irrelevant: it still reached the proxy. + let arrived = log.served + usize::from(log.divergence.is_some()); + drop(log); + + println!(); + println!("model requests that reached the proxy: {arrived}"); + if run.timed_out { + println!( + "the agent was still running after {timeout_secs}s and was stopped. That is an \ + observation, not a wiring failure: an agent waiting for a useful reply will hang \ + against a canned one." + ); + // Found while testing this command: the deadline kills the process + // flowproof started, but a GRANDCHILD holding the inherited stdout + // pipe keeps the read blocking, so the wall-clock wait can exceed + // the timeout by a lot. Say so rather than let it look like a hang + // with no explanation. + println!( + " (if this took much longer than {timeout_secs}s, the agent spawned a child that \ + outlived it and kept the output pipe open. flowproof stops the process it started, \ + not the tree.)" + ); + } else { + println!( + "the agent exited {}", + run.exit_code + .map(|c| c.to_string()) + .unwrap_or_else(|| "with no code".into()) + ); + } + + if arrived == 0 { + println!(); + println!("NOTHING reached the proxy. The client is not honouring the base URL flowproof"); + println!("injected (OPENAI_BASE_URL, OPENAI_API_BASE, OPENAI_BASE, ANTHROPIC_BASE_URL,"); + println!("FLOWPROOF_LLM_PROXY). If it reads a different variable, or builds its base URL"); + println!("from a config object, map it in `agent.env`:"); + println!(); + println!(" env:"); + println!(" YOUR_VARIABLE: \"${{flowproof.proxy_url}}\" # includes /v1"); + println!( + " OR: \"${{flowproof.proxy_url_no_v1}}\" # client appends its own" + ); + println!(); + println!("If the model call happens in a CHILD process, check that it inherits the"); + println!("environment - that is the usual cause when the command itself looks right."); + if !run.stderr.trim().is_empty() { + println!(); + println!("stderr:\n{}", run.stderr.trim()); + } + return Ok(crate::EXIT_FAIL); + } + + println!(); + println!("At least one client reached the proxy, so recording can capture that traffic."); + println!("This does NOT prove every model call goes through flowproof: an agent with more"); + println!("than one client can reach the proxy with one and the real provider with another."); + println!("`record` is the check that settles it - it fails and writes no trace if nothing"); + println!("is captured."); + Ok(crate::EXIT_PASS) +} + /// Record an `app: agent` flow: run it against a real model, capture the /// trajectory, check the assertions, and write the cassette to `out`. /// Warn when a flow reasons about a tool that nothing actually intercepts. diff --git a/crates/flowproof-cli/src/lib.rs b/crates/flowproof-cli/src/lib.rs index d54a90a..99a2159 100644 --- a/crates/flowproof-cli/src/lib.rs +++ b/crates/flowproof-cli/src/lib.rs @@ -146,6 +146,25 @@ enum Command { #[arg(long, conflicts_with = "run")] since: Option, }, + /// Check that an agent's model traffic actually reaches flowproof, before + /// you write a spec or spend a key on a recording. + /// + /// Starts the proxy, runs the command once, and reports what ARRIVED. + /// It deliberately does not tell you the wiring is correct: it tells you + /// what it saw, because an agent with two clients can reach the proxy + /// with one and the real provider with the other. + Doctor { + /// The command that starts the agent, exactly as `agent.command:` + /// would spell it. + #[arg(long)] + agent: String, + /// Seconds to let the agent run before giving up on it. + #[arg(long, default_value_t = 60)] + timeout: u64, + /// The task handed to the agent through FLOWPROOF_PROMPT. + #[arg(long, default_value = "Say hello.")] + prompt: String, + }, /// Re-author the flow against the live app and propose a reviewable /// trace diff. Never modifies the trace unless --apply is passed. Heal { @@ -1668,6 +1687,11 @@ where since, } => cmd_audit(&dir, json, run, since), Command::Capture { port, out, json } => capture::cmd_capture(port, Some(out), json), + Command::Doctor { + agent, + timeout, + prompt, + } => agent_flow::cmd_doctor(&agent, timeout, &prompt), Command::Heal { spec, trace, diff --git a/crates/flowproof-cli/tests/doctor_e2e.rs b/crates/flowproof-cli/tests/doctor_e2e.rs new file mode 100644 index 0000000..e63d881 --- /dev/null +++ b/crates/flowproof-cli/tests/doctor_e2e.rs @@ -0,0 +1,83 @@ +//! `flowproof doctor` end to end: the whole point is that it distinguishes +//! an agent that reached the proxy from one that did not, without a spec, +//! without assertions and without an API key. +#![cfg(unix)] + +use std::io::Write; + +fn work_dir(name: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!("flowproof-doctor-e2e-{name}")); + std::fs::remove_dir_all(&dir).ok(); + std::fs::create_dir_all(&dir).expect("temp dir"); + dir +} + +fn script(dir: &std::path::Path, name: &str, body: &str) -> std::path::PathBuf { + let path = dir.join(name); + let mut f = std::fs::File::create(&path).expect("create"); + f.write_all(body.as_bytes()).expect("write"); + drop(f); + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).expect("chmod"); + path +} + +/// An agent that honours the injected base URL reaches the proxy, and doctor +/// exits 0. No key is set anywhere in this test. +#[test] +fn an_agent_that_honours_the_base_url_reaches_the_proxy() { + let dir = work_dir("wired"); + let agent = script( + &dir, + "agent.sh", + "#!/bin/sh\n\ + curl -sS -X POST \"$OPENAI_BASE_URL/chat/completions\" \ + -H 'content-type: application/json' \ + -d '{\"model\":\"x\",\"messages\":[]}' >/dev/null\n", + ); + let code = flowproof_cli::run_cli([ + "doctor", + "--agent", + agent.to_str().expect("utf8"), + "--timeout", + "30", + ]); + assert_eq!(code, 0, "a wired agent must pass doctor"); + std::fs::remove_dir_all(&dir).ok(); +} + +/// The failure this command exists for: a client reading its own variable, +/// which flowproof never set. It must FAIL, not pass quietly - a green +/// doctor on an unwired agent would send someone off to record and waste a +/// key discovering the same thing. +#[test] +fn an_agent_that_ignores_the_base_url_fails_doctor() { + let dir = work_dir("unwired"); + let agent = script( + &dir, + "agent.sh", + "#!/bin/sh\necho \"would call ${MY_OWN_GATEWAY:-https://real.example}\" >&2\n", + ); + let code = flowproof_cli::run_cli([ + "doctor", + "--agent", + agent.to_str().expect("utf8"), + "--timeout", + "30", + ]); + assert_ne!(code, 0, "an unwired agent must fail doctor"); + std::fs::remove_dir_all(&dir).ok(); +} + +/// A command that cannot start is a setup error, not a wiring verdict. +#[test] +fn a_command_that_does_not_exist_is_an_error() { + let code = flowproof_cli::run_cli([ + "doctor", + "--agent", + "/nonexistent/flowproof-doctor-agent", + "--timeout", + "10", + ]); + assert_ne!(code, 0); +} diff --git a/docs/agent-testing.md b/docs/agent-testing.md index 396e03f..8ea2fe6 100644 --- a/docs/agent-testing.md +++ b/docs/agent-testing.md @@ -130,6 +130,28 @@ from one base all route to the same stand-in. You do not need one listener per path; you need the base to point at the stand-in, which is what `${flowproof.mcp_url.}` is for. +**Check the wiring before writing a spec.** The failure above is the +commonest one in adoption, and it used to be found only after a spec was +written and a key spent. `flowproof doctor` answers the same question in +seconds, with no spec, no assertions and no key: + +```bash +flowproof doctor --agent "./start-agent" +``` + +It starts the proxy, runs the command once against a canned reply, and +reports how many model requests ARRIVED. Zero means the client is not +honouring the injected base URL, and the output names the handles to map. + +It reports what it saw rather than declaring the wiring correct, because an +agent with more than one client can reach the proxy with one and the real +provider with another. `record` is what settles that. + +Two limits worth knowing. It cannot tell a hang from a slow agent, so a +process waiting for a useful answer sits until `--timeout`. And if the agent +spawns a child that outlives it, the wall clock can exceed that timeout, +because flowproof stops the process it started rather than the tree. + **A record run that captures nothing FAILS.** If zero model requests reach the proxy, `record` errors and writes NO trace. That is the one failure a determinism tool must never let through: an agent that reached the real