Skip to content

fix: handle undefined type in toGeminiSchema for enum/const-only schemas#370

Open
nuthalapativarun wants to merge 6 commits into
google:mainfrom
nuthalapativarun:fix/gemini-schema-undefined-type
Open

fix: handle undefined type in toGeminiSchema for enum/const-only schemas#370
nuthalapativarun wants to merge 6 commits into
google:mainfrom
nuthalapativarun:fix/gemini-schema-undefined-type

Conversation

@nuthalapativarun
Copy link
Copy Markdown
Contributor

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

toGeminiType() in core/src/utils/gemini_schema_util.ts had its parameter typed as string (not string | undefined), and recursiveConvert did not handle schemas that define shape via enum or const without a type field. This caused a TypeError at runtime when such schemas were encountered (e.g., from MCP tools).

Changes:

  • Updated toGeminiType() signature to accept string | undefined (the existing !mcpType guard already handles the undefined value safely).
  • Extended the type-inference block in recursiveConvert to infer the primitive type from enum values (when all enum entries share the same JS type) or from a const value.
  • Added passthrough of the enum field to the resulting Gemini Schema object.

Testing Plan

Unit Tests:

  • Added unit tests for enum-only schemas (all-string, mixed-type, with description)
  • Added unit tests for enum with explicit type field
  • Added unit tests for const-only schemas (string and numeric values)
  • All unit tests pass locally (27/27 in gemini_schema_util_test.ts)

Manual E2E Tests:

  • Verified that MCP tools with sub-schemas lacking type field no longer crash

Checklist

  • Read CONTRIBUTING.md
  • Self-reviewed code
  • Commented hard-to-understand areas
  • Added tests proving fix/feature works
  • Unit tests pass locally
  • Manually tested end-to-end
  • Dependent changes merged/published

@wanseob
Copy link
Copy Markdown

wanseob commented May 25, 2026

Hey @nuthalapativarun, my apology first: I went back and tested across versions, and #367 has actually been fixed since 0.2.5 (the if (!mcpType) guard). I was just stuck on 0.1.3 when I filed. Sorry for the
chase.

@nuthalapativarun
Copy link
Copy Markdown
Contributor Author

Thanks @wanseob — no worries at all! You're right that the if (!mcpType) runtime guard (added in 0.2.5) prevents the crash when type is undefined. But this PR goes a bit further in two ways that are still unaddressed in the current codebase:

  1. TypeScript type correctness: toGeminiType is still typed as (mcpType: string) rather than (mcpType: string | undefined), so TypeScript won't catch callers that pass undefined at compile time.
  2. Enum/const type inference: When an MCP schema omits type but provides enum or const, recursiveConvert currently falls through without setting a Gemini type — the resulting schema will have an empty/unspecified type field. This PR infers the type from the enum values (e.g. all strings → STRING) or from the const value, and also forwards the enum array to the Gemini schema so tools with enumerated options work correctly.

Happy to scope it down to just the enum/const inference if the type-signature change is considered unnecessary noise. Let me know!

const schema = toGeminiSchema(input as unknown as MCPToolSchema);

expect(schema).toEqual({
type: Type.NUMBER,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't it be instead?

expect(schema).toEqual({
  type: Type.NUMBER,
  enum: [42],
});

@nuthalapativarun
Copy link
Copy Markdown
Contributor Author

Good catch @kalenkevich! I've pushed the fix — const: X is now forwarded as a single-element enum: [X] so the constraint is preserved in the Gemini schema.

One note on the value type: the existing if (mcp.enum) path always calls .map(String) before writing to geminiSchema.enum (matching Gemini's enum: string[] type), so the test asserts enum: ['42'] rather than [42]. The string const case (const: 'fixed-value') is updated in the same commit.

@kalenkevich
Copy link
Copy Markdown
Collaborator

Thanks for the update, please fix the tests

kalenkevich and others added 2 commits May 28, 2026 13:47
The const-only schema branch was directly mutating mcp.type on the
original input object before spreading it into a new mcp. On a second
call with the same input, the type was already set so if (!mcp.type)
was skipped, leaving enum unset.

Fix by collecting the inferred type into a local variable and setting
both type and enum atomically via the spread assignment.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@nuthalapativarun
Copy link
Copy Markdown
Contributor Author

Found and fixed the root cause: the const branch was directly mutating mcp.type on the original input object before spreading it into a new mcp. When the test calls oGeminiSchema twice with the same input object (once inside .not.toThrow() and once to capture the value), the type field is already set on the second call so if (!mcp.type) is skipped entirely — leaving enum unset.

Fix: collect the inferred type into a local variable and set both ype and enum atomically via the spread assignment (mcp = {...mcp, type: inferredType, enum: [mcp.const]}). All 27 schema util tests + 1482 core unit tests now pass locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

toGeminiType crashes on undefined type (MCP tool sub-schema without type field)

3 participants