fix: handle missing tool_call.id in runTools for providers#1946
fix: handle missing tool_call.id in runTools for providers#1946anishesg wants to merge 1 commit into
Conversation
Some providers do not populate the `tool_call.id` field when returning tool calls, or may set it to an empty string. When the SDK's `AbstractChatCompletionRunner._runTools()` method constructs tool result messages to send back to the API, it uses this empty or missing `tool_call_id`, which causes the API to reject the request with a 400 status code. Signed-off-by: anish <anishesg@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e91393f9bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const tool_call_id = tool_call.id; | ||
| const { name, arguments: args } = tool_call.function; | ||
| // Some LLM providers may not populate tool_call.id; use function name as fallback | ||
| const tool_call_id = tool_call.id || name; |
There was a problem hiding this comment.
When a provider omits or empties tool_call.id, this fallback is only used on the generated tool message; the assistant message already stored in this.messages still has tool_calls[].id as undefined/''. On the next _createChatCompletion call the request contains a tool result with tool_call_id set to the function name that does not match the preceding assistant tool call, and finalFunctionToolCallResult() also cannot find the result because it explicitly matches y.id === message.tool_call_id. The missing-id path needs to write a generated/stable id back onto the tool_call before the tool message is added.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Yeah good point, we need to assign the fallback ID back to the original tool_call object so it matches when we look it up later.
There was a problem hiding this comment.
Yeah good point, we need to assign the fallback ID back to the original tool_call object so it matches when we look it up later. Let me fix that.
1 participant
Some providers do not populate the
tool_call.idfield when returning tool calls, or may set it to an empty string. When the SDK'sAbstractChatCompletionRunner._runTools()method constructs tool result messages to send back to the API, it uses this empty or missingtool_call_id, which causes the API to reject the request with a 400 status code.This fix adds a defensive fallback in
AbstractChatCompletionRunner.tsat line 346. Whentool_call.idis missing or empty, the function name is used as thetool_call_idinstead. This ensures that tool result messages always have a valid identifier, preventing the 400 error while maintaining compatibility with all providers.Fixes #1528