-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix(google): Fix 400 Bad Request error with Gemini 3 thought signatures #6035
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
Conversation
9ceacd9 to
9b0f1ec
Compare
|
thanks @rabi for diving on this. this is a good start, but I am not sure it does everything yet. the spec says that gemini can reply with thinking messages and that we then have to keep the conversation going. other things to note: shouldn't we just always have an optional thought signature on toolcall/response messages? the testing seems rather convoluted, I'd rather do one big round-trip thing where we construct a conversation in our native format and, convert it to google, have some asserts and then convert it back and then verify that it is the same. that would be then a parttern we can easily copy into other format checkers (since they need to be cross provider compatible) let me know if you want to continue working on this, I do really appreciate the effort and gemini has been annoying in how they change their api without telling us |
|
here's the related bug report: #6091 |
9b0f1ec to
e4e6d2e
Compare
Thanks @DOsinga for reviewing. I think I've addressed your comments in the last push. Let me know. |
|
FWIW I did a skim through this PR and it looks sane, though I am not an expert in some of the core tool calling APIs. Having a generic extensible metadata looks like the right thing to do for sure. It also does fix the problem for me. |
DOsinga
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like, looks clean - if you could remove the more obvious comments, I'd say it is good to go
alexhancock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I concur with @DOsinga let's remove the comments which aren't adding much to what the code says, then get this in
Gemini 3 models require thought_signature in tool calls and responses, and thinking content must be echoed back in the conversation loop. Changes: - Replace thought_signature field with generic metadata field on ToolRequest/ToolResponse - Add ProviderMetadata type alias for provider-specific data - Google provider stores thoughtSignature in metadata - Helper functions for Google-specific metadata extraction - Preserve thinking content in conversation when tool calls are processed - Handle signature inheritance for multi-part responses - Text-only responses treated as final answer (not thinking) This design keeps provider-specific details out of the core model. Assisted-by: Claude 4.5 Opus Signed-off-by: rabi <ramishra@redhat.com>
e4e6d2e to
fd32563
Compare
Done! |
DOsinga
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. thanks and sorry for taking so long. Did you check that this is backwards compatible?
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub thought_signature: Option<String>, | ||
| #[schema(value_type = Object)] | ||
| pub metadata: Option<ProviderMetadata>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this break compatibility with sessions where we've already written out the thought_signature?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it does not. I tested resuming session with builds before and after the PR and it worked fine. I also tested with older gemini models like gemini-2.0-flash and did not see any issues. Also, I notice that the thought_signature is a recent addition cfdf015.
|
excellent, thank you so much! |
* origin/main: (57 commits) docs: create/edit recipe button (#6145) fix(google): Fix 400 Bad Request error with Gemini 3 thought signatures (#6035) fix: we don't need to warn about tool count when in code mode (#6149) deps: upgrade agent-client-protocol to 0.9.0 (#6109) fix(providers): fix for gemini-cli on windows to work around cmd's multiline prompt limitations #5911 (#5966) More slash commands (#5858) fix: MCP UI not rendering due to CallToolResult structure change (#6143) fix: display shell output as static text instead of spinner (#6041) fix : Custom providers with empty API keys show as configured in desktop (#6105) Add .agents/skills and ~/.config/agent/skills to skills discovery paths (#6139) fix: use instructions for system prompt and prompt for user message in subagents (#6121) Fix compaction loop for small models or large input (#5803) feat: Centralize theme management with ThemeContext (#6137) OpenRouter & Xai streaming (#5873) fix: resolve mcp-hermit cleanup path expansion issue (#5953) feat: add goose PR reviewer workflow (#6124) perf: Avoid repeated MCP queries during streaming responses (#6138) Fix YAML serialization for recipes with special characters (#5796) Add more posthog analytics (privacy aware) (#6122) docs: add Sugar MCP server to extensions registry (#6077) ...
…icing * 'main' of github.com:block/goose: (35 commits) docs: skills (#6062) fix: add conditional configuration for GOOSE_BIN_DIR in PATH (#5940) Update dependencies to help in Fedora packaging (#5835) fix: make goose reviewer less bad (#6154) docs: create/edit recipe button (#6145) fix(google): Fix 400 Bad Request error with Gemini 3 thought signatures (#6035) fix: we don't need to warn about tool count when in code mode (#6149) deps: upgrade agent-client-protocol to 0.9.0 (#6109) fix(providers): fix for gemini-cli on windows to work around cmd's multiline prompt limitations #5911 (#5966) More slash commands (#5858) fix: MCP UI not rendering due to CallToolResult structure change (#6143) fix: display shell output as static text instead of spinner (#6041) fix : Custom providers with empty API keys show as configured in desktop (#6105) Add .agents/skills and ~/.config/agent/skills to skills discovery paths (#6139) fix: use instructions for system prompt and prompt for user message in subagents (#6121) Fix compaction loop for small models or large input (#5803) feat: Centralize theme management with ThemeContext (#6137) OpenRouter & Xai streaming (#5873) fix: resolve mcp-hermit cleanup path expansion issue (#5953) feat: add goose PR reviewer workflow (#6124) ...
* 'main' of github.com:block/goose: (28 commits) Clean PR preview sites from gh-pages branch history (#6161) fix: make goose reviewer less sycophantic (#6171) revert /reply to previous behavior (replacing session history) when full conversation provided (#6058) chore: manually update version (#6166) Integrate pricing with canonical model (#6130) Regenerate canonical models when release branch is created. (#6127) fix: use correct parameter name in read_module handler (#6148) docs: blog for code mode MCP (#6126) test: add ACP integration test (#6150) docs: auto download updates (#6163) fix: respect default_enabled value of platform extensions (#6159) docs: skills (#6062) fix: add conditional configuration for GOOSE_BIN_DIR in PATH (#5940) Update dependencies to help in Fedora packaging (#5835) fix: make goose reviewer less bad (#6154) docs: create/edit recipe button (#6145) fix(google): Fix 400 Bad Request error with Gemini 3 thought signatures (#6035) fix: we don't need to warn about tool count when in code mode (#6149) deps: upgrade agent-client-protocol to 0.9.0 (#6109) fix(providers): fix for gemini-cli on windows to work around cmd's multiline prompt limitations #5911 (#5966) ... # Conflicts: # ui/desktop/src/api/sdk.gen.ts # ui/desktop/src/hooks/useAgent.ts
Summary
Gemini 3 models require thought_signature in tool calls and responses, and thinking content must be echoed back in the conversation loop.
Changes:
- Replace thought_signature field with generic metadata field on ToolRequest/ToolResponse
- Add ProviderMetadata type alias for provider-specific data
- Google provider stores thoughtSignature in metadata
- Helper functions for Google-specific metadata extraction
- Preserve thinking content in conversation when tool calls are processed
- Handle signature inheritance for multi-part responses
- Text-only responses treated as final answer (not thinking)
Type of Change
AI Assistance
Testing
Unit tests and manual testing
Related Issues
Relates to #5792
Relates to #6091