From f341458dda1c7a019d1657adb10a57db924b6abc Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 29 Jul 2026 06:09:19 +0700 Subject: [PATCH] Add structured output, so an answer can be data rather than prose A consumer building anything on top of this, a code review being the case in hand, wants findings back as data. Without a schema it has to parse prose, which is a guess about formatting the model never promised. `Request::schema` constrains the answer and `Outcome::structured` carries the parsed value. The template that uses it, review prompts, a findings shape, diff handling, belongs a layer up in the consumer: this is the capability, not the policy, and only this crate can express it because it is a CLI flag. The three agents differ and the difference is hidden rather than leaked: - Claude takes the schema inline and reports the conforming value in its own `structured_output` field. - Codex reads it from a file, so the runner writes one for the run and removes it afterwards through a guard, meaning cancel and timeout clean up too. Its answer is the JSON itself, parsed only when a schema was asked for, so prose is never reinterpreted as data. - Copilot 1.0.75 has no schema support at all, so asking is Error::Unsupported rather than prose presented as data. Both shapes verified live against the real CLIs. Two things found while building it, both worth keeping: Codex sends the schema to a provider requiring `additionalProperties: false` on every object, and rejects the request with a 400 before the model runs otherwise. Claude accepts the looser form, so a schema that works there can still fail on Codex. Documented on the builder and in the README, since the error arrives from the provider rather than from anything this crate could check. That rejection also exposed a reporting bug. Error::Failed took the first line of stderr as the cause, but CLIs open with progress chatter: the report read "Reading additional input from stdin...", which explains nothing, while the real cause sat in Codex's JSON error event on stdout. A line that looks like an error now wins over one that narrates, and stdout is consulted when stderr only narrates. That misdirection cost me three rounds of diagnosis here, which is exactly what it would cost a user. --- CHANGELOG.md | 16 +++++ README.md | 26 +++++++ src/agent.rs | 102 +++++++++++++++++++++++++++ src/event.rs | 7 ++ src/outcome.rs | 6 ++ src/request.rs | 47 +++++++++++++ src/run.rs | 184 +++++++++++++++++++++++++++++++++++++++++++++---- tests/live.rs | 95 +++++++++++++++++++++++++ 8 files changed, 470 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60aed63..1afd595 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,11 @@ them through an ordinary `cargo update`. *highest-precedence* credential variable Copilot accepts and it was missing from the list, so a host authenticating that way would have failed to authenticate at all once `Minimal` became the default. +- **A failure now reports its cause rather than a status line.** The first line of stderr + was taken as the explanation, but CLIs open with progress chatter: a rejected Codex schema + reported "Reading additional input from stdin...", which is not what went wrong. A line + that looks like an error now wins, and stdout is consulted when stderr only narrates, since + Codex reports errors as JSON events on stdout. - **Dropping a `Run` now reliably kills the process group.** It signalled the driver and aborted it, which left the kill waiting on the runtime to poll the aborted task. On Linux that did not reliably happen and grandchildren survived, while `cancel` and timeouts were @@ -51,6 +56,17 @@ them through an ordinary `cargo update`. ### Added +- **`Request::schema`** constrains an answer to a JSON Schema, with the conforming value on + `Outcome::structured` already parsed. For answers that are data rather than prose, such as + a set of review findings, this replaces guessing at formatting the model never promised. + Delivery differs and is hidden: Claude takes the schema inline and reports the value in its + own field, Codex reads it from a file this crate writes and removes. Copilot 1.0.75 has no + schema support, so asking is `Error::Unsupported` rather than prose presented as data. + + Write schemas strictly: Codex sends yours to a provider that requires + `"additionalProperties": false` on every object, and rejects the request with a 400 before + the model runs otherwise. Claude is more forgiving, so a schema that works there can still + fail on Codex. - **`AuthStatus::check(agent)`** answers whether an agent is logged in without spending a request, which a missing login otherwise only revealed by running a turn and failing. Claude reports JSON, Codex reports prose, and Copilot offers neither: that case is diff --git a/README.md b/README.md index 5e4ab61..5f9cf36 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,32 @@ the first one's conversation. Uppercase is encoded too, since macOS and Windows case-insensitive and would otherwise collide `Chat` with `chat`. The record keeps your original name, so `list()` hands back what you passed, not a mangled segment. +## Structured answers + +When the answer is data rather than prose, constrain it with a JSON Schema and read it +back parsed instead of guessing at formatting the model never promised: + +```rust +let outcome = run(&Request::new(Agent::Codex, "Alice is 30 years old.") + .schema(r#"{"type":"object", + "properties":{"name":{"type":"string"},"age":{"type":"integer"}}, + "required":["name","age"],"additionalProperties":false}"#)) + .await?; + +assert_eq!(outcome.structured.unwrap()["name"], "Alice"); +``` + +The delivery differs and is hidden: Claude takes the schema inline and reports the value in +its own field, Codex reads it from a file this crate writes and removes, and returns the +value as its answer text. **Copilot 1.0.75 has no schema support at all**, so asking is an +`Error::Unsupported` rather than prose dressed up as data. + +**Write schemas strictly.** Codex sends yours to OpenAI's structured-output API, which +requires `"additionalProperties": false` on every object and every property in `required`. +Without it the request fails with a 400 before the model runs. Claude is more forgiving, so +a schema that works there can still fail on Codex; writing to the stricter rule keeps one +schema usable for both. + ## Cancellation **Dropping a `Run` kills the agent, and everything it spawned.** Closing a window or diff --git a/src/agent.rs b/src/agent.rs index f1cfd42..e0feb39 100644 --- a/src/agent.rs +++ b/src/agent.rs @@ -51,6 +51,21 @@ pub struct Caps { /// Whether the agent takes a real system-prompt flag. When false the system /// text is prepended to the prompt so it still reaches the model. pub native_system: bool, + /// How the agent accepts a JSON Schema for its answer, if at all. + pub schema: SchemaSupport, +} + +/// How an agent accepts a JSON Schema constraining its answer. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SchemaSupport { + /// The schema rides the command line (`claude --json-schema `). + Inline, + /// The schema must be a file the agent reads + /// (`codex exec --output-schema `), so the runner writes one. + File, + /// No structured-output support. Asking is an error rather than a prose + /// answer dressed up as data. + None, } /// Permission posture for a run, mapped onto each agent's own vocabulary. @@ -216,6 +231,15 @@ pub struct Plan { pub cont: Continue, /// True when the prompt is piped on stdin instead of riding the argv. pub stdin_prompt: bool, + /// A JSON Schema the answer must conform to, as text. + /// + /// Delivered differently per agent: Claude takes it inline, Codex takes a + /// path, so this is the source and `schema_file` is where the runner put it + /// when a file was needed. + pub schema: Option, + /// Path to the schema on disk, materialized by the runner for agents that + /// take a file rather than an inline value. + pub schema_file: Option, } /// Prompts at or above this many bytes are piped on stdin rather than placed on @@ -416,6 +440,9 @@ impl Agent { fork: true, events: true, native_system: true, + // Verified: `--json-schema ` puts the conforming value + // in the result document's `structured_output`. + schema: SchemaSupport::Inline, }, // `codex exec --json` emits `thread_id`; continuation is the // `resume` subcommand and is linear (`codex fork` is TUI-only). @@ -424,6 +451,9 @@ impl Agent { fork: false, events: true, native_system: false, + // Verified: `--output-schema ` makes the final + // `agent_message` the conforming JSON, with no separate field. + schema: SchemaSupport::File, }, // Verified against Copilot CLI 1.0.75: `--session-id ` both // mints a new session and resumes an existing one (one flag, both @@ -434,6 +464,8 @@ impl Agent { fork: false, events: true, native_system: false, + // Copilot 1.0.75 exposes no schema flag at all. + schema: SchemaSupport::None, }, } } @@ -482,6 +514,12 @@ impl Agent { what: "assigning a session id up front", }); } + if plan.schema.is_some() && caps.schema == SchemaSupport::None { + return Err(Error::Unsupported { + agent: self, + what: "constraining its answer to a JSON schema", + }); + } if plan.format == Format::Stream && !caps.events { return Err(Error::Unsupported { agent: self, @@ -693,6 +731,10 @@ fn argv_claude(plan: &Plan) -> Vec { Format::Stream => "stream-json", }, ); + if let Some(schema) = &plan.schema { + // Inline, and the conforming value comes back in `structured_output`. + a.secret("--json-schema", schema, Sensitivity::Prompt); + } if plan.format == Format::Stream { // Claude refuses `-p --output-format stream-json` without it: // "--print with --output-format=stream-json requires --verbose". @@ -741,6 +783,15 @@ fn argv_codex(plan: &Plan) -> Vec { }; a.opt("--model", plan.model.as_ref()); + // Codex reads the schema from a file, which the runner writes before the + // spawn. `Request::argv` has no file to name, so it shows a placeholder: + // the preview is for display, and the real path exists only at spawn time. + if plan.schema.is_some() { + a.pair( + "--output-schema", + plan.schema_file.as_deref().unwrap_or(""), + ); + } // `--json` is Codex's event stream and the only place `thread_id` appears. if plan.format != Format::Text { a.bare("--json"); @@ -827,6 +878,8 @@ mod tests { format: Format::Json, cont: Continue::New, stdin_prompt: false, + schema: None, + schema_file: None, } } @@ -1022,6 +1075,55 @@ mod tests { /// `codex exec resume` rejects `--sandbox` and takes `-c sandbox_mode=` /// instead. Getting this wrong makes every second turn fail with an /// "unexpected argument" error, which only a multi-turn run reveals. + /// The two CLIs take a schema differently, and the difference is the + /// whole reason this needs handling rather than one shared flag. + #[test] + fn each_agent_takes_a_schema_in_its_own_shape() { + let schema = r#"{"type":"object"}"#; + + let mut claude = plan("claude"); + claude.schema = Some(schema.into()); + let a = argv(Agent::Claude, &claude); + assert_eq!( + a[pos(&a, "--json-schema").unwrap() + 1], + schema, + "claude takes it inline" + ); + + let mut codex = plan("codex"); + codex.schema = Some(schema.into()); + codex.schema_file = Some("/tmp/s.json".into()); + let a = argv(Agent::Codex, &codex); + assert_eq!( + a[pos(&a, "--output-schema").unwrap() + 1], + "/tmp/s.json", + "codex takes a path, never the schema itself" + ); + assert!(!a.iter().any(|arg| arg.contains("\"type\""))); + } + + /// Copilot 1.0.75 has no schema flag, and a prose answer presented as data + /// is exactly the silent downgrade this crate refuses elsewhere. + #[test] + fn copilot_refuses_a_schema_rather_than_answering_in_prose() { + let mut p = plan("copilot"); + p.schema = Some(r#"{"type":"object"}"#.into()); + assert!(matches!( + Agent::Copilot.argv(&p), + Err(Error::Unsupported { .. }) + )); + } + + /// A caller inspecting the command before running it has no file yet, since + /// it is written at spawn time. + #[test] + fn a_codex_schema_preview_shows_a_placeholder_path() { + let mut p = plan("codex"); + p.schema = Some(r#"{"type":"object"}"#.into()); + let a = argv(Agent::Codex, &p); + assert_eq!(a[pos(&a, "--output-schema").unwrap() + 1], ""); + } + #[test] fn codex_sets_the_sandbox_by_flag_when_fresh_and_by_config_when_resuming() { let mut fresh = plan("codex"); diff --git a/src/event.rs b/src/event.rs index 27e311b..d988d18 100644 --- a/src/event.rs +++ b/src/event.rs @@ -272,6 +272,8 @@ pub struct Terminal { pub unparsed: usize, /// The first line that failed to parse, as evidence for the above. pub first_unparsed: Option, + /// The schema-conforming answer, where the agent reports one separately. + pub structured: Option, } /// Incrementally turns one agent's output into [`Event`]s and a [`Terminal`]. @@ -476,6 +478,11 @@ impl Parser { if let Some(text) = v.get("result").and_then(Value::as_str) { self.term.text = text.to_string(); } + // Claude returns the conforming value as its own field, so it + // needs no re-parsing out of the answer text. + if let Some(value) = v.get("structured_output") { + self.term.structured = Some(value.clone()); + } self.term.usage = claude_usage(v); self.term.stop = if v.get("is_error").and_then(Value::as_bool) == Some(true) { Stop::Error diff --git a/src/outcome.rs b/src/outcome.rs index 74745c4..e79b655 100644 --- a/src/outcome.rs +++ b/src/outcome.rs @@ -105,6 +105,12 @@ pub struct Outcome { pub unparsed: usize, /// The first line that failed to parse. pub first_unparsed: Option, + /// The answer parsed against the schema given to [`crate::Request::schema`]. + /// + /// `None` when no schema was asked for, or when the agent's answer did not + /// parse as JSON. Never a re-interpretation of prose: this is only set from + /// a value the agent produced under a schema. + pub structured: Option, } impl Outcome { diff --git a/src/request.rs b/src/request.rs index 1999be4..0bd3c3a 100644 --- a/src/request.rs +++ b/src/request.rs @@ -34,6 +34,9 @@ pub struct Request { pub(crate) env: Vec<(String, String)>, pub(crate) extra_args: Vec, pub(crate) env_policy: EnvPolicy, + pub(crate) schema: Option, + /// Set by the runner for agents that read the schema from a file. + pub(crate) schema_file: Option, pub(crate) timeout: Option, /// Set when [`Request::session`] resolved a named session, so the runner /// knows to write the binding back. @@ -69,6 +72,8 @@ impl Request { env: Vec::new(), extra_args: Vec::new(), env_policy: EnvPolicy::Minimal, + schema: None, + schema_file: None, timeout: None, binding: None, } @@ -174,6 +179,44 @@ impl Request { self } + /// Constrain the answer to a JSON Schema. + /// + /// The agent is asked to return a value conforming to `schema`, which + /// [`Outcome::structured`] then carries already parsed. Useful when the + /// answer is data rather than prose: a set of review findings, an + /// extraction, a classification. Reading it beats parsing prose, which is + /// a guess about formatting the model never promised. + /// + /// The two CLIs that support this take it differently, and the difference + /// is hidden: Claude accepts the schema inline, Codex reads it from a file + /// this crate writes for the run and removes afterwards. **Copilot 1.0.75 + /// has no schema support**, so asking is [`crate::Error::Unsupported`] + /// rather than a prose answer presented as data. + /// + /// The schema is passed through unvalidated; a malformed one surfaces as + /// the agent's own error. + /// + /// # Write the schema strictly + /// + /// Codex sends it to `OpenAI`'s structured-output API, which rejects anything + /// permissive. Every object needs `"additionalProperties": false` and every + /// property listed in `required`, or the request fails with a 400 before + /// the model runs: + /// + /// ```text + /// 'additionalProperties' is required to be supplied and to be false + /// ``` + /// + /// Claude is more forgiving, so a schema that works there can still fail on + /// Codex. Writing to the stricter rule keeps one schema usable for both. + /// + /// [`Outcome::structured`]: crate::Outcome::structured + #[must_use] + pub fn schema(mut self, schema: impl Into) -> Self { + self.schema = Some(schema.into()); + self + } + /// Continue an earlier conversation by its native id, bypassing the session /// store. Prefer [`Request::session`] unless you are tracking ids yourself. #[must_use] @@ -264,6 +307,8 @@ impl Request { self.prompt.len() + self.system.as_ref().map_or(0, String::len) + self.extra_args.iter().map(String::len).sum::() + // Claude's schema rides the command line too. + + self.schema.as_ref().map_or(0, String::len) } /// The format this request will actually use. @@ -304,6 +349,8 @@ impl Request { // system prompt rides its own argument. A small prompt with a large // system prompt would otherwise still hit E2BIG. stdin_prompt: self.argv_weight() >= STDIN_THRESHOLD, + schema: self.schema.clone(), + schema_file: self.schema_file.clone(), } } diff --git a/src/run.rs b/src/run.rs index 7c32525..79f6885 100644 --- a/src/run.rs +++ b/src/run.rs @@ -284,6 +284,22 @@ pub fn stream(request: &Request) -> Result { // hide that, so the context is checked and reported as an ordinary error. let runtime = tokio::runtime::Handle::try_current().map_err(|_| Error::NoRuntime)?; + // Written before the argv is built, because the argv has to name it. + let schema_file = match (&request.schema, request.agent.caps().schema) { + (Some(schema), crate::agent::SchemaSupport::File) => { + Some(SchemaFile::write(schema).map_err(|source| Error::Spawn { + bin: request.agent.bin().to_string(), + source, + })?) + } + _ => None, + }; + let mut request = request.clone(); + if let Some(file) = &schema_file { + request.schema_file = Some(file.0.display().to_string()); + } + let request = &request; + let plan = request.plan(); let typed = request.typed_argv()?; let argv: Vec = typed.iter().map(|a| a.value.clone()).collect(); @@ -361,14 +377,13 @@ pub fn stream(request: &Request) -> Result { let (tx, rx) = mpsc::channel(EVENT_BUFFER); let (cancel_tx, cancel_rx) = tokio::sync::oneshot::channel(); let reaped = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + let reaped_for_task = std::sync::Arc::clone(&reaped); let request = request.clone(); - let task = runtime.spawn(drive( - child, - request, - tx, - cancel_rx, - std::sync::Arc::clone(&reaped), - )); + let task = runtime.spawn(async move { + // Moved in so the file outlives the run and is removed with it. + let _schema_file = schema_file; + drive(child, request, tx, cancel_rx, reaped_for_task).await + }); Ok(Run { events: rx, typed, @@ -390,6 +405,45 @@ fn inherit_named>(command: &mut Command, names: &[S]) { } } +/// A schema file written for one run, removed when the run ends. +/// +/// Codex reads its schema from disk, so the file has to outlive the spawn and +/// not outlive the process. Tying it to a guard means every exit path removes +/// it, including a cancel or a timeout, without each one remembering. +struct SchemaFile(std::path::PathBuf); + +impl SchemaFile { + /// Write `schema` somewhere the agent can read it. + fn write(schema: &str) -> std::io::Result { + use std::io::Write as _; + use std::sync::atomic::{AtomicU64, Ordering}; + static COUNTER: AtomicU64 = AtomicU64::new(0); + + let path = std::env::temp_dir().join(format!( + "agent-abstraction-schema-{}-{}.json", + std::process::id(), + COUNTER.fetch_add(1, Ordering::Relaxed) + )); + let mut options = std::fs::OpenOptions::new(); + options.write(true).create_new(true); + // A schema can encode what a caller is looking for, so it is no more + // public than the prompt. + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt as _; + options.mode(0o600); + } + options.open(&path)?.write_all(schema.as_bytes())?; + Ok(SchemaFile(path)) + } +} + +impl Drop for SchemaFile { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.0); + } +} + /// Owns the child and tears down its whole process group when dropped. /// /// `kill_on_drop` alone is not enough: it kills the CLI, leaving the commands @@ -619,6 +673,13 @@ async fn drive( // A fork lands on a *new* id the agent only reveals at the end, so the name // has to be repointed once the run settles. Everything else was bound above. persist_result?; + // Resolved before the terminal is consumed by the Outcome below. + let structured = terminal.structured.clone().or_else(|| { + request + .schema + .as_ref() + .and_then(|_| serde_json::from_str(&terminal.text).ok()) + }); if let Some(token) = &terminal.session && !bound { @@ -635,6 +696,10 @@ async fn drive( stderr, unparsed: terminal.unparsed, first_unparsed: terminal.first_unparsed, + // Claude reports the conforming value separately; Codex returns it as + // the answer text, so that is parsed only when a schema was asked for. + // Prose is never reinterpreted as data. + structured, }) } @@ -727,10 +792,31 @@ fn classify(bin: &str, code: i32, stderr: &str, stdout: &str, terminal: &Termina Error::Failed { bin: bin.to_string(), code, - stderr: first_meaningful_line(stderr).unwrap_or_default(), + // Fall back to stdout when stderr explains nothing. Codex reports a + // rejected schema as an `{"type":"error"}` event on *stdout* while + // stderr carries only "Reading additional input from stdin...", so + // reporting stderr alone describes the failure as a status message. + stderr: first_meaningful_line(stderr) + .filter(|line| looks_explanatory(line)) + .or_else(|| first_meaningful_line(stdout)) + .or_else(|| first_meaningful_line(stderr)) + .unwrap_or_default(), } } +/// Whether a line plausibly explains a failure rather than narrating progress. +fn looks_explanatory(line: &str) -> bool { + const NOISE: &[&str] = &[ + "reading additional input", + "reading prompt", + "waiting", + "connecting", + "loading", + ]; + let lower = line.to_ascii_lowercase(); + !NOISE.iter().any(|noise| lower.contains(noise)) +} + /// The CLI's complaint, if it refused an argument. /// /// The phrasings are clap's and commander's, which is what all three CLIs are @@ -771,13 +857,41 @@ fn looks_rate_limited(text: &str) -> bool { .any(|needle| lower.contains(needle)) } -/// The first non-blank line, trimmed. Enough to identify a failure without -/// pasting an entire stack trace into an error message. +/// The most useful line of a CLI's output for an error message. +/// +/// Not simply the first non-blank one. CLIs open with progress and status +/// chatter, so the first line is often "Reading additional input from stdin..." +/// while the actual cause is further down. That turns a report into a +/// misdirection: it looks like an explanation and is not one. +/// +/// So a line that looks like an error wins, and the first non-blank line is the +/// fallback when nothing does. fn first_meaningful_line(text: &str) -> Option { - text.lines() + const ERROR_MARKERS: &[&str] = &[ + "error", + "failed", + "fatal", + "panic", + "denied", + "invalid", + "unexpected", + "cannot", + "unable", + ]; + let lines: Vec<&str> = text + .lines() .map(str::trim) - .find(|line| !line.is_empty()) - .map(str::to_string) + .filter(|line| !line.is_empty()) + .collect(); + + lines + .iter() + .find(|line| { + let lower = line.to_ascii_lowercase(); + ERROR_MARKERS.iter().any(|marker| lower.contains(marker)) + }) + .or_else(|| lines.first()) + .map(|line| (*line).to_string()) } /// Write the session binding back, reporting any store failure. @@ -959,6 +1073,50 @@ mod tests { } } + /// Real output from a failing codex run: the first line is status, the + /// cause is below it. Reporting the first line looks like an explanation + /// while pointing at the wrong thing. + #[test] + fn a_status_line_does_not_masquerade_as_the_cause() { + let stderr = "Reading additional input from stdin...\n\ + error: invalid value 'nope' for '--sandbox '"; + let err = classify_run(Agent::Codex, "codex", 1, stderr, "", &Terminal::default()); + let Error::Failed { + stderr: reported, .. + } = err + else { + panic!("expected Failed, got {err:?}") + }; + assert!(reported.contains("invalid value"), "reported {reported:?}"); + } + + /// Codex reports a rejected schema as a JSON error event on **stdout** + /// while stderr carries only a status line. Reporting stderr alone + /// described the failure as "Reading additional input from stdin...", + /// which is not what went wrong. + #[test] + fn a_cause_on_stdout_is_reported_when_stderr_only_narrates() { + let stdout = r#"{"type":"error","message":"invalid_json_schema: 'additionalProperties' is required to be supplied and to be false."}"#; + let err = classify_run( + Agent::Codex, + "codex", + 1, + "Reading additional input from stdin...", + stdout, + &Terminal::default(), + ); + let Error::Failed { + stderr: reported, .. + } = err + else { + panic!("expected Failed, got {err:?}") + }; + assert!( + reported.contains("additionalProperties"), + "reported {reported:?}, which explains nothing" + ); + } + #[test] fn failures_report_the_first_useful_line() { let err = classify( diff --git a/tests/live.rs b/tests/live.rs index b1ec786..d83c90e 100644 --- a/tests/live.rs +++ b/tests/live.rs @@ -513,3 +513,98 @@ async fn installed_agents_report_their_login_state() { eprintln!("{agent}: {}", status.summary()); } } + +/// Structured output is the capability a consumer needs to get findings back as +/// data rather than prose to re-parse. The two CLIs deliver it differently, +/// Claude inline and Codex through a file this crate writes, so this proves the +/// unified interface against both rather than against the flag mapping alone. +#[tokio::test] +#[ignore = "spawns a real agent and consumes quota"] +async fn a_schema_constrains_the_answer_to_data() { + const SCHEMA: &str = r#"{ + "type": "object", + "properties": {"name": {"type": "string"}, "age": {"type": "integer"}}, + "required": ["name", "age"], + "additionalProperties": false + }"#; + + for agent in [Agent::Claude, Agent::Codex] { + if !available(agent) { + continue; + } + let request = Request::new(agent, "Alice is 30 years old.") + .permission(Permission::ReadOnly) + .timeout(Duration::from_secs(180)) + .schema(SCHEMA); + let request = match agent { + Agent::Claude => request.model("haiku"), + _ => request, + }; + + let outcome = run(&request) + .await + .unwrap_or_else(|e| panic!("{agent} schema run failed: {e}")); + + let value = outcome + .structured + .unwrap_or_else(|| panic!("{agent} returned no structured answer: {:?}", outcome.text)); + assert_eq!(value["name"], "Alice", "{agent}: {value}"); + assert_eq!(value["age"], 30, "{agent}: {value}"); + } +} + +/// Copilot has no schema support, so asking must fail before spawning rather +/// than returning prose that a caller would try to parse as data. +#[tokio::test] +async fn copilot_refuses_a_schema_before_spawning() { + let err = Request::new(Agent::Copilot, "hi") + .schema(r#"{"type":"object"}"#) + .argv() + .unwrap_err(); + assert!( + matches!(err, agent_abstraction::Error::Unsupported { .. }), + "got {err:?}" + ); +} + +/// The schema file Codex reads must not outlive the run that needed it. +#[tokio::test] +#[ignore = "spawns a real agent and consumes quota"] +async fn a_codex_schema_file_is_cleaned_up() { + if !available(Agent::Codex) { + return; + } + let before = schema_files_in_temp(); + let outcome = run(&Request::new(Agent::Codex, "Alice is 30 years old.") + .permission(Permission::ReadOnly) + .timeout(Duration::from_secs(180)) + // `additionalProperties: false` is mandatory for Codex: without it the + // provider rejects the schema with a 400 before the model runs. + .schema( + r#"{"type":"object","properties":{"name":{"type":"string"}}, + "required":["name"],"additionalProperties":false}"#, + )) + .await + .expect("run failed"); + + assert!(outcome.structured.is_some()); + assert_eq!( + schema_files_in_temp(), + before, + "a schema file was left behind in the temp directory" + ); +} + +/// Count this crate's schema files currently in the temp directory. +fn schema_files_in_temp() -> usize { + std::fs::read_dir(std::env::temp_dir()).map_or(0, |entries| { + entries + .flatten() + .filter(|e| { + e.file_name() + .to_string_lossy() + .starts_with("agent-abstraction-schema-") + }) + .count() + }) +}