Disable dynamic model refresh for custom model providers (#11239)
The dynamic model refresh feature (`https://api.openai.com/v1/models` endpoint) is currently gated on a runtime check for an auth method other than API Key. It should be gated on a check specifically for ChatGPT Auth because some custom model providers (e.g. for local models) use no auth mechanism. A call to `self.auth_manager.auth_mode()` will return `None` in this case. Addresses #11213
This commit is contained in:
parent
c0994b363d
commit
bb974c78de
1 changed files with 62 additions and 18 deletions
|
|
@ -194,9 +194,16 @@ impl ModelsManager {
|
|||
config: &Config,
|
||||
refresh_strategy: RefreshStrategy,
|
||||
) -> CoreResult<()> {
|
||||
if !config.features.enabled(Feature::RemoteModels)
|
||||
|| self.auth_manager.auth_mode() == Some(AuthMode::ApiKey)
|
||||
{
|
||||
if !config.features.enabled(Feature::RemoteModels) {
|
||||
return Ok(());
|
||||
}
|
||||
if self.auth_manager.auth_mode() != Some(AuthMode::Chatgpt) {
|
||||
if matches!(
|
||||
refresh_strategy,
|
||||
RefreshStrategy::Offline | RefreshStrategy::OnlineIfUncached
|
||||
) {
|
||||
self.try_load_cache().await;
|
||||
}
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
|
|
@ -526,11 +533,8 @@ mod tests {
|
|||
.await
|
||||
.expect("load default test config");
|
||||
config.features.enable(Feature::RemoteModels);
|
||||
let auth_manager = Arc::new(AuthManager::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
false,
|
||||
AuthCredentialsStoreMode::File,
|
||||
));
|
||||
let auth_manager =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::create_dummy_chatgpt_auth_for_testing());
|
||||
let provider = provider_for(server.uri());
|
||||
let manager =
|
||||
ModelsManager::with_provider(codex_home.path().to_path_buf(), auth_manager, provider);
|
||||
|
|
@ -573,11 +577,8 @@ mod tests {
|
|||
.await
|
||||
.expect("load default test config");
|
||||
config.features.enable(Feature::RemoteModels);
|
||||
let auth_manager = Arc::new(AuthManager::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
false,
|
||||
AuthCredentialsStoreMode::File,
|
||||
));
|
||||
let auth_manager =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::create_dummy_chatgpt_auth_for_testing());
|
||||
let provider = provider_for(server.uri());
|
||||
let manager =
|
||||
ModelsManager::with_provider(codex_home.path().to_path_buf(), auth_manager, provider);
|
||||
|
|
@ -642,11 +643,8 @@ mod tests {
|
|||
.await
|
||||
.expect("load default test config");
|
||||
config.features.enable(Feature::RemoteModels);
|
||||
let auth_manager = Arc::new(AuthManager::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
false,
|
||||
AuthCredentialsStoreMode::File,
|
||||
));
|
||||
let auth_manager =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::create_dummy_chatgpt_auth_for_testing());
|
||||
let provider = provider_for(server.uri());
|
||||
let manager =
|
||||
ModelsManager::with_provider(codex_home.path().to_path_buf(), auth_manager, provider);
|
||||
|
|
@ -761,6 +759,52 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn refresh_available_models_skips_network_without_chatgpt_auth() {
|
||||
let server = MockServer::start().await;
|
||||
let dynamic_slug = "dynamic-model-only-for-test-noauth";
|
||||
let models_mock = mount_models_once(
|
||||
&server,
|
||||
ModelsResponse {
|
||||
models: vec![remote_model(dynamic_slug, "No Auth", 1)],
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let codex_home = tempdir().expect("temp dir");
|
||||
let mut config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.build()
|
||||
.await
|
||||
.expect("load default test config");
|
||||
config.features.enable(Feature::RemoteModels);
|
||||
let auth_manager = Arc::new(AuthManager::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
false,
|
||||
AuthCredentialsStoreMode::File,
|
||||
));
|
||||
let provider = provider_for(server.uri());
|
||||
let manager =
|
||||
ModelsManager::with_provider(codex_home.path().to_path_buf(), auth_manager, provider);
|
||||
|
||||
manager
|
||||
.refresh_available_models(&config, RefreshStrategy::Online)
|
||||
.await
|
||||
.expect("refresh should no-op without chatgpt auth");
|
||||
let cached_remote = manager.get_remote_models(&config).await;
|
||||
assert!(
|
||||
!cached_remote
|
||||
.iter()
|
||||
.any(|candidate| candidate.slug == dynamic_slug),
|
||||
"remote refresh should be skipped without chatgpt auth"
|
||||
);
|
||||
assert_eq!(
|
||||
models_mock.requests().len(),
|
||||
0,
|
||||
"no auth should avoid /models requests"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_available_models_picks_default_after_hiding_hidden_models() {
|
||||
let codex_home = tempdir().expect("temp dir");
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue