mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Add verbose diagnostics for /mcp (#18610)
Fixes #18539. ## Summary The recent `/mcp` performance work kept the default command fast by avoiding resource and resource-template inventory probes, but it also removed useful diagnostics for users trying to confirm MCP server state. This keeps bare `/mcp` on the fast tools/auth path and adds `/mcp verbose` for the slower diagnostic view. Verbose mode requests full MCP server status from the app-server and restores status, resources, and resource templates in the TUI output. ## Testing In addition to running automation, I manually tested the feature to confirm that it works.
This commit is contained in:
committed by
GitHub
Unverified
parent
e53e6bc48f
commit
5a8700abcc
+29
-19
@@ -2055,14 +2055,18 @@ impl App {
|
||||
/// tradeoff because the effect is limited to stale inventory output in history,
|
||||
/// while request-token invalidation would add cross-cutting async state for a
|
||||
/// low-severity path.
|
||||
fn fetch_mcp_inventory(&mut self, app_server: &AppServerSession) {
|
||||
fn fetch_mcp_inventory(
|
||||
&mut self,
|
||||
app_server: &AppServerSession,
|
||||
detail: McpServerStatusDetail,
|
||||
) {
|
||||
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)
|
||||
let result = fetch_all_mcp_server_statuses(request_handle, detail)
|
||||
.await
|
||||
.map_err(|err| err.to_string());
|
||||
app_event_tx.send(AppEvent::McpInventoryLoaded { result });
|
||||
app_event_tx.send(AppEvent::McpInventoryLoaded { result, detail });
|
||||
});
|
||||
}
|
||||
|
||||
@@ -2387,7 +2391,11 @@ impl App {
|
||||
///
|
||||
/// When both the local config and the app-server report zero servers, a special
|
||||
/// "empty" cell is shown instead of the full table.
|
||||
fn handle_mcp_inventory_result(&mut self, result: Result<Vec<McpServerStatus>, String>) {
|
||||
fn handle_mcp_inventory_result(
|
||||
&mut self,
|
||||
result: Result<Vec<McpServerStatus>, String>,
|
||||
detail: McpServerStatusDetail,
|
||||
) {
|
||||
let config = self.chat_widget.config_ref().clone();
|
||||
self.chat_widget.clear_mcp_inventory_loading();
|
||||
self.clear_committed_mcp_inventory_loading();
|
||||
@@ -2409,9 +2417,7 @@ impl App {
|
||||
|
||||
self.chat_widget
|
||||
.add_to_history(history_cell::new_mcp_tools_output_from_statuses(
|
||||
&config,
|
||||
&statuses,
|
||||
McpServerStatusDetail::ToolsAndAuthOnly,
|
||||
&config, &statuses, detail,
|
||||
));
|
||||
}
|
||||
|
||||
@@ -4969,11 +4975,11 @@ impl App {
|
||||
.on_plugin_enabled_set(cwd, plugin_id, enabled, result);
|
||||
}
|
||||
}
|
||||
AppEvent::FetchMcpInventory => {
|
||||
self.fetch_mcp_inventory(app_server);
|
||||
AppEvent::FetchMcpInventory { detail } => {
|
||||
self.fetch_mcp_inventory(app_server, detail);
|
||||
}
|
||||
AppEvent::McpInventoryLoaded { result } => {
|
||||
self.handle_mcp_inventory_result(result);
|
||||
AppEvent::McpInventoryLoaded { result, detail } => {
|
||||
self.handle_mcp_inventory_result(result, detail);
|
||||
}
|
||||
AppEvent::SkillsListLoaded { result } => {
|
||||
self.handle_skills_list_result(
|
||||
@@ -6671,6 +6677,7 @@ fn side_return_shortcut_matches(key_event: KeyEvent) -> bool {
|
||||
/// the inventory atomically. Each page requests up to 100 entries.
|
||||
async fn fetch_all_mcp_server_statuses(
|
||||
request_handle: AppServerRequestHandle,
|
||||
detail: McpServerStatusDetail,
|
||||
) -> Result<Vec<McpServerStatus>> {
|
||||
let mut cursor = None;
|
||||
let mut statuses = Vec::new();
|
||||
@@ -6683,7 +6690,7 @@ async fn fetch_all_mcp_server_statuses(
|
||||
params: ListMcpServerStatusParams {
|
||||
cursor: cursor.clone(),
|
||||
limit: Some(100),
|
||||
detail: Some(McpServerStatusDetail::ToolsAndAuthOnly),
|
||||
detail: Some(detail),
|
||||
},
|
||||
})
|
||||
.await
|
||||
@@ -7142,13 +7149,16 @@ mod tests {
|
||||
/*animations_enabled*/ false,
|
||||
)));
|
||||
|
||||
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,
|
||||
}]));
|
||||
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,
|
||||
}]),
|
||||
McpServerStatusDetail::ToolsAndAuthOnly,
|
||||
);
|
||||
|
||||
assert_eq!(app.transcript_cells.len(), 0);
|
||||
}
|
||||
|
||||
@@ -14,6 +14,7 @@ use codex_app_server_protocol::AddCreditsNudgeCreditType;
|
||||
use codex_app_server_protocol::AddCreditsNudgeEmailStatus;
|
||||
use codex_app_server_protocol::AppInfo;
|
||||
use codex_app_server_protocol::McpServerStatus;
|
||||
use codex_app_server_protocol::McpServerStatusDetail;
|
||||
use codex_app_server_protocol::PluginInstallResponse;
|
||||
use codex_app_server_protocol::PluginListResponse;
|
||||
use codex_app_server_protocol::PluginReadParams;
|
||||
@@ -334,11 +335,14 @@ pub(crate) enum AppEvent {
|
||||
PluginInstallAuthAbandon,
|
||||
|
||||
/// Fetch MCP inventory via app-server RPCs and render it into history.
|
||||
FetchMcpInventory,
|
||||
FetchMcpInventory {
|
||||
detail: McpServerStatusDetail,
|
||||
},
|
||||
|
||||
/// Result of fetching MCP inventory via app-server RPCs.
|
||||
McpInventoryLoaded {
|
||||
result: Result<Vec<McpServerStatus>, String>,
|
||||
detail: McpServerStatusDetail,
|
||||
},
|
||||
|
||||
/// Result of the startup skills refresh that runs after the first frame is scheduled.
|
||||
|
||||
@@ -95,6 +95,7 @@ use codex_app_server_protocol::GuardianApprovalReviewAction;
|
||||
use codex_app_server_protocol::ItemCompletedNotification;
|
||||
use codex_app_server_protocol::ItemStartedNotification;
|
||||
use codex_app_server_protocol::McpServerStartupState;
|
||||
use codex_app_server_protocol::McpServerStatusDetail;
|
||||
use codex_app_server_protocol::McpServerStatusUpdatedNotification;
|
||||
use codex_app_server_protocol::ServerNotification;
|
||||
use codex_app_server_protocol::ServerRequest;
|
||||
@@ -10212,7 +10213,7 @@ impl ChatWidget {
|
||||
///
|
||||
/// The spinner lives in `active_cell` and is cleared by
|
||||
/// [`clear_mcp_inventory_loading`] once the result arrives.
|
||||
pub(crate) fn add_mcp_output(&mut self) {
|
||||
pub(crate) fn add_mcp_output(&mut self, detail: McpServerStatusDetail) {
|
||||
self.flush_answer_stream_with_separator();
|
||||
self.flush_active_cell();
|
||||
self.active_cell = Some(Box::new(history_cell::new_mcp_inventory_loading(
|
||||
@@ -10220,7 +10221,8 @@ impl ChatWidget {
|
||||
)));
|
||||
self.bump_active_cell_revision();
|
||||
self.request_redraw();
|
||||
self.app_event_tx.send(AppEvent::FetchMcpInventory);
|
||||
self.app_event_tx
|
||||
.send(AppEvent::FetchMcpInventory { detail });
|
||||
}
|
||||
|
||||
/// Remove the MCP loading spinner if it is still the active cell.
|
||||
|
||||
@@ -359,7 +359,7 @@ impl ChatWidget {
|
||||
self.add_app_server_stub_message("Memory maintenance");
|
||||
}
|
||||
SlashCommand::Mcp => {
|
||||
self.add_mcp_output();
|
||||
self.add_mcp_output(McpServerStatusDetail::ToolsAndAuthOnly);
|
||||
}
|
||||
SlashCommand::Apps => {
|
||||
self.add_connectors_output();
|
||||
@@ -543,6 +543,10 @@ impl ChatWidget {
|
||||
}
|
||||
}
|
||||
}
|
||||
SlashCommand::Mcp => match trimmed.to_ascii_lowercase().as_str() {
|
||||
"verbose" => self.add_mcp_output(McpServerStatusDetail::Full),
|
||||
_ => self.add_error_message("Usage: /mcp [verbose]".to_string()),
|
||||
},
|
||||
SlashCommand::Rename if !trimmed.is_empty() => {
|
||||
if !self.ensure_thread_rename_allowed() {
|
||||
return;
|
||||
|
||||
@@ -73,6 +73,7 @@ pub(super) use codex_app_server_protocol::ItemGuardianApprovalReviewStartedNotif
|
||||
pub(super) use codex_app_server_protocol::ItemStartedNotification;
|
||||
pub(super) use codex_app_server_protocol::MarketplaceInterface;
|
||||
pub(super) use codex_app_server_protocol::McpServerStartupState;
|
||||
pub(super) use codex_app_server_protocol::McpServerStatusDetail;
|
||||
pub(super) use codex_app_server_protocol::McpServerStatusUpdatedNotification;
|
||||
pub(super) use codex_app_server_protocol::PatchApplyStatus as AppServerPatchApplyStatus;
|
||||
pub(super) use codex_app_server_protocol::PatchChangeKind;
|
||||
|
||||
@@ -1111,7 +1111,48 @@ async fn slash_mcp_requests_inventory_via_app_server() {
|
||||
chat.dispatch_command(SlashCommand::Mcp);
|
||||
|
||||
assert!(active_blob(&chat).contains("Loading MCP inventory"));
|
||||
assert_matches!(rx.try_recv(), Ok(AppEvent::FetchMcpInventory));
|
||||
assert_matches!(
|
||||
rx.try_recv(),
|
||||
Ok(AppEvent::FetchMcpInventory {
|
||||
detail: McpServerStatusDetail::ToolsAndAuthOnly
|
||||
})
|
||||
);
|
||||
assert!(op_rx.try_recv().is_err(), "expected no core op to be sent");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn slash_mcp_verbose_requests_full_inventory_via_app_server() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
submit_composer_text(&mut chat, "/mcp verbose");
|
||||
|
||||
assert!(active_blob(&chat).contains("Loading MCP inventory"));
|
||||
assert_matches!(
|
||||
rx.try_recv(),
|
||||
Ok(AppEvent::FetchMcpInventory {
|
||||
detail: McpServerStatusDetail::Full
|
||||
})
|
||||
);
|
||||
assert!(op_rx.try_recv().is_err(), "expected no core op to be sent");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn slash_mcp_invalid_args_show_usage() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
submit_composer_text(&mut chat, "/mcp full");
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
let rendered = cells
|
||||
.iter()
|
||||
.map(|cell| lines_to_single_string(cell))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(
|
||||
rendered.contains("Usage: /mcp [verbose]"),
|
||||
"expected usage message, got: {rendered:?}"
|
||||
);
|
||||
assert_eq!(recall_latest_after_clearing(&mut chat), "/mcp full");
|
||||
assert!(op_rx.try_recv().is_err(), "expected no core op to be sent");
|
||||
}
|
||||
|
||||
|
||||
@@ -2097,6 +2097,18 @@ pub(crate) fn new_mcp_tools_output_from_statuses(
|
||||
let header: Vec<Span<'static>> = vec![" • ".into(), server.clone().into()];
|
||||
|
||||
lines.push(header.into());
|
||||
if matches!(detail, McpServerStatusDetail::Full) {
|
||||
let enabled = cfg.map(|cfg| cfg.enabled).unwrap_or(true);
|
||||
let status_text = if enabled {
|
||||
"enabled".green()
|
||||
} else {
|
||||
"disabled".red()
|
||||
};
|
||||
lines.push(vec![" • Status: ".into(), status_text].into());
|
||||
if let Some(reason) = cfg.and_then(|cfg| cfg.disabled_reason.as_ref()) {
|
||||
lines.push(vec![" • Reason: ".into(), reason.to_string().dim()].into());
|
||||
}
|
||||
}
|
||||
let auth_status = status
|
||||
.map(|status| match status.auth_status {
|
||||
codex_app_server_protocol::McpAuthStatus::Unsupported => McpAuthStatus::Unsupported,
|
||||
@@ -3446,6 +3458,61 @@ mod tests {
|
||||
insta::assert_snapshot!(rendered);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_tools_output_from_statuses_renders_verbose_inventory() {
|
||||
let mut config = test_config().await;
|
||||
let plugin_docs =
|
||||
stdio_server_config("docs-server", vec!["--stdio"], /*env*/ None, vec![]);
|
||||
let servers = HashMap::from([("plugin_docs".to_string(), plugin_docs)]);
|
||||
config
|
||||
.mcp_servers
|
||||
.set(servers)
|
||||
.expect("test mcp servers should accept any configuration");
|
||||
|
||||
let statuses = vec![McpServerStatus {
|
||||
name: "plugin_docs".to_string(),
|
||||
tools: HashMap::from([(
|
||||
"lookup".to_string(),
|
||||
Tool {
|
||||
description: None,
|
||||
name: "lookup".to_string(),
|
||||
title: None,
|
||||
input_schema: serde_json::json!({"type": "object", "properties": {}}),
|
||||
output_schema: None,
|
||||
annotations: None,
|
||||
icons: None,
|
||||
meta: None,
|
||||
},
|
||||
)]),
|
||||
resources: vec![Resource {
|
||||
annotations: None,
|
||||
description: None,
|
||||
mime_type: None,
|
||||
name: "docs".to_string(),
|
||||
size: None,
|
||||
title: Some("Docs".to_string()),
|
||||
uri: "file:///docs".to_string(),
|
||||
icons: None,
|
||||
meta: None,
|
||||
}],
|
||||
resource_templates: vec![ResourceTemplate {
|
||||
annotations: None,
|
||||
uri_template: "file:///docs/{id}".to_string(),
|
||||
name: "doc-template".to_string(),
|
||||
title: Some("Doc Template".to_string()),
|
||||
description: None,
|
||||
mime_type: None,
|
||||
}],
|
||||
auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported,
|
||||
}];
|
||||
|
||||
let cell =
|
||||
new_mcp_tools_output_from_statuses(&config, &statuses, McpServerStatusDetail::Full);
|
||||
let rendered = render_lines(&cell.display_lines(/*width*/ 120)).join("\n");
|
||||
|
||||
insta::assert_snapshot!(rendered);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_agent_message_cell_transcript() {
|
||||
let cell = AgentMessageCell::new(vec![Line::default()], /*is_first_line*/ false);
|
||||
|
||||
@@ -115,7 +115,7 @@ impl SlashCommand {
|
||||
}
|
||||
SlashCommand::Experimental => "toggle experimental features",
|
||||
SlashCommand::Memories => "configure memory use and generation",
|
||||
SlashCommand::Mcp => "list configured MCP tools",
|
||||
SlashCommand::Mcp => "list configured MCP tools; use /mcp verbose for details",
|
||||
SlashCommand::Apps => "manage apps",
|
||||
SlashCommand::Plugins => "browse plugins",
|
||||
SlashCommand::Logout => "log out of Codex",
|
||||
@@ -138,6 +138,7 @@ impl SlashCommand {
|
||||
| SlashCommand::Rename
|
||||
| SlashCommand::Plan
|
||||
| SlashCommand::Fast
|
||||
| SlashCommand::Mcp
|
||||
| SlashCommand::Side
|
||||
| SlashCommand::Resume
|
||||
| SlashCommand::SandboxReadRoot
|
||||
|
||||
+15
@@ -0,0 +1,15 @@
|
||||
---
|
||||
source: tui/src/history_cell.rs
|
||||
expression: rendered
|
||||
---
|
||||
/mcp
|
||||
|
||||
🔌 MCP Tools
|
||||
|
||||
• plugin_docs
|
||||
• Status: enabled
|
||||
• Auth: Unsupported
|
||||
• Command: docs-server --stdio
|
||||
• Tools: lookup
|
||||
• Resources: Docs (file:///docs)
|
||||
• Resource templates: Doc Template (file:///docs/{id})
|
||||
Reference in New Issue
Block a user