From 464ed2cad3fb53e649c040136be6b2e75400723c Mon Sep 17 00:00:00 2001 From: Aska Date: Sun, 6 Sep 2026 17:11:52 +0800 Subject: [PATCH] =?UTF-8?q?feat(tool):=20Top5#2=20=E5=AE=8C=E6=95=B4=20JSO?= =?UTF-8?q?N-Schema=20=E6=A0=A1=E9=AA=8C=20+=20=E7=B1=BB=E5=9E=8B=E5=BC=BA?= =?UTF-8?q?=E8=BD=AC=20+=20=E9=94=99=E8=AF=AF=E8=B7=AF=E5=BE=84=20+=20defa?= =?UTF-8?q?ults=EF=BC=88=E5=AF=B9=E9=BD=90=20OpenClaw=20validation?= =?UTF-8?q?=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 引入 github.com/santhosh-tekuri/jsonschema/v5（纯 Go，OpenClaw/MCP SDK 同思路）。 - validate.go 重写：validateArgs 用 jsonschema Compile+Validate 替换手写子集（required单层）， 支持 required/properties/嵌套 object/array items/additionalProperties/allOf/anyOf/oneOf/ 类型联合/enum/min-max/pattern/defaults；编译器缓存 sync.Map；Draft7。 - coerce.go：对齐 OpenClaw coerceWithJsonSchema 类型强转（string→number/integer/boolean/ array/object，number/bool→string；union 处理；object/array 递归）。 - defaults.go：applyDefaults 缺失属性按 default 填（含嵌套），先填默认再强转再校验。 - 错误路径：ValidationError.InstanceLocation → 带路径报告（/count: expected integer...）。 - 测试：tool_test.go 更新 required 语义 + 补强转/default/错误路径用例。 go build/vet/test 全绿 + windows/darwin 交叉编译 + gofmt。 --- go.mod | 9 +- go.sum | 16 +- internal/runtime/tool/coerce.go | 407 +++++++++++++++++++++++++++++ internal/runtime/tool/defaults.go | 81 ++++++ internal/runtime/tool/tool_test.go | 95 ++++++- internal/runtime/tool/validate.go | 266 ++++++++----------- 6 files changed, 718 insertions(+), 156 deletions(-) create mode 100644 internal/runtime/tool/coerce.go create mode 100644 internal/runtime/tool/defaults.go diff --git a/go.mod b/go.mod index 633c78c..fd9d2e7 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,10 @@ module git.yeij.top/AskaEth/Cyrene_v2 go 1.26.0 require ( + github.com/gorilla/websocket v1.5.3 + github.com/jackc/pgx/v5 v5.10.0 + github.com/robfig/cron/v3 v3.0.1 + github.com/santhosh-tekuri/jsonschema/v5 v5.3.1 gopkg.in/yaml.v3 v3.0.1 modernc.org/sqlite v1.58.0 ) @@ -10,15 +14,14 @@ require ( require ( github.com/dustin/go-humanize v1.0.1 // indirect github.com/google/uuid v1.6.0 // indirect - github.com/gorilla/websocket v1.5.3 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect - github.com/jackc/pgx/v5 v5.10.0 // indirect github.com/jackc/puddle/v2 v2.2.2 // indirect + github.com/kr/text v0.2.0 // indirect github.com/mattn/go-isatty v0.0.24 // indirect github.com/ncruces/go-strftime v1.0.0 // indirect github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect - github.com/robfig/cron/v3 v3.0.1 // indirect + github.com/rogpeppe/go-internal v1.16.0 // indirect golang.org/x/sync v0.22.0 // indirect golang.org/x/sys v0.47.0 // indirect golang.org/x/text v0.29.0 // indirect diff --git a/go.sum b/go.sum index 35152ec..2049d04 100644 --- a/go.sum +++ b/go.sum @@ -1,4 +1,7 @@ +github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= github.com/google/pprof v0.0.0-20260802141513-ef3492d7dac3 h1:LMLX+LgTNWpfvCBdFebv6EsYotImrt/Ppc5cXIriCSo= @@ -17,18 +20,29 @@ github.com/jackc/pgx/v5 v5.10.0 h1:VhSvgU2jSli8o3AqIEOTJr7rZwAEUVo4E4XhR94Zfr0= github.com/jackc/pgx/v5 v5.10.0/go.mod h1:mal1tBGAFfLHvZzaYh77YS/eC6IX9OWbRV1QIIM0Jn4= github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo= github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4= +github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0= +github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/mattn/go-isatty v0.0.24 h1:tGZZoVgT/KiqK1c8ocVLeDS8BSWMRd47J3Lbz7vsReI= github.com/mattn/go-isatty v0.0.24/go.mod h1:nMCL3Zebbrt45jsMDgnfIwz6ydEQApk5oEI3HqDio6A= github.com/ncruces/go-strftime v1.0.0 h1:HMFp8mLCTPp341M/ZnA4qaf7ZlsbTc+miZjCLOFAw7w= github.com/ncruces/go-strftime v1.0.0/go.mod h1:Fwc5htZGVVkseilnfgOVb9mKy6w1naJmn9CehxcKcls= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec h1:W09IVJc94icq4NjY3clb7Lk8O1qJ8BdBEF8z0ibU0rE= github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo= github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs= github.com/robfig/cron/v3 v3.0.1/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro= +github.com/rogpeppe/go-internal v1.16.0 h1:O9DK+vNMDVGLr2BeZqmpLeMjiMNkuXfcqntWbZV6S5g= +github.com/rogpeppe/go-internal v1.16.0/go.mod h1:DrUVZyrJU+txYW5/1kwtXQSMFio52ZOxX7yM1VHvnxs= +github.com/santhosh-tekuri/jsonschema/v5 v5.3.1 h1:lZUw3E0/J3roVtGQ+SCrUrg3ON6NgVqpn3+iol9aGu4= +github.com/santhosh-tekuri/jsonschema/v5 v5.3.1/go.mod h1:uToXkOrWAZ6/Oc07xWQrPOhJotwFIyu2bBVN41fcDUY= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= golang.org/x/mod v0.38.0 h1:MECBjubtXD7yj4HrhIUcywNaGeNVUdfVnxmPajOk4yk= golang.org/x/mod v0.38.0/go.mod h1:V6Xz0pq8TQ3dGqVQ1FVHuelZpAL0uNhSkk9ogYP3c40= golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= @@ -39,9 +53,9 @@ golang.org/x/text v0.29.0 h1:1neNs90w9YzJ9BocxfsQNHKuAT4pkghyXc4nhZ6sJvk= golang.org/x/text v0.29.0/go.mod h1:7MhJOA9CD2qZyOKYazxdYMF85OwPdEr9jTtBpO7ydH4= golang.org/x/tools v0.48.0 h1:3+hClM1aLL5mjMKm5ovokw9epgRXPuu2tILgismM6RE= golang.org/x/tools v0.48.0/go.mod h1:08xX0orndb/F7jJxGDicx061tyd5pcMto75YMAXr6lk= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/runtime/tool/coerce.go b/internal/runtime/tool/coerce.go new file mode 100644 index 0000000..e5e7496 --- /dev/null +++ b/internal/runtime/tool/coerce.go @@ -0,0 +1,407 @@ +package tool + +import ( + "encoding/json" + "math" + "regexp" + "strconv" + "strings" +) + +// maxJSONCoerceLen caps how long a string we will attempt to JSON-parse when +// coercing a model-emitted string into an array or object. It mirrors the cap +// OpenClaw uses so a huge string argument fails fast instead of being parsed. +const maxJSONCoerceLen = 64 * 1024 + +// jsonNumberRe matches a JSON number token; used to decide whether a model +// string such as "3.5" may be safely coerced to a number. +var jsonNumberRe = regexp.MustCompile(`^[+-]?(?:\d+\.?\d*|\.\d+)(?:[eE][+-]?\d+)?$`) + +// coerceWithSchema mirrors OpenClaw's coerceWithJsonSchema: it walks the +// schema's allOf/anyOf/oneOf, coerces a value whose type does not yet match, +// then recurses into object properties and array items. It returns the value it +// was given (mutating maps/slices in place) so validation sees coerced types. +func coerceWithSchema(value any, schema map[string]any) any { + next := value + + if arr, ok := schema["allOf"].([]any); ok { + for _, raw := range arr { + if s, ok := raw.(map[string]any); ok { + next = coerceWithSchema(next, s) + } + } + } + if arr, ok := schema["anyOf"].([]any); ok { + next = coerceWithUnion(next, arr) + } + if arr, ok := schema["oneOf"].([]any); ok { + next = coerceWithUnion(next, arr) + } + + types := schemaTypes(schema) + if len(types) > 0 && !matchesAnyType(next, types) { + for _, t := range types { + if coerced, ok := coercePrimitiveByType(next, t); ok { + next = coerced + break + } + } + } + + if isObject(next) { + applyObjectCoercion(next.(map[string]any), schema) + } + if isArray(next) { + applyArrayCoercion(next.([]any), schema) + } + return next +} + +// coerceWithUnion tries each union member in turn, cloning the value and +// coercing against that member, returning the first candidate that validates. +// A null value is returned as-is when some member accepts null so it is not +// mangled into the empty string by a string branch. +func coerceWithUnion(value any, schemas []any) any { + if value == nil { + for _, raw := range schemas { + s, ok := raw.(map[string]any) + if !ok || !contains(schemaTypes(s), "null") { + continue + } + if compiled, err := compileSchema(s); err == nil && compiled.Validate(value) == nil { + return value + } + } + } + for _, raw := range schemas { + s, ok := raw.(map[string]any) + if !ok { + continue + } + candidate := coerceWithSchema(deepCopy(value), s) + if compiled, err := compileSchema(s); err == nil && compiled.Validate(candidate) == nil { + return candidate + } + } + return value +} + +// applyObjectCoercion recurses into declared properties and, when +// additionalProperties is itself a schema, into undeclared keys. +func applyObjectCoercion(value map[string]any, schema map[string]any) { + props, _ := schema["properties"].(map[string]any) + for name, raw := range props { + ps, ok := raw.(map[string]any) + if !ok { + continue + } + if val, present := value[name]; present { + value[name] = coerceWithSchema(val, ps) + } + } + if ap, ok := schema["additionalProperties"].(map[string]any); ok { + for name, val := range value { + if _, defined := props[name]; !defined { + value[name] = coerceWithSchema(val, ap) + } + } + } +} + +// applyArrayCoercion recurses into array items, handling the schematic, +// tuple-in-items and 2020-12 prefixItems forms. +func applyArrayCoercion(value []any, schema map[string]any) { + switch items := schema["items"].(type) { + case map[string]any: + for i := range value { + value[i] = coerceWithSchema(value[i], items) + } + case []any: + for i, item := range items { + if i >= len(value) { + break + } + if is, ok := item.(map[string]any); ok { + value[i] = coerceWithSchema(value[i], is) + } + } + } + if prefix, ok := schema["prefixItems"].([]any); ok { + for i, item := range prefix { + if i >= len(value) { + break + } + if is, ok := item.(map[string]any); ok { + value[i] = coerceWithSchema(value[i], is) + } + } + } +} + +// coercePrimitiveByType maps a requested JSON type to its coercion helper; when +// no helper can apply it returns the value unchanged. +func coercePrimitiveByType(value any, typ string) (any, bool) { + switch typ { + case "number": + return coerceNumber(value) + case "integer": + return coerceInteger(value) + case "boolean": + return coerceBoolean(value) + case "string": + return coerceString(value) + case "array": + return coerceArray(value) + case "object": + return coerceObject(value) + case "null": + return coerceNull(value) + default: + return value, false + } +} + +func coerceNumber(value any) (any, bool) { + switch v := value.(type) { + case nil: + return 0, true + case string: + if trimmed := strings.TrimSpace(v); trimmed != "" { + if n, ok := parseNumber(trimmed); ok { + return n, true + } + } + case bool: + if v { + return 1, true + } + return 0, true + } + return value, false +} + +func coerceInteger(value any) (any, bool) { + switch v := value.(type) { + case nil: + return 0, true + case string: + if trimmed := strings.TrimSpace(v); trimmed != "" { + if n, ok := parseNumber(trimmed); ok && math.Trunc(n) == n { + return n, true + } + } + case bool: + if v { + return 1, true + } + return 0, true + } + return value, false +} + +func coerceBoolean(value any) (any, bool) { + switch v := value.(type) { + case nil: + return false, true + case string: + if v == "true" { + return true, true + } + if v == "false" { + return false, true + } + case float64: + if v == 1 { + return true, true + } + if v == 0 { + return false, true + } + } + return value, false +} + +func coerceString(value any) (any, bool) { + switch v := value.(type) { + case nil: + return "", true + case bool: + return strconv.FormatBool(v), true + case float64: + return strconv.FormatFloat(v, 'f', -1, 64), true + case float32: + return strconv.FormatFloat(float64(v), 'f', -1, 32), true + case int: + return strconv.Itoa(v), true + case int64: + return strconv.FormatInt(v, 10), true + case json.Number: + return v.String(), true + } + return value, false +} + +func coerceArray(value any) (any, bool) { + s, ok := value.(string) + if !ok || len(s) > maxJSONCoerceLen || strings.TrimSpace(s) == "" { + return value, false + } + var parsed any + if err := json.Unmarshal([]byte(s), &parsed); err != nil { + return value, false + } + if arr, ok := parsed.([]any); ok { + return arr, true + } + return value, false +} + +func coerceObject(value any) (any, bool) { + s, ok := value.(string) + if !ok || len(s) > maxJSONCoerceLen || strings.TrimSpace(s) == "" { + return value, false + } + var parsed any + if err := json.Unmarshal([]byte(s), &parsed); err != nil { + return value, false + } + if obj, ok := parsed.(map[string]any); ok { + return obj, true + } + return value, false +} + +func coerceNull(value any) (any, bool) { + switch v := value.(type) { + case string: + if strings.TrimSpace(v) == "" { + return nil, true + } + case float64: + if v == 0 { + return nil, true + } + case bool: + if !v { + return nil, true + } + } + return value, false +} + +// parseNumber parses a strictly JSON-shaped numeric token (no hex, Inf, NaN). +func parseNumber(s string) (float64, bool) { + if !jsonNumberRe.MatchString(s) { + return 0, false + } + f, err := strconv.ParseFloat(s, 64) + if err != nil || math.IsInf(f, 0) || math.IsNaN(f) { + return 0, false + } + return f, true +} + +// schemaTypes returns the type names declared by a schema (string or union). +func schemaTypes(schema map[string]any) []string { + switch t := schema["type"].(type) { + case string: + return []string{t} + case []any: + out := make([]string, 0, len(t)) + for _, item := range t { + if s, ok := item.(string); ok { + out = append(out, s) + } + } + return out + default: + return nil + } +} + +// matchesAnyType reports whether value already satisfies one of the type names. +func matchesAnyType(value any, types []string) bool { + for _, t := range types { + if matchesJSONType(value, t) { + return true + } + } + return false +} + +// matchesJSONType reports whether value satisfies a single JSON type name. +func matchesJSONType(value any, typ string) bool { + switch typ { + case "number": + return isNumber(value) + case "integer": + return isInteger(value) + case "boolean": + _, ok := value.(bool) + return ok + case "string": + _, ok := value.(string) + return ok + case "null": + return value == nil + case "array": + return isArray(value) + case "object": + return isObject(value) + default: + return false + } +} + +// isObject reports whether v is a JSON object (map[string]any). +func isObject(v any) bool { + _, ok := v.(map[string]any) + return ok +} + +// isArray reports whether v is a JSON array ([]any). +func isArray(v any) bool { + _, ok := v.([]any) + return ok +} + +// contains reports whether slice has s. +func contains(slice []string, s string) bool { + for _, item := range slice { + if item == s { + return true + } + } + return false +} + +// isNumber reports whether v is any integer or floating-point numeric type. +func isNumber(v any) bool { + switch v.(type) { + case json.Number, float32, float64, + int, int8, int16, int32, int64, + uint, uint8, uint16, uint32, uint64: + return true + default: + return false + } +} + +// isInteger reports whether v is an integer type or a floating-point / json +// Number that holds an integral value. +func isInteger(v any) bool { + switch x := v.(type) { + case int, int8, int16, int32, int64, + uint, uint8, uint16, uint32, uint64: + return true + case float32: + return math.Trunc(float64(x)) == float64(x) + case float64: + return math.Trunc(x) == x + case json.Number: + _, err := strconv.ParseInt(x.String(), 10, 64) + return err == nil + default: + return false + } +} diff --git a/internal/runtime/tool/defaults.go b/internal/runtime/tool/defaults.go new file mode 100644 index 0000000..c0847b5 --- /dev/null +++ b/internal/runtime/tool/defaults.go @@ -0,0 +1,81 @@ +package tool + +// applyDefaults fills absent properties with their schema "default", recursing +// into objects and arrays so nested defaults are honoured too. +func applyDefaults(schema map[string]any, args map[string]any) { + props, _ := schema["properties"].(map[string]any) + for name, raw := range props { + ps, ok := raw.(map[string]any) + if !ok { + continue + } + if rawDefault, has := ps["default"]; has { + if _, present := args[name]; !present { + args[name] = deepCopy(rawDefault) + } + } + if val, present := args[name]; present { + applyDefaultsToValue(ps, val) + } + } +} + +// applyDefaultsToValue recurses into a present argument value to fill nested +// defaults according to the sub-schema. +func applyDefaultsToValue(schema map[string]any, val any) { + switch v := val.(type) { + case map[string]any: + applyDefaults(schema, v) + case []any: + applyDefaultsToArray(schema, v) + } +} + +// applyDefaultsToArray fills defaults for array items, handling the schematic, +// tuple-in-items and 2020-12 prefixItems forms. +func applyDefaultsToArray(schema map[string]any, v []any) { + switch items := schema["items"].(type) { + case map[string]any: + for _, item := range v { + applyDefaultsToValue(items, item) + } + case []any: + for i, item := range v { + if i < len(items) { + if is, ok := items[i].(map[string]any); ok { + applyDefaultsToValue(is, item) + } + } + } + } + if prefix, ok := schema["prefixItems"].([]any); ok { + for i, item := range v { + if i < len(prefix) { + if is, ok := prefix[i].(map[string]any); ok { + applyDefaultsToValue(is, item) + } + } + } + } +} + +// deepCopy returns a structurally independent copy of a JSON value so defaults +// taken from a compiled schema are not corrupted by later in-place coercion. +func deepCopy(v any) any { + switch vv := v.(type) { + case map[string]any: + out := make(map[string]any, len(vv)) + for k, val := range vv { + out[k] = deepCopy(val) + } + return out + case []any: + out := make([]any, len(vv)) + for i, item := range vv { + out[i] = deepCopy(item) + } + return out + default: + return vv + } +} diff --git a/internal/runtime/tool/tool_test.go b/internal/runtime/tool/tool_test.go index ed6d4a8..4a71619 100644 --- a/internal/runtime/tool/tool_test.go +++ b/internal/runtime/tool/tool_test.go @@ -140,7 +140,7 @@ func TestInvoke_MissingRequired(t *testing.T) { if !errors.Is(err, ErrInvalidArgs) { t.Fatalf("Invoke error = %v, want ErrInvalidArgs", err) } - if !strings.Contains(err.Error(), "missing required argument") { + if !strings.Contains(err.Error(), "missing properties") { t.Fatalf("error message = %q, want missing-required hint", err.Error()) } } @@ -354,13 +354,20 @@ func TestValidateArgs_Direct(t *testing.T) { {name: "required missing", schema: map[string]any{"required": []any{"r"}}, args: map[string]any{}, wantErr: true}, {name: "required present", schema: map[string]any{"required": []any{"r"}}, args: map[string]any{"r": 1}, wantErr: false}, {name: "type string ok", schema: props("string"), args: map[string]any{"v": "x"}, wantErr: false}, - {name: "type string mismatch", schema: props("string"), args: map[string]any{"v": 1}, wantErr: true}, + {name: "type string mismatch", schema: props("string"), args: map[string]any{"v": []any{"x"}}, wantErr: true}, {name: "type array ok", schema: props("array"), args: map[string]any{"v": []any{1}}, wantErr: false}, {name: "type object ok", schema: props("object"), args: map[string]any{"v": map[string]any{}}, wantErr: false}, {name: "type boolean ok", schema: props("boolean"), args: map[string]any{"v": true}, wantErr: false}, {name: "type number ok", schema: props("number"), args: map[string]any{"v": 2.5}, wantErr: false}, {name: "integer as number ok", schema: props("number"), args: map[string]any{"v": 2}, wantErr: false}, {name: "unknown field ignored", schema: props("string"), args: map[string]any{"v": "x", "u": 9}, wantErr: false}, + {name: "coerce string to number", schema: props("number"), args: map[string]any{"v": "3"}, wantErr: false}, + {name: "coerce string to integer", schema: props("integer"), args: map[string]any{"v": "3"}, wantErr: false}, + {name: "coerce string to boolean", schema: props("boolean"), args: map[string]any{"v": "true"}, wantErr: false}, + {name: "coerce string to array", schema: props("array"), args: map[string]any{"v": "[1,2]"}, wantErr: false}, + {name: "coerce string to object", schema: props("object"), args: map[string]any{"v": `{"a":1}`}, wantErr: false}, + {name: "bad coercion still error", schema: props("number"), args: map[string]any{"v": "abc"}, wantErr: true}, + {name: "default fills missing", schema: map[string]any{"type": "object", "properties": map[string]any{"v": map[string]any{"type": "string", "default": "x"}}}, args: map[string]any{}, wantErr: false}, } for _, tt := range cases { tt := tt @@ -373,3 +380,87 @@ func TestValidateArgs_Direct(t *testing.T) { }) } } + +func TestValidateArgs_CoercionValues(t *testing.T) { + t.Parallel() + schema := func(typ string) map[string]any { + return map[string]any{"type": "object", "properties": map[string]any{"v": map[string]any{"type": typ}}} + } + cases := []struct { + name string + typ string + input any + want any + }{ + {name: "string to number", typ: "number", input: "3", want: float64(3)}, + {name: "string to integer", typ: "integer", input: "4", want: float64(4)}, + {name: "string to boolean true", typ: "boolean", input: "true", want: true}, + {name: "number to string", typ: "string", input: 7, want: "7"}, + } + for _, tt := range cases { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + args := map[string]any{"v": tt.input} + if err := validateArgs(schema(tt.typ), args); err != nil { + t.Fatalf("validateArgs error = %v, want nil", err) + } + if !reflect.DeepEqual(args["v"], tt.want) { + t.Fatalf("coerced v = %#v, want %#v", args["v"], tt.want) + } + }) + } +} + +func TestValidateArgs_Defaults(t *testing.T) { + t.Parallel() + schema := map[string]any{ + "type": "object", + "properties": map[string]any{ + "q": map[string]any{"type": "string", "default": "hello"}, + }, + } + args := map[string]any{} + if err := validateArgs(schema, args); err != nil { + t.Fatalf("validateArgs error = %v, want nil", err) + } + if got := args["q"]; got != "hello" { + t.Fatalf("args[q] = %#v, want default hello", got) + } +} + +func TestValidateArgs_ErrorPath(t *testing.T) { + t.Parallel() + nested := map[string]any{ + "type": "object", + "properties": map[string]any{ + "user": map[string]any{ + "type": "object", + "properties": map[string]any{ + "age": map[string]any{"type": "integer"}, + }, + }, + }, + "required": []any{"user"}, + } + t.Run("nested property path", func(t *testing.T) { + t.Parallel() + err := validateArgs(nested, map[string]any{"user": map[string]any{"age": "x"}}) + if err == nil { + t.Fatalf("validateArgs error = nil, want failure") + } + if !strings.Contains(err.Error(), "/user/age") { + t.Fatalf("error = %q, want instance path /user/age", err.Error()) + } + }) + t.Run("required property path", func(t *testing.T) { + t.Parallel() + err := validateArgs(nested, map[string]any{}) + if err == nil { + t.Fatalf("validateArgs error = nil, want failure") + } + if !strings.Contains(err.Error(), "/user") { + t.Fatalf("error = %q, want instance path /user", err.Error()) + } + }) +} diff --git a/internal/runtime/tool/validate.go b/internal/runtime/tool/validate.go index c099fe0..205c85a 100644 --- a/internal/runtime/tool/validate.go +++ b/internal/runtime/tool/validate.go @@ -1,19 +1,37 @@ package tool import ( + "bytes" "encoding/json" + "errors" "fmt" - "math" - "reflect" - "strconv" + "regexp" + "strings" + "sync" + + "github.com/santhosh-tekuri/jsonschema/v5" ) -// validateArgs performs a lightweight JSON Schema check on args against the -// tool's Parameters map: every name in "required" must be present, and each -// present argument matching a "properties" entry must satisfy its "type". -// Unknown argument fields are ignored (lenient) so a tool can accept optional -// extras without the dispatcher rejecting them. Full JSON Schema validation is -// deferred to M5 per the milestone plan. +// quotedRe extracts single-quoted tokens, e.g. the property names inside the +// library's required error message "missing properties: 'a', 'b'". +var quotedRe = regexp.MustCompile(`'([^']+)'`) + +// schemaCache reuses compiled schemas keyed by their canonical JSON bytes. +// Tool.Parameters is called on every dispatch, so recompiling each round is +// wasteful; the map is concurrency-safe so concurrent dispatches share entries. +var schemaCache sync.Map // key: string (schema JSON), value: *jsonschema.Schema + +// validateArgs validates args against the tool's JSON Schema. It first applies +// schema "default" values for absent properties (mirroring MCP applySchema and +// OpenClaw's Convert), then coerces model-emitted scalar strings into the types +// the schema asks for (OpenClaw coerceWithJsonSchema), then runs a full +// santhosh-tekuri/jsonschema validation supporting required, properties, nested +// objects, array items, additionalProperties, allOf/anyOf/oneOf, type unions, +// enum, min/max and pattern. Validation failures carry the offending instance +// location (e.g. /count) so the caller and front-end can pinpoint the argument. +// +// The args map is mutated in place: defaults are inserted and values are coerced +// so the tool subsequently receives the arguments it declared. func validateArgs(schema map[string]any, args map[string]any) error { if schema == nil { return nil @@ -21,165 +39,113 @@ func validateArgs(schema map[string]any, args map[string]any) error { if args == nil { args = map[string]any{} } - if raw, ok := schema["required"]; ok { - reqs, ok := raw.([]any) - if !ok { - return fmt.Errorf("schema required must be an array") - } - for _, r := range reqs { - name, ok := r.(string) - if !ok { - return fmt.Errorf("schema required entries must be strings") - } - if _, present := args[name]; !present { - return fmt.Errorf("missing required argument %q", name) - } - } + + compiled, err := compileSchema(schema) + if err != nil { + return fmt.Errorf("schema: %w", err) } - props, _ := schema["properties"].(map[string]any) - for name, val := range args { - ps, ok := props[name] - if !ok { - continue // unknown field: ignored - } - pSchema, ok := ps.(map[string]any) - if !ok { - continue // unconstrained property: not checkable - } - if err := checkType(pSchema, name, val); err != nil { - return err - } + + applyDefaults(schema, args) + coerceWithSchema(args, schema) + + if verr := compiled.Validate(args); verr != nil { + return newValidationError(verr) } return nil } -// checkType validates val against the "type" constraint of a property schema. -func checkType(schema map[string]any, name string, val any) error { - raw, ok := schema["type"] - if !ok || raw == nil { - return nil +// compileSchema compiles schema (or returns a cached compiled schema) using +// draft-7 semantics for schemas that do not declare "$schema", since tool input +// schemas are overwhelmingly OpenAI-function-shaped (tuple "items" arrays +// included). Schemas that declare their own "$schema" are honored as-is. +func compileSchema(schema map[string]any) (*jsonschema.Schema, error) { + raw, err := json.Marshal(schema) + if err != nil { + return nil, fmt.Errorf("schema is not json-marshalable: %w", err) } - switch t := raw.(type) { - case string: - return checkOne(t, name, val) - case []any: - for _, item := range t { - ts, ok := item.(string) - if !ok { - continue - } - if checkOne(ts, name, val) == nil { - return nil - } + key := string(raw) + if cached, ok := schemaCache.Load(key); ok { + return cached.(*jsonschema.Schema), nil + } + + c := jsonschema.NewCompiler() + c.Draft = jsonschema.Draft7 + if err := c.AddResource("tool-schema.json", bytes.NewReader(raw)); err != nil { + return nil, err + } + sch, err := c.Compile("tool-schema.json") + if err != nil { + return nil, err + } + actual, _ := schemaCache.LoadOrStore(key, sch) + return actual.(*jsonschema.Schema), nil +} + +// newValidationError renders a *jsonschema.ValidationError into a compact, +// human-readable report of the leaf failures, each carrying the instance +// location (e.g. /count or /user/age) so callers can locate the exact property. +func newValidationError(err error) error { + var ve *jsonschema.ValidationError + if !errors.As(err, &ve) { + return err + } + var sb strings.Builder + for _, e := range flattenLeaves(ve) { + if sb.Len() > 0 { + sb.WriteString("; ") } - return fmt.Errorf("argument %q has type %s, want one of %v", name, typeName(val), t) - default: - return nil + sb.WriteString(formatErrorPath(e)) + sb.WriteString(": ") + sb.WriteString(e.Message) } -} - -// checkOne validates val against a single JSON Schema type name. -func checkOne(t, name string, val any) error { - if matchesType(t, val) { - return nil + if sb.Len() == 0 { + return err } - return fmt.Errorf("argument %q has type %s, want %s", name, typeName(val), t) + return errors.New(sb.String()) } -// matchesType reports whether v satisfies a single JSON Schema type name. -// Unknown type names are treated as unconstrained (lenient) and the types -// "any"/"" impose no constraint. -func matchesType(t string, v any) bool { - switch t { - case "any", "": - return true - case "string": - _, ok := v.(string) - return ok - case "boolean": - _, ok := v.(bool) - return ok - case "number": - return isNumber(v) - case "integer": - return isInteger(v) - case "array": - return isArrayLike(v) - case "object": - return isMapLike(v) - case "null": - return v == nil - default: - return true +// flattenLeaves walks the ValidationError tree skipping the root wrapper +// ("doesn't validate with ...") and returning every sub-error that carries its +// own message. +func flattenLeaves(ve *jsonschema.ValidationError) []*jsonschema.ValidationError { + var leaves []*jsonschema.ValidationError + var walk func(*jsonschema.ValidationError) + walk = func(e *jsonschema.ValidationError) { + if e.Message != "" && !strings.HasPrefix(e.Message, "doesn't validate with") { + leaves = append(leaves, e) + } + for _, c := range e.Causes { + walk(c) + } } -} - -// isNumber reports whether v is any integer or floating-point numeric type. -func isNumber(v any) bool { - switch v.(type) { - case json.Number, float32, float64, - int, int8, int16, int32, int64, - uint, uint8, uint16, uint32, uint64: - return true - default: - return false + for _, c := range ve.Causes { + walk(c) } + return leaves } -// isInteger reports whether v is an integer type or a floating-point / json -// Number that holds an integral value. -func isInteger(v any) bool { - switch x := v.(type) { - case int, int8, int16, int32, int64, - uint, uint8, uint16, uint32, uint64: - return true - case float32: - return math.Trunc(float64(x)) == float64(x) - case float64: - return math.Trunc(x) == x - case json.Number: - _, err := strconv.ParseInt(x.String(), 10, 64) - return err == nil - default: - return false +// formatErrorPath renders the instance location for a leaf error as a JSON +// pointer (e.g. /count). For a missing-required error the instance location +// points at the enclosing object, so the first missing property name is +// appended to give a precise path. +func formatErrorPath(e *jsonschema.ValidationError) string { + if e.KeywordLocation == "required" || strings.HasSuffix(e.KeywordLocation, "/required") { + if names := quotedTokens(e.Message); len(names) > 0 { + return strings.TrimRight(e.InstanceLocation, "/") + "/" + names[0] + } } -} - -// isArrayLike reports whether v is a slice or array. -func isArrayLike(v any) bool { - rv := reflect.ValueOf(v) - return rv.IsValid() && (rv.Kind() == reflect.Slice || rv.Kind() == reflect.Array) -} - -// isMapLike reports whether v is a map. -func isMapLike(v any) bool { - rv := reflect.ValueOf(v) - return rv.IsValid() && rv.Kind() == reflect.Map -} - -// typeName renders a short, human-readable name for v for error messages. -func typeName(v any) string { - switch v.(type) { - case nil: - return "null" - case bool: - return "boolean" - case string: - return "string" - case []any: - return "array" - case map[string]any: - return "object" - default: - return fmt.Sprintf("%s", kindName(v)) + if e.InstanceLocation == "" { + return "/" } + return e.InstanceLocation } -// kindName maps a reflected kind to a readable label. -func kindName(v any) string { - rv := reflect.ValueOf(v) - if !rv.IsValid() { - return "invalid" +// quotedTokens extracts every single-quoted token from s. +func quotedTokens(s string) []string { + matches := quotedRe.FindAllStringSubmatch(s, -1) + out := make([]string, 0, len(matches)) + for _, m := range matches { + out = append(out, m[1]) } - return rv.Kind().String() + return out }