fix(bedrock): sanitize tool names to meet Bedrock API constraints#1784
Closed
AnantKumar17 wants to merge 1 commit intokagent-dev:mainfrom
Closed
fix(bedrock): sanitize tool names to meet Bedrock API constraints#1784AnantKumar17 wants to merge 1 commit intokagent-dev:mainfrom
AnantKumar17 wants to merge 1 commit intokagent-dev:mainfrom
Conversation
MCP servers can produce tool names containing dots, spaces, or other characters that Bedrock rejects. The Converse API requires tool names to match [a-zA-Z0-9_-]+ but the previous code passed names through unchanged, causing ValidationException errors at runtime. This change adds _sanitize_tool_name, which replaces any invalid character with an underscore and falls back to a generated name when the input is empty. A name_map shared across both _convert_tools_to_converse and _convert_content_to_converse_messages ensures the same original name always maps to the same sanitized name within a single request. A reverse_name_map is then used to restore original names in Bedrock responses before they are returned to the ADK framework, so the rest of the agent runtime sees the names it originally registered. A warning is logged whenever a name is sanitized to aid debugging. Fixes kagent-dev#1473 Signed-off-by: AnantKumar17 <anant.kumar17@example.com>
AnantKumar17
requested review from
EItanya,
iplay88keys,
jmhbh,
peterj,
supreme-gg-gg and
yuval-k
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Bedrock Converse adapter to sanitize MCP tool names into Bedrock-compatible identifiers and to round-trip tool names by restoring the original names in Bedrock responses.
Changes:
- Add regexes +
_sanitize_tool_nameto enforce Bedrock tool name constraints ([a-zA-Z0-9_-]+). - Sanitize tool names in both outgoing tool declarations and message history (function calls).
- Track a shared name mapping per request and restore original tool names in both streaming and non-streaming Bedrock responses.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
python/packages/kagent-adk/src/kagent/adk/models/_bedrock.py |
Introduces tool-name sanitization and response name restoration via shared per-request maps. |
python/packages/kagent-adk/tests/unittests/models/test_bedrock.py |
Adds unit + integration coverage for tool name sanitization and round-trip restoration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sanitized = _BEDROCK_TOOL_NAME_INVALID.sub("_", name) | ||
| if not sanitized or not _BEDROCK_TOOL_NAME_VALID.match(sanitized): | ||
| counter[0] += 1 | ||
| sanitized = f"unknown_tool_{counter[0]}" |
Comment on lines
+60
to
+76
| def _sanitize_tool_name(name: str, name_map: dict[str, str], counter: list[int]) -> str: | ||
| """Return a valid Bedrock tool name. | ||
|
|
||
| Bedrock requires tool names to match [a-zA-Z0-9_-]+ and be non-empty. | ||
| name_map caches original->sanitized so the same tool name is consistently | ||
| mapped throughout a single request. counter is a single-element list used | ||
| as a mutable integer for generating unique fallback names. | ||
|
|
||
| See https://github.com/kagent-dev/kagent/issues/1473 | ||
| """ | ||
| if name in name_map: | ||
| return name_map[name] | ||
| sanitized = _BEDROCK_TOOL_NAME_INVALID.sub("_", name) | ||
| if not sanitized or not _BEDROCK_TOOL_NAME_VALID.match(sanitized): | ||
| counter[0] += 1 | ||
| sanitized = f"unknown_tool_{counter[0]}" | ||
| return sanitized |
Comment on lines
+145
to
+148
| result = _sanitize_tool_name("", name_map, counter) | ||
| assert result == "unknown_tool_1" | ||
| assert counter[0] == 1 | ||
|
|
Contributor
supreme-gg-gg
left a comment
supreme-gg-gg
left a comment
There was a problem hiding this comment.
Duplicate of #1730
Author
|
Closing this in favour of #1730, which I noticed is already in review and covers the same ground. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
MCP servers can produce tool names containing dots, spaces, or other characters that Bedrock rejects. The Converse API requires tool names to match
[a-zA-Z0-9_-]+, but the previous code passed names through unchanged, which causedValidationExceptionerrors at runtime.This PR adds
_sanitize_tool_name, which replaces invalid characters with underscores and falls back to a generated name for empty inputs. Aname_mapis shared across_convert_tools_to_converseand_convert_content_to_converse_messagesso the same original name always maps to the same sanitized name within a single request. Areverse_name_mapis built from it and used to restore original names in Bedrock responses, so the rest of the agent runtime sees the names it registered.A
WARNINGlog is emitted whenever a name is sanitized to help with debugging.Closes #1473
Changes
python/packages/kagent-adk/src/kagent/adk/models/_bedrock.py_BEDROCK_TOOL_NAME_VALIDand_BEDROCK_TOOL_NAME_INVALIDregex constants_sanitize_tool_namefunction (follows the same pattern as the existing_sanitize_tool_id)_convert_content_to_converse_messagesto sanitizefunction_call.namein history_convert_tools_to_converseto sanitize tool declaration namesgenerate_content_async, built a sharedname_map/name_counterand areverse_name_mapto restore names in both streaming and non-streaming responsespython/packages/kagent-adk/tests/unittests/models/test_bedrock.pyTestSanitizeToolName: unit tests for the new functionTestConvertToolsToConverse: verifies dotted names are sanitized in the tool specTestBedrockToolNameRoundTrip: integration tests confirming original names are restored in both non-streaming and streaming responsesTesting
All 264 existing unit tests continue to pass. 12 new tests cover the added behaviour.