Stop using AuthManager as the source of codex_home (#8846)

This commit is contained in:
pakrym-oai
2026-01-07 18:56:20 +00:00
committed by GitHub
parent c31960b13a
commit 018de994b0
13 changed files with 79 additions and 47 deletions
+2 -19
View File
@@ -32,11 +32,7 @@ use crate::token_data::parse_id_token;
use crate::util::try_parse_error_message;
use codex_client::CodexHttpClient;
use codex_protocol::account::PlanType as AccountPlanType;
#[cfg(any(test, feature = "test-support"))]
use once_cell::sync::Lazy;
use serde_json::Value;
#[cfg(any(test, feature = "test-support"))]
use tempfile::TempDir;
use thiserror::Error;
#[derive(Debug, Clone)]
@@ -66,9 +62,6 @@ const REFRESH_TOKEN_UNKNOWN_MESSAGE: &str =
const REFRESH_TOKEN_URL: &str = "https://auth.openai.com/oauth/token";
pub const REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR: &str = "CODEX_REFRESH_TOKEN_URL_OVERRIDE";
#[cfg(any(test, feature = "test-support"))]
static TEST_AUTH_TEMP_DIRS: Lazy<Mutex<Vec<TempDir>>> = Lazy::new(|| Mutex::new(Vec::new()));
#[derive(Debug, Error)]
pub enum RefreshTokenError {
#[error("{0}")]
@@ -672,18 +665,12 @@ impl AuthManager {
}
#[cfg(any(test, feature = "test-support"))]
#[expect(clippy::expect_used)]
/// Create an AuthManager with a specific CodexAuth, for testing only.
pub fn from_auth_for_testing(auth: CodexAuth) -> Arc<Self> {
let cached = CachedAuth { auth: Some(auth) };
let temp_dir = tempfile::tempdir().expect("temp codex home");
let codex_home = temp_dir.path().to_path_buf();
TEST_AUTH_TEMP_DIRS
.lock()
.expect("lock test codex homes")
.push(temp_dir);
Arc::new(Self {
codex_home,
codex_home: PathBuf::from("non-existent"),
inner: RwLock::new(cached),
enable_codex_api_key_env: false,
auth_credentials_store_mode: AuthCredentialsStoreMode::File,
@@ -707,10 +694,6 @@ impl AuthManager {
self.inner.read().ok().and_then(|c| c.auth.clone())
}
pub fn codex_home(&self) -> &Path {
&self.codex_home
}
/// Force a reload of the auth information from auth.json. Returns
/// whether the auth value changed.
pub fn reload(&self) -> bool {
+8 -2
View File
@@ -3479,7 +3479,10 @@ mod tests {
let conversation_id = ThreadId::default();
let auth_manager =
AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
let models_manager = Arc::new(ModelsManager::new(auth_manager.clone()));
let models_manager = Arc::new(ModelsManager::new(
config.codex_home.clone(),
auth_manager.clone(),
));
let agent_control = AgentControl::default();
let exec_policy = ExecPolicyManager::default();
let agent_status = Arc::new(RwLock::new(AgentStatus::PendingInit));
@@ -3570,7 +3573,10 @@ mod tests {
let conversation_id = ThreadId::default();
let auth_manager =
AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
let models_manager = Arc::new(ModelsManager::new(auth_manager.clone()));
let models_manager = Arc::new(ModelsManager::new(
config.codex_home.clone(),
auth_manager.clone(),
));
let agent_control = AgentControl::default();
let exec_policy = ExecPolicyManager::default();
let agent_status = Arc::new(RwLock::new(AgentStatus::PendingInit));
+17 -9
View File
@@ -47,8 +47,7 @@ pub struct ModelsManager {
impl ModelsManager {
/// Construct a manager scoped to the provided `AuthManager`.
pub fn new(auth_manager: Arc<AuthManager>) -> Self {
let codex_home = auth_manager.codex_home().to_path_buf();
pub fn new(codex_home: PathBuf, auth_manager: Arc<AuthManager>) -> Self {
Self {
local_models: builtin_model_presets(auth_manager.get_auth_mode()),
remote_models: RwLock::new(Self::load_remote_models_from_file().unwrap_or_default()),
@@ -62,8 +61,11 @@ impl ModelsManager {
#[cfg(any(test, feature = "test-support"))]
/// Construct a manager scoped to the provided `AuthManager` with a specific provider. Used for integration tests.
pub fn with_provider(auth_manager: Arc<AuthManager>, provider: ModelProviderInfo) -> Self {
let codex_home = auth_manager.codex_home().to_path_buf();
pub fn with_provider(
codex_home: PathBuf,
auth_manager: Arc<AuthManager>,
provider: ModelProviderInfo,
) -> Self {
Self {
local_models: builtin_model_presets(auth_manager.get_auth_mode()),
remote_models: RwLock::new(Self::load_remote_models_from_file().unwrap_or_default()),
@@ -422,7 +424,8 @@ mod tests {
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(auth_manager, provider);
let manager =
ModelsManager::with_provider(codex_home.path().to_path_buf(), auth_manager, provider);
manager
.refresh_available_models_with_cache(&config)
@@ -481,7 +484,8 @@ mod tests {
AuthCredentialsStoreMode::File,
));
let provider = provider_for(server.uri());
let manager = ModelsManager::with_provider(auth_manager, provider);
let manager =
ModelsManager::with_provider(codex_home.path().to_path_buf(), auth_manager, provider);
manager
.refresh_available_models_with_cache(&config)
@@ -535,7 +539,8 @@ mod tests {
AuthCredentialsStoreMode::File,
));
let provider = provider_for(server.uri());
let manager = ModelsManager::with_provider(auth_manager, provider);
let manager =
ModelsManager::with_provider(codex_home.path().to_path_buf(), auth_manager, provider);
manager
.refresh_available_models_with_cache(&config)
@@ -605,7 +610,8 @@ mod tests {
let auth_manager =
AuthManager::from_auth_for_testing(CodexAuth::create_dummy_chatgpt_auth_for_testing());
let provider = provider_for(server.uri());
let mut manager = ModelsManager::with_provider(auth_manager, provider);
let mut manager =
ModelsManager::with_provider(codex_home.path().to_path_buf(), auth_manager, provider);
manager.cache_ttl = Duration::ZERO;
manager
@@ -653,10 +659,12 @@ mod tests {
#[test]
fn build_available_models_picks_default_after_hiding_hidden_models() {
let codex_home = tempdir().expect("temp dir");
let auth_manager =
AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
let provider = provider_for("http://example.test".to_string());
let mut manager = ModelsManager::with_provider(auth_manager, provider);
let mut manager =
ModelsManager::with_provider(codex_home.path().to_path_buf(), auth_manager, provider);
manager.local_models = Vec::new();
let hidden_model = remote_model_with_visibility("hidden", "Hidden", 0, "hide");
+12 -8
View File
@@ -59,14 +59,19 @@ pub(crate) struct ThreadManagerState {
}
impl ThreadManager {
pub fn new(auth_manager: Arc<AuthManager>, session_source: SessionSource) -> Self {
pub fn new(
codex_home: PathBuf,
auth_manager: Arc<AuthManager>,
session_source: SessionSource,
) -> Self {
Self {
state: Arc::new(ThreadManagerState {
threads: Arc::new(RwLock::new(HashMap::new())),
models_manager: Arc::new(ModelsManager::new(auth_manager.clone())),
skills_manager: Arc::new(SkillsManager::new(
auth_manager.codex_home().to_path_buf(),
models_manager: Arc::new(ModelsManager::new(
codex_home.clone(),
auth_manager.clone(),
)),
skills_manager: Arc::new(SkillsManager::new(codex_home)),
auth_manager,
session_source,
}),
@@ -94,17 +99,16 @@ impl ThreadManager {
provider: ModelProviderInfo,
codex_home: PathBuf,
) -> Self {
let auth_manager = AuthManager::from_auth_for_testing_with_home(auth, codex_home);
let auth_manager = AuthManager::from_auth_for_testing(auth);
Self {
state: Arc::new(ThreadManagerState {
threads: Arc::new(RwLock::new(HashMap::new())),
models_manager: Arc::new(ModelsManager::with_provider(
codex_home.clone(),
auth_manager.clone(),
provider,
)),
skills_manager: Arc::new(SkillsManager::new(
auth_manager.codex_home().to_path_buf(),
)),
skills_manager: Arc::new(SkillsManager::new(codex_home)),
auth_manager,
session_source: SessionSource::Exec,
}),