-
Notifications
You must be signed in to change notification settings - Fork 3.6k
fix: reduce MCP tool name delimiter to prevent OpenAI 64-char limit errors #1371
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.
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
base: main
Are you sure you want to change the base?
Conversation
…rrors - Change delimiter from "__OAI_CODEX_MCP__" (17 chars) to "__" (2 chars) - Add validation and truncation logic for tool names exceeding 64 chars - Skip tools when server name alone is too long - Add hash suffix for uniqueness when truncating tool names - Add comprehensive tests for new functionality Fixes openai#1289 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
@@ -193,7 +198,46 @@ pub async fn list_all_tools( | |||
|
|||
for tool in list_result.tools { | |||
// TODO(mbolin): escape tool names that contain invalid characters. | |||
let fq_name = fully_qualified_tool_name(&server_name, &tool.name); | |||
let mut fq_name = fully_qualified_tool_name(&server_name, &tool.name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fully_qualified_tool_name()
and try_parse_fully_qualified_tool_name()
must be symmetric. It is not clear that this is the case given this implementation.
Someone told me that, empirically, the model doesn't care about the names of the functions all that much and therefore, we could SHA1 the long name or something and things would still work.
Another solution that is somewhat stateful, but more readable for users, would be to get the full list of tool names and only attempt to "fully qualify them" when there is a naming collision.
Can this be shipped please? we need it :) |
Sorry for the delay, I'll take a look at this today |
Fixes issue where MCP tool names exceed OpenAI's 64-character limit by using a shorter delimiter.
Fixes #1289