Skip to content

Conversation

@kevinvandijk
Copy link
Collaborator

Some MCP servers, like the Github one have tools that are not defining the additionalProperties param. This is required with native tools in strict mode. This PR adds the param when it's missing.

@changeset-bot
Copy link

changeset-bot bot commented Dec 12, 2025

🦋 Changeset detected

Latest commit: fdc0c0a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
kilo-code Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@kiloconnect kiloconnect bot left a comment

Choose a reason for hiding this comment

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

✅ No Issues Found

5 files reviewed | Confidence: 92% | Recommendation: Merge

Review Details

Files:

  • .changeset/stale-lines-care.md - Changeset file (trivial)
  • src/api/providers/kilocode/openai-strict-schema.ts - New utility function
  • src/api/providers/base-provider.ts - Integration of normalization
  • src/api/providers/openai-native.ts - Integration of normalization
  • src/api/providers/__tests__/openai-strict-schema.spec.ts - Test coverage

Analysis:

The PR correctly addresses the issue where MCP servers (like GitHub) define tool schemas without the additionalProperties field, which is required by OpenAI's strict mode. The solution:

  1. New utility function normalizeObjectAdditionalPropertiesFalse() recursively traverses JSON schemas and adds additionalProperties: false to any object schema with properties that doesn't already have it defined.

  2. Comprehensive recursion - The function handles:

    • Nested properties
    • Schema composition keywords (anyOf, oneOf, allOf)
    • Array items
    • $defs and definitions containers
    • additionalProperties when it's a schema object itself
  3. Safe behavior - Only adds the property when undefined, preserving intentional dictionary semantics when additionalProperties is explicitly set.

  4. Applied in two locations:

  5. Test coverage - Tests verify both the standalone function and the integration through BaseProvider.

Minor note: There's a typo in a comment at line 300 (kilocode_chang end instead of kilocode_change end), but this is cosmetic and doesn't affect functionality.

Checked: Security, bugs, edge cases, error handling, type safety

@kevinvandijk kevinvandijk merged commit e72df2a into main Dec 12, 2025
12 checks passed
@kevinvandijk kevinvandijk deleted the kevinvandijk/fix-non-strict-schemas branch December 12, 2025 22:00
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.

3 participants