mirror of
https://github.com/openai/codex.git
synced 2026-10-27 18:48:37 +00:00
Fix TUI remote config reads
This commit is contained in:
@@ -95,12 +95,11 @@ impl App {
|
||||
}
|
||||
ServerNotification::ExternalAgentConfigImportCompleted(_) => {
|
||||
let cwd = self.chat_widget.config_ref().cwd.to_path_buf();
|
||||
if let Err(err) = self.refresh_in_memory_config_from_disk().await {
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
"failed to refresh config after external agent config import"
|
||||
);
|
||||
}
|
||||
self.refresh_config_layer_stack_from_app_server_best_effort(
|
||||
app_server_client,
|
||||
"external agent config import",
|
||||
)
|
||||
.await;
|
||||
self.chat_widget.refresh_plugin_mentions();
|
||||
self.chat_widget.submit_op(AppCommand::reload_user_config());
|
||||
self.fetch_plugins_list(app_server_client, cwd);
|
||||
|
||||
@@ -6,6 +6,8 @@
|
||||
|
||||
use super::plugin_mentions::fetch_plugin_mentions;
|
||||
use super::*;
|
||||
use codex_app_server_protocol::ConfigReadParams;
|
||||
use codex_app_server_protocol::ConfigReadResponse;
|
||||
use codex_app_server_protocol::MarketplaceAddParams;
|
||||
use codex_app_server_protocol::MarketplaceAddResponse;
|
||||
use codex_app_server_protocol::MarketplaceRemoveParams;
|
||||
@@ -14,7 +16,9 @@ use codex_app_server_protocol::MarketplaceUpgradeParams;
|
||||
use codex_app_server_protocol::MarketplaceUpgradeResponse;
|
||||
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_config::types::McpServerConfig;
|
||||
|
||||
use crate::app_event::McpInventory;
|
||||
use crate::hooks_rpc::fetch_hooks_list;
|
||||
use crate::hooks_rpc::write_hook_trust;
|
||||
use crate::hooks_rpc::write_hook_trusts;
|
||||
@@ -26,10 +30,11 @@ impl App {
|
||||
app_server: &AppServerSession,
|
||||
detail: McpServerStatusDetail,
|
||||
) {
|
||||
let cwd = self.chat_widget.config_ref().cwd.to_path_buf();
|
||||
let request_handle = app_server.request_handle();
|
||||
let app_event_tx = self.app_event_tx.clone();
|
||||
tokio::spawn(async move {
|
||||
let result = fetch_all_mcp_server_statuses(request_handle, detail)
|
||||
let result = fetch_mcp_inventory_from_app_server(request_handle, detail, cwd)
|
||||
.await
|
||||
.map_err(|err| err.to_string());
|
||||
app_event_tx.send(AppEvent::McpInventoryLoaded { result, detail });
|
||||
@@ -360,16 +365,12 @@ impl App {
|
||||
}
|
||||
|
||||
pub(super) fn refresh_plugin_mentions(&mut self, app_server: &AppServerSession) {
|
||||
let config = self.config.clone();
|
||||
let cwd = self.chat_widget.config_ref().cwd.to_path_buf();
|
||||
let request_handle = app_server.request_handle();
|
||||
let app_event_tx = self.app_event_tx.clone();
|
||||
if !config.features.enabled(Feature::Plugins) {
|
||||
app_event_tx.send(AppEvent::PluginMentionsLoaded { plugins: None });
|
||||
return;
|
||||
}
|
||||
|
||||
tokio::spawn(async move {
|
||||
match fetch_plugin_mentions(request_handle, config).await {
|
||||
match fetch_plugin_mentions(request_handle, cwd).await {
|
||||
Ok(plugins) => {
|
||||
app_event_tx.send(AppEvent::PluginMentionsLoaded {
|
||||
plugins: Some(plugins),
|
||||
@@ -505,19 +506,18 @@ impl App {
|
||||
/// Process the completed MCP inventory fetch: clear the loading spinner, then
|
||||
/// render either the full tool/resource listing or an error into chat history.
|
||||
///
|
||||
/// When both the local config and the app-server report zero servers, a special
|
||||
/// "empty" cell is shown instead of the full table.
|
||||
/// When app-server status and config both report zero servers, a special "empty" cell is shown
|
||||
/// instead of the full table.
|
||||
pub(super) fn handle_mcp_inventory_result(
|
||||
&mut self,
|
||||
result: Result<Vec<McpServerStatus>, String>,
|
||||
result: Result<McpInventory, String>,
|
||||
detail: McpServerStatusDetail,
|
||||
) {
|
||||
let config = self.chat_widget.config_ref().clone();
|
||||
self.chat_widget.clear_mcp_inventory_loading();
|
||||
self.clear_committed_mcp_inventory_loading();
|
||||
|
||||
let statuses = match result {
|
||||
Ok(statuses) => statuses,
|
||||
let inventory = match result {
|
||||
Ok(inventory) => inventory,
|
||||
Err(err) => {
|
||||
self.chat_widget
|
||||
.add_error_message(format!("Failed to load MCP inventory: {err}"));
|
||||
@@ -525,7 +525,7 @@ impl App {
|
||||
}
|
||||
};
|
||||
|
||||
if config.mcp_servers.get().is_empty() && statuses.is_empty() {
|
||||
if inventory.statuses.is_empty() && inventory.configured_servers.is_empty() {
|
||||
self.chat_widget
|
||||
.add_to_history(history_cell::empty_mcp_output());
|
||||
return;
|
||||
@@ -533,7 +533,9 @@ impl App {
|
||||
|
||||
self.chat_widget
|
||||
.add_to_history(history_cell::new_mcp_tools_output_from_statuses(
|
||||
&config, &statuses, detail,
|
||||
&inventory.configured_servers,
|
||||
&inventory.statuses,
|
||||
detail,
|
||||
));
|
||||
}
|
||||
|
||||
@@ -553,6 +555,64 @@ impl App {
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) async fn fetch_mcp_inventory_from_app_server(
|
||||
request_handle: AppServerRequestHandle,
|
||||
detail: McpServerStatusDetail,
|
||||
cwd: PathBuf,
|
||||
) -> Result<McpInventory> {
|
||||
let configured_servers = match fetch_app_server_mcp_config(request_handle.clone(), cwd).await {
|
||||
Ok(configured_servers) => configured_servers,
|
||||
Err(err) => {
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
"config/read failed while loading MCP inventory config; rendering status data only"
|
||||
);
|
||||
HashMap::new()
|
||||
}
|
||||
};
|
||||
let statuses = fetch_all_mcp_server_statuses(request_handle, detail).await?;
|
||||
Ok(McpInventory {
|
||||
statuses,
|
||||
configured_servers,
|
||||
})
|
||||
}
|
||||
|
||||
pub(super) async fn fetch_app_server_config(
|
||||
request_handle: AppServerRequestHandle,
|
||||
cwd: Option<PathBuf>,
|
||||
include_layers: bool,
|
||||
) -> Result<ConfigReadResponse> {
|
||||
let request_id = RequestId::String(format!("config-read-{}", Uuid::new_v4()));
|
||||
request_handle
|
||||
.request_typed(ClientRequest::ConfigRead {
|
||||
request_id,
|
||||
params: ConfigReadParams {
|
||||
include_layers,
|
||||
cwd: cwd.map(|cwd| cwd.display().to_string()),
|
||||
},
|
||||
})
|
||||
.await
|
||||
.wrap_err("config/read failed in TUI")
|
||||
}
|
||||
|
||||
async fn fetch_app_server_mcp_config(
|
||||
request_handle: AppServerRequestHandle,
|
||||
cwd: PathBuf,
|
||||
) -> Result<HashMap<String, McpServerConfig>> {
|
||||
let response =
|
||||
fetch_app_server_config(request_handle, Some(cwd), /*include_layers*/ false).await?;
|
||||
mcp_servers_from_app_server_config(response.config)
|
||||
}
|
||||
|
||||
fn mcp_servers_from_app_server_config(
|
||||
config: codex_app_server_protocol::Config,
|
||||
) -> Result<HashMap<String, McpServerConfig>> {
|
||||
let Some(mcp_servers) = config.additional.get("mcp_servers") else {
|
||||
return Ok(HashMap::new());
|
||||
};
|
||||
serde_json::from_value(mcp_servers.clone()).wrap_err("failed to decode app-server MCP config")
|
||||
}
|
||||
|
||||
pub(super) async fn fetch_all_mcp_server_statuses(
|
||||
request_handle: AppServerRequestHandle,
|
||||
detail: McpServerStatusDetail,
|
||||
|
||||
@@ -4,7 +4,11 @@
|
||||
//! and the ChatWidget copy of session settings, keeping persistence-heavy code out of the main app
|
||||
//! loop.
|
||||
|
||||
use super::background_requests::fetch_app_server_config;
|
||||
use super::*;
|
||||
use codex_app_server_protocol::ConfigLayer as AppServerConfigLayer;
|
||||
use codex_config::ConfigLayerEntry;
|
||||
use codex_config::ConfigLayerStack;
|
||||
|
||||
impl App {
|
||||
pub(super) async fn rebuild_config_for_cwd(&self, cwd: PathBuf) -> Result<Config> {
|
||||
@@ -41,6 +45,40 @@ impl App {
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) async fn refresh_config_layer_stack_from_app_server_best_effort(
|
||||
&mut self,
|
||||
app_server: &AppServerSession,
|
||||
action: &str,
|
||||
) {
|
||||
let cwd = self.chat_widget.config_ref().cwd.to_path_buf();
|
||||
match fetch_app_server_config(
|
||||
app_server.request_handle(),
|
||||
Some(cwd),
|
||||
/*include_layers*/ true,
|
||||
)
|
||||
.await
|
||||
.and_then(|response| {
|
||||
let Some(layers) = response.layers else {
|
||||
return Err(color_eyre::eyre::eyre!(
|
||||
"config/read response did not include layers"
|
||||
));
|
||||
};
|
||||
config_layer_stack_from_app_server_layers(layers, &self.config.config_layer_stack)
|
||||
}) {
|
||||
Ok(config_layer_stack) => {
|
||||
self.config.config_layer_stack = config_layer_stack;
|
||||
self.chat_widget.sync_plugin_mentions_config(&self.config);
|
||||
}
|
||||
Err(err) => {
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
action,
|
||||
"failed to refresh config layers from app server"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) async fn rebuild_config_for_resume_or_fallback(
|
||||
&mut self,
|
||||
current_cwd: &Path,
|
||||
@@ -584,12 +622,46 @@ impl App {
|
||||
}
|
||||
}
|
||||
|
||||
fn config_layer_stack_from_app_server_layers(
|
||||
layers: Vec<AppServerConfigLayer>,
|
||||
current_stack: &ConfigLayerStack,
|
||||
) -> Result<ConfigLayerStack> {
|
||||
let mut layers = layers
|
||||
.into_iter()
|
||||
.map(config_layer_entry_from_app_server_layer)
|
||||
.collect::<Result<Vec<_>>>()?;
|
||||
// config/read returns layers highest-precedence first; ConfigLayerStack stores them lowest
|
||||
// precedence first.
|
||||
layers.reverse();
|
||||
ConfigLayerStack::new(
|
||||
layers,
|
||||
current_stack.requirements().clone(),
|
||||
current_stack.requirements_toml().clone(),
|
||||
)
|
||||
.wrap_err("invalid app-server config layer stack")
|
||||
}
|
||||
|
||||
fn config_layer_entry_from_app_server_layer(
|
||||
layer: AppServerConfigLayer,
|
||||
) -> Result<ConfigLayerEntry> {
|
||||
let config = serde_json::from_value(layer.config)
|
||||
.wrap_err("failed to decode app-server config layer")?;
|
||||
let mut entry = if let Some(disabled_reason) = layer.disabled_reason {
|
||||
ConfigLayerEntry::new_disabled(layer.name, config, disabled_reason)
|
||||
} else {
|
||||
ConfigLayerEntry::new(layer.name, config)
|
||||
};
|
||||
entry.version = layer.version;
|
||||
Ok(entry)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::app::test_support::app_enabled_in_effective_config;
|
||||
use crate::app::test_support::make_test_app;
|
||||
use crate::test_support::PathBufExt;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
@@ -651,6 +723,50 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_layer_stack_from_app_server_layers_uses_server_user_config() -> Result<()> {
|
||||
let user_file = AbsolutePathBuf::try_from(PathBuf::from("/tmp/codex/config.toml"))?;
|
||||
let stack = config_layer_stack_from_app_server_layers(
|
||||
vec![AppServerConfigLayer {
|
||||
name: ConfigLayerSource::User {
|
||||
file: user_file,
|
||||
profile: None,
|
||||
},
|
||||
version: "server-version".to_string(),
|
||||
config: serde_json::json!({
|
||||
"marketplaces": {
|
||||
"server-marketplace": {
|
||||
"source_type": "git",
|
||||
},
|
||||
},
|
||||
}),
|
||||
disabled_reason: None,
|
||||
}],
|
||||
&ConfigLayerStack::default(),
|
||||
)?;
|
||||
|
||||
let user_config = stack
|
||||
.effective_user_config()
|
||||
.expect("server user config layer");
|
||||
let source_type = user_config
|
||||
.get("marketplaces")
|
||||
.and_then(toml::Value::as_table)
|
||||
.and_then(|marketplaces| marketplaces.get("server-marketplace"))
|
||||
.and_then(toml::Value::as_table)
|
||||
.and_then(|marketplace| marketplace.get("source_type"))
|
||||
.and_then(toml::Value::as_str);
|
||||
|
||||
assert_eq!(source_type, Some("git"));
|
||||
assert_eq!(
|
||||
stack
|
||||
.get_active_user_layer()
|
||||
.expect("active user layer")
|
||||
.version,
|
||||
"server-version"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn refresh_in_memory_config_from_disk_best_effort_keeps_current_config_on_error()
|
||||
-> Result<()> {
|
||||
|
||||
@@ -502,9 +502,11 @@ impl App {
|
||||
self.chat_widget
|
||||
.on_marketplace_add_loaded(cwd.clone(), source, result);
|
||||
if add_succeeded && self.chat_widget.config_ref().cwd.as_path() == cwd.as_path() {
|
||||
if let Err(err) = self.refresh_in_memory_config_from_disk().await {
|
||||
tracing::warn!(error = %err, "failed to refresh config after marketplace add");
|
||||
}
|
||||
self.refresh_config_layer_stack_from_app_server_best_effort(
|
||||
app_server,
|
||||
"marketplace add",
|
||||
)
|
||||
.await;
|
||||
self.fetch_plugins_list(app_server, cwd);
|
||||
}
|
||||
}
|
||||
@@ -512,12 +514,11 @@ impl App {
|
||||
let marketplace_contents_changed =
|
||||
matches!(&result, Ok(response) if !response.upgraded_roots.is_empty());
|
||||
if marketplace_contents_changed {
|
||||
if let Err(err) = self.refresh_in_memory_config_from_disk().await {
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
"failed to refresh config after marketplace upgrade"
|
||||
);
|
||||
}
|
||||
self.refresh_config_layer_stack_from_app_server_best_effort(
|
||||
app_server,
|
||||
"marketplace upgrade",
|
||||
)
|
||||
.await;
|
||||
self.chat_widget.refresh_plugin_mentions();
|
||||
self.chat_widget.submit_op(AppCommand::reload_user_config());
|
||||
}
|
||||
@@ -554,9 +555,11 @@ impl App {
|
||||
);
|
||||
if remove_succeeded && self.chat_widget.config_ref().cwd.as_path() == cwd.as_path()
|
||||
{
|
||||
if let Err(err) = self.refresh_in_memory_config_from_disk().await {
|
||||
tracing::warn!(error = %err, "failed to refresh config after marketplace remove");
|
||||
}
|
||||
self.refresh_config_layer_stack_from_app_server_best_effort(
|
||||
app_server,
|
||||
"marketplace remove",
|
||||
)
|
||||
.await;
|
||||
self.chat_widget.refresh_plugin_mentions();
|
||||
self.chat_widget.submit_op(AppCommand::reload_user_config());
|
||||
self.fetch_plugins_list(app_server, cwd);
|
||||
@@ -605,9 +608,11 @@ impl App {
|
||||
} => {
|
||||
let install_succeeded = result.is_ok();
|
||||
if install_succeeded {
|
||||
if let Err(err) = self.refresh_in_memory_config_from_disk().await {
|
||||
tracing::warn!(error = %err, "failed to refresh config after plugin install");
|
||||
}
|
||||
self.refresh_config_layer_stack_from_app_server_best_effort(
|
||||
app_server,
|
||||
"plugin install",
|
||||
)
|
||||
.await;
|
||||
self.chat_widget.refresh_plugin_mentions();
|
||||
self.chat_widget.submit_op(AppCommand::reload_user_config());
|
||||
}
|
||||
@@ -661,12 +666,11 @@ impl App {
|
||||
self.pending_plugin_enabled_writes.remove(&plugin_id);
|
||||
let update_succeeded = result.is_ok();
|
||||
if update_succeeded {
|
||||
if let Err(err) = self.refresh_in_memory_config_from_disk().await {
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
"failed to refresh config after plugin toggle"
|
||||
);
|
||||
}
|
||||
self.refresh_config_layer_stack_from_app_server_best_effort(
|
||||
app_server,
|
||||
"plugin toggle",
|
||||
)
|
||||
.await;
|
||||
self.chat_widget.refresh_plugin_mentions();
|
||||
self.chat_widget.submit_op(AppCommand::reload_user_config());
|
||||
}
|
||||
@@ -1238,12 +1242,11 @@ impl App {
|
||||
} => {
|
||||
let uninstall_succeeded = result.is_ok();
|
||||
if uninstall_succeeded {
|
||||
if let Err(err) = self.refresh_in_memory_config_from_disk().await {
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
"failed to refresh config after plugin uninstall"
|
||||
);
|
||||
}
|
||||
self.refresh_config_layer_stack_from_app_server_best_effort(
|
||||
app_server,
|
||||
"plugin uninstall",
|
||||
)
|
||||
.await;
|
||||
self.chat_widget.refresh_plugin_mentions();
|
||||
self.chat_widget.submit_op(AppCommand::reload_user_config());
|
||||
}
|
||||
@@ -1261,10 +1264,7 @@ impl App {
|
||||
AppEvent::RefreshPluginMentions => {
|
||||
self.refresh_plugin_mentions(app_server);
|
||||
}
|
||||
AppEvent::PluginMentionsLoaded { mut plugins } => {
|
||||
if !self.config.features.enabled(Feature::Plugins) {
|
||||
plugins = None;
|
||||
}
|
||||
AppEvent::PluginMentionsLoaded { plugins } => {
|
||||
self.chat_widget.on_plugin_mentions_loaded(plugins);
|
||||
}
|
||||
AppEvent::PersistPersonalitySelection { personality } => {
|
||||
|
||||
@@ -1,109 +1,108 @@
|
||||
//! Plugin mention capability enrichment for the TUI.
|
||||
//!
|
||||
//! Mention inventory comes from app-server `plugin/list`, while mention eligibility still reuses
|
||||
//! the older local bulk capability summaries. That keeps the feature app-server-shaped without
|
||||
//! paying for a `plugin/read` per plugin.
|
||||
//! Mention inventory comes from app-server `plugin/list`, then eligible plugins are hydrated with
|
||||
//! `plugin/read` so the TUI does not derive mention capabilities from the client machine's local
|
||||
//! config or plugin cache.
|
||||
|
||||
use super::background_requests::fetch_plugin_detail;
|
||||
use super::background_requests::request_plugin_list;
|
||||
use super::*;
|
||||
use codex_app_server_protocol::PluginDetail;
|
||||
use codex_app_server_protocol::PluginListResponse;
|
||||
use codex_app_server_protocol::PluginMarketplaceEntry;
|
||||
use codex_app_server_protocol::PluginReadParams;
|
||||
use codex_app_server_protocol::PluginSummary;
|
||||
use codex_core_plugins::PluginsManager;
|
||||
use codex_plugin::AppConnectorId;
|
||||
use codex_plugin::PluginCapabilitySummary;
|
||||
use std::collections::HashMap;
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
struct PluginMentionEntry {
|
||||
config_name: String,
|
||||
display_name: String,
|
||||
description: Option<String>,
|
||||
}
|
||||
|
||||
impl PluginMentionEntry {
|
||||
fn capability_summary(
|
||||
self,
|
||||
capabilities_by_config_name: &HashMap<String, PluginCapabilitySummary>,
|
||||
) -> Option<PluginCapabilitySummary> {
|
||||
let capabilities = capabilities_by_config_name.get(&self.config_name)?;
|
||||
Some(PluginCapabilitySummary {
|
||||
config_name: self.config_name,
|
||||
display_name: self.display_name,
|
||||
description: self.description,
|
||||
has_skills: capabilities.has_skills,
|
||||
mcp_server_names: capabilities.mcp_server_names.clone(),
|
||||
app_connector_ids: capabilities.app_connector_ids.clone(),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) async fn fetch_plugin_mentions(
|
||||
request_handle: AppServerRequestHandle,
|
||||
config: crate::legacy_core::config::Config,
|
||||
cwd: PathBuf,
|
||||
) -> Result<Vec<PluginCapabilitySummary>> {
|
||||
let response = request_plugin_list(request_handle, config.cwd.to_path_buf()).await?;
|
||||
let mention_entries = plugin_mention_entries_from_list_response(response);
|
||||
let capabilities_by_config_name = load_plugin_mention_capabilities(&config).await;
|
||||
let response = request_plugin_list(request_handle.clone(), cwd).await?;
|
||||
let mention_reads = plugin_mention_reads_from_list_response(response);
|
||||
|
||||
Ok(mention_entries
|
||||
.into_iter()
|
||||
.filter_map(|entry| entry.capability_summary(&capabilities_by_config_name))
|
||||
.collect())
|
||||
}
|
||||
|
||||
async fn load_plugin_mention_capabilities(
|
||||
config: &crate::legacy_core::config::Config,
|
||||
) -> HashMap<String, PluginCapabilitySummary> {
|
||||
let plugins_input = config.plugins_config_input();
|
||||
PluginsManager::new(config.codex_home.to_path_buf())
|
||||
.plugins_for_config(&plugins_input)
|
||||
.await
|
||||
.capability_summaries()
|
||||
.iter()
|
||||
.cloned()
|
||||
.map(|summary| (summary.config_name.clone(), summary))
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn plugin_mention_entries_from_list_response(
|
||||
response: PluginListResponse,
|
||||
) -> Vec<PluginMentionEntry> {
|
||||
response
|
||||
.marketplaces
|
||||
.into_iter()
|
||||
.flat_map(plugin_mention_entries_from_marketplace)
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn plugin_mention_entries_from_marketplace(
|
||||
marketplace: PluginMarketplaceEntry,
|
||||
) -> Vec<PluginMentionEntry> {
|
||||
let marketplace_name = marketplace.name;
|
||||
marketplace
|
||||
.plugins
|
||||
.into_iter()
|
||||
.filter_map(|plugin| plugin_mention_entry(&marketplace_name, plugin))
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn plugin_mention_entry(
|
||||
marketplace_name: &str,
|
||||
plugin: PluginSummary,
|
||||
) -> Option<PluginMentionEntry> {
|
||||
if !plugin_is_eligible_for_mentions(&plugin) {
|
||||
return None;
|
||||
let mut capabilities = Vec::new();
|
||||
for read_params in mention_reads {
|
||||
let plugin_name = read_params.plugin_name.clone();
|
||||
match fetch_plugin_detail(request_handle.clone(), read_params).await {
|
||||
Ok(response) => {
|
||||
if let Some(summary) = plugin_detail_capability_summary(response.plugin) {
|
||||
capabilities.push(summary);
|
||||
}
|
||||
}
|
||||
Err(err) => {
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
plugin_name,
|
||||
"plugin/read failed while refreshing plugin mention candidates"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(capabilities)
|
||||
}
|
||||
|
||||
let config_name = plugin_mention_config_name(marketplace_name, &plugin)?;
|
||||
Some(PluginMentionEntry {
|
||||
fn plugin_detail_capability_summary(detail: PluginDetail) -> Option<PluginCapabilitySummary> {
|
||||
let config_name = plugin_mention_config_name(&detail.marketplace_name, &detail.summary)?;
|
||||
Some(PluginCapabilitySummary {
|
||||
config_name,
|
||||
display_name: plugin_mention_display_name(&plugin),
|
||||
description: plugin_mention_description(&plugin),
|
||||
display_name: plugin_mention_display_name(&detail.summary),
|
||||
description: plugin_mention_description(&detail.summary),
|
||||
has_skills: detail.skills.iter().any(|skill| skill.enabled),
|
||||
mcp_server_names: detail.mcp_servers,
|
||||
app_connector_ids: detail
|
||||
.apps
|
||||
.into_iter()
|
||||
.map(|app| AppConnectorId(app.id))
|
||||
.collect(),
|
||||
})
|
||||
}
|
||||
|
||||
fn plugin_is_eligible_for_mentions(plugin: &PluginSummary) -> bool {
|
||||
plugin.installed && plugin.enabled
|
||||
fn plugin_mention_reads_from_list_response(response: PluginListResponse) -> Vec<PluginReadParams> {
|
||||
response
|
||||
.marketplaces
|
||||
.into_iter()
|
||||
.flat_map(|marketplace| {
|
||||
let marketplace_name = marketplace.name;
|
||||
let marketplace_path = marketplace.path;
|
||||
marketplace.plugins.into_iter().filter_map(move |plugin| {
|
||||
plugin_mention_read(&marketplace_name, marketplace_path.clone(), plugin)
|
||||
})
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn plugin_mention_read(
|
||||
marketplace_name: &str,
|
||||
marketplace_path: Option<AbsolutePathBuf>,
|
||||
plugin: PluginSummary,
|
||||
) -> Option<PluginReadParams> {
|
||||
if !plugin.installed || !plugin.enabled {
|
||||
return None;
|
||||
}
|
||||
|
||||
plugin_mention_config_name(marketplace_name, &plugin)?;
|
||||
if let Some(marketplace_path) = marketplace_path {
|
||||
Some(PluginReadParams {
|
||||
marketplace_path: Some(marketplace_path),
|
||||
remote_marketplace_name: None,
|
||||
plugin_name: plugin.name,
|
||||
})
|
||||
} else {
|
||||
let Some(remote_plugin_id) = plugin.remote_plugin_id else {
|
||||
tracing::warn!(
|
||||
plugin_name = plugin.name,
|
||||
marketplace_name,
|
||||
"skipping remote plugin mention without remote plugin id"
|
||||
);
|
||||
return None;
|
||||
};
|
||||
Some(PluginReadParams {
|
||||
marketplace_path: None,
|
||||
remote_marketplace_name: Some(marketplace_name.to_string()),
|
||||
plugin_name: remote_plugin_id,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
fn plugin_mention_config_name(marketplace_name: &str, plugin: &PluginSummary) -> Option<String> {
|
||||
@@ -145,3 +144,127 @@ fn plugin_mention_description(plugin: &PluginSummary) -> Option<String> {
|
||||
.filter(|description| !description.is_empty())
|
||||
.map(str::to_string)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_app_server_protocol::AppSummary;
|
||||
use codex_app_server_protocol::PluginAuthPolicy;
|
||||
use codex_app_server_protocol::PluginAvailability;
|
||||
use codex_app_server_protocol::PluginInstallPolicy;
|
||||
use codex_app_server_protocol::PluginMarketplaceEntry;
|
||||
use codex_app_server_protocol::PluginSource;
|
||||
use codex_app_server_protocol::SkillSummary;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn test_absolute_path(path: &str) -> AbsolutePathBuf {
|
||||
AbsolutePathBuf::try_from(PathBuf::from(path)).expect("absolute test path")
|
||||
}
|
||||
|
||||
fn plugin_summary(name: &str, installed: bool, enabled: bool) -> PluginSummary {
|
||||
PluginSummary {
|
||||
id: format!("{name}@test-marketplace"),
|
||||
remote_plugin_id: None,
|
||||
local_version: None,
|
||||
name: name.to_string(),
|
||||
share_context: None,
|
||||
source: PluginSource::Remote,
|
||||
installed,
|
||||
enabled,
|
||||
install_policy: PluginInstallPolicy::Available,
|
||||
auth_policy: PluginAuthPolicy::OnUse,
|
||||
availability: PluginAvailability::Available,
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plugin_mentions_use_app_server_detail_capabilities() {
|
||||
let summary = plugin_detail_capability_summary(PluginDetail {
|
||||
marketplace_name: "test-marketplace".to_string(),
|
||||
marketplace_path: Some(test_absolute_path("/marketplace")),
|
||||
summary: plugin_summary("calendar", /*installed*/ true, /*enabled*/ true),
|
||||
description: None,
|
||||
skills: vec![SkillSummary {
|
||||
name: "schedule".to_string(),
|
||||
description: "Schedule things".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: None,
|
||||
enabled: true,
|
||||
}],
|
||||
hooks: Vec::new(),
|
||||
apps: vec![AppSummary {
|
||||
id: "calendar-app".to_string(),
|
||||
name: "Calendar".to_string(),
|
||||
description: None,
|
||||
install_url: None,
|
||||
needs_auth: false,
|
||||
}],
|
||||
mcp_servers: vec!["calendar-mcp".to_string()],
|
||||
})
|
||||
.expect("valid plugin detail");
|
||||
|
||||
assert_eq!(
|
||||
summary,
|
||||
PluginCapabilitySummary {
|
||||
config_name: "calendar@test-marketplace".to_string(),
|
||||
display_name: "calendar".to_string(),
|
||||
description: None,
|
||||
has_skills: true,
|
||||
mcp_server_names: vec!["calendar-mcp".to_string()],
|
||||
app_connector_ids: vec![AppConnectorId("calendar-app".to_string())],
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plugin_mention_reads_skip_disabled_and_handle_local_and_remote_sources() {
|
||||
let marketplace_path = test_absolute_path("/marketplace");
|
||||
let mut remote_plugin = plugin_summary(
|
||||
"remote_name",
|
||||
/*installed*/ true,
|
||||
/*enabled*/ true,
|
||||
);
|
||||
remote_plugin.remote_plugin_id = Some("plugins~remote_id".to_string());
|
||||
|
||||
let reads = plugin_mention_reads_from_list_response(PluginListResponse {
|
||||
marketplaces: vec![
|
||||
PluginMarketplaceEntry {
|
||||
name: "test-marketplace".to_string(),
|
||||
path: Some(marketplace_path.clone()),
|
||||
interface: None,
|
||||
plugins: vec![
|
||||
plugin_summary("enabled", /*installed*/ true, /*enabled*/ true),
|
||||
plugin_summary("disabled", /*installed*/ true, /*enabled*/ false),
|
||||
],
|
||||
},
|
||||
PluginMarketplaceEntry {
|
||||
name: "chatgpt-global".to_string(),
|
||||
path: None,
|
||||
interface: None,
|
||||
plugins: vec![remote_plugin],
|
||||
},
|
||||
],
|
||||
marketplace_load_errors: Vec::new(),
|
||||
featured_plugin_ids: Vec::new(),
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
reads,
|
||||
vec![
|
||||
PluginReadParams {
|
||||
marketplace_path: Some(marketplace_path),
|
||||
remote_marketplace_name: None,
|
||||
plugin_name: "enabled".to_string(),
|
||||
},
|
||||
PluginReadParams {
|
||||
marketplace_path: None,
|
||||
remote_marketplace_name: Some("chatgpt-global".to_string()),
|
||||
plugin_name: "plugins~remote_id".to_string(),
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -117,19 +117,74 @@ async fn handle_mcp_inventory_result_clears_committed_loading_cell() {
|
||||
)));
|
||||
|
||||
app.handle_mcp_inventory_result(
|
||||
Ok(vec![McpServerStatus {
|
||||
name: "docs".to_string(),
|
||||
tools: HashMap::new(),
|
||||
resources: Vec::new(),
|
||||
resource_templates: Vec::new(),
|
||||
auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported,
|
||||
}]),
|
||||
Ok(crate::app_event::McpInventory {
|
||||
statuses: vec![McpServerStatus {
|
||||
name: "docs".to_string(),
|
||||
tools: HashMap::new(),
|
||||
resources: Vec::new(),
|
||||
resource_templates: Vec::new(),
|
||||
auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported,
|
||||
}],
|
||||
configured_servers: HashMap::new(),
|
||||
}),
|
||||
McpServerStatusDetail::ToolsAndAuthOnly,
|
||||
);
|
||||
|
||||
assert_eq!(app.transcript_cells.len(), 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn handle_mcp_inventory_result_uses_server_empty_inventory_not_client_config() {
|
||||
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
let local_server: codex_config::types::McpServerConfig =
|
||||
toml::from_str("command = \"local-only\"").expect("valid test MCP config");
|
||||
let local_servers = HashMap::from([("local_only".to_string(), local_server)]);
|
||||
app.config
|
||||
.mcp_servers
|
||||
.set(local_servers)
|
||||
.expect("test config accepts MCP servers");
|
||||
|
||||
app.handle_mcp_inventory_result(
|
||||
Ok(crate::app_event::McpInventory {
|
||||
statuses: Vec::new(),
|
||||
configured_servers: HashMap::new(),
|
||||
}),
|
||||
McpServerStatusDetail::ToolsAndAuthOnly,
|
||||
);
|
||||
|
||||
let cell = match app_event_rx.try_recv() {
|
||||
Ok(AppEvent::InsertHistoryCell(cell)) => cell,
|
||||
other => panic!("expected MCP inventory history cell, got {other:?}"),
|
||||
};
|
||||
let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 120));
|
||||
assert!(rendered.contains("No MCP servers configured."));
|
||||
assert!(!rendered.contains("local-only"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn handle_mcp_inventory_result_uses_server_config_when_status_inventory_is_empty() {
|
||||
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
let server_config: codex_config::types::McpServerConfig =
|
||||
toml::from_str("command = \"server-only\"").expect("valid test MCP config");
|
||||
|
||||
app.handle_mcp_inventory_result(
|
||||
Ok(crate::app_event::McpInventory {
|
||||
statuses: Vec::new(),
|
||||
configured_servers: HashMap::from([("remote_only".to_string(), server_config)]),
|
||||
}),
|
||||
McpServerStatusDetail::Full,
|
||||
);
|
||||
|
||||
let cell = match app_event_rx.try_recv() {
|
||||
Ok(AppEvent::InsertHistoryCell(cell)) => cell,
|
||||
other => panic!("expected MCP inventory history cell, got {other:?}"),
|
||||
};
|
||||
let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 120));
|
||||
assert!(!rendered.contains("No MCP servers configured."));
|
||||
assert!(rendered.contains("remote_only"));
|
||||
assert!(rendered.contains("Command: server-only"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bypass_hook_trust_startup_warning_snapshot() {
|
||||
let rendered = lines_to_single_string(
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
//! Exit is modelled explicitly via `AppEvent::Exit(ExitMode)` so callers can request shutdown-first
|
||||
//! quits without reaching into the app loop or coupling to shutdown/exit sequencing.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use codex_app_server_protocol::AddCreditsNudgeCreditType;
|
||||
@@ -39,6 +40,7 @@ use crate::bottom_pane::TerminalTitleItem;
|
||||
use crate::chatwidget::UserMessage;
|
||||
use codex_app_server_protocol::AskForApproval;
|
||||
use codex_config::types::ApprovalsReviewer;
|
||||
use codex_config::types::McpServerConfig;
|
||||
use codex_features::Feature;
|
||||
use codex_plugin::PluginCapabilitySummary;
|
||||
use codex_protocol::config_types::CollaborationModeMask;
|
||||
@@ -108,6 +110,12 @@ pub(crate) struct ConnectorsSnapshot {
|
||||
pub(crate) connectors: Vec<AppInfo>,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub(crate) struct McpInventory {
|
||||
pub(crate) statuses: Vec<McpServerStatus>,
|
||||
pub(crate) configured_servers: HashMap<String, McpServerConfig>,
|
||||
}
|
||||
|
||||
/// Distinguishes why a rate-limit refresh was requested so the completion
|
||||
/// handler can route the result correctly.
|
||||
///
|
||||
@@ -538,7 +546,7 @@ pub(crate) enum AppEvent {
|
||||
|
||||
/// Result of fetching MCP inventory via app-server RPCs.
|
||||
McpInventoryLoaded {
|
||||
result: Result<Vec<McpServerStatus>, String>,
|
||||
result: Result<McpInventory, String>,
|
||||
detail: McpServerStatusDetail,
|
||||
},
|
||||
|
||||
|
||||
@@ -1356,10 +1356,6 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
fn plugins_for_mentions(&self) -> Option<&[PluginCapabilitySummary]> {
|
||||
if !self.config.features.enabled(Feature::Plugins) {
|
||||
return None;
|
||||
}
|
||||
|
||||
self.bottom_pane.plugins().map(Vec::as_slice)
|
||||
}
|
||||
|
||||
@@ -1730,11 +1726,6 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
pub(crate) fn refresh_plugin_mentions(&mut self) {
|
||||
if !self.config.features.enabled(Feature::Plugins) {
|
||||
self.bottom_pane.set_plugin_mentions(/*plugins*/ None);
|
||||
return;
|
||||
}
|
||||
|
||||
self.app_event_tx.send(AppEvent::RefreshPluginMentions);
|
||||
}
|
||||
|
||||
|
||||
@@ -34,7 +34,6 @@ use codex_app_server_protocol::PluginReadResponse;
|
||||
use codex_app_server_protocol::PluginSummary;
|
||||
use codex_app_server_protocol::PluginUninstallResponse;
|
||||
use codex_core_plugins::OPENAI_CURATED_MARKETPLACE_NAME;
|
||||
use codex_features::Feature;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
@@ -169,14 +168,6 @@ pub(super) enum PluginsCacheState {
|
||||
|
||||
impl ChatWidget {
|
||||
pub(crate) fn add_plugins_output(&mut self) {
|
||||
if !self.config.features.enabled(Feature::Plugins) {
|
||||
self.add_info_message(
|
||||
"Plugins are disabled.".to_string(),
|
||||
Some("Enable the plugins feature to use /plugins.".to_string()),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
self.plugins_active_tab_id = Some(ALL_PLUGINS_TAB_ID.to_string());
|
||||
self.prefetch_plugins();
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
//! MCP tool-call, inventory, and output history cells.
|
||||
|
||||
use super::*;
|
||||
use codex_config::McpServerConfig;
|
||||
|
||||
#[derive(Debug)]
|
||||
struct CompletedMcpToolCallWithImageOutput {
|
||||
@@ -511,15 +512,15 @@ pub(crate) fn new_mcp_tools_output(
|
||||
|
||||
/// Build the `/mcp` history cell from app-server `McpServerStatus` responses.
|
||||
///
|
||||
/// The server list comes directly from the app-server status response, sorted
|
||||
/// alphabetically. Local config is only used to enrich returned servers with
|
||||
/// transport details such as command, URL, cwd, and environment display.
|
||||
/// The server list comes from app-server status plus app-server config, sorted alphabetically.
|
||||
/// Config enriches returned servers with transport details such as command, URL, cwd, and
|
||||
/// environment display, and keeps configured-only servers visible when status inventory is empty.
|
||||
///
|
||||
/// This mirrors the layout of [`new_mcp_tools_output`] but sources data from
|
||||
/// the paginated RPC response rather than the in-process `McpManager`. The
|
||||
/// `detail` flag controls whether resources and resource templates are rendered.
|
||||
pub(crate) fn new_mcp_tools_output_from_statuses(
|
||||
config: &Config,
|
||||
configured_servers: &HashMap<String, McpServerConfig>,
|
||||
statuses: &[McpServerStatus],
|
||||
detail: McpServerStatusDetail,
|
||||
) -> PlainHistoryCell {
|
||||
@@ -536,6 +537,12 @@ pub(crate) fn new_mcp_tools_output_from_statuses(
|
||||
}
|
||||
|
||||
let mut server_names: Vec<String> = statuses.iter().map(|status| status.name.clone()).collect();
|
||||
server_names.extend(
|
||||
configured_servers
|
||||
.keys()
|
||||
.filter(|server| !statuses_by_name.contains_key(server.as_str()))
|
||||
.cloned(),
|
||||
);
|
||||
server_names.sort();
|
||||
|
||||
let has_any_tools = statuses.iter().any(|status| !status.tools.is_empty());
|
||||
@@ -545,7 +552,7 @@ pub(crate) fn new_mcp_tools_output_from_statuses(
|
||||
}
|
||||
|
||||
for server in server_names {
|
||||
let cfg = config.mcp_servers.get().get(server.as_str());
|
||||
let cfg = configured_servers.get(server.as_str());
|
||||
let status = statuses_by_name.get(server.as_str()).copied();
|
||||
let header: Vec<Span<'static>> = vec![" • ".into(), server.clone().into()];
|
||||
|
||||
|
||||
+17
@@ -0,0 +1,17 @@
|
||||
---
|
||||
source: tui/src/history_cell/tests.rs
|
||||
expression: rendered
|
||||
---
|
||||
/mcp
|
||||
|
||||
🔌 MCP Tools
|
||||
|
||||
• No MCP tools available.
|
||||
|
||||
• server_only
|
||||
• Status: enabled
|
||||
• Auth: Unsupported
|
||||
• Command: server-only --stdio
|
||||
• Tools: (none)
|
||||
• Resources: (none)
|
||||
• Resource templates: (none)
|
||||
@@ -835,7 +835,7 @@ async fn mcp_tools_output_from_statuses_renders_status_only_servers() {
|
||||
}];
|
||||
|
||||
let cell = new_mcp_tools_output_from_statuses(
|
||||
&config,
|
||||
config.mcp_servers.get(),
|
||||
&statuses,
|
||||
McpServerStatusDetail::ToolsAndAuthOnly,
|
||||
);
|
||||
@@ -892,7 +892,24 @@ async fn mcp_tools_output_from_statuses_renders_verbose_inventory() {
|
||||
auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported,
|
||||
}];
|
||||
|
||||
let cell = new_mcp_tools_output_from_statuses(&config, &statuses, McpServerStatusDetail::Full);
|
||||
let cell = new_mcp_tools_output_from_statuses(
|
||||
config.mcp_servers.get(),
|
||||
&statuses,
|
||||
McpServerStatusDetail::Full,
|
||||
);
|
||||
let rendered = render_lines(&cell.display_lines(/*width*/ 120)).join("\n");
|
||||
|
||||
insta::assert_snapshot!(rendered);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_tools_output_from_statuses_renders_configured_server_without_status() {
|
||||
let servers = HashMap::from([(
|
||||
"server_only".to_string(),
|
||||
stdio_server_config("server-only", vec!["--stdio"], /*env*/ None, vec![]),
|
||||
)]);
|
||||
|
||||
let cell = new_mcp_tools_output_from_statuses(&servers, &[], McpServerStatusDetail::Full);
|
||||
let rendered = render_lines(&cell.display_lines(/*width*/ 120)).join("\n");
|
||||
|
||||
insta::assert_snapshot!(rendered);
|
||||
|
||||
Reference in New Issue
Block a user