Repository navigation
Conversation
Converse validates every toolUse.name against [a-zA-Z0-9_-]{1,64}, and it
revalidates the whole message history on each request. So a single malformed
name recorded in a past assistant turn makes every later request in that
session fail with a 400 ValidationException -- a plain "hi" included -- and
the session stays broken until it is abandoned (NousResearch#90008).
Normalize the name only where a past tool call is replayed onto the wire, and
deliberately not in convert_tools_to_converse. A replayed name has no reader
left: the call already carries its toolResult, and Converse pairs those by
toolUseId rather than by name. A toolSpec name is not interchangeable that
way, because the model would call back under the folded name and
tools/registry.py resolves tools by exact name.
Measured against zai.glm-4.7-flash in us-east-2 through this module:
- pattern violation reports at messages.N.member.content.M.member.toolUse.name
(RequestId c7ba8744-619a-49e5-90c1-3321ebd3465a)
- the cap reports "length less than or equal to 64"
(RequestId 57594bc1-3671-4ef6-8193-b0e25ef360a9)
- the hyphen is accepted, so it is kept rather than folded
(RequestId b837dffa-02ee-4f7a-889e-7134fef982cf)
- a folded name present in no toolConfig.tools entry is accepted and the
turn completes (RequestId 85adb3e8-971b-490c-8e78-e31881e415e4)
1 task
This branch has not been deployed
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.
Summary
Fixes #90008. Converse validates
toolUse.nameagainst[a-zA-Z0-9_-]{1,64}and revalidates the whole message history on every request, so one malformed name recorded in a past assistant turn makes every later request in that session fail with a 400 — a plainhiincluded. The session cannot recover; today the only way out is to abandon it.This coerces the name only where a past tool call is replayed onto the wire, at the two outbound sites in
convert_messages_to_converse(thetool_callspath in_assistant_blocks, and the orderedbedrock_content_blockssidecar path in_replay_ordered_blocks).convert_tools_to_converseis left alone on purpose — see Scope below.agent/bedrock_adapter.py+32/-2,tests/agent/test_bedrock_adapter.py+107. Based onmain2ef41d2b58.Live before/after
Same history, same model (
zai.glm-4.7-flash,us-east-2), driven through this module's ownconvert_messages_to_converse. The recorded name is the one from the report,brain</arg_value>; the last message is a plainhi:toolUse.namemain2ef41d2b58'brain</arg_value>'messages.2.member.content.2.member.toolUse.name— RequestIdb615b12a-d85c-46ec-8e19-0151a29e0a65'brain__arg_value_'stopReason=end_turn, model answers thehi— RequestId10393a11-7688-4363-acfe-ffe91da40728That also removes the
needs-reprolabel's question: the report reproduces exactly as written.Four measured facts that shaped the fix
All against
zai.glm-4.7-flashinus-east-2:messages[].content[].toolUse.name, not only ontoolConfig— RequestIdc7ba8744-619a-49e5-90c1-3321ebd3465a.length less than or equal to 64) — RequestId57594bc1-3671-4ef6-8193-b0e25ef360a9.brain-v2is replayed verbatim rather than folded — RequestIdb837dffa-02ee-4f7a-889e-7134fef982cf. A test pins this, because folding it would rename working tools for no reason.toolUse.namethat appears in notoolConfig.toolsentry is accepted and the turn completes normally — RequestIds85adb3e8-971b-490c-8e78-e31881e415e4ande7e0d12f-aaa7-48a5-8abd-a586b72aae3b. This is the fact the whole approach rests on:toolResultpairs totoolUsebytoolUseId, not by name, so a replayed name has no reader left and can be rewritten safely. In the second of those runs the model's next call came back as{"name": "brain", ...}— under thetoolSpecname, not the folded one in the history, which is the measured reason the two sides are not symmetric.Scope: why
toolConfigis not touchedA
toolSpecname is not interchangeable the same way. The model calls back under whatever name the spec advertises, andtools/registry.pyresolves by exact name (tool_error(f"Unknown tool: {name}")), so folding a spec name needs a reverse mapping to survive the round trip. That is a different change with a different failure mode, and it is what #54881 builds (below). A test assertsconvert_tools_to_converseis unchanged, so a later change has to state that it is changing it.Honest trade-off
If a historical
read.filefolds toread_fileand a real tool of that name exists, the replayed history shows a call that never happened under that name. That is semantically imprecise. It is also strictly better than a session that 400s forever, and the imprecision is confined to history a model only reads for context — thetoolUseIdpairing, the arguments and the result text are all untouched. Names are still recorded verbatim bynormalize_converse_response(asserted in a test); the folding happens at the wire boundary only.Relationship to #90016 and #54881
Both are the same code area and both are currently
mergeable=false / dirty, so neither lands as-is; I checked before writing this.tools/registry.py(it foldstoolSpecnames without a reverse mapping). If the maintainers prefer that PR rebased instead, that is fine by me.toolSpecside and the inbound restore on both the streaming and non-streaming paths. It is complementary, not duplicate:_bedrock_wire_tool_namereturns early for any name<= 64chars, sobrain</arg_value>(17 chars) passes through it untouched and still 400s. Neithermainnor any merged commit has length or character handling here — MCP tool name exceeds AWS Bedrock 64-character limit #54863 was closed asduplicateby the Wave: stale duplicates resolved by 2026-09-11→18 landings (11 rows) #115413 stale-duplicate wave, but no fix landed._assistant_blocksand will conflict textually. The composition I would suggest is character-fold first, then alias for length; happy to rebase onto whichever lands first.Tests
tests/agent/test_bedrock_adapter.py::TestReplayedToolNameConformsToConverse, 18 node ids. They cover the reported name on both replay paths, hyphen/underscore names replayed verbatim, the 64-char truncation,["", None, " ", "<|>", 12]all yielding a name the API accepts,toolResultstill pairing bytoolUseId,convert_tools_to_converseunchanged, inbound names recorded verbatim, and a mixed history where only the bad entry changes.Full file on this branch: 97 passed, 2 failed. The 2 failures are
TestAgentBedrockStreamRecovery::test_streaming_denial_falls_back_to_converse_via_bedrock_streamand::test_stale_connection_evicts_client_on_agent_stream_path, bothModuleNotFoundError: No module named 'psutil'on my machine, and both failing identically on cleanmain.Repo-wide, on the EC2 box with the full venv (82 test files touching
agent/,tests/agent/,tests/e2e/core/providers/and the Bedrock-adjacent CLI paths):The +18 passing is exactly this PR's new tests. The 3 shared failures (
test_provider_catalog.py::test_api_key_providers_expose_a_credential_env_var,test_local_env_blocklist.py::test_runtime_provenance_is_independent_of_aliases_and_virtual_env[True],test_tui_gateway_server.py::test_load_cfg_raw_sees_replacement_with_pinned_mtime_and_size) are pre-existing onmain. Separately,tests/acp_adapter/test_named_provider_catalogs.pyfails at collection on cleanmain(ModuleNotFoundError: No module named 'acp') and aborts the whole run, so it is excluded from both sides of that comparison; it is unrelated to this change.🤖 Generated with Claude Code