fix(converters): skip thinking tag injection when toolResults present#23
Conversation
Fixes jwadow#20 - OpenCode compaction returns 400 Improperly formed request error. Root cause: When the last message contains tool results, Kiro API rejects requests that have both toolResults in userInputMessageContext AND thinking tags in content. The combination causes 400 'Improperly formed request' error. Solution: Check for toolResults before injecting thinking tags. Skip injection when toolResults are present in the current message. Changes: - Reorder build_kiro_payload to build user_input_context before thinking tag injection - Add check to skip thinking tags when toolResults present - Add debug log for skipped injection Tests: - test_skips_thinking_tags_when_tool_results_present - test_injects_thinking_tags_when_no_tool_results
|
Thanks for the PR! 🎉 Before merge, we need a one-time CLA confirmation. Full CLA text: Please reply once with: |
|
I have read the CLA and I accept its terms |
jwadow
left a comment
jwadow
left a comment
There was a problem hiding this comment.
Nice catch! You nailed the root cause - Kiro API really doesn't like thinking tags mixed with toolResults. The fix looks good, minimal changes and does exactly what's needed.
One small thing I noticed - the elif has_tool_results could technically trigger even when role isn't user, so the debug log might be a bit misleading in edge cases. Not a blocker though, I can clean that up later if needed.
Thanks for the PR, merging!
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
2 participants
Summary
Fixes #20 - OpenCode compaction returns 400 "Improperly formed request" error.
Root Cause
When the last message contains tool results:
merge_adjacent_messagesconverts tool messages to user message withtoolResultsinject_thinking_tagsadds<thinking_mode>...tags to contenttoolResultsinuserInputMessageContextAND thinking tags in contentSolution
Check for
toolResultsbefore injecting thinking tags. Skip injection whentoolResultsare present in the current message.Changes
build_kiro_payloadto builduser_input_contextbefore thinking tag injectiontoolResultspresentTests
test_skips_thinking_tags_when_tool_results_present- verifies thinking tags are NOT injected when toolResults presenttest_injects_thinking_tags_when_no_tool_results- verifies normal messages still get thinking tagsVerification