From c283f9f6ce350eac608d48e958c5de51380e9403 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 18 Aug 2025 12:59:19 -0700 Subject: [PATCH 01/14] Add an operation to override current task context (#2431) - Added an operation to override current task context - Added a test to check that cache stays the same --- codex-rs/core/src/client.rs | 25 +++++ codex-rs/core/src/codex.rs | 81 +++++++++++++++- codex-rs/core/tests/prompt_caching.rs | 127 ++++++++++++++++++++++++++ codex-rs/protocol/src/protocol.rs | 32 +++++++ 4 files changed, 263 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 7319c9a026..86a711e436 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -33,6 +33,7 @@ use crate::error::CodexErr; use crate::error::Result; use crate::error::UsageLimitReachedError; use crate::flags::CODEX_RS_SSE_FIXTURE; +use crate::model_family::ModelFamily; use crate::model_provider_info::ModelProviderInfo; use crate::model_provider_info::WireApi; use crate::models::ResponseItem; @@ -311,6 +312,30 @@ impl ModelClient { pub fn get_provider(&self) -> ModelProviderInfo { self.provider.clone() } + + /// Returns the currently configured model slug. + pub fn get_model(&self) -> String { + self.config.model.clone() + } + + /// Returns the currently configured model family. + pub fn get_model_family(&self) -> ModelFamily { + self.config.model_family.clone() + } + + /// Returns the current reasoning effort setting. + pub fn get_reasoning_effort(&self) -> ReasoningEffortConfig { + self.effort + } + + /// Returns the current reasoning summary setting. + pub fn get_reasoning_summary(&self) -> ReasoningSummaryConfig { + self.summary + } + + pub fn get_auth(&self) -> Option { + self.auth.clone() + } } #[derive(Debug, Deserialize, Serialize)] diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 8670978e86..397246a7de 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -989,7 +989,7 @@ async fn submission_loop( rx_sub: Receiver, ) { // Wrap once to avoid cloning TurnContext for each task. - let turn_context = Arc::new(turn_context); + let mut turn_context = Arc::new(turn_context); // To break out of this loop, send Op::Shutdown. while let Ok(sub) = rx_sub.recv().await { debug!(?sub, "Submission"); @@ -997,6 +997,83 @@ async fn submission_loop( Op::Interrupt => { sess.interrupt_task(); } + Op::OverrideTurnContext { + cwd, + approval_policy, + sandbox_policy, + model, + effort, + summary, + } => { + // Recalculate the persistent turn context with provided overrides. + let prev = Arc::clone(&turn_context); + let provider = prev.client.get_provider(); + + // Effective model + family + let (effective_model, effective_family) = if let Some(m) = model { + let fam = + find_family_for_model(&m).unwrap_or_else(|| config.model_family.clone()); + (m, fam) + } else { + (prev.client.get_model(), prev.client.get_model_family()) + }; + + // Effective reasoning settings + let effective_effort = effort.unwrap_or(prev.client.get_reasoning_effort()); + let effective_summary = summary.unwrap_or(prev.client.get_reasoning_summary()); + + let auth = prev.client.get_auth(); + // Build updated config for the client + let mut updated_config = (*config).clone(); + updated_config.model = effective_model.clone(); + updated_config.model_family = effective_family.clone(); + + let client = ModelClient::new( + Arc::new(updated_config), + auth, + provider, + effective_effort, + effective_summary, + sess.session_id, + ); + + let new_approval_policy = approval_policy.unwrap_or(prev.approval_policy); + let new_sandbox_policy = sandbox_policy + .clone() + .unwrap_or(prev.sandbox_policy.clone()); + let new_cwd = cwd.clone().unwrap_or_else(|| prev.cwd.clone()); + + let tools_config = ToolsConfig::new( + &effective_family, + new_approval_policy, + new_sandbox_policy.clone(), + config.include_plan_tool, + config.include_apply_patch_tool, + ); + + let new_turn_context = TurnContext { + client, + tools_config, + user_instructions: prev.user_instructions.clone(), + base_instructions: prev.base_instructions.clone(), + approval_policy: new_approval_policy, + sandbox_policy: new_sandbox_policy.clone(), + shell_environment_policy: prev.shell_environment_policy.clone(), + cwd: new_cwd.clone(), + disable_response_storage: prev.disable_response_storage, + }; + + // Install the new persistent context for subsequent tasks/turns. + turn_context = Arc::new(new_turn_context); + if cwd.is_some() || approval_policy.is_some() || sandbox_policy.is_some() { + sess.record_conversation_items(&[ResponseItem::from(EnvironmentContext::new( + new_cwd, + new_approval_policy, + new_sandbox_policy, + ))]) + .await; + } + } Op::UserInput { items } => { // attempt to inject input into current task if let Err(items) = sess.inject_input(items) { @@ -1057,7 +1134,7 @@ async fn submission_loop( cwd, disable_response_storage: turn_context.disable_response_storage, }; - + // TODO: record the new environment context in the conversation history // no current task, spawn a new one with the per‑turn context let task = AgentTask::spawn(sess.clone(), Arc::new(fresh_turn_context), sub.id, items); diff --git a/codex-rs/core/tests/prompt_caching.rs b/codex-rs/core/tests/prompt_caching.rs index d637eb674e..e528cb7a64 100644 --- a/codex-rs/core/tests/prompt_caching.rs +++ b/codex-rs/core/tests/prompt_caching.rs @@ -1,9 +1,13 @@ use codex_core::ConversationManager; use codex_core::ModelProviderInfo; use codex_core::built_in_model_providers; +use codex_core::protocol::AskForApproval; use codex_core::protocol::EventMsg; use codex_core::protocol::InputItem; use codex_core::protocol::Op; +use codex_core::protocol::SandboxPolicy; +use codex_core::protocol_config_types::ReasoningEffort as ReasoningEffortConfig; +use codex_core::protocol_config_types::ReasoningSummary as ReasoningSummaryConfig; use codex_login::CodexAuth; use core_test_support::load_default_config_for_test; use core_test_support::load_sse_fixture_with_id; @@ -129,3 +133,126 @@ async fn prefixes_context_and_instructions_once_and_consistently_across_requests ); assert_eq!(body2["input"], expected_body2); } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() { + use pretty_assertions::assert_eq; + + let server = MockServer::start().await; + + let sse = sse_completed("resp"); + let template = ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_raw(sse, "text/event-stream"); + + // Expect two POSTs to /v1/responses + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with(template) + .expect(2) + .mount(&server) + .await; + + let model_provider = ModelProviderInfo { + base_url: Some(format!("{}/v1", server.uri())), + ..built_in_model_providers()["openai"].clone() + }; + + let cwd = TempDir::new().unwrap(); + let codex_home = TempDir::new().unwrap(); + let mut config = load_default_config_for_test(&codex_home); + config.cwd = cwd.path().to_path_buf(); + config.model_provider = model_provider; + config.user_instructions = Some("be consistent and helpful".to_string()); + + let conversation_manager = ConversationManager::default(); + let codex = conversation_manager + .new_conversation_with_auth(config, Some(CodexAuth::from_api_key("Test API Key"))) + .await + .expect("create new conversation") + .conversation; + + // First turn + codex + .submit(Op::UserInput { + items: vec![InputItem::Text { + text: "hello 1".into(), + }], + }) + .await + .unwrap(); + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + // Change everything about the turn context. + let new_cwd = TempDir::new().unwrap(); + let writable = TempDir::new().unwrap(); + codex + .submit(Op::OverrideTurnContext { + cwd: Some(new_cwd.path().to_path_buf()), + approval_policy: Some(AskForApproval::Never), + sandbox_policy: Some(SandboxPolicy::WorkspaceWrite { + writable_roots: vec![writable.path().to_path_buf()], + network_access: true, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }), + model: Some("o3".to_string()), + effort: Some(ReasoningEffortConfig::High), + summary: Some(ReasoningSummaryConfig::Detailed), + }) + .await + .unwrap(); + + // Second turn after overrides + codex + .submit(Op::UserInput { + items: vec![InputItem::Text { + text: "hello 2".into(), + }], + }) + .await + .unwrap(); + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + // Verify we issued exactly two requests, and the cached prefix stayed identical. + let requests = server.received_requests().await.unwrap(); + assert_eq!(requests.len(), 2, "expected two POST requests"); + + let body1 = requests[0].body_json::().unwrap(); + let body2 = requests[1].body_json::().unwrap(); + + // prompt_cache_key should remain constant across overrides + assert_eq!( + body1["prompt_cache_key"], body2["prompt_cache_key"], + "prompt_cache_key should not change across overrides" + ); + + // The entire prefix from the first request should be identical and reused + // as the prefix of the second request, ensuring cache hit potential. + let expected_user_message_2 = serde_json::json!({ + "type": "message", + "id": serde_json::Value::Null, + "role": "user", + "content": [ { "type": "input_text", "text": "hello 2" } ] + }); + // After overriding the turn context, the environment context should be emitted again + // reflecting the new cwd, approval policy and sandbox settings. + let expected_env_text_2 = format!( + "\nCurrent working directory: {}\nApproval policy: never\nSandbox mode: workspace-write\nNetwork access: enabled\n", + new_cwd.path().to_string_lossy() + ); + let expected_env_msg_2 = serde_json::json!({ + "type": "message", + "id": serde_json::Value::Null, + "role": "user", + "content": [ { "type": "input_text", "text": expected_env_text_2 } ] + }); + let expected_body2 = serde_json::json!( + [ + body1["input"].as_array().unwrap().as_slice(), + [expected_env_msg_2, expected_user_message_2].as_slice(), + ] + .concat() + ); + assert_eq!(body2["input"], expected_body2); +} diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 4b9a290206..2aea218905 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -75,6 +75,38 @@ pub enum Op { summary: ReasoningSummaryConfig, }, + /// Override parts of the persistent turn context for subsequent turns. + /// + /// All fields are optional; when omitted, the existing value is preserved. + /// This does not enqueue any input – it only updates defaults used for + /// future `UserInput` turns. + OverrideTurnContext { + /// Updated `cwd` for sandbox/tool calls. + #[serde(skip_serializing_if = "Option::is_none")] + cwd: Option, + + /// Updated command approval policy. + #[serde(skip_serializing_if = "Option::is_none")] + approval_policy: Option, + + /// Updated sandbox policy for tool calls. + #[serde(skip_serializing_if = "Option::is_none")] + sandbox_policy: Option, + + /// Updated model slug. When set, the model family is derived + /// automatically. + #[serde(skip_serializing_if = "Option::is_none")] + model: Option, + + /// Updated reasoning effort (honored only for reasoning-capable models). + #[serde(skip_serializing_if = "Option::is_none")] + effort: Option, + + /// Updated reasoning summary preference (honored only for reasoning-capable models). + #[serde(skip_serializing_if = "Option::is_none")] + summary: Option, + }, + /// Approve a command execution ExecApproval { /// The id of the submission we are approving From fc6cfd5ecc946299a6f3011d966e1c0e0f5a380e Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Aug 2025 13:08:53 -0700 Subject: [PATCH 02/14] protocol-ts (#2425) --- codex-rs/Cargo.lock | 13 ++++ codex-rs/Cargo.toml | 1 + codex-rs/cli/Cargo.toml | 1 + codex-rs/cli/src/main.rs | 18 +++++ codex-rs/protocol-ts/Cargo.toml | 21 +++++ codex-rs/protocol-ts/generate-ts | 10 +++ codex-rs/protocol-ts/src/lib.rs | 108 ++++++++++++++++++++++++++ codex-rs/protocol-ts/src/main.rs | 20 +++++ codex-rs/protocol/Cargo.toml | 1 + codex-rs/protocol/src/config_types.rs | 7 +- codex-rs/protocol/src/mcp_protocol.rs | 51 ++++++------ codex-rs/protocol/src/protocol.rs | 11 +-- 12 files changed, 229 insertions(+), 33 deletions(-) create mode 100644 codex-rs/protocol-ts/Cargo.toml create mode 100755 codex-rs/protocol-ts/generate-ts create mode 100644 codex-rs/protocol-ts/src/lib.rs create mode 100644 codex-rs/protocol-ts/src/main.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 639a946752..55f754e4aa 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -666,6 +666,7 @@ dependencies = [ "codex-login", "codex-mcp-server", "codex-protocol", + "codex-protocol-ts", "codex-tui", "serde_json", "tokio", @@ -906,9 +907,20 @@ dependencies = [ "serde_json", "strum 0.27.2", "strum_macros 0.27.2", + "ts-rs", "uuid", ] +[[package]] +name = "codex-protocol-ts" +version = "0.0.0" +dependencies = [ + "anyhow", + "clap", + "codex-protocol", + "ts-rs", +] + [[package]] name = "codex-tui" version = "0.0.0" @@ -5225,6 +5237,7 @@ dependencies = [ "serde_json", "thiserror 2.0.12", "ts-rs-macros", + "uuid", ] [[package]] diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 2fb9b9271e..8a48ef8187 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -16,6 +16,7 @@ members = [ "mcp-types", "ollama", "protocol", + "protocol-ts", "tui", ] resolver = "2" diff --git a/codex-rs/cli/Cargo.toml b/codex-rs/cli/Cargo.toml index 0183ae28d8..f7af3349e0 100644 --- a/codex-rs/cli/Cargo.toml +++ b/codex-rs/cli/Cargo.toml @@ -37,3 +37,4 @@ tokio = { version = "1", features = [ ] } tracing = "0.1.41" tracing-subscriber = "0.3.19" +codex-protocol-ts = { path = "../protocol-ts" } diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 8f59d2d401..d237fe6729 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -72,6 +72,10 @@ enum Subcommand { /// Apply the latest diff produced by Codex agent as a `git apply` to your local working tree. #[clap(visible_alias = "a")] Apply(ApplyCommand), + + /// Internal: generate TypeScript protocol bindings. + #[clap(hide = true)] + GenerateTs(GenerateTsCommand), } #[derive(Debug, Parser)] @@ -120,6 +124,17 @@ struct LogoutCommand { config_overrides: CliConfigOverrides, } +#[derive(Debug, Parser)] +struct GenerateTsCommand { + /// Output directory where .ts files will be written + #[arg(short = 'o', long = "out", value_name = "DIR")] + out_dir: PathBuf, + + /// Optional path to the Prettier executable to format generated files + #[arg(short = 'p', long = "prettier", value_name = "PRETTIER_BIN")] + prettier: Option, +} + fn main() -> anyhow::Result<()> { arg0_dispatch_or_else(|codex_linux_sandbox_exe| async move { cli_main(codex_linux_sandbox_exe).await?; @@ -194,6 +209,9 @@ async fn cli_main(codex_linux_sandbox_exe: Option) -> anyhow::Result<() prepend_config_flags(&mut apply_cli.config_overrides, cli.config_overrides); run_apply_command(apply_cli, None).await?; } + Some(Subcommand::GenerateTs(gen_cli)) => { + codex_protocol_ts::generate_ts(&gen_cli.out_dir, gen_cli.prettier.as_deref())?; + } } Ok(()) diff --git a/codex-rs/protocol-ts/Cargo.toml b/codex-rs/protocol-ts/Cargo.toml new file mode 100644 index 0000000000..9faa9344e9 --- /dev/null +++ b/codex-rs/protocol-ts/Cargo.toml @@ -0,0 +1,21 @@ +[package] +edition = "2024" +name = "codex-protocol-ts" +version = { workspace = true } + +[lints] +workspace = true + +[lib] +name = "codex_protocol_ts" +path = "src/lib.rs" + +[[bin]] +name = "codex-protocol-ts" +path = "src/main.rs" + +[dependencies] +anyhow = "1" +codex-protocol = { path = "../protocol" } +ts-rs = "11" +clap = { version = "4", features = ["derive"] } diff --git a/codex-rs/protocol-ts/generate-ts b/codex-rs/protocol-ts/generate-ts new file mode 100755 index 0000000000..8f90bced6b --- /dev/null +++ b/codex-rs/protocol-ts/generate-ts @@ -0,0 +1,10 @@ +#!/bin/bash + +set -euo pipefail + +cd "$(dirname "$0")"/.. + +tmpdir=$(mktemp -d) +just codex generate-ts --prettier ../node_modules/.bin/prettier --out "$tmpdir" + +echo "wrote output to $tmpdir" diff --git a/codex-rs/protocol-ts/src/lib.rs b/codex-rs/protocol-ts/src/lib.rs new file mode 100644 index 0000000000..a37130b83b --- /dev/null +++ b/codex-rs/protocol-ts/src/lib.rs @@ -0,0 +1,108 @@ +use anyhow::Context; +use anyhow::Result; +use anyhow::anyhow; +use std::ffi::OsStr; +use std::fs; +use std::io::Read; +use std::io::Write; +use std::path::Path; +use std::path::PathBuf; +use std::process::Command; +use ts_rs::TS; + +const HEADER: &str = "// GENERATED CODE! DO NOT MODIFY BY HAND!\n\n"; + +pub fn generate_ts(out_dir: &Path, prettier: Option<&Path>) -> Result<()> { + ensure_dir(out_dir)?; + + // Generate TS bindings + codex_protocol::mcp_protocol::ConversationId::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::InputItem::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::ClientRequest::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::ServerRequest::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::NewConversationParams::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::NewConversationResponse::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::AddConversationListenerParams::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::AddConversationSubscriptionResponse::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::RemoveConversationListenerParams::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::RemoveConversationSubscriptionResponse::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::SendUserMessageParams::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::SendUserMessageResponse::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::SendUserTurnParams::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::SendUserTurnResponse::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::InterruptConversationParams::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::InterruptConversationResponse::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::LoginChatGptResponse::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::LoginChatGptCompleteNotification::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::CancelLoginChatGptParams::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::CancelLoginChatGptResponse::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::ApplyPatchApprovalParams::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::ApplyPatchApprovalResponse::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::ExecCommandApprovalParams::export_all_to(out_dir)?; + codex_protocol::mcp_protocol::ExecCommandApprovalResponse::export_all_to(out_dir)?; + + // Prepend header to each generated .ts file + let ts_files = ts_files_in(out_dir)?; + for file in &ts_files { + prepend_header_if_missing(file)?; + } + + // Format with Prettier by passing individual files (no shell globbing) + if let Some(prettier_bin) = prettier { + if !ts_files.is_empty() { + let status = Command::new(prettier_bin) + .arg("--write") + .args(ts_files.iter().map(|p| p.as_os_str())) + .status() + .with_context(|| { + format!("Failed to invoke Prettier at {}", prettier_bin.display()) + })?; + if !status.success() { + return Err(anyhow!("Prettier failed with status {}", status)); + } + } + } + + Ok(()) +} + +fn ensure_dir(dir: &Path) -> Result<()> { + fs::create_dir_all(dir) + .with_context(|| format!("Failed to create output directory {}", dir.display())) +} + +fn prepend_header_if_missing(path: &Path) -> Result<()> { + let mut content = String::new(); + { + let mut f = fs::File::open(path) + .with_context(|| format!("Failed to open {} for reading", path.display()))?; + f.read_to_string(&mut content) + .with_context(|| format!("Failed to read {}", path.display()))?; + } + + if content.starts_with(HEADER) { + return Ok(()); + } + + let mut f = fs::File::create(path) + .with_context(|| format!("Failed to open {} for writing", path.display()))?; + f.write_all(HEADER.as_bytes()) + .with_context(|| format!("Failed to write header to {}", path.display()))?; + f.write_all(content.as_bytes()) + .with_context(|| format!("Failed to write content to {}", path.display()))?; + Ok(()) +} + +fn ts_files_in(dir: &Path) -> Result> { + let mut files = Vec::new(); + for entry in + fs::read_dir(dir).with_context(|| format!("Failed to read dir {}", dir.display()))? + { + let entry = entry?; + let path = entry.path(); + if path.is_file() && path.extension() == Some(OsStr::new("ts")) { + files.push(path); + } + } + Ok(files) +} diff --git a/codex-rs/protocol-ts/src/main.rs b/codex-rs/protocol-ts/src/main.rs new file mode 100644 index 0000000000..f477b9f5a6 --- /dev/null +++ b/codex-rs/protocol-ts/src/main.rs @@ -0,0 +1,20 @@ +use anyhow::Result; +use clap::Parser; +use std::path::PathBuf; + +#[derive(Parser, Debug)] +#[command(about = "Generate TypeScript bindings for the Codex protocol")] +struct Args { + /// Output directory where .ts files will be written + #[arg(short = 'o', long = "out", value_name = "DIR")] + out_dir: PathBuf, + + /// Optional path to the Prettier executable to format generated files + #[arg(short = 'p', long = "prettier", value_name = "PRETTIER_BIN")] + prettier: Option, +} + +fn main() -> Result<()> { + let args = Args::parse(); + codex_protocol_ts::generate_ts(&args.out_dir, args.prettier.as_deref()) +} diff --git a/codex-rs/protocol/Cargo.toml b/codex-rs/protocol/Cargo.toml index 9525d0439f..c94bdb8e1f 100644 --- a/codex-rs/protocol/Cargo.toml +++ b/codex-rs/protocol/Cargo.toml @@ -17,6 +17,7 @@ serde_bytes = "0.11" serde_json = "1" strum = "0.27.2" strum_macros = "0.27.2" +ts-rs = { version = "11", features = ["uuid-impl", "serde-json-impl"] } uuid = { version = "1", features = ["serde", "v4"] } [dev-dependencies] diff --git a/codex-rs/protocol/src/config_types.rs b/codex-rs/protocol/src/config_types.rs index 4d72e27af8..1c88e9cbdd 100644 --- a/codex-rs/protocol/src/config_types.rs +++ b/codex-rs/protocol/src/config_types.rs @@ -1,9 +1,10 @@ use serde::Deserialize; use serde::Serialize; use strum_macros::Display; +use ts_rs::TS; /// See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning -#[derive(Debug, Serialize, Deserialize, Default, Clone, Copy, PartialEq, Eq, Display)] +#[derive(Debug, Serialize, Deserialize, Default, Clone, Copy, PartialEq, Eq, Display, TS)] #[serde(rename_all = "lowercase")] #[strum(serialize_all = "lowercase")] pub enum ReasoningEffort { @@ -19,7 +20,7 @@ pub enum ReasoningEffort { /// A summary of the reasoning performed by the model. This can be useful for /// debugging and understanding the model's reasoning process. /// See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#reasoning-summaries -#[derive(Debug, Serialize, Deserialize, Default, Clone, Copy, PartialEq, Eq, Display)] +#[derive(Debug, Serialize, Deserialize, Default, Clone, Copy, PartialEq, Eq, Display, TS)] #[serde(rename_all = "lowercase")] #[strum(serialize_all = "lowercase")] pub enum ReasoningSummary { @@ -31,7 +32,7 @@ pub enum ReasoningSummary { None, } -#[derive(Deserialize, Debug, Clone, Copy, PartialEq, Default, Serialize, Display)] +#[derive(Deserialize, Debug, Clone, Copy, PartialEq, Default, Serialize, Display, TS)] #[serde(rename_all = "kebab-case")] #[strum(serialize_all = "kebab-case")] pub enum SandboxMode { diff --git a/codex-rs/protocol/src/mcp_protocol.rs b/codex-rs/protocol/src/mcp_protocol.rs index 5110f46976..383b2033d7 100644 --- a/codex-rs/protocol/src/mcp_protocol.rs +++ b/codex-rs/protocol/src/mcp_protocol.rs @@ -13,10 +13,11 @@ use crate::protocol::TurnAbortReason; use mcp_types::RequestId; use serde::Deserialize; use serde::Serialize; +use ts_rs::TS; use uuid::Uuid; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -#[serde(transparent)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, TS)] +#[ts(type = "string")] pub struct ConversationId(pub Uuid); impl Display for ConversationId { @@ -26,7 +27,7 @@ impl Display for ConversationId { } /// Request from the client to the server. -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(tag = "method", rename_all = "camelCase")] pub enum ClientRequest { NewConversation { @@ -70,7 +71,7 @@ pub enum ClientRequest { }, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, TS)] #[serde(rename_all = "camelCase")] pub struct NewConversationParams { /// Optional override for the model name (e.g. "o3", "o4-mini"). @@ -113,24 +114,24 @@ pub struct NewConversationParams { pub include_apply_patch_tool: Option, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(rename_all = "camelCase")] pub struct NewConversationResponse { pub conversation_id: ConversationId, pub model: String, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(rename_all = "camelCase")] pub struct AddConversationSubscriptionResponse { pub subscription_id: Uuid, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(rename_all = "camelCase")] pub struct RemoveConversationSubscriptionResponse {} -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(rename_all = "camelCase")] pub struct LoginChatGptResponse { pub login_id: Uuid, @@ -141,7 +142,7 @@ pub struct LoginChatGptResponse { // Event name for notifying client of login completion or failure. pub const LOGIN_CHATGPT_COMPLETE_EVENT: &str = "codex/event/login_chatgpt_complete"; -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(rename_all = "camelCase")] pub struct LoginChatGptCompleteNotification { pub login_id: Uuid, @@ -150,24 +151,24 @@ pub struct LoginChatGptCompleteNotification { pub error: Option, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(rename_all = "camelCase")] pub struct CancelLoginChatGptParams { pub login_id: Uuid, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(rename_all = "camelCase")] pub struct CancelLoginChatGptResponse {} -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(rename_all = "camelCase")] pub struct SendUserMessageParams { pub conversation_id: ConversationId, pub items: Vec, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(rename_all = "camelCase")] pub struct SendUserTurnParams { pub conversation_id: ConversationId, @@ -180,39 +181,39 @@ pub struct SendUserTurnParams { pub summary: ReasoningSummary, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(rename_all = "camelCase")] pub struct SendUserTurnResponse {} -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(rename_all = "camelCase")] pub struct InterruptConversationParams { pub conversation_id: ConversationId, } -#[derive(Serialize, Deserialize, Debug, Clone)] +#[derive(Serialize, Deserialize, Debug, Clone, TS)] #[serde(rename_all = "camelCase")] pub struct InterruptConversationResponse { pub abort_reason: TurnAbortReason, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(rename_all = "camelCase")] pub struct SendUserMessageResponse {} -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(rename_all = "camelCase")] pub struct AddConversationListenerParams { pub conversation_id: ConversationId, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(rename_all = "camelCase")] pub struct RemoveConversationListenerParams { pub subscription_id: Uuid, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(rename_all = "camelCase")] #[serde(tag = "type", content = "data")] pub enum InputItem { @@ -237,7 +238,7 @@ pub const APPLY_PATCH_APPROVAL_METHOD: &str = "applyPatchApproval"; pub const EXEC_COMMAND_APPROVAL_METHOD: &str = "execCommandApproval"; /// Request initiated from the server and sent to the client. -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[serde(tag = "method", rename_all = "camelCase")] pub enum ServerRequest { /// Request to approve a patch. @@ -254,7 +255,7 @@ pub enum ServerRequest { }, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] pub struct ApplyPatchApprovalParams { pub conversation_id: ConversationId, /// Use to correlate this with [codex_core::protocol::PatchApplyBeginEvent] @@ -270,7 +271,7 @@ pub struct ApplyPatchApprovalParams { pub grant_root: Option, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] pub struct ExecCommandApprovalParams { pub conversation_id: ConversationId, /// Use to correlate this with [codex_core::protocol::ExecCommandBeginEvent] @@ -282,12 +283,12 @@ pub struct ExecCommandApprovalParams { pub reason: Option, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] pub struct ExecCommandApprovalResponse { pub decision: ReviewDecision, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] pub struct ApplyPatchApprovalResponse { pub decision: ReviewDecision, } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 2aea218905..23ef4668f1 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -15,6 +15,7 @@ use serde::Deserialize; use serde::Serialize; use serde_bytes::ByteBuf; use strum_macros::Display; +use ts_rs::TS; use uuid::Uuid; use crate::config_types::ReasoningEffort as ReasoningEffortConfig; @@ -145,7 +146,7 @@ pub enum Op { /// Determines the conditions under which the user is consulted to approve /// running the command proposed by Codex. -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize, Deserialize, Display)] +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize, Deserialize, Display, TS)] #[serde(rename_all = "kebab-case")] #[strum(serialize_all = "kebab-case")] pub enum AskForApproval { @@ -172,7 +173,7 @@ pub enum AskForApproval { } /// Determines execution restrictions for model shell commands. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Display)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Display, TS)] #[strum(serialize_all = "kebab-case")] #[serde(tag = "mode", rename_all = "kebab-case")] pub enum SandboxPolicy { @@ -737,7 +738,7 @@ pub struct SessionConfiguredEvent { } /// User's decision in response to an ExecApprovalRequest. -#[derive(Debug, Default, Clone, Copy, Deserialize, Serialize, PartialEq, Eq)] +#[derive(Debug, Default, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, TS)] #[serde(rename_all = "snake_case")] pub enum ReviewDecision { /// User has approved this command and the agent should execute it. @@ -758,7 +759,7 @@ pub enum ReviewDecision { Abort, } -#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)] +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, TS)] #[serde(rename_all = "snake_case")] pub enum FileChange { Add { @@ -784,7 +785,7 @@ pub struct TurnAbortedEvent { pub reason: TurnAbortReason, } -#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)] +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, TS)] #[serde(rename_all = "snake_case")] pub enum TurnAbortReason { Interrupted, From ecb388045c5d542de3b124c3fc255a78e74c2a84 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 18 Aug 2025 14:28:09 -0700 Subject: [PATCH 03/14] Add cache tests for UserTurn (#2432) --- codex-rs/core/tests/prompt_caching.rs | 112 +++++++++++++++++++++++++- 1 file changed, 108 insertions(+), 4 deletions(-) diff --git a/codex-rs/core/tests/prompt_caching.rs b/codex-rs/core/tests/prompt_caching.rs index e528cb7a64..9f5829e113 100644 --- a/codex-rs/core/tests/prompt_caching.rs +++ b/codex-rs/core/tests/prompt_caching.rs @@ -6,8 +6,8 @@ use codex_core::protocol::EventMsg; use codex_core::protocol::InputItem; use codex_core::protocol::Op; use codex_core::protocol::SandboxPolicy; -use codex_core::protocol_config_types::ReasoningEffort as ReasoningEffortConfig; -use codex_core::protocol_config_types::ReasoningSummary as ReasoningSummaryConfig; +use codex_core::protocol_config_types::ReasoningEffort; +use codex_core::protocol_config_types::ReasoningSummary; use codex_login::CodexAuth; use core_test_support::load_default_config_for_test; use core_test_support::load_sse_fixture_with_id; @@ -197,8 +197,8 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() { exclude_slash_tmp: true, }), model: Some("o3".to_string()), - effort: Some(ReasoningEffortConfig::High), - summary: Some(ReasoningSummaryConfig::Detailed), + effort: Some(ReasoningEffort::High), + summary: Some(ReasoningSummary::Detailed), }) .await .unwrap(); @@ -256,3 +256,107 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() { ); assert_eq!(body2["input"], expected_body2); } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn per_turn_overrides_keep_cached_prefix_and_key_constant() { + use pretty_assertions::assert_eq; + + let server = MockServer::start().await; + + let sse = sse_completed("resp"); + let template = ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_raw(sse, "text/event-stream"); + + // Expect two POSTs to /v1/responses + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with(template) + .expect(2) + .mount(&server) + .await; + + let model_provider = ModelProviderInfo { + base_url: Some(format!("{}/v1", server.uri())), + ..built_in_model_providers()["openai"].clone() + }; + + let cwd = TempDir::new().unwrap(); + let codex_home = TempDir::new().unwrap(); + let mut config = load_default_config_for_test(&codex_home); + config.cwd = cwd.path().to_path_buf(); + config.model_provider = model_provider; + config.user_instructions = Some("be consistent and helpful".to_string()); + + let conversation_manager = ConversationManager::default(); + let codex = conversation_manager + .new_conversation_with_auth(config, Some(CodexAuth::from_api_key("Test API Key"))) + .await + .expect("create new conversation") + .conversation; + + // First turn + codex + .submit(Op::UserInput { + items: vec![InputItem::Text { + text: "hello 1".into(), + }], + }) + .await + .unwrap(); + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + // Second turn using per-turn overrides via UserTurn + let new_cwd = TempDir::new().unwrap(); + let writable = TempDir::new().unwrap(); + codex + .submit(Op::UserTurn { + items: vec![InputItem::Text { + text: "hello 2".into(), + }], + cwd: new_cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::WorkspaceWrite { + writable_roots: vec![writable.path().to_path_buf()], + network_access: true, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }, + model: "o3".to_string(), + effort: ReasoningEffort::High, + summary: ReasoningSummary::Detailed, + }) + .await + .unwrap(); + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + // Verify we issued exactly two requests, and the cached prefix stayed identical. + let requests = server.received_requests().await.unwrap(); + assert_eq!(requests.len(), 2, "expected two POST requests"); + + let body1 = requests[0].body_json::().unwrap(); + let body2 = requests[1].body_json::().unwrap(); + + // prompt_cache_key should remain constant across per-turn overrides + assert_eq!( + body1["prompt_cache_key"], body2["prompt_cache_key"], + "prompt_cache_key should not change across per-turn overrides" + ); + + // The entire prefix from the first request should be identical and reused + // as the prefix of the second request. + let expected_user_message_2 = serde_json::json!({ + "type": "message", + "id": serde_json::Value::Null, + "role": "user", + "content": [ { "type": "input_text", "text": "hello 2" } ] + }); + let expected_body2 = serde_json::json!( + [ + body1["input"].as_array().unwrap().as_slice(), + [expected_user_message_2].as_slice(), + ] + .concat() + ); + assert_eq!(body2["input"], expected_body2); +} From db30a6f5d86650ae7c5a3d40975c7c4e9daf98af Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Tue, 19 Aug 2025 08:00:29 +0900 Subject: [PATCH 04/14] Fix #2391 Add Ctrl+H as backspace keyboard shortcut (#2412) This pull request resolves #2391. ctrl + h is not assigned to any other operations at this moment, and this feature request sounds valid to me. If we don't prefer having this, please feel free to close this. --- codex-rs/tui/src/bottom_pane/textarea.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index 33cdbbbc1d..029ffc8590 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -230,6 +230,11 @@ impl TextArea { code: KeyCode::Backspace, modifiers: KeyModifiers::NONE, .. + } + | KeyEvent { + code: KeyCode::Char('h'), + modifiers: KeyModifiers::CONTROL, + .. } => self.delete_backward(1), KeyEvent { code: KeyCode::Delete, From f9d3dde478d768dca306ea4bc488fb71838c8115 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 18 Aug 2025 16:01:58 -0700 Subject: [PATCH 05/14] chore(deps): bump anyhow from 1.0.98 to 1.0.99 in /codex-rs (#2405) Bumps [anyhow](https://github.com/dtolnay/anyhow) from 1.0.98 to 1.0.99.
Release notes

Sourced from anyhow's releases.

1.0.99

  • Allow build-script cleanup failure with NFSv3 output directory to be non-fatal (#420)
Commits
  • f2b963a Release 1.0.99
  • 2c64c15 Merge pull request #420 from dtolnay/enotempty
  • 8cf66f7 Allow build-script cleanup failure with NFSv3 output directory to be non-fatal
  • f5e145c Revert "Pin nightly toolchain used for miri job"
  • 1d7ef1d Update ui test suite to nightly-2025-06-30
  • 6929572 Update ui test suite to nightly-2025-06-18
  • 37224e3 Ignore mismatched_lifetime_syntaxes lint
  • 11f0e81 Pin nightly toolchain used for miri job
  • d04c999 Raise required compiler for backtrace feature to rust 1.82
  • 219d163 Update test suite to nightly-2025-05-01
  • See full diff in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=anyhow&package-manager=cargo&previous-version=1.0.98&new-version=1.0.99)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- codex-rs/Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 55f754e4aa..b9f33a45b4 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -176,9 +176,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.98" +version = "1.0.99" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e16d2d3311acee920a9eb8d33b8cbc1787ce4a264e85f964c2404b969bdcd487" +checksum = "b0674a1ddeecb70197781e945de4b3b8ffb61fa939a5597bcf48503737663100" [[package]] name = "arbitrary" From 52f0b95102622aef4bc6955663aff29e0b585b7a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 18 Aug 2025 16:17:29 -0700 Subject: [PATCH 06/14] chore(deps): bump libc from 0.2.174 to 0.2.175 in /codex-rs (#2406) Bumps [libc](https://github.com/rust-lang/libc) from 0.2.174 to 0.2.175.
Release notes

Sourced from libc's releases.

0.2.175

Added

  • AIX: Add getpeereid (#4524)
  • AIX: Add struct ld_info and friends (#4578)
  • AIX: Retore struct winsize (#4577)
  • Android: Add UDP socket option constants (#4619)
  • Android: Add CLONE_CLEAR_SIGHAND and CLONE_INTO_CGROUP (#4502)
  • Android: Add more prctl constants (#4531)
  • FreeBSD Add further TCP stack-related constants (#4196)
  • FreeBSD x86-64: Add mcontext_t.mc_tlsbase (#4503)
  • FreeBSD15: Add kinfo_proc.ki_uerrmsg (#4552)
  • FreeBSD: Add in_conninfo (#4482)
  • FreeBSD: Add xinpgen and related types (#4482)
  • FreeBSD: Add xktls_session (#4482)
  • Haiku: Add functionality from libbsd (#4221)
  • Linux: Add SECBIT_* (#4480)
  • NetBSD, OpenBSD: Export ioctl request generator macros (#4460)
  • NetBSD: Add ptsname_r (#4608)
  • RISCV32: Add time-related syscalls (#4612)
  • Solarish: Add strftime* (#4453)
  • linux: Add EXEC_RESTRICT_* and EXEC_DENY_* (#4545)

Changed

  • AIX: Add const to signatures to be consistent with other platforms (#4563)

Fixed

  • AIX: Fix the type of struct statvfs.f_fsid (#4576)
  • AIX: Fix the type of constants for the ioctl request argument (#4582)
  • AIX: Fix the types of stat{,64}.st_*tim (#4597)
  • AIX: Use unique errno values (#4507)
  • Build: Fix an incorrect target_os -> target_arch check (#4550)
  • FreeBSD: Fix the type of xktls_session_onedir.ifnet (#4552)
  • Mips64 musl: Fix the type of nlink_t (#4509)
  • Mips64 musl: Use a special MIPS definition of stack_t (#4528)
  • Mips64: Fix SI_TIMER, SI_MESGQ and SI_ASYNCIO definitions (#4529)
  • Musl Mips64: Swap the order of si_errno and si_code in siginfo_t (#4530)
  • Musl Mips64: Use a special MIPS definition of statfs (#4527)
  • Musl: Fix the definition of fanotify_event_metadata (#4510)
  • NetBSD: Correct enum fae_action to be #[repr(C)] (#60a8cfd5)
  • PSP: Correct char -> c_char (eaab4fc3)
  • PowerPC musl: Fix termios definitions (#4518)
  • PowerPC musl: Fix the definition of EDEADLK (#4517)
  • PowerPC musl: Fix the definition of NCCS (#4513)
  • PowerPC musl: Fix the definitions of MAP_LOCKED and MAP_NORESERVE (#4516)
  • PowerPC64 musl: Fix the definition of shmid_ds (#4519)

Deprecated

... (truncated)

Changelog

Sourced from libc's changelog.

0.2.175 - 2025-08-10

Added

  • AIX: Add getpeereid (#4524)
  • AIX: Add struct ld_info and friends (#4578)
  • AIX: Retore struct winsize (#4577)
  • Android: Add UDP socket option constants (#4619)
  • Android: Add CLONE_CLEAR_SIGHAND and CLONE_INTO_CGROUP (#4502)
  • Android: Add more prctl constants (#4531)
  • FreeBSD Add further TCP stack-related constants (#4196)
  • FreeBSD x86-64: Add mcontext_t.mc_tlsbase (#4503)
  • FreeBSD15: Add kinfo_proc.ki_uerrmsg (#4552)
  • FreeBSD: Add in_conninfo (#4482)
  • FreeBSD: Add xinpgen and related types (#4482)
  • FreeBSD: Add xktls_session (#4482)
  • Haiku: Add functionality from libbsd (#4221)
  • Linux: Add SECBIT_* (#4480)
  • NetBSD, OpenBSD: Export ioctl request generator macros (#4460)
  • NetBSD: Add ptsname_r (#4608)
  • RISCV32: Add time-related syscalls (#4612)
  • Solarish: Add strftime* (#4453)
  • linux: Add EXEC_RESTRICT_* and EXEC_DENY_* (#4545)

Changed

  • AIX: Add const to signatures to be consistent with other platforms (#4563)

Fixed

  • AIX: Fix the type of struct statvfs.f_fsid (#4576)
  • AIX: Fix the type of constants for the ioctl request argument (#4582)
  • AIX: Fix the types of stat{,64}.st_*tim (#4597)
  • AIX: Use unique errno values (#4507)
  • Build: Fix an incorrect target_os -> target_arch check (#4550)
  • FreeBSD: Fix the type of xktls_session_onedir.ifnet (#4552)
  • Mips64 musl: Fix the type of nlink_t (#4509)
  • Mips64 musl: Use a special MIPS definition of stack_t (#4528)
  • Mips64: Fix SI_TIMER, SI_MESGQ and SI_ASYNCIO definitions (#4529)
  • Musl Mips64: Swap the order of si_errno and si_code in siginfo_t (#4530)
  • Musl Mips64: Use a special MIPS definition of statfs (#4527)
  • Musl: Fix the definition of fanotify_event_metadata (#4510)
  • NetBSD: Correct enum fae_action to be #[repr(C)] (#60a8cfd5)
  • PSP: Correct char -> c_char (eaab4fc3)
  • PowerPC musl: Fix termios definitions (#4518)
  • PowerPC musl: Fix the definition of EDEADLK (#4517)
  • PowerPC musl: Fix the definition of NCCS (#4513)
  • PowerPC musl: Fix the definitions of MAP_LOCKED and MAP_NORESERVE (#4516)
  • PowerPC64 musl: Fix the definition of shmid_ds (#4519)

... (truncated)

Commits
  • 84e26e6 Update the lockfile
  • 4d04aee chore: release libc 0.2.175
  • 94a7f32 cleanup: Format a file that was missed
  • 1725273 Rename the ctest file from main to ctest
  • e9b021b freebsd adding further TCP stack related constants.
  • 9606a29 freebsd15: Add ki_uerrmsg to struct kinfo_proc
  • 2816bc2 libc-test: include sys/ktls.h on freebsd
  • adfe283 libc-test: Account for xktls_session_onedir::gen (freebsd)
  • 4cc1bf4 freebsd: Document avoidance of reserved name gen
  • 7cdcaa6 freebsd: Fix type of struct xktls_session_onedir, field ifnet
  • Additional commits viewable in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=libc&package-manager=cargo&previous-version=0.2.174&new-version=0.2.175)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- codex-rs/Cargo.lock | 4 ++-- codex-rs/core/Cargo.toml | 2 +- codex-rs/linux-sandbox/Cargo.toml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index b9f33a45b4..8d2d5f7c31 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2639,9 +2639,9 @@ checksum = "03087c2bad5e1034e8cace5926dec053fb3790248370865f5117a7d0213354c8" [[package]] name = "libc" -version = "0.2.174" +version = "0.2.175" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1171693293099992e19cddea4e8b849964e9846f4acee11b3948bcc337be8776" +checksum = "6a82ae493e598baaea5209805c49bbf2ea7de956d50d7da0da1164f9c6d28543" [[package]] name = "libfuzzer-sys" diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 74eaf6704b..78942ff69f 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -25,7 +25,7 @@ env-flags = "0.1.1" eventsource-stream = "0.2.3" fs2 = "0.4.3" futures = "0.3" -libc = "0.2.174" +libc = "0.2.175" mcp-types = { path = "../mcp-types" } mime_guess = "2.0" os_info = "3.12.0" diff --git a/codex-rs/linux-sandbox/Cargo.toml b/codex-rs/linux-sandbox/Cargo.toml index ea7052c409..d769fae2c6 100644 --- a/codex-rs/linux-sandbox/Cargo.toml +++ b/codex-rs/linux-sandbox/Cargo.toml @@ -20,7 +20,7 @@ clap = { version = "4", features = ["derive"] } codex-common = { path = "../common", features = ["cli"] } codex-core = { path = "../core" } landlock = "0.4.1" -libc = "0.2.172" +libc = "0.2.175" seccompiler = "0.5.0" [target.'cfg(target_os = "linux")'.dev-dependencies] From 37e5b087a7f7faeec09f8be4c1153ae28bf91279 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Aug 2025 16:37:07 -0700 Subject: [PATCH 07/14] chore: prefer returning Err to expect() (#2389) Letting the caller deal with `Err` seems preferable to using `expect()` (which would `panic!()`), particularly given that the function already returns `Result`. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2389). * #2399 * #2398 * #2396 * #2395 * #2394 * #2393 * __->__ #2389 --- codex-rs/login/src/server.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index ef85df69ea..19ef4c1cfe 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -59,10 +59,9 @@ pub struct LoginServer { impl LoginServer { pub fn block_until_done(self) -> io::Result<()> { - #[expect(clippy::expect_used)] self.server_handle .join() - .expect("can't join on the server thread") + .map_err(|err| io::Error::other(format!("login server thread panicked: {err:?}")))? } pub fn cancel(&self) { From 6e8c055fd50e9b88f00b77891cde185ed23c2676 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Aug 2025 17:23:40 -0700 Subject: [PATCH 08/14] fix: async-ify login flow (#2393) This replaces blocking I/O with async/non-blocking I/O in a number of cases. This facilitates the use of `tokio::sync::Notify` and `tokio::select!` in #2394. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2393). * #2399 * #2398 * #2396 * #2395 * #2394 * __->__ #2393 * #2389 --- codex-rs/cli/src/login.rs | 2 +- codex-rs/login/src/server.rs | 155 ++++++++++-------- codex-rs/login/tests/login_server_e2e.rs | 20 +-- .../mcp-server/src/codex_message_processor.rs | 12 +- codex-rs/tui/src/onboarding/auth.rs | 36 ++-- 5 files changed, 126 insertions(+), 99 deletions(-) diff --git a/codex-rs/cli/src/login.rs b/codex-rs/cli/src/login.rs index 5f9dc5f908..fc40a0271f 100644 --- a/codex-rs/cli/src/login.rs +++ b/codex-rs/cli/src/login.rs @@ -21,7 +21,7 @@ pub async fn login_with_chatgpt(codex_home: PathBuf) -> std::io::Result<()> { server.actual_port, server.auth_url, ); - server.block_until_done() + server.block_until_done().await } pub async fn run_login_with_chatgpt(cli_config_overrides: CliConfigOverrides) -> ! { diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index 19ef4c1cfe..060b333c6e 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -52,15 +52,15 @@ impl ServerOptions { pub struct LoginServer { pub auth_url: String, pub actual_port: u16, - pub server_handle: thread::JoinHandle>, pub shutdown_flag: Arc, - pub server: Arc, + server_handle: tokio::task::JoinHandle>, + server: Arc, } impl LoginServer { - pub fn block_until_done(self) -> io::Result<()> { + pub async fn block_until_done(self) -> io::Result<()> { self.server_handle - .join() + .await .map_err(|err| io::Error::other(format!("login server thread panicked: {err:?}")))? } @@ -118,7 +118,8 @@ pub fn run_login_server( if opts.open_browser { let _ = webbrowser::open(&auth_url); } - let shutdown_flag = shutdown_flag.unwrap_or_else(|| Arc::new(AtomicBool::new(false))); + let shutdown_flag: Arc = + shutdown_flag.unwrap_or_else(|| Arc::new(AtomicBool::new(false))); let shutdown_flag_clone = shutdown_flag.clone(); let timeout_flag = Arc::new(AtomicBool::new(false)); @@ -135,31 +136,46 @@ pub fn run_login_server( ); } - let server_for_thread = server.clone(); - let server_handle = thread::spawn(move || { - while !shutdown_flag.load(Ordering::SeqCst) { - let req = match server_for_thread.recv() { - Ok(r) => r, - Err(e) => { - // If we've been asked to shut down, break gracefully so that - // we can report timeout or cancellation status uniformly. - if shutdown_flag.load(Ordering::SeqCst) { - break; - } else { - return Err(io::Error::other(e)); + let (tx, mut rx) = tokio::sync::mpsc::channel::(16); + let _server_handle = { + let server = server.clone(); + let shutdown_flag = shutdown_flag.clone(); + thread::spawn(move || { + while !shutdown_flag.load(Ordering::SeqCst) { + match server.recv() { + Ok(request) => tx.blocking_send(request).map_err(|e| { + eprintln!("Failed to send request to channel: {e}"); + io::Error::other("Failed to send request to channel") + })?, + Err(e) => { + // If we've been asked to shut down, break gracefully so that + // we can report timeout or cancellation status uniformly. + if shutdown_flag.load(Ordering::SeqCst) { + break; + } else { + return Err(io::Error::other(e)); + } } - } - }; + }; + } + Ok(()) + }) + }; + + let server_handle = tokio::spawn(async move { + while let Some(req) = rx.recv().await { + let url_raw = req.url().to_string(); + let response = + process_request(&url_raw, &opts, &redirect_uri, &pkce, actual_port, &state).await; - let response = process_request(&req, &opts, &redirect_uri, &pkce, actual_port, &state); let is_login_complete = matches!(response, HandledRequest::ResponseAndExit(_)); match response { HandledRequest::Response(r) | HandledRequest::ResponseAndExit(r) => { - let _ = req.respond(r); + let _ = tokio::task::spawn_blocking(move || req.respond(r)).await; } HandledRequest::RedirectWithHeader(header) => { let redirect = Response::empty(302).with_header(header); - let _ = req.respond(redirect); + let _ = tokio::task::spawn_blocking(move || req.respond(redirect)).await; } } @@ -196,15 +212,14 @@ enum HandledRequest { ResponseAndExit(Response>>), } -fn process_request( - req: &Request, +async fn process_request( + url_raw: &str, opts: &ServerOptions, redirect_uri: &str, pkce: &PkceCodes, actual_port: u16, state: &str, ) -> HandledRequest { - let url_raw = req.url().to_string(); let parsed_url = match url::Url::parse(&format!("http://localhost{url_raw}")) { Ok(u) => u, Err(e) => { @@ -235,18 +250,22 @@ fn process_request( }; match exchange_code_for_tokens(&opts.issuer, &opts.client_id, redirect_uri, pkce, &code) + .await { Ok(tokens) => { // Obtain API key via token-exchange and persist - let api_key = - obtain_api_key(&opts.issuer, &opts.client_id, &tokens.id_token).ok(); - if let Err(err) = persist_tokens( + let api_key = obtain_api_key(&opts.issuer, &opts.client_id, &tokens.id_token) + .await + .ok(); + if let Err(err) = persist_tokens_async( &opts.codex_home, api_key.clone(), tokens.id_token.clone(), Some(tokens.access_token.clone()), Some(tokens.refresh_token.clone()), - ) { + ) + .await + { eprintln!("Persist error: {err}"); return HandledRequest::Response( Response::from_string(format!("Unable to persist auth file: {err}")) @@ -352,7 +371,7 @@ struct ExchangedTokens { refresh_token: String, } -fn exchange_code_for_tokens( +async fn exchange_code_for_tokens( issuer: &str, client_id: &str, redirect_uri: &str, @@ -366,7 +385,7 @@ fn exchange_code_for_tokens( refresh_token: String, } - let client = reqwest::blocking::Client::new(); + let client = reqwest::Client::new(); let resp = client .post(format!("{issuer}/oauth/token")) .header("Content-Type", "application/x-www-form-urlencoded") @@ -378,6 +397,7 @@ fn exchange_code_for_tokens( urlencoding::encode(&pkce.code_verifier) )) .send() + .await .map_err(io::Error::other)?; if !resp.status().is_success() { @@ -387,7 +407,7 @@ fn exchange_code_for_tokens( ))); } - let tokens: TokenResponse = resp.json().map_err(io::Error::other)?; + let tokens: TokenResponse = resp.json().await.map_err(io::Error::other)?; Ok(ExchangedTokens { id_token: tokens.id_token, access_token: tokens.access_token, @@ -395,43 +415,49 @@ fn exchange_code_for_tokens( }) } -fn persist_tokens( +async fn persist_tokens_async( codex_home: &Path, api_key: Option, id_token: String, access_token: Option, refresh_token: Option, ) -> io::Result<()> { - let auth_file = get_auth_file(codex_home); - if let Some(parent) = auth_file.parent() { - if !parent.exists() { - std::fs::create_dir_all(parent).map_err(io::Error::other)?; + // Reuse existing synchronous logic but run it off the async runtime. + let codex_home = codex_home.to_path_buf(); + tokio::task::spawn_blocking(move || { + let auth_file = get_auth_file(&codex_home); + if let Some(parent) = auth_file.parent() { + if !parent.exists() { + std::fs::create_dir_all(parent).map_err(io::Error::other)?; + } } - } - let mut auth = read_or_default(&auth_file); - if let Some(key) = api_key { - auth.openai_api_key = Some(key); - } - let tokens = auth - .tokens - .get_or_insert_with(crate::token_data::TokenData::default); - tokens.id_token = crate::token_data::parse_id_token(&id_token).map_err(io::Error::other)?; - // Persist chatgpt_account_id if present in claims - if let Some(acc) = jwt_auth_claims(&id_token) - .get("chatgpt_account_id") - .and_then(|v| v.as_str()) - { - tokens.account_id = Some(acc.to_string()); - } - if let Some(at) = access_token { - tokens.access_token = at; - } - if let Some(rt) = refresh_token { - tokens.refresh_token = rt; - } - auth.last_refresh = Some(Utc::now()); - super::write_auth_json(&auth_file, &auth) + let mut auth = read_or_default(&auth_file); + if let Some(key) = api_key { + auth.openai_api_key = Some(key); + } + let tokens = auth + .tokens + .get_or_insert_with(crate::token_data::TokenData::default); + tokens.id_token = crate::token_data::parse_id_token(&id_token).map_err(io::Error::other)?; + // Persist chatgpt_account_id if present in claims + if let Some(acc) = jwt_auth_claims(&id_token) + .get("chatgpt_account_id") + .and_then(|v| v.as_str()) + { + tokens.account_id = Some(acc.to_string()); + } + if let Some(at) = access_token { + tokens.access_token = at; + } + if let Some(rt) = refresh_token { + tokens.refresh_token = rt; + } + auth.last_refresh = Some(Utc::now()); + super::write_auth_json(&auth_file, &auth) + }) + .await + .map_err(|e| io::Error::other(format!("persist task failed: {e}")))? } fn read_or_default(path: &Path) -> AuthDotJson { @@ -524,13 +550,13 @@ fn jwt_auth_claims(jwt: &str) -> serde_json::Map { serde_json::Map::new() } -fn obtain_api_key(issuer: &str, client_id: &str, id_token: &str) -> io::Result { +async fn obtain_api_key(issuer: &str, client_id: &str, id_token: &str) -> io::Result { // Token exchange for an API key access token #[derive(serde::Deserialize)] struct ExchangeResp { access_token: String, } - let client = reqwest::blocking::Client::new(); + let client = reqwest::Client::new(); let resp = client .post(format!("{issuer}/oauth/token")) .header("Content-Type", "application/x-www-form-urlencoded") @@ -543,6 +569,7 @@ fn obtain_api_key(issuer: &str, client_id: &str, id_token: &str) -> io::Result io::Result (SocketAddr, thread::JoinHandle<()>) { (addr, handle) } -#[test] -fn end_to_end_login_flow_persists_auth_json() { +#[tokio::test] +async fn end_to_end_login_flow_persists_auth_json() { if std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() { println!( "Skipping test because it cannot execute when network is disabled in a Codex sandbox." @@ -106,16 +106,16 @@ fn end_to_end_login_flow_persists_auth_json() { let login_port = server.actual_port; // Simulate browser callback, and follow redirect to /success - let client = reqwest::blocking::Client::builder() + let client = reqwest::Client::builder() .redirect(reqwest::redirect::Policy::limited(5)) .build() .unwrap(); let url = format!("http://127.0.0.1:{login_port}/auth/callback?code=abc&state=test_state_123"); - let resp = client.get(&url).send().unwrap(); + let resp = client.get(&url).send().await.unwrap(); assert!(resp.status().is_success()); // Wait for server shutdown - server.block_until_done().unwrap(); + server.block_until_done().await.unwrap(); // Validate auth.json let auth_path = codex_home.join("auth.json"); @@ -133,8 +133,8 @@ fn end_to_end_login_flow_persists_auth_json() { drop(issuer_handle); } -#[test] -fn creates_missing_codex_home_dir() { +#[tokio::test] +async fn creates_missing_codex_home_dir() { if std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() { println!( "Skipping test because it cannot execute when network is disabled in a Codex sandbox." @@ -164,12 +164,12 @@ fn creates_missing_codex_home_dir() { let server = run_login_server(opts, None).unwrap(); let login_port = server.actual_port; - let client = reqwest::blocking::Client::new(); + let client = reqwest::Client::new(); let url = format!("http://127.0.0.1:{login_port}/auth/callback?code=abc&state=state2"); - let resp = client.get(&url).send().unwrap(); + let resp = client.get(&url).send().await.unwrap(); assert!(resp.status().is_success()); - server.block_until_done().unwrap(); + server.block_until_done().await.unwrap(); let auth_path = codex_home.join("auth.json"); assert!( diff --git a/codex-rs/mcp-server/src/codex_message_processor.rs b/codex-rs/mcp-server/src/codex_message_processor.rs index d13bdbf346..7e5da55a32 100644 --- a/codex-rs/mcp-server/src/codex_message_processor.rs +++ b/codex-rs/mcp-server/src/codex_message_processor.rs @@ -180,15 +180,9 @@ impl CodexMessageProcessor { let outgoing_clone = self.outgoing.clone(); let active_login = self.active_login.clone(); tokio::spawn(async move { - let result = - tokio::task::spawn_blocking(move || server.block_until_done()).await; - let (success, error_msg) = match result { - Ok(Ok(())) => (true, None), - Ok(Err(err)) => (false, Some(format!("Login server error: {err}"))), - Err(join_err) => ( - false, - Some(format!("failed to join login server thread: {join_err}")), - ), + let (success, error_msg) = match server.block_until_done().await { + Ok(()) => (true, None), + Err(err) => (false, Some(format!("Login server error: {err}"))), }; let notification = LoginChatGptCompleteNotification { login_id, diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 8407e84e21..7961d75b5a 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -27,7 +27,6 @@ use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; -use std::thread::JoinHandle; use super::onboarding_screen::StepState; // no additional imports @@ -47,7 +46,7 @@ pub(crate) enum SignInState { pub(crate) struct ContinueInBrowserState { auth_url: String, shutdown_flag: Option>, - _login_wait_handle: Option>, + _login_wait_handle: Option>, } impl Drop for ContinueInBrowserState { fn drop(&mut self) { @@ -288,11 +287,16 @@ impl AuthModeWidget { Ok(child) => { let auth_url = child.auth_url.clone(); let shutdown_flag = child.shutdown_flag.clone(); + + let event_tx = self.event_tx.clone(); + let join_handle = tokio::spawn(async move { + spawn_completion_poller(child, event_tx).await; + }); self.sign_in_state = SignInState::ChatGptContinueInBrowser(ContinueInBrowserState { auth_url, shutdown_flag: Some(shutdown_flag), - _login_wait_handle: Some(self.spawn_completion_poller(child)), + _login_wait_handle: Some(join_handle), }); self.event_tx.send(AppEvent::RequestRedraw); } @@ -313,19 +317,21 @@ impl AuthModeWidget { } self.event_tx.send(AppEvent::RequestRedraw); } +} - fn spawn_completion_poller(&self, child: codex_login::LoginServer) -> JoinHandle<()> { - let event_tx = self.event_tx.clone(); - std::thread::spawn(move || { - if let Ok(()) = child.block_until_done() { - event_tx.send(AppEvent::OnboardingAuthComplete(Ok(()))); - } else { - event_tx.send(AppEvent::OnboardingAuthComplete(Err( - "login failed".to_string() - ))); - } - }) - } +async fn spawn_completion_poller( + child: codex_login::LoginServer, + event_tx: AppEventSender, +) -> tokio::task::JoinHandle<()> { + tokio::spawn(async move { + if let Ok(()) = child.block_until_done().await { + event_tx.send(AppEvent::OnboardingAuthComplete(Ok(()))); + } else { + event_tx.send(AppEvent::OnboardingAuthComplete(Err( + "login failed".to_string() + ))); + } + }) } impl StepStateProvider for AuthModeWidget { From 38b84ffd43136bb06f3f2fef5ff75e4e10e86d5d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 18 Aug 2025 17:29:50 -0700 Subject: [PATCH 09/14] chore(deps): bump clap from 4.5.43 to 4.5.45 in /codex-rs (#2404) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [//]: # (dependabot-start) ⚠️ **Dependabot is rebasing this PR** ⚠️ Rebasing might not happen immediately, so don't worry if this takes some time. Note: if you make any changes to this PR yourself, they will take precedence over the rebase. --- [//]: # (dependabot-end) Bumps [clap](https://github.com/clap-rs/clap) from 4.5.43 to 4.5.45.
Release notes

Sourced from clap's releases.

v4.5.45

[4.5.45] - 2025-08-12

Fixes

  • (unstable-v5) ValueEnum variants now use the full doc comment, not summary, for PossibleValue::help

v4.5.44

[4.5.44] - 2025-08-11

Features

  • Add Command::mut_subcommands
Changelog

Sourced from clap's changelog.

[4.5.45] - 2025-08-12

Fixes

  • (unstable-v5) ValueEnum variants now use the full doc comment, not summary, for PossibleValue::help

[4.5.44] - 2025-08-11

Features

  • Add Command::mut_subcommands
Commits
  • 246d972 chore: Release
  • a35a076 docs: Update changelog
  • 9b985a3 Merge pull request #5912 from epage/takes
  • 389fbe8 feat(builder): Allow flags to take num_args=0..=1
  • c395d02 test(parser): Show flag behavior
  • 32c119e refactor(assert): Be more specific than action.takes_values
  • 80ea3e7 fix(assert): Clean up num_args/action assert
  • 2bc0f45 fix(builder): Make ValueRange display independent of usize::MAX
  • a0187c6 test(assert): Verify num_args/action compat
  • a8f9885 refactor(builder): Be more explicit in how takes_values is used
  • Additional commits viewable in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=clap&package-manager=cargo&previous-version=4.5.43&new-version=4.5.45)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- codex-rs/Cargo.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 8d2d5f7c31..05e804c148 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -538,9 +538,9 @@ checksum = "6e4de3bc4ea267985becf712dc6d9eed8b04c953b3fcfb339ebc87acd9804901" [[package]] name = "clap" -version = "4.5.43" +version = "4.5.45" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "50fd97c9dc2399518aa331917ac6f274280ec5eb34e555dd291899745c48ec6f" +checksum = "1fc0e74a703892159f5ae7d3aac52c8e6c392f5ae5f359c70b5881d60aaac318" dependencies = [ "clap_builder", "clap_derive", @@ -548,9 +548,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.5.43" +version = "4.5.44" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c35b5830294e1fa0462034af85cc95225a4cb07092c088c55bda3147cfcd8f65" +checksum = "b3e7f4214277f3c7aa526a59dd3fbe306a370daee1f8b7b8c987069cd8e888a8" dependencies = [ "anstream", "anstyle", @@ -570,9 +570,9 @@ dependencies = [ [[package]] name = "clap_derive" -version = "4.5.41" +version = "4.5.45" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ef4f52386a59ca4c860f7393bcf8abd8dfd91ecccc0f774635ff68e92eeef491" +checksum = "14cb31bb0a7d536caef2639baa7fad459e15c3144efefa6dbd1c84562c4739f6" dependencies = [ "heck", "proc-macro2", From d58df2828683763ec59249e34d507afd3765add5 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Aug 2025 17:32:03 -0700 Subject: [PATCH 10/14] fix: change `shutdown_flag` from `Arc` to `tokio::sync::Notify` (#2394) Prior to this PR, we had: https://github.com/openai/codex/blob/71cae06e6643b4adf644d9769208c3c5fcd1f2be/codex-rs/login/src/server.rs#L141-L142 which means that we could be blocked waiting for a new request in `server_for_thread.recv()` and not notice that the state of `shutdown_flag` had changed. With this PR, we use `shutdown_flag: Notify` so that we can `tokio::select!` on `shutdown_notify.notified()` and `rx.recv()` (which is the "async stream" of requests read from `server_for_thread.recv()`) and handle whichever one happens first. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2394). * #2399 * #2398 * #2396 * #2395 * __->__ #2394 * #2393 * #2389 --- codex-rs/login/src/server.rs | 134 +++++++++++++++------------- codex-rs/tui/src/onboarding/auth.rs | 15 ++-- 2 files changed, 77 insertions(+), 72 deletions(-) diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index 060b333c6e..419874e7a0 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -52,7 +52,7 @@ impl ServerOptions { pub struct LoginServer { pub auth_url: String, pub actual_port: u16, - pub shutdown_flag: Arc, + shutdown_flag: Arc, server_handle: tokio::task::JoinHandle>, server: Arc, } @@ -70,7 +70,7 @@ impl LoginServer { pub fn cancel_handle(&self) -> ShutdownHandle { ShutdownHandle { - shutdown_flag: self.shutdown_flag.clone(), + shutdown_notify: self.shutdown_flag.clone(), server: self.server.clone(), } } @@ -78,24 +78,32 @@ impl LoginServer { #[derive(Clone)] pub struct ShutdownHandle { - shutdown_flag: Arc, + shutdown_notify: Arc, server: Arc, } +impl std::fmt::Debug for ShutdownHandle { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ShutdownHandle") + .field("shutdown_notify", &self.shutdown_notify) + .finish() + } +} + impl ShutdownHandle { pub fn cancel(&self) { - shutdown(&self.shutdown_flag, &self.server); + shutdown(&self.shutdown_notify, &self.server); } } -pub fn shutdown(shutdown_flag: &AtomicBool, server: &Server) { - shutdown_flag.store(true, Ordering::SeqCst); +pub fn shutdown(shutdown_notify: &tokio::sync::Notify, server: &Server) { + shutdown_notify.notify_waiters(); server.unblock(); } pub fn run_login_server( opts: ServerOptions, - shutdown_flag: Option>, + shutdown_flag: Option>, ) -> io::Result { let pkce = generate_pkce(); let state = opts.force_state.clone().unwrap_or_else(generate_state); @@ -118,9 +126,9 @@ pub fn run_login_server( if opts.open_browser { let _ = webbrowser::open(&auth_url); } - let shutdown_flag: Arc = - shutdown_flag.unwrap_or_else(|| Arc::new(AtomicBool::new(false))); - let shutdown_flag_clone = shutdown_flag.clone(); + let shutdown_notify: Arc = + shutdown_flag.unwrap_or_else(|| Arc::new(tokio::sync::Notify::new())); + let shutdown_notify_clone = shutdown_notify.clone(); let timeout_flag = Arc::new(AtomicBool::new(false)); // Channel used to signal completion to timeout watcher. @@ -130,7 +138,7 @@ pub fn run_login_server( spawn_timeout_watcher( done_rx, timeout, - shutdown_flag.clone(), + shutdown_notify.clone(), timeout_flag.clone(), server.clone(), ); @@ -139,61 +147,62 @@ pub fn run_login_server( let (tx, mut rx) = tokio::sync::mpsc::channel::(16); let _server_handle = { let server = server.clone(); - let shutdown_flag = shutdown_flag.clone(); - thread::spawn(move || { - while !shutdown_flag.load(Ordering::SeqCst) { - match server.recv() { - Ok(request) => tx.blocking_send(request).map_err(|e| { - eprintln!("Failed to send request to channel: {e}"); - io::Error::other("Failed to send request to channel") - })?, - Err(e) => { - // If we've been asked to shut down, break gracefully so that - // we can report timeout or cancellation status uniformly. - if shutdown_flag.load(Ordering::SeqCst) { - break; - } else { - return Err(io::Error::other(e)); - } - } - }; + thread::spawn(move || -> io::Result<()> { + while let Ok(request) = server.recv() { + tx.blocking_send(request).map_err(|e| { + eprintln!("Failed to send request to channel: {e}"); + io::Error::other("Failed to send request to channel") + })?; } Ok(()) }) }; + let server_for_task = server.clone(); let server_handle = tokio::spawn(async move { - while let Some(req) = rx.recv().await { - let url_raw = req.url().to_string(); - let response = - process_request(&url_raw, &opts, &redirect_uri, &pkce, actual_port, &state).await; - - let is_login_complete = matches!(response, HandledRequest::ResponseAndExit(_)); - match response { - HandledRequest::Response(r) | HandledRequest::ResponseAndExit(r) => { - let _ = tokio::task::spawn_blocking(move || req.respond(r)).await; + loop { + tokio::select! { + _ = shutdown_notify.notified() => { + let _ = done_tx.send(()); + if timeout_flag.load(Ordering::SeqCst) { + return Err(io::Error::other("Login timed out")); + } else { + return Err(io::Error::other("Login was not completed")); + } } - HandledRequest::RedirectWithHeader(header) => { - let redirect = Response::empty(302).with_header(header); - let _ = tokio::task::spawn_blocking(move || req.respond(redirect)).await; + maybe_req = rx.recv() => { + let Some(req) = maybe_req else { + let _ = done_tx.send(()); + if timeout_flag.load(Ordering::SeqCst) { + return Err(io::Error::other("Login timed out")); + } else { + return Err(io::Error::other("Login was not completed")); + } + }; + + let url_raw = req.url().to_string(); + let response = + process_request(&url_raw, &opts, &redirect_uri, &pkce, actual_port, &state).await; + + let is_login_complete = matches!(response, HandledRequest::ResponseAndExit(_)); + match response { + HandledRequest::Response(r) | HandledRequest::ResponseAndExit(r) => { + let _ = tokio::task::spawn_blocking(move || req.respond(r)).await; + } + HandledRequest::RedirectWithHeader(header) => { + let redirect = Response::empty(302).with_header(header); + let _ = tokio::task::spawn_blocking(move || req.respond(redirect)).await; + } + } + + if is_login_complete { + shutdown_notify.notify_waiters(); + let _ = done_tx.send(()); + server_for_task.unblock(); + return Ok(()); + } } } - - if is_login_complete { - shutdown_flag.store(true, Ordering::SeqCst); - // Login has succeeded, so disarm the timeout watcher. - let _ = done_tx.send(()); - return Ok(()); - } - } - - // Login has failed or timed out, so disarm the timeout watcher. - let _ = done_tx.send(()); - - if timeout_flag.load(Ordering::SeqCst) { - Err(io::Error::other("Login timed out")) - } else { - Err(io::Error::other("Login was not completed")) } }); @@ -201,7 +210,7 @@ pub fn run_login_server( auth_url: auth_url.clone(), actual_port, server_handle, - shutdown_flag: shutdown_flag_clone, + shutdown_flag: shutdown_notify_clone, server, }) } @@ -317,17 +326,14 @@ async fn process_request( fn spawn_timeout_watcher( done_rx: mpsc::Receiver<()>, timeout: Duration, - shutdown_flag: Arc, + shutdown_notify: Arc, timeout_flag: Arc, server: Arc, ) { thread::spawn(move || { - if done_rx.recv_timeout(timeout).is_err() - && shutdown_flag - .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) - .is_ok() - { + if done_rx.recv_timeout(timeout).is_err() { timeout_flag.store(true, Ordering::SeqCst); + shutdown_notify.notify_waiters(); server.unblock(); } }); diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 7961d75b5a..7166e349c3 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -1,5 +1,6 @@ use codex_login::CLIENT_ID; use codex_login::ServerOptions; +use codex_login::ShutdownHandle; use codex_login::run_login_server; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; @@ -24,9 +25,6 @@ use crate::onboarding::onboarding_screen::KeyboardHandler; use crate::onboarding::onboarding_screen::StepStateProvider; use crate::shimmer::shimmer_spans; use std::path::PathBuf; -use std::sync::Arc; -use std::sync::atomic::AtomicBool; -use std::sync::atomic::Ordering; use super::onboarding_screen::StepState; // no additional imports @@ -45,13 +43,14 @@ pub(crate) enum SignInState { /// Used to manage the lifecycle of SpawnedLogin and ensure it gets cleaned up. pub(crate) struct ContinueInBrowserState { auth_url: String, - shutdown_flag: Option>, + shutdown_handle: Option, _login_wait_handle: Option>, } + impl Drop for ContinueInBrowserState { fn drop(&mut self) { - if let Some(flag) = &self.shutdown_flag { - flag.store(true, Ordering::SeqCst); + if let Some(flag) = &self.shutdown_handle { + flag.cancel(); } } } @@ -286,7 +285,7 @@ impl AuthModeWidget { match server { Ok(child) => { let auth_url = child.auth_url.clone(); - let shutdown_flag = child.shutdown_flag.clone(); + let shutdown_handle = child.cancel_handle(); let event_tx = self.event_tx.clone(); let join_handle = tokio::spawn(async move { @@ -295,7 +294,7 @@ impl AuthModeWidget { self.sign_in_state = SignInState::ChatGptContinueInBrowser(ContinueInBrowserState { auth_url, - shutdown_flag: Some(shutdown_flag), + shutdown_handle: Some(shutdown_handle), _login_wait_handle: Some(join_handle), }); self.event_tx.send(AppEvent::RequestRedraw); From ba466d1c2e237dc785f244e4b20772eff8facae5 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Aug 2025 17:35:35 -0700 Subject: [PATCH 11/14] fix: eliminate ServerOptions.login_timeout and have caller use tokio::time::timeout() instead --- codex-rs/login/src/server.rs | 115 +++++------------- codex-rs/login/tests/login_server_e2e.rs | 2 - .../mcp-server/src/codex_message_processor.rs | 20 ++- 3 files changed, 48 insertions(+), 89 deletions(-) diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index 419874e7a0..f33f1ae4fb 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -3,11 +3,7 @@ use std::io::{self}; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; -use std::sync::atomic::AtomicBool; -use std::sync::atomic::Ordering; -use std::sync::mpsc; use std::thread; -use std::time::Duration; use crate::AuthDotJson; use crate::get_auth_file; @@ -32,7 +28,6 @@ pub struct ServerOptions { pub port: u16, pub open_browser: bool, pub force_state: Option, - pub login_timeout: Option, } impl ServerOptions { @@ -44,7 +39,6 @@ impl ServerOptions { port: DEFAULT_PORT, open_browser: true, force_state: None, - login_timeout: None, } } } @@ -126,24 +120,8 @@ pub fn run_login_server( if opts.open_browser { let _ = webbrowser::open(&auth_url); } - let shutdown_notify: Arc = - shutdown_flag.unwrap_or_else(|| Arc::new(tokio::sync::Notify::new())); - let shutdown_notify_clone = shutdown_notify.clone(); - let timeout_flag = Arc::new(AtomicBool::new(false)); - - // Channel used to signal completion to timeout watcher. - let (done_tx, done_rx) = mpsc::channel::<()>(); - - if let Some(timeout) = opts.login_timeout { - spawn_timeout_watcher( - done_rx, - timeout, - shutdown_notify.clone(), - timeout_flag.clone(), - server.clone(), - ); - } + // Map blocking reads from server.recv() to an async channel. let (tx, mut rx) = tokio::sync::mpsc::channel::(16); let _server_handle = { let server = server.clone(); @@ -158,59 +136,52 @@ pub fn run_login_server( }) }; - let server_for_task = server.clone(); - let server_handle = tokio::spawn(async move { - loop { - tokio::select! { - _ = shutdown_notify.notified() => { - let _ = done_tx.send(()); - if timeout_flag.load(Ordering::SeqCst) { - return Err(io::Error::other("Login timed out")); - } else { + let shutdown_notify = shutdown_flag.unwrap_or_else(|| Arc::new(tokio::sync::Notify::new())); + let server_handle = { + let shutdown_notify = shutdown_notify.clone(); + let server = server.clone(); + tokio::spawn(async move { + loop { + tokio::select! { + _ = shutdown_notify.notified() => { return Err(io::Error::other("Login was not completed")); } - } - maybe_req = rx.recv() => { - let Some(req) = maybe_req else { - let _ = done_tx.send(()); - if timeout_flag.load(Ordering::SeqCst) { - return Err(io::Error::other("Login timed out")); - } else { + maybe_req = rx.recv() => { + let Some(req) = maybe_req else { return Err(io::Error::other("Login was not completed")); - } - }; + }; - let url_raw = req.url().to_string(); - let response = - process_request(&url_raw, &opts, &redirect_uri, &pkce, actual_port, &state).await; + let url_raw = req.url().to_string(); + let response = + process_request(&url_raw, &opts, &redirect_uri, &pkce, actual_port, &state).await; - let is_login_complete = matches!(response, HandledRequest::ResponseAndExit(_)); - match response { - HandledRequest::Response(r) | HandledRequest::ResponseAndExit(r) => { - let _ = tokio::task::spawn_blocking(move || req.respond(r)).await; + let is_login_complete = matches!(response, HandledRequest::ResponseAndExit(_)); + match response { + HandledRequest::Response(r) | HandledRequest::ResponseAndExit(r) => { + let _ = tokio::task::spawn_blocking(move || req.respond(r)).await; + } + HandledRequest::RedirectWithHeader(header) => { + let redirect = Response::empty(302).with_header(header); + let _ = tokio::task::spawn_blocking(move || req.respond(redirect)).await; + } } - HandledRequest::RedirectWithHeader(header) => { - let redirect = Response::empty(302).with_header(header); - let _ = tokio::task::spawn_blocking(move || req.respond(redirect)).await; - } - } - if is_login_complete { - shutdown_notify.notify_waiters(); - let _ = done_tx.send(()); - server_for_task.unblock(); - return Ok(()); + if is_login_complete { + shutdown_notify.notify_waiters(); + server.unblock(); + return Ok(()); + } } } } - } - }); + }) + }; Ok(LoginServer { - auth_url: auth_url.clone(), + auth_url, actual_port, server_handle, - shutdown_flag: shutdown_notify_clone, + shutdown_flag: shutdown_notify, server, }) } @@ -319,26 +290,6 @@ async fn process_request( } } -/// Spawns a detached thread that waits for either a completion signal on `done_rx` -/// or the specified `timeout` to elapse. If the timeout elapses first it marks -/// the `shutdown_flag`, records `timeout_flag`, and unblocks the HTTP server so -/// that the main server loop can exit promptly. -fn spawn_timeout_watcher( - done_rx: mpsc::Receiver<()>, - timeout: Duration, - shutdown_notify: Arc, - timeout_flag: Arc, - server: Arc, -) { - thread::spawn(move || { - if done_rx.recv_timeout(timeout).is_err() { - timeout_flag.store(true, Ordering::SeqCst); - shutdown_notify.notify_waiters(); - server.unblock(); - } - }); -} - fn build_authorize_url( issuer: &str, client_id: &str, diff --git a/codex-rs/login/tests/login_server_e2e.rs b/codex-rs/login/tests/login_server_e2e.rs index 09a447d565..ef387f575e 100644 --- a/codex-rs/login/tests/login_server_e2e.rs +++ b/codex-rs/login/tests/login_server_e2e.rs @@ -100,7 +100,6 @@ async fn end_to_end_login_flow_persists_auth_json() { port: 0, open_browser: false, force_state: Some(state), - login_timeout: None, }; let server = run_login_server(opts, None).unwrap(); let login_port = server.actual_port; @@ -159,7 +158,6 @@ async fn creates_missing_codex_home_dir() { port: 0, open_browser: false, force_state: Some(state), - login_timeout: None, }; let server = run_login_server(opts, None).unwrap(); let login_port = server.actual_port; diff --git a/codex-rs/mcp-server/src/codex_message_processor.rs b/codex-rs/mcp-server/src/codex_message_processor.rs index 7e5da55a32..00c8717c43 100644 --- a/codex-rs/mcp-server/src/codex_message_processor.rs +++ b/codex-rs/mcp-server/src/codex_message_processor.rs @@ -146,7 +146,6 @@ impl CodexMessageProcessor { let opts = LoginServerOptions { open_browser: false, - login_timeout: Some(LOGIN_CHATGPT_TIMEOUT), ..LoginServerOptions::new(config.codex_home.clone(), CLIENT_ID.to_string()) }; @@ -158,6 +157,7 @@ impl CodexMessageProcessor { let reply = match run_login_server(opts, None) { Ok(server) => { let login_id = Uuid::new_v4(); + let shutdown_handle = server.cancel_handle(); // Replace active login if present. { @@ -166,7 +166,7 @@ impl CodexMessageProcessor { existing.drop(); } *guard = Some(ActiveLogin { - shutdown_handle: server.cancel_handle(), + shutdown_handle: shutdown_handle.clone(), login_id, }); } @@ -180,9 +180,19 @@ impl CodexMessageProcessor { let outgoing_clone = self.outgoing.clone(); let active_login = self.active_login.clone(); tokio::spawn(async move { - let (success, error_msg) = match server.block_until_done().await { - Ok(()) => (true, None), - Err(err) => (false, Some(format!("Login server error: {err}"))), + let (success, error_msg) = match tokio::time::timeout( + LOGIN_CHATGPT_TIMEOUT, + server.block_until_done(), + ) + .await + { + Ok(Ok(())) => (true, None), + Ok(Err(err)) => (false, Some(format!("Login server error: {err}"))), + Err(_elapsed) => { + // Timeout: cancel server and report + shutdown_handle.cancel(); + (false, Some("Login timed out".to_string())) + } }; let notification = LoginChatGptCompleteNotification { login_id, From edf3fa842fc0a55a18f1fe03112fc6513d6c5f21 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Aug 2025 17:35:35 -0700 Subject: [PATCH 12/14] fix: make ShutdownHandle a private field of LoginServer --- codex-rs/login/src/server.rs | 26 +++++++------------ .../mcp-server/src/codex_message_processor.rs | 4 +-- codex-rs/tui/src/onboarding/auth.rs | 2 +- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index f33f1ae4fb..f3256ee6c8 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -46,9 +46,8 @@ impl ServerOptions { pub struct LoginServer { pub auth_url: String, pub actual_port: u16, - shutdown_flag: Arc, server_handle: tokio::task::JoinHandle>, - server: Arc, + shutdown_handle: ShutdownHandle, } impl LoginServer { @@ -59,14 +58,11 @@ impl LoginServer { } pub fn cancel(&self) { - shutdown(&self.shutdown_flag, &self.server); + self.shutdown_handle.shutdown(); } pub fn cancel_handle(&self) -> ShutdownHandle { - ShutdownHandle { - shutdown_notify: self.shutdown_flag.clone(), - server: self.server.clone(), - } + self.shutdown_handle.clone() } } @@ -85,16 +81,12 @@ impl std::fmt::Debug for ShutdownHandle { } impl ShutdownHandle { - pub fn cancel(&self) { - shutdown(&self.shutdown_notify, &self.server); + pub fn shutdown(&self) { + self.shutdown_notify.notify_waiters(); + self.server.unblock(); } } -pub fn shutdown(shutdown_notify: &tokio::sync::Notify, server: &Server) { - shutdown_notify.notify_waiters(); - server.unblock(); -} - pub fn run_login_server( opts: ServerOptions, shutdown_flag: Option>, @@ -181,8 +173,10 @@ pub fn run_login_server( auth_url, actual_port, server_handle, - shutdown_flag: shutdown_notify, - server, + shutdown_handle: ShutdownHandle { + shutdown_notify, + server, + }, }) } diff --git a/codex-rs/mcp-server/src/codex_message_processor.rs b/codex-rs/mcp-server/src/codex_message_processor.rs index 00c8717c43..4f2b3bb693 100644 --- a/codex-rs/mcp-server/src/codex_message_processor.rs +++ b/codex-rs/mcp-server/src/codex_message_processor.rs @@ -66,7 +66,7 @@ struct ActiveLogin { impl ActiveLogin { fn drop(&self) { - self.shutdown_handle.cancel(); + self.shutdown_handle.shutdown(); } } @@ -190,7 +190,7 @@ impl CodexMessageProcessor { Ok(Err(err)) => (false, Some(format!("Login server error: {err}"))), Err(_elapsed) => { // Timeout: cancel server and report - shutdown_handle.cancel(); + shutdown_handle.shutdown(); (false, Some("Login timed out".to_string())) } }; diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 7166e349c3..490f85bff8 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -50,7 +50,7 @@ pub(crate) struct ContinueInBrowserState { impl Drop for ContinueInBrowserState { fn drop(&mut self) { if let Some(flag) = &self.shutdown_handle { - flag.cancel(); + flag.shutdown(); } } } From b04a546f6195b3b40c07aece7d5b1e81bc9cf532 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Aug 2025 17:35:35 -0700 Subject: [PATCH 13/14] fix: reduce references to Server in codex-login crate --- codex-rs/login/src/server.rs | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index f3256ee6c8..e8af09fba0 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -66,24 +66,14 @@ impl LoginServer { } } -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct ShutdownHandle { shutdown_notify: Arc, - server: Arc, -} - -impl std::fmt::Debug for ShutdownHandle { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("ShutdownHandle") - .field("shutdown_notify", &self.shutdown_notify) - .finish() - } } impl ShutdownHandle { pub fn shutdown(&self) { self.shutdown_notify.notify_waiters(); - self.server.unblock(); } } @@ -133,14 +123,14 @@ pub fn run_login_server( let shutdown_notify = shutdown_notify.clone(); let server = server.clone(); tokio::spawn(async move { - loop { + let result = loop { tokio::select! { _ = shutdown_notify.notified() => { - return Err(io::Error::other("Login was not completed")); + break Err(io::Error::other("Login was not completed")); } maybe_req = rx.recv() => { let Some(req) = maybe_req else { - return Err(io::Error::other("Login was not completed")); + break Err(io::Error::other("Login was not completed")); }; let url_raw = req.url().to_string(); @@ -159,13 +149,16 @@ pub fn run_login_server( } if is_login_complete { - shutdown_notify.notify_waiters(); - server.unblock(); - return Ok(()); + break Ok(()); } } } - } + }; + + // Ensure that the server is unblocked so the thread dedicated to + // running `server.recv()` in a loop exits cleanly. + server.unblock(); + result }) }; @@ -173,10 +166,7 @@ pub fn run_login_server( auth_url, actual_port, server_handle, - shutdown_handle: ShutdownHandle { - shutdown_notify, - server, - }, + shutdown_handle: ShutdownHandle { shutdown_notify }, }) } From 074ef26b962e0ed72ad913b376157a885941adf9 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Aug 2025 17:35:35 -0700 Subject: [PATCH 14/14] fix: remove shutdown_flag param to run_login_server() --- codex-rs/cli/src/login.rs | 2 +- codex-rs/login/src/server.rs | 7 ++----- codex-rs/login/tests/login_server_e2e.rs | 4 ++-- codex-rs/mcp-server/src/codex_message_processor.rs | 2 +- codex-rs/tui/src/onboarding/auth.rs | 2 +- 5 files changed, 7 insertions(+), 10 deletions(-) diff --git a/codex-rs/cli/src/login.rs b/codex-rs/cli/src/login.rs index fc40a0271f..36bbf2208d 100644 --- a/codex-rs/cli/src/login.rs +++ b/codex-rs/cli/src/login.rs @@ -14,7 +14,7 @@ use std::path::PathBuf; pub async fn login_with_chatgpt(codex_home: PathBuf) -> std::io::Result<()> { let opts = ServerOptions::new(codex_home, CLIENT_ID.to_string()); - let server = run_login_server(opts, None)?; + let server = run_login_server(opts)?; eprintln!( "Starting local login server on http://localhost:{}.\nIf your browser did not open, navigate to this URL to authenticate:\n\n{}", diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index e8af09fba0..32229484ff 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -77,10 +77,7 @@ impl ShutdownHandle { } } -pub fn run_login_server( - opts: ServerOptions, - shutdown_flag: Option>, -) -> io::Result { +pub fn run_login_server(opts: ServerOptions) -> io::Result { let pkce = generate_pkce(); let state = opts.force_state.clone().unwrap_or_else(generate_state); @@ -118,7 +115,7 @@ pub fn run_login_server( }) }; - let shutdown_notify = shutdown_flag.unwrap_or_else(|| Arc::new(tokio::sync::Notify::new())); + let shutdown_notify = Arc::new(tokio::sync::Notify::new()); let server_handle = { let shutdown_notify = shutdown_notify.clone(); let server = server.clone(); diff --git a/codex-rs/login/tests/login_server_e2e.rs b/codex-rs/login/tests/login_server_e2e.rs index ef387f575e..ceb0a94733 100644 --- a/codex-rs/login/tests/login_server_e2e.rs +++ b/codex-rs/login/tests/login_server_e2e.rs @@ -101,7 +101,7 @@ async fn end_to_end_login_flow_persists_auth_json() { open_browser: false, force_state: Some(state), }; - let server = run_login_server(opts, None).unwrap(); + let server = run_login_server(opts).unwrap(); let login_port = server.actual_port; // Simulate browser callback, and follow redirect to /success @@ -159,7 +159,7 @@ async fn creates_missing_codex_home_dir() { open_browser: false, force_state: Some(state), }; - let server = run_login_server(opts, None).unwrap(); + let server = run_login_server(opts).unwrap(); let login_port = server.actual_port; let client = reqwest::Client::new(); diff --git a/codex-rs/mcp-server/src/codex_message_processor.rs b/codex-rs/mcp-server/src/codex_message_processor.rs index 4f2b3bb693..1decf11da6 100644 --- a/codex-rs/mcp-server/src/codex_message_processor.rs +++ b/codex-rs/mcp-server/src/codex_message_processor.rs @@ -154,7 +154,7 @@ impl CodexMessageProcessor { Error(JSONRPCErrorError), } - let reply = match run_login_server(opts, None) { + let reply = match run_login_server(opts) { Ok(server) => { let login_id = Uuid::new_v4(); let shutdown_handle = server.cancel_handle(); diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 490f85bff8..facd06c64a 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -281,7 +281,7 @@ impl AuthModeWidget { fn start_chatgpt_login(&mut self) { self.error = None; let opts = ServerOptions::new(self.codex_home.clone(), CLIENT_ID.to_string()); - let server = run_login_server(opts, None); + let server = run_login_server(opts); match server { Ok(child) => { let auth_url = child.auth_url.clone();