-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Fix: Wrap Gemini function response arrays in object to prevent API error #2745
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
Fix: Wrap Gemini function response arrays in object to prevent API error #2745
Conversation
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.
Pull request overview
This PR fixes a bug where the Gemini API rejects FunctionResponse payloads when tool responses are JSON arrays. The fix wraps array responses in an object to satisfy the Gemini API's requirement that the response field be a JSON Object (Protobuf Struct) rather than a ListValue.
Key changes:
- Modified the type check in
geminiMessageConverter.tsto detect arrays and wrap them in{ result: [...] }objects - Added a regression test to verify array responses are properly wrapped
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/extension/byok/common/geminiMessageConverter.ts |
Added Array.isArray() check to wrap array responses in an object before sending to Gemini API |
src/extension/byok/common/test/geminiMessageConverter.spec.ts |
Added test case verifying that array tool responses are wrapped in { result: [...] } format |
| it('should wrap array responses in an object', () => { | ||
| const toolResult = new LanguageModelToolResultPart('listRepos_12345', [new LanguageModelTextPart('["repo1", "repo2"]')]); | ||
| const messages: LanguageModelChatMessage[] = [ | ||
| { | ||
| role: LanguageModelChatMessageRole.Assistant, | ||
| content: [toolResult], | ||
| name: undefined | ||
| } | ||
| ]; | ||
|
|
||
| const result = apiMessageToGeminiMessage(messages); | ||
|
|
||
| expect(result.contents).toHaveLength(1); | ||
| expect(result.contents[0].role).toBe('user'); | ||
| const fr: any = result.contents[0].parts![0]; | ||
| expect(fr.functionResponse.response).toEqual({ result: ['repo1', 'repo2'] }); | ||
| }); |
Copilot
AI
Jan 8, 2026
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.
While the test for array wrapping is good, the fix also wraps other primitive types (strings, numbers, booleans, null) in an object. Consider adding tests for these cases as well to ensure comprehensive coverage. For example:
- Test with a string primitive:
"simple string" - Test with a number:
42 - Test with a boolean:
true - Test with null:
null
This would ensure that all code paths through the wrapping logic are tested.
| if (textContent) { | ||
| // Handle case with text content (may also have images) | ||
| try { | ||
| responsePayload = JSON.parse(textContent); |
Copilot
AI
Jan 8, 2026
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.
Consider adding a comment explaining why arrays (and other primitive types) need to be wrapped in an object. This would help future maintainers understand the Gemini API requirement. For example:
// Wrap non-object values (arrays, primitives, null) in an object because
// Gemini's FunctionResponse expects response to be a google.protobuf.Struct (JSON Object)
This provides context about the API constraint that necessitates this wrapping behavior.
| responsePayload = JSON.parse(textContent); | |
| responsePayload = JSON.parse(textContent); | |
| // Wrap non-object values (arrays, primitives, null) in an object because | |
| // Gemini's FunctionResponse.response must be a google.protobuf.Struct (JSON object) |
4321f6c to
cca7bbb
Compare
|
@vijayupadya Hi, can you please review this? |
Description
Fixes an issue where the Gemini API rejects FunctionResponse payloads when the tool response is a JSON Array. The API requires the
responsefield to be a JSON Object (Protobuf Struct).When a tool returns a list (e.g. from GitHub MCP listing repos), the API throws the following error:
Changes
src/extension/byok/common/geminiMessageConverter.tsto detect ifresponsePayloadis an array and wrap it in an object:{ result: [...] }.src/extension/byok/common/test/geminiMessageConverter.spec.ts.Context / References
FunctionResponseexpects theresponsefield to be agoogle.protobuf.Struct(JSON Object), not aListValue. Sending a top-level array causes a protocol buffer type mismatch.Verification
should wrap array responses in an objectwhich passes.npx vitest run src/extension/byok/common/test/geminiMessageConverter.spec.ts.