mirror of
https://github.com/openai/codex.git
synced 2026-11-11 06:31:37 +00:00
Remove the experimental thread config endpoint (#39115)
## What changed - Remove `experimental_thread_config_endpoint` from the configuration model and schema. - Stop app-server and the in-process client from constructing a remote thread config loader from local configuration. - Keep the thread config loader supplied at app-server startup fixed for the lifetime of the config manager. GitOrigin-RevId: e7907fee2d631e7ecf89ec6bac8f22bc2c221350
This commit is contained in:
1 parent
fd5018e044
commit
ca08a58ab4
8 files changed
+5
-128
No files matched your search
@@ -48,8 +48,6 @@ use codex_arg0::Arg0DispatchPaths;
|
|||||||
use codex_config::CloudConfigBundleLoader;
|
use codex_config::CloudConfigBundleLoader;
|
||||||
use codex_config::LoaderOverrides;
|
use codex_config::LoaderOverrides;
|
||||||
use codex_config::NoopThreadConfigLoader;
|
use codex_config::NoopThreadConfigLoader;
|
||||||
use codex_config::RemoteThreadConfigLoader;
|
|
||||||
use codex_config::ThreadConfigLoader;
|
|
||||||
use codex_core::config::Config;
|
use codex_core::config::Config;
|
||||||
pub use codex_core::otel_init::build_provider as build_otel_provider;
|
pub use codex_core::otel_init::build_provider as build_otel_provider;
|
||||||
pub use codex_exec_server::EnvironmentManager;
|
pub use codex_exec_server::EnvironmentManager;
|
||||||
@@ -212,13 +210,6 @@ pub struct InProcessClientStartArgs {
|
|||||||
pub channel_capacity: usize,
|
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 {
|
impl InProcessClientStartArgs {
|
||||||
/// Builds initialize params from caller-provided metadata.
|
/// Builds initialize params from caller-provided metadata.
|
||||||
pub fn initialize_params(&self) -> InitializeParams {
|
pub fn initialize_params(&self) -> InitializeParams {
|
||||||
@@ -246,7 +237,6 @@ impl InProcessClientStartArgs {
|
|||||||
|
|
||||||
fn into_runtime_start_args(self) -> InProcessStartArgs {
|
fn into_runtime_start_args(self) -> InProcessStartArgs {
|
||||||
let initialize = self.initialize_params();
|
let initialize = self.initialize_params();
|
||||||
let thread_config_loader = configured_thread_config_loader(&self.config);
|
|
||||||
InProcessStartArgs {
|
InProcessStartArgs {
|
||||||
arg0_paths: self.arg0_paths,
|
arg0_paths: self.arg0_paths,
|
||||||
config: self.config,
|
config: self.config,
|
||||||
@@ -254,7 +244,7 @@ impl InProcessClientStartArgs {
|
|||||||
loader_overrides: self.loader_overrides,
|
loader_overrides: self.loader_overrides,
|
||||||
strict_config: self.strict_config,
|
strict_config: self.strict_config,
|
||||||
cloud_config_bundle: self.cloud_config_bundle,
|
cloud_config_bundle: self.cloud_config_bundle,
|
||||||
thread_config_loader,
|
thread_config_loader: Arc::new(NoopThreadConfigLoader),
|
||||||
feedback: self.feedback,
|
feedback: self.feedback,
|
||||||
log_db: self.log_db,
|
log_db: self.log_db,
|
||||||
state_db: self.state_db,
|
state_db: self.state_db,
|
||||||
@@ -2014,45 +2004,6 @@ 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(),
|
|
||||||
strict_config: false,
|
|
||||||
cloud_config_bundle: CloudConfigBundleLoader::default(),
|
|
||||||
feedback: CodexFeedback::new(),
|
|
||||||
log_db: None,
|
|
||||||
state_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,
|
|
||||||
mcp_server_openai_form_elicitation: false,
|
|
||||||
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]
|
#[tokio::test]
|
||||||
async fn shutdown_completes_promptly_without_retained_managers() {
|
async fn shutdown_completes_promptly_without_retained_managers() {
|
||||||
let client = start_test_client(SessionSource::Cli).await;
|
let client = start_test_client(SessionSource::Cli).await;
|
||||||
|
|||||||
@@ -35,7 +35,7 @@ pub(crate) struct ConfigManager {
|
|||||||
strict_config: bool,
|
strict_config: bool,
|
||||||
cloud_config_bundle: Arc<RwLock<CloudConfigBundleLoader>>,
|
cloud_config_bundle: Arc<RwLock<CloudConfigBundleLoader>>,
|
||||||
arg0_paths: Arg0DispatchPaths,
|
arg0_paths: Arg0DispatchPaths,
|
||||||
thread_config_loader: Arc<RwLock<Arc<dyn ThreadConfigLoader>>>,
|
thread_config_loader: Arc<dyn ThreadConfigLoader>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl ConfigManager {
|
impl ConfigManager {
|
||||||
@@ -56,7 +56,7 @@ impl ConfigManager {
|
|||||||
strict_config,
|
strict_config,
|
||||||
cloud_config_bundle: Arc::new(RwLock::new(cloud_config_bundle)),
|
cloud_config_bundle: Arc::new(RwLock::new(cloud_config_bundle)),
|
||||||
arg0_paths,
|
arg0_paths,
|
||||||
thread_config_loader: Arc::new(RwLock::new(thread_config_loader)),
|
thread_config_loader,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -119,24 +119,6 @@ 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) {
|
pub(crate) async fn sync_default_client_residency_requirement(&self) {
|
||||||
match self.load_latest_config(/*fallback_cwd*/ None).await {
|
match self.load_latest_config(/*fallback_cwd*/ None).await {
|
||||||
Ok(config) => {
|
Ok(config) => {
|
||||||
@@ -256,7 +238,7 @@ impl ConfigManager {
|
|||||||
.harness_overrides(typesafe_overrides)
|
.harness_overrides(typesafe_overrides)
|
||||||
.fallback_cwd(fallback_cwd)
|
.fallback_cwd(fallback_cwd)
|
||||||
.cloud_config_bundle(self.current_cloud_config_bundle())
|
.cloud_config_bundle(self.current_cloud_config_bundle())
|
||||||
.thread_config_loader(self.current_thread_config_loader())
|
.thread_config_loader(Arc::clone(&self.thread_config_loader))
|
||||||
.build()
|
.build()
|
||||||
.await?;
|
.await?;
|
||||||
self.apply_runtime_feature_enablement(&mut config);
|
self.apply_runtime_feature_enablement(&mut config);
|
||||||
@@ -275,7 +257,6 @@ impl ConfigManager {
|
|||||||
&self,
|
&self,
|
||||||
cwd: Option<AbsolutePathBuf>,
|
cwd: Option<AbsolutePathBuf>,
|
||||||
) -> std::io::Result<ConfigLayerStack> {
|
) -> std::io::Result<ConfigLayerStack> {
|
||||||
let thread_config_loader = self.current_thread_config_loader();
|
|
||||||
load_config_layers_state(
|
load_config_layers_state(
|
||||||
LOCAL_FS.as_ref(),
|
LOCAL_FS.as_ref(),
|
||||||
&self.codex_home,
|
&self.codex_home,
|
||||||
@@ -286,7 +267,7 @@ impl ConfigManager {
|
|||||||
strict_config: self.strict_config,
|
strict_config: self.strict_config,
|
||||||
cloud_config_bundle: self.current_cloud_config_bundle(),
|
cloud_config_bundle: self.current_cloud_config_bundle(),
|
||||||
},
|
},
|
||||||
thread_config_loader.as_ref(),
|
self.thread_config_loader.as_ref(),
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -7,8 +7,6 @@ use codex_code_mode::GrpcCodeModeSessionProvider;
|
|||||||
use codex_code_mode::WebSocketCodeModeSessionProvider;
|
use codex_code_mode::WebSocketCodeModeSessionProvider;
|
||||||
use codex_config::LoaderOverrides;
|
use codex_config::LoaderOverrides;
|
||||||
use codex_config::NoopThreadConfigLoader;
|
use codex_config::NoopThreadConfigLoader;
|
||||||
use codex_config::RemoteThreadConfigLoader;
|
|
||||||
use codex_config::ThreadConfigLoader;
|
|
||||||
use codex_core::config::Config;
|
use codex_core::config::Config;
|
||||||
use codex_core::resolve_installation_id;
|
use codex_core::resolve_installation_id;
|
||||||
use codex_login::AuthManager;
|
use codex_login::AuthManager;
|
||||||
@@ -148,13 +146,6 @@ enum LogFormat {
|
|||||||
|
|
||||||
type StderrLogLayer = Box<dyn Layer<Registry> + Send + Sync + 'static>;
|
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.
|
/// Control-plane messages from the processor/transport side to the outbound router task.
|
||||||
///
|
///
|
||||||
/// `run_main_with_transport_options` uses two loops/tasks:
|
/// `run_main_with_transport_options` uses two loops/tasks:
|
||||||
@@ -503,9 +494,6 @@ pub async fn run_main_with_transport_options(
|
|||||||
.await
|
.await
|
||||||
{
|
{
|
||||||
Ok(config) => {
|
Ok(config) => {
|
||||||
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 =
|
let auth_manager =
|
||||||
AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false)
|
AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false)
|
||||||
.await
|
.await
|
||||||
|
|||||||
@@ -409,10 +409,6 @@ pub struct ConfigToml {
|
|||||||
/// active.
|
/// active.
|
||||||
pub experimental_realtime_start_instructions: Option<String>,
|
pub experimental_realtime_start_instructions: 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>,
|
|
||||||
|
|
||||||
/// Removed. Former remote thread-store endpoint setting kept only so we can
|
/// Removed. Former remote thread-store endpoint setting kept only so we can
|
||||||
/// fail fast instead of silently falling back to local persistence.
|
/// fail fast instead of silently falling back to local persistence.
|
||||||
#[schemars(skip)]
|
#[schemars(skip)]
|
||||||
|
|||||||
@@ -5196,10 +5196,6 @@
|
|||||||
"description": "Experimental / do not use. Replaces the synthesized realtime startup context appended to websocket session instructions. An empty string disables startup context injection entirely.",
|
"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"
|
"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": {
|
"experimental_thread_store": {
|
||||||
"allOf": [
|
"allOf": [
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -11921,35 +11921,6 @@ experimental_realtime_start_instructions = "start instructions from config"
|
|||||||
Ok(())
|
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]
|
#[tokio::test]
|
||||||
async fn experimental_realtime_ws_base_url_loads_from_config_toml() -> std::io::Result<()> {
|
async fn experimental_realtime_ws_base_url_loads_from_config_toml() -> std::io::Result<()> {
|
||||||
let cfg: ConfigToml = toml::from_str(
|
let cfg: ConfigToml = toml::from_str(
|
||||||
|
|||||||
@@ -954,10 +954,6 @@ pub struct Config {
|
|||||||
/// instructions inserted into developer messages when realtime becomes
|
/// instructions inserted into developer messages when realtime becomes
|
||||||
/// active.
|
/// active.
|
||||||
pub experimental_realtime_start_instructions: Option<String>,
|
pub experimental_realtime_start_instructions: 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>,
|
|
||||||
|
|
||||||
/// Experimental / do not use. Selects the thread persistence backend.
|
/// Experimental / do not use. Selects the thread persistence backend.
|
||||||
pub experimental_thread_store: ThreadStoreConfig,
|
pub experimental_thread_store: ThreadStoreConfig,
|
||||||
/// When set, restricts ChatGPT login to one or more workspace identifiers.
|
/// When set, restricts ChatGPT login to one or more workspace identifiers.
|
||||||
@@ -4126,7 +4122,6 @@ impl Config {
|
|||||||
experimental_realtime_ws_backend_prompt: cfg.experimental_realtime_ws_backend_prompt,
|
experimental_realtime_ws_backend_prompt: cfg.experimental_realtime_ws_backend_prompt,
|
||||||
experimental_realtime_ws_startup_context: cfg.experimental_realtime_ws_startup_context,
|
experimental_realtime_ws_startup_context: cfg.experimental_realtime_ws_startup_context,
|
||||||
experimental_realtime_start_instructions: cfg.experimental_realtime_start_instructions,
|
experimental_realtime_start_instructions: cfg.experimental_realtime_start_instructions,
|
||||||
experimental_thread_config_endpoint: cfg.experimental_thread_config_endpoint,
|
|
||||||
experimental_thread_store: thread_store_config(cfg.experimental_thread_store),
|
experimental_thread_store: thread_store_config(cfg.experimental_thread_store),
|
||||||
forced_chatgpt_workspace_id,
|
forced_chatgpt_workspace_id,
|
||||||
forced_login_method,
|
forced_login_method,
|
||||||
|
|||||||
@@ -287,7 +287,6 @@ fn new_config(model: Option<String>, arg0_paths: Arg0DispatchPaths) -> anyhow::R
|
|||||||
experimental_realtime_ws_backend_prompt: None,
|
experimental_realtime_ws_backend_prompt: None,
|
||||||
experimental_realtime_ws_startup_context: None,
|
experimental_realtime_ws_startup_context: None,
|
||||||
experimental_realtime_start_instructions: None,
|
experimental_realtime_start_instructions: None,
|
||||||
experimental_thread_config_endpoint: None,
|
|
||||||
experimental_thread_store: ThreadStoreConfig::Local,
|
experimental_thread_store: ThreadStoreConfig::Local,
|
||||||
forced_chatgpt_workspace_id: None,
|
forced_chatgpt_workspace_id: None,
|
||||||
forced_login_method: None,
|
forced_login_method: None,
|
||||||
|
|||||||
Reference in new issue
Block a user