feat(mcp): show per-server tool status (fixes #526)

This commit is contained in:
James Peter
2026-01-23 23:05:20 +10:00
parent d07f6f6300
commit b26e4d6fee
5 changed files with 317 additions and 46 deletions
+39 -4
View File
@@ -18,6 +18,7 @@ use super::session::{
use crate::auth;
use crate::auth_accounts;
use crate::account_switching::RateLimitSwitchState;
use crate::protocol::McpListToolsResponseEvent;
use code_app_server_protocol::AuthMode as AppAuthMode;
#[derive(Clone, Debug, Eq, PartialEq)]
@@ -566,9 +567,16 @@ pub(super) async fn submission_loop(
// Surface individual client start-up failures to the user.
if !failed_clients.is_empty() {
for (server_name, err) in failed_clients {
let message =
format!("MCP client for `{server_name}` failed to start: {err:#}");
for (server_name, failure) in failed_clients {
let detail = failure.message;
let message = match failure.phase {
crate::protocol::McpServerFailurePhase::Start => {
format!("MCP server `{server_name}` failed to start: {detail}")
}
crate::protocol::McpServerFailurePhase::ListTools => format!(
"MCP server `{server_name}` failed to list tools: {detail}"
),
};
error!("{message}");
mcp_connection_errors.push(message);
}
@@ -997,7 +1005,34 @@ pub(super) async fn submission_loop(
}
});
}
// Upstream protocol no longer includes ListMcpTools; skip handling here.
Op::ListMcpTools => {
let sess = match sess.as_ref() {
Some(sess) => Arc::clone(sess),
None => {
send_no_session_event(sub.id).await;
continue;
}
};
let tools = sess.mcp_connection_manager.list_all_tools();
let server_tools = sess.mcp_connection_manager.list_tools_by_server();
let server_failures = sess.mcp_connection_manager.list_server_failures();
let event = Event {
id: sub.id.clone(),
event_seq: 0,
msg: EventMsg::McpListToolsResponse(McpListToolsResponseEvent {
tools,
server_tools: Some(server_tools),
server_failures: Some(server_failures),
}),
order: None,
};
if let Err(e) = tx_event.send(event).await {
warn!("failed to send McpListToolsResponse event: {e}");
}
}
Op::ListCustomPrompts => {
let sess = match sess.as_ref() {
Some(sess) => Arc::clone(sess),
+59 -10
View File
@@ -30,6 +30,7 @@ use tracing::warn;
use crate::config_types::McpServerConfig;
use crate::config_types::McpServerTransportConfig;
use crate::protocol::{McpServerFailure, McpServerFailurePhase};
/// Delimiter used to separate the server name from the tool name in a fully
/// qualified tool name.
@@ -81,9 +82,8 @@ fn append_sha1_suffix(base: &str, raw: &str) -> String {
/// Default timeout for initializing MCP server & initially listing tools.
const DEFAULT_STARTUP_TIMEOUT: Duration = Duration::from_secs(10);
/// Map that holds a startup error for every MCP server that could **not** be
/// spawned successfully.
pub type ClientStartErrors = HashMap<String, anyhow::Error>;
/// Map that holds startup or tool-list errors for MCP servers.
pub type ClientStartErrors = HashMap<String, McpServerFailure>;
fn qualify_tools(tools: Vec<ToolInfo>) -> HashMap<String, ToolInfo> {
let mut used_names = HashSet::new();
@@ -211,6 +211,8 @@ pub struct McpConnectionManager {
/// Fully qualified tool name -> tool instance.
tools: HashMap<String, ToolInfo>,
server_names: Vec<String>,
failures: HashMap<String, McpServerFailure>,
}
impl McpConnectionManager {
@@ -220,8 +222,8 @@ impl McpConnectionManager {
/// are human-readable server identifiers and *values* are the spawn
/// instructions.
///
/// Servers that fail to start are reported in `ClientStartErrors`: the
/// user should be informed about these errors.
/// Servers that fail to start or list tools are reported in `ClientStartErrors`:
/// the user should be informed about these errors.
pub async fn new(
mcp_servers: HashMap<String, McpServerConfig>,
excluded_tools: HashSet<(String, String)>,
@@ -238,10 +240,13 @@ impl McpConnectionManager {
for (server_name, cfg) in mcp_servers {
// Validate server name before spawning
if !is_valid_mcp_server_name(&server_name) {
let error = anyhow::anyhow!(
let message = format!(
"invalid server name '{server_name}': must match pattern ^[a-zA-Z0-9_-]+$"
);
errors.insert(server_name, error);
errors.insert(
server_name,
McpServerFailure { phase: McpServerFailurePhase::Start, message },
);
continue;
}
@@ -339,7 +344,14 @@ impl McpConnectionManager {
);
}
Err(e) => {
errors.insert(server_name, e);
let message = format!("server '{server_name}': {e:#}");
errors.insert(
server_name,
McpServerFailure {
phase: McpServerFailurePhase::Start,
message,
},
);
}
}
}
@@ -348,9 +360,15 @@ impl McpConnectionManager {
let tools = qualify_tools(all_tools);
let mut server_names: Vec<String> = clients.keys().cloned().collect();
server_names.sort();
let failures = errors.clone();
Ok((Self {
clients: RwLock::new(clients),
tools,
server_names,
failures,
}, errors))
}
@@ -363,6 +381,31 @@ impl McpConnectionManager {
.collect()
}
pub fn list_tools_by_server(&self) -> HashMap<String, Vec<String>> {
let mut tools_by_server: HashMap<String, Vec<String>> = HashMap::new();
for tool in self.tools.values() {
tools_by_server
.entry(tool.server_name.clone())
.or_default()
.push(tool.tool_name.clone());
}
for server_name in &self.server_names {
tools_by_server.entry(server_name.clone()).or_default();
}
for tools in tools_by_server.values_mut() {
tools.sort();
tools.dedup();
}
tools_by_server
}
pub fn list_server_failures(&self) -> HashMap<String, McpServerFailure> {
self.failures.clone()
}
/// Invoke the tool indicated by the (server, tool) pair.
pub async fn call_tool(
&self,
@@ -460,7 +503,13 @@ async fn list_all_tools(
warn!(
"Failed to list tools for MCP server '{server_name}': {err:#?}"
);
errors.insert(server_name, err.into());
errors.insert(
server_name,
McpServerFailure {
phase: McpServerFailurePhase::ListTools,
message: format!("{err:#}"),
},
);
}
}
}
@@ -613,7 +662,7 @@ mod tests {
let err = errors
.get("context7-mcp")
.expect("missing executable should be reported under server name");
let msg = format!("{err:#}");
let msg = err.message.as_str();
assert!(msg.contains("context7-mcp"), "error should mention the server name");
assert!(
+10
View File
@@ -39,6 +39,9 @@ pub use code_protocol::protocol::GitInfo;
pub use code_protocol::protocol::RolloutItem;
pub use code_protocol::protocol::RolloutLine;
pub use code_protocol::protocol::ConversationPathResponseEvent;
pub use code_protocol::protocol::McpListToolsResponseEvent;
pub use code_protocol::protocol::McpServerFailure;
pub use code_protocol::protocol::McpServerFailurePhase;
pub use code_protocol::protocol::ListCustomPromptsResponseEvent;
pub use code_protocol::protocol::ListSkillsResponseEvent;
pub use code_protocol::protocol::ViewImageToolCallEvent;
@@ -246,6 +249,10 @@ pub enum Op {
/// Request a single history entry identified by `log_id` + `offset`.
GetHistoryEntryRequest { offset: usize, log_id: u64 },
/// Request the list of MCP tools available across configured servers.
/// Reply is delivered via `EventMsg::McpListToolsResponse`.
ListMcpTools,
/// Request the list of available custom prompts.
/// Reply is delivered via `EventMsg::ListCustomPromptsResponse`.
ListCustomPrompts,
@@ -816,6 +823,9 @@ pub enum EventMsg {
/// Response to GetHistoryEntryRequest.
GetHistoryEntryResponse(GetHistoryEntryResponseEvent),
/// List of MCP tools available to the agent.
McpListToolsResponse(McpListToolsResponseEvent),
/// List of custom prompts available to the agent.
ListCustomPromptsResponse(ListCustomPromptsResponseEvent),
+17
View File
@@ -1421,11 +1421,28 @@ pub struct GetHistoryEntryResponseEvent {
pub entry: Option<HistoryEntry>,
}
#[derive(Debug, Clone, Deserialize, Serialize, TS)]
#[serde(rename_all = "snake_case")]
pub enum McpServerFailurePhase {
Start,
ListTools,
}
#[derive(Debug, Clone, Deserialize, Serialize, TS)]
pub struct McpServerFailure {
pub phase: McpServerFailurePhase,
pub message: String,
}
/// Response payload for `Op::ListMcpTools`.
#[derive(Debug, Clone, Deserialize, Serialize, TS)]
pub struct McpListToolsResponseEvent {
/// Fully qualified tool name -> tool definition.
pub tools: std::collections::HashMap<String, McpTool>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub server_tools: Option<std::collections::HashMap<String, Vec<String>>>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub server_failures: Option<std::collections::HashMap<String, McpServerFailure>>,
}
/// Response payload for `Op::ListCustomPrompts`.
+192 -32
View File
@@ -169,6 +169,8 @@ use code_core::protocol::ExecOutputStream;
use code_core::protocol::EnvironmentContextDeltaEvent;
use code_core::protocol::EnvironmentContextFullEvent;
use code_core::protocol::InputItem;
use code_core::protocol::McpServerFailure;
use code_core::protocol::McpServerFailurePhase;
use code_core::protocol::SessionConfiguredEvent;
// MCP tool call handlers moved into chatwidget::tools
use code_core::protocol::Op;
@@ -1507,6 +1509,8 @@ pub(crate) struct ChatWidget<'a> {
context_last_sequence: Option<u64>,
context_browser_sequence: Option<u64>,
config: Config,
mcp_tools_by_server: HashMap<String, Vec<String>>,
mcp_server_failures: HashMap<String, McpServerFailure>,
/// Optional remote-merged presets list delivered asynchronously.
/// When absent, the TUI falls back to built-in presets.
@@ -6414,6 +6418,8 @@ impl ChatWidget<'_> {
active_exec_cell: None,
history_cells,
config: config.clone(),
mcp_tools_by_server: HashMap::new(),
mcp_server_failures: HashMap::new(),
remote_model_presets: None,
allow_remote_default_at_startup: !config.model_explicit,
chat_model_selected_explicitly: false,
@@ -6773,6 +6779,8 @@ impl ChatWidget<'_> {
active_exec_cell: None,
history_cells,
config: config.clone(),
mcp_tools_by_server: HashMap::new(),
mcp_server_failures: HashMap::new(),
remote_model_presets: None,
allow_remote_default_at_startup: !config.model_explicit,
chat_model_selected_explicitly: false,
@@ -13125,6 +13133,11 @@ impl ChatWidget<'_> {
// Ask core for custom prompts so the slash menu can show them.
self.submit_op(Op::ListCustomPrompts);
self.submit_op(Op::ListSkills);
self.mcp_tools_by_server.clear();
self.mcp_server_failures.clear();
if !self.config.mcp_servers.is_empty() {
self.submit_op(Op::ListMcpTools);
}
if self.resume_placeholder_visible && event.history_entry_count == 0 {
self.replace_resume_placeholder_with_notice(RESUME_NO_HISTORY_NOTICE);
@@ -14749,6 +14762,11 @@ impl ChatWidget<'_> {
debug!("received {len} custom prompts");
self.bottom_pane.set_custom_prompts(ev.custom_prompts);
}
EventMsg::McpListToolsResponse(ev) => {
self.mcp_tools_by_server = ev.server_tools.unwrap_or_default();
self.mcp_server_failures = ev.server_failures.unwrap_or_default();
self.refresh_mcp_settings_overlay();
}
EventMsg::ListSkillsResponse(ev) => {
let len = ev.skills.len();
debug!("received {len} skills");
@@ -23688,17 +23706,19 @@ Have we met every part of this goal and is there no further work to do?"#
let mut rows: McpServerRows = Vec::new();
for (name, cfg) in enabled.into_iter() {
let summary = self.format_mcp_server_summary(&name, &cfg, true);
rows.push(McpServerRow {
name,
enabled: true,
summary: Self::format_mcp_summary(&cfg),
summary,
});
}
for (name, cfg) in disabled.into_iter() {
let summary = self.format_mcp_server_summary(&name, &cfg, false);
rows.push(McpServerRow {
name,
enabled: false,
summary: Self::format_mcp_summary(&cfg),
summary,
});
}
rows.sort_by(|a, b| a.name.cmp(&b.name));
@@ -24195,6 +24215,18 @@ Have we met every part of this goal and is there no further work to do?"#
Some(format!("Skills loaded: {count}"))
}
fn refresh_mcp_settings_overlay(&mut self) {
let content = self.build_mcp_settings_content();
let Some(content) = content else {
return;
};
let Some(overlay) = self.settings.overlay.as_mut() else {
return;
};
overlay.set_mcp_content(content);
self.request_redraw();
}
fn refresh_settings_overview_rows(&mut self) {
if self.settings.overlay.is_none() {
return;
@@ -27768,10 +27800,77 @@ Have we met every part of this goal and is there no further work to do?"#
}
}
fn format_mcp_server_summary(
&self,
name: &str,
cfg: &code_core::config_types::McpServerConfig,
enabled: bool,
) -> String {
let transport = Self::format_mcp_summary(cfg);
let status = self.format_mcp_tool_status(name, enabled);
if status.is_empty() {
transport
} else {
format!("{transport} · {status}")
}
}
fn format_mcp_tool_status(&self, name: &str, enabled: bool) -> String {
if !enabled {
return "Tools: disabled".to_string();
}
if let Some(failure) = self.mcp_server_failures.get(name) {
return self.format_mcp_failure(failure);
}
if let Some(tools) = self.mcp_tools_by_server.get(name) {
let list = Self::format_mcp_tool_list(tools);
return format!("Tools: {list}");
}
"Tools: pending".to_string()
}
fn format_mcp_tool_list(tools: &[String]) -> String {
const MAX_TOOLS: usize = 6;
const MAX_CHARS: usize = 120;
if tools.is_empty() {
return "none".to_string();
}
let mut display = tools
.iter()
.take(MAX_TOOLS)
.cloned()
.collect::<Vec<_>>()
.join(", ");
if tools.len() > MAX_TOOLS {
let remaining = tools.len().saturating_sub(MAX_TOOLS);
display.push_str(&format!(", +{remaining} more"));
}
Self::truncate_with_ellipsis(&display, MAX_CHARS)
}
fn format_mcp_failure(&self, failure: &McpServerFailure) -> String {
const MAX_CHARS: usize = 160;
let normalized = failure.message.replace('\n', " ");
let message = Self::truncate_with_ellipsis(&normalized, MAX_CHARS);
match failure.phase {
McpServerFailurePhase::Start => format!("Failed to start: {message}"),
McpServerFailurePhase::ListTools => format!("Failed to list tools: {message}"),
}
}
/// Handle `/mcp` command: manage MCP servers (status/on/off/add).
pub(crate) fn handle_mcp_command(&mut self, command_text: String) {
let trimmed = command_text.trim();
if trimmed.is_empty() {
if !self.config.mcp_servers.is_empty() {
self.submit_op(Op::ListMcpTools);
}
self.show_settings_overlay(Some(SettingsSection::Mcp));
return;
}
@@ -27780,42 +27879,51 @@ Have we met every part of this goal and is there no further work to do?"#
let sub = parts.next().unwrap_or("");
match sub {
"status" => match find_code_home() {
Ok(home) => match code_core::config::list_mcp_servers(&home) {
Ok((enabled, disabled)) => {
let mut lines = String::new();
if enabled.is_empty() && disabled.is_empty() {
lines.push_str("No MCP servers configured. Use /mcp add … to add one.");
} else {
lines.push_str(&format!("Enabled ({}):\n", enabled.len()));
for (name, cfg) in enabled {
lines.push_str(&format!(
"• {} — {}\n",
name,
Self::format_mcp_summary(&cfg)
));
}
lines.push_str(&format!("\nDisabled ({}):\n", disabled.len()));
for (name, cfg) in disabled {
lines.push_str(&format!(
"• {} — {}\n",
name,
Self::format_mcp_summary(&cfg)
));
"status" => {
if !self.config.mcp_servers.is_empty() {
self.submit_op(Op::ListMcpTools);
}
match find_code_home() {
Ok(home) => match code_core::config::list_mcp_servers(&home) {
Ok((enabled, disabled)) => {
let mut lines = String::new();
if enabled.is_empty() && disabled.is_empty() {
lines.push_str(
"No MCP servers configured. Use /mcp add … to add one.",
);
} else {
let enabled_count = enabled.len();
lines.push_str(&format!("Enabled ({enabled_count}):\n"));
for (name, cfg) in enabled {
lines.push_str(&format!(
"• {} — {}\n",
name,
self.format_mcp_server_summary(&name, &cfg, true)
));
}
let disabled_count = disabled.len();
lines.push_str(&format!("\nDisabled ({disabled_count}):\n"));
for (name, cfg) in disabled {
lines.push_str(&format!(
"• {} — {}\n",
name,
self.format_mcp_server_summary(&name, &cfg, false)
));
}
}
self.push_background_tail(lines);
}
self.push_background_tail(lines);
}
Err(e) => {
let msg = format!("Failed to read MCP config: {e}");
self.history_push_plain_state(history_cell::new_error_event(msg));
}
},
Err(e) => {
let msg = format!("Failed to read MCP config: {}", e);
let msg = format!("Failed to locate CODEX_HOME: {e}");
self.history_push_plain_state(history_cell::new_error_event(msg));
}
},
Err(e) => {
let msg = format!("Failed to locate CODEX_HOME: {}", e);
self.history_push_plain_state(history_cell::new_error_event(msg));
}
},
}
"on" | "off" => {
let name = parts.next().unwrap_or("");
if name.is_empty() {
@@ -29396,6 +29504,7 @@ impl Drop for AutoReviewStubGuard {
};
use code_core::parse_command::ParsedCommand;
use code_core::protocol::OrderMeta;
use code_core::config_types::{McpServerConfig, McpServerTransportConfig};
use code_core::protocol::{
AskForApproval,
AgentMessageEvent,
@@ -29404,6 +29513,8 @@ use code_core::protocol::OrderMeta;
Event,
EventMsg,
ExecCommandBeginEvent,
McpServerFailure,
McpServerFailurePhase,
TaskCompleteEvent,
};
use code_core::protocol::AgentInfo as CoreAgentInfo;
@@ -29447,6 +29558,55 @@ use code_core::protocol::OrderMeta;
assert!(summary_text.contains("bug"));
}
#[test]
fn mcp_summary_includes_tools_and_failures() {
let mut harness = ChatWidgetHarness::new();
harness.with_chat(|chat| {
chat.mcp_tools_by_server.insert(
"alpha".to_string(),
vec!["fetch".to_string(), "search".to_string()],
);
chat.mcp_server_failures.insert(
"beta".to_string(),
McpServerFailure {
phase: McpServerFailurePhase::ListTools,
message: "timeout".to_string(),
},
);
let ok_cfg = McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command: "alpha-bin".to_string(),
args: Vec::new(),
env: None,
},
startup_timeout_sec: None,
tool_timeout_sec: None,
};
let fail_cfg = McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command: "beta-bin".to_string(),
args: Vec::new(),
env: None,
},
startup_timeout_sec: None,
tool_timeout_sec: None,
};
let ok_summary = chat.format_mcp_server_summary("alpha", &ok_cfg, true);
let fail_summary = chat.format_mcp_server_summary("beta", &fail_cfg, true);
assert!(
ok_summary.contains("Tools: fetch, search"),
"expected tool list in summary, got: {ok_summary}"
);
assert!(
fail_summary.contains("Failed to list tools: timeout"),
"expected failure message in summary, got: {fail_summary}"
);
});
}
#[test]
fn parse_agent_review_result_json_multi_run() {
let json = r#"{