diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 20a55a69ac..8eb7df52fa 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -427,7 +427,7 @@ dependencies = [ "codex-exec-server", "codex-features", "codex-login", - "codex-models-manager", + "codex-model-provider", "codex-protocol", "codex-utils-cargo-bin", "core_test_support", diff --git a/codex-rs/app-server/tests/common/Cargo.toml b/codex-rs/app-server/tests/common/Cargo.toml index 8dd43051c7..8efd1763c8 100644 --- a/codex-rs/app-server/tests/common/Cargo.toml +++ b/codex-rs/app-server/tests/common/Cargo.toml @@ -21,7 +21,7 @@ codex-core = { workspace = true } codex-exec-server = { workspace = true } codex-features = { workspace = true } codex-login = { workspace = true } -codex-models-manager = { workspace = true } +codex-model-provider = { workspace = true } codex-protocol = { workspace = true } codex-utils-cargo-bin = { workspace = true } serde = { workspace = true } diff --git a/codex-rs/app-server/tests/common/models_cache.rs b/codex-rs/app-server/tests/common/models_cache.rs index 1bd6f316d7..6f44943e3d 100644 --- a/codex-rs/app-server/tests/common/models_cache.rs +++ b/codex-rs/app-server/tests/common/models_cache.rs @@ -1,7 +1,4 @@ -use chrono::DateTime; -use chrono::Utc; use codex_core::test_support::all_model_presets; -use codex_models_manager::client_version_to_whole; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::openai_models::ConfigShellToolType; use codex_protocol::openai_models::ModelInfo; @@ -10,7 +7,6 @@ use codex_protocol::openai_models::ModelPreset; use codex_protocol::openai_models::ModelVisibility; use codex_protocol::openai_models::TruncationPolicyConfig; use codex_protocol::openai_models::default_input_modalities; -use serde_json::json; use std::path::Path; /// Convert a ModelPreset to ModelInfo for cache storage. @@ -85,7 +81,7 @@ fn preset_to_info(preset: &ModelPreset, priority: i32) -> ModelInfo { /// This prevents ModelsManager from making network requests to refresh models. /// The cache will be treated as fresh (within TTL) and used instead of fetching from the network. /// Uses bundled-catalog-derived presets, converted to ModelInfo format. -pub fn write_models_cache(codex_home: &Path) -> std::io::Result<()> { +pub async fn write_models_cache(codex_home: &Path) -> std::io::Result<()> { // Get a stable bundled-catalog-derived preset list and filter for picker-visible entries. let presets: Vec<&ModelPreset> = all_model_presets() .iter() @@ -103,24 +99,33 @@ pub fn write_models_cache(codex_home: &Path) -> std::io::Result<()> { }) .collect(); - write_models_cache_with_models(codex_home, models) + write_models_cache_with_models(codex_home, models).await } /// Write a models_cache.json file with specific models. /// Useful when tests need specific models to be available. -pub fn write_models_cache_with_models( +pub async fn write_models_cache_with_models( codex_home: &Path, models: Vec, ) -> std::io::Result<()> { + let config = codex_core::config::ConfigBuilder::default() + .loader_overrides(codex_config::LoaderOverrides::without_managed_config_for_tests()) + .codex_home(codex_home.to_path_buf()) + .build() + .await?; + let auth = codex_login::CodexAuth::from_auth_storage( + codex_home, + config.cli_auth_credentials_store_mode, + Some(&config.chatgpt_base_url), + config.auth_keyring_backend_kind(), + &codex_login::test_support::transport_default_auth_route_config(), + ) + .await?; + let cache = codex_model_provider::test_support::models_cache_entry( + &config.model_provider, + auth.as_ref(), + models, + ); let cache_path = codex_home.join("models_cache.json"); - // DateTime serializes to RFC3339 format by default with serde - let fetched_at: DateTime = Utc::now(); - let client_version = client_version_to_whole(); - let cache = json!({ - "fetched_at": fetched_at, - "etag": null, - "client_version": client_version, - "models": models - }); std::fs::write(cache_path, serde_json::to_string_pretty(&cache)?) } diff --git a/codex-rs/app-server/tests/suite/logging.rs b/codex-rs/app-server/tests/suite/logging.rs index f1a6dc8c92..9c62bb5b88 100644 --- a/codex-rs/app-server/tests/suite/logging.rs +++ b/codex-rs/app-server/tests/suite/logging.rs @@ -97,7 +97,7 @@ supports_websockets = false "# )) .write(codex_home.path())?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let mut app_server = TestAppServer::builder() .with_codex_home(codex_home.path()) .with_env_overrides(&[("OPENAI_API_KEY", None)]) diff --git a/codex-rs/app-server/tests/suite/v2/account.rs b/codex-rs/app-server/tests/suite/v2/account.rs index 3b67408538..6db61e21fe 100644 --- a/codex-rs/app-server/tests/suite/v2/account.rs +++ b/codex-rs/app-server/tests/suite/v2/account.rs @@ -416,7 +416,7 @@ async fn set_auth_token_updates_account_and_notifies() -> Result<()> { ..Default::default() }, )?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let access_token = encode_id_token( &ChatGptIdTokenClaims::new() @@ -499,7 +499,7 @@ async fn account_read_refresh_token_is_noop_in_external_mode() -> Result<()> { ..Default::default() }, )?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let access_token = encode_id_token( &ChatGptIdTokenClaims::new() @@ -611,7 +611,7 @@ async fn external_auth_refreshes_on_unauthorized() -> Result<()> { ..Default::default() }, )?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let success_sse = responses::sse(vec![ responses::ev_response_created("resp-turn"), @@ -726,7 +726,7 @@ async fn external_auth_refresh_error_fails_turn() -> Result<()> { ..Default::default() }, )?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let unauthorized = ResponseTemplate::new(401).set_body_json(json!({ "error": { "message": "unauthorized" } @@ -837,7 +837,7 @@ async fn external_auth_refresh_mismatched_workspace_fails_turn() -> Result<()> { ..Default::default() }, )?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let unauthorized = ResponseTemplate::new(401).set_body_json(json!({ "error": { "message": "unauthorized" } @@ -953,7 +953,7 @@ async fn external_auth_refresh_invalid_access_token_fails_turn() -> Result<()> { ..Default::default() }, )?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let unauthorized = ResponseTemplate::new(401).set_body_json(json!({ "error": { "message": "unauthorized" } @@ -2003,7 +2003,7 @@ async fn login_account_chatgpt_device_code_returns_error_when_disabled() -> Resu ..Default::default() }, )?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; mock_device_code_usercode_failure(&mock_server, /*status*/ 404).await; let issuer = mock_server.uri(); @@ -2059,7 +2059,7 @@ async fn login_account_chatgpt_device_code_succeeds_and_notifies() -> Result<()> ..Default::default() }, )?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; mock_device_code_usercode(&mock_server, /*interval_seconds*/ 0).await; mock_device_code_token_success(&mock_server).await; @@ -2139,7 +2139,7 @@ async fn login_account_chatgpt_device_code_failure_notifies_without_account_upda ..Default::default() }, )?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; mock_device_code_usercode(&mock_server, /*interval_seconds*/ 0).await; mock_device_code_token_failure(&mock_server, /*status*/ 500).await; @@ -2210,7 +2210,7 @@ async fn login_account_chatgpt_device_code_can_be_cancelled() -> Result<()> { ..Default::default() }, )?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; mock_device_code_usercode(&mock_server, /*interval_seconds*/ 1).await; mock_device_code_token_failure(&mock_server, /*status*/ 404).await; diff --git a/codex-rs/app-server/tests/suite/v2/attestation.rs b/codex-rs/app-server/tests/suite/v2/attestation.rs index 0260b05808..8b6b30b519 100644 --- a/codex-rs/app-server/tests/suite/v2/attestation.rs +++ b/codex-rs/app-server/tests/suite/v2/attestation.rs @@ -56,7 +56,6 @@ async fn attestation_generate_round_trip_adds_header_to_responses_websocket_hand .await; let codex_home = TempDir::new()?; - write_models_cache(codex_home.path())?; create_chatgpt_websocket_config( codex_home.path(), &websocket_server.uri().replacen("ws://", "http://", 1), @@ -66,6 +65,7 @@ async fn attestation_generate_round_trip_adds_header_to_responses_websocket_hand ChatGptAuthFixture::new("access-chatgpt").plan_type("pro"), AuthCredentialsStoreMode::File, )?; + write_models_cache(codex_home.path()).await?; let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) diff --git a/codex-rs/app-server/tests/suite/v2/dynamic_tools.rs b/codex-rs/app-server/tests/suite/v2/dynamic_tools.rs index 349f4573d1..38cad9c430 100644 --- a/codex-rs/app-server/tests/suite/v2/dynamic_tools.rs +++ b/codex-rs/app-server/tests/suite/v2/dynamic_tools.rs @@ -601,7 +601,7 @@ async fn start_function_dynamic_tool_call(call_id: &str) -> Result Result<()> let mut model = codex_core::test_support::construct_model_info_offline(MODEL, &config); // Strict MCP approvals must work under the inline delegate's `never` policy. model.node_repl_auto_review_required = true; - write_models_cache_with_models(codex_home.path(), vec![model])?; + write_models_cache_with_models(codex_home.path(), vec![model]).await?; let mut app = TestAppServer::builder() .with_codex_home(codex_home.path()) .build_initialized_with_timeout(TIMEOUT) diff --git a/codex-rs/app-server/tests/suite/v2/guardian_v2.rs b/codex-rs/app-server/tests/suite/v2/guardian_v2.rs index 05f56210a2..8cb99e9ac2 100644 --- a/codex-rs/app-server/tests/suite/v2/guardian_v2.rs +++ b/codex-rs/app-server/tests/suite/v2/guardian_v2.rs @@ -131,7 +131,7 @@ async fn resumed_thread_does_not_wait_for_guardian_websocket_warmup() -> Result< let config = load_default_config_for_test(&codex_home).await; let mut model = codex_core::test_support::construct_model_info_offline(MODEL, &config); model.node_repl_auto_review_required = true; - write_models_cache_with_models(codex_home.path(), vec![model])?; + write_models_cache_with_models(codex_home.path(), vec![model]).await?; let thread_id = create_fake_rollout( codex_home.path(), "2025-01-05T12-00-00", @@ -833,7 +833,7 @@ async fn guardian_v2_routes_scoped_tool_approvals( let config = load_default_config_for_test(&codex_home).await; let mut model_info = codex_core::test_support::construct_model_info_offline(MODEL, &config); model_info.node_repl_auto_review_required = true; - write_models_cache_with_models(codex_home.path(), vec![model_info])?; + write_models_cache_with_models(codex_home.path(), vec![model_info]).await?; } let original_thread_id = match lifecycle { ThreadLifecycle::New @@ -2042,7 +2042,7 @@ async fn first_cua_review_does_not_wait_for_initial_score( let config = load_default_config_for_test(&codex_home).await; let mut model_info = codex_core::test_support::construct_model_info_offline(MODEL, &config); model_info.node_repl_auto_review_required = true; - write_models_cache_with_models(codex_home.path(), vec![model_info])?; + write_models_cache_with_models(codex_home.path(), vec![model_info]).await?; let mut app_server = TestAppServer::builder() .with_codex_home(codex_home.path()) .build_initialized_with_timeout(TIMEOUT) @@ -2128,7 +2128,7 @@ async fn user_approval_skips_async_guardian_without_changing_other_modes() -> Re let config = load_default_config_for_test(&codex_home).await; let mut model_info = codex_core::test_support::construct_model_info_offline(MODEL, &config); model_info.node_repl_auto_review_required = true; - write_models_cache_with_models(codex_home.path(), vec![model_info])?; + write_models_cache_with_models(codex_home.path(), vec![model_info]).await?; let mut app_server = TestAppServer::builder() .with_codex_home(codex_home.path()) .build_initialized_with_timeout(TIMEOUT) diff --git a/codex-rs/app-server/tests/suite/v2/guardian_v2_history_tests.rs b/codex-rs/app-server/tests/suite/v2/guardian_v2_history_tests.rs index 9f626946b1..e5b5908ff3 100644 --- a/codex-rs/app-server/tests/suite/v2/guardian_v2_history_tests.rs +++ b/codex-rs/app-server/tests/suite/v2/guardian_v2_history_tests.rs @@ -314,12 +314,12 @@ async fn guardians_retain_evidence_after_compaction_and_discard_it_after_rollbac info }) .collect(); - write_models_cache_with_models(codex_home.path(), models)?; write_chatgpt_auth( codex_home.path(), ChatGptAuthFixture::new("access-chatgpt").plan_type("pro"), AuthCredentialsStoreMode::File, )?; + write_models_cache_with_models(codex_home.path(), models).await?; let mut app_server = TestAppServer::builder() .with_codex_home(codex_home.path()) .with_env_overrides(&[("OPENAI_API_KEY", None)]) diff --git a/codex-rs/app-server/tests/suite/v2/guardian_v2_model_tests.rs b/codex-rs/app-server/tests/suite/v2/guardian_v2_model_tests.rs index e2fb38d0ff..8e87490490 100644 --- a/codex-rs/app-server/tests/suite/v2/guardian_v2_model_tests.rs +++ b/codex-rs/app-server/tests/suite/v2/guardian_v2_model_tests.rs @@ -77,7 +77,7 @@ async fn computer_use_scoring_follows_model_review_requirement( let mut reviewed_model = codex_core::test_support::construct_model_info_offline(REVIEWED_MODEL, &config); reviewed_model.node_repl_auto_review_required = true; - write_models_cache_with_models(codex_home.path(), vec![ordinary_model, reviewed_model])?; + write_models_cache_with_models(codex_home.path(), vec![ordinary_model, reviewed_model]).await?; let mut app_server = TestAppServer::builder() .with_codex_home(codex_home.path()) .build_initialized_with_timeout(TIMEOUT) diff --git a/codex-rs/app-server/tests/suite/v2/model_list.rs b/codex-rs/app-server/tests/suite/v2/model_list.rs index 7e21fb6d8e..fbc262a308 100644 --- a/codex-rs/app-server/tests/suite/v2/model_list.rs +++ b/codex-rs/app-server/tests/suite/v2/model_list.rs @@ -60,7 +60,7 @@ fn model_from_preset(preset: &ModelPreset) -> Model { .collect(), default_reasoning_effort: preset.default_reasoning_effort.clone(), input_modalities: preset.input_modalities.clone(), - // `write_models_cache()` round-trips through a simplified ModelInfo fixture that does not + // `write_models_cache().await` round-trips through a simplified ModelInfo fixture that does not // preserve personality placeholders in base instructions, so app-server list results from // cache report `supports_personality = false`. // todo(sayan): fix, maybe make roundtrip use ModelInfo only @@ -101,7 +101,7 @@ fn expected_visible_models() -> Vec { #[tokio::test] async fn list_models_returns_all_models_with_large_limit() -> Result<()> { let codex_home = TempDir::new()?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) .without_auto_env() @@ -131,7 +131,7 @@ async fn list_models_returns_all_models_with_large_limit() -> Result<()> { #[tokio::test] async fn list_models_includes_hidden_models() -> Result<()> { let codex_home = TempDir::new()?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) .without_auto_env() @@ -286,7 +286,7 @@ openai_base_url = "{server_uri}/v1" #[tokio::test] async fn list_models_pagination_works() -> Result<()> { let codex_home = TempDir::new()?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) .without_auto_env() @@ -332,7 +332,7 @@ async fn list_models_pagination_works() -> Result<()> { #[tokio::test] async fn list_models_rejects_invalid_cursor() -> Result<()> { let codex_home = TempDir::new()?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) .without_auto_env() diff --git a/codex-rs/app-server/tests/suite/v2/multi_agent_v2_developer_instructions.rs b/codex-rs/app-server/tests/suite/v2/multi_agent_v2_developer_instructions.rs index e5553bffdf..d4e0c71e43 100644 --- a/codex-rs/app-server/tests/suite/v2/multi_agent_v2_developer_instructions.rs +++ b/codex-rs/app-server/tests/suite/v2/multi_agent_v2_developer_instructions.rs @@ -198,7 +198,7 @@ async fn spawned_subagents_apply_configured_developer_instruction_precedence( config .with_extra_config(&feature_config) .write(codex_home.path())?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let mut app_server = TestAppServer::builder() .with_codex_home(codex_home.path()) .build_initialized() @@ -379,7 +379,7 @@ async fn compacted_full_history_fork_replaces_parent_developer_instructions() -> "[features.multi_agent_v2]\nenabled = true\nsubagent_developer_instructions = {CHILD_INSTRUCTIONS:?}" )) .write(codex_home.path())?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let mut app_server = TestAppServer::builder() .with_codex_home(codex_home.path()) @@ -641,7 +641,7 @@ async fn cold_resume_preserves_effective_developer_instructions_for_worker( )) .with_extra_config(&feature_config) .write(codex_home.path())?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let (thread_id, child_resume_params, baseline) = { let mut app_server = TestAppServer::builder() diff --git a/codex-rs/app-server/tests/suite/v2/otel.rs b/codex-rs/app-server/tests/suite/v2/otel.rs index bbaa3692be..197af70f4d 100644 --- a/codex-rs/app-server/tests/suite/v2/otel.rs +++ b/codex-rs/app-server/tests/suite/v2/otel.rs @@ -48,7 +48,7 @@ async fn account_switch_reloads_telemetry_collectors_and_preserves_trace_context .email(INITIAL_EMAIL), AuthCredentialsStoreMode::File, )?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let mut app_server = TestAppServer::builder() .with_codex_home(codex_home.path()) diff --git a/codex-rs/app-server/tests/suite/v2/residency.rs b/codex-rs/app-server/tests/suite/v2/residency.rs index 74cb932a67..89799dc4f7 100644 --- a/codex-rs/app-server/tests/suite/v2/residency.rs +++ b/codex-rs/app-server/tests/suite/v2/residency.rs @@ -27,7 +27,7 @@ enum ModelTransport { Websocket, } -fn write_provider_config( +async fn write_provider_config( codex_home: &Path, model_base_url: &str, transport: ModelTransport, @@ -60,7 +60,7 @@ supports_websockets = {supports_websockets} "# ); std::fs::write(codex_home.join("config.toml"), config)?; - write_models_cache(codex_home)?; + write_models_cache(codex_home).await?; Ok(()) } @@ -98,7 +98,7 @@ async fn managed_residency_overrides_provider_headers(transport: ModelTransport) }; let codex_home = TempDir::new()?; - write_provider_config(codex_home.path(), &model_base_url, transport)?; + write_provider_config(codex_home.path(), &model_base_url, transport).await?; std::fs::write( codex_home.path().join("requirements.toml"), "enforce_residency = \"us\"\n", diff --git a/codex-rs/app-server/tests/suite/v2/thread_fork_multi_agent_tests.rs b/codex-rs/app-server/tests/suite/v2/thread_fork_multi_agent_tests.rs index d8bde37558..305953ed83 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_fork_multi_agent_tests.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_fork_multi_agent_tests.rs @@ -47,7 +47,7 @@ async fn fork_before_first_turn_preserves_model_selected_multi_agent_version( let config = load_default_config_for_test(&codex_home).await; let mut model = codex_core::test_support::construct_model_info_offline("mock-model", &config); model.multi_agent_version = Some(MultiAgentVersion::V2); - write_models_cache_with_models(codex_home.path(), vec![model])?; + write_models_cache_with_models(codex_home.path(), vec![model]).await?; let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) .build_initialized() diff --git a/codex-rs/app-server/tests/suite/v2/thread_revert.rs b/codex-rs/app-server/tests/suite/v2/thread_revert.rs index 9bb8ff9281..a8a9cc8a49 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_revert.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_revert.rs @@ -63,7 +63,7 @@ async fn thread_revert_preserves_model_selected_multi_agent_version(restart: boo let config = load_default_config_for_test(&codex_home).await; let mut model = codex_core::test_support::construct_model_info_offline("mock-model", &config); model.multi_agent_version = Some(MultiAgentVersion::V2); - write_models_cache_with_models(codex_home.path(), vec![model])?; + write_models_cache_with_models(codex_home.path(), vec![model]).await?; let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) .build() diff --git a/codex-rs/app-server/tests/suite/v2/thread_settings_update.rs b/codex-rs/app-server/tests/suite/v2/thread_settings_update.rs index dee123f590..d51578dfdb 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_settings_update.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_settings_update.rs @@ -47,7 +47,7 @@ async fn thread_settings_update_emits_notification_and_updates_future_turns() -> .await; let codex_home = TempDir::new()?; create_config_toml(codex_home.path(), &server.uri())?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let (model_id, service_tier_id) = service_tier_model_and_tier_id()?; let mut mcp = TestAppServer::builder() @@ -289,7 +289,7 @@ async fn thread_settings_update_null_service_tier_uses_default() -> Result<()> { .await; let codex_home = TempDir::new()?; create_config_toml(codex_home.path(), &server.uri())?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let (model_id, service_tier_id) = service_tier_model_and_tier_id()?; let mut mcp = TestAppServer::builder() diff --git a/codex-rs/app-server/tests/suite/v2/turn_cost_otel.rs b/codex-rs/app-server/tests/suite/v2/turn_cost_otel.rs index 80e3f2eabf..48cfbe1d14 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_cost_otel.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_cost_otel.rs @@ -113,7 +113,7 @@ metrics_exporter = {{ otlp-http = {{ endpoint = "{}/metrics", protocol = "json" .chatgpt_user_id("user-a"), AuthCredentialsStoreMode::File, )?; - write_models_cache(home.path())?; + write_models_cache(home.path()).await?; let loader_overrides = LoaderOverrides::without_managed_config_for_tests(); let config = Arc::new( ConfigBuilder::default() diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index e567bceb4a..d5c35e3d74 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -777,7 +777,7 @@ async fn turn_start_emits_thread_scoped_warning_notification_for_trimmed_skills( MockResponsesConfig::new(&server.uri()) .enable_feature(Feature::Personality) .write(codex_home.path())?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let cache_path = codex_home.path().join("models_cache.json"); let mut cache: serde_json::Value = serde_json::from_str(&std::fs::read_to_string(&cache_path)?)?; @@ -874,7 +874,7 @@ async fn turn_start_sends_service_tier_id_to_model_request() -> Result<()> { let codex_home = TempDir::new()?; MockResponsesConfig::new(&server.uri()).write(codex_home.path())?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let service_tier_model = all_model_presets() .iter() .find(|preset| preset.show_in_picker && !preset.service_tiers.is_empty()) @@ -998,7 +998,7 @@ async fn turn_start_emits_raw_response_completed_with_upstream_usage( let codex_home = TempDir::new()?; MockResponsesConfig::new(&server.uri()).write(codex_home.path())?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) @@ -3978,7 +3978,7 @@ async fn turn_start_streams_apply_patch_change_updates_v2() -> Result<()> { .disable_feature(Feature::RemoteModels) .disable_feature(Feature::ShellSnapshot) .write(&codex_home)?; - write_models_cache(&codex_home)?; + write_models_cache(&codex_home).await?; let cache_path = codex_home.join("models_cache.json"); let mut cache: serde_json::Value = serde_json::from_str(&std::fs::read_to_string(&cache_path)?)?; @@ -4368,7 +4368,7 @@ async fn direct_input_to_multi_agent_v2_subagent_is_rejected( .enable_feature(Feature::Goals) .with_root_config(&format!("chatgpt_base_url = \"{}\"", server.uri())) .write(codex_home.path())?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; mount_analytics_capture(&server, codex_home.path()).await?; let mut mcp = TestAppServer::builder() diff --git a/codex-rs/app-server/tests/suite/v2/view_image.rs b/codex-rs/app-server/tests/suite/v2/view_image.rs index 2d88d82c61..8ea35efabd 100644 --- a/codex-rs/app-server/tests/suite/v2/view_image.rs +++ b/codex-rs/app-server/tests/suite/v2/view_image.rs @@ -50,7 +50,7 @@ async fn fresh_context_subagent_inherits_disabled_view_image_and_mcp_tools() -> "[mcp_servers.{TEST_SERVER_NAME}]\nurl = \"{mcp_server_url}/mcp\"\n\n[features.multi_agent_v2]\nenabled = true" )) .write(codex_home.path())?; - write_models_cache(codex_home.path())?; + write_models_cache(codex_home.path()).await?; let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) diff --git a/codex-rs/core/tests/suite/injected_models_cache.rs b/codex-rs/core/tests/suite/injected_models_cache.rs index 3d0d114c68..8d2ab86bd8 100644 --- a/codex-rs/core/tests/suite/injected_models_cache.rs +++ b/codex-rs/core/tests/suite/injected_models_cache.rs @@ -1,3 +1,5 @@ +//! Integration coverage for model catalogs supplied through the public cache interface. + use codex_core::TurnInputRequest; use std::sync::Arc; use std::sync::Mutex; @@ -5,10 +7,13 @@ use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; use anyhow::Result; +use base64::Engine; use chrono::Utc; use codex_http_client::HttpClientFactory; use codex_login::AuthManager; use codex_login::CodexAuth; +use codex_model_provider::create_model_provider; +use codex_model_provider_info::built_in_model_providers; use codex_models_manager::cache::ModelsCache; use codex_models_manager::cache::ModelsCacheEntry; use codex_models_manager::cache::ModelsCacheError; @@ -20,9 +25,11 @@ use codex_models_manager::manager::OpenAiModelsManager; use codex_models_manager::manager::RefreshStrategy; use codex_models_manager::manager::SharedModelsManager; use codex_models_manager::model_info::model_info_from_slug; +use codex_protocol::config_types::ReasoningSummary; use codex_protocol::error::Result as CoreResult; use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ModelVisibility; +use codex_protocol::openai_models::ModelsResponse; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ThreadSettingsOverrides; use codex_protocol::user_input::UserInput; @@ -36,12 +43,23 @@ use core_test_support::submit_thread_settings; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use pretty_assertions::assert_eq; +use tempfile::TempDir; +use tokio::sync::oneshot; +use tokio::time::Duration; +use tokio::time::timeout; + +#[derive(Debug)] +struct StoreGate { + entered: oneshot::Sender<()>, + release: oneshot::Receiver<()>, +} #[derive(Debug)] struct TestModelsCache { entry: Option, load_error: bool, stored_entries: Mutex>, + store_gate: Mutex>, } impl ModelsCache for TestModelsCache { @@ -53,7 +71,13 @@ impl ModelsCache for TestModelsCache { if self.load_error { Err(ModelsCacheError::new("test load failure")) } else { - Ok(self.entry.clone()) + Ok(self + .stored_entries + .lock() + .expect("stored entries lock should not be poisoned") + .last() + .cloned() + .or_else(|| self.entry.clone())) } }) } @@ -67,6 +91,15 @@ impl ModelsCache for TestModelsCache { .lock() .expect("stored entries lock should not be poisoned") .push(entry.clone()); + let gate = self + .store_gate + .lock() + .expect("store gate lock should not be poisoned") + .take(); + if let Some(StoreGate { entered, release }) = gate { + let _ = entered.send(()); + let _ = release.await; + } Ok(()) }) } @@ -74,6 +107,8 @@ impl ModelsCache for TestModelsCache { fn refresh_ttl<'a>( &'a self, _client_version: &'a str, + _identity: &'a str, + _etag: &'a str, ) -> ModelsCacheFuture<'a, Result<(), ModelsCacheError>> { Box::pin(async move { let mut entry = self @@ -227,6 +262,7 @@ async fn injected_cache_hit_drives_agent_model_selection() -> Result<()> { }), load_error: false, stored_entries: Mutex::new(Vec::new()), + store_gate: Mutex::new(None), }); let endpoint = TestModelsEndpoint::new(Vec::new()); @@ -243,6 +279,7 @@ async fn injected_cache_error_falls_back_for_agent_model_selection() -> Result<( entry: None, load_error: true, stored_entries: Mutex::new(Vec::new()), + store_gate: Mutex::new(None), }); let endpoint = TestModelsEndpoint::new(vec![remote_model(model_slug)]); @@ -258,3 +295,133 @@ async fn injected_cache_error_falls_back_for_agent_model_selection() -> Result<( ); Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn account_switch_during_cache_store_preserves_new_catalog_for_next_turn() -> Result<()> { + let server = wiremock::MockServer::start().await; + let home = Arc::new(TempDir::new()?); + let initial_auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let auth = AuthManager::from_auth_for_testing_with_home( + initial_auth.clone(), + home.path().to_path_buf(), + ); + let mut provider_info = built_in_model_providers(/*openai_base_url*/ None)["openai"].clone(); + provider_info.base_url = Some(format!("{}/v1", server.uri())); + provider_info.supports_websockets = false; + let cache = Arc::new(TestModelsCache { + entry: None, + load_error: false, + stored_entries: Mutex::new(Vec::new()), + store_gate: Mutex::new(None), + }); + let manager = create_model_provider(provider_info.clone(), Some(auth.clone())) + .models_manager_with_cache(/*config_model_catalog*/ None, cache.clone()); + let mut old_model = remote_model("gpt-5.4"); + old_model.default_reasoning_summary = ReasoningSummary::Concise; + let initial = responses::mount_models_once( + &server, + ModelsResponse { + models: vec![old_model.clone()], + }, + ) + .await; + let test = test_codex() + .with_home(home.clone()) + .with_auth(initial_auth.clone()) + .with_model("gpt-5.4") + .with_models_manager(manager.clone()) + .with_config(move |config| { + config.model_provider = provider_info; + config.model_reasoning_summary = None; + }) + .build_with_auto_env(&server) + .await?; + assert_eq!(initial.requests().len(), 1); + + // Pause after the old catalog has been stored, before its refresh can publish it. + let (store_entered, entered) = oneshot::channel(); + let (release_store, release) = oneshot::channel(); + *cache.store_gate.lock().unwrap() = Some(StoreGate { + entered: store_entered, + release, + }); + let old_request = responses::mount_models_once( + &server, + ModelsResponse { + models: vec![old_model], + }, + ) + .await; + let old_refresh = tokio::spawn({ + let manager = manager.clone(); + async move { + manager + .list_models( + RefreshStrategy::Online, + codex_core::test_support::default_http_client_factory(), + ) + .await + } + }); + timeout(Duration::from_secs(10), entered).await??; + + let mut tokens = initial_auth.get_token_data()?; + tokens.id_token.raw_jwt = format!( + "e30.{}.signature", + base64::engine::general_purpose::URL_SAFE_NO_PAD + .encode(br#"{"email":"second@example.com"}"#) + ); + tokens.account_id = Some("second-account".into()); + tokens.access_token = "second-account-token".into(); + std::fs::write( + home.path().join("auth.json"), + serde_json::to_vec(&serde_json::json!({ + "auth_mode": "chatgpt", "tokens": tokens, "last_refresh": Utc::now(), + }))?, + )?; + auth.reload().await; + test.thread_manager.auth_manager().reload().await; + + let mut new_model = remote_model("gpt-5.4"); + new_model.default_reasoning_summary = ReasoningSummary::Detailed; + let new_request = responses::mount_models_once( + &server, + ModelsResponse { + models: vec![new_model.clone()], + }, + ) + .await; + manager + .list_models( + RefreshStrategy::Online, + codex_core::test_support::default_http_client_factory(), + ) + .await; + assert_eq!(manager.get_remote_models().await, vec![new_model.clone()]); + release_store.send(()).unwrap(); + timeout(Duration::from_secs(10), old_refresh).await??; + assert_eq!(manager.get_remote_models().await, vec![new_model]); + for (mock, account) in [(old_request, "account_id"), (new_request, "second-account")] { + let requests = mock.requests(); + assert_eq!(requests.len(), 1); + assert_eq!(requests[0].headers["chatgpt-account-id"], account); + } + + let response = mount_sse_once( + &server, + sse(vec![ + ev_response_created("next-turn"), + ev_completed("next-turn"), + ]), + ) + .await; + test.submit_turn("use the new account's model defaults") + .await?; + let request = response.single_request(); + assert_eq!( + request.header("chatgpt-account-id").as_deref(), + Some("second-account") + ); + assert_eq!(request.body_json()["reasoning"]["summary"], "detailed"); + Ok(()) +} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 1c23e2aba9..95eb4a4e0b 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -108,6 +108,7 @@ mod model_overrides; mod model_runtime_selectors; mod model_switching; mod model_visible_layout; +mod models_cache_auth; mod models_cache_ttl; mod models_etag_responses; mod multi_agent_mode; diff --git a/codex-rs/core/tests/suite/models_cache_auth.rs b/codex-rs/core/tests/suite/models_cache_auth.rs new file mode 100644 index 0000000000..f1a1009e91 --- /dev/null +++ b/codex-rs/core/tests/suite/models_cache_auth.rs @@ -0,0 +1,144 @@ +//! Catalogs must follow auth and provider changes without leaking default billing tiers. + +use std::sync::Arc; + +use anyhow::Result; +use codex_login::AuthCredentialsStoreMode; +use codex_login::AuthKeyringBackendKind; +use codex_login::CodexAuth; +use codex_login::login_with_api_key; +use codex_models_manager::bundled_models_response; +use codex_models_manager::manager::RefreshStrategy; +use codex_protocol::openai_models::ModelVisibility; +use codex_protocol::openai_models::ModelsResponse; +use core_test_support::responses; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::sse; +use core_test_support::test_codex::test_codex; +use pretty_assertions::assert_eq; +use tempfile::TempDir; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn auth_and_provider_switches_do_not_reuse_chatgpt_catalog() -> Result<()> { + let server = wiremock::MockServer::start().await; + let home = Arc::new(TempDir::new()?); + let mut model = bundled_models_response()? + .models + .into_iter() + .find(|model| model.slug == "gpt-5.4") + .unwrap(); + model.visibility = ModelVisibility::List; + model.default_service_tier = Some("priority".into()); + let models_mock = responses::mount_models_once( + &server, + ModelsResponse { + models: vec![model.clone()], + }, + ) + .await; + let chatgpt = test_codex() + .with_home(home.clone()) + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_model("gpt-5.4") + .build_with_auto_env(&server) + .await?; + assert!(home.path().join("models_cache.json").exists()); + assert_eq!( + chatgpt + .thread_manager + .get_models_manager() + .get_remote_models() + .await, + vec![model.clone()] + ); + login_with_api_key( + home.path(), + "api-key", + AuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), + )?; + assert!(chatgpt.thread_manager.auth_manager().reload().await); + let manager = chatgpt.thread_manager.get_models_manager(); + let bundled = bundled_models_response()?.models; + assert_eq!(manager.get_remote_models().await, bundled); + assert_eq!( + manager + .raw_model_catalog( + RefreshStrategy::Offline, + codex_core::test_support::default_http_client_factory() + ) + .await + .models, + bundled + ); + drop(chatgpt); + + let api = test_codex() + .with_home(home.clone()) + .with_auth(CodexAuth::from_api_key("api-key")) + .with_model("gpt-5.4") + .build_with_auto_env(&server) + .await?; + let ordinary = responses::mount_sse_once( + &server, + sse(vec![ + ev_response_created("ordinary"), + ev_completed("ordinary"), + ]), + ) + .await; + api.submit_turn("hello").await?; + assert_eq!( + ordinary.single_request().body_json().get("service_tier"), + None + ); + + let explicit = responses::mount_sse_once( + &server, + sse(vec![ + ev_response_created("explicit"), + ev_completed("explicit"), + ]), + ) + .await; + api.submit_turn_with_service_tier("hello again", Some("priority")) + .await?; + assert_eq!( + explicit.single_request().body_json()["service_tier"], + "priority" + ); + assert_eq!(models_mock.requests().len(), 1); + drop(api); + + let other_server = wiremock::MockServer::start().await; + model.default_service_tier = None; + model.display_name = "Second provider model".into(); + let other_models = responses::mount_models_once( + &other_server, + ModelsResponse { + models: vec![model.clone()], + }, + ) + .await; + let other = test_codex() + .with_home(home) + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_model("gpt-5.4") + .with_config(|config| { + config.model_provider_id = "second".into(); + config.model_provider.name = "Second".into(); + }) + .build_with_auto_env(&other_server) + .await?; + assert_eq!( + other + .thread_manager + .get_models_manager() + .get_remote_models() + .await, + vec![model] + ); + assert_eq!(other_models.requests().len(), 1); + Ok(()) +} diff --git a/codex-rs/core/tests/suite/models_cache_ttl.rs b/codex-rs/core/tests/suite/models_cache_ttl.rs index 701ada2f8c..7a3fa935ba 100644 --- a/codex-rs/core/tests/suite/models_cache_ttl.rs +++ b/codex-rs/core/tests/suite/models_cache_ttl.rs @@ -289,9 +289,19 @@ async fn uses_cache_when_version_matches() -> Result<()> { .await; let mut builder = test_codex().with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()); + let identity = codex_model_provider::test_support::models_cache_entry( + &codex_model_provider_info::ModelProviderInfo::create_openai_provider(Some(format!( + "{}/v1", + server.uri() + ))), + Some(&CodexAuth::create_dummy_chatgpt_auth_for_testing()), + Vec::new(), + ) + .identity; builder = builder .with_pre_build_hook(move |home| { let mut cache = serde_json::to_value(ModelsCache { + identity, fetched_at: Utc::now(), etag: None, client_version: Some(client_version_to_whole()), @@ -354,9 +364,19 @@ async fn refreshes_when_cache_version_missing() -> Result<()> { .await; let mut builder = test_codex().with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()); + let identity = codex_model_provider::test_support::models_cache_entry( + &codex_model_provider_info::ModelProviderInfo::create_openai_provider(Some(format!( + "{}/v1", + server.uri() + ))), + Some(&CodexAuth::create_dummy_chatgpt_auth_for_testing()), + Vec::new(), + ) + .identity; builder = builder .with_pre_build_hook(move |home| { let cache = ModelsCache { + identity, fetched_at: Utc::now(), etag: None, client_version: None, @@ -404,10 +424,20 @@ async fn refreshes_when_cache_version_differs() -> Result<()> { } let mut builder = test_codex().with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()); + let identity = codex_model_provider::test_support::models_cache_entry( + &codex_model_provider_info::ModelProviderInfo::create_openai_provider(Some(format!( + "{}/v1", + server.uri() + ))), + Some(&CodexAuth::create_dummy_chatgpt_auth_for_testing()), + Vec::new(), + ) + .identity; builder = builder .with_pre_build_hook(move |home| { let client_version = client_version_to_whole(); let cache = ModelsCache { + identity, fetched_at: Utc::now(), etag: None, client_version: Some(format!("{client_version}-diff")), @@ -471,6 +501,8 @@ fn write_cache_sync(path: &Path, cache: &ModelsCache) -> Result<()> { #[derive(Debug, Clone, Serialize, Deserialize)] struct ModelsCache { + #[serde(default)] + identity: Option, fetched_at: DateTime, #[serde(default)] etag: Option, diff --git a/codex-rs/model-provider/src/lib.rs b/codex-rs/model-provider/src/lib.rs index f248f963fe..b3ca480d8c 100644 --- a/codex-rs/model-provider/src/lib.rs +++ b/codex-rs/model-provider/src/lib.rs @@ -5,6 +5,7 @@ mod models_endpoint; mod models_identity; mod provider; mod shared_state; +pub mod test_support; pub use amazon_bedrock::is_supported_amazon_bedrock_region; pub use auth::AgentIdentitySessionFallback; diff --git a/codex-rs/model-provider/src/models_endpoint.rs b/codex-rs/model-provider/src/models_endpoint.rs index 76bf039f2a..12a2917bc5 100644 --- a/codex-rs/model-provider/src/models_endpoint.rs +++ b/codex-rs/model-provider/src/models_endpoint.rs @@ -456,4 +456,104 @@ mod tests { Some(&"eu".into()) ); } + + #[derive(Debug)] + struct RotatingAuth(std::sync::atomic::AtomicUsize); + + impl codex_login::ExternalAuth for RotatingAuth { + fn resolve(&self) -> codex_login::ExternalAuthFuture<'_, CodexAuth> { + Box::pin(async move { + let generation = self.0.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + Ok(CodexAuth::from_api_key(&format!("token-{generation}"))) + }) + } + + fn refresh( + &self, + _context: codex_login::ExternalAuthRefreshContext, + ) -> codex_login::ExternalAuthFuture<'_, CodexAuth> { + self.resolve() + } + } + + #[tokio::test] + async fn command_auth_refresh_fetches_a_catalog_for_the_current_credentials() { + use codex_models_manager::manager::ModelsManager; + use codex_models_manager::manager::OpenAiModelsManager; + use codex_models_manager::manager::RefreshStrategy; + + let server = MockServer::start().await; + let auth = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("initial")); + auth.set_external_auth(Arc::new(RotatingAuth(std::sync::atomic::AtomicUsize::new( + 0, + )))) + .await + .unwrap(); + let model = codex_protocol::openai_models::ModelInfo { + used_fallback_model_metadata: false, + ..codex_models_manager::model_info::model_info_from_slug("command-auth-model") + }; + Mock::given(method("GET")) + .and(path("/models")) + .respond_with(ResponseTemplate::new(200).set_body_json(ModelsResponse { + models: vec![model.clone()], + })) + .expect(2) + .mount(&server) + .await; + let mut provider = provider_info_with_command_auth(); + provider.base_url = Some(server.uri()); + // Keep this test independent of the residency override exercised in parallel. + provider.http_headers = Some(std::collections::HashMap::from([( + RESIDENCY_HEADER_NAME.to_string(), + "us".into(), + )])); + let manager = OpenAiModelsManager::new_without_cache( + Arc::new(OpenAiModelsEndpoint::new(provider, Some(auth.clone()))), + Some(auth.clone()), + ); + let catalog = manager + .raw_model_catalog( + RefreshStrategy::OnlineIfUncached, + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + ) + .await; + assert_eq!( + catalog + .models + .iter() + .find(|candidate| candidate.slug == model.slug), + Some(&model) + ); + auth.auth().await; + let bundled = codex_models_manager::bundled_models_response().unwrap(); + assert_eq!(manager.get_remote_models().await, bundled.models); + assert_eq!(manager.try_get_remote_models().unwrap(), bundled.models); + assert_eq!( + manager + .raw_model_catalog( + RefreshStrategy::Offline, + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + ) + .await, + bundled + ); + assert_eq!( + manager + .raw_model_catalog( + RefreshStrategy::OnlineIfUncached, + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + ) + .await, + catalog + ); + let requests = server.received_requests().await.unwrap(); + assert_eq!( + requests + .iter() + .map(|request| request.headers["authorization"].to_str().unwrap()) + .collect::>(), + vec!["Bearer token-2", "Bearer token-6"] + ); + } } diff --git a/codex-rs/model-provider/src/test_support.rs b/codex-rs/model-provider/src/test_support.rs new file mode 100644 index 0000000000..2e017aee55 --- /dev/null +++ b/codex-rs/model-provider/src/test_support.rs @@ -0,0 +1,21 @@ +//! Fixtures for integration tests that seed the provider's model cache. + +use codex_login::CodexAuth; +use codex_model_provider_info::ModelProviderInfo; +use codex_models_manager::cache::ModelsCacheEntry; +use codex_protocol::openai_models::ModelInfo; + +/// Constructs a cache fixture matching the supplied test provider and authentication. +pub fn models_cache_entry( + provider_info: &ModelProviderInfo, + auth: Option<&CodexAuth>, + models: Vec, +) -> ModelsCacheEntry { + ModelsCacheEntry { + fetched_at: std::time::SystemTime::now().into(), + etag: None, + client_version: Some(codex_models_manager::client_version_to_whole()), + identity: crate::models_identity::identity(provider_info, auth).ok(), + models, + } +} diff --git a/codex-rs/models-manager/src/cache.rs b/codex-rs/models-manager/src/cache.rs index 240ec7e11b..14d08977f4 100644 --- a/codex-rs/models-manager/src/cache.rs +++ b/codex-rs/models-manager/src/cache.rs @@ -17,8 +17,8 @@ use tracing::info; /// /// Implementations own cache freshness and lookup partitioning. [`ModelsCache::load`] must not /// return stale entries or entries for a different client version. A shared backend must also keep -/// catalogs for different providers and tenants separate, typically through configuration captured -/// by the implementation. +/// catalogs for different providers and tenants separate. The manager also validates the entry +/// identity against the current endpoint before using it. /// /// Cache failures are non-fatal. The models manager logs them and falls back to the configured /// models endpoint. @@ -42,16 +42,18 @@ pub trait ModelsCache: fmt::Debug + Send + Sync { entry: &'a ModelsCacheEntry, ) -> ModelsCacheFuture<'a, Result<(), ModelsCacheError>>; - /// Extends the freshness of the entry for `client_version` without changing its catalog. + /// Extends freshness only if the stored version, identity, and ETag match. /// /// The models manager calls this after the endpoint confirms that the cached ETag is still - /// current. Implementations must preserve the models, ETag, and client version. This operation + /// current. Implementations must preserve the models, identity, ETag, and client version. This operation /// cannot be expressed in terms of [`ModelsCache::load`], because `load` intentionally omits /// expired entries. File-backed implementations can read and revalidate the stored entry /// directly, while backends with native TTL support can extend the entry in place. fn refresh_ttl<'a>( &'a self, client_version: &'a str, + identity: &'a str, + etag: &'a str, ) -> ModelsCacheFuture<'a, Result<(), ModelsCacheError>>; } @@ -71,7 +73,7 @@ pub struct ModelsCacheEntry { /// The models manager rejects entries whose value is absent or differs from its current version. #[serde(default, skip_serializing_if = "Option::is_none")] pub client_version: Option, - /// Opaque provider and auth identity associated with this catalog. + /// Opaque provider and auth identity. Unscoped legacy entries are cache misses. #[serde(default, skip_serializing_if = "Option::is_none")] pub identity: Option, /// Models returned by the catalog endpoint. @@ -154,14 +156,20 @@ impl ModelsCache for FileModelsCache { fn refresh_ttl<'a>( &'a self, - _client_version: &'a str, + client_version: &'a str, + identity: &'a str, + etag: &'a str, ) -> ModelsCacheFuture<'a, Result<(), ModelsCacheError>> { Box::pin(async move { let mut entry = load_file(&self.cache_path) .await .map_err(cache_error)? .ok_or_else(|| ModelsCacheError::new("cache not found"))?; - if entry.is_fresh(self.cache_ttl / 2) { + if entry.client_version.as_deref() != Some(client_version) + || entry.identity.as_deref() != Some(identity) + || entry.etag.as_deref() != Some(etag) + || entry.is_fresh(self.cache_ttl / 2) + { return Ok(()); } entry.fetched_at = Utc::now(); diff --git a/codex-rs/models-manager/src/cache_identity_tests.rs b/codex-rs/models-manager/src/cache_identity_tests.rs new file mode 100644 index 0000000000..b4fbe252c0 --- /dev/null +++ b/codex-rs/models-manager/src/cache_identity_tests.rs @@ -0,0 +1,83 @@ +//! Coverage for cache identity validation and conditional freshness updates. + +use super::*; +use pretty_assertions::assert_eq; + +#[tokio::test] +async fn mismatched_and_legacy_cache_entries_fetch_the_current_catalog() { + for identity in [None, Some("other-provider-or-account".to_string())] { + let stale = remote_model("stale", "Stale", /*priority*/ 0); + let current = remote_model("current", "Current", /*priority*/ 0); + let cache = TestModelsCache::with_entry(ModelsCacheEntry { + fetched_at: Utc::now(), + etag: Some("old-etag".into()), + client_version: Some(crate::client_version_to_whole()), + identity, + models: vec![stale], + }); + let endpoint = TestModelsEndpoint::new(vec![vec![current.clone()]]); + let manager = OpenAiModelsManager::new_with_cache( + cache.clone(), + endpoint.clone(), + Some(AuthManager::from_auth_for_testing( + CodexAuth::create_dummy_chatgpt_auth_for_testing(), + )), + ); + assert_eq!( + manager + .raw_model_catalog( + RefreshStrategy::OnlineIfUncached, + DEFAULT_HTTP_CLIENT_FACTORY + ) + .await + .models, + vec![current.clone()] + ); + assert_eq!(endpoint.fetch_count(), 1); + } +} + +#[tokio::test] +async fn matching_etag_cannot_renew_a_different_identity_or_catalog() { + let home = tempdir().unwrap(); + let cache = FileModelsCache::new(home.path().join(MODEL_CACHE_FILE), DEFAULT_MODEL_CACHE_TTL); + let expected = ModelsCacheEntry { + fetched_at: Utc::now() - chrono::Duration::hours(1), + etag: Some("etag".into()), + client_version: Some(crate::client_version_to_whole()), + identity: Some("first-account".into()), + models: vec![remote_model("model", "Model", /*priority*/ 0)], + }; + for (key, etag, version) in [ + ("other-account", "etag", crate::client_version_to_whole()), + ( + "first-account", + "new-etag", + crate::client_version_to_whole(), + ), + ("first-account", "etag", "other-version".into()), + ] { + let stored = ModelsCacheEntry { + identity: Some(key.into()), + etag: Some(etag.into()), + client_version: Some(version), + ..expected.clone() + }; + cache.store(&stored).await.unwrap(); + cache + .refresh_ttl( + expected.client_version.as_deref().unwrap(), + expected.identity.as_deref().unwrap(), + expected.etag.as_deref().unwrap(), + ) + .await + .unwrap(); + let actual: ModelsCacheEntry = serde_json::from_slice( + &tokio::fs::read(home.path().join(MODEL_CACHE_FILE)) + .await + .unwrap(), + ) + .unwrap(); + assert_eq!(actual, stored); + } +} diff --git a/codex-rs/models-manager/src/manager.rs b/codex-rs/models-manager/src/manager.rs index 9b37abe10e..cb38919900 100644 --- a/codex-rs/models-manager/src/manager.rs +++ b/codex-rs/models-manager/src/manager.rs @@ -38,7 +38,7 @@ pub trait ModelsEndpointClient: fmt::Debug + Send + Sync { /// Opaque identity of the current provider and credentials, without resolving auth. /// /// Must change when account, email, plan, provider, or API credentials change. - /// Return `None` when identity is unavailable. + /// Return `None` when identity is unavailable; cached catalog reuse is then disabled. fn identity(&self) -> Option; /// Returns whether this provider can authenticate command-scoped requests. @@ -232,8 +232,7 @@ pub type SharedModelsManager = Arc; /// OpenAI-compatible model manager backed by bundled models, cache, and `/models`. #[derive(Debug)] pub struct OpenAiModelsManager { - remote_models: RwLock>, - etag: RwLock>, + remote_models: RwLock, cache: Option>, endpoint_client: SharedModelsEndpointClient, auth_manager: Option>, @@ -291,8 +290,13 @@ impl OpenAiModelsManager { ) -> Self { let remote_models = load_remote_models_from_file().unwrap_or_default(); Self { - remote_models: RwLock::new(remote_models), - etag: RwLock::new(None), + remote_models: RwLock::new(ModelsCacheEntry { + fetched_at: Utc::now(), + etag: None, + client_version: Some(crate::client_version_to_whole()), + identity: endpoint_client.identity(), + models: remote_models, + }), cache, endpoint_client, auth_manager, @@ -324,11 +328,25 @@ impl ModelsManager for OpenAiModelsManager { } fn get_remote_models(&self) -> ModelsManagerFuture<'_, Vec> { - Box::pin(async move { self.remote_models.read().await.clone() }) + Box::pin(async move { + let entry = self.remote_models.read().await; + if entry.identity.is_some() && entry.identity == self.endpoint_client.identity() { + entry.models.clone() + } else { + load_remote_models_from_file().unwrap_or_default() + } + }) } fn try_get_remote_models(&self) -> Result, TryLockError> { - Ok(self.remote_models.try_read()?.clone()) + let entry = self.remote_models.try_read()?; + Ok( + if entry.identity.is_some() && entry.identity == self.endpoint_client.identity() { + entry.models.clone() + } else { + load_remote_models_from_file().unwrap_or_default() + }, + ) } fn auth_manager(&self) -> Option<&AuthManager> { @@ -370,10 +388,18 @@ impl OpenAiModelsManager { } async fn refresh_if_new_etag(&self, etag: String, http_client_factory: HttpClientFactory) { - let current_etag = self.get_etag().await; - if current_etag.clone().is_some() && current_etag.as_deref() == Some(etag.as_str()) { + let (identity, current_etag) = { + let entry = self.remote_models.read().await; + (entry.identity.clone(), entry.etag.clone()) + }; + if let Some(identity) = identity + && Some(&identity) == self.endpoint_client.identity().as_ref() + && current_etag.as_deref() == Some(etag.as_str()) + { if let Some(cache) = self.cache.as_ref() - && let Err(err) = cache.refresh_ttl(&crate::client_version_to_whole()).await + && let Err(err) = cache + .refresh_ttl(&crate::client_version_to_whole(), &identity, &etag) + .await { error!("failed to renew cache TTL: {err}"); } @@ -402,26 +428,18 @@ impl OpenAiModelsManager { } return Ok(()); } - match refresh_strategy { RefreshStrategy::Offline => { - // Only try to load from cache, never fetch self.try_load_cache().await; Ok(()) } RefreshStrategy::OnlineIfUncached => { - // Try cache first, fall back to online if unavailable if self.try_load_cache().await { - info!("models cache: using cached models for OnlineIfUncached"); return Ok(()); } - info!("models cache: cache miss, fetching remote models"); - self.fetch_and_update_models(http_client_factory).await - } - RefreshStrategy::Online => { - // Always fetch from network self.fetch_and_update_models(http_client_factory).await } + RefreshStrategy::Online => self.fetch_and_update_models(http_client_factory).await, } } @@ -438,20 +456,22 @@ impl OpenAiModelsManager { .endpoint_client .list_models(&client_version, http_client_factory.clone()) .await?; - self.apply_remote_models(models.clone()).await; - *self.etag.write().await = etag.clone(); - if let Some(cache) = self.cache.as_ref() { - let entry = ModelsCacheEntry { - fetched_at: Utc::now(), - etag, - client_version: Some(client_version), - identity: Some(identity), - models, - }; - if let Err(err) = cache.store(&entry).await { - error!("failed to write models cache: {err}"); - } + if Some(&identity) != self.endpoint_client.identity().as_ref() { + return Ok(()); } + let entry = ModelsCacheEntry { + fetched_at: Utc::now(), + etag, + client_version: Some(client_version), + identity: Some(identity), + models, + }; + if let Some(cache) = self.cache.as_ref() + && let Err(err) = cache.store(&entry).await + { + error!("failed to write models cache: {err}"); + } + self.apply_remote_models(entry).await; Ok(()) } @@ -459,40 +479,38 @@ impl OpenAiModelsManager { self.endpoint_client.uses_codex_backend().await || self.endpoint_client.has_command_auth() } - async fn get_etag(&self) -> Option { - self.etag.read().await.clone() - } - - /// Replace the cached remote models and rebuild the derived presets list. - async fn apply_remote_models(&self, models: Vec) { - // Use the remote models list as the source of truth if it contains at least one - // non-hidden model and the user is using ChatGPT auth. - let should_use_remote_models_only = !models.is_empty() - && models - .iter() - .any(|model| model.visibility == ModelVisibility::List) + /// Publish only while the request identity still matches, including after async storage. + async fn apply_remote_models(&self, mut entry: ModelsCacheEntry) -> bool { + let mut current = self.remote_models.write().await; + if entry.identity != self.endpoint_client.identity() { + return false; + } + // Visible ChatGPT catalogs are authoritative; other catalogs augment bundled metadata. + let remote_only = entry + .models + .iter() + .any(|model| model.visibility == ModelVisibility::List) && self.auth_manager.as_ref().is_some_and(|auth_manager| { auth_manager .auth_mode() .is_some_and(AuthMode::has_chatgpt_account) }); - if should_use_remote_models_only { - *self.remote_models.write().await = models; - return; - } - - let mut existing_models = load_remote_models_from_file().unwrap_or_default(); - for model in models { - if let Some(existing_index) = existing_models - .iter() - .position(|existing| existing.slug == model.slug) - { - existing_models[existing_index] = model; - } else { - existing_models.push(model); + if !remote_only { + let mut models = load_remote_models_from_file().unwrap_or_default(); + for model in entry.models { + if let Some(index) = models + .iter() + .position(|existing| existing.slug == model.slug) + { + models[index] = model; + } else { + models.push(model); + } } + entry.models = models; } - *self.remote_models.write().await = existing_models; + *current = entry; + true } /// Attempt to satisfy the refresh from the cache when it matches the provider and TTL. @@ -504,8 +522,9 @@ impl OpenAiModelsManager { codex_otel::start_global_timer("codex.remote_models.load_cache.duration_ms", &[]); let client_version = crate::client_version_to_whole(); info!(client_version, "models cache: evaluating cache eligibility"); - // TODO(celia-oai): Include provider identity in cache eligibility so switching - // providers does not reuse a fresh models_cache.json entry from another provider. + let Some(identity) = self.endpoint_client.identity() else { + return false; + }; let cache_entry = match cache.load(&client_version).await { Ok(Some(cache_entry)) => cache_entry, Ok(None) => { @@ -525,15 +544,13 @@ impl OpenAiModelsManager { ); return false; } - let models = cache_entry.models.clone(); - *self.etag.write().await = cache_entry.etag.clone(); - self.apply_remote_models(models.clone()).await; - info!( - models_count = models.len(), - etag = ?cache_entry.etag, - "models cache: cache entry applied" - ); - true + if cache_entry.identity.as_ref() != Some(&identity) + || self.endpoint_client.identity().as_ref() != Some(&identity) + { + info!("models cache: provider or auth identity mismatch"); + return false; + } + self.apply_remote_models(cache_entry).await } } diff --git a/codex-rs/models-manager/src/manager_tests.rs b/codex-rs/models-manager/src/manager_tests.rs index 78ecf95336..b3f732c0ae 100644 --- a/codex-rs/models-manager/src/manager_tests.rs +++ b/codex-rs/models-manager/src/manager_tests.rs @@ -27,6 +27,9 @@ use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; use tempfile::tempdir; +#[path = "cache_identity_tests.rs"] +mod cache_identity_tests; + #[path = "model_info_overrides_tests.rs"] mod model_info_overrides_tests; @@ -164,6 +167,8 @@ impl ModelsCache for TestModelsCache { fn refresh_ttl<'a>( &'a self, _client_version: &'a str, + _identity: &'a str, + _etag: &'a str, ) -> ModelsCacheFuture<'a, Result<(), ModelsCacheError>> { Box::pin(async move { let refreshed = { @@ -394,7 +399,11 @@ async fn file_cache_refresh_ttl_renews_expired_entry_without_serving_it_stale() ); cache - .refresh_ttl(&client_version) + .refresh_ttl( + &client_version, + entry.identity.as_deref().unwrap(), + entry.etag.as_deref().unwrap(), + ) .await .expect("TTL refresh succeeds"); diff --git a/codex-rs/tui/src/status/tests.rs b/codex-rs/tui/src/status/tests.rs index e644dcb2cc..b6482d462b 100644 --- a/codex-rs/tui/src/status/tests.rs +++ b/codex-rs/tui/src/status/tests.rs @@ -358,7 +358,6 @@ async fn status_snapshot_includes_reasoning_details() { #[tokio::test] async fn status_snapshot_shows_chatgpt_plan_without_email() { let temp_home = TempDir::new().expect("temp home"); - write_models_cache(temp_home.path()).expect("write models cache"); let mut config = test_config(&temp_home).await; config.model = Some("gpt-5.1-codex-max".to_string()); config.model_provider_id = "openai".to_string(); @@ -371,6 +370,9 @@ async fn status_snapshot_shows_chatgpt_plan_without_email() { AuthCredentialsStoreMode::File, ) .expect("write email-less ChatGPT auth"); + write_models_cache(temp_home.path()) + .await + .expect("write models cache"); let mut app_server = crate::start_embedded_app_server_for_picker(&config) .await .expect("start embedded app server");