mirror of
https://github.com/openai/codex.git
synced 2026-10-25 18:28:38 +00:00
Allow injecting the models manager into ThreadManager (#32911)
## Why Embedding callers need to control whether model catalogs are persisted to disk. ## What changed - Accept a shared models manager when constructing `ThreadManager` instead of always creating a cached manager internally. - Add provider APIs for creating uncached model managers. OpenAI-compatible providers fetch on each `OnlineIfUncached` refresh without reading or writing `models_cache.json`. - Keep existing app server, MCP server, sample, and test callers on the standard cached manager. ## Testing - Verify an uncached manager fetches on every refresh. - Verify an injected uncached manager controls thread refresh behavior and does not create `models_cache.json`. GitOrigin-RevId: 8bb646054ecec4ccb865b2bf56249384916ea9db
This commit is contained in:
@@ -30,8 +30,8 @@ impl ModelsCacheManager {
|
||||
/// Attempt to load a fresh cache entry. Returns `None` if the cache doesn't exist or is stale.
|
||||
pub(crate) async fn load_fresh(&self, expected_version: &str) -> Option<ModelsCache> {
|
||||
info!(
|
||||
cache_path = %self.cache_path.display(),
|
||||
expected_version,
|
||||
cache_path = %self.cache_path.display(),
|
||||
expected_version,
|
||||
"models cache: attempting load_fresh"
|
||||
);
|
||||
let cache = match self.load().await {
|
||||
|
||||
@@ -215,7 +215,7 @@ pub type SharedModelsManager = Arc<dyn ModelsManager>;
|
||||
pub struct OpenAiModelsManager {
|
||||
remote_models: RwLock<Vec<ModelInfo>>,
|
||||
etag: RwLock<Option<String>>,
|
||||
cache_manager: ModelsCacheManager,
|
||||
cache_manager: Option<ModelsCacheManager>,
|
||||
endpoint_client: SharedModelsEndpointClient,
|
||||
auth_manager: Option<Arc<AuthManager>>,
|
||||
}
|
||||
@@ -235,7 +235,26 @@ impl OpenAiModelsManager {
|
||||
auth_manager: Option<Arc<AuthManager>>,
|
||||
) -> Self {
|
||||
let cache_path = codex_home.join(MODEL_CACHE_FILE);
|
||||
let cache_manager = ModelsCacheManager::new(cache_path, DEFAULT_MODEL_CACHE_TTL);
|
||||
Self::new_with_cache_manager(
|
||||
Some(ModelsCacheManager::new(cache_path, DEFAULT_MODEL_CACHE_TTL)),
|
||||
endpoint_client,
|
||||
auth_manager,
|
||||
)
|
||||
}
|
||||
|
||||
/// Construct an OpenAI-compatible model manager with caching disabled.
|
||||
pub fn new_without_cache(
|
||||
endpoint_client: Arc<dyn ModelsEndpointClient>,
|
||||
auth_manager: Option<Arc<AuthManager>>,
|
||||
) -> Self {
|
||||
Self::new_with_cache_manager(/*cache_manager*/ None, endpoint_client, auth_manager)
|
||||
}
|
||||
|
||||
fn new_with_cache_manager(
|
||||
cache_manager: Option<ModelsCacheManager>,
|
||||
endpoint_client: Arc<dyn ModelsEndpointClient>,
|
||||
auth_manager: Option<Arc<AuthManager>>,
|
||||
) -> Self {
|
||||
let remote_models = load_remote_models_from_file().unwrap_or_default();
|
||||
Self {
|
||||
remote_models: RwLock::new(remote_models),
|
||||
@@ -319,7 +338,9 @@ 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()) {
|
||||
if let Err(err) = self.cache_manager.renew_cache_ttl().await {
|
||||
if let Some(cache_manager) = self.cache_manager.as_ref()
|
||||
&& let Err(err) = cache_manager.renew_cache_ttl().await
|
||||
{
|
||||
error!("failed to renew cache TTL: {err}");
|
||||
}
|
||||
return;
|
||||
@@ -381,9 +402,11 @@ impl OpenAiModelsManager {
|
||||
.await?;
|
||||
self.apply_remote_models(models.clone()).await;
|
||||
*self.etag.write().await = etag.clone();
|
||||
self.cache_manager
|
||||
.persist_cache(&models, etag, client_version)
|
||||
.await;
|
||||
if let Some(cache_manager) = self.cache_manager.as_ref() {
|
||||
cache_manager
|
||||
.persist_cache(&models, etag, client_version)
|
||||
.await;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -429,13 +452,16 @@ impl OpenAiModelsManager {
|
||||
|
||||
/// Attempt to satisfy the refresh from the cache when it matches the provider and TTL.
|
||||
async fn try_load_cache(&self) -> bool {
|
||||
let Some(cache_manager) = self.cache_manager.as_ref() else {
|
||||
return false;
|
||||
};
|
||||
let _timer =
|
||||
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 cache = match self.cache_manager.load_fresh(&client_version).await {
|
||||
let cache = match cache_manager.load_fresh(&client_version).await {
|
||||
Some(cache) => cache,
|
||||
None => {
|
||||
info!("models cache: no usable cache entry");
|
||||
|
||||
@@ -209,6 +209,36 @@ fn static_manager_for_tests(model_catalog: ModelsResponse) -> StaticModelsManage
|
||||
StaticModelsManager::new(/*auth_manager*/ None, model_catalog)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn manager_without_cache_fetches_on_every_refresh() {
|
||||
let remote_models = vec![remote_model("remote", "Remote", /*priority*/ 0)];
|
||||
let endpoint = TestModelsEndpoint::new(vec![remote_models.clone(), remote_models.clone()]);
|
||||
let manager = OpenAiModelsManager::new_without_cache(
|
||||
endpoint.clone(),
|
||||
Some(AuthManager::from_auth_for_testing(
|
||||
CodexAuth::create_dummy_chatgpt_auth_for_testing(),
|
||||
)),
|
||||
);
|
||||
|
||||
let catalog = manager
|
||||
.raw_model_catalog(
|
||||
RefreshStrategy::OnlineIfUncached,
|
||||
DEFAULT_HTTP_CLIENT_FACTORY,
|
||||
)
|
||||
.await;
|
||||
let second_catalog = manager
|
||||
.raw_model_catalog(
|
||||
RefreshStrategy::OnlineIfUncached,
|
||||
DEFAULT_HTTP_CLIENT_FACTORY,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(catalog.models, remote_models);
|
||||
assert_eq!(second_catalog, catalog);
|
||||
assert_eq!(manager.get_remote_models().await, remote_models);
|
||||
assert_eq!(endpoint.fetch_count(), 2);
|
||||
}
|
||||
|
||||
async fn chatgpt_auth_tokens_for_tests(codex_home: &Path) -> CodexAuth {
|
||||
let auth_dot_json = codex_login::AuthDotJson {
|
||||
auth_mode: Some(AuthMode::ChatgptAuthTokens),
|
||||
@@ -719,6 +749,8 @@ async fn refresh_available_models_refetches_when_cache_stale() {
|
||||
// Rewrite cache with an old timestamp so it is treated as stale.
|
||||
manager
|
||||
.cache_manager
|
||||
.as_ref()
|
||||
.expect("cached model manager")
|
||||
.manipulate_cache_for_test(|fetched_at| {
|
||||
*fetched_at = Utc::now() - chrono::Duration::hours(1);
|
||||
})
|
||||
@@ -758,6 +790,8 @@ async fn refresh_available_models_refetches_when_version_mismatch() {
|
||||
|
||||
manager
|
||||
.cache_manager
|
||||
.as_ref()
|
||||
.expect("cached model manager")
|
||||
.mutate_cache_for_test(|cache| {
|
||||
let client_version = crate::client_version_to_whole();
|
||||
cache.client_version = Some(format!("{client_version}-mismatch"));
|
||||
@@ -795,7 +829,11 @@ async fn refresh_available_models_drops_removed_remote_models() {
|
||||
)];
|
||||
let endpoint = TestModelsEndpoint::new(vec![initial_models, refreshed_models]);
|
||||
let mut manager = openai_manager_for_tests(codex_home.path().to_path_buf(), endpoint.clone());
|
||||
manager.cache_manager.set_ttl(Duration::ZERO);
|
||||
manager
|
||||
.cache_manager
|
||||
.as_mut()
|
||||
.expect("cached model manager")
|
||||
.set_ttl(Duration::ZERO);
|
||||
|
||||
manager
|
||||
.refresh_available_models(
|
||||
|
||||
Reference in New Issue
Block a user