fix: enable AJV coerceTypes to handle string-typed numeric tool args - #2500
fix: enable AJV coerceTypes to handle string-typed numeric tool args#2500lekifier wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: cf75a34 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c642eb74ea
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import addFormats from 'ajv-formats'; | ||
|
|
||
| const DRAFT_07_AJV = new Ajv({ strict: false, allErrors: true }); | ||
| const DRAFT_07_AJV = new Ajv({ strict: false, allErrors: true, coerceTypes: true }); |
There was a problem hiding this comment.
With this global option, Ajv does more than string→number coercion: its documented rules also turn null into '' for string schemas (https://ajv.js.org/coercion.html). Because the executor passes the mutated parsedArgs.data straight to tools, a call like Write({ path: 'file', content: null }) now validates against the required string fields in WriteInputSchema and overwrites the file with empty content instead of being rejected. Please narrow this to numeric/integer fields or pre-normalize only stringified numbers before validation; the same issue exists in the legacy validator changed in this commit.
Useful? React with 👍 / 👎.
7f50281 to
2033d94
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2033d94842
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /** | ||
| * Recursively coerce string values to their likely intended types. | ||
| * Models sometimes serialize typed parameters as strings: | ||
| * - `"3"` instead of `3` (number) | ||
| * - `"true"` instead of `true` (boolean) |
There was a problem hiding this comment.
The new helper-level JSDoc (and the nearby inline implementation comments) violates the v2 comment convention: implementation comments must live only in the top-of-file block, not beside functions or statements. Please either fold the externally useful rationale into the file header or remove these local narration comments.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
| for (const [key, child] of Object.entries(value)) { | ||
| result[key] = coerceStringValues(child); |
There was a problem hiding this comment.
Because the retry walks every property without looking at that property's schema, a single typed field that needs coercion can make unrelated string fields invalid. For example, with the Read schema, { path: '123', line_offset: '3' } first fails on line_offset, then this recursion turns path into the number 123, so the retry fails with /path must be string instead of accepting the otherwise valid call; the duplicated legacy helper has the same gap. Please make coercion schema-guided (or restrict it to fields whose schema accepts the coerced type) rather than applying it to all strings.
Useful? React with 👍 / 👎.
2033d94 to
f847db1
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f847db1ac9
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (mutated && validator(args)) { | ||
| return null; |
There was a problem hiding this comment.
When a string value is coercible but still violates another keyword, the retry runs validator(args) and then falls through to format the pre-coercion errors captured above. For inputs like Read({ path, line_offset: "0" }) or SetGoalBudget({ value: "0", unit: "turns" }), the argument is converted to 0, the retry fails on the range constraint, but the model still sees the stale type error instead of the actionable range error; the legacy copy has the same pattern. Refresh validator.errors from the failed retry, or avoid mutating until the retry succeeds.
Useful? React with 👍 / 👎.
| }); | ||
|
|
||
| it('coerces numeric strings to numbers on validation failure', () => { | ||
| // Models sometimes serialize numbers as strings, e.g. "3" instead of 3. |
There was a problem hiding this comment.
This v2 test adds explanatory // comments inside it blocks, but the scoped guide requires comments to live only in the top-of-file block and not beside functions or statements. Please remove these local narration comments or fold the scenario notes into a file header; the same applies to the other added inline comments in this test.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L15-L17
Useful? React with 👍 / 👎.
f847db1 to
9ed1c63
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ed1c63b1a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "@moonshot-ai/agent-core": patch | ||
| "@moonshot-ai/agent-core-v2": patch |
There was a problem hiding this comment.
For this user-visible tool-call validation fix, the changeset should bump @moonshot-ai/kimi-code rather than only the internal engine packages: the CLI ships the agent-core-v2 tool execution path, and changesets will not automatically propagate internal package bumps into the CLI release/changelog. Please list the CLI package with a patch entry and describe the visible behavior change so the next CLI release carries it.
AGENTS.md reference: AGENTS.md:L83-L83
Useful? React with 👍 / 👎.
Models sometimes serialize typed tool parameters as strings:
- "3" instead of 3 (number/integer)
- "true" instead of true (boolean)
- "[\"a\"]" instead of ["a"] (JSON array/object)
On validation failure, inspect AJV type-mismatch errors and coerce
only the failing fields whose value is a string. Fields whose schema
accepts strings are never touched (e.g. path: '123' stays a string).
Unlike AJV's coerceTypes option, null is never coerced, so invalid
args like Write({ content: null }) are still correctly rejected.
Closes MoonshotAI#2118
9ed1c63 to
cf75a34
Compare
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
Problem
Models sometimes serialize numeric tool parameters as strings (e.g.
"line_offset": "3"instead of"line_offset": 3), causing AJV validation to reject otherwise-valid tool calls with:This has been observed with multiple models (Qwen3.8 Max Preview, Kimi K3, etc.).
Solution
Enable
coerceTypes: trueon all three AJV instances (draft-07, 2019-09, 2020-12) in bothagent-coreandagent-core-v2. AJV's built-in type coercion automatically converts string values to the schema-declared type before validation (e.g."3"→3). Non-coercible values (e.g."abc"for an integer field) still fail validation as expected.This matches the behavior of other agent frameworks (OpenAI SDK, Anthropic SDK) that perform similar type coercion on tool call arguments.
Changes
packages/agent-core/src/tools/args-validator.ts: addcoerceTypes: trueto all AJV instancespackages/agent-core-v2/src/tool/args-validator.ts: samepackages/agent-core-v2/test/tool/args-validator.test.ts: add test verifying coercion behaviorTesting
All existing tests pass. New test verifies:
{ line_offset: "3" }passes validation fortype: "integer"schema{ line_offset: "abc" }still fails with "must be integer"Closes #2118