Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 20 additions & 13 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ already rendering events, check the first item.
- `run()` parses a JSONL stream rather than one document. The `Outcome` is unchanged.

Pin `Format::Json` to keep the old behaviour.
- **A turn the agent says failed is now an error, not an answer.** All three exit `0` for
some failures and put the explanation where the answer belongs: ask Claude for a model
that does not exist and it exits cleanly, reports `subtype: "success"`, and returns
"There's an issue with the selected model" as its reply. A caller checking `Result::is_ok`
rendered that as the model's answer. These are now `Error::AgentError`, carrying the
agent's own wording and the provider's status where one was given (typically 404 for an
unknown model). `NotAuthenticated` and `RateLimited` keep their own variants, since the
remedy differs; this covers the rest of that family. Codex forwards the upstream body as a
JSON *string*, so its message is unwrapped to the sentence and the status pulled out of it.
- **An unauthenticated Copilot run is now reported as one.** Its wording shares no phrase with
Claude's or Codex's, so it previously fell through to a generic `Error::Failed` with no
login hint. Callers matching on `Error` should expect `NotAuthenticated` from Copilot now.
Expand All @@ -51,6 +60,12 @@ already rendering events, check the first item.

### Fixed

- **A healthy quota heartbeat is no longer read as a refusal.** Every Claude `stream-json`
run prints a `rate_limit_event` record, and on a healthy run its status is `allowed`.
Classification scanned the raw stream for the substring `rate_limit`, which that record
contains, so any Claude failure came back as `Error::RateLimited`: a caller was told to
back off when the real cause was one they could fix. The parsed signal and the agent's own
prose now decide, and the raw scan is only the fallback for output that produced neither.
- **A failure 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
Expand All @@ -60,22 +75,14 @@ already rendering events, check the first item.
## 0.2.0

Nothing was removed or renamed, so this compiles against any 0.1 code. It is a minor bump
rather than a patch because two changes alter behaviour, and `"0.1"` would have delivered
rather than a patch because some changes alter behaviour, and `"0.1"` would have delivered
them through an ordinary `cargo update`.

### Behaviour changes
*Corrected after release: this entry originally also credited 0.2.0 with streaming by default
and token-by-token Claude text. Those landed three hours after 0.2.0 was published and ship in
0.2.1, where they are now listed. The published 0.2.0 does not have them.*

- **Streaming is now the default**, and a named session no longer disables it. `Format`
defaulted to `Json`, under which nothing is observable until the turn ends, so a caller
watching a twenty-minute run saw nothing for twenty minutes. Worse, `Request::session`
*pinned* that format, meaning the multi-turn path a chat UI always uses could not stream
at all. `Stream` carries everything `Json` does, so the default costs only parsing, and
`session()` now validates the format instead of overriding it. Pin `Format::Json`
explicitly if you only want the answer.
- **Claude streams token by token.** `--include-partial-messages` is passed under `Stream`,
without which Claude emits only completed messages and text arrives a paragraph at a time.
Claude sends both the deltas and the finished message; only the deltas are emitted, so a
transcript does not show every answer twice.
### Behaviour changes

- **An unauthenticated run is now an error.** Claude exits `0` and puts
"Not logged in · Please run /login" in its result text, so a missing login previously came
Expand Down
33 changes: 33 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,10 @@ own variables do not reach the child.
as a positional and the deny is silently lost. This crate always emits the combined form.
- **Copilot needs `--allow-all-tools` to run headlessly at all**, or it stalls at the first
tool confirmation. It is always emitted; `Permission` then narrows via denies.
- **A failed turn still exits 0.** Ask Claude for a model that does not exist and it exits
cleanly, reports `subtype: "success"`, and puts "There's an issue with the selected model"
where the answer belongs. Codex does the same and wraps the upstream body in a JSON string.
Both come back as `Error::AgentError`; see below.
- **Claude's `stream-json` requires `--verbose`**, or it refuses to start. Handled.
- **Large prompts move to stdin automatically** above 128 KiB, so a long prompt never fails
with `E2BIG`.
Expand Down Expand Up @@ -334,6 +338,35 @@ Under a structured format there is no silent fallback to raw stdout: a run that
recognizable records, or never reached its terminal record, returns `Error::Parse` rather
than a plausible-looking answer assembled from whatever was printed.

## A clean exit is not a successful turn

All three agents report some failures with exit code 0, with the explanation where the answer
should be. An unknown model, a rejected schema and an upstream outage all arrive this way, so
a caller checking only `Result::is_ok` renders the error as the model's reply.

These are `Error::AgentError`, carrying the agent's own wording and the provider's status
where one was reported:

```rust
match run(&request).await {
Err(Error::AgentError { status: Some(404), message, .. }) => {
// Typically a model the account cannot reach. `message` is the agent's wording.
eprintln!("{message}");
}
Err(e) if e.is_auth_failure() => { /* prompt for login */ }
Err(e) if e.is_transient() => { /* back off */ }
other => { /* ... */ }
}
```

`NotAuthenticated` and `RateLimited` are the two members of this family that predate it and
keep their own variants, because the remedy differs. Everything else in it lands here.

Codex needs one extra step: it forwards the provider's response body as a *string*, so its
`turn.failed` message is JSON containing the sentence and the status. This crate unwraps it,
so `message` is the sentence and `status` is the code. A message that is not that shape is
passed through untouched.

## No shell, ever

Arguments are built as a `Vec<String>` and passed straight to `exec`. There is no shell in
Expand Down
4 changes: 3 additions & 1 deletion docs/operating-limits.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ of that line, so a future change does not quietly cross it.
unedited, and surfaces Claude's `rate_limit_event` as an ordinary `Event::RateLimit`
during a run.
- **Passes model names through verbatim.** An unknown model is the provider's error to
raise, not this crate's to guess around.
raise, not this crate's to guess around. It is reported as [`Error::AgentError`] with the
provider's status, since the agents answer an unknown model with a clean exit and an
explanation where the answer belongs.

## What this crate deliberately does not do

Expand Down
25 changes: 25 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,31 @@ pub enum Error {
hint: &'static str,
},

/// The agent ran, exited cleanly, and reported that the turn itself failed.
///
/// Its own variant because the process succeeding says nothing about the
/// turn succeeding. An unknown model, a schema the provider rejects or an
/// upstream outage all arrive this way: exit code 0, with the failure
/// described in the output. Reporting that as `Ok` hands back an
/// [`crate::Outcome`] whose `text` is an error message, which a caller
/// checking only `Result::is_ok` will render as the answer.
///
/// The same reasoning already applied to [`Error::NotAuthenticated`] and
/// [`Error::RateLimited`], which are also reported with a zero exit; this
/// covers everything else in that family.
#[error("{agent} reported a failed turn{}: {message}", status.map(|s| format!(" (status {s})")).unwrap_or_default())]
AgentError {
/// The agent that failed.
agent: Agent,
/// The binary that ran.
bin: String,
/// The provider's status code, where the agent reported one. A 404 is
/// typically an unknown model, a 400 a rejected request.
status: Option<u16>,
/// The agent's own description, unedited.
message: String,
},

/// The CLI rejected an argument this crate passed it.
///
/// Almost always a version mismatch: the flag was verified against the
Expand Down
92 changes: 91 additions & 1 deletion src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,36 @@ pub struct Terminal {
pub first_unparsed: Option<String>,
/// The schema-conforming answer, where the agent reports one separately.
pub structured: Option<Value>,
/// The provider status code when the agent reported a failed turn, such as
/// a 404 for an unknown model.
pub error_status: Option<u16>,
/// The agent's own description of a failed turn, where it gives one apart
/// from the answer text. Claude puts its explanation in `result`, so this
/// stays `None` there; Codex reports it under `turn.failed`.
pub error_message: Option<String>,
}

/// Unwrap an error body an agent passed through as a JSON string.
///
/// Codex 0.145.0 forwards the upstream response verbatim, so `turn.failed`
/// carries `{"type":"error","status":400,"error":{"message":"..."}}` encoded as
/// a *string*. Showing that to a user means showing them JSON, and the status
/// worth branching on is buried inside it. Anything that is not that shape is
/// returned as-is.
fn unwrap_error_body(message: &str) -> (Option<u16>, String) {
let Ok(body) = serde_json::from_str::<Value>(message) else {
return (None, message.to_string());
};
let status = body
.get("status")
.and_then(Value::as_u64)
.and_then(|s| u16::try_from(s).ok());
let inner = body
.get("error")
.and_then(|e| e.get("message"))
.and_then(Value::as_str)
.map(str::to_string);
(status, inner.unwrap_or_else(|| message.to_string()))
}

/// Incrementally turns one agent's output into [`Event`]s and a [`Terminal`].
Expand Down Expand Up @@ -507,7 +537,13 @@ impl Parser {
self.term.structured = Some(value.clone());
}
self.term.usage = claude_usage(v);
// `subtype` says "success" even for a failed turn, so
// `is_error` is the field that actually decides.
self.term.stop = if v.get("is_error").and_then(Value::as_bool) == Some(true) {
self.term.error_status = v
.get("api_error_status")
.and_then(Value::as_u64)
.and_then(|s| u16::try_from(s).ok());
Stop::Error
} else {
stop_from(v.get("stop_reason"))
Expand Down Expand Up @@ -651,6 +687,15 @@ impl Parser {
"turn.failed" => {
self.seen.terminal = true;
self.term.stop = Stop::Error;
if let Some(message) = v
.get("error")
.and_then(|e| e.get("message"))
.and_then(Value::as_str)
{
let (status, message) = unwrap_error_body(message);
self.term.error_status = status;
self.term.error_message = Some(bound_text(message));
}
Vec::new()
}
"item.started" | "item.updated" | "item.completed" => {
Expand Down Expand Up @@ -796,8 +841,13 @@ impl Parser {
self.term.usage.premium_requests =
usage.get("premiumRequests").and_then(Value::as_u64);
}
if v.get("exitCode").and_then(Value::as_i64).unwrap_or(0) != 0 {
if let Some(code) = v.get("exitCode").and_then(Value::as_i64)
&& code != 0
{
self.term.stop = Stop::Error;
// Copilot reports no explanation with the code, so name the
// code rather than leave the failure blank.
self.term.error_message = Some(format!("copilot exited with code {code}"));
}
Vec::new()
}
Expand Down Expand Up @@ -1151,6 +1201,46 @@ mod tests {
);
}

/// Verbatim from codex-cli 0.145.0 with an unknown model. It exits **0**
/// and forwards the upstream body as a JSON *string*, so the status worth
/// branching on is nested one level inside a field that is itself text.
#[test]
fn a_codex_failed_turn_yields_the_reason_and_the_status() {
let (_, term) = run(
Agent::Codex,
&[
r#"{"type":"thread.started","thread_id":"019fad62"}"#,
r#"{"type":"turn.failed","error":{"message":"{\"type\":\"error\",\"status\":400,\"error\":{\"type\":\"invalid_request_error\",\"message\":\"The 'bogus-model-xyz' model is not supported when using Codex with a ChatGPT account.\"}}"}}"#,
],
);
assert_eq!(term.stop, Stop::Error);
assert_eq!(term.error_status, Some(400));
assert_eq!(
term.error_message.as_deref(),
Some(
"The 'bogus-model-xyz' model is not supported when using Codex with a ChatGPT account."
),
"the caller should get the sentence, not the envelope"
);
}

/// An error that is not the double-encoded shape must survive untouched
/// rather than be dropped for failing to match it.
#[test]
fn a_plain_codex_failure_message_passes_through() {
let (_, term) = run(
Agent::Codex,
&[
r#"{"type":"turn.failed","error":{"message":"stream disconnected before completion"}}"#,
],
);
assert_eq!(term.error_status, None);
assert_eq!(
term.error_message.as_deref(),
Some("stream disconnected before completion")
);
}

#[test]
fn codex_reads_the_thread_id_and_the_completed_message() {
let (events, term) = run(
Expand Down
Loading
Loading