Skip to content

fix(moonshot): add schema cleaning for Moonshot/Kimi tool definitions#39618

Closed
MPavleski wants to merge 4 commits intoopenclaw:mainfrom
MPavleski:fix/moonshot-tool-schema-cleaning
Closed

fix(moonshot): add schema cleaning for Moonshot/Kimi tool definitions#39618
MPavleski wants to merge 4 commits intoopenclaw:mainfrom
MPavleski:fix/moonshot-tool-schema-cleaning

Conversation

@MPavleski
Copy link
Copy Markdown

Summary

Fixes #39603

This PR fixes tool call failures with Moonshot/Kimi K2.5 agents by adding provider-specific schema cleaning for Moonshot's API validation constraints.

Root Cause

The regression was introduced in commit fe94e83 (February 16, 2026) when tool schema normalization became provider-aware. Before this commit, cleanSchemaForGemini() was applied universally to all providers, including Moonshot. After the commit, only Gemini and xAI received schema cleaning, while Moonshot was left without any cleaning, causing it to fail when tool schemas contained validation keywords it doesn't support.

Changes

  • Created src/agents/schema/clean-for-moonshot.ts with stripMoonshotUnsupportedKeywords() and isMoonshotProvider()
  • Integrated Moonshot schema cleaning into normalizeToolParameters() in pi-tools.schema.ts
  • Added comprehensive test coverage in src/agents/schema/clean-for-moonshot.test.ts

Technical Details

Moonshot Kimi API (like Gemini and xAI) rejects certain JSON Schema keywords in tool parameter schemas:

  • Validation constraints: minLength, maxLength, minimum, maximum, pattern, format, etc.
  • Meta keywords: $schema, $ref, $defs, definitions, etc.

The implementation mirrors the existing pattern used for Gemini and xAI schema cleaning.

Testing

  • 19 new tests for Moonshot schema cleaning (all passing)
  • 22 existing tests for Gemini and xAI continue to pass
  • No regressions in existing functionality

Impact

This restores Moonshot functionality that was working before February 16, 2026, and affects:

  • Direct Moonshot provider usage
  • OpenRouter proxy with Moonshot models
  • DeepInfra proxy with Moonshot models

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: M labels Mar 8, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 8, 2026

Greptile Summary

This PR restores Moonshot/Kimi API compatibility by adding provider-specific schema cleaning (stripMoonshotUnsupportedKeywords) that was missing after the February 16 refactor made schema normalization provider-aware. The implementation correctly follows the established xAI/Gemini pattern and is well-tested.

Key observations:

  • The new clean-for-moonshot.ts module and its integration into normalizeToolParameters are structurally sound and consistent with existing cleaner patterns.
  • MOONSHOT_UNSUPPORTED_SCHEMA_KEYWORDS omits additionalProperties, which was stripped before the regression (via cleanSchemaForGemini applied universally). If Moonshot also rejects additionalProperties, this could cause residual 400 errors for any tool schema that includes it. Gemini explicitly lists it as unsupported.
  • The isMoonshotProvider proxy-detection heuristic covers moonshotai/ and moonshot/ prefixes on OpenRouter/DeepInfra, but would miss model IDs that use only the kimi family name without a moonshot prefix.
  • 19 new tests provide good coverage of the cleaning and provider detection logic.

Confidence Score: 3/5

  • Safe to merge for most tool schemas, but may have residual 400 errors if Moonshot rejects additionalProperties (which Gemini does and was previously stripped for Moonshot).
  • The PR correctly restores the missing Moonshot schema cleaning and is well-tested. However, the unsupported keyword list diverges from the Gemini list (which was previously applied) by omitting additionalProperties. Since the pre-regression behavior stripped additionalProperties for Moonshot too, this omission is a potential incomplete fix rather than a full restoration of prior behavior.
  • src/agents/schema/clean-for-moonshot.ts — specifically the MOONSHOT_UNSUPPORTED_SCHEMA_KEYWORDS set, which may need additionalProperties added to match the prior behavior.

Last reviewed commit: 5f2f2da

Comment on lines +6 to +29
export const MOONSHOT_UNSUPPORTED_SCHEMA_KEYWORDS = new Set([
// Validation constraints that frequently cause issues
"minLength",
"maxLength",
"minimum",
"maximum",
"multipleOf",
"pattern",
"format",
"minItems",
"maxItems",
"uniqueItems",
"minProperties",
"maxProperties",
"minContains",
"maxContains",

// Meta keywords that may not be supported
"patternProperties",
"$schema",
"$id",
"$ref",
"$defs",
"definitions",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

additionalProperties may need to be stripped

MOONSHOT_UNSUPPORTED_SCHEMA_KEYWORDS is described as mirroring Gemini's constraints, but it omits additionalProperties, which is present in GEMINI_UNSUPPORTED_SCHEMA_KEYWORDS. If Moonshot's API rejects additionalProperties in tool schemas (a common restriction in OpenAI-compatible providers), passing it through will produce 400 errors.

The PR description states the root cause was that Moonshot used to get the Gemini cleaning applied universally — meaning additionalProperties was always stripped for it before the regression. If that worked correctly, the new implementation should replicate that behavior:

Suggested change
export const MOONSHOT_UNSUPPORTED_SCHEMA_KEYWORDS = new Set([
// Validation constraints that frequently cause issues
"minLength",
"maxLength",
"minimum",
"maximum",
"multipleOf",
"pattern",
"format",
"minItems",
"maxItems",
"uniqueItems",
"minProperties",
"maxProperties",
"minContains",
"maxContains",
// Meta keywords that may not be supported
"patternProperties",
"$schema",
"$id",
"$ref",
"$defs",
"definitions",
export const MOONSHOT_UNSUPPORTED_SCHEMA_KEYWORDS = new Set([
// Validation constraints that frequently cause issues
"minLength",
"maxLength",
"minimum",
"maximum",
"multipleOf",
"pattern",
"format",
"minItems",
"maxItems",
"uniqueItems",
"minProperties",
"maxProperties",
"minContains",
"maxContains",
// Meta keywords that may not be supported
"patternProperties",
"additionalProperties",
"$schema",
"$id",
"$ref",
"$defs",
"definitions",
"examples",
]);
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/schema/clean-for-moonshot.ts
Line: 6-29

Comment:
**`additionalProperties` may need to be stripped**

`MOONSHOT_UNSUPPORTED_SCHEMA_KEYWORDS` is described as mirroring Gemini's constraints, but it omits `additionalProperties`, which **is** present in `GEMINI_UNSUPPORTED_SCHEMA_KEYWORDS`. If Moonshot's API rejects `additionalProperties` in tool schemas (a common restriction in OpenAI-compatible providers), passing it through will produce 400 errors.

The PR description states the root cause was that Moonshot used to get the Gemini cleaning applied universally — meaning `additionalProperties` was always stripped for it before the regression. If that worked correctly, the new implementation should replicate that behavior:

```suggestion
export const MOONSHOT_UNSUPPORTED_SCHEMA_KEYWORDS = new Set([
  // Validation constraints that frequently cause issues
  "minLength",
  "maxLength",
  "minimum",
  "maximum",
  "multipleOf",
  "pattern",
  "format",
  "minItems",
  "maxItems",
  "uniqueItems",
  "minProperties",
  "maxProperties",
  "minContains",
  "maxContains",

  // Meta keywords that may not be supported
  "patternProperties",
  "additionalProperties",
  "$schema",
  "$id",
  "$ref",
  "$defs",
  "definitions",
  "examples",
]);
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +63 to +78
return cleaned;
}

export function isMoonshotProvider(modelProvider?: string, modelId?: string): boolean {
const provider = modelProvider?.toLowerCase() ?? "";
if (provider.includes("moonshot")) {
return true;
}
// OpenRouter and other proxies may use moonshotai/ prefix
const lowerModelId = modelId?.toLowerCase() ?? "";
if (provider === "openrouter" && lowerModelId.includes("moonshot")) {
return true;
}
// DeepInfra proxies Moonshot models
if (provider === "deepinfra" && lowerModelId.includes("moonshot")) {
return true;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Kimi model IDs without moonshot prefix won't be detected

The proxy-provider checks rely on lowerModelId.includes("moonshot"), which works for moonshotai/kimi-k2.5 and moonshot/kimi-k2. However, if a proxy ever routes using only the model family name (e.g. "kimi/kimi-k2.5" or "kimi-k2.5") it will not be caught.

This is a minor false-negative risk rather than a regression — it's strictly better than before — but it's worth considering whether "kimi" should also be part of the detection heuristic for proxy routes, since Kimi is Moonshot's primary model line.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/schema/clean-for-moonshot.ts
Line: 63-78

Comment:
**Kimi model IDs without `moonshot` prefix won't be detected**

The proxy-provider checks rely on `lowerModelId.includes("moonshot")`, which works for `moonshotai/kimi-k2.5` and `moonshot/kimi-k2`. However, if a proxy ever routes using only the model family name (e.g. `"kimi/kimi-k2.5"` or `"kimi-k2.5"`) it will not be caught.

This is a minor false-negative risk rather than a regression — it's strictly better than before — but it's worth considering whether `"kimi"` should also be part of the detection heuristic for proxy routes, since Kimi is Moonshot's primary model line.

How can I resolve this? If you propose a fix, please make it concise.

Moonshot Kimi API rejects certain JSON Schema keywords (minLength, maxLength,
minimum, maximum, format, etc.) in tool parameter schemas, causing tool calls
to fail with 400 errors.

This commit introduces Moonshot-specific schema cleaning similar to the existing
Gemini and xAI cleaners:

- Created src/agents/schema/clean-for-moonshot.ts with stripMoonshotUnsupportedKeywords
- Added isMoonshotProvider to detect Moonshot directly or via proxies (OpenRouter, DeepInfra)
- Integrated Moonshot schema cleaning into normalizeToolParameters in pi-tools.schema.ts

This fixes the regression introduced in commit fe94e83 (Feb 16, 2026) where
schema cleaning became provider-aware but Moonshot was not included, causing
previously working Moonshot agents to fail tool calls.

Fixes: openclaw#39586 (assumed issue number for Moonshot tool call failures)
@MPavleski MPavleski force-pushed the fix/moonshot-tool-schema-cleaning branch from 5f2f2da to 497bff2 Compare March 8, 2026 08:09
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 20006db02f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agents/schema/clean-for-moonshot.ts
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 20006db02f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agents/schema/clean-for-moonshot.ts
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e91948866b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +90 to +94
const name = decodeJsonPointerSegment(match[1] ?? "");
if (!name) {
return undefined;
}
return defs.get(name);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Resolve full JSON Pointer refs instead of top-level def keys

tryResolveLocalRef only does defs.get(name) on the whole suffix after #/$defs/, so valid pointers like #/$defs/Foo/properties/bar are treated as unresolved and then stripped to {} when $ref is removed. This silently weakens those parameter schemas for Moonshot (the property loses its type/enum shape), which can lead to malformed tool arguments passing model-side validation and failing downstream. The new cleaner now handles simple refs, but nested local JSON Pointer refs still regress in this path.

Useful? React with 👍 / 👎.

@MPavleski
Copy link
Copy Markdown
Author

Closing this PR in favor of a follow-up fix for Moonshot tool-calling behavior regression.

@MPavleski MPavleski closed this Mar 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Tool calling when Moonshot / Kimi model used is broken

1 participant