Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 47 additions & 1 deletion codex-rs/app-server-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ use codex_app_server_protocol::ServerNotification;
use codex_app_server_protocol::ServerRequest;
use codex_arg0::Arg0DispatchPaths;
use codex_config::NoopThreadConfigLoader;
use codex_config::RemoteThreadConfigLoader;
use codex_config::ThreadConfigLoader;
use codex_core::config::Config;
use codex_core::config_loader::CloudRequirementsLoader;
use codex_core::config_loader::LoaderOverrides;
Expand Down Expand Up @@ -357,6 +359,13 @@ pub struct InProcessClientStartArgs {
pub channel_capacity: usize,
}

fn configured_thread_config_loader(config: &Config) -> Arc<dyn ThreadConfigLoader> {
match config.experimental_thread_config_endpoint.as_deref() {
Some(endpoint) => Arc::new(RemoteThreadConfigLoader::new(endpoint)),
None => Arc::new(NoopThreadConfigLoader),
}
}

impl InProcessClientStartArgs {
/// Builds initialize params from caller-provided metadata.
pub fn initialize_params(&self) -> InitializeParams {
Expand All @@ -381,13 +390,14 @@ impl InProcessClientStartArgs {

fn into_runtime_start_args(self) -> InProcessStartArgs {
let initialize = self.initialize_params();
let thread_config_loader = configured_thread_config_loader(&self.config);
InProcessStartArgs {
arg0_paths: self.arg0_paths,
config: self.config,
cli_overrides: self.cli_overrides,
loader_overrides: self.loader_overrides,
cloud_requirements: self.cloud_requirements,
thread_config_loader: Arc::new(NoopThreadConfigLoader),
thread_config_loader,
feedback: self.feedback,
log_db: self.log_db,
environment_manager: self.environment_manager,
Expand Down Expand Up @@ -2013,6 +2023,42 @@ mod tests {
);
}

#[tokio::test]
async fn runtime_start_args_use_remote_thread_config_loader_when_configured() {
let mut config = build_test_config().await;
config.experimental_thread_config_endpoint = Some("not-a-valid-endpoint".to_string());

let runtime_args = InProcessClientStartArgs {
arg0_paths: Arg0DispatchPaths::default(),
config: Arc::new(config),
cli_overrides: Vec::new(),
loader_overrides: LoaderOverrides::default(),
cloud_requirements: CloudRequirementsLoader::default(),
feedback: CodexFeedback::new(),
log_db: None,
environment_manager: Arc::new(EnvironmentManager::default_for_tests()),
config_warnings: Vec::new(),
session_source: SessionSource::Exec,
enable_codex_api_key_env: false,
client_name: "codex-app-server-client-test".to_string(),
client_version: "0.0.0-test".to_string(),
experimental_api: true,
opt_out_notification_methods: Vec::new(),
channel_capacity: DEFAULT_IN_PROCESS_CHANNEL_CAPACITY,
}
.into_runtime_start_args();

let err = runtime_args
.thread_config_loader
.load(Default::default())
.await
.expect_err("configured remote loader should try to connect");
assert_eq!(
err.code(),
codex_config::ThreadConfigLoadErrorCode::RequestFailed
);
}

#[tokio::test]
async fn shutdown_completes_promptly_without_retained_managers() {
let client = start_test_client(SessionSource::Cli).await;
Expand Down
27 changes: 23 additions & 4 deletions codex-rs/app-server/src/config_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub(crate) struct ConfigManager {
loader_overrides: LoaderOverrides,
cloud_requirements: Arc<RwLock<CloudRequirementsLoader>>,
arg0_paths: Arg0DispatchPaths,
thread_config_loader: Arc<dyn ThreadConfigLoader>,
thread_config_loader: Arc<RwLock<Arc<dyn ThreadConfigLoader>>>,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we create loader early enough not to need hot-replace?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tricky part is that we depend on the config to resolve config settings, similar to how the cloud requirements depend on chatgpt_base_url. We can resolve the config values again here if we want to but that also seems a bit dirty

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, circular dependencies here are brutal 😢

host_name: Option<String>,
}

Expand Down Expand Up @@ -73,7 +73,7 @@ impl ConfigManager {
loader_overrides,
cloud_requirements: Arc::new(RwLock::new(cloud_requirements)),
arg0_paths,
thread_config_loader,
thread_config_loader: Arc::new(RwLock::new(thread_config_loader)),
host_name,
}
}
Expand Down Expand Up @@ -120,6 +120,24 @@ impl ConfigManager {
}
}

pub(crate) fn replace_thread_config_loader(
&self,
thread_config_loader: Arc<dyn ThreadConfigLoader>,
) {
if let Ok(mut guard) = self.thread_config_loader.write() {
*guard = thread_config_loader;
} else {
warn!("failed to update thread config loader");
}
}

fn current_thread_config_loader(&self) -> Arc<dyn ThreadConfigLoader> {
self.thread_config_loader
.read()
.map(|guard| Arc::clone(&*guard))
.unwrap_or_else(|_| Arc::new(codex_config::NoopThreadConfigLoader))
}

pub(crate) async fn sync_default_client_residency_requirement(&self) {
match self.load_latest_config(/*fallback_cwd*/ None).await {
Ok(config) => {
Expand Down Expand Up @@ -210,7 +228,7 @@ impl ConfigManager {
.harness_overrides(typesafe_overrides)
.fallback_cwd(fallback_cwd)
.cloud_requirements(self.current_cloud_requirements())
.thread_config_loader(Arc::clone(&self.thread_config_loader))
.thread_config_loader(self.current_thread_config_loader())
.host_name(self.host_name.clone())
.build()
.await?;
Expand All @@ -230,14 +248,15 @@ impl ConfigManager {
&self,
cwd: Option<AbsolutePathBuf>,
) -> std::io::Result<ConfigLayerStack> {
let thread_config_loader = self.current_thread_config_loader();
load_config_layers_state(
LOCAL_FS.as_ref(),
&self.codex_home,
cwd,
&self.current_cli_overrides(),
self.loader_overrides.clone(),
self.current_cloud_requirements(),
self.thread_config_loader.as_ref(),
thread_config_loader.as_ref(),
self.host_name.as_deref(),
)
.await
Expand Down
14 changes: 12 additions & 2 deletions codex-rs/app-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use codex_arg0::Arg0DispatchPaths;
use codex_config::NoopThreadConfigLoader;
use codex_config::RemoteThreadConfigLoader;
use codex_config::ThreadConfigLoader;
use codex_core::config::Config;
use codex_core::config_loader::ConfigLayerStackOrdering;
Expand Down Expand Up @@ -107,6 +108,13 @@ enum LogFormat {

type StderrLogLayer = Box<dyn Layer<Registry> + Send + Sync + 'static>;

fn configured_thread_config_loader(config: &Config) -> Arc<dyn ThreadConfigLoader> {
match config.experimental_thread_config_endpoint.as_deref() {
Some(endpoint) => Arc::new(RemoteThreadConfigLoader::new(endpoint)),
None => Arc::new(NoopThreadConfigLoader),
}
}

/// Control-plane messages from the processor/transport side to the outbound router task.
///
/// `run_main_with_transport` now uses two loops/tasks:
Expand Down Expand Up @@ -382,14 +390,13 @@ pub async fn run_main_with_transport(
)
})?;
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(),
Arc::new(NoopThreadConfigLoader),
);
match config_manager
.load_latest_config(/*fallback_cwd*/ None)
Expand All @@ -413,6 +420,9 @@ pub async fn run_main_with_transport(
}
}

let discovered_thread_config_loader = configured_thread_config_loader(&config);
config_manager
.replace_thread_config_loader(Arc::clone(&discovered_thread_config_loader));
let auth_manager =
AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false);
config_manager.replace_cloud_requirements_loader(auth_manager, config.chatgpt_base_url);
Expand Down
4 changes: 4 additions & 0 deletions codex-rs/config/src/config_toml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,10 @@ pub struct ConfigToml {
/// Experimental / do not use. When set, app-server uses a remote thread
/// store at this endpoint instead of the local filesystem/SQLite store.
pub experimental_thread_store_endpoint: Option<String>,

/// Experimental / do not use. When set, app-server fetches thread-scoped
/// config from a remote service at this endpoint.
pub experimental_thread_config_endpoint: Option<String>,
pub projects: Option<HashMap<String, ProjectConfig>>,

/// Controls the web search tool mode: disabled, cached, or live.
Expand Down
4 changes: 4 additions & 0 deletions codex-rs/core/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -2484,6 +2484,10 @@
"description": "Experimental / do not use. Replaces the synthesized realtime startup context appended to websocket session instructions. An empty string disables startup context injection entirely.",
"type": "string"
},
"experimental_thread_config_endpoint": {
"description": "Experimental / do not use. When set, app-server fetches thread-scoped config from a remote service at this endpoint.",
"type": "string"
},
"experimental_thread_store_endpoint": {
"description": "Experimental / do not use. When set, app-server uses a remote thread store at this endpoint instead of the local filesystem/SQLite store.",
"type": "string"
Expand Down
33 changes: 33 additions & 0 deletions codex-rs/core/src/config/config_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5237,6 +5237,7 @@ async fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
experimental_realtime_ws_backend_prompt: None,
experimental_realtime_ws_startup_context: None,
experimental_thread_store_endpoint: None,
experimental_thread_config_endpoint: None,
base_instructions: None,
developer_instructions: None,
guardian_policy_config: None,
Expand Down Expand Up @@ -5433,6 +5434,7 @@ async fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
experimental_realtime_ws_backend_prompt: None,
experimental_realtime_ws_startup_context: None,
experimental_thread_store_endpoint: None,
experimental_thread_config_endpoint: None,
base_instructions: None,
developer_instructions: None,
guardian_policy_config: None,
Expand Down Expand Up @@ -5583,6 +5585,7 @@ async fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
experimental_realtime_ws_backend_prompt: None,
experimental_realtime_ws_startup_context: None,
experimental_thread_store_endpoint: None,
experimental_thread_config_endpoint: None,
base_instructions: None,
developer_instructions: None,
guardian_policy_config: None,
Expand Down Expand Up @@ -5718,6 +5721,7 @@ async fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
experimental_realtime_ws_backend_prompt: None,
experimental_realtime_ws_startup_context: None,
experimental_thread_store_endpoint: None,
experimental_thread_config_endpoint: None,
base_instructions: None,
developer_instructions: None,
guardian_policy_config: None,
Expand Down Expand Up @@ -7240,6 +7244,35 @@ experimental_realtime_start_instructions = "start instructions from config"
Ok(())
}

#[tokio::test]
async fn experimental_thread_config_endpoint_loads_from_config_toml() -> std::io::Result<()> {
let cfg: ConfigToml = toml::from_str(
r#"
experimental_thread_config_endpoint = "http://127.0.0.1:8061"
"#,
)
.expect("TOML deserialization should succeed");

assert_eq!(
cfg.experimental_thread_config_endpoint.as_deref(),
Some("http://127.0.0.1:8061")
);

let codex_home = TempDir::new()?;
let config = Config::load_from_base_config_with_overrides(
cfg,
ConfigOverrides::default(),
codex_home.abs(),
)
.await?;

assert_eq!(
config.experimental_thread_config_endpoint.as_deref(),
Some("http://127.0.0.1:8061")
);
Ok(())
}

#[tokio::test]
async fn experimental_realtime_ws_base_url_loads_from_config_toml() -> std::io::Result<()> {
let cfg: ConfigToml = toml::from_str(
Expand Down
5 changes: 5 additions & 0 deletions codex-rs/core/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,10 @@ pub struct Config {
/// Experimental / do not use. When set, app-server uses a remote thread
/// store at this endpoint instead of the local filesystem/SQLite store.
pub experimental_thread_store_endpoint: Option<String>,

/// Experimental / do not use. When set, app-server fetches thread-scoped
/// config from a remote service at this endpoint.
pub experimental_thread_config_endpoint: Option<String>,
/// When set, restricts ChatGPT login to a specific workspace identifier.
pub forced_chatgpt_workspace_id: Option<String>,

Expand Down Expand Up @@ -2419,6 +2423,7 @@ impl Config {
experimental_realtime_ws_startup_context: cfg.experimental_realtime_ws_startup_context,
experimental_realtime_start_instructions: cfg.experimental_realtime_start_instructions,
experimental_thread_store_endpoint: cfg.experimental_thread_store_endpoint,
experimental_thread_config_endpoint: cfg.experimental_thread_config_endpoint,
forced_chatgpt_workspace_id,
forced_login_method,
include_apply_patch_tool: include_apply_patch_tool_flag,
Expand Down
Loading