Skip to content

fix(mcp): switch tool names from underscore to hyphen separator #237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jpoehnelt wants to merge 4 commits into from
Closed

fix(mcp): switch tool names from underscore to hyphen separator #237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ddd8261
fix(mcp): switch tool names from underscore to hyphen separator
jpoehnelt-bot Mar 6, 2026
4224965
fix: use prefix-matching for service alias in tool name parsing
jpoehnelt-bot Mar 6, 2026
e5c4999
fix: use longest prefix match for service alias disambiguation
jpoehnelt-bot Mar 6, 2026
25d9652
fix: validate tool name has resource and method parts
jpoehnelt-bot Mar 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/mcp-hyphen-tool-names.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@googleworkspace/cli": minor
---

Switch MCP tool names from underscore to hyphen separator (e.g., `drive-files-list` instead of `drive_files_list`). This resolves parsing ambiguity for services/resources with underscores in their names like `admin_reports`. Also fixes the alias mismatch where `tools/list` used Discovery doc names instead of configured service aliases.

**Breaking:** MCP tool names have changed format. Well-behaved clients that discover tools via `tools/list` will pick up new names automatically.
64 changes: 37 additions & 27 deletions src/mcp_server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ async fn build_tools_list(config: &ServerConfig) -> Result<Vec<Value>, GwsError>
let (api_name, version) =
crate::parse_service_and_version(&[svc_name.to_string()], svc_name)?;
if let Ok(doc) = crate::discovery::fetch_discovery_document(&api_name, &version).await {
walk_resources(&doc.name, &doc.resources, &mut tools);
walk_resources(svc_name, &doc.resources, &mut tools);
} else {
eprintln!("[gws mcp] Warning: Failed to load discovery document for service '{}'. It will not be available as a tool.", svc_name);
}
Expand Down Expand Up @@ -327,7 +327,7 @@ async fn build_compact_tools_list(config: &ServerConfig) -> Result<Vec<Value>, G

// Add gws_discover meta-tool
tools.push(json!({
"name": "gws_discover",
"name": "gws-discover",
"description": "Query available resources, methods, and parameter schemas for any enabled service. Call with service only to list resources; add resource to list methods; add method to get full parameter schema.",
"inputSchema": {
"type": "object",
Expand Down Expand Up @@ -359,7 +359,7 @@ async fn build_compact_tools_list(config: &ServerConfig) -> Result<Vec<Value>, G

fn append_workflow_tools(tools: &mut Vec<Value>) {
tools.push(json!({
"name": "workflow_standup_report",
"name": "workflow-standup-report",
"description": "Today's meetings + open tasks as a standup summary",
"inputSchema": {
"type": "object",
Expand All @@ -369,7 +369,7 @@ fn append_workflow_tools(tools: &mut Vec<Value>) {
}
}));
tools.push(json!({
"name": "workflow_meeting_prep",
"name": "workflow-meeting-prep",
"description": "Prepare for your next meeting: agenda, attendees, and linked docs",
"inputSchema": {
"type": "object",
Expand All @@ -379,7 +379,7 @@ fn append_workflow_tools(tools: &mut Vec<Value>) {
}
}));
tools.push(json!({
"name": "workflow_email_to_task",
"name": "workflow-email-to-task",
"description": "Convert a Gmail message into a Google Tasks entry",
"inputSchema": {
"type": "object",
Expand All @@ -391,7 +391,7 @@ fn append_workflow_tools(tools: &mut Vec<Value>) {
}
}));
tools.push(json!({
"name": "workflow_weekly_digest",
"name": "workflow-weekly-digest",
"description": "Weekly summary: this week's meetings + unread email count",
"inputSchema": {
"type": "object",
Expand All @@ -401,7 +401,7 @@ fn append_workflow_tools(tools: &mut Vec<Value>) {
}
}));
tools.push(json!({
"name": "workflow_file_announce",
"name": "workflow-file-announce",
"description": "Announce a Drive file in a Chat space",
"inputSchema": {
"type": "object",
Expand All @@ -417,10 +417,10 @@ fn append_workflow_tools(tools: &mut Vec<Value>) {

fn walk_resources(prefix: &str, resources: &HashMap<String, RestResource>, tools: &mut Vec<Value>) {
for (res_name, res) in resources {
let new_prefix = format!("{}_{}", prefix, res_name);
let new_prefix = format!("{}-{}", prefix, res_name);

for (method_name, method) in &res.methods {
let tool_name = format!("{}_{}", new_prefix, method_name);
let tool_name = format!("{}-{}", new_prefix, method_name);
let mut description = method.description.clone().unwrap_or_default();
if description.is_empty() {
description = format!("Execute the {} Google API method", tool_name);
Expand Down Expand Up @@ -664,13 +664,13 @@ async fn handle_tools_call(params: &Value, config: &ServerConfig) -> Result<Valu
let default_args = json!({});
let arguments = params.get("arguments").unwrap_or(&default_args);

if tool_name.starts_with("workflow_") {
if tool_name.starts_with("workflow-") {
return Err(GwsError::Other(anyhow::anyhow!(
"Workflows are not yet fully implemented via MCP"
)));
}

if tool_name == "gws_discover" {
if tool_name == "gws-discover" {
return handle_discover(arguments, config).await;
}

Expand Down Expand Up @@ -714,21 +714,31 @@ async fn handle_tools_call(params: &Value, config: &ServerConfig) -> Result<Valu
return execute_mcp_method(&doc, method, arguments).await;
}

// Full mode: tool_name encodes service_resource_method (e.g., drive_files_list)
let parts: Vec<&str> = tool_name.split('_').collect();
if parts.len() < 3 {
return Err(GwsError::Validation(format!(
"Invalid API tool name: {}",
// Full mode: tool_name encodes service-resource-method (e.g., drive-files-list)
// Find the enabled service that is a prefix of the tool name.
// This correctly handles hyphenated aliases like "admin-reports".
let (svc_alias, rest) = config
.services
.iter()
.filter_map(|s| {
tool_name
)));
}

let svc_alias = parts[0];
.strip_prefix(s.as_str())
.and_then(|r| r.strip_prefix('-'))
.map(|remainder| (s.as_str(), remainder))
})
.max_by_key(|(s, _)| s.len())
.ok_or_else(|| {
GwsError::Validation(format!(
"Could not determine service from tool name '{}'. No enabled service is a prefix.",
tool_name
))
})?;
Comment on lines +720 to +735
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 6, 2026

Choose a reason for hiding this comment

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

high

The current implementation uses find_map to identify the service from the tool name. This can lead to incorrect parsing if one enabled service alias is a prefix of another (e.g., admin and admin-reports). find_map will return the first match, which may not be the longest (most specific) one.

For example, with services admin and admin-reports enabled, a tool call for admin-reports-activities-list would be incorrectly attributed to the admin service.

To fix this, you should find the longest matching prefix instead of the first one. Using filter_map followed by max_by_key(|(s, _)| s.len()) will ensure the most specific service alias is always chosen.

Suggested change
let (svc_alias, rest) = config
.services
.iter()
.find_map(|s| {
tool_name
)));
}
let svc_alias = parts[0];
.strip_prefix(s.as_str())
.and_then(|r| r.strip_prefix('-'))
.map(|remainder| (s.as_str(), remainder))
})
.ok_or_else(|| {
GwsError::Validation(format!(
"Could not determine service from tool name '{}'. No enabled service is a prefix.",
tool_name
))
})?;
let (svc_alias, rest) = config
.services
.iter()
.filter_map(|s| {
tool_name
.strip_prefix(s.as_str())
.and_then(|r| r.strip_prefix('-'))
.map(|remainder| (s.as_str(), remainder))
})
.max_by_key(|(s, _)| s.len())
.ok_or_else(|| {
GwsError::Validation(format!(
"Could not determine service from tool name '{}'. No enabled service is a prefix.",
tool_name
))
})?;


if !config.services.contains(&svc_alias.to_string()) {
let parts: Vec<&str> = rest.split('-').collect();
if parts.len() < 2 || parts.iter().any(|p| p.is_empty()) {
return Err(GwsError::Validation(format!(
"Service '{}' is not enabled in this MCP session",
svc_alias
"Invalid API tool name: '{}'. Expected format: <service>-<resource...>-<method>",
tool_name
)));
}

Expand All @@ -739,8 +749,8 @@ async fn handle_tools_call(params: &Value, config: &ServerConfig) -> Result<Valu
let mut current_resources = &doc.resources;
let mut current_res = None;

// Walk: ["drive", "files", "list"] — iterate resource path segments between service and method
for res_name in &parts[1..parts.len() - 1] {
// Walk resource path segments: everything except the last part (which is the method)
for res_name in &parts[..parts.len() - 1] {
if let Some(res) = current_resources.get(*res_name) {
current_res = Some(res);
current_resources = &res.resources;
Expand Down Expand Up @@ -1146,7 +1156,7 @@ mod tests {
let mut tools = Vec::new();
append_workflow_tools(&mut tools);
assert_eq!(tools.len(), 5);
assert_eq!(tools[0]["name"], "workflow_standup_report");
assert_eq!(tools[4]["name"], "workflow_file_announce");
assert_eq!(tools[0]["name"], "workflow-standup-report");
assert_eq!(tools[4]["name"], "workflow-file-announce");
}
}
Loading