Restore API key and query param overrides (#1826)
Addresses https://github.com/openai/codex/issues/1796
This commit is contained in:
@@ -120,7 +120,7 @@ pub(crate) async fn stream_chat_completions(
|
|||||||
|
|
||||||
debug!(
|
debug!(
|
||||||
"POST to {}: {}",
|
"POST to {}: {}",
|
||||||
provider.get_full_url(),
|
provider.get_full_url(&None),
|
||||||
serde_json::to_string_pretty(&payload).unwrap_or_default()
|
serde_json::to_string_pretty(&payload).unwrap_or_default()
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -129,7 +129,7 @@ pub(crate) async fn stream_chat_completions(
|
|||||||
loop {
|
loop {
|
||||||
attempt += 1;
|
attempt += 1;
|
||||||
|
|
||||||
let req_builder = provider.create_request_builder(client)?;
|
let req_builder = provider.create_request_builder(client, &None).await?;
|
||||||
|
|
||||||
let res = req_builder
|
let res = req_builder
|
||||||
.header(reqwest::header::ACCEPT, "text/event-stream")
|
.header(reqwest::header::ACCEPT, "text/event-stream")
|
||||||
|
|||||||
@@ -30,7 +30,6 @@ use crate::config::Config;
|
|||||||
use crate::config_types::ReasoningEffort as ReasoningEffortConfig;
|
use crate::config_types::ReasoningEffort as ReasoningEffortConfig;
|
||||||
use crate::config_types::ReasoningSummary as ReasoningSummaryConfig;
|
use crate::config_types::ReasoningSummary as ReasoningSummaryConfig;
|
||||||
use crate::error::CodexErr;
|
use crate::error::CodexErr;
|
||||||
use crate::error::EnvVarError;
|
|
||||||
use crate::error::Result;
|
use crate::error::Result;
|
||||||
use crate::flags::CODEX_RS_SSE_FIXTURE;
|
use crate::flags::CODEX_RS_SSE_FIXTURE;
|
||||||
use crate::model_provider_info::ModelProviderInfo;
|
use crate::model_provider_info::ModelProviderInfo;
|
||||||
@@ -122,24 +121,11 @@ impl ModelClient {
|
|||||||
return stream_from_fixture(path, self.provider.clone()).await;
|
return stream_from_fixture(path, self.provider.clone()).await;
|
||||||
}
|
}
|
||||||
|
|
||||||
let auth = self.auth.as_ref().ok_or_else(|| {
|
let auth = self.auth.clone();
|
||||||
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 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() {
|
let store = prompt.store && auth_mode != Some(AuthMode::ChatGPT);
|
||||||
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 full_instructions = prompt.get_full_instructions(&self.config.model);
|
let full_instructions = prompt.get_full_instructions(&self.config.model);
|
||||||
let tools_json = create_tools_json_for_responses_api(
|
let tools_json = create_tools_json_for_responses_api(
|
||||||
@@ -180,35 +166,36 @@ impl ModelClient {
|
|||||||
include,
|
include,
|
||||||
};
|
};
|
||||||
|
|
||||||
trace!(
|
|
||||||
"POST to {}: {}",
|
|
||||||
self.provider.get_full_url(),
|
|
||||||
serde_json::to_string(&payload)?
|
|
||||||
);
|
|
||||||
|
|
||||||
let mut attempt = 0;
|
let mut attempt = 0;
|
||||||
let max_retries = self.provider.request_max_retries();
|
let max_retries = self.provider.request_max_retries();
|
||||||
|
|
||||||
|
trace!(
|
||||||
|
"POST to {}: {}",
|
||||||
|
self.provider.get_full_url(&auth),
|
||||||
|
serde_json::to_string(&payload)?
|
||||||
|
);
|
||||||
|
|
||||||
loop {
|
loop {
|
||||||
attempt += 1;
|
attempt += 1;
|
||||||
|
|
||||||
let mut req_builder = self
|
let mut req_builder = self
|
||||||
.client
|
.provider
|
||||||
.post(format!("{base_url}/responses"))
|
.create_request_builder(&self.client, &auth)
|
||||||
|
.await?;
|
||||||
|
|
||||||
|
req_builder = req_builder
|
||||||
.header("OpenAI-Beta", "responses=experimental")
|
.header("OpenAI-Beta", "responses=experimental")
|
||||||
.header("session_id", self.session_id.to_string())
|
.header("session_id", self.session_id.to_string())
|
||||||
.bearer_auth(&token)
|
|
||||||
.header(reqwest::header::ACCEPT, "text/event-stream")
|
.header(reqwest::header::ACCEPT, "text/event-stream")
|
||||||
.json(&payload);
|
.json(&payload);
|
||||||
|
|
||||||
if auth.mode == AuthMode::ChatGPT {
|
if let Some(auth) = auth.as_ref()
|
||||||
if let Some(account_id) = auth.get_account_id().await {
|
&& auth.mode == AuthMode::ChatGPT
|
||||||
req_builder = req_builder.header("chatgpt-account-id", account_id);
|
&& 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
|
let originator = self
|
||||||
.config
|
.config
|
||||||
.internal_originator
|
.internal_originator
|
||||||
|
|||||||
@@ -5,8 +5,11 @@
|
|||||||
//! 2. User-defined entries inside `~/.codex/config.toml` under the `model_providers`
|
//! 2. User-defined entries inside `~/.codex/config.toml` under the `model_providers`
|
||||||
//! key. These override or extend the defaults at runtime.
|
//! key. These override or extend the defaults at runtime.
|
||||||
|
|
||||||
|
use codex_login::AuthMode;
|
||||||
|
use codex_login::CodexAuth;
|
||||||
use serde::Deserialize;
|
use serde::Deserialize;
|
||||||
use serde::Serialize;
|
use serde::Serialize;
|
||||||
|
use std::borrow::Cow;
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
use std::env::VarError;
|
use std::env::VarError;
|
||||||
use std::time::Duration;
|
use std::time::Duration;
|
||||||
@@ -88,25 +91,30 @@ impl ModelProviderInfo {
|
|||||||
/// When `require_api_key` is true and the provider declares an `env_key`
|
/// 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
|
/// but the variable is missing/empty, returns an [`Err`] identical to the
|
||||||
/// one produced by [`ModelProviderInfo::api_key`].
|
/// one produced by [`ModelProviderInfo::api_key`].
|
||||||
pub fn create_request_builder<'a>(
|
pub async fn create_request_builder<'a>(
|
||||||
&'a self,
|
&'a self,
|
||||||
client: &'a reqwest::Client,
|
client: &'a reqwest::Client,
|
||||||
|
auth: &Option<CodexAuth>,
|
||||||
) -> crate::error::Result<reqwest::RequestBuilder> {
|
) -> crate::error::Result<reqwest::RequestBuilder> {
|
||||||
let url = self.get_full_url();
|
let auth: Cow<'_, Option<CodexAuth>> = 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 mut builder = client.post(url);
|
||||||
|
|
||||||
let api_key = self.api_key()?;
|
if let Some(auth) = auth.as_ref() {
|
||||||
if let Some(key) = api_key {
|
builder = builder.bearer_auth(auth.get_token().await?);
|
||||||
builder = builder.bearer_auth(key);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
Ok(self.apply_http_headers(builder))
|
Ok(self.apply_http_headers(builder))
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn get_full_url(&self) -> String {
|
fn get_query_string(&self) -> String {
|
||||||
let query_string = self
|
self.query_params
|
||||||
.query_params
|
|
||||||
.as_ref()
|
.as_ref()
|
||||||
.map_or_else(String::new, |params| {
|
.map_or_else(String::new, |params| {
|
||||||
let full_params = params
|
let full_params = params
|
||||||
@@ -115,16 +123,29 @@ impl ModelProviderInfo {
|
|||||||
.collect::<Vec<_>>()
|
.collect::<Vec<_>>()
|
||||||
.join("&");
|
.join("&");
|
||||||
format!("?{full_params}")
|
format!("?{full_params}")
|
||||||
});
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn get_full_url(&self, auth: &Option<CodexAuth>) -> 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
|
let base_url = self
|
||||||
.base_url
|
.base_url
|
||||||
.clone()
|
.clone()
|
||||||
.unwrap_or("https://api.openai.com/v1".to_string());
|
.unwrap_or(default_base_url.to_string());
|
||||||
|
|
||||||
match self.wire_api {
|
match self.wire_api {
|
||||||
WireApi::Responses => {
|
WireApi::Responses => format!("{base_url}/responses{query_string}"),
|
||||||
format!("{base_url}/responses{query_string}")
|
|
||||||
}
|
|
||||||
WireApi::Chat => format!("{base_url}/chat/completions{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)
|
/// Apply provider-specific HTTP headers (both static and environment-based)
|
||||||
/// onto an existing `reqwest::RequestBuilder` and return the updated
|
/// onto an existing `reqwest::RequestBuilder` and return the updated
|
||||||
/// builder.
|
/// builder.
|
||||||
pub fn apply_http_headers(
|
fn apply_http_headers(&self, mut builder: reqwest::RequestBuilder) -> reqwest::RequestBuilder {
|
||||||
&self,
|
|
||||||
mut builder: reqwest::RequestBuilder,
|
|
||||||
) -> reqwest::RequestBuilder {
|
|
||||||
if let Some(extra) = &self.http_headers {
|
if let Some(extra) = &self.http_headers {
|
||||||
for (k, v) in extra {
|
for (k, v) in extra {
|
||||||
builder = builder.header(k, v);
|
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
|
/// 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
|
/// (and non-empty) in the environment. If `env_key` is required but
|
||||||
/// cannot be found, returns an error.
|
/// cannot be found, returns an error.
|
||||||
fn api_key(&self) -> crate::error::Result<Option<String>> {
|
pub fn api_key(&self) -> crate::error::Result<Option<String>> {
|
||||||
match &self.env_key {
|
match &self.env_key {
|
||||||
Some(env_key) => {
|
Some(env_key) => {
|
||||||
let env_value = std::env::var(env_key);
|
let env_value = std::env::var(env_key);
|
||||||
@@ -198,6 +216,14 @@ impl ModelProviderInfo {
|
|||||||
.map(Duration::from_millis)
|
.map(Duration::from_millis)
|
||||||
.unwrap_or(Duration::from_millis(DEFAULT_STREAM_IDLE_TIMEOUT_MS))
|
.unwrap_or(Duration::from_millis(DEFAULT_STREAM_IDLE_TIMEOUT_MS))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn get_fallback_auth(&self) -> crate::error::Result<Option<CodexAuth>> {
|
||||||
|
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.
|
/// Built-in default provider list.
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ use chrono::Utc;
|
|||||||
use codex_core::Codex;
|
use codex_core::Codex;
|
||||||
use codex_core::CodexSpawnOk;
|
use codex_core::CodexSpawnOk;
|
||||||
use codex_core::ModelProviderInfo;
|
use codex_core::ModelProviderInfo;
|
||||||
|
use codex_core::WireApi;
|
||||||
use codex_core::built_in_model_providers;
|
use codex_core::built_in_model_providers;
|
||||||
use codex_core::protocol::EventMsg;
|
use codex_core::protocol::EventMsg;
|
||||||
use codex_core::protocol::InputItem;
|
use codex_core::protocol::InputItem;
|
||||||
@@ -21,8 +22,10 @@ use tempfile::TempDir;
|
|||||||
use wiremock::Mock;
|
use wiremock::Mock;
|
||||||
use wiremock::MockServer;
|
use wiremock::MockServer;
|
||||||
use wiremock::ResponseTemplate;
|
use wiremock::ResponseTemplate;
|
||||||
|
use wiremock::matchers::header_regex;
|
||||||
use wiremock::matchers::method;
|
use wiremock::matchers::method;
|
||||||
use wiremock::matchers::path;
|
use wiremock::matchers::path;
|
||||||
|
use wiremock::matchers::query_param;
|
||||||
|
|
||||||
/// Build minimal SSE stream with completed marker using the JSON fixture.
|
/// Build minimal SSE stream with completed marker using the JSON fixture.
|
||||||
fn sse_completed(id: &str) -> String {
|
fn sse_completed(id: &str) -> String {
|
||||||
@@ -376,6 +379,81 @@ async fn includes_user_instructions_message_in_request() {
|
|||||||
.starts_with("be nice")
|
.starts_with("be nice")
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[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 {
|
fn auth_from_token(id_token: String) -> CodexAuth {
|
||||||
CodexAuth::new(
|
CodexAuth::new(
|
||||||
None,
|
None,
|
||||||
|
|||||||
@@ -22,7 +22,7 @@ const SOURCE_FOR_PYTHON_SERVER: &str = include_str!("./login_with_chatgpt.py");
|
|||||||
const CLIENT_ID: &str = "app_EMoamEEZ73f0CkXaXp7hrann";
|
const CLIENT_ID: &str = "app_EMoamEEZ73f0CkXaXp7hrann";
|
||||||
pub const OPENAI_API_KEY_ENV_VAR: &str = "OPENAI_API_KEY";
|
pub const OPENAI_API_KEY_ENV_VAR: &str = "OPENAI_API_KEY";
|
||||||
|
|
||||||
#[derive(Clone, Debug, PartialEq)]
|
#[derive(Clone, Debug, PartialEq, Copy)]
|
||||||
pub enum AuthMode {
|
pub enum AuthMode {
|
||||||
ApiKey,
|
ApiKey,
|
||||||
ChatGPT,
|
ChatGPT,
|
||||||
|
|||||||
Reference in New Issue
Block a user