fix: validate JSON before raw-embedding function call outputs in Responses API#2162
Conversation
…onses API
gjson.Parse() marks any string starting with { or [ as gjson.JSON type,
even when the content is not valid JSON (e.g. macOS plist format, truncated
tool results). This caused sjson.SetRaw to embed non-JSON content directly
into the Gemini API request payload, producing 400 errors.
Add json.Valid() check before using SetRaw to ensure only actually valid
JSON is embedded raw. Non-JSON content now falls through to sjson.Set
which properly escapes it as a JSON string.
Fixes router-for-me#2161
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the stability of the Gemini API integration by addressing a critical issue where invalid JSON structures were being incorrectly processed. By implementing a pre-check for JSON validity, the system now prevents malformed data from causing API errors, ensuring that all function call outputs are correctly formatted before transmission. This change improves the reliability of the translation layer without altering the core logic for valid JSON payloads. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a bug where non-JSON strings were being embedded as raw JSON, causing API errors. The addition of a json.Valid() check before using sjson.SetRaw is an effective solution.
I noticed a similar pattern at line 307 in the same file (internal/translator/gemini/openai/responses/gemini_openai-responses_request.go), where functionCall arguments are processed:
arguments := item.Get("arguments").String()
// ...
argsResult := gjson.Parse(arguments)
functionCall, _ = sjson.SetRaw(functionCall, "functionCall.args", argsResult.Raw)This could be vulnerable to the same issue if the arguments string is not valid JSON. You might consider applying a similar validation there to improve robustness.
xkonjin
left a comment
There was a problem hiding this comment.
Code Review
Verdict: Looks good ✅
Clean, minimal fix for a real correctness issue. The gjson type-sniffing producing false positives for plist syntax and bracket-prefixed strings is a known foot-gun, and gating with json.Valid() is the right call.
What's good:
- Single-line change, minimal blast radius
- The fallthrough to
sjson.Set(string-escaped) is the correct safe default - Both trigger cases (plist
{and truncated[) are well documented in the PR description
Minor observations (non-blocking):
- Consider whether a debug/trace-level log when the
json.Validguard rejects input would help future debugging of unexpected Gemini API payloads. Not required, but could save time if a third edge case surfaces. - The import of
encoding/jsonjust forjson.Validis fine; it's stdlib with zero overhead.
No bugs, no security concerns. Ship it.
Includes bug fixes, new features (Codex plan support, Vertex API, Amp integration, streaming keepalive, model alias enhancements), CI/CD updates, and community contributions through PR router-for-me#2162. Preserves custom config.yaml and cankao.txt.
Summary
json.Valid()check before usingsjson.SetRawfor function call outputs in the Responses API translator, preventing non-JSON content from being embedded raw into the Gemini API request payload.Problem
gjson.Parse()marks any string starting with{or[asgjson.JSON, even when the content is not valid JSON. This causessjson.SetRawto embed non-JSON content directly into the Gemini request, producing 400 errors from Google's API.Two known triggers:
defaults readreturns{ "$archiver" = NSKeyedArchiver; ... }which starts with{but uses=instead of:[Old tool result content cleared]which starts with[Fix
One-line change: add
json.Valid([]byte(output.Raw))guard beforeSetRaw. Non-JSON content falls through tosjson.Setwhich properly escapes it as a JSON string.Fixes #2161