Skip to content

Commit b64dcd2

Browse files
committed
feat: deterministic param ordering and type validation for execute_code
Replace extractParamNames with extractParamSchema to fix non-deterministic positional argument mapping (Go map iteration order). Parameters are now ordered: required first (JSON array order), then non-required sorted lexicographically. Add type validation against JSON Schema before calling downstream tools. Monty values are checked against the tool's declared parameter types and a TypeError is raised on mismatch. Also use the Monty-passed context (fnCtx) instead of the captured outer ctx for downstream CallTool calls.
1 parent 595616b commit b64dcd2

2 files changed

Lines changed: 392 additions & 32 deletions

File tree

internal/tools/execute_code.go

Lines changed: 121 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7+
"sort"
78

89
"skillful-mcp/internal/clientmanager"
910

@@ -74,33 +75,46 @@ func buildToolFunctions(ctx context.Context, mgr *clientmanager.Manager) (map[st
7475

7576
fns := make(map[string]monty.ExternalFunction, len(resolved))
7677
for _, rt := range resolved {
77-
rt := rt
7878
session, err := mgr.GetSession(rt.SkillName)
7979
if err != nil {
8080
continue
8181
}
82-
paramNames := extractParamNames(rt.Tool.InputSchema)
82+
params := extractParamSchema(rt.Tool.InputSchema)
83+
paramByName := make(map[string]paramInfo, len(params))
84+
for _, p := range params {
85+
paramByName[p.Name] = p
86+
}
8387

84-
fns[rt.ResolvedName] = func(_ context.Context, call monty.Call) (monty.Result, error) {
88+
fns[rt.ResolvedName] = func(fnCtx context.Context, call monty.Call) (monty.Result, error) {
8589
args := make(map[string]any)
8690

87-
// Map positional args to parameter names from the schema.
91+
// Map positional args to parameter names from the schema, with type validation.
8892
for i, val := range call.Args {
89-
if i < len(paramNames) {
90-
args[paramNames[i]] = montyValueToAny(val)
93+
if i < len(params) {
94+
if err := validateMontyValue(val, params[i]); err != nil {
95+
msg := err.Error()
96+
return monty.Raise(monty.Exception{Type: "TypeError", Arg: &msg}), nil
97+
}
98+
args[params[i].Name] = montyValueToAny(val)
9199
}
92100
}
93101

94-
// Keyword args override positional.
102+
// Keyword args override positional, with type validation.
95103
for _, pair := range call.Kwargs {
96104
key, ok := pair.Key.Raw().(string)
97105
if !ok {
98106
continue
99107
}
108+
if pi, ok := paramByName[key]; ok {
109+
if err := validateMontyValue(pair.Value, pi); err != nil {
110+
msg := err.Error()
111+
return monty.Raise(monty.Exception{Type: "TypeError", Arg: &msg}), nil
112+
}
113+
}
100114
args[key] = montyValueToAny(pair.Value)
101115
}
102116

103-
toolResult, err := session.CallTool(ctx, &mcp.CallToolParams{
117+
toolResult, err := session.CallTool(fnCtx, &mcp.CallToolParams{
104118
Name: rt.OriginalName,
105119
Arguments: args,
106120
})
@@ -120,9 +134,16 @@ func buildToolFunctions(ctx context.Context, mgr *clientmanager.Manager) (map[st
120134
return fns, nil
121135
}
122136

123-
// extractParamNames extracts ordered property names from a JSON Schema input schema.
124-
// The InputSchema from the client is typically a map[string]any.
125-
func extractParamNames(schema any) []string {
137+
// paramInfo describes a single parameter extracted from a tool's JSON Schema.
138+
type paramInfo struct {
139+
Name string // property name from JSON Schema
140+
Types []string // allowed JSON Schema types, e.g. ["string"] or ["string", "null"]
141+
}
142+
143+
// extractParamSchema extracts ordered parameter definitions from a JSON Schema.
144+
// Ordering: required params first (in JSON array order), then non-required sorted
145+
// lexicographically. This ensures deterministic positional argument mapping.
146+
func extractParamSchema(schema any) []paramInfo {
126147
m, ok := schema.(map[string]any)
127148
if !ok {
128149
return nil
@@ -131,35 +152,103 @@ func extractParamNames(schema any) []string {
131152
if !ok {
132153
return nil
133154
}
134-
// Check for explicit ordering via "required" array (common convention).
155+
156+
requiredSet := make(map[string]bool)
157+
var params []paramInfo
158+
159+
// Required params come first, in the order declared by the JSON array.
135160
if required, ok := m["required"].([]any); ok {
136-
names := make([]string, 0, len(required))
137161
for _, r := range required {
138-
if s, ok := r.(string); ok {
139-
names = append(names, s)
162+
if name, ok := r.(string); ok {
163+
requiredSet[name] = true
164+
params = append(params, paramInfo{
165+
Name: name,
166+
Types: extractPropertyTypes(props[name]),
167+
})
140168
}
141169
}
142-
// Append non-required properties.
143-
for name := range props {
144-
found := false
145-
for _, n := range names {
146-
if n == name {
147-
found = true
148-
break
149-
}
150-
}
151-
if !found {
152-
names = append(names, name)
170+
}
171+
172+
// Non-required params sorted lexicographically for deterministic ordering.
173+
var optional []string
174+
for name := range props {
175+
if !requiredSet[name] {
176+
optional = append(optional, name)
177+
}
178+
}
179+
sort.Strings(optional)
180+
181+
for _, name := range optional {
182+
params = append(params, paramInfo{
183+
Name: name,
184+
Types: extractPropertyTypes(props[name]),
185+
})
186+
}
187+
188+
return params
189+
}
190+
191+
// extractPropertyTypes extracts the "type" field from a JSON Schema property.
192+
// Handles both string ("type": "string") and array ("type": ["string", "null"]) forms.
193+
func extractPropertyTypes(propSchema any) []string {
194+
pm, ok := propSchema.(map[string]any)
195+
if !ok {
196+
return nil
197+
}
198+
switch t := pm["type"].(type) {
199+
case string:
200+
return []string{t}
201+
case []any:
202+
types := make([]string, 0, len(t))
203+
for _, item := range t {
204+
if s, ok := item.(string); ok {
205+
types = append(types, s)
153206
}
154207
}
155-
return names
208+
if len(types) == 0 {
209+
return nil
210+
}
211+
return types
212+
default:
213+
return nil
156214
}
157-
// No required field — just return property names.
158-
names := make([]string, 0, len(props))
159-
for name := range props {
160-
names = append(names, name)
215+
}
216+
217+
// validateMontyValue checks that a Monty value matches the expected JSON Schema
218+
// types for a parameter. Returns nil if validation passes or types are unknown.
219+
func validateMontyValue(v monty.Value, param paramInfo) error {
220+
if len(param.Types) == 0 {
221+
return nil
222+
}
223+
kind := v.Kind()
224+
for _, t := range param.Types {
225+
if jsonSchemaTypeMatchesMonty(t, kind) {
226+
return nil
227+
}
228+
}
229+
return fmt.Errorf("parameter %q: expected %s, got %s", param.Name, param.Types[0], kind)
230+
}
231+
232+
// jsonSchemaTypeMatchesMonty checks if a JSON Schema type is compatible with a Monty ValueKind.
233+
func jsonSchemaTypeMatchesMonty(schemaType string, kind monty.ValueKind) bool {
234+
switch schemaType {
235+
case "string":
236+
return kind == "string"
237+
case "integer":
238+
return kind == "int" || kind == "big_int"
239+
case "number":
240+
return kind == "int" || kind == "big_int" || kind == "float"
241+
case "boolean":
242+
return kind == "bool"
243+
case "array":
244+
return kind == "list" || kind == "tuple"
245+
case "object":
246+
return kind == "dict"
247+
case "null":
248+
return kind == "none"
249+
default:
250+
return false
161251
}
162-
return names
163252
}
164253

165254
// montyValueToAny converts a Monty Value to a Go value suitable for JSON tool arguments.

0 commit comments

Comments
 (0)