From ab43f9df8adb3fd09344d6bc2e3cfaf544edb117 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Fri, 15 May 2026 15:34:30 -0700 Subject: [PATCH 1/5] tui: persist app and skill enablement via app server --- codex-rs/tui/src/app/config_persistence.rs | 1 + codex-rs/tui/src/app/event_dispatch.rs | 66 ++++++++++------------ 2 files changed, 30 insertions(+), 37 deletions(-) diff --git a/codex-rs/tui/src/app/config_persistence.rs b/codex-rs/tui/src/app/config_persistence.rs index 6f4fda608e47..5b2b5ab23a6b 100644 --- a/codex-rs/tui/src/app/config_persistence.rs +++ b/codex-rs/tui/src/app/config_persistence.rs @@ -589,6 +589,7 @@ mod tests { use super::*; use crate::app::test_support::app_enabled_in_effective_config; use crate::app::test_support::make_test_app; + use crate::legacy_core::config::edit::ConfigEdit; use crate::test_support::PathBufExt; use codex_protocol::models::PermissionProfile; use pretty_assertions::assert_eq; diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 9271089c4358..4ac15d6c2d76 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1662,18 +1662,18 @@ impl App { self.chat_widget.open_manage_skills_popup(); } AppEvent::SetSkillEnabled { path, enabled } => { - let edits = [ConfigEdit::SetSkillConfig { - path: path.to_path_buf(), + match crate::config_rpc::write_skill_enabled( + app_server.request_handle(), + path.clone(), enabled, - }]; - match ConfigEditsBuilder::for_config(&self.config) - .with_edits(edits) - .apply() - .await + ) + .await { - Ok(()) => { + Ok(_) => { self.chat_widget.update_skill_enabled(path, enabled); - if let Err(err) = self.refresh_in_memory_config_from_disk().await { + if !app_server.is_remote() + && let Err(err) = self.refresh_in_memory_config_from_disk().await + { tracing::warn!( error = %err, "failed to refresh config after skill toggle" @@ -1691,41 +1691,33 @@ impl App { AppEvent::SetAppEnabled { id, enabled } => { let edits = if enabled { vec![ - ConfigEdit::ClearPath { - segments: vec!["apps".to_string(), id.clone(), "enabled".to_string()], - }, - ConfigEdit::ClearPath { - segments: vec![ - "apps".to_string(), - id.clone(), - "disabled_reason".to_string(), - ], - }, + crate::config_rpc::clear_config_value(format!("apps.{id}.enabled")), + crate::config_rpc::clear_config_value(format!("apps.{id}.disabled_reason")), ] } else { vec![ - ConfigEdit::SetPath { - segments: vec!["apps".to_string(), id.clone(), "enabled".to_string()], - value: false.into(), - }, - ConfigEdit::SetPath { - segments: vec![ - "apps".to_string(), - id.clone(), - "disabled_reason".to_string(), - ], - value: "user".into(), - }, + crate::config_rpc::replace_config_value( + format!("apps.{id}.enabled"), + serde_json::json!(false), + ), + crate::config_rpc::replace_config_value( + format!("apps.{id}.disabled_reason"), + serde_json::json!("user"), + ), ] }; - match ConfigEditsBuilder::for_config(&self.config) - .with_edits(edits) - .apply() - .await + match crate::config_rpc::write_config_batch( + app_server.request_handle(), + edits, + /*reload_user_config*/ true, + ) + .await { - Ok(()) => { + Ok(_) => { self.chat_widget.update_connector_enabled(&id, enabled); - if let Err(err) = self.refresh_in_memory_config_from_disk().await { + if !app_server.is_remote() + && let Err(err) = self.refresh_in_memory_config_from_disk().await + { tracing::warn!(error = %err, "failed to refresh config after app toggle"); } self.chat_widget.submit_op(AppCommand::reload_user_config()); From fd994c7a16ff5d8fe8c39623f010c5b4c9471a87 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Fri, 15 May 2026 15:44:43 -0700 Subject: [PATCH 2/5] codex: fix CI failure on PR #22914 --- codex-rs/tui/src/app/event_dispatch.rs | 16 ++++++---------- codex-rs/tui/src/config_update.rs | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 4ac15d6c2d76..c4f379f46a0f 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1662,7 +1662,7 @@ impl App { self.chat_widget.open_manage_skills_popup(); } AppEvent::SetSkillEnabled { path, enabled } => { - match crate::config_rpc::write_skill_enabled( + match crate::config_update::write_skill_enabled( app_server.request_handle(), path.clone(), enabled, @@ -1691,26 +1691,22 @@ impl App { AppEvent::SetAppEnabled { id, enabled } => { let edits = if enabled { vec![ - crate::config_rpc::clear_config_value(format!("apps.{id}.enabled")), - crate::config_rpc::clear_config_value(format!("apps.{id}.disabled_reason")), + crate::config_update::clear_config_value(format!("apps.{id}.enabled")), + crate::config_update::clear_config_value(format!("apps.{id}.disabled_reason")), ] } else { vec![ - crate::config_rpc::replace_config_value( + crate::config_update::replace_config_value( format!("apps.{id}.enabled"), serde_json::json!(false), ), - crate::config_rpc::replace_config_value( + crate::config_update::replace_config_value( format!("apps.{id}.disabled_reason"), serde_json::json!("user"), ), ] }; - match crate::config_rpc::write_config_batch( - app_server.request_handle(), - edits, - /*reload_user_config*/ true, - ) + match crate::config_update::write_config_batch(app_server.request_handle(), edits) .await { Ok(_) => { diff --git a/codex-rs/tui/src/config_update.rs b/codex-rs/tui/src/config_update.rs index bcdbe29a0d54..617647885591 100644 --- a/codex-rs/tui/src/config_update.rs +++ b/codex-rs/tui/src/config_update.rs @@ -11,6 +11,9 @@ use codex_app_server_protocol::ConfigEdit; use codex_app_server_protocol::ConfigWriteResponse; use codex_app_server_protocol::MergeStrategy; use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::SkillsConfigWriteParams; +use codex_app_server_protocol::SkillsConfigWriteResponse; +use codex_utils_absolute_path::AbsolutePathBuf; use color_eyre::eyre::Result; use color_eyre::eyre::WrapErr; use serde_json::Value as JsonValue; @@ -109,6 +112,25 @@ pub(crate) async fn write_config_batch( Ok(()) } +pub(crate) async fn write_skill_enabled( + request_handle: AppServerRequestHandle, + path: AbsolutePathBuf, + enabled: bool, +) -> Result { + let request_id = RequestId::String(format!("tui-skill-config-write-{}", Uuid::new_v4())); + request_handle + .request_typed(ClientRequest::SkillsConfigWrite { + request_id, + params: SkillsConfigWriteParams { + path: Some(path), + name: None, + enabled, + }, + }) + .await + .wrap_err("skills/config/write failed in TUI") +} + #[cfg(test)] mod tests { use super::*; From f5004e8a0dc7292d39603ea99496500e8d60b67d Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 16 May 2026 14:44:15 -0700 Subject: [PATCH 3/5] codex: address PR review feedback (#22914) --- codex-rs/tui/src/app/event_dispatch.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index c4f379f46a0f..d78cfa8b538a 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1692,7 +1692,9 @@ impl App { let edits = if enabled { vec![ crate::config_update::clear_config_value(format!("apps.{id}.enabled")), - crate::config_update::clear_config_value(format!("apps.{id}.disabled_reason")), + crate::config_update::clear_config_value(format!( + "apps.{id}.disabled_reason" + )), ] } else { vec![ @@ -1707,7 +1709,7 @@ impl App { ] }; match crate::config_update::write_config_batch(app_server.request_handle(), edits) - .await + .await { Ok(_) => { self.chat_widget.update_connector_enabled(&id, enabled); @@ -1716,7 +1718,6 @@ impl App { { tracing::warn!(error = %err, "failed to refresh config after app toggle"); } - self.chat_widget.submit_op(AppCommand::reload_user_config()); } Err(err) => { self.chat_widget.add_error_message(format!( From 00c31b8f54d8e69a3f8ef9e6c8b9fe14f4fec2c4 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 18 May 2026 20:10:56 -0700 Subject: [PATCH 4/5] tui: load app catalog state through app server --- codex-rs/tui/src/app.rs | 4 - codex-rs/tui/src/app/background_requests.rs | 43 ++++++++++ codex-rs/tui/src/app/event_dispatch.rs | 3 + codex-rs/tui/src/app/tests.rs | 2 - codex-rs/tui/src/app_event.rs | 5 ++ codex-rs/tui/src/chatwidget.rs | 3 - codex-rs/tui/src/chatwidget/connectors.rs | 82 ++++--------------- codex-rs/tui/src/chatwidget/constructor.rs | 2 - codex-rs/tui/src/chatwidget/tests/helpers.rs | 1 - .../tui/src/chatwidget/tests/plan_mode.rs | 1 - .../chatwidget/tests/popups_and_settings.rs | 1 - .../src/chatwidget/tests/status_and_layout.rs | 1 - 12 files changed, 66 insertions(+), 82 deletions(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 46a7be00ddf9..5cd5a2c474ab 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -614,7 +614,6 @@ impl App { ) -> crate::chatwidget::ChatWidgetInit { crate::chatwidget::ChatWidgetInit { config: cfg, - environment_manager: self.environment_manager.clone(), frame_requester: tui.frame_requester(), app_event_tx: self.app_event_tx.clone(), workspace_command_runner: self.workspace_command_runner.clone(), @@ -781,7 +780,6 @@ impl App { .await; let init = crate::chatwidget::ChatWidgetInit { config: config.clone(), - environment_manager: environment_manager.clone(), frame_requester: tui.frame_requester(), app_event_tx: app_event_tx.clone(), workspace_command_runner: Some(workspace_command_runner.clone()), @@ -818,7 +816,6 @@ impl App { })?; let init = crate::chatwidget::ChatWidgetInit { config: config.clone(), - environment_manager: environment_manager.clone(), frame_requester: tui.frame_requester(), app_event_tx: app_event_tx.clone(), workspace_command_runner: Some(workspace_command_runner.clone()), @@ -860,7 +857,6 @@ impl App { })?; let init = crate::chatwidget::ChatWidgetInit { config: config.clone(), - environment_manager: environment_manager.clone(), frame_requester: tui.frame_requester(), app_event_tx: app_event_tx.clone(), workspace_command_runner: Some(workspace_command_runner.clone()), diff --git a/codex-rs/tui/src/app/background_requests.rs b/codex-rs/tui/src/app/background_requests.rs index c323a2d5bf5b..2a7dbf338850 100644 --- a/codex-rs/tui/src/app/background_requests.rs +++ b/codex-rs/tui/src/app/background_requests.rs @@ -6,6 +6,9 @@ use super::plugin_mentions::fetch_plugin_mentions; use super::*; +use crate::app_event::ConnectorsSnapshot; +use codex_app_server_protocol::AppsListParams; +use codex_app_server_protocol::AppsListResponse; use codex_app_server_protocol::MarketplaceAddParams; use codex_app_server_protocol::MarketplaceAddResponse; use codex_app_server_protocol::MarketplaceRemoveParams; @@ -92,6 +95,24 @@ impl App { }); } + pub(super) fn fetch_connectors_list( + &mut self, + app_server: &AppServerSession, + force_refetch: bool, + ) { + let request_handle = app_server.request_handle(); + let app_event_tx = self.app_event_tx.clone(); + tokio::spawn(async move { + let result = fetch_connectors_list(request_handle, force_refetch) + .await + .map_err(|err| err.to_string()); + app_event_tx.send(AppEvent::ConnectorsLoaded { + result, + is_final: true, + }); + }); + } + pub(super) fn fetch_plugins_list(&mut self, app_server: &AppServerSession, cwd: PathBuf) { let request_handle = app_server.request_handle(); let app_event_tx = self.app_event_tx.clone(); @@ -634,6 +655,28 @@ pub(super) async fn fetch_skills_list( .wrap_err("skills/list failed in TUI") } +pub(super) async fn fetch_connectors_list( + request_handle: AppServerRequestHandle, + force_refetch: bool, +) -> Result { + let request_id = RequestId::String(format!("apps-list-{}", Uuid::new_v4())); + let response: AppsListResponse = request_handle + .request_typed(ClientRequest::AppsList { + request_id, + params: AppsListParams { + cursor: None, + limit: None, + thread_id: None, + force_refetch, + }, + }) + .await + .wrap_err("app/list failed in TUI")?; + Ok(ConnectorsSnapshot { + connectors: response.data, + }) +} + pub(super) async fn fetch_plugins_list( request_handle: AppServerRequestHandle, cwd: PathBuf, diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index d78cfa8b538a..5f5b6d3a4651 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -416,6 +416,9 @@ impl App { AppEvent::RefreshConnectors { force_refetch } => { self.chat_widget.refresh_connectors(force_refetch); } + AppEvent::FetchConnectorsList { force_refetch } => { + self.fetch_connectors_list(app_server, force_refetch); + } AppEvent::PluginInstallAuthAdvance { refresh_connectors } => { if refresh_connectors { self.chat_widget.refresh_connectors(/*force_refetch*/ true); diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index c42f42c257e6..547f70890c08 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -265,7 +265,6 @@ async fn enqueue_primary_thread_session_replays_turns_before_initial_prompt_subm let model = crate::legacy_core::test_support::get_model_offline(config.model.as_deref()); app.chat_widget = ChatWidget::new_with_app_event(ChatWidgetInit { config, - environment_manager: app.environment_manager.clone(), frame_requester: crate::tui::FrameRequester::test_dummy(), app_event_tx: app.app_event_tx.clone(), workspace_command_runner: None, @@ -4842,7 +4841,6 @@ async fn replace_chat_widget_reseeds_collab_agent_metadata_for_replay() { let replacement = ChatWidget::new_with_app_event(ChatWidgetInit { config: app.config.clone(), - environment_manager: app.environment_manager.clone(), frame_requester: crate::tui::FrameRequester::test_dummy(), app_event_tx: app.app_event_tx.clone(), workspace_command_runner: None, diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 57ec371749d6..07a5b5117536 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -355,6 +355,11 @@ pub(crate) enum AppEvent { force_refetch: bool, }, + /// Fetch app connector state from the app server after the widget accepts a refresh request. + FetchConnectorsList { + force_refetch: bool, + }, + /// Fetch plugin marketplace state for the provided working directory. FetchPluginsList { cwd: PathBuf, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index a37371bf73ef..a9c5dcecfd6d 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -128,7 +128,6 @@ use codex_config::types::ApprovalsReviewer; use codex_config::types::Notifications; use codex_config::types::WindowsSandboxModeToml; use codex_core_skills::model::SkillMetadata; -use codex_exec_server::EnvironmentManager; use codex_features::FEATURES; use codex_features::Feature; #[cfg(test)] @@ -464,7 +463,6 @@ const MAX_AGENT_COPY_HISTORY: usize = 32; /// Common initialization parameters shared by all `ChatWidget` constructors. pub(crate) struct ChatWidgetInit { pub(crate) config: Config, - pub(crate) environment_manager: Arc, pub(crate) frame_requester: FrameRequester, pub(crate) app_event_tx: AppEventSender, /// App-server-backed runner used by status surfaces for workspace metadata probes. @@ -516,7 +514,6 @@ pub(crate) struct ChatWidget { bottom_pane: BottomPane, transcript: TranscriptState, config: Config, - environment_manager: Arc, raw_output_mode: bool, /// Runtime value resolved by core. `config.service_tier` remains the explicit user choice. effective_service_tier: Option, diff --git a/codex-rs/tui/src/chatwidget/connectors.rs b/codex-rs/tui/src/chatwidget/connectors.rs index 942f2e9da8c4..2df64df7d72c 100644 --- a/codex-rs/tui/src/chatwidget/connectors.rs +++ b/codex-rs/tui/src/chatwidget/connectors.rs @@ -22,86 +22,36 @@ pub(super) struct ConnectorsState { impl ChatWidget { pub(crate) fn refresh_connectors(&mut self, force_refetch: bool) { - self.prefetch_connectors_with_options(force_refetch); + self.queue_connectors_refresh(force_refetch); } pub(super) fn prefetch_connectors(&mut self) { - self.prefetch_connectors_with_options(/*force_refetch*/ false); + self.queue_connectors_refresh(/*force_refetch*/ false); } - fn prefetch_connectors_with_options(&mut self, force_refetch: bool) { + fn queue_connectors_refresh(&mut self, force_refetch: bool) { + if self.begin_connectors_refresh(force_refetch) { + self.app_event_tx + .send(AppEvent::FetchConnectorsList { force_refetch }); + } + } + + fn begin_connectors_refresh(&mut self, force_refetch: bool) -> bool { if !self.connectors_enabled() { - return; + return false; } if self.connectors.prefetch_in_flight { if force_refetch { self.connectors.force_refetch_pending = true; } - return; + return false; } self.connectors.prefetch_in_flight = true; if !matches!(self.connectors.cache, ConnectorsCacheState::Ready(_)) { self.connectors.cache = ConnectorsCacheState::Loading; } - - let config = self.config.clone(); - let environment_manager = Arc::clone(&self.environment_manager); - let app_event_tx = self.app_event_tx.clone(); - tokio::spawn(async move { - let accessible_result = - match chatgpt_connectors::list_accessible_connectors_from_mcp_tools_with_environment_manager( - &config, - force_refetch, - &environment_manager, - ) - .await - { - Ok(connectors) => connectors, - Err(err) => { - app_event_tx.send(AppEvent::ConnectorsLoaded { - result: Err(format!("Failed to load apps: {err}")), - is_final: true, - }); - return; - } - }; - let should_schedule_force_refetch = - !force_refetch && !accessible_result.codex_apps_ready; - let accessible_connectors = accessible_result.connectors; - - app_event_tx.send(AppEvent::ConnectorsLoaded { - result: Ok(ConnectorsSnapshot { - connectors: accessible_connectors.clone(), - }), - is_final: false, - }); - - let result: Result = async { - let all_connectors = - chatgpt_connectors::list_all_connectors_with_options(&config, force_refetch) - .await?; - let connectors = chatgpt_connectors::merge_connectors_with_accessible( - all_connectors, - accessible_connectors, - /*all_connectors_loaded*/ true, - ); - Ok(ConnectorsSnapshot { connectors }) - } - .await - .map_err(|err: anyhow::Error| format!("Failed to load apps: {err}")); - - app_event_tx.send(AppEvent::ConnectorsLoaded { - result, - is_final: true, - }); - - if should_schedule_force_refetch { - app_event_tx.send(AppEvent::RefreshConnectors { - force_refetch: true, - }); - } - }); + true } pub(super) fn connectors_enabled(&self) -> bool { @@ -135,7 +85,7 @@ impl ChatWidget { let connectors_cache = self.connectors.cache.clone(); let should_force_refetch = !self.connectors.prefetch_in_flight || matches!(connectors_cache, ConnectorsCacheState::Ready(_)); - self.prefetch_connectors_with_options(should_force_refetch); + self.queue_connectors_refresh(should_force_refetch); match connectors_cache { ConnectorsCacheState::Ready(snapshot) => { @@ -360,8 +310,6 @@ impl ChatWidget { /*all_connectors_loaded*/ false, ); } - snapshot.connectors = - chatgpt_connectors::with_app_enabled_state(snapshot.connectors, &self.config); if let ConnectorsCacheState::Ready(existing_snapshot) = &self.connectors.cache { let enabled_by_id: HashMap<&str, bool> = existing_snapshot .connectors @@ -404,7 +352,7 @@ impl ChatWidget { } if trigger_pending_force_refetch { - self.prefetch_connectors_with_options(/*force_refetch*/ true); + self.queue_connectors_refresh(/*force_refetch*/ true); } } diff --git a/codex-rs/tui/src/chatwidget/constructor.rs b/codex-rs/tui/src/chatwidget/constructor.rs index db55be3a885c..56bdc6bf6898 100644 --- a/codex-rs/tui/src/chatwidget/constructor.rs +++ b/codex-rs/tui/src/chatwidget/constructor.rs @@ -13,7 +13,6 @@ impl ChatWidget { ) -> Self { let ChatWidgetInit { config, - environment_manager, frame_requester, app_event_tx, workspace_command_runner, @@ -104,7 +103,6 @@ impl ChatWidget { transcript: TranscriptState::new(active_cell), raw_output_mode: config.tui_raw_output_mode, config, - environment_manager, effective_service_tier, skills_all: Vec::new(), skills_initial_state: None, diff --git a/codex-rs/tui/src/chatwidget/tests/helpers.rs b/codex-rs/tui/src/chatwidget/tests/helpers.rs index 28ed69139b9f..f3abe693aad6 100644 --- a/codex-rs/tui/src/chatwidget/tests/helpers.rs +++ b/codex-rs/tui/src/chatwidget/tests/helpers.rs @@ -161,7 +161,6 @@ pub(super) async fn make_chatwidget_manual( let model_catalog = test_model_catalog(&cfg); let common = ChatWidgetInit { config: cfg, - environment_manager: Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), frame_requester: FrameRequester::test_dummy(), app_event_tx, workspace_command_runner: None, diff --git a/codex-rs/tui/src/chatwidget/tests/plan_mode.rs b/codex-rs/tui/src/chatwidget/tests/plan_mode.rs index b97695e800a2..091f92c3fddf 100644 --- a/codex-rs/tui/src/chatwidget/tests/plan_mode.rs +++ b/codex-rs/tui/src/chatwidget/tests/plan_mode.rs @@ -1481,7 +1481,6 @@ async fn make_startup_chat_with_cli_overrides( let session_telemetry = test_session_telemetry(&cfg, resolved_model.as_str()); let init = ChatWidgetInit { config: cfg.clone(), - environment_manager: Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), frame_requester: FrameRequester::test_dummy(), app_event_tx: AppEventSender::new(unbounded_channel::().0), workspace_command_runner: None, diff --git a/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs b/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs index 379ff7c6de7d..f1205bdc405f 100644 --- a/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs +++ b/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs @@ -72,7 +72,6 @@ async fn experimental_mode_plan_is_ignored_on_startup() { let session_telemetry = test_session_telemetry(&cfg, resolved_model.as_str()); let init = ChatWidgetInit { config: cfg.clone(), - environment_manager: Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), frame_requester: FrameRequester::test_dummy(), app_event_tx: AppEventSender::new(unbounded_channel::().0), workspace_command_runner: None, diff --git a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs index acd6b7111bc2..83c0dd6f90f0 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -413,7 +413,6 @@ async fn configured_pet_load_is_deferred_until_after_construction() { let session_telemetry = test_session_telemetry(&cfg, resolved_model.as_str()); let init = ChatWidgetInit { config: cfg.clone(), - environment_manager: Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), frame_requester: FrameRequester::test_dummy(), app_event_tx: tx, workspace_command_runner: None, From 4edf7c5535974eb69b85578982afe29c0cf70796 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 18 May 2026 21:53:23 -0700 Subject: [PATCH 5/5] codex: fix PR2 app catalog state handling --- codex-rs/app-server/src/request_processors.rs | 1 + .../src/request_processors/apps_processor.rs | 55 ++++- codex-rs/tui/src/app/app_server_events.rs | 10 + codex-rs/tui/src/app/background_requests.rs | 8 +- codex-rs/tui/src/app/event_dispatch.rs | 16 +- codex-rs/tui/src/chatwidget/tests.rs | 5 - .../chatwidget/tests/popups_and_settings.rs | 192 ------------------ codex-rs/tui/src/config_update.rs | 20 +- 8 files changed, 87 insertions(+), 220 deletions(-) diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index d5853b56c557..8c2668862e19 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -274,6 +274,7 @@ use codex_core::config::ConfigOverrides; use codex_core::config::NetworkProxyAuditMetadata; use codex_core::config::edit::ConfigEdit; use codex_core::config::edit::ConfigEditsBuilder; +use codex_core::connectors::AccessibleConnectorsStatus; use codex_core::exec::ExecCapturePolicy; use codex_core::exec::ExecExpiration; use codex_core::exec::ExecParams; diff --git a/codex-rs/app-server/src/request_processors/apps_processor.rs b/codex-rs/app-server/src/request_processors/apps_processor.rs index da2956dbab69..caf60a91a014 100644 --- a/codex-rs/app-server/src/request_processors/apps_processor.rs +++ b/codex-rs/app-server/src/request_processors/apps_processor.rs @@ -41,11 +41,19 @@ impl AppsRequestProcessor { request_id: &ConnectionRequestId, params: AppsListParams, ) -> Result, JSONRPCErrorError> { - let mut config = self.load_latest_config(/*fallback_cwd*/ None).await?; - - if let Some(thread_id) = params.thread_id.as_deref() { - let (_, thread) = self.load_thread(thread_id).await?; + let thread = if let Some(thread_id) = params.thread_id.as_deref() { + let (_, loaded_thread) = self.load_thread(thread_id).await?; + Some(loaded_thread) + } else { + None + }; + let fallback_cwd = match thread.as_ref() { + Some(thread) => Some(thread.config_snapshot().await.cwd.to_path_buf()), + None => None, + }; + let mut config = self.load_latest_config(fallback_cwd).await?; + if let Some(thread) = thread { let _ = config .features .set_enabled(Feature::Apps, thread.enabled(Feature::Apps)); @@ -88,8 +96,31 @@ impl AppsRequestProcessor { config: Config, environment_manager: Arc, ) { + let retry_params = params.clone(); + let retry_config = config.clone(); + let retry_environment_manager = Arc::clone(&environment_manager); let result = Self::apps_list_response(&outgoing, params, config, environment_manager).await; - outgoing.send_result(request_id, result).await; + let should_retry = result + .as_ref() + .is_ok_and(|(_, codex_apps_ready)| !codex_apps_ready); + outgoing + .send_result(request_id, result.map(|(response, _)| response)) + .await; + + if should_retry && !retry_params.force_refetch { + let mut retry_params = retry_params; + retry_params.force_refetch = true; + if let Err(err) = Self::apps_list_response( + &outgoing, + retry_params, + retry_config, + retry_environment_manager, + ) + .await + { + warn!("failed to refresh app list after codex-apps readiness retry: {err:?}"); + } + } } async fn apps_list_response( @@ -97,7 +128,7 @@ impl AppsRequestProcessor { params: AppsListParams, config: Config, environment_manager: Arc, - ) -> Result { + ) -> Result<(AppsListResponse, bool), JSONRPCErrorError> { let AppsListParams { cursor, limit, @@ -130,7 +161,6 @@ impl AppsRequestProcessor { &environment_manager, ) .await - .map(|status| status.connectors) .map_err(|err| format!("failed to load accessible apps: {err}")); let _ = accessible_tx.send(AppListLoadResult::Accessible(result)); }); @@ -146,6 +176,7 @@ impl AppsRequestProcessor { let app_list_deadline = tokio::time::Instant::now() + APP_LIST_LOAD_TIMEOUT; let mut accessible_loaded = false; let mut all_loaded = false; + let mut codex_apps_ready = true; let mut last_notified_apps = None; if accessible_connectors.is_some() || all_connectors.is_some() { @@ -178,9 +209,10 @@ impl AppsRequestProcessor { }; match result { - AppListLoadResult::Accessible(Ok(connectors)) => { - accessible_connectors = Some(connectors); + AppListLoadResult::Accessible(Ok(status)) => { + accessible_connectors = Some(status.connectors); accessible_loaded = true; + codex_apps_ready = status.codex_apps_ready; } AppListLoadResult::Accessible(Err(err)) => { return Err(internal_error(err)); @@ -222,7 +254,8 @@ impl AppsRequestProcessor { } if accessible_loaded && all_loaded { - return paginate_apps(merged.as_slice(), start, limit); + let response = paginate_apps(merged.as_slice(), start, limit)?; + return Ok((response, codex_apps_ready)); } } } @@ -279,7 +312,7 @@ impl AppsRequestProcessor { const APP_LIST_LOAD_TIMEOUT: Duration = Duration::from_secs(90); enum AppListLoadResult { - Accessible(Result, String>), + Accessible(Result), Directory(Result, String>), } diff --git a/codex-rs/tui/src/app/app_server_events.rs b/codex-rs/tui/src/app/app_server_events.rs index 413e8b8c8070..18e6116e588e 100644 --- a/codex-rs/tui/src/app/app_server_events.rs +++ b/codex-rs/tui/src/app/app_server_events.rs @@ -6,6 +6,7 @@ use super::app_server_event_targets::server_notification_thread_target; use super::app_server_event_targets::server_request_thread_id; use crate::app_command::AppCommand; use crate::app_event::AppEvent; +use crate::app_event::ConnectorsSnapshot; use crate::app_server_session::AppServerSession; use crate::app_server_session::status_account_display_from_auth_mode; use codex_app_server_client::AppServerEvent; @@ -106,6 +107,15 @@ impl App { self.fetch_plugins_list(app_server_client, cwd); return; } + ServerNotification::AppListUpdated(notification) => { + self.chat_widget.on_connectors_loaded( + Ok(ConnectorsSnapshot { + connectors: notification.data.clone(), + }), + /*is_final*/ false, + ); + return; + } _ => {} } diff --git a/codex-rs/tui/src/app/background_requests.rs b/codex-rs/tui/src/app/background_requests.rs index 2a7dbf338850..dcf1f672bee5 100644 --- a/codex-rs/tui/src/app/background_requests.rs +++ b/codex-rs/tui/src/app/background_requests.rs @@ -102,8 +102,11 @@ impl App { ) { let request_handle = app_server.request_handle(); let app_event_tx = self.app_event_tx.clone(); + let thread_id = self + .current_displayed_thread_id() + .map(|thread_id| thread_id.to_string()); tokio::spawn(async move { - let result = fetch_connectors_list(request_handle, force_refetch) + let result = fetch_connectors_list(request_handle, force_refetch, thread_id) .await .map_err(|err| err.to_string()); app_event_tx.send(AppEvent::ConnectorsLoaded { @@ -658,6 +661,7 @@ pub(super) async fn fetch_skills_list( pub(super) async fn fetch_connectors_list( request_handle: AppServerRequestHandle, force_refetch: bool, + thread_id: Option, ) -> Result { let request_id = RequestId::String(format!("apps-list-{}", Uuid::new_v4())); let response: AppsListResponse = request_handle @@ -666,7 +670,7 @@ pub(super) async fn fetch_connectors_list( params: AppsListParams { cursor: None, limit: None, - thread_id: None, + thread_id, force_refetch, }, }) diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 5f5b6d3a4651..f0418e66357f 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1672,7 +1672,7 @@ impl App { ) .await { - Ok(_) => { + Ok(()) => { self.chat_widget.update_skill_enabled(path, enabled); if !app_server.is_remote() && let Err(err) = self.refresh_in_memory_config_from_disk().await @@ -1694,19 +1694,21 @@ impl App { AppEvent::SetAppEnabled { id, enabled } => { let edits = if enabled { vec![ - crate::config_update::clear_config_value(format!("apps.{id}.enabled")), - crate::config_update::clear_config_value(format!( - "apps.{id}.disabled_reason" - )), + crate::config_update::clear_config_value( + crate::config_update::app_scoped_key_path(&id, "enabled"), + ), + crate::config_update::clear_config_value( + crate::config_update::app_scoped_key_path(&id, "disabled_reason"), + ), ] } else { vec![ crate::config_update::replace_config_value( - format!("apps.{id}.enabled"), + crate::config_update::app_scoped_key_path(&id, "enabled"), serde_json::json!(false), ), crate::config_update::replace_config_value( - format!("apps.{id}.disabled_reason"), + crate::config_update::app_scoped_key_path(&id, "disabled_reason"), serde_json::json!("user"), ), ] diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index e0dda9eb5cc1..77aa742c0161 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -119,11 +119,7 @@ pub(super) use codex_app_server_protocol::TurnStatus as AppServerTurnStatus; pub(super) use codex_app_server_protocol::UserInput; pub(super) use codex_app_server_protocol::UserInput as AppServerUserInput; pub(super) use codex_app_server_protocol::WarningNotification; -pub(super) use codex_config::AppRequirementToml; -pub(super) use codex_config::AppsRequirementsToml; pub(super) use codex_config::ConfigLayerStack; -pub(super) use codex_config::ConfigRequirements; -pub(super) use codex_config::ConfigRequirementsToml; pub(super) use codex_config::RequirementSource; pub(super) use codex_config::types::ApprovalsReviewer; pub(super) use codex_config::types::Notifications; @@ -178,7 +174,6 @@ pub(super) use insta::assert_snapshot; pub(super) use serde_json::json; #[cfg(target_os = "windows")] pub(super) use serial_test::serial; -pub(super) use std::collections::BTreeMap; pub(super) use std::collections::HashMap; pub(super) use std::path::PathBuf; pub(super) use tempfile::NamedTempFile; diff --git a/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs b/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs index f1205bdc405f..2a25415c032b 100644 --- a/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs +++ b/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs @@ -1858,198 +1858,6 @@ async fn apps_popup_shows_disabled_status_for_installed_but_disabled_apps() { ); } -#[tokio::test] -async fn apps_initial_load_applies_enabled_state_from_config() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; - set_chatgpt_auth(&mut chat); - chat.config - .features - .enable(Feature::Apps) - .expect("test config should allow feature update"); - chat.bottom_pane.set_connectors_enabled(/*enabled*/ true); - - let temp = tempdir().expect("tempdir"); - let config_toml_path = temp.path().join("config.toml").abs(); - let user_config = toml::from_str::( - "[apps.connector_1]\nenabled = false\ndisabled_reason = \"user\"\n", - ) - .expect("apps config"); - chat.config.config_layer_stack = chat - .config - .config_layer_stack - .with_user_config(&config_toml_path, user_config); - - chat.on_connectors_loaded( - Ok(ConnectorsSnapshot { - connectors: vec![AppInfo { - id: "connector_1".to_string(), - name: "Notion".to_string(), - description: Some("Workspace docs".to_string()), - logo_url: None, - logo_url_dark: None, - distribution_channel: None, - branding: None, - app_metadata: None, - labels: None, - install_url: Some("https://example.test/notion".to_string()), - is_accessible: true, - is_enabled: true, - plugin_display_names: Vec::new(), - }], - }), - /*is_final*/ true, - ); - - assert_matches!( - &chat.connectors.cache, - ConnectorsCacheState::Ready(snapshot) - if snapshot - .connectors - .iter() - .find(|connector| connector.id == "connector_1") - .is_some_and(|connector| !connector.is_enabled) - ); -} - -#[tokio::test] -async fn apps_initial_load_applies_enabled_state_from_requirements_with_user_override() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; - set_chatgpt_auth(&mut chat); - chat.config - .features - .enable(Feature::Apps) - .expect("test config should allow feature update"); - chat.bottom_pane.set_connectors_enabled(/*enabled*/ true); - - let requirements = ConfigRequirementsToml { - apps: Some(AppsRequirementsToml { - apps: BTreeMap::from([( - "connector_1".to_string(), - AppRequirementToml { - enabled: Some(false), - tools: None, - }, - )]), - }), - ..Default::default() - }; - let temp = tempdir().expect("tempdir"); - let config_toml_path = temp.path().join("config.toml").abs(); - chat.config.config_layer_stack = - ConfigLayerStack::new(Vec::new(), ConfigRequirements::default(), requirements) - .expect("requirements stack") - .with_user_config( - &config_toml_path, - toml::from_str::( - "[apps.connector_1]\nenabled = true\ndisabled_reason = \"user\"\n", - ) - .expect("apps config"), - ); - - chat.on_connectors_loaded( - Ok(ConnectorsSnapshot { - connectors: vec![AppInfo { - id: "connector_1".to_string(), - name: "Notion".to_string(), - description: Some("Workspace docs".to_string()), - logo_url: None, - logo_url_dark: None, - distribution_channel: None, - branding: None, - app_metadata: None, - labels: None, - install_url: Some("https://example.test/notion".to_string()), - is_accessible: true, - is_enabled: true, - plugin_display_names: Vec::new(), - }], - }), - /*is_final*/ true, - ); - - assert_matches!( - &chat.connectors.cache, - ConnectorsCacheState::Ready(snapshot) - if snapshot - .connectors - .iter() - .find(|connector| connector.id == "connector_1") - .is_some_and(|connector| !connector.is_enabled) - ); - - chat.add_connectors_output(); - let popup = render_bottom_popup(&chat, /*width*/ 80); - assert!( - popup.contains("Installed · Disabled. Press Enter to open the app page"), - "expected requirements-disabled connector to render as disabled, got:\n{popup}" - ); -} - -#[tokio::test] -async fn apps_initial_load_applies_enabled_state_from_requirements_without_user_entry() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; - set_chatgpt_auth(&mut chat); - chat.config - .features - .enable(Feature::Apps) - .expect("test config should allow feature update"); - chat.bottom_pane.set_connectors_enabled(/*enabled*/ true); - - let requirements = ConfigRequirementsToml { - apps: Some(AppsRequirementsToml { - apps: BTreeMap::from([( - "connector_1".to_string(), - AppRequirementToml { - enabled: Some(false), - tools: None, - }, - )]), - }), - ..Default::default() - }; - chat.config.config_layer_stack = - ConfigLayerStack::new(Vec::new(), ConfigRequirements::default(), requirements) - .expect("requirements stack"); - - chat.on_connectors_loaded( - Ok(ConnectorsSnapshot { - connectors: vec![AppInfo { - id: "connector_1".to_string(), - name: "Notion".to_string(), - description: Some("Workspace docs".to_string()), - logo_url: None, - logo_url_dark: None, - distribution_channel: None, - branding: None, - app_metadata: None, - labels: None, - install_url: Some("https://example.test/notion".to_string()), - is_accessible: true, - is_enabled: true, - plugin_display_names: Vec::new(), - }], - }), - /*is_final*/ true, - ); - - assert_matches!( - &chat.connectors.cache, - ConnectorsCacheState::Ready(snapshot) - if snapshot - .connectors - .iter() - .find(|connector| connector.id == "connector_1") - .is_some_and(|connector| !connector.is_enabled) - ); - - chat.add_connectors_output(); - let popup = render_bottom_popup(&chat, /*width*/ 80); - assert!( - popup.contains("Installed · Disabled. Press Enter to open the app page"), - "expected requirements-disabled connector to render as disabled, got:\n{popup}" - ); -} - #[tokio::test] async fn apps_refresh_preserves_toggled_enabled_state() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; diff --git a/codex-rs/tui/src/config_update.rs b/codex-rs/tui/src/config_update.rs index 617647885591..cac79f648e3a 100644 --- a/codex-rs/tui/src/config_update.rs +++ b/codex-rs/tui/src/config_update.rs @@ -40,6 +40,11 @@ pub(crate) fn profile_scoped_key_path(profile: Option<&str>, key_path: &str) -> } } +pub(crate) fn app_scoped_key_path(app_id: &str, key_path: &str) -> String { + let app_id = serde_json::Value::String(app_id.to_string()).to_string(); + format!("apps.{app_id}.{key_path}") +} + pub(crate) fn build_model_selection_edits( profile: Option<&str>, model: &str, @@ -116,9 +121,9 @@ pub(crate) async fn write_skill_enabled( request_handle: AppServerRequestHandle, path: AbsolutePathBuf, enabled: bool, -) -> Result { +) -> Result<()> { let request_id = RequestId::String(format!("tui-skill-config-write-{}", Uuid::new_v4())); - request_handle + let _: SkillsConfigWriteResponse = request_handle .request_typed(ClientRequest::SkillsConfigWrite { request_id, params: SkillsConfigWriteParams { @@ -128,7 +133,8 @@ pub(crate) async fn write_skill_enabled( }, }) .await - .wrap_err("skills/config/write failed in TUI") + .wrap_err("skills/config/write failed in TUI")?; + Ok(()) } #[cfg(test)] @@ -143,4 +149,12 @@ mod tests { "profiles.\"team.prod\".model" ); } + + #[test] + fn app_scoped_key_path_quotes_dotted_app_ids() { + assert_eq!( + app_scoped_key_path("plugin.linear", "enabled"), + "apps.\"plugin.linear\".enabled" + ); + } }