-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Add turn started/completed events and correct exit code on error #4309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
caab6ce
9639157
44475a8
58fab60
fbd69e8
e9b35ba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -331,7 +331,13 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option<PathBuf>) -> any | |
info!("Sent prompt with event ID: {initial_prompt_task_id}"); | ||
|
||
// Run the loop until the task is complete. | ||
// Track whether a fatal error was reported by the server so we can | ||
// exit with a non-zero status for automation-friendly signaling. | ||
let mut error_seen = false; | ||
while let Some(event) = rx.recv().await { | ||
if matches!(event.msg, EventMsg::Error(_)) { | ||
error_seen = true; | ||
} | ||
let shutdown: CodexStatus = event_processor.process_event(event); | ||
match shutdown { | ||
CodexStatus::Running => continue, | ||
|
@@ -343,6 +349,9 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option<PathBuf>) -> any | |
} | ||
} | ||
} | ||
if error_seen { | ||
std::process::exit(1); | ||
} | ||
Comment on lines
+352
to
+354
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you want to return a special type of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, but outside of this PR, there are many |
||
|
||
Ok(()) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,3 +3,4 @@ mod apply_patch; | |
mod output_schema; | ||
mod resume; | ||
mod sandbox; | ||
mod server_error_exit; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
#![cfg(not(target_os = "windows"))] | ||
#![allow(clippy::expect_used, clippy::unwrap_used)] | ||
|
||
use core_test_support::responses; | ||
use core_test_support::test_codex_exec::test_codex_exec; | ||
use wiremock::matchers::any; | ||
|
||
/// Verify that when the server reports an error, `codex-exec` exits with a | ||
/// non-zero status code so automation can detect failures. | ||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] | ||
async fn exits_non_zero_when_server_reports_error() -> anyhow::Result<()> { | ||
let test = test_codex_exec(); | ||
|
||
// Mock a simple Responses API SSE stream that immediately reports a | ||
// `response.failed` event with an error message. | ||
let server = responses::start_mock_server().await; | ||
let body = responses::sse(vec![serde_json::json!({ | ||
"type": "response.failed", | ||
"response": { | ||
"id": "resp_err_1", | ||
"error": {"code": "rate_limit_exceeded", "message": "synthetic server error"} | ||
} | ||
})]); | ||
responses::mount_sse_once(&server, any(), body).await; | ||
|
||
test.cmd_with_server(&server) | ||
.arg("--skip-git-repo-check") | ||
.arg("tell me something") | ||
.arg("--experimental-json") | ||
.assert() | ||
.code(1); | ||
|
||
Ok(()) | ||
} |
Uh oh!
There was an error while loading. Please reload this page.