mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Load app-server config through ConfigManager (#18870)
## Summary - Load app-server startup config through `ConfigManager` instead of direct `ConfigBuilder` calls. - Move `ConfigManager` constructor-owned state (`cli_overrides`, runtime feature map, cloud requirements loader) behind internal manager fields. - Pass `ConfigManager` into `MessageProcessor` directly instead of reconstructing it from raw args. ## Tests - `cargo check -p codex-app-server` - `cargo test -p codex-app-server` - `just fix -p codex-app-server` - `just fmt`
This commit is contained in:
committed by
GitHub
Unverified
parent
15b8cde2a4
commit
ffa6944587
@@ -10352,7 +10352,6 @@ mod tests {
|
||||
use std::collections::BTreeMap;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::sync::RwLock;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[test]
|
||||
@@ -10628,10 +10627,9 @@ mod tests {
|
||||
};
|
||||
let config_manager = ConfigManager::new(
|
||||
temp_dir.path().to_path_buf(),
|
||||
Arc::new(RwLock::new(Vec::new())),
|
||||
Arc::new(RwLock::new(BTreeMap::new())),
|
||||
Vec::new(),
|
||||
LoaderOverrides::default(),
|
||||
Arc::new(RwLock::new(CloudRequirementsLoader::default())),
|
||||
CloudRequirementsLoader::default(),
|
||||
Arg0DispatchPaths::default(),
|
||||
Arc::new(StaticThreadConfigLoader::new(vec![
|
||||
ThreadConfigSource::Session(SessionThreadConfig {
|
||||
|
||||
@@ -432,7 +432,6 @@ mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::collections::BTreeMap;
|
||||
use std::sync::RwLock;
|
||||
use std::sync::atomic::AtomicUsize;
|
||||
use std::sync::atomic::Ordering;
|
||||
use tempfile::TempDir;
|
||||
@@ -733,10 +732,9 @@ mod tests {
|
||||
let config_api = ConfigApi::new(
|
||||
ConfigManager::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
Arc::new(RwLock::new(Vec::new())),
|
||||
Arc::new(RwLock::new(BTreeMap::new())),
|
||||
Vec::new(),
|
||||
LoaderOverrides::default(),
|
||||
Arc::new(RwLock::new(CloudRequirementsLoader::default())),
|
||||
CloudRequirementsLoader::default(),
|
||||
Arg0DispatchPaths::default(),
|
||||
Arc::new(codex_config::NoopThreadConfigLoader),
|
||||
),
|
||||
|
||||
@@ -39,17 +39,15 @@ pub(crate) struct ConfigManager {
|
||||
impl ConfigManager {
|
||||
pub(crate) fn new(
|
||||
codex_home: PathBuf,
|
||||
cli_overrides: Arc<RwLock<Vec<(String, TomlValue)>>>,
|
||||
runtime_feature_enablement: Arc<RwLock<BTreeMap<String, bool>>>,
|
||||
cli_overrides: Vec<(String, TomlValue)>,
|
||||
loader_overrides: LoaderOverrides,
|
||||
cloud_requirements: Arc<RwLock<CloudRequirementsLoader>>,
|
||||
cloud_requirements: CloudRequirementsLoader,
|
||||
arg0_paths: Arg0DispatchPaths,
|
||||
thread_config_loader: Arc<dyn ThreadConfigLoader>,
|
||||
) -> Self {
|
||||
Self::new_with_host_name(
|
||||
codex_home,
|
||||
cli_overrides,
|
||||
runtime_feature_enablement,
|
||||
loader_overrides,
|
||||
cloud_requirements,
|
||||
arg0_paths,
|
||||
@@ -61,20 +59,19 @@ impl ConfigManager {
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn new_with_host_name(
|
||||
codex_home: PathBuf,
|
||||
cli_overrides: Arc<RwLock<Vec<(String, TomlValue)>>>,
|
||||
runtime_feature_enablement: Arc<RwLock<BTreeMap<String, bool>>>,
|
||||
cli_overrides: Vec<(String, TomlValue)>,
|
||||
loader_overrides: LoaderOverrides,
|
||||
cloud_requirements: Arc<RwLock<CloudRequirementsLoader>>,
|
||||
cloud_requirements: CloudRequirementsLoader,
|
||||
arg0_paths: Arg0DispatchPaths,
|
||||
thread_config_loader: Arc<dyn ThreadConfigLoader>,
|
||||
host_name: Option<String>,
|
||||
) -> Self {
|
||||
Self {
|
||||
codex_home,
|
||||
cli_overrides,
|
||||
runtime_feature_enablement,
|
||||
cli_overrides: Arc::new(RwLock::new(cli_overrides)),
|
||||
runtime_feature_enablement: Arc::new(RwLock::new(BTreeMap::new())),
|
||||
loader_overrides,
|
||||
cloud_requirements,
|
||||
cloud_requirements: Arc::new(RwLock::new(cloud_requirements)),
|
||||
arg0_paths,
|
||||
thread_config_loader,
|
||||
host_name,
|
||||
@@ -148,6 +145,17 @@ impl ConfigManager {
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) async fn load_default_config(&self) -> std::io::Result<Config> {
|
||||
let mut config = Config::load_default_with_cli_overrides_for_codex_home(
|
||||
self.codex_home.clone(),
|
||||
self.current_cli_overrides(),
|
||||
)
|
||||
.await?;
|
||||
self.apply_runtime_feature_enablement(&mut config);
|
||||
self.apply_arg0_paths(&mut config);
|
||||
Ok(config)
|
||||
}
|
||||
|
||||
pub(crate) async fn load_with_overrides(
|
||||
&self,
|
||||
request_overrides: Option<HashMap<String, serde_json::Value>>,
|
||||
@@ -262,10 +270,9 @@ impl ConfigManager {
|
||||
) -> Self {
|
||||
Self::new_with_host_name(
|
||||
codex_home,
|
||||
Arc::new(RwLock::new(cli_overrides)),
|
||||
Arc::new(RwLock::new(BTreeMap::new())),
|
||||
cli_overrides,
|
||||
loader_overrides,
|
||||
Arc::new(RwLock::new(cloud_requirements)),
|
||||
cloud_requirements,
|
||||
Arg0DispatchPaths::default(),
|
||||
Arc::new(codex_config::NoopThreadConfigLoader),
|
||||
host_name,
|
||||
|
||||
@@ -50,6 +50,7 @@ use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::Ordering;
|
||||
use std::time::Duration;
|
||||
|
||||
use crate::config_manager::ConfigManager;
|
||||
use crate::error_code::INTERNAL_ERROR_CODE;
|
||||
use crate::error_code::INVALID_REQUEST_ERROR_CODE;
|
||||
use crate::error_code::OVERLOADED_ERROR_CODE;
|
||||
@@ -390,17 +391,22 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle {
|
||||
let processor_outgoing = Arc::clone(&outgoing_message_sender);
|
||||
let auth_manager =
|
||||
AuthManager::shared_from_config(args.config.as_ref(), args.enable_codex_api_key_env);
|
||||
let config_manager = ConfigManager::new(
|
||||
args.config.codex_home.to_path_buf(),
|
||||
args.cli_overrides,
|
||||
args.loader_overrides,
|
||||
args.cloud_requirements,
|
||||
args.arg0_paths.clone(),
|
||||
args.thread_config_loader,
|
||||
);
|
||||
let (processor_tx, mut processor_rx) = mpsc::channel::<ProcessorCommand>(channel_capacity);
|
||||
let mut processor_handle = tokio::spawn(async move {
|
||||
let processor = Arc::new(MessageProcessor::new(MessageProcessorArgs {
|
||||
outgoing: Arc::clone(&processor_outgoing),
|
||||
arg0_paths: args.arg0_paths,
|
||||
config: args.config,
|
||||
config_manager,
|
||||
environment_manager: args.environment_manager,
|
||||
cli_overrides: args.cli_overrides,
|
||||
loader_overrides: args.loader_overrides,
|
||||
cloud_requirements: args.cloud_requirements,
|
||||
thread_config_loader: args.thread_config_loader,
|
||||
feedback: args.feedback,
|
||||
log_db: args.log_db,
|
||||
config_warnings: args.config_warnings,
|
||||
|
||||
@@ -1,11 +1,9 @@
|
||||
#![deny(clippy::print_stdout, clippy::print_stderr)]
|
||||
|
||||
use codex_arg0::Arg0DispatchPaths;
|
||||
use codex_cloud_requirements::cloud_requirements_loader;
|
||||
use codex_config::NoopThreadConfigLoader;
|
||||
use codex_config::ThreadConfigLoader;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::ConfigBuilder;
|
||||
use codex_core::config_loader::CloudRequirementsLoader;
|
||||
use codex_core::config_loader::ConfigLayerStackOrdering;
|
||||
use codex_core::config_loader::LoaderOverrides;
|
||||
use codex_features::Feature;
|
||||
@@ -19,6 +17,7 @@ use std::sync::Arc;
|
||||
use std::sync::RwLock;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
|
||||
use crate::config_manager::ConfigManager;
|
||||
use crate::message_processor::MessageProcessor;
|
||||
use crate::message_processor::MessageProcessorArgs;
|
||||
use crate::outgoing_message::ConnectionId;
|
||||
@@ -43,6 +42,7 @@ use codex_app_server_protocol::TextPosition as AppTextPosition;
|
||||
use codex_app_server_protocol::TextRange as AppTextRange;
|
||||
use codex_core::ExecPolicyError;
|
||||
use codex_core::check_execpolicy_for_warnings;
|
||||
use codex_core::config::find_codex_home;
|
||||
use codex_core::config_loader::ConfigLoadError;
|
||||
use codex_core::config_loader::TextRange as CoreTextRange;
|
||||
use codex_exec_server::EnvironmentManager;
|
||||
@@ -54,7 +54,6 @@ use tokio::sync::mpsc;
|
||||
use tokio::sync::oneshot;
|
||||
use tokio::task::JoinHandle;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use toml::Value as TomlValue;
|
||||
use tracing::Level;
|
||||
use tracing::error;
|
||||
use tracing::info;
|
||||
@@ -381,10 +380,18 @@ pub async fn run_main_with_transport(
|
||||
format!("error parsing -c overrides: {e}"),
|
||||
)
|
||||
})?;
|
||||
let cloud_requirements = match ConfigBuilder::default()
|
||||
.cli_overrides(cli_kv_overrides.clone())
|
||||
.loader_overrides(loader_overrides.clone())
|
||||
.build()
|
||||
let codex_home = find_codex_home()?;
|
||||
let thread_config_loader: Arc<dyn ThreadConfigLoader> = Arc::new(NoopThreadConfigLoader);
|
||||
let config_manager = ConfigManager::new(
|
||||
codex_home.to_path_buf(),
|
||||
cli_kv_overrides.clone(),
|
||||
loader_overrides,
|
||||
Default::default(),
|
||||
arg0_paths.clone(),
|
||||
thread_config_loader.clone(),
|
||||
);
|
||||
match config_manager
|
||||
.load_latest_config(/*fallback_cwd*/ None)
|
||||
.await
|
||||
{
|
||||
Ok(config) => {
|
||||
@@ -407,40 +414,28 @@ pub async fn run_main_with_transport(
|
||||
|
||||
let auth_manager =
|
||||
AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false);
|
||||
cloud_requirements_loader(
|
||||
auth_manager,
|
||||
config.chatgpt_base_url,
|
||||
config.codex_home.to_path_buf(),
|
||||
)
|
||||
config_manager.replace_cloud_requirements_loader(auth_manager, config.chatgpt_base_url);
|
||||
}
|
||||
Err(err) => {
|
||||
warn!(error = %err, "Failed to preload config for cloud requirements");
|
||||
// TODO(gt): Make cloud requirements preload failures blocking once we can fail-closed.
|
||||
CloudRequirementsLoader::default()
|
||||
}
|
||||
};
|
||||
let loader_overrides_for_config_api = loader_overrides.clone();
|
||||
let thread_config_loader = Arc::new(NoopThreadConfigLoader);
|
||||
let mut config_warnings = Vec::new();
|
||||
let config = match ConfigBuilder::default()
|
||||
.cli_overrides(cli_kv_overrides.clone())
|
||||
.loader_overrides(loader_overrides)
|
||||
.cloud_requirements(cloud_requirements.clone())
|
||||
.build()
|
||||
let config = match config_manager
|
||||
.load_latest_config(/*fallback_cwd*/ None)
|
||||
.await
|
||||
{
|
||||
Ok(config) => config,
|
||||
Err(err) => {
|
||||
let message = config_warning_from_error("Invalid configuration; using defaults.", &err);
|
||||
config_warnings.push(message);
|
||||
Config::load_default_with_cli_overrides(cli_kv_overrides.clone())
|
||||
.await
|
||||
.map_err(|e| {
|
||||
std::io::Error::new(
|
||||
ErrorKind::InvalidData,
|
||||
format!("error loading default config after config error: {e}"),
|
||||
)
|
||||
})?
|
||||
config_manager.load_default_config().await.map_err(|e| {
|
||||
std::io::Error::new(
|
||||
ErrorKind::InvalidData,
|
||||
format!("error loading default config after config error: {e}"),
|
||||
)
|
||||
})?
|
||||
}
|
||||
};
|
||||
|
||||
@@ -654,17 +649,12 @@ pub async fn run_main_with_transport(
|
||||
let outbound_control_tx = outbound_control_tx;
|
||||
let auth_manager =
|
||||
AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false);
|
||||
let cli_overrides: Vec<(String, TomlValue)> = cli_kv_overrides.clone();
|
||||
let loader_overrides = loader_overrides_for_config_api;
|
||||
let processor = Arc::new(MessageProcessor::new(MessageProcessorArgs {
|
||||
outgoing: outgoing_message_sender,
|
||||
arg0_paths,
|
||||
config: Arc::new(config),
|
||||
config_manager,
|
||||
environment_manager,
|
||||
cli_overrides,
|
||||
loader_overrides,
|
||||
cloud_requirements: cloud_requirements.clone(),
|
||||
thread_config_loader,
|
||||
feedback: feedback.clone(),
|
||||
log_db,
|
||||
config_warnings,
|
||||
|
||||
@@ -1,9 +1,7 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashSet;
|
||||
use std::future::Future;
|
||||
use std::sync::Arc;
|
||||
use std::sync::OnceLock;
|
||||
use std::sync::RwLock;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::Ordering;
|
||||
|
||||
@@ -64,11 +62,8 @@ use codex_app_server_protocol::ServerRequestPayload;
|
||||
use codex_app_server_protocol::experimental_required_message;
|
||||
use codex_arg0::Arg0DispatchPaths;
|
||||
use codex_chatgpt::connectors;
|
||||
use codex_config::ThreadConfigLoader;
|
||||
use codex_core::ThreadManager;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config_loader::CloudRequirementsLoader;
|
||||
use codex_core::config_loader::LoaderOverrides;
|
||||
use codex_exec_server::EnvironmentManager;
|
||||
use codex_features::Feature;
|
||||
use codex_feedback::CodexFeedback;
|
||||
@@ -92,7 +87,6 @@ use tokio::sync::broadcast;
|
||||
use tokio::sync::watch;
|
||||
use tokio::time::Duration;
|
||||
use tokio::time::timeout;
|
||||
use toml::Value as TomlValue;
|
||||
use tracing::Instrument;
|
||||
|
||||
const EXTERNAL_AUTH_REFRESH_TIMEOUT: Duration = Duration::from_secs(10);
|
||||
@@ -233,11 +227,8 @@ pub(crate) struct MessageProcessorArgs {
|
||||
pub(crate) outgoing: Arc<OutgoingMessageSender>,
|
||||
pub(crate) arg0_paths: Arg0DispatchPaths,
|
||||
pub(crate) config: Arc<Config>,
|
||||
pub(crate) config_manager: ConfigManager,
|
||||
pub(crate) environment_manager: Arc<EnvironmentManager>,
|
||||
pub(crate) cli_overrides: Vec<(String, TomlValue)>,
|
||||
pub(crate) loader_overrides: LoaderOverrides,
|
||||
pub(crate) cloud_requirements: CloudRequirementsLoader,
|
||||
pub(crate) thread_config_loader: Arc<dyn ThreadConfigLoader>,
|
||||
pub(crate) feedback: CodexFeedback,
|
||||
pub(crate) log_db: Option<LogDbLayer>,
|
||||
pub(crate) config_warnings: Vec<ConfigWarningNotification>,
|
||||
@@ -255,11 +246,8 @@ impl MessageProcessor {
|
||||
outgoing,
|
||||
arg0_paths,
|
||||
config,
|
||||
config_manager,
|
||||
environment_manager,
|
||||
cli_overrides,
|
||||
loader_overrides,
|
||||
cloud_requirements,
|
||||
thread_config_loader,
|
||||
feedback,
|
||||
log_db,
|
||||
config_warnings,
|
||||
@@ -292,18 +280,6 @@ impl MessageProcessor {
|
||||
.plugins_manager()
|
||||
.set_analytics_events_client(analytics_events_client.clone());
|
||||
|
||||
let cli_overrides = Arc::new(RwLock::new(cli_overrides));
|
||||
let runtime_feature_enablement = Arc::new(RwLock::new(BTreeMap::new()));
|
||||
let cloud_requirements = Arc::new(RwLock::new(cloud_requirements));
|
||||
let config_manager = ConfigManager::new(
|
||||
config.codex_home.to_path_buf(),
|
||||
cli_overrides,
|
||||
runtime_feature_enablement,
|
||||
loader_overrides,
|
||||
cloud_requirements,
|
||||
arg0_paths.clone(),
|
||||
thread_config_loader,
|
||||
);
|
||||
let codex_message_processor = CodexMessageProcessor::new(CodexMessageProcessorArgs {
|
||||
auth_manager: auth_manager.clone(),
|
||||
thread_manager: Arc::clone(&thread_manager),
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use super::ConnectionSessionState;
|
||||
use super::MessageProcessor;
|
||||
use super::MessageProcessorArgs;
|
||||
use crate::config_manager::ConfigManager;
|
||||
use crate::outgoing_message::ConnectionId;
|
||||
use crate::outgoing_message::OutgoingMessageSender;
|
||||
use crate::transport::AppServerTransport;
|
||||
@@ -233,15 +234,20 @@ fn build_test_processor(
|
||||
let outgoing = Arc::new(OutgoingMessageSender::new(outgoing_tx));
|
||||
let auth_manager =
|
||||
AuthManager::shared_from_config(config.as_ref(), /*enable_codex_api_key_env*/ false);
|
||||
let config_manager = ConfigManager::new(
|
||||
config.codex_home.to_path_buf(),
|
||||
Vec::new(),
|
||||
LoaderOverrides::default(),
|
||||
CloudRequirementsLoader::default(),
|
||||
Arg0DispatchPaths::default(),
|
||||
Arc::new(codex_config::NoopThreadConfigLoader),
|
||||
);
|
||||
let processor = Arc::new(MessageProcessor::new(MessageProcessorArgs {
|
||||
outgoing,
|
||||
arg0_paths: Arg0DispatchPaths::default(),
|
||||
config,
|
||||
config_manager,
|
||||
environment_manager: Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)),
|
||||
cli_overrides: Vec::new(),
|
||||
loader_overrides: LoaderOverrides::default(),
|
||||
cloud_requirements: CloudRequirementsLoader::default(),
|
||||
thread_config_loader: Arc::new(codex_config::NoopThreadConfigLoader),
|
||||
feedback: CodexFeedback::new(),
|
||||
log_db: None,
|
||||
config_warnings: Vec::new(),
|
||||
|
||||
Reference in New Issue
Block a user