From 4bc65edc4db3b4534a68c4801a928eaae2b8f844 Mon Sep 17 00:00:00 2001 From: _Kerman Date: Fri, 10 Jul 2026 15:25:19 +0800 Subject: [PATCH] fix(agent-core-v2): restore JSON Schema format validation in tool-args Align with v1: replace the hand-rolled subset validator with v1's Ajv- based implementation (draft-07/2019/2020 + ajv-formats), so tool-call argument validation once again honors the JSON Schema `format` keyword (and the full keyword set), not just the previously hard-coded subset. - args-validator.ts is now byte-identical to v1 (93 lines, replacing the 289-line hand-rolled subset). - Adds ajv@^8.18.0 and ajv-formats@^3.0.1 (same versions as v1) plus the pnpm-lock.yaml update. - The two call sites (compileToolArgsValidator -> validateToolArgs) keep working unchanged; a small test locks in format / required / additionalProperties / subset behavior. --- packages/agent-core-v2/package.json | 2 + .../src/_base/tools/args-validator.ts | 326 ++++-------------- .../test/_base/tools/args-validator.test.ts | 45 +++ pnpm-lock.yaml | 85 ++++- 4 files changed, 195 insertions(+), 263 deletions(-) create mode 100644 packages/agent-core-v2/test/_base/tools/args-validator.test.ts diff --git a/packages/agent-core-v2/package.json b/packages/agent-core-v2/package.json index 978b0ac41..dfa772107 100644 --- a/packages/agent-core-v2/package.json +++ b/packages/agent-core-v2/package.json @@ -67,6 +67,8 @@ "@moonshot-ai/minidb": "workspace:^", "@moonshot-ai/protocol": "workspace:^", "@mozilla/readability": "^0.6.0", + "ajv": "^8.18.0", + "ajv-formats": "^3.0.1", "chokidar": "^4.0.3", "ignore": "^5.3.2", "jimp": "^1.6.1", diff --git a/packages/agent-core-v2/src/_base/tools/args-validator.ts b/packages/agent-core-v2/src/_base/tools/args-validator.ts index 5c5c372fd..ce967ccb0 100644 --- a/packages/agent-core-v2/src/_base/tools/args-validator.ts +++ b/packages/agent-core-v2/src/_base/tools/args-validator.ts @@ -1,7 +1,55 @@ -/** - * `tools` support module — validates tool-call arguments against the JSON - * Schema subset used by built-in and MCP tool definitions. - */ +import Ajv, { type ErrorObject, type ValidateFunction } from 'ajv'; +import Ajv2019 from 'ajv/dist/2019'; +import Ajv2020 from 'ajv/dist/2020'; +import addFormats from 'ajv-formats'; + +const DRAFT_07_AJV = new Ajv({ strict: false, allErrors: true }); +addFormats(DRAFT_07_AJV); + +const DRAFT_2019_AJV = new Ajv2019({ strict: false, allErrors: true }); +addFormats(DRAFT_2019_AJV); + +const DRAFT_2020_AJV = new Ajv2020({ strict: false, allErrors: true }); +addFormats(DRAFT_2020_AJV); + +const DRAFT_2019_KEYWORDS = new Set([ + 'dependentRequired', + 'dependentSchemas', + 'maxContains', + 'minContains', + 'unevaluatedItems', + 'unevaluatedProperties', + '$recursiveAnchor', + '$recursiveRef', +]); + +const DRAFT_2020_KEYWORDS = new Set(['prefixItems', '$dynamicAnchor', '$dynamicRef']); + +// Mixing JSON Schema dialects in a single Ajv instance is unsafe because +// keyword semantics differ, e.g. draft-07 tuple `items` vs 2020-12 `prefixItems`. +function ajvFor(schema: Record): Ajv | Ajv2019 | Ajv2020 { + const $schema = schema['$schema']; + if (typeof $schema === 'string') { + if ($schema.includes('2020-12')) return DRAFT_2020_AJV; + if ($schema.includes('2019-09')) return DRAFT_2019_AJV; + return DRAFT_07_AJV; + } + if (containsSchemaKeyword(schema, DRAFT_2020_KEYWORDS)) return DRAFT_2020_AJV; + if (containsSchemaKeyword(schema, DRAFT_2019_KEYWORDS)) return DRAFT_2019_AJV; + return DRAFT_07_AJV; +} + +function containsSchemaKeyword(value: unknown, keywords: ReadonlySet): boolean { + if (Array.isArray(value)) { + return value.some((item) => containsSchemaKeyword(item, keywords)); + } + if (typeof value !== 'object' || value === null) return false; + for (const [key, child] of Object.entries(value)) { + if (keywords.has(key)) return true; + if (containsSchemaKeyword(child, keywords)) return true; + } + return false; +} export type JsonType = null | number | string | boolean | JsonArray | JsonObject; @@ -11,279 +59,35 @@ export interface JsonArray extends Array {} /** @internal */ export interface JsonObject extends Record {} -export interface ToolArgsValidator { - readonly schema: Record; -} +export type ToolArgsValidator = ValidateFunction; -interface ValidationError { - readonly keyword: string; - readonly instancePath: string; - readonly message: string; - readonly params?: Record; -} - -function formatValidationError(error: ValidationError): string { - if ( - error.keyword === 'required' && - error.params !== undefined && - 'missingProperty' in error.params - ) { +function formatValidationError(error: ErrorObject): string { + if (error.keyword === 'required' && 'missingProperty' in error.params) { return `must have required property '${String(error.params['missingProperty'])}'`; } - if ( - error.keyword === 'additionalProperties' && - error.params !== undefined && - 'additionalProperty' in error.params - ) { + if (error.keyword === 'additionalProperties' && 'additionalProperty' in error.params) { return `must NOT have additional property '${String(error.params['additionalProperty'])}'`; } const path = error.instancePath ? `${error.instancePath} ` : ''; - return `${path}${error.message}`; + return `${path}${error.message ?? 'is invalid'}`; } export function compileToolArgsValidator(schema: Record): ToolArgsValidator { - return { schema }; + return ajvFor(schema).compile(schema) as ToolArgsValidator; } export function validateToolArgs(validator: ToolArgsValidator, args: JsonType): string | null { - const errors: ValidationError[] = []; - validateSchema(validator.schema, args, '', errors); - if (errors.length === 0) { + const valid = validator(args); + if (valid) { return null; } + const errors = validator.errors ?? []; + if (errors.length === 0) { + return 'Tool parameter validation failed'; + } + return errors.map((error) => formatValidationError(error)).join('; '); } - -function validateSchema( - schema: Record, - value: unknown, - instancePath: string, - errors: ValidationError[], -): void { - if (schema['const'] !== undefined && !deepEqual(value, schema['const'])) { - errors.push({ keyword: 'const', instancePath, message: 'must be equal to constant' }); - } - - const enumValues = schema['enum']; - if (Array.isArray(enumValues) && !enumValues.some((item) => deepEqual(item, value))) { - errors.push({ keyword: 'enum', instancePath, message: 'must be equal to one of the allowed values' }); - } - - const type = schema['type']; - if (type !== undefined && !matchesType(value, type)) { - errors.push({ keyword: 'type', instancePath, message: `must be ${formatType(type)}` }); - return; - } - - validateCombinators(schema, value, instancePath, errors); - - if (isPlainObject(value)) { - validateObject(schema, value, instancePath, errors); - } - - if (Array.isArray(value)) { - validateArray(schema, value, instancePath, errors); - } - - if (typeof value === 'string') { - validateString(schema, value, instancePath, errors); - } - - if (typeof value === 'number') { - validateNumber(schema, value, instancePath, errors); - } -} - -function validateObject( - schema: Record, - value: Record, - instancePath: string, - errors: ValidationError[], -): void { - const required = schema['required']; - if (Array.isArray(required)) { - for (const property of required) { - if (typeof property === 'string' && !(property in value)) { - errors.push({ - keyword: 'required', - instancePath, - message: `must have required property '${property}'`, - params: { missingProperty: property }, - }); - } - } - } - - const properties = schema['properties']; - if (isSchemaMap(properties)) { - for (const [property, propertySchema] of Object.entries(properties)) { - if (property in value) { - validateSchema(propertySchema, value[property], `${instancePath}/${escapePath(property)}`, errors); - } - } - } - - if (schema['additionalProperties'] === false && isSchemaMap(properties)) { - for (const property of Object.keys(value)) { - if (!(property in properties)) { - errors.push({ - keyword: 'additionalProperties', - instancePath, - message: `must NOT have additional property '${property}'`, - params: { additionalProperty: property }, - }); - } - } - } -} - -function validateArray( - schema: Record, - value: unknown[], - instancePath: string, - errors: ValidationError[], -): void { - const minItems = schema['minItems']; - if (typeof minItems === 'number' && value.length < minItems) { - errors.push({ keyword: 'minItems', instancePath, message: `must NOT have fewer than ${minItems} items` }); - } - - const maxItems = schema['maxItems']; - if (typeof maxItems === 'number' && value.length > maxItems) { - errors.push({ keyword: 'maxItems', instancePath, message: `must NOT have more than ${maxItems} items` }); - } - - const items = schema['items']; - if (isSchema(items)) { - value.forEach((item, index) => { - validateSchema(items, item, `${instancePath}/${index}`, errors); - }); - } else if (Array.isArray(items)) { - items.forEach((itemSchema, index) => { - if (isSchema(itemSchema) && index < value.length) { - validateSchema(itemSchema, value[index], `${instancePath}/${index}`, errors); - } - }); - } -} - -function validateString( - schema: Record, - value: string, - instancePath: string, - errors: ValidationError[], -): void { - const minLength = schema['minLength']; - if (typeof minLength === 'number' && value.length < minLength) { - errors.push({ keyword: 'minLength', instancePath, message: `must NOT have fewer than ${minLength} characters` }); - } - - const maxLength = schema['maxLength']; - if (typeof maxLength === 'number' && value.length > maxLength) { - errors.push({ keyword: 'maxLength', instancePath, message: `must NOT have more than ${maxLength} characters` }); - } - - const pattern = schema['pattern']; - if (typeof pattern === 'string' && !new RegExp(pattern).test(value)) { - errors.push({ keyword: 'pattern', instancePath, message: `must match pattern "${pattern}"` }); - } -} - -function validateNumber( - schema: Record, - value: number, - instancePath: string, - errors: ValidationError[], -): void { - const minimum = schema['minimum']; - if (typeof minimum === 'number' && value < minimum) { - errors.push({ keyword: 'minimum', instancePath, message: `must be >= ${minimum}` }); - } - - const maximum = schema['maximum']; - if (typeof maximum === 'number' && value > maximum) { - errors.push({ keyword: 'maximum', instancePath, message: `must be <= ${maximum}` }); - } -} - -function validateCombinators( - schema: Record, - value: unknown, - instancePath: string, - errors: ValidationError[], -): void { - const allOf = schema['allOf']; - if (Array.isArray(allOf)) { - for (const child of allOf) { - if (isSchema(child)) validateSchema(child, value, instancePath, errors); - } - } - - const anyOf = schema['anyOf']; - if (Array.isArray(anyOf) && !anyOf.some((child) => isSchema(child) && schemaMatches(child, value))) { - errors.push({ keyword: 'anyOf', instancePath, message: 'must match at least one schema' }); - } - - const oneOf = schema['oneOf']; - if (Array.isArray(oneOf)) { - const matches = oneOf.filter((child) => isSchema(child) && schemaMatches(child, value)).length; - if (matches !== 1) { - errors.push({ keyword: 'oneOf', instancePath, message: 'must match exactly one schema' }); - } - } -} - -function schemaMatches(schema: Record, value: unknown): boolean { - const errors: ValidationError[] = []; - validateSchema(schema, value, '', errors); - return errors.length === 0; -} - -function matchesType(value: unknown, type: unknown): boolean { - if (Array.isArray(type)) return type.some((item) => matchesType(value, item)); - switch (type) { - case 'null': - return value === null; - case 'boolean': - return typeof value === 'boolean'; - case 'integer': - return Number.isInteger(value); - case 'number': - return typeof value === 'number' && Number.isFinite(value); - case 'string': - return typeof value === 'string'; - case 'array': - return Array.isArray(value); - case 'object': - return isPlainObject(value); - default: - return true; - } -} - -function formatType(type: unknown): string { - return Array.isArray(type) ? type.map((item) => String(item)).join(' or ') : String(type); -} - -function isSchema(value: unknown): value is Record { - return isPlainObject(value); -} - -function isSchemaMap(value: unknown): value is Record> { - return isPlainObject(value) && Object.values(value).every((child) => isSchema(child)); -} - -function isPlainObject(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} - -function escapePath(value: string): string { - return value.replaceAll('~', '~0').replaceAll('/', '~1'); -} - -function deepEqual(left: unknown, right: unknown): boolean { - return JSON.stringify(left) === JSON.stringify(right); -} diff --git a/packages/agent-core-v2/test/_base/tools/args-validator.test.ts b/packages/agent-core-v2/test/_base/tools/args-validator.test.ts new file mode 100644 index 000000000..2404359c0 --- /dev/null +++ b/packages/agent-core-v2/test/_base/tools/args-validator.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from 'vitest'; + +import { + compileToolArgsValidator, + type JsonType, + validateToolArgs, +} from '#/_base/tools/args-validator'; + +function validate(schema: Record, value: JsonType): string | null { + return validateToolArgs(compileToolArgsValidator(schema), value); +} + +describe('args-validator (Ajv, format support)', () => { + it('validates string format (email)', () => { + const schema = { type: 'string', format: 'email' }; + expect(validate(schema, 'a@b.com')).toBeNull(); + expect(validate(schema, 'not-an-email')).toContain('format'); + }); + + it('validates string format (uri)', () => { + const schema = { type: 'string', format: 'uri' }; + expect(validate(schema, 'https://example.com/x')).toBeNull(); + expect(validate(schema, 'not a uri')).toContain('format'); + }); + + it('format is ignored on non-strings', () => { + const schema = { type: 'number', format: 'email' }; + expect(validate(schema, 42)).toBeNull(); + }); + + it('keeps required / additionalProperties messages', () => { + expect(validate({ type: 'object', required: ['a'] }, {})).toContain( + "must have required property 'a'", + ); + expect( + validate({ type: 'object', properties: { a: {} }, additionalProperties: false }, { b: 1 }), + ).toContain("must NOT have additional property 'b'"); + }); + + it('still validates the JSON-Schema subset (type / enum / const)', () => { + expect(validate({ type: 'integer' }, 1.5)).toContain('must be integer'); + expect(validate({ enum: ['a', 'b'] }, 'c')).toContain('allowed values'); + expect(validate({ const: 'x' }, 'y')).toContain('constant'); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9d8c14a4c..113511148 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -59,7 +59,7 @@ importers: version: 2.13.1 tsdown: specifier: 0.22.0 - version: 0.22.0(@arethetypeswrong/core@0.18.2)(publint@0.3.18)(tsx@4.21.0)(typescript@6.0.2)(unrun@0.2.34(@emnapi/core@1.10.0)(@emnapi/runtime@1.10.0))(vue-tsc@3.2.9(typescript@6.0.2)) + version: 0.22.0(@arethetypeswrong/core@0.18.2)(publint@0.3.18)(tsx@4.21.0)(typescript@6.0.2)(unrun@0.2.34)(vue-tsc@3.2.9(typescript@6.0.2)) tsx: specifier: ^4.21.0 version: 4.21.0 @@ -517,6 +517,12 @@ importers: '@mozilla/readability': specifier: ^0.6.0 version: 0.6.0 + ajv: + specifier: ^8.18.0 + version: 8.18.0 + ajv-formats: + specifier: ^3.0.1 + version: 3.0.1(ajv@8.18.0) chokidar: specifier: ^4.0.3 version: 4.0.3 @@ -9282,6 +9288,11 @@ snapshots: '@mozilla/readability@0.6.0': {} + '@napi-rs/wasm-runtime@1.1.4': + dependencies: + '@tybys/wasm-util': 0.10.1 + optional: true + '@napi-rs/wasm-runtime@1.1.4(@emnapi/core@1.10.0)(@emnapi/runtime@1.10.0)': dependencies: '@emnapi/core': 1.10.0 @@ -9542,6 +9553,14 @@ snapshots: '@rolldown/binding-openharmony-arm64@1.0.1': optional: true + '@rolldown/binding-wasm32-wasi@1.0.0-rc.12': + dependencies: + '@napi-rs/wasm-runtime': 1.1.4 + transitivePeerDependencies: + - '@emnapi/core' + - '@emnapi/runtime' + optional: true + '@rolldown/binding-wasm32-wasi@1.0.0-rc.12(@emnapi/core@1.10.0)(@emnapi/runtime@1.10.0)': dependencies: '@napi-rs/wasm-runtime': 1.1.4(@emnapi/core@1.10.0)(@emnapi/runtime@1.10.0) @@ -10245,7 +10264,7 @@ snapshots: obug: 2.1.1 std-env: 4.0.0 tinyrainbow: 3.1.0 - vitest: 4.1.4(@types/node@22.19.17)(@vitest/coverage-v8@4.1.4)(jsdom@25.0.1)(vite@8.0.8(@types/node@22.19.17)(esbuild@0.27.7)(jiti@2.6.1)(tsx@4.21.0)(yaml@2.8.3)) + vitest: 4.1.4(@types/node@22.19.17)(@vitest/coverage-v8@4.1.4)(jsdom@25.0.1)(vite@6.4.2(@types/node@22.19.17)(jiti@2.6.1)(lightningcss@1.32.0)(tsx@4.21.0)(yaml@2.8.3)) '@vitest/expect@4.1.4': dependencies: @@ -14088,6 +14107,31 @@ snapshots: transitivePeerDependencies: - oxc-resolver + rolldown@1.0.0-rc.12: + dependencies: + '@oxc-project/types': 0.122.0 + '@rolldown/pluginutils': 1.0.0-rc.12 + optionalDependencies: + '@rolldown/binding-android-arm64': 1.0.0-rc.12 + '@rolldown/binding-darwin-arm64': 1.0.0-rc.12 + '@rolldown/binding-darwin-x64': 1.0.0-rc.12 + '@rolldown/binding-freebsd-x64': 1.0.0-rc.12 + '@rolldown/binding-linux-arm-gnueabihf': 1.0.0-rc.12 + '@rolldown/binding-linux-arm64-gnu': 1.0.0-rc.12 + '@rolldown/binding-linux-arm64-musl': 1.0.0-rc.12 + '@rolldown/binding-linux-ppc64-gnu': 1.0.0-rc.12 + '@rolldown/binding-linux-s390x-gnu': 1.0.0-rc.12 + '@rolldown/binding-linux-x64-gnu': 1.0.0-rc.12 + '@rolldown/binding-linux-x64-musl': 1.0.0-rc.12 + '@rolldown/binding-openharmony-arm64': 1.0.0-rc.12 + '@rolldown/binding-wasm32-wasi': 1.0.0-rc.12 + '@rolldown/binding-win32-arm64-msvc': 1.0.0-rc.12 + '@rolldown/binding-win32-x64-msvc': 1.0.0-rc.12 + transitivePeerDependencies: + - '@emnapi/core' + - '@emnapi/runtime' + optional: true + rolldown@1.0.0-rc.12(@emnapi/core@1.10.0)(@emnapi/runtime@1.10.0): dependencies: '@oxc-project/types': 0.122.0 @@ -14844,6 +14888,35 @@ snapshots: - oxc-resolver - vue-tsc + tsdown@0.22.0(@arethetypeswrong/core@0.18.2)(publint@0.3.18)(tsx@4.21.0)(typescript@6.0.2)(unrun@0.2.34)(vue-tsc@3.2.9(typescript@6.0.2)): + dependencies: + ansis: 4.2.0 + cac: 7.0.0 + defu: 6.1.7 + empathic: 2.0.0 + hookable: 6.1.1 + import-without-cache: 0.4.0 + obug: 2.1.1 + picomatch: 4.0.4 + rolldown: 1.0.1 + rolldown-plugin-dts: 0.25.1(rolldown@1.0.1)(typescript@6.0.2)(vue-tsc@3.2.9(typescript@6.0.2)) + semver: 7.7.4 + tinyexec: 1.1.2 + tinyglobby: 0.2.16 + tree-kill: 1.2.2 + unconfig-core: 7.5.0 + optionalDependencies: + '@arethetypeswrong/core': 0.18.2 + publint: 0.3.18 + tsx: 4.21.0 + typescript: 6.0.2 + unrun: 0.2.34 + transitivePeerDependencies: + - '@ts-macro/tsc' + - '@typescript/native-preview' + - oxc-resolver + - vue-tsc + tslib@2.8.1: {} tsx@4.21.0: @@ -15007,6 +15080,14 @@ snapshots: picomatch: 4.0.4 webpack-virtual-modules: 0.6.2 + unrun@0.2.34: + dependencies: + rolldown: 1.0.0-rc.12 + transitivePeerDependencies: + - '@emnapi/core' + - '@emnapi/runtime' + optional: true + unrun@0.2.34(@emnapi/core@1.10.0)(@emnapi/runtime@1.10.0): dependencies: rolldown: 1.0.0-rc.12(@emnapi/core@1.10.0)(@emnapi/runtime@1.10.0)