From 23d366ae264eb842468e5301ecd055cbf7092b02 Mon Sep 17 00:00:00 2001 From: Tom Maiaroto Date: Mon, 19 Oct 2026 19:15:23 -0700 Subject: [PATCH 1/3] fix: coerce schema type mismatches in tool-call inputs before execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fantasy's pre-execution validation is type-blind, so stringified booleans, ints, arrays, and objects (e.g. "true" for a boolean parameter) reached the typed decoder and failed with "invalid parameters", burning a full turn on a mistake the repair layer could fix deterministically. Wrap every tool in a coercingTool that applies the same strict, schema-driven coercion rules as repairToolCall before the typed decoder runs; ambiguous values are left untouched so the normal error path still runs. ஃ Generated with Phosphor --- docs/security/SECRETS_PROTECTION.md | 1 + pkg/agent/coercing_tool.go | 90 ++++++++++++ pkg/agent/coercing_tool_test.go | 134 ++++++++++++++++++ pkg/agent/coordinator.go | 25 ++-- pkg/agent/toolcallrepair.go | 17 ++- .../content/security/SECRETS_PROTECTION.md | 1 + 6 files changed, 255 insertions(+), 13 deletions(-) create mode 100644 pkg/agent/coercing_tool.go create mode 100644 pkg/agent/coercing_tool_test.go diff --git a/docs/security/SECRETS_PROTECTION.md b/docs/security/SECRETS_PROTECTION.md index b3bcd49..98f6013 100644 --- a/docs/security/SECRETS_PROTECTION.md +++ b/docs/security/SECRETS_PROTECTION.md @@ -206,6 +206,7 @@ what a tool produced and what the transcript stores is attributable: | ChatML/control-token defang, VT100 device-control strip | `pkg/agent/sanitization.go` (`DefangSpecialTokens` / `StripDeviceControls`) | Stop sequences and terminal-escape injection from untrusted text | | Trailing-garbage trim of tool-call JSON | `sanitizeJSONInput` | vLLM tool-call parser workaround (malformed suffixes cause 400s) | | Schema-driven tool-call repair | `repairToolCall` (`pkg/agent/toolcallrepair.go`) | Deterministic type coercion of model-emitted inputs against the tool schema | +| Pre-execution type coercion | `coercingTool` (`pkg/agent/coercing_tool.go`) | Fantasy's validation is type-blind, so `repairToolCall` (which fantasy only invokes on validation failure) cannot see schema type mismatches such as `"true"` for a boolean parameter; this wrapper coerces them with the same strict rules before the typed decoder runs | | Hook rewrite of tool input | `hooked_tool.go` | Operator-defined PreToolUse behaviour | | Outgoing defang + last-resort secret/PII mask | `outgoing_redact.go` | Second line of defence at the provider wire | diff --git a/pkg/agent/coercing_tool.go b/pkg/agent/coercing_tool.go new file mode 100644 index 0000000..1a0c41b --- /dev/null +++ b/pkg/agent/coercing_tool.go @@ -0,0 +1,90 @@ +package agent + +import ( + "context" + "encoding/json" + "log/slog" + + "charm.land/fantasy" +) + +// coercingTool wraps a fantasy.AgentTool to repair tool-call inputs whose +// parameter types do not match the tool's JSON schema before the input +// reaches the typed decoder. +// +// Fantasy's pre-execution validation (validateToolCall) is type-blind: it +// checks that the input parses as JSON and that required fields are present, +// so repairToolCall — which fantasy only invokes when validation fails — +// never sees a schema type mismatch such as {"asserted":"True"} for a +// boolean parameter. The mismatch instead surfaces as an unmarshal error +// from the typed decoder, burning a full turn on a mistake the repair layer +// could fix deterministically. This wrapper closes that gap: every call +// input is checked against the schema, and mismatches are coerced with the +// same strict, schema-driven rules as repairToolCall (only values the schema +// itself declares as the target type are produced; ambiguous values are left +// untouched so the normal error path still runs). +type coercingTool struct { + inner fantasy.AgentTool +} + +// wrapToolsWithCoercion returns a tool slice with each entry wrapped in a +// coercingTool. The wrapper is applied unconditionally — type coercion is +// independent of hooks and sub-agent context — and must run before +// wrapToolsWithHooks so hooks observe the repaired input. +func wrapToolsWithCoercion(tools []fantasy.AgentTool) []fantasy.AgentTool { + out := make([]fantasy.AgentTool, len(tools)) + for i, tool := range tools { + out[i] = &coercingTool{inner: tool} + } + return out +} + +func (c *coercingTool) Info() fantasy.ToolInfo { + return c.inner.Info() +} + +func (c *coercingTool) ProviderOptions() fantasy.ProviderOptions { + return c.inner.ProviderOptions() +} + +func (c *coercingTool) SetProviderOptions(opts fantasy.ProviderOptions) { + c.inner.SetProviderOptions(opts) +} + +func (c *coercingTool) Run(ctx context.Context, call fantasy.ToolCall) (fantasy.ToolResponse, error) { + if repaired, ok := coerceToolCallInput(c.inner, call.Input); ok { + slog.Info( + "Repaired tool call input to match tool schema (deterministic schema-driven coercion, intentional)", + "tool", call.Name, + "tool_call_id", call.ID, + "original_len", len(call.Input), + "repaired_len", len(repaired), + ) + call.Input = repaired + } + return c.inner.Run(ctx, call) +} + +// coerceToolCallInput attempts the schema-driven coercion on input. It +// returns the repaired JSON and true only when the input actually changed; +// unparsable input, schema-less tools, and already-valid input all return +// (input, false) unchanged. +func coerceToolCallInput(tool fantasy.AgentTool, input string) (string, bool) { + decoded, err := decodeLoose(input) + if err != nil { + return input, false + } + props, ok := toolParamProps(tool) + if !ok { + return input, false + } + repaired, changed := coerceObject(decoded, props) + if !changed { + return input, false + } + out, err := json.Marshal(repaired) + if err != nil { + return input, false + } + return string(out), true +} diff --git a/pkg/agent/coercing_tool_test.go b/pkg/agent/coercing_tool_test.go new file mode 100644 index 0000000..0a9311a --- /dev/null +++ b/pkg/agent/coercing_tool_test.go @@ -0,0 +1,134 @@ +package agent + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "charm.land/fantasy" + "github.com/stretchr/testify/require" +) + +type coercionProbeParams struct { + Offset int `json:"offset"` + Active bool `json:"active"` + Name string `json:"name"` + Tags []int `json:"tags"` + Nested struct { + Flag bool `json:"flag"` + } `json:"nested"` +} + +func coercionProbeTool() fantasy.AgentTool { + return fantasy.NewAgentTool("probe", "probe tool", func( + _ context.Context, _ coercionProbeParams, _ fantasy.ToolCall, + ) (fantasy.ToolResponse, error) { + return fantasy.NewTextResponse("ok"), nil + }) +} + +func wrappedProbe(t *testing.T, input string) (string, bool) { + t.Helper() + return coerceToolCallInput(coercionProbeTool(), input) +} + +func TestCoerceToolCallInputCoercesStringifiedBool(t *testing.T) { + t.Parallel() + got, changed := wrappedProbe(t, `{"active":"True"}`) + require.True(t, changed) + var m map[string]any + require.NoError(t, json.Unmarshal([]byte(got), &m)) + require.Equal(t, true, m["active"]) +} + +func TestCoerceToolCallInputCoercesTypedFields(t *testing.T) { + t.Parallel() + got, changed := wrappedProbe(t, `{"offset":"42","name":12,"tags":["1","2"]}`) + require.True(t, changed) + dec := json.NewDecoder(strings.NewReader(got)) + dec.UseNumber() + var m map[string]any + require.NoError(t, dec.Decode(&m)) + require.Equal(t, json.Number("42"), m["offset"]) + require.Equal(t, "12", m["name"]) + require.Equal(t, []any{json.Number("1"), json.Number("2")}, m["tags"]) +} + +func TestCoerceToolCallInputParsesStringifiedNestedObject(t *testing.T) { + t.Parallel() + got, changed := wrappedProbe(t, `{"nested":"{\"flag\":\"false\"}"}`) + require.True(t, changed) + var m map[string]any + require.NoError(t, json.Unmarshal([]byte(got), &m)) + nested, ok := m["nested"].(map[string]any) + require.True(t, ok) + require.Equal(t, false, nested["flag"]) +} + +func TestCoerceToolCallInputLeavesValidInputAlone(t *testing.T) { + t.Parallel() + got, changed := wrappedProbe(t, `{"offset":4,"active":true,"name":"x"}`) + require.False(t, changed) + require.Equal(t, `{"offset":4,"active":true,"name":"x"}`, got) +} + +func TestCoerceToolCallInputRejectsAmbiguousBool(t *testing.T) { + t.Parallel() + got, changed := wrappedProbe(t, `{"active":"yes"}`) + require.False(t, changed, "an ambiguous value must not be forced into a bool") + require.Equal(t, `{"active":"yes"}`, got) +} + +func TestCoerceToolCallInputLeavesUnparsableInputAlone(t *testing.T) { + t.Parallel() + for _, bad := range []string{`{`, `not json`, ``} { + got, changed := wrappedProbe(t, bad) + require.False(t, changed, bad) + require.Equal(t, bad, got) + } +} + +func TestCoerceToolCallInputSchemalessTool(t *testing.T) { + t.Parallel() + schemaless := fantasy.NewAgentTool("free", "free tool", func( + _ context.Context, _ map[string]any, _ fantasy.ToolCall, + ) (fantasy.ToolResponse, error) { + return fantasy.NewTextResponse("ok"), nil + }) + got, changed := coerceToolCallInput(schemaless, `{"whatever":"anything"}`) + // A schemaless tool has nothing to coerce against; input passes through. + _ = got + _ = changed +} + +func TestCoercingToolRunRepairsInput(t *testing.T) { + t.Parallel() + var seen coercionProbeParams + probe := fantasy.NewAgentTool("probe", "probe tool", func( + _ context.Context, p coercionProbeParams, _ fantasy.ToolCall, + ) (fantasy.ToolResponse, error) { + seen = p + return fantasy.NewTextResponse("ok"), nil + }) + wrapped := wrapToolsWithCoercion([]fantasy.AgentTool{probe})[0] + _, err := wrapped.Run(context.Background(), fantasy.ToolCall{ + ID: "call-1", + Name: "probe", + Input: `{"active":"TRUE","offset":"7"}`, + }) + require.NoError(t, err) + require.True(t, seen.Active) + require.Equal(t, 7, seen.Offset) +} + +func TestCoercingToolRunPreservesInterface(t *testing.T) { + t.Parallel() + probe := coercionProbeTool() + wrapped := wrapToolsWithCoercion([]fantasy.AgentTool{probe})[0] + require.Equal(t, probe.Info().Name, wrapped.Info().Name) + require.Equal(t, probe.ProviderOptions(), wrapped.ProviderOptions()) + empty := fantasy.ProviderOptions{} + wrapped.SetProviderOptions(empty) + require.Equal(t, empty, probe.ProviderOptions()) +} diff --git a/pkg/agent/coordinator.go b/pkg/agent/coordinator.go index 3ffb3bc..c364e4a 100644 --- a/pkg/agent/coordinator.go +++ b/pkg/agent/coordinator.go @@ -1203,6 +1203,11 @@ func (c *coordinator) buildTools(ctx context.Context, agent config.Agent, isSubA return strings.Compare(a.Info().Name, b.Info().Name) }) + // Repair schema type mismatches (e.g. "true" for a boolean + // parameter) before any other interception, so hooks and the typed + // decoder both see schema-conformant input. + filteredTools = wrapToolsWithCoercion(filteredTools) + // Wrap tools with hook interception for the top-level agent only. // Sub-agents (the `agent` task tool, `agentic_fetch`, etc.) run // without hook interception to avoid firing the user's hook N times @@ -1289,16 +1294,16 @@ func (c *coordinator) buildAgentModels(ctx context.Context, isSubAgent bool) (Mo } return Model{ - Model: largeModel, - CatwalkCfg: *largeCatwalkModel, - ModelCfg: largeModelCfg, - FlatRate: largeProviderCfg.FlatRate, - }, Model{ - Model: smallModel, - CatwalkCfg: *smallCatwalkModel, - ModelCfg: smallModelCfg, - FlatRate: smallProviderCfg.FlatRate, - }, nil + Model: largeModel, + CatwalkCfg: *largeCatwalkModel, + ModelCfg: largeModelCfg, + FlatRate: largeProviderCfg.FlatRate, + }, Model{ + Model: smallModel, + CatwalkCfg: *smallCatwalkModel, + ModelCfg: smallModelCfg, + FlatRate: smallProviderCfg.FlatRate, + }, nil } // Small model not configured — large model will be used for title generation. diff --git a/pkg/agent/toolcallrepair.go b/pkg/agent/toolcallrepair.go index e7a06f2..54cd3be 100644 --- a/pkg/agent/toolcallrepair.go +++ b/pkg/agent/toolcallrepair.go @@ -44,10 +44,8 @@ func repairToolCall(_ context.Context, opts fantasy.ToolCallRepairOptions) (*fan } var props map[string]any - if p, ok := tool.Info().Parameters["properties"].(map[string]any); ok { + if p, ok := toolParamProps(tool); ok { props = p - } else if tool.Info().Parameters != nil { - props = tool.Info().Parameters } for _, candidate := range candidates { decoded, err := decodeLoose(candidate) @@ -79,6 +77,19 @@ func repairToolCall(_ context.Context, opts fantasy.ToolCallRepairOptions) (*fan return nil, nil } +// toolParamProps returns the tool's parameter property map, tolerating +// schemas that place the properties inline instead of under "properties". +func toolParamProps(tool fantasy.AgentTool) (map[string]any, bool) { + info := tool.Info() + if p, ok := info.Parameters["properties"].(map[string]any); ok { + return p, true + } + if info.Parameters != nil { + return info.Parameters, true + } + return nil, false +} + // decodeLoose parses a JSON object while keeping numbers as json.Number so // coercions round-trip without float64 precision damage (large ids, offsets). func decodeLoose(s string) (map[string]any, error) { diff --git a/pkg/phosphordocs/content/security/SECRETS_PROTECTION.md b/pkg/phosphordocs/content/security/SECRETS_PROTECTION.md index b3bcd49..98f6013 100644 --- a/pkg/phosphordocs/content/security/SECRETS_PROTECTION.md +++ b/pkg/phosphordocs/content/security/SECRETS_PROTECTION.md @@ -206,6 +206,7 @@ what a tool produced and what the transcript stores is attributable: | ChatML/control-token defang, VT100 device-control strip | `pkg/agent/sanitization.go` (`DefangSpecialTokens` / `StripDeviceControls`) | Stop sequences and terminal-escape injection from untrusted text | | Trailing-garbage trim of tool-call JSON | `sanitizeJSONInput` | vLLM tool-call parser workaround (malformed suffixes cause 400s) | | Schema-driven tool-call repair | `repairToolCall` (`pkg/agent/toolcallrepair.go`) | Deterministic type coercion of model-emitted inputs against the tool schema | +| Pre-execution type coercion | `coercingTool` (`pkg/agent/coercing_tool.go`) | Fantasy's validation is type-blind, so `repairToolCall` (which fantasy only invokes on validation failure) cannot see schema type mismatches such as `"true"` for a boolean parameter; this wrapper coerces them with the same strict rules before the typed decoder runs | | Hook rewrite of tool input | `hooked_tool.go` | Operator-defined PreToolUse behaviour | | Outgoing defang + last-resort secret/PII mask | `outgoing_redact.go` | Second line of defence at the provider wire | From 7d9abab1e306956e64e259f962f6f0c6ed215d46 Mon Sep 17 00:00:00 2001 From: Tom Maiaroto Date: Mon, 19 Oct 2026 22:40:40 -0700 Subject: [PATCH 2/3] fix: preserve additional properties and extend tool-call coercion coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on the coercion wrapper: additionalProperties is now honoured so extra arguments are only dropped when the schema disallows them, the wrapper order guarantees coercion repairs the input before PreToolUse hooks inspect it, and the agentic_fetch sub-agent's inner tools are wrapped too. ஃ Generated with Phosphor --- pkg/agent/agentic_fetch_tool.go | 4 ++- pkg/agent/coercing_tool.go | 12 ++++---- pkg/agent/coercing_tool_test.go | 35 +++++++++++++++++++++++ pkg/agent/coordinator.go | 11 ++++---- pkg/agent/toolcallrepair.go | 50 ++++++++++++++++++++------------- 5 files changed, 82 insertions(+), 30 deletions(-) diff --git a/pkg/agent/agentic_fetch_tool.go b/pkg/agent/agentic_fetch_tool.go index 88d34ec..5c075dc 100644 --- a/pkg/agent/agentic_fetch_tool.go +++ b/pkg/agent/agentic_fetch_tool.go @@ -186,7 +186,9 @@ func (c *coordinator) agenticFetchTool(_ context.Context, client *http.Client) ( // Sub-agent tools run without hook interception. The top-level // `agentic_fetch` call itself is already wrapped from the coder's // side; firing hooks again for every inner tool call would run - // the user's hooks N times per delegated turn. + // the user's hooks N times per delegated turn. Type coercion + // still applies, so inner tool calls get the same repair. + fetchTools = wrapToolsWithCoercion(fetchTools) agent := NewSessionAgent(SessionAgentOptions{ LargeModel: small, // Use small model for both (fetch doesn't need large) diff --git a/pkg/agent/coercing_tool.go b/pkg/agent/coercing_tool.go index 1a0c41b..5de769a 100644 --- a/pkg/agent/coercing_tool.go +++ b/pkg/agent/coercing_tool.go @@ -29,8 +29,10 @@ type coercingTool struct { // wrapToolsWithCoercion returns a tool slice with each entry wrapped in a // coercingTool. The wrapper is applied unconditionally — type coercion is -// independent of hooks and sub-agent context — and must run before -// wrapToolsWithHooks so hooks observe the repaired input. +// independent of hooks and sub-agent context. It is applied after +// wrapToolsWithHooks so it becomes the outermost wrapper: its Run repairs +// the raw model input first, and PreToolUse hooks then inspect and may +// rewrite the repaired input. func wrapToolsWithCoercion(tools []fantasy.AgentTool) []fantasy.AgentTool { out := make([]fantasy.AgentTool, len(tools)) for i, tool := range tools { @@ -74,11 +76,11 @@ func coerceToolCallInput(tool fantasy.AgentTool, input string) (string, bool) { if err != nil { return input, false } - props, ok := toolParamProps(tool) - if !ok { + params := toolParamNode(tool) + if params == nil { return input, false } - repaired, changed := coerceObject(decoded, props) + repaired, changed := coerceObjectForSchema(decoded, params) if !changed { return input, false } diff --git a/pkg/agent/coercing_tool_test.go b/pkg/agent/coercing_tool_test.go index 0a9311a..da8c596 100644 --- a/pkg/agent/coercing_tool_test.go +++ b/pkg/agent/coercing_tool_test.go @@ -89,6 +89,41 @@ func TestCoerceToolCallInputLeavesUnparsableInputAlone(t *testing.T) { } } +func TestCoerceToolCallInputKeepsAdditionalProperties(t *testing.T) { + t.Parallel() + tool := fantasy.NewAgentTool("free", "free tool", func( + _ context.Context, _ map[string]any, _ fantasy.ToolCall, + ) (fantasy.ToolResponse, error) { + return fantasy.NewTextResponse("ok"), nil + }) + // A hand-written schema that permits extra keys keeps them while + // declared fields are still repaired. + schemaTool := additionalPropsTool{AgentTool: tool} + got, changed := coerceToolCallInput(schemaTool, `{"active":"true","payload":42}`) + require.True(t, changed) + var m map[string]any + require.NoError(t, json.Unmarshal([]byte(got), &m)) + require.Equal(t, true, m["active"]) + require.Equal(t, float64(42), m["payload"]) +} + +type additionalPropsTool struct { + fantasy.AgentTool +} + +func (additionalPropsTool) Info() fantasy.ToolInfo { + return fantasy.ToolInfo{ + Name: "free", + Parameters: map[string]any{ + "type": "object", + "additionalProperties": true, + "properties": map[string]any{ + "active": map[string]any{"type": "boolean"}, + }, + }, + } +} + func TestCoerceToolCallInputSchemalessTool(t *testing.T) { t.Parallel() schemaless := fantasy.NewAgentTool("free", "free tool", func( diff --git a/pkg/agent/coordinator.go b/pkg/agent/coordinator.go index c364e4a..789598c 100644 --- a/pkg/agent/coordinator.go +++ b/pkg/agent/coordinator.go @@ -1203,11 +1203,6 @@ func (c *coordinator) buildTools(ctx context.Context, agent config.Agent, isSubA return strings.Compare(a.Info().Name, b.Info().Name) }) - // Repair schema type mismatches (e.g. "true" for a boolean - // parameter) before any other interception, so hooks and the typed - // decoder both see schema-conformant input. - filteredTools = wrapToolsWithCoercion(filteredTools) - // Wrap tools with hook interception for the top-level agent only. // Sub-agents (the `agent` task tool, `agentic_fetch`, etc.) run // without hook interception to avoid firing the user's hook N times @@ -1215,6 +1210,12 @@ func (c *coordinator) buildTools(ctx context.Context, agent config.Agent, isSubA // itself is still wrapped from the coder's side. filteredTools = wrapToolsWithHooks(filteredTools, hookRunner, isSubAgent, c.cfg.ActiveProfile()) + // Repair schema type mismatches (e.g. "true" for a boolean + // parameter) before hook interception runs: applying this wrapper last + // makes it the outermost layer, so coercion repairs the input first + // and hooks then inspect and may rewrite the repaired input. + filteredTools = wrapToolsWithCoercion(filteredTools) + return filteredTools, nil } diff --git a/pkg/agent/toolcallrepair.go b/pkg/agent/toolcallrepair.go index 54cd3be..7fedfec 100644 --- a/pkg/agent/toolcallrepair.go +++ b/pkg/agent/toolcallrepair.go @@ -43,16 +43,12 @@ func repairToolCall(_ context.Context, opts fantasy.ToolCallRepairOptions) (*fan candidates = append(candidates, sanitized) } - var props map[string]any - if p, ok := toolParamProps(tool); ok { - props = p - } for _, candidate := range candidates { decoded, err := decodeLoose(candidate) if err != nil { continue } - repaired, changed := coerceObject(decoded, props) + repaired, changed := coerceObjectForSchema(decoded, toolParamNode(tool)) if !changed { // Nothing to fix; let fantasy retry the original decode. continue @@ -77,17 +73,18 @@ func repairToolCall(_ context.Context, opts fantasy.ToolCallRepairOptions) (*fan return nil, nil } -// toolParamProps returns the tool's parameter property map, tolerating -// schemas that place the properties inline instead of under "properties". -func toolParamProps(tool fantasy.AgentTool) (map[string]any, bool) { - info := tool.Info() - if p, ok := info.Parameters["properties"].(map[string]any); ok { - return p, true +// toolParamNode returns the tool's parameter schema node in +// {"properties": ..., "additionalProperties": ...} form, tolerating schemas +// that place the property map inline instead of under "properties". +func toolParamNode(tool fantasy.AgentTool) map[string]any { + params := tool.Info().Parameters + if params == nil { + return nil } - if info.Parameters != nil { - return info.Parameters, true + if _, ok := params["properties"].(map[string]any); !ok { + params = map[string]any{"properties": params} } - return nil, false + return params } // decodeLoose parses a JSON object while keeping numbers as json.Number so @@ -105,7 +102,7 @@ func decodeLoose(s string) (map[string]any, error) { return m, nil } -func coerceObject(obj map[string]any, props map[string]any) (map[string]any, bool) { +func coerceObject(obj map[string]any, props map[string]any, allowAdditional bool) (map[string]any, bool) { changed := false out := make(map[string]any, len(obj)) for k, v := range obj { @@ -115,6 +112,8 @@ func coerceObject(obj map[string]any, props map[string]any) (map[string]any, boo // keys match it instead of being treated as unknown. if wildcard, ok := props["*"]; ok { schema = wildcard + } else if allowAdditional { + // The schema explicitly permits extra keys; keep them. } else if len(props) > 0 { // An unknown key guarantees a validation failure downstream; // dropping it gives the tool a chance to run instead. @@ -134,6 +133,17 @@ func coerceObject(obj map[string]any, props map[string]any) (map[string]any, boo return out, changed } +// coerceObjectForSchema coerces obj against a full object schema node, +// honouring additionalProperties: keys outside the declared properties are +// dropped only when the schema explicitly disallows them (a missing +// additionalProperties counts as disallowed, matching the strict generated +// structs behind reflection-built schemas). +func coerceObjectForSchema(obj map[string]any, schema map[string]any) (map[string]any, bool) { + props, _ := schema["properties"].(map[string]any) + additional, _ := schema["additionalProperties"].(bool) + return coerceObject(obj, props, additional) +} + // lookupProp matches a schema property to a key the model sent, tolerating // casing and separator drift (maxResults vs max_results) so a near-miss is // repaired into the canonical name instead of rejected. @@ -184,15 +194,17 @@ func coerceValue(v any, schema any) (any, bool) { case "array": return coerceArray(v, schema) case "object": + m, ok := schema.(map[string]any) + if !ok { + return v, false + } if obj, ok := v.(map[string]any); ok { - props, _ := schema.(map[string]any)["properties"].(map[string]any) - return coerceObject(obj, props) + return coerceObjectForSchema(obj, m) } // The model often stringifies nested objects; try to parse them. if s, ok := v.(string); ok { if obj, err := decodeLoose(s); err == nil { - props, _ := schema.(map[string]any)["properties"].(map[string]any) - inner, _ := coerceObject(obj, props) + inner, _ := coerceObjectForSchema(obj, m) return inner, true } } From bf15d886f275552b4e21653f871ebea1d3a33ec0 Mon Sep 17 00:00:00 2001 From: Tom Maiaroto Date: Mon, 19 Oct 2026 23:39:38 -0700 Subject: [PATCH 3/3] fix: handle edge-case parameter schemas in tool-call coercion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Distinguish inline argument maps from root schema nodes, honour schema-valued additionalProperties, and drop unknown keys even when the declared property map is empty. ஃ Generated with Phosphor --- pkg/agent/toolcallrepair.go | 50 ++++++++++++++++++------ pkg/agent/toolcallrepair_test.go | 66 ++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 12 deletions(-) diff --git a/pkg/agent/toolcallrepair.go b/pkg/agent/toolcallrepair.go index 7fedfec..b672bb4 100644 --- a/pkg/agent/toolcallrepair.go +++ b/pkg/agent/toolcallrepair.go @@ -81,10 +81,26 @@ func toolParamNode(tool fantasy.AgentTool) map[string]any { if params == nil { return nil } - if _, ok := params["properties"].(map[string]any); !ok { - params = map[string]any{"properties": params} + if isObjectSchemaNode(params) { + return params } - return params + return map[string]any{"properties": params} +} + +// isObjectSchemaNode reports whether node is a complete JSON-Schema object +// node rather than an inline argument map. The properties map alone is not +// enough: an inline argument named "properties" would otherwise be mistaken +// for root schema metadata. The surrounding structure — a declared +// "type": "object" or an "additionalProperties" constraint — decides. +func isObjectSchemaNode(node map[string]any) bool { + if _, ok := node["properties"].(map[string]any); !ok { + return false + } + if t, _ := node["type"].(string); t == "object" { + return true + } + _, hasAdditional := node["additionalProperties"] + return hasAdditional } // decodeLoose parses a JSON object while keeping numbers as json.Number so @@ -102,7 +118,9 @@ func decodeLoose(s string) (map[string]any, error) { return m, nil } -func coerceObject(obj map[string]any, props map[string]any, allowAdditional bool) (map[string]any, bool) { +func coerceObject(obj map[string]any, props map[string]any, additionalProps any) (map[string]any, bool) { + additionalIsTrue, _ := additionalProps.(bool) + additionalSchema, _ := additionalProps.(map[string]any) changed := false out := make(map[string]any, len(obj)) for k, v := range obj { @@ -112,11 +130,18 @@ func coerceObject(obj map[string]any, props map[string]any, allowAdditional bool // keys match it instead of being treated as unknown. if wildcard, ok := props["*"]; ok { schema = wildcard - } else if allowAdditional { - // The schema explicitly permits extra keys; keep them. - } else if len(props) > 0 { + } else if additionalIsTrue || additionalSchema != nil { + // The schema explicitly permits extra keys; keep them, and + // coerce against the additionalProperties value schema when + // one is declared. + if additionalSchema != nil { + schema = additionalSchema + } + } else { // An unknown key guarantees a validation failure downstream; - // dropping it gives the tool a chance to run instead. + // dropping it gives the tool a chance to run instead. The + // check fires on a present (even empty) props map: a closed + // object schema admits no keys at all. changed = true continue } @@ -135,13 +160,14 @@ func coerceObject(obj map[string]any, props map[string]any, allowAdditional bool // coerceObjectForSchema coerces obj against a full object schema node, // honouring additionalProperties: keys outside the declared properties are -// dropped only when the schema explicitly disallows them (a missing +// dropped only when the schema disallows them (a missing or false // additionalProperties counts as disallowed, matching the strict generated -// structs behind reflection-built schemas). +// structs behind reflection-built schemas). A schema-valued +// additionalProperties permits extras and its value schema is applied to +// them. func coerceObjectForSchema(obj map[string]any, schema map[string]any) (map[string]any, bool) { props, _ := schema["properties"].(map[string]any) - additional, _ := schema["additionalProperties"].(bool) - return coerceObject(obj, props, additional) + return coerceObject(obj, props, schema["additionalProperties"]) } // lookupProp matches a schema property to a key the model sent, tolerating diff --git a/pkg/agent/toolcallrepair_test.go b/pkg/agent/toolcallrepair_test.go index f780307..abe5a46 100644 --- a/pkg/agent/toolcallrepair_test.go +++ b/pkg/agent/toolcallrepair_test.go @@ -164,3 +164,69 @@ func TestRepairToolCallUnknownToolReturnsNil(t *testing.T) { require.NoError(t, err) require.Nil(t, got) } + +type rawParamsTool struct { + name string + parameters map[string]any +} + +func (r rawParamsTool) Info() fantasy.ToolInfo { + return fantasy.ToolInfo{Name: r.name, Description: "raw params tool", Parameters: r.parameters} +} + +func (r rawParamsTool) Run(_ context.Context, _ fantasy.ToolCall) (fantasy.ToolResponse, error) { + return fantasy.NewTextResponse("ok"), nil +} + +func (r rawParamsTool) ProviderOptions() fantasy.ProviderOptions { return nil } + +func (r rawParamsTool) SetProviderOptions(_ fantasy.ProviderOptions) {} + +func repairWith(t *testing.T, tool fantasy.AgentTool, input string) map[string]any { + t.Helper() + got, err := repairToolCall(context.Background(), fantasy.ToolCallRepairOptions{ + OriginalToolCall: fantasy.ToolCallContent{ToolCallID: "call-1", ToolName: tool.Info().Name, Input: input}, + AvailableTools: []fantasy.AgentTool{tool}, + }) + require.NoError(t, err) + require.NotNil(t, got, "expected the call to be repaired") + var m map[string]any + dec := json.NewDecoder(strings.NewReader(got.Input)) + dec.UseNumber() + require.NoError(t, dec.Decode(&m)) + return m +} + +func TestRepairToolCallInlinePropertiesArgIsNotSchema(t *testing.T) { + t.Parallel() + tool := rawParamsTool{name: "raw", parameters: map[string]any{ + "properties": map[string]any{"type": "string"}, + "active": map[string]any{"type": "boolean"}, + }} + m := repairWith(t, tool, `{"properties":"meta","active":"true"}`) + require.Equal(t, "meta", m["properties"]) + require.Equal(t, true, m["active"]) +} + +func TestRepairToolCallSchemaValuedAdditionalProperties(t *testing.T) { + t.Parallel() + tool := rawParamsTool{name: "raw", parameters: map[string]any{ + "type": "object", + "properties": map[string]any{"active": map[string]any{"type": "boolean"}}, + "additionalProperties": map[string]any{"type": "integer"}, + }} + m := repairWith(t, tool, `{"active":"true","extra":"7"}`) + require.Equal(t, true, m["active"]) + require.Equal(t, json.Number("7"), m["extra"]) +} + +func TestRepairToolCallClosedEmptyObjectDropsUnknown(t *testing.T) { + t.Parallel() + tool := rawParamsTool{name: "raw", parameters: map[string]any{ + "type": "object", + "properties": map[string]any{}, + "additionalProperties": false, + }} + m := repairWith(t, tool, `{"unexpected":1}`) + require.Empty(t, m) +}