fix(tui): implement /mcp inventory for tui_app_server#14931
fix(tui): implement /mcp inventory for tui_app_server#14931etraut-openai merged 7 commits intoopenai:mainfrom
Conversation
Route `/mcp` through app-server inventory RPCs in `tui_app_server` and render the returned server status data in the history view. This removes the app-server TUI fallback stub without changing `app-server` itself. Fetch MCP inventory on a background request handle and post the result back through `AppEvent` so the TUI can repaint immediately. This adds a transient `Loading MCP inventory…` row and avoids the silent delay caused by awaiting the RPC in the main event loop.
Documentation-only pass over the MCP inventory change: adds doc comments to all new public/crate-visible functions and types.
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Implements a fully functional /mcp command in the app-server TUI (remote mode) by asynchronously fetching MCP server inventory from the app-server via the paginated mcpServerStatus/list RPC and rendering the results into chat history (with a transient loading spinner).
Changes:
- Add an async
/mcpflow: insert a loading cell, emitAppEvent::FetchMcpInventory, fetch statuses via paginated RPC, then render results (or an error) into history. - Introduce rendering logic to build MCP inventory output directly from
McpServerStatusresponses (including disabled servers from local config). - Add tests and snapshots covering the loading state, RPC request emission, and rendering behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
codex-rs/tui_app_server/src/history_cell.rs |
Adds new_mcp_tools_output_from_statuses renderer and a McpInventoryLoadingCell spinner cell; adds snapshot tests. |
codex-rs/tui_app_server/src/chatwidget.rs |
Updates /mcp handling to show a loading cell and emit FetchMcpInventory; adds clearing logic for the loading cell. |
codex-rs/tui_app_server/src/app_event.rs |
Adds FetchMcpInventory and McpInventoryLoaded events for the new async lifecycle. |
codex-rs/tui_app_server/src/app.rs |
Implements background RPC fetch (pagination) and result handling to render inventory or errors into history. |
codex-rs/tui_app_server/src/chatwidget/tests.rs |
Adds a test ensuring /mcp triggers the inventory fetch event (and no core op). |
codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_tools_output_from_statuses_renders_status_only_servers.snap |
New snapshot for rendering status-only servers + disabled config server output. |
codex-rs/tui_app_server/src/snapshots/codex_tui_app_server__history_cell__tests__mcp_inventory_loading_snapshot.snap |
New snapshot for the loading spinner cell output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f3194f602
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Derive the app-server TUI `/mcp` inventory from `mcpServerStatus/list` results only, instead of merging in local-only configured servers. This keeps remote inventory output aligned with the connected app-server. Retain local config only as display enrichment for servers returned by the app-server, and update the snapshot coverage to match the new remote-authoritative rendering behavior.
Add a `McpInventoryMaps` alias for the test helper that converts `McpServerStatus` entries into the in-process map layout. This keeps the app-server `/mcp` coverage intact while avoiding the `clippy::type_complexity` failure that broke rust-ci on the follow-up review fix.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ab7c0dd91
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Remove the RPC-backed `/mcp` status row so the app-server TUI no longer claims servers are enabled without authoritative data from `mcpServerStatus/list`. Also clear committed MCP loading cells when the async inventory result arrives, so transient spinner rows do not remain in history after active cell churn.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e3202ee81
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Remove the most recent committed `McpInventoryLoadingCell` from transcript history when `/mcp` finishes loading, instead of only checking the transcript tail. This keeps stale loading rows from surviving when later output was committed before the inventory RPC completed.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b25954b9e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Document the app-server `/mcp` async fetch tradeoff next to the fire-and-forget request path. This records why late completions are currently accepted as a low- severity UI artifact instead of introducing request-token invalidation state in this change.
Problem
The
/mcpcommand did not work in the app-server TUI (remote mode). Onmain,add_mcp_output()calledMcpManager::effective_servers()in-process, which only sees locally configured servers, and then emitted a generic stub message for the app-server to handle. In remote usage, that left/mcpwithout a real inventory view.Solution
Implement
/mcpfor the app-server TUI by fetching MCP server inventory directly from the app-server via the paginatedmcpServerStatus/listRPC and rendering the results into chat history.The command now follows a three-phase lifecycle:
ChatWidget::add_mcp_output()inserts a transientMcpInventoryLoadingCelland emitsAppEvent::FetchMcpInventory. This gives immediate feedback that the command registered.App::fetch_mcp_inventory()spawns a background task that callsfetch_all_mcp_server_statuses()over an app-server request handle. When the RPC completes, it sendsAppEvent::McpInventoryLoaded { result }.App::handle_mcp_inventory_result()clears the loading cell and renders eithernew_mcp_tools_output_from_statuses(...)or an error message.This keeps the main app event loop responsive, so the TUI can repaint before the remote RPC finishes.
Notes
app-serverchanges were required.enabledordisabled_reasonstate for MCP servers, so the remote/mcpview no longer renders aStatus:row rather than guessing from local config.Failed to load MCP inventory: ....Tests
slash_mcp_requests_inventory_via_app_servermcp_inventory_maps_prefix_tool_names_by_serverhandle_mcp_inventory_result_clears_committed_loading_cellmcp_tools_output_from_statuses_renders_status_only_serversmcp_inventory_loading_snapshot