fix(mcp): #2744 preserve underscored names in displays#2774
Conversation
Follow up the #2744 MCP routing harvest by reusing the registered-server parser in the runtime API tool listing path and by making approval summaries show the full MCP target route instead of a guessed first underscore segment. This keeps tool-call routing, runtime metadata, and approval copy aligned for servers such as my_db while avoiding an impossible server-only guess in approval cards that do not have the live MCP registry. Refs #2744 Verification: cargo fmt --all -- --check; git diff --check; ./scripts/release/check-versions.sh; cargo test -p codewhale-tui --bin codewhale-tui --locked underscored -- --nocapture; cargo test -p codewhale-tui --bin codewhale-tui --locked mcp_pool_call_tool -- --nocapture; cargo clippy -p codewhale-tui --bin codewhale-tui --locked -- -D warnings. Co-authored-by: cyq1017 <61975706+cyq1017@users.noreply.github.com> Co-authored-by: puneetdixit200 <236133619+puneetdixit200@users.noreply.github.com>
There was a problem hiding this comment.
Hmbown has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
Code Review
This pull request ensures that underscored MCP server names are no longer incorrectly split at the first underscore in tool listings and approval summaries. It updates the list endpoint to reuse the existing parse_prefixed_name parser and modifies approval cards to display the full MCP target route. A review comment suggests optimizing mcp_target_hint by returning Option<&str> instead of Option<String> to avoid unnecessary heap allocations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| fn mcp_target_hint(tool_name: &str) -> Option<String> { | ||
| let remainder = tool_name.strip_prefix("mcp_")?; | ||
| let (server, _) = remainder.split_once('_')?; | ||
| if server.is_empty() { | ||
| if remainder.is_empty() { | ||
| None | ||
| } else { | ||
| Some(server.to_string()) | ||
| Some(remainder.to_string()) | ||
| } | ||
| } |
There was a problem hiding this comment.
The mcp_target_hint function currently returns an owned String by calling to_string(). Since the returned hint is only used for formatting impact summary strings (which accept &str), we can avoid this unnecessary heap allocation by returning Option<&str> instead.
| fn mcp_target_hint(tool_name: &str) -> Option<String> { | |
| let remainder = tool_name.strip_prefix("mcp_")?; | |
| let (server, _) = remainder.split_once('_')?; | |
| if server.is_empty() { | |
| if remainder.is_empty() { | |
| None | |
| } else { | |
| Some(server.to_string()) | |
| Some(remainder.to_string()) | |
| } | |
| } | |
| fn mcp_target_hint(tool_name: &str) -> Option<&str> { | |
| let remainder = tool_name.strip_prefix("mcp_")?; | |
| if remainder.is_empty() { | |
| None | |
| } else { | |
| Some(remainder) | |
| } | |
| } |
Summary
/v1/apps/mcp/toolsnow reusesMcpPool::parse_prefixed_name, so underscored server names are listed and filtered correctly._, which avoids misleadingServer: mycopy formcp_my_db_execute_sql.Contributor credit
Verification
cargo fmt --all -- --checkgit diff --check./scripts/release/check-versions.shcargo test -p codewhale-tui --bin codewhale-tui --locked underscored -- --nocapturecargo test -p codewhale-tui --bin codewhale-tui --locked mcp_pool_call_tool -- --nocapturecargo clippy -p codewhale-tui --bin codewhale-tui --locked -- -D warningspython3 scripts/check-coauthor-trailers.py --author-map .github/AUTHOR_MAP --range origin/codex/v0.9.0-stewardship..HEAD --check-authorsRelease boundary
No tag, publish, GitHub Release, version bump, or release artifact push was performed.