Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions codex-rs/core/src/models_manager/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use std::sync::Arc;
use std::time::Duration;
use tokio::sync::RwLock;
use tokio::sync::TryLockError;
use tokio::time::timeout;
use tracing::error;

use super::cache;
Expand All @@ -21,6 +22,7 @@ use crate::api_bridge::map_api_error;
use crate::auth::AuthManager;
use crate::config::Config;
use crate::default_client::build_reqwest_client;
use crate::error::CodexErr;
use crate::error::Result as CoreResult;
use crate::features::Feature;
use crate::model_provider_info::ModelProviderInfo;
Expand All @@ -29,6 +31,7 @@ use crate::models_manager::model_presets::builtin_model_presets;

const MODEL_CACHE_FILE: &str = "models_cache.json";
const DEFAULT_MODEL_CACHE_TTL: Duration = Duration::from_secs(300);
const MODELS_REFRESH_TIMEOUT: Duration = Duration::from_secs(5);
const OPENAI_DEFAULT_API_MODEL: &str = "gpt-5.1-codex-max";
const OPENAI_DEFAULT_CHATGPT_MODEL: &str = "gpt-5.2-codex";
const CODEX_AUTO_BALANCED_MODEL: &str = "codex-auto-balanced";
Expand Down Expand Up @@ -105,10 +108,13 @@ impl ModelsManager {
let client = ModelsClient::new(transport, api_provider, api_auth);

let client_version = format_client_version_to_whole();
let (models, etag) = client
.list_models(&client_version, HeaderMap::new())
.await
.map_err(map_api_error)?;
let (models, etag) = timeout(
MODELS_REFRESH_TIMEOUT,
client.list_models(&client_version, HeaderMap::new()),
)
.await
.map_err(|_| CodexErr::Timeout)?
.map_err(map_api_error)?;

self.apply_remote_models(models.clone()).await;
*self.etag.write().await = etag.clone();
Expand Down
19 changes: 19 additions & 0 deletions codex-rs/core/tests/common/responses.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::sync::Arc;
use std::sync::Mutex;
use std::time::Duration;

use anyhow::Result;
use base64::Engine;
Expand Down Expand Up @@ -670,6 +671,24 @@ pub async fn mount_models_once(server: &MockServer, body: ModelsResponse) -> Mod
models_mock
}

pub async fn mount_models_once_with_delay(
server: &MockServer,
body: ModelsResponse,
delay: Duration,
) -> ModelsMock {
let (mock, models_mock) = models_mock();
mock.respond_with(
ResponseTemplate::new(200)
.insert_header("content-type", "application/json")
.set_body_json(body.clone())
.set_delay(delay),
)
.up_to_n_times(1)
.mount(server)
.await;
models_mock
}

pub async fn mount_models_once_with_etag(
server: &MockServer,
body: ModelsResponse,
Expand Down
72 changes: 72 additions & 0 deletions codex-rs/core/tests/suite/remote_models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use codex_core::ModelProviderInfo;
use codex_core::ThreadManager;
use codex_core::built_in_model_providers;
use codex_core::config::Config;
use codex_core::error::CodexErr;
use codex_core::features::Feature;
use codex_core::models_manager::manager::ModelsManager;
use codex_core::protocol::AskForApproval;
Expand All @@ -32,6 +33,7 @@ use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_function_call;
use core_test_support::responses::ev_response_created;
use core_test_support::responses::mount_models_once;
use core_test_support::responses::mount_models_once_with_delay;
use core_test_support::responses::mount_sse_once;
use core_test_support::responses::mount_sse_sequence;
use core_test_support::responses::sse;
Expand All @@ -45,6 +47,7 @@ use tempfile::TempDir;
use tokio::time::Duration;
use tokio::time::Instant;
use tokio::time::sleep;
use tokio::time::timeout;
use wiremock::BodyPrintLimit;
use wiremock::MockServer;

Expand Down Expand Up @@ -442,6 +445,75 @@ async fn remote_models_preserve_builtin_presets() -> Result<()> {
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn remote_models_request_times_out_after_5s() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));

let server = MockServer::start().await;
let remote_model = test_remote_model("remote-timeout", ModelVisibility::List, 0);
let models_mock = mount_models_once_with_delay(
&server,
ModelsResponse {
models: vec![remote_model],
},
Duration::from_secs(6),
)
.await;

let codex_home = TempDir::new()?;
let mut config = load_default_config_for_test(&codex_home).await;
config.features.enable(Feature::RemoteModels);

let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
let provider = ModelProviderInfo {
base_url: Some(format!("{}/v1", server.uri())),
..built_in_model_providers()["openai"].clone()
};
let manager = ModelsManager::with_provider(
codex_home.path().to_path_buf(),
codex_core::auth::AuthManager::from_auth_for_testing(auth),
provider,
);

let start = Instant::now();
let refresh = timeout(
Duration::from_secs(7),
manager.refresh_available_models_with_cache(&config),
)
.await;
let elapsed = start.elapsed();
let err = refresh
.expect("refresh should finish")
.expect_err("refresh should time out");
let request_summaries: Vec<String> = server
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a lot of code for an error message

.received_requests()
.await
.expect("mock server should capture requests")
.iter()
.map(|req| format!("{} {}", req.method, req.url.path()))
.collect();
assert!(
elapsed >= Duration::from_millis(4_500),
"expected models call to block near the timeout; took {elapsed:?}"
);
assert!(
elapsed < Duration::from_millis(5_800),
"expected models call to time out before the delayed response; took {elapsed:?}"
);
match err {
CodexErr::Timeout => {}
other => panic!("expected timeout error, got {other:?}; requests: {request_summaries:?}"),
}
assert_eq!(
models_mock.requests().len(),
1,
"expected a single /models request"
);

Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn remote_models_hide_picker_only_models() -> Result<()> {
skip_if_no_network!(Ok(()));
Expand Down
Loading