fix(mcp): align tools/list namespace with aliased services#165
fix(mcp): align tools/list namespace with aliased services#165abhinavkale-dev wants to merge 3 commits intogoogleworkspace:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 569af9e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical inconsistency in the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly aligns the tool names generated by tools/list with the aliases expected by tools/call, fixing a key usability issue in MCP sessions with aliased services. The change is simple and effective. The addition of regression tests is excellent, ensuring this issue does not reappear. I've left a couple of minor suggestions in the test code to remove unnecessary unsafe blocks for better code clarity and adherence to Rust's safety principles.
src/mcp_server.rs
Outdated
| unsafe { | ||
| if let Some(old) = &self.old { | ||
| std::env::set_var(self.key, old); | ||
| } else { | ||
| std::env::remove_var(self.key); | ||
| } | ||
| } |
There was a problem hiding this comment.
The unsafe block here is unnecessary. Neither std::env::set_var nor std::env::remove_var are unsafe functions. While modifying environment variables is not thread-safe, the #[serial_test::serial] attribute already ensures these tests run sequentially, mitigating the risk of data races. Using unsafe for operations that are not memory-unsafe can be misleading.
if let Some(old) = &self.old {
std::env::set_var(self.key, old);
} else {
std::env::remove_var(self.key);
}
src/mcp_server.rs
Outdated
| unsafe { | ||
| std::env::set_var(key, value); | ||
| } |
There was a problem hiding this comment.
Similar to the drop implementation, this unsafe block is not necessary because std::env::set_var is a safe function. The #[serial_test::serial] attribute handles the thread-safety concerns for these tests.
std::env::set_var(key, value);1 participant
Fixes #162.
Summary
When
gws mcpis started with aliased services (for example-s events),tools/listpreviously returned Discovery-prefixed names (for exampleworkspaceevents_*) whiletools/callvalidated alias prefixes (for exampleevents_*).This made listed tools uncallable in the same MCP session.
This PR makes
tools/listuse the configured service alias prefix sotools/listandtools/callshare the same namespace.What changed
src/mcp_server.rs:build_tools_listnow prefixes tool names withsvc_name(configured alias) instead ofdoc.name.src/mcp_server.rs:build_tools_list_uses_alias_prefixes_for_aliased_serviceshandle_tools_call_rejects_discovery_prefix_when_alias_is_enabled.changeset/fix-mcp-alias-tools-mismatch.mdChecklist:
AGENTS.mdguidelines (no generatedgoogle-*crates).cargo fmt --allto format the code perfectly.cargo clippy -- -D warningsand resolved all warnings.pnpx changeset) to document my changes.