fix: hashing long namespace flatten name#135
Conversation
There was a problem hiding this comment.
the generated shortened name needs to check against MCP registry keys.
typed_top_level_tool_names excludes ResponsesTool::Mcp, but ToolRegistry later inserts MCP entries under p.name.
If a later MCP declaration has a p.name matching the shortened name, it overwrites the client-owned namespace entry and the model’s call is dispatched as gateway-owned MCP work.
Before this change the long name was rejected upstream; shortening makes this misrouting path reachable.
Please reject collisions across all model-visible registry keys and add a regression test with a long namespace member followed by a matching MCP declaration.
There was a problem hiding this comment.
@haoshan98 Thanks for the fix can you fix the merge conflict please. LGTM.
maralbahari
left a comment
There was a problem hiding this comment.
@haoshan98 will merge after addressing @franciscojavierarceo comment and merge conflict
Signed-off-by: haoshan98 <haoshanw@gmail.com>
Signed-off-by: haoshan98 <haoshanw@gmail.com>
b8c935f to
902c073
Compare
Signed-off-by: haoshan98 <haoshanw@gmail.com>
| } | ||
|
|
||
| #[derive(Clone, Copy, Debug, Eq, PartialEq)] | ||
| enum TopLevelRegistryToolKind { |
There was a problem hiding this comment.
this Enum sounds the same to the ResponsesTool. the ResponsesTool has tool_type function see if you can reuse that?
There was a problem hiding this comment.
i thought we wanted more separation between Messages and Responses not less?
There was a problem hiding this comment.
@franciscojavierarceo yes there will be more separation my request is simply that we do have ToolType enum and it is used in ResponsesToo for mapping between tool and tool_type also used in tool_registry. so the ToolType enum as one single ground truth of what is supported on gateway rather than having to maintain multiple enums and might forget to update them. the ResponseTool is only used for responses api for now. on messages api as off now based on #131 PR merged is maintaining separate contract and a tool build registry that is plugin web search only as off now.
Signed-off-by: haoshan98 <haoshanw@gmail.com>
tool_choicevalues and restore function calls to their public{ namespace, name }representation.mcpbuilt-in tool, a normalized built-in tool, or another namespace member. Validation runs before upstream inference or MCP handler construction.tool_choice, restoration, WebSocket round-trip, and MCP collision regression coverage, and document the bounded model-visible format and collision behavior.