From d51654822fa0ad4327c7465fe03ea9427be9e150 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 22 Jul 2025 00:41:27 -0700 Subject: [PATCH 1/3] fix: use PR_SET_PDEATHSIG so to ensure child processes are killed in a timely manner (#1626) Some users have reported issues where child processes are not cleaned up after Codex exits (e.g., https://github.com/openai/codex/issues/1570). This is generally a tricky issue on operating systems: if a parent process receives `SIGKILL`, then it terminates immediately and cannot communicate with the child. **It only helps on Linux**, but this PR introduces the use of `prctl(2)` so that if the parent process dies, `SIGTERM` will be delivered to the child process. Whereas previously, I believe that if Codex spawned a long-running process (like `tsc --watch`) and the Codex process received `SIGKILL`, the `tsc --watch` process would be reparented to the init process and would never be killed. Now with the use of `prctl(2)`, the `tsc --watch` process should receive `SIGTERM` in that scenario. We still need to come up with a solution for macOS. I've started to look at `launchd`, but I'm researching a number of options. --- codex-rs/Cargo.lock | 1 + codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/exec.rs | 25 +++++++++++++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 9c604e7948..9b4a4e32d4 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -669,6 +669,7 @@ dependencies = [ "fs2", "futures", "landlock", + "libc", "maplit", "mcp-types", "mime_guess", diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index e192a71f39..a87894bc4d 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -22,6 +22,7 @@ env-flags = "0.1.1" eventsource-stream = "0.2.3" fs2 = "0.4.3" futures = "0.3" +libc = "0.2.174" mcp-types = { path = "../mcp-types" } mime_guess = "2.0" rand = "0.9" diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 3b37cb538d..4b33b0b3b5 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -384,6 +384,31 @@ async fn spawn_child_async( cmd.env(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR, "1"); } + // If this Codex process dies (including being killed via SIGKILL), we want + // any child processes that were spawned as part of a `"shell"` tool call + // to also be terminated. + + // This relies on prctl(2), so it only works on Linux. + #[cfg(target_os = "linux")] + unsafe { + cmd.pre_exec(|| { + // This prctl call effectively requests, "deliver SIGTERM when my + // current parent dies." + if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM) == -1 { + return Err(io::Error::last_os_error()); + } + + // Though if there was a race condition and this pre_exec() block is + // run _after_ the parent (i.e., the Codex process) has already + // exited, then the parent is the _init_ process (which will never + // die), so we should just terminate the child process now. + if libc::getppid() == 1 { + libc::raise(libc::SIGTERM); + } + Ok(()) + }); + } + match stdio_policy { StdioPolicy::RedirectForShellTool => { // Do not create a file descriptor for stdin because otherwise some From ed206d568780ef2d757db638e359b4c497f417f1 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Tue, 22 Jul 2025 09:28:00 -0700 Subject: [PATCH 2/3] Log response.failed error message and request-id (#1649) To help with diagnosing failures. --- codex-rs/core/src/client.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 62fcabe05b..beeaa453ad 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -151,6 +151,17 @@ impl ModelClient { .json(&payload); let res = req_builder.send().await; + if let Ok(resp) = &res { + trace!( + "Response status: {}, request-id: {}", + resp.status(), + resp.headers() + .get("x-request-id") + .map(|v| v.to_str().unwrap_or_default()) + .unwrap_or_default() + ); + } + match res { Ok(resp) if resp.status().is_success() => { let (tx_event, rx_event) = mpsc::channel::>(1600); @@ -374,6 +385,19 @@ async fn process_sse( let _ = tx_event.send(Ok(ResponseEvent::Created {})).await; } } + "response.failed" => { + if let Some(resp_val) = event.response { + let error = resp_val + .get("error") + .and_then(|v| v.get("message")) + .and_then(|v| v.as_str()) + .unwrap_or("response.failed event received"); + + let _ = tx_event + .send(Err(CodexErr::Stream(error.to_string()))) + .await; + } + } // Final response completed – includes array of output items & id "response.completed" => { if let Some(resp_val) = event.response { From d5809ef6ef2343d2260cb0758f8d57df5891e8b3 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 22 Jul 2025 10:11:07 -0700 Subject: [PATCH 3/3] chore: install an extension for TOML syntax highlighting in the devcontainer --- .devcontainer/devcontainer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index f276868484..1bed79c3ca 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -21,7 +21,7 @@ "settings": { "terminal.integrated.defaultProfile.linux": "bash" }, - "extensions": ["rust-lang.rust-analyzer"] + "extensions": ["rust-lang.rust-analyzer", "tamasfe.even-better-toml"] } } }