Skip to content

fix(translator): prevent schema property name collision in Gemini sanitizer (#13057, #13477) - #13690

Merged
diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.51from
zcrew0x:fix/gemini-schema-properties-collision
Sep 18, 2026
Merged

diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.51from
zcrew0x:fix/gemini-schema-properties-collision

Conversation

@zcrew0x

@zcrew0x zcrew0x commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Fixes #13057
Closes #13477
Supersedes #13059 (rebased onto release/v3.8.51 with regression test for delivery.pin.required)

Problem

When a tool parameter schema contains a property named "properties", "required", etc. (e.g. in Roblox Studio, Blender, JSON Patch, or delivery.pin from #13477):

{
  "type": "object",
  "properties": {
    "action": { "type": "string" },
    "properties": { "type": "array", "items": { "type": "string" } }
  },
  "required": ["action"]
}

Schema visitor functions in open-sse/translator/helpers/geminiHelper.ts (such as injectObjectType, addPlaceholders, cleanupRequired, etc.) traversed schemas by doing a blind for (const value of Object.values(record)). When visiting schema.properties, because record.type was undefined and record.properties (or record.required) existed, injectObjectType injected schema.properties.type = "object" directly into the property dictionary.

This corrupted the schema sent to Google CloudCode/Gemini:

{
  "type": "object",
  "properties": {
    "action": { "type": "string" },
    "properties": { "type": "array", "items": { "type": "string" } },
    "type": "object"  <-- Invalid schema property
  }
}

Google rejected the request with HTTP 400:

"Invalid value at 'request.tools[0].function_declarations[1].parameters.properties[2].value' (type.googleapis.com/google.cloud.aiplatform.master.Schema), 'object'"

Solution

  1. Introduce forEachSubschema in geminiHelper.ts to traverse subschemas according to JSON Schema structure (descending into values of properties, patternProperties, $defs, definitions rather than treating the container maps as schema nodes).
  2. Use forEachSubschema across all schema visitors in geminiHelper.ts (injectObjectType, addPlaceholders, cleanupRequired, normalizeAdditionalProperties, convertConstToEnum, convertEnumValuesToStrings, mergeAllOf, flattenAnyOfOneOf, and flattenTypeArrays).
  3. Add regression unit test suite in tests/unit/gemini-schema-properties-name-collision-13057.test.ts covering both property named "properties" (fix(providers): Gemini schema sanitizer corrupts tools containing property named 'properties' (HTTP 400) & keepalive chunk desync in AI-SDK #13057) and property named "required" ([BUG] Antigravity (AGY) Provider Returns 400 Bad Request - Schema Incompatibility with Google Gemini Tools #13477).

@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks for the thorough fix — verified the regression test passes clean at your
head (2/2), and confirmed the underlying bug (properties-map-as-schema-node) is
still live on the current release tip across every visitor function you touched,
not just injectObjectType. This is the most complete fix in the batch: it also
closes #13477 (delivery.pin) which a narrower competing fix does not actually
reproduce.

Two small asks before merge: (1) add a changelog.d/fixes/ fragment — none is
included; (2) SCHEMA_MAP_KEYS already exists in this same file (from #12269)
with the canonical list of schema-map keys including dependentSchemas, which
your new forEachSubschema() doesn't check — reusing that constant instead of
the hand-rolled list would close that last gap and avoid a second source of
truth for the same concept.

@zcrew0x

zcrew0x commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

@diegosouzapw Thank you for the review! I've updated the PR with both requested changes:

  1. Reused the canonical SCHEMA_MAP_KEYS constant in forEachSubschema(), covering dependentSchemas and removing duplication.
  2. Added the changelog fragment at changelog.d/fixes/13690-gemini-schema-properties-name-collision.md (verified dry-run aggregation).

…ssion test

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
diegosouzapw added a commit that referenced this pull request Sep 17, 2026
…-09-15) (#13731)

Second `npm run release:reconcile` pass on `release/v3.8.50..release/v3.8.51`
(0915890..c0f92ec, 916 non-merge commits, 877 merged PRs):

- fold the 173 changelog.d fragments accumulated since #12971 under
  `## [3.8.51]` and delete them
- generate bullets for the 58 cycle commits that had no fragment
  (4 features / 42 fixes / 12 maintenance), each with the merged PR link and
  `— thanks @author`
- link 137 fragment bullets to the PR of the commit that added them and
  credit the author; two prefix/origin mismatches reviewed (#12945→#13392,
  #13001→#13379, both maintainer rebaselines of other people's PRs)
- refresh "Release by the numbers" + Top-25 and regenerate the
  `### 🙌 Contributors` hall (112 external contributors + maintainer; every
  non-bot author of the 877 merged PRs present)
- closed-PR credit audit for the window: nothing to add (#13215→#13361 and
  #13059→#13690 are still open, #12998 was independently fixed earlier by
  #12853); no human co-author trailers, no commits without a PR
- resync the 58 i18n CHANGELOG mirrors

Gates: check:changelog-integrity OK, check:docs-sync PASS.
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks @zcrew0x — merging via the release merge-train. Validated in local merge-train (merge-train-20260918-120911-suite.log) on the devbox @ train tip 8c305709478b7052fb981a75852bbf88fe3a959d with the sibling PRs of this batch: typecheck:core, file-size, complexity, cognitive-complexity, changelog-integrity green; changed-area node:test 310/311 (the single red, hard-lease inventory, reproduces on the pure release tip) + vitest green (fast parity mode — full suite ran today on the tip via the base-red and 3b trains). Merged --admin per merge-gates §4/§7.

@diegosouzapw
diegosouzapw merged commit 994245a into diegosouzapw:release/v3.8.51 Sep 18, 2026
3 checks passed
diegosouzapw added a commit to Ardem2025/OmniRoute that referenced this pull request Sep 19, 2026
…d dedup

The traversal-isolation bug this PR targeted (a schema property literally
named "required" corrupting the properties map) is already fixed on tip by
diegosouzapw#13690 (forEachSubschema/SCHEMA_MAP_KEYS applied uniformly across all
traversal phases), so that part of the diff is redundant churn.

Restore schemaCoercion.ts to its tip state — the PR's conflict resolution
had deleted normalizeClaudeToolInputSchema/hasRootLevelSchemaUnion (the
already-shipped diegosouzapw#13552 fix for Anthropic's root-level oneOf/anyOf/allOf
rejection) and dropped cleanJSONSchemaForAntigravity's preserveNullable
option while a live caller still passes it, breaking the build (TS2305).

Keep only the two genuinely new contributions from this PR:
- sanitizeProtobufTypes/sanitizeTypeName: normalize protobuf-flavored type
  spellings (dict, bool, int32, float, list, ...) that some MCP/agent
  clients emit onto the JSON Schema type names Gemini's Schema proto
  accepts.
- Deduplicate a schema's required array before filtering it against the
  properties map, so a repeated field name isn't counted more than once.

forEachSubschema/SCHEMA_MAP_KEYS keep their names, and
cleanJSONSchemaForAntigravity keeps its preserveNullable option/signature
unchanged, matching tip. Dropped two now-redundant traversal-isolation
tests (already covered by tests/unit/gemini-schema-properties-name-collision-13057.test.ts)
and kept the two tests that cover the new behavior.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…-09-15) (diegosouzapw#13731)

Second `npm run release:reconcile` pass on `release/v3.8.50..release/v3.8.51`
(104d4f8..d61b804, 916 non-merge commits, 877 merged PRs):

- fold the 173 changelog.d fragments accumulated since diegosouzapw#12971 under
  `## [3.8.51]` and delete them
- generate bullets for the 58 cycle commits that had no fragment
  (4 features / 42 fixes / 12 maintenance), each with the merged PR link and
  `— thanks @author`
- link 137 fragment bullets to the PR of the commit that added them and
  credit the author; two prefix/origin mismatches reviewed (diegosouzapw#12945→diegosouzapw#13392,
  diegosouzapw#13001→diegosouzapw#13379, both maintainer rebaselines of other people's PRs)
- refresh "Release by the numbers" + Top-25 and regenerate the
  `### 🙌 Contributors` hall (112 external contributors + maintainer; every
  non-bot author of the 877 merged PRs present)
- closed-PR credit audit for the window: nothing to add (diegosouzapw#13215→diegosouzapw#13361 and
  diegosouzapw#13059→diegosouzapw#13690 are still open, diegosouzapw#12998 was independently fixed earlier by
  diegosouzapw#12853); no human co-author trailers, no commits without a PR
- resync the 58 i18n CHANGELOG mirrors

Gates: check:changelog-integrity OK, check:docs-sync PASS.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…itizer (diegosouzapw#13057, diegosouzapw#13477) (diegosouzapw#13690)

* fix(translator): prevent schema property name collision in Gemini sanitizer (diegosouzapw#13057, diegosouzapw#13477)

* refactor(translator): reuse SCHEMA_MAP_KEYS in forEachSubschema and add changelog fragment

* test(translator): avoid explicit any in gemini schema collision regression test

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: zcrew0x <zcrew0x@users.noreply.github.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants