mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Fix model selection after provider setup (#58945)
## Why After provider setup, new threads could still use the models manager created for the server's startup provider, leaving their default model inconsistent with the model picker. ## What changed - Build a models manager from the thread's configuration when its provider differs from the startup provider. - Invalidate the model catalog after successful Amazon Bedrock setup so subsequent model listings reflect the configured provider. ## Testing Extend Bedrock and GCP setup integration tests to verify that a new thread's model matches the refreshed model listing. The Bedrock test also checks that the thread uses `amazon-bedrock`. GitOrigin-RevId: 1db30ad040ba97e2f4f31c9e162a2b73af1377fa
This commit is contained in:
@@ -134,6 +134,7 @@ impl AccountRequestProcessor {
|
||||
.commit_provider_config(write)
|
||||
.await
|
||||
.map_err(map_config_error)?;
|
||||
self.model_catalog.invalidate().await;
|
||||
|
||||
Ok(Some(BedrockSetupResponse {}.into()))
|
||||
}
|
||||
|
||||
@@ -5,7 +5,10 @@ use codex_app_server_protocol::BedrockDiscoverResponse;
|
||||
use codex_app_server_protocol::BedrockSetupParams;
|
||||
use codex_app_server_protocol::BedrockSetupResponse;
|
||||
use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::ModelListParams;
|
||||
use codex_app_server_protocol::ModelListResponse;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::ThreadStartParams;
|
||||
use codex_config::types::AuthCredentialsStoreMode;
|
||||
use codex_login::AuthKeyringBackendKind;
|
||||
use codex_login::login_with_bedrock_api_key;
|
||||
@@ -115,6 +118,21 @@ async fn discover_bedrock_profiles_and_environment_credentials() -> Result<()> {
|
||||
);
|
||||
assert!(!codex_home.path().join(".env").exists());
|
||||
|
||||
// The same server must route both the picker and the first thread to Bedrock.
|
||||
let models: ModelListResponse = app_server
|
||||
.request(|request_id| ClientRequest::ModelList {
|
||||
request_id,
|
||||
params: ModelListParams::default(),
|
||||
})
|
||||
.await?;
|
||||
let default_model = models.data.iter().find(|model| model.is_default).unwrap();
|
||||
assert!(default_model.model.starts_with("openai."));
|
||||
let thread = app_server
|
||||
.start_thread(ThreadStartParams::default())
|
||||
.await?;
|
||||
assert_eq!(thread.model_provider, "amazon-bedrock");
|
||||
assert_eq!(thread.model, default_model.model);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ use codex_app_server_protocol::JSONRPCErrorError;
|
||||
use codex_app_server_protocol::ModelListParams;
|
||||
use codex_app_server_protocol::ModelListResponse;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::ThreadStartParams;
|
||||
use codex_protocol::openai_models::ModelVisibility;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
@@ -105,6 +106,8 @@ async fn setup_gcp_saves_overrides_restores_adc_and_refreshes_catalog() -> Resul
|
||||
model.supported_in_api = true;
|
||||
let mut unsupported_model = model.clone();
|
||||
unsupported_model.slug = "test-non-vertex-model".to_string();
|
||||
// Make the startup catalog's default distinguishable from the GCP default.
|
||||
unsupported_model.priority = model.priority - 1;
|
||||
catalog.models.push(unsupported_model);
|
||||
let catalog_path = home.join("models.json");
|
||||
std::fs::write(&catalog_path, serde_json::to_vec(&catalog)?)?;
|
||||
@@ -191,6 +194,10 @@ async fn setup_gcp_saves_overrides_restores_adc_and_refreshes_catalog() -> Resul
|
||||
.collect::<Vec<_>>(),
|
||||
[selected.as_path(), ambient.as_path()]
|
||||
);
|
||||
let thread = app_server
|
||||
.start_thread(ThreadStartParams::default())
|
||||
.await?;
|
||||
assert_eq!(thread.model, after.data[0].model);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -526,6 +526,7 @@ pub(crate) struct ThreadManagerState {
|
||||
agent_control_factory: Option<AgentControlFactory>,
|
||||
auth_manager: Arc<AuthManager>,
|
||||
models_manager: SharedModelsManager,
|
||||
models_manager_provider: ModelProviderInfo,
|
||||
git_root_discovery: Arc<GitRootDiscovery>,
|
||||
environment_manager: Arc<EnvironmentManager>,
|
||||
starting_mcp_runtimes: std::sync::Mutex<Vec<std::sync::Weak<AtomicBool>>>,
|
||||
@@ -697,6 +698,7 @@ impl ThreadManager {
|
||||
thread_id_generator: default_thread_id_generator(),
|
||||
agent_control_factory: None,
|
||||
models_manager,
|
||||
models_manager_provider: config.model_provider.clone(),
|
||||
git_root_discovery: Arc::default(),
|
||||
environment_manager,
|
||||
starting_mcp_runtimes: std::sync::Mutex::new(Vec::new()),
|
||||
@@ -905,6 +907,7 @@ impl ThreadManager {
|
||||
thread_created_tx,
|
||||
thread_id_generator: default_thread_id_generator(),
|
||||
agent_control_factory: None,
|
||||
models_manager_provider: provider.clone(),
|
||||
models_manager: create_model_provider(provider, Some(auth_manager.clone()))
|
||||
.models_manager(codex_home, /*config_model_catalog*/ None),
|
||||
git_root_discovery: Arc::default(),
|
||||
@@ -2584,6 +2587,14 @@ impl ThreadManagerState {
|
||||
} else {
|
||||
codex_sandboxing::WindowsSandboxProxySettingsMode::Reconcile
|
||||
};
|
||||
let models_manager = if config.model_provider == self.models_manager_provider {
|
||||
Arc::clone(&self.models_manager)
|
||||
} else {
|
||||
build_models_manager(&config, Arc::clone(&auth_manager))
|
||||
};
|
||||
let models_manager = models_manager
|
||||
.for_originator(&startup_context.originator)
|
||||
.unwrap_or(models_manager);
|
||||
let spawn_result = Session::spawn(SessionSpawnArgs {
|
||||
attached_thread,
|
||||
startup,
|
||||
@@ -2593,10 +2604,7 @@ impl ThreadManagerState {
|
||||
instructions: startup_context.instructions,
|
||||
installation_id: self.installation_id.clone(),
|
||||
auth_manager,
|
||||
models_manager: self
|
||||
.models_manager
|
||||
.for_originator(&startup_context.originator)
|
||||
.unwrap_or_else(|| Arc::clone(&self.models_manager)),
|
||||
models_manager,
|
||||
git_root_discovery: Arc::clone(&self.git_root_discovery),
|
||||
environment_manager: Arc::clone(&self.environment_manager),
|
||||
skills_service: Arc::clone(&self.skills_service),
|
||||
|
||||
Reference in New Issue
Block a user