Repository navigation
fix(tools): keep opaque object schemas open - #3097
diegosouzapw merged 3 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces changes to keep opaque object schemas open-world by setting additionalProperties: true when properties is undefined or empty. This logic is implemented in both toolSchemaSanitizer.ts and schemaCoercion.ts, and is accompanied by a new unit test suite. The review feedback suggests enhancing the object schema detection in both files to correctly handle cases where the schema type is defined as an array of strings (e.g., ["object", "null"]) to support nullable objects.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const properties = schema.properties; | ||
| const isObjectSchema = schema.type === "object" || isPlainObject(properties); | ||
| if (!isObjectSchema) return; |
There was a problem hiding this comment.
In JSON Schema, the type property can sometimes be defined as an array of strings (for example, type: ["object", "null"] to represent a nullable object). To ensure these schemas are also correctly identified as object schemas when properties is undefined, we should check if type is either "object" or an array containing "object".
const properties = schema.properties;
const type = schema.type;
const isObjectSchema =
type === "object" ||
(Array.isArray(type) && type.includes("object")) ||
isPlainObject(properties);
if (!isObjectSchema) return;| const properties = schema.properties; | ||
| const isObjectSchema = schema.type === "object" || isPlainObject(properties); | ||
| if (!isObjectSchema) return; |
There was a problem hiding this comment.
In JSON Schema, the type property can sometimes be defined as an array of strings (for example, type: ["object", "null"] to represent a nullable object). To ensure these schemas are also correctly identified as object schemas when properties is undefined, we should check if type is either "object" or an array containing "object".
const properties = schema.properties;
const type = schema.type;
const isObjectSchema =
type === "object" ||
(Array.isArray(type) && type.includes("object")) ||
isPlainObject(properties);
if (!isObjectSchema) return;There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9ac4786eb
ℹ️ 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".
| if (isPlainObject(parameters)) return sanitizeSchema(parameters); | ||
| if (parameters === null || parameters === undefined) { | ||
| return { type: "object", properties: {} }; | ||
| return { type: "object", properties: {}, additionalProperties: true }; |
There was a problem hiding this comment.
Update sanitizer expectations for open schemas
When parameters is null or omitted, this now returns { type: "object", properties: {}, additionalProperties: true }, but the existing tests/unit/tool-schema-sanitizer.test.mjs cases for missing/null Chat Completions parameters and missing Responses parameters still assert the old { type: "object", properties: {} } shape. That leaves the committed unit suite failing for this intentional behavior change, so the old expectations need to be updated alongside this return value.
Useful? React with 👍 / 👎.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Reviewed by laguna-m.1-20260312:free · 3,348,714 tokens |
…paque-tool-schemas
…es:true The opaque-schema fix intentionally injects additionalProperties:true on empty object schemas (incl. the web_search passthrough shim and null/missing parameter fallbacks). Update the pre-fix snapshot assertions to match the new behavior.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d458501793
ℹ️ 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".
| result.else = coerceSchemaNumericFields(result.else); | ||
| } | ||
|
|
||
| keepOpaqueObjectSchemasOpen(result); |
There was a problem hiding this comment.
Avoid re-adding unsupported Gemini schema keyword
For Gemini/Antigravity targets, openaiToGeminiBase() has already converted tools via buildGeminiTools(), whose cleanJSONSchemaForAntigravity() strips additionalProperties because Gemini function declarations reject it with Unknown name. This new final pass runs afterward on functionDeclarations.parameters, so an empty object parameter schema gets additionalProperties: true reintroduced and those Gemini tool requests can start failing with 400s.
Useful? React with 👍 / 👎.
|
Thanks, @nmime! 🙏 Keeping opaque object schemas open (injecting |
* fix(tools): keep opaque object schemas open * test: align opaque-object-schema expectations with additionalProperties:true The opaque-schema fix intentionally injects additionalProperties:true on empty object schemas (incl. the web_search passthrough shim and null/missing parameter fallbacks). Update the pre-fix snapshot assertions to match the new behavior. --------- Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
…ors hall Adds entries for diegosouzapw#3097, diegosouzapw#3101 (deepseek-web diegosouzapw#2942/diegosouzapw#2820), diegosouzapw#3104, diegosouzapw#3105, diegosouzapw#3107, diegosouzapw#3109, diegosouzapw#3111, diegosouzapw#3113, diegosouzapw#3115, diegosouzapw#3122, diegosouzapw#3125, diegosouzapw#3127, diegosouzapw#3129, plus a Contributors section crediting all v3.8.9 contributors. Stamps the 3.8.9 release date.
* fix(tools): keep opaque object schemas open * test: align opaque-object-schema expectations with additionalProperties:true The opaque-schema fix intentionally injects additionalProperties:true on empty object schemas (incl. the web_search passthrough shim and null/missing parameter fallbacks). Update the pre-fix snapshot assertions to match the new behavior. --------- Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
…ors hall Adds entries for diegosouzapw#3097, diegosouzapw#3101 (deepseek-web diegosouzapw#2942/diegosouzapw#2820), diegosouzapw#3104, diegosouzapw#3105, diegosouzapw#3107, diegosouzapw#3109, diegosouzapw#3111, diegosouzapw#3113, diegosouzapw#3115, diegosouzapw#3122, diegosouzapw#3125, diegosouzapw#3127, diegosouzapw#3129, plus a Contributors section crediting all v3.8.9 contributors. Stamps the 3.8.9 release date.
* fix(tools): keep opaque object schemas open * test: align opaque-object-schema expectations with additionalProperties:true The opaque-schema fix intentionally injects additionalProperties:true on empty object schemas (incl. the web_search passthrough shim and null/missing parameter fallbacks). Update the pre-fix snapshot assertions to match the new behavior. --------- Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
…ors hall Adds entries for diegosouzapw#3097, diegosouzapw#3101 (deepseek-web diegosouzapw#2942/diegosouzapw#2820), diegosouzapw#3104, diegosouzapw#3105, diegosouzapw#3107, diegosouzapw#3109, diegosouzapw#3111, diegosouzapw#3113, diegosouzapw#3115, diegosouzapw#3122, diegosouzapw#3125, diegosouzapw#3127, diegosouzapw#3129, plus a Contributors section crediting all v3.8.9 contributors. Stamps the 3.8.9 release date.
Branch now: https://github.com/nmime/OmniRoute/tree/fix/gpt55-opaque-tool-schemas
Current branch head: e9ac478
Compare against diegosouzapw/OmniRoute:main:
ahead: 1
behind: 0
What was fixed:
Original branch was accidentally based on stale nmime/main, causing 104 ahead / 931 behind.
I rebuilt the fix branch from current diegosouzapw/OmniRoute:main.
Reapplied only the OmniRoute tool-schema fix.
Removed the dumb long comment from:
open-sse/services/toolSchemaSanitizer.ts
open-sse/translator/helpers/schemaCoercion.ts
Verified comment matches now: 0.
What the code fix does:
Before: empty object tool schemas like { type: "object", properties: {} } could make GPT-5.5/Codex prune nested values to {}.
Example broken case: SPLOX_EXECUTE_TOOL.args became {} instead of { action: "create" }.
Now: unspecified empty object schemas are kept open with additionalProperties: true.
Explicitly closed schemas still stay closed with additionalProperties: false.
Tests/checks:
Focused unit tests: 26 passed
Branch clean: 1 commit ahead, 0 behind
Production still working:
PM2 process alive: yes
local /status: 200
public /status: 200
runtime check: args.additionalProperties=true