diff --git a/codex-rs/core/prompt.md b/codex-rs/core/prompt.md index 4e55003b9f..f194eba4e2 100644 --- a/codex-rs/core/prompt.md +++ b/codex-rs/core/prompt.md @@ -9,6 +9,8 @@ You MUST adhere to the following criteria when executing the task: - Analyzing code for vulnerabilities is allowed. - Showing user code and tool call details is allowed. - User instructions may overwrite the _CODING GUIDELINES_ section in this developer message. +- `user_instructions` are not part of the user's request, but guidance for how to complete the task. +- Do not cite `user_instructions` back to the user unless a specific piece is relevant. - Do not use \`ls -R\`, \`find\`, or \`grep\` - these are slow in large repos. Use \`rg\` and \`rg --files\`. - Use \`apply_patch\` to edit files: {"command":["apply_patch","*** Begin Patch\\n*** Update File: path/to/file.py\\n@@ def example():\\n- pass\\n+ return 123\\n*** End Patch"]} - If completing the user's task requires writing or modifying files: diff --git a/codex-rs/core/src/chat_completions.rs b/codex-rs/core/src/chat_completions.rs index 5ede774b1c..b5ade23b9d 100644 --- a/codex-rs/core/src/chat_completions.rs +++ b/codex-rs/core/src/chat_completions.rs @@ -40,7 +40,7 @@ pub(crate) async fn stream_chat_completions( let full_instructions = prompt.get_full_instructions(model); messages.push(json!({"role": "system", "content": full_instructions})); - if let Some(instr) = &prompt.user_instructions { + if let Some(instr) = &prompt.get_formatted_user_instructions() { messages.push(json!({"role": "user", "content": instr})); } @@ -120,7 +120,7 @@ pub(crate) async fn stream_chat_completions( debug!( "POST to {}: {}", - provider.get_full_url(), + provider.get_full_url(&None), serde_json::to_string_pretty(&payload).unwrap_or_default() ); @@ -129,7 +129,7 @@ pub(crate) async fn stream_chat_completions( loop { attempt += 1; - let req_builder = provider.create_request_builder(client)?; + let req_builder = provider.create_request_builder(client, &None).await?; let res = req_builder .header(reqwest::header::ACCEPT, "text/event-stream") diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index b9ea6b13f4..00762a8a67 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -30,7 +30,6 @@ use crate::config::Config; use crate::config_types::ReasoningEffort as ReasoningEffortConfig; use crate::config_types::ReasoningSummary as ReasoningSummaryConfig; use crate::error::CodexErr; -use crate::error::EnvVarError; use crate::error::Result; use crate::flags::CODEX_RS_SSE_FIXTURE; use crate::model_provider_info::ModelProviderInfo; @@ -122,24 +121,11 @@ impl ModelClient { return stream_from_fixture(path, self.provider.clone()).await; } - let auth = self.auth.as_ref().ok_or_else(|| { - CodexErr::EnvVar(EnvVarError { - var: "OPENAI_API_KEY".to_string(), - instructions: Some("Create an API key (https://platform.openai.com) and export it as an environment variable.".to_string()), - }) - })?; + let auth = self.auth.clone(); - let store = prompt.store && auth.mode != AuthMode::ChatGPT; + let auth_mode = auth.as_ref().map(|a| a.mode); - let base_url = match self.provider.base_url.clone() { - Some(url) => url, - None => match auth.mode { - AuthMode::ChatGPT => "https://chatgpt.com/backend-api/codex".to_string(), - AuthMode::ApiKey => "https://api.openai.com/v1".to_string(), - }, - }; - - let token = auth.get_token().await?; + let store = prompt.store && auth_mode != Some(AuthMode::ChatGPT); let full_instructions = prompt.get_full_instructions(&self.config.model); let tools_json = create_tools_json_for_responses_api( @@ -158,11 +144,11 @@ impl ModelClient { }; let mut input_with_instructions = Vec::with_capacity(prompt.input.len() + 1); - if let Some(ui) = &prompt.user_instructions { + if let Some(ui) = prompt.get_formatted_user_instructions() { input_with_instructions.push(ResponseItem::Message { id: None, role: "user".to_string(), - content: vec![ContentItem::InputText { text: ui.clone() }], + content: vec![ContentItem::InputText { text: ui }], }); } input_with_instructions.extend(prompt.input.clone()); @@ -180,35 +166,36 @@ impl ModelClient { include, }; - trace!( - "POST to {}: {}", - self.provider.get_full_url(), - serde_json::to_string(&payload)? - ); - let mut attempt = 0; let max_retries = self.provider.request_max_retries(); + trace!( + "POST to {}: {}", + self.provider.get_full_url(&auth), + serde_json::to_string(&payload)? + ); + loop { attempt += 1; let mut req_builder = self - .client - .post(format!("{base_url}/responses")) + .provider + .create_request_builder(&self.client, &auth) + .await?; + + req_builder = req_builder .header("OpenAI-Beta", "responses=experimental") .header("session_id", self.session_id.to_string()) - .bearer_auth(&token) .header(reqwest::header::ACCEPT, "text/event-stream") .json(&payload); - if auth.mode == AuthMode::ChatGPT { - if let Some(account_id) = auth.get_account_id().await { - req_builder = req_builder.header("chatgpt-account-id", account_id); - } + if let Some(auth) = auth.as_ref() + && auth.mode == AuthMode::ChatGPT + && let Some(account_id) = auth.get_account_id().await + { + req_builder = req_builder.header("chatgpt-account-id", account_id); } - req_builder = self.provider.apply_http_headers(req_builder); - let originator = self .config .internal_originator diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index 157f35872a..6d9524cc92 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -17,6 +17,10 @@ use tokio::sync::mpsc; /// with this content. const BASE_INSTRUCTIONS: &str = include_str!("../prompt.md"); +/// wraps user instructions message in a tag for the model to parse more easily. +const USER_INSTRUCTIONS_START: &str = "\n\n"; +const USER_INSTRUCTIONS_END: &str = "\n\n"; + /// API request payload for a single model turn. #[derive(Default, Debug, Clone)] pub struct Prompt { @@ -49,6 +53,12 @@ impl Prompt { } Cow::Owned(sections.join("\n")) } + + pub(crate) fn get_formatted_user_instructions(&self) -> Option { + self.user_instructions + .as_ref() + .map(|ui| format!("{USER_INSTRUCTIONS_START}{ui}{USER_INSTRUCTIONS_END}")) + } } #[derive(Debug)] diff --git a/codex-rs/core/src/model_provider_info.rs b/codex-rs/core/src/model_provider_info.rs index 2936637779..49478660f4 100644 --- a/codex-rs/core/src/model_provider_info.rs +++ b/codex-rs/core/src/model_provider_info.rs @@ -5,8 +5,11 @@ //! 2. User-defined entries inside `~/.codex/config.toml` under the `model_providers` //! key. These override or extend the defaults at runtime. +use codex_login::AuthMode; +use codex_login::CodexAuth; use serde::Deserialize; use serde::Serialize; +use std::borrow::Cow; use std::collections::HashMap; use std::env::VarError; use std::time::Duration; @@ -88,25 +91,30 @@ impl ModelProviderInfo { /// When `require_api_key` is true and the provider declares an `env_key` /// but the variable is missing/empty, returns an [`Err`] identical to the /// one produced by [`ModelProviderInfo::api_key`]. - pub fn create_request_builder<'a>( + pub async fn create_request_builder<'a>( &'a self, client: &'a reqwest::Client, + auth: &Option, ) -> crate::error::Result { - let url = self.get_full_url(); + let auth: Cow<'_, Option> = if auth.is_some() { + Cow::Borrowed(auth) + } else { + Cow::Owned(self.get_fallback_auth()?) + }; + + let url = self.get_full_url(&auth); let mut builder = client.post(url); - let api_key = self.api_key()?; - if let Some(key) = api_key { - builder = builder.bearer_auth(key); + if let Some(auth) = auth.as_ref() { + builder = builder.bearer_auth(auth.get_token().await?); } Ok(self.apply_http_headers(builder)) } - pub(crate) fn get_full_url(&self) -> String { - let query_string = self - .query_params + fn get_query_string(&self) -> String { + self.query_params .as_ref() .map_or_else(String::new, |params| { let full_params = params @@ -115,16 +123,29 @@ impl ModelProviderInfo { .collect::>() .join("&"); format!("?{full_params}") - }); + }) + } + + pub(crate) fn get_full_url(&self, auth: &Option) -> String { + let default_base_url = if matches!( + auth, + Some(CodexAuth { + mode: AuthMode::ChatGPT, + .. + }) + ) { + "https://chatgpt.com/backend-api/codex" + } else { + "https://api.openai.com/v1" + }; + let query_string = self.get_query_string(); let base_url = self .base_url .clone() - .unwrap_or("https://api.openai.com/v1".to_string()); + .unwrap_or(default_base_url.to_string()); match self.wire_api { - WireApi::Responses => { - format!("{base_url}/responses{query_string}") - } + WireApi::Responses => format!("{base_url}/responses{query_string}"), WireApi::Chat => format!("{base_url}/chat/completions{query_string}"), } } @@ -132,10 +153,7 @@ impl ModelProviderInfo { /// Apply provider-specific HTTP headers (both static and environment-based) /// onto an existing `reqwest::RequestBuilder` and return the updated /// builder. - pub fn apply_http_headers( - &self, - mut builder: reqwest::RequestBuilder, - ) -> reqwest::RequestBuilder { + fn apply_http_headers(&self, mut builder: reqwest::RequestBuilder) -> reqwest::RequestBuilder { if let Some(extra) = &self.http_headers { for (k, v) in extra { builder = builder.header(k, v); @@ -157,7 +175,7 @@ impl ModelProviderInfo { /// If `env_key` is Some, returns the API key for this provider if present /// (and non-empty) in the environment. If `env_key` is required but /// cannot be found, returns an error. - fn api_key(&self) -> crate::error::Result> { + pub fn api_key(&self) -> crate::error::Result> { match &self.env_key { Some(env_key) => { let env_value = std::env::var(env_key); @@ -198,6 +216,14 @@ impl ModelProviderInfo { .map(Duration::from_millis) .unwrap_or(Duration::from_millis(DEFAULT_STREAM_IDLE_TIMEOUT_MS)) } + + fn get_fallback_auth(&self) -> crate::error::Result> { + let api_key = self.api_key()?; + if let Some(api_key) = api_key { + return Ok(Some(CodexAuth::from_api_key(api_key))); + } + Ok(None) + } } /// Built-in default provider list. diff --git a/codex-rs/core/tests/client.rs b/codex-rs/core/tests/client.rs index a22a94388b..f493020210 100644 --- a/codex-rs/core/tests/client.rs +++ b/codex-rs/core/tests/client.rs @@ -4,6 +4,7 @@ use chrono::Utc; use codex_core::Codex; use codex_core::CodexSpawnOk; use codex_core::ModelProviderInfo; +use codex_core::WireApi; use codex_core::built_in_model_providers; use codex_core::protocol::EventMsg; use codex_core::protocol::InputItem; @@ -21,8 +22,10 @@ use tempfile::TempDir; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; +use wiremock::matchers::header_regex; use wiremock::matchers::method; use wiremock::matchers::path; +use wiremock::matchers::query_param; /// Build minimal SSE stream with completed marker using the JSON fixture. fn sse_completed(id: &str) -> String { @@ -373,9 +376,90 @@ async fn includes_user_instructions_message_in_request() { request_body["input"][0]["content"][0]["text"] .as_str() .unwrap() - .starts_with("be nice") + .starts_with("\n\nbe nice") + ); + assert!( + request_body["input"][0]["content"][0]["text"] + .as_str() + .unwrap() + .ends_with("") ); } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn azure_overrides_assign_properties_used_for_responses_url() { + #![allow(clippy::unwrap_used)] + + let existing_env_var_with_random_value = if cfg!(windows) { "USERNAME" } else { "USER" }; + + // Mock server + let server = MockServer::start().await; + + // First request – must NOT include `previous_response_id`. + let first = ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_raw(sse_completed("resp1"), "text/event-stream"); + + // Expect POST to /openai/responses with api-version query param + Mock::given(method("POST")) + .and(path("/openai/responses")) + .and(query_param("api-version", "2025-04-01-preview")) + .and(header_regex("Custom-Header", "Value")) + .and(header_regex( + "Authorization", + format!( + "Bearer {}", + std::env::var(existing_env_var_with_random_value).unwrap() + ) + .as_str(), + )) + .respond_with(first) + .expect(1) + .mount(&server) + .await; + + let provider = ModelProviderInfo { + name: "custom".to_string(), + base_url: Some(format!("{}/openai", server.uri())), + // Reuse the existing environment variable to avoid using unsafe code + env_key: Some(existing_env_var_with_random_value.to_string()), + query_params: Some(std::collections::HashMap::from([( + "api-version".to_string(), + "2025-04-01-preview".to_string(), + )])), + env_key_instructions: None, + wire_api: WireApi::Responses, + http_headers: Some(std::collections::HashMap::from([( + "Custom-Header".to_string(), + "Value".to_string(), + )])), + env_http_headers: None, + request_max_retries: None, + stream_max_retries: None, + stream_idle_timeout_ms: None, + requires_auth: false, + }; + + // Init session + let codex_home = TempDir::new().unwrap(); + let mut config = load_default_config_for_test(&codex_home); + config.model_provider = provider; + + let ctrl_c = std::sync::Arc::new(tokio::sync::Notify::new()); + let CodexSpawnOk { codex, .. } = Codex::spawn(config, None, ctrl_c.clone()).await.unwrap(); + + codex + .submit(Op::UserInput { + items: vec![InputItem::Text { + text: "hello".into(), + }], + }) + .await + .unwrap(); + + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; +} + fn auth_from_token(id_token: String) -> CodexAuth { CodexAuth::new( None, diff --git a/codex-rs/exec/src/event_processor.rs b/codex-rs/exec/src/event_processor.rs index 741f89d7cb..0f189f3fa2 100644 --- a/codex-rs/exec/src/event_processor.rs +++ b/codex-rs/exec/src/event_processor.rs @@ -44,20 +44,14 @@ pub(crate) fn create_config_summary_entries(config: &Config) -> Vec<(&'static st entries } -pub(crate) fn handle_last_message( - last_agent_message: Option<&str>, - last_message_path: Option<&Path>, -) { - match (last_message_path, last_agent_message) { - (Some(path), Some(msg)) => write_last_message_file(msg, Some(path)), - (Some(path), None) => { - write_last_message_file("", Some(path)); - eprintln!( - "Warning: no last agent message; wrote empty content to {}", - path.display() - ); - } - (None, _) => eprintln!("Warning: no file to write last message to."), +pub(crate) fn handle_last_message(last_agent_message: Option<&str>, output_file: &Path) { + let message = last_agent_message.unwrap_or_default(); + write_last_message_file(message, Some(output_file)); + if last_agent_message.is_none() { + eprintln!( + "Warning: no last agent message; wrote empty content to {}", + output_file.display() + ); } } diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index c290d9336b..7703c138fc 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -170,10 +170,9 @@ impl EventProcessor for EventProcessorWithHumanOutput { // Ignore. } EventMsg::TaskComplete(TaskCompleteEvent { last_agent_message }) => { - handle_last_message( - last_agent_message.as_deref(), - self.last_message_path.as_deref(), - ); + if let Some(output_file) = self.last_message_path.as_deref() { + handle_last_message(last_agent_message.as_deref(), output_file); + } return CodexStatus::InitiateShutdown; } EventMsg::TokenCount(TokenUsage { total_tokens, .. }) => { diff --git a/codex-rs/exec/src/event_processor_with_json_output.rs b/codex-rs/exec/src/event_processor_with_json_output.rs index e7a658b76f..1d153add6e 100644 --- a/codex-rs/exec/src/event_processor_with_json_output.rs +++ b/codex-rs/exec/src/event_processor_with_json_output.rs @@ -46,10 +46,9 @@ impl EventProcessor for EventProcessorWithJsonOutput { CodexStatus::Running } EventMsg::TaskComplete(TaskCompleteEvent { last_agent_message }) => { - handle_last_message( - last_agent_message.as_deref(), - self.last_message_path.as_deref(), - ); + if let Some(output_file) = self.last_message_path.as_deref() { + handle_last_message(last_agent_message.as_deref(), output_file); + } CodexStatus::InitiateShutdown } EventMsg::ShutdownComplete => CodexStatus::Shutdown, diff --git a/codex-rs/login/src/lib.rs b/codex-rs/login/src/lib.rs index 3d55c20264..35f67e7109 100644 --- a/codex-rs/login/src/lib.rs +++ b/codex-rs/login/src/lib.rs @@ -22,7 +22,7 @@ const SOURCE_FOR_PYTHON_SERVER: &str = include_str!("./login_with_chatgpt.py"); const CLIENT_ID: &str = "app_EMoamEEZ73f0CkXaXp7hrann"; pub const OPENAI_API_KEY_ENV_VAR: &str = "OPENAI_API_KEY"; -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Copy)] pub enum AuthMode { ApiKey, ChatGPT, diff --git a/codex-rs/login/src/login_with_chatgpt.py b/codex-rs/login/src/login_with_chatgpt.py index 4c07feeba0..14ccfa9ed4 100644 --- a/codex-rs/login/src/login_with_chatgpt.py +++ b/codex-rs/login/src/login_with_chatgpt.py @@ -458,6 +458,7 @@ class _ApiKeyHTTPServer(http.server.HTTPServer): "code_challenge": self.pkce.code_challenge, "code_challenge_method": "S256", "id_token_add_organizations": "true", + "codex_cli_simplified_flow": "true", "state": self.state, } return f"{self.issuer}/oauth/authorize?" + urllib.parse.urlencode(params)