Skip to content

fix(mcp): #2744 preserve underscored names in displays#2774

Merged
Hmbown merged 1 commit into
codex/v0.9.0-stewardshipfrom
codex/harvest-2744-mcp-display
Jun 5, 2026
Merged

fix(mcp): #2744 preserve underscored names in displays#2774
Hmbown merged 1 commit into
codex/v0.9.0-stewardshipfrom
codex/harvest-2744-mcp-display

Conversation

@Hmbown

@Hmbown Hmbown commented Jun 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • Follows up the MCP tool name parsing breaks when server name contains underscores #2744 MCP underscore harvest by aligning display/metadata paths with the fixed routing parser.
  • Runtime API /v1/apps/mcp/tools now reuses McpPool::parse_prefixed_name, so underscored server names are listed and filtered correctly.
  • Approval summaries now show the full MCP target route instead of guessing a server from the first _, which avoids misleading Server: my copy for mcp_my_db_execute_sql.

Contributor credit

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
  • python3 scripts/check-coauthor-trailers.py --author-map .github/AUTHOR_MAP --range origin/codex/v0.9.0-stewardship..HEAD --check-authors

Release boundary

No tag, publish, GitHub Release, version bump, or release artifact push was performed.

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>

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmbown has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@Hmbown Hmbown merged commit b1d4d74 into codex/v0.9.0-stewardship Jun 5, 2026
2 checks passed
@Hmbown Hmbown deleted the codex/harvest-2744-mcp-display branch June 5, 2026 03:56

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +347 to 354
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())
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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)
}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant