Skip to content

fix: preserve only originally-required properties in strict tool schemas#471

Open
auriti wants to merge 2 commits intoGitlawb:mainfrom
auriti:fix/optional-props-not-required-strict-mode
Open

fix: preserve only originally-required properties in strict tool schemas#471
auriti wants to merge 2 commits intoGitlawb:mainfrom
auriti:fix/optional-props-not-required-strict-mode

Conversation

@auriti
Copy link
Copy Markdown
Collaborator

@auriti auriti commented Apr 7, 2026

Summary

Fixes #430.

normalizeSchemaForOpenAI() was adding every property key to required[] when strict = true (the default for non-Gemini providers). This caused Groq, Azure OpenAI, and other strict-validation providers to reject valid tool calls with:

400 tool_use_failed: missing properties: 'offset', 'limit', 'pages'

The model correctly omits optional arguments, but the provider sees them as missing required fields.

Root Cause

Branch Behaviour
strict = true (non-Gemini) required = [...existingRequired, ...allKeys] — adds ALL property keys
strict = false (Gemini) required = existingRequired.filter(k => k in normalizedProps) — correct

The Gemini branch already had the right logic. The strict branch did not.

Changes

File Change
src/services/api/openaiShim.ts Align the strict branch: use existingRequired.filter(k => k in normalizedProps). Keep additionalProperties: false.
src/services/api/openaiShim.test.ts Add regression test simulating the Read tool schema (file_path required, offset/limit/pages optional).

Issues Addressed

Test Plan

  • bun test src/services/api/openaiShim.test.ts — all 5 tests pass
  • New regression test covers the exact Read tool failure pattern from the issue
  • Existing MCP schema sanitization test updated to reflect correct behaviour (no required[] when schema has none)

Fixes Gitlawb#430. In normalizeSchemaForOpenAI(), the strict branch was adding every
property key to required[], including optional ones. This caused providers like
Groq, Azure OpenAI, and others to reject valid tool calls with a 400 /
tool_use_failed error because the model correctly omits optional arguments but
the provider sees them as missing required fields.

Root cause: the strict branch used `[...existingRequired, ...allKeys]` instead
of `existingRequired.filter(k => k in normalizedProps)`. The Gemini branch
already had the correct logic.

Fix: align the strict branch with the Gemini branch — only keep properties that
were already marked required in the original schema. The additionalProperties:
false constraint is preserved as strict-mode providers still require it.

Add regression test covering the Read tool schema (file_path required,
offset/limit/pages optional).
@kevincodex1
Copy link
Copy Markdown
Contributor

please fix conflicts @auriti

@auriti
Copy link
Copy Markdown
Collaborator Author

auriti commented Apr 7, 2026

Conflicts are now resolved by merging the latest main into this branch.

Re-ran the focused src/services/api/openaiShim.test.ts suite and bun run smoke successfully.

Copy link
Copy Markdown
Contributor

@kevincodex1 kevincodex1 left a comment

Choose a reason for hiding this comment

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

this looks good to me

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.

Tool call validation fails: all optional properties sent as required (strict mode)

2 participants