-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix: resolve native tool protocol race condition causing 400 errors #9363
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
Merged
+7
−6
Conversation
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
The issue occurred when tool results were being formatted. The code was calling resolveToolProtocol() to determine whether to use native or XML protocol formatting. However, if the API configuration changed between when the tool call was made and when the result was being formatted (e.g., user switched profiles), it would use the wrong protocol format. This caused native protocol tool calls to be formatted with XML-style text headers like '[tool_name] Result:', creating malformed message structures that violated API expectations and resulted in 400 errors. Solution: Instead of recalculating the protocol, we now check for the presence of an ID on the tool call itself: - Native protocol tool calls ALWAYS have an ID (set during parsing) - XML protocol tool calls NEVER have an ID (parsed from XML text) This approach is race-condition-free because the ID is an immutable property of each tool call that definitively indicates which protocol was used to create it. It also gracefully handles provider switches - if a provider doesn't support native tools, the tool calls simply won't have IDs and will be formatted correctly for XML protocol.
Contributor
✅ Review Complete - No New Issues Found Reviewed commit f30d413 which removes a redundant comment. The change is clean and improves code clarity by eliminating duplicate documentation. Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
mrubens
approved these changes
Nov 18, 2025
mini2s
added a commit
to zgsm-ai/costrict
that referenced
this pull request
Nov 19, 2025
Problem
The issue occurred when tool results were being formatted. The code was calling
resolveToolProtocol()to determine whether to use native or XML protocol formatting. However, if the API configuration changed between when the tool call was made and when the result was being formatted (e.g., user switched profiles), it would use the wrong protocol format.This caused native protocol tool calls to be formatted with XML-style text headers like
[tool_name] Result:, creating malformed message structures that violated API expectations and resulted in 400 errors.Solution
Instead of recalculating the protocol, we now check for the presence of an ID on the tool call itself:
This approach is race-condition-free because the ID is an immutable property of each tool call that definitively indicates which protocol was used to create it. It also gracefully handles provider switches - if a provider doesn't support native tools, the tool calls simply won't have IDs and will be formatted correctly for XML protocol.
Changes
presentAssistantMessage.tsto determine protocol by checking for tool call ID presence instead of callingresolveToolProtocol()Testing
Important
Fix race condition in
presentAssistantMessage.tsby determining protocol via tool call ID presence, resolving 400 errors.presentAssistantMessage.ts, determine protocol by checking for tool call ID presence instead of usingresolveToolProtocol().This description was created by
for f30d413. You can customize this summary. It will automatically update as commits are pushed.