fix: tolerantly decode stringified JSON object arguments in MCP tool calls

[nishantmonu51]

Some MCP clients JSON-encode object/array-typed sub-fields of tool arguments as strings (tracked upstream in anthropics/claude-code#25865). Rill's MCP server rejects these during input schema validation, which fully broke query_metrics_view from affected clients for any query with time_range, comparison_time_range, where, or having.

• Adds jsonschemautil.CoerceStringifiedJSON, which walks tool arguments alongside the input schema and JSON-decodes string values only where the schema unambiguously expects an object or array (handles $ref/$defs resolution, including nested $defs scopes, type unions, anyOf/oneOf, and additionalProperties).
• Applies it in an MCP receiving middleware in Session.MCPServer, before the SDK validates arguments. This covers all tools generically, fails open on any decode error, and re-marshals only when something changed.
• Deliberately conservative: untyped fields (e.g. Expression.val), string-typed fields containing JSON text (e.g. write_file contents), and unions that allow strings are never touched, and genuinely malformed input still fails validation with the normal error.
• Adds unit tests for the coercion matrix and end-to-end tests through a real MCP client that reproduce the client bug verbatim.

Checklist:

• Covered by tests
• Ran it and it works as intended
• Reviewed the diff before requesting a review
• Checked for unhandled edge cases
• Linked the issues it closes
• Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
• Intend to cherry-pick into the release branch
• I'm proud of this work!

[Copilot]

Pull request overview

This PR improves interoperability of Rill’s MCP server with clients that incorrectly JSON-stringify nested tool-argument fields that are schema-typed as objects/arrays, by adding a tolerant coercion pass prior to SDK schema validation.

Changes:

• Added jsonschemautil.CoerceStringifiedJSON to selectively JSON-decode string values when the JSON Schema unambiguously expects an object or array (including $ref/$defs, unions, combinators, and additionalProperties).
• Integrated coercion into Session.MCPServer via receiving middleware for tools/call, rewriting raw tool arguments only when coercion changes occur.
• Added unit tests for coercion behavior and an end-to-end MCP test reproducing the client-side stringification bug.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
runtime/server/mcp_test.go	Extends MCP e2e test to cover stringified nested args for query_metrics_view.
runtime/pkg/jsonschemautil/coerce.go	Implements schema-guided coercion for stringified JSON objects/arrays.
runtime/pkg/jsonschemautil/coerce_test.go	Adds unit tests covering coercion matrix and integer fidelity.
runtime/ai/mcp.go	Adds MCP receiving middleware to coerce raw tool arguments before validation.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (3)

runtime/pkg/jsonschemautil/coerce.go:180

• Decoder.More does not verify EOF at the top level; it only checks whether another array/object element is available. As a result, malformed strings such as {}] or []} are accepted and rewritten as valid arguments, contrary to the fail-open contract. Check that the unconsumed suffix is whitespace instead.

	if dec.More() {
		return nil, false
	}
	return v, true

runtime/pkg/jsonschemautil/coerce.go:196

• “Exactly one branch defines the key” is not sufficient for anyOf/oneOf: a branch without that property permits it by default, so the original string may validly select that branch. Coercing it can silently change handler input or make a previously valid oneOf match multiple branches and fail validation. Union traversal should only occur when every applicable branch constrains the property to a compatible container type (accounting for additionalProperties).

	// Search combinator branches; only trust the result if exactly one branch defines the key.
	var found *jsonschema.Schema
	var foundDefs map[string]*jsonschema.Schema
	for _, branches := range [][]*jsonschema.Schema{s.AllOf, s.AnyOf, s.OneOf} {
		for _, b := range branches {

runtime/pkg/jsonschemautil/coerce.go:224

• The same union ambiguity applies to array items: an anyOf/oneOf branch with no items keyword allows arbitrary elements, so finding items in only one branch does not mean a string element unambiguously needs coercion. Only recurse when every applicable union branch constrains items compatibly; keep allOf handling separate because its constraints are conjunctive.

	var found *jsonschema.Schema
	var foundDefs map[string]*jsonschema.Schema
	for _, branches := range [][]*jsonschema.Schema{s.AllOf, s.AnyOf, s.OneOf} {
		for _, b := range branches {
			b, branchDefs := resolveSchema(b, defs)

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (2)

runtime/pkg/jsonschemautil/coerce.go:227

• The same ambiguity applies to array alternatives: an anyOf/oneOf branch without items allows arbitrary element values by default. For example, anyOf: [{items: {type: object}}, {maxItems: 10}] accepts string elements through the second branch, but this loop selects the sole explicit items schema and rewrites those strings. Require compatible item constraints across every applicable alternative before recursing.

	for _, branches := range [][]*jsonschema.Schema{s.AllOf, s.AnyOf, s.OneOf} {
		for _, b := range branches {
			b, branchDefs := resolveSchema(b, defs)
			if b == nil {
				continue

runtime/pkg/jsonschemautil/coerce.go:197

• An anyOf/oneOf branch that does not declare this key still permits it by default, so finding exactly one branch with an explicit property schema is not unambiguous. For example, with anyOf: [{properties: {x: {type: object}}}, {properties: {mode: {type: string}}}], a string-valued x is valid through the second branch, but this code rewrites it to an object. That changes already-valid string arguments despite the conservative contract. Coercion should proceed only when every alternative that can accept the key imposes a compatible container type (or otherwise rejects the key).

This issue also appears on line 223 of the same file.

	// Search combinator branches; only trust the result if exactly one branch defines the key.
	var found *jsonschema.Schema
	var foundDefs map[string]*jsonschema.Schema
	for _, branches := range [][]*jsonschema.Schema{s.AllOf, s.AnyOf, s.OneOf} {
		for _, b := range branches {

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

runtime/pkg/jsonschemautil/coerce.go:135

• effectiveTypes never inspects allOf, so an object/array constraint wrapped in allOf is treated as unknown and its stringified value is not decoded. For example, {"allOf":[{"$ref":"#/$defs/Expression"}]} unambiguously requires an object but currently remains a string; this also makes the allOf traversal in propertySchema/itemsSchema ineffective when the child schema itself uses this common wrapper. Derive the allowed type by intersecting the known allOf branch types (while retaining the current union behavior for anyOf/oneOf) and add a regression case.

	if len(s.AnyOf) > 0 || len(s.OneOf) > 0 {

[pjain1]

---

P1 — Add a depth guard to propertySchema and itemsSchema

runtime/pkg/jsonschemautil/coerce.go:185, :216

Both take (s, defs, ...) and recurse through AllOf/AnyOf/OneOf branches with no bound. Unlike coerceValue, whose recursion is bounded by the depth of the value, this walk is driven purely by the schema — a combinator branch that $refs back to its own parent never terminates, and the failure is fatal error: stack overflow, which no recover() catches. One tool call takes down the runtime process.

Change: give both functions a depth int parameter, return nil, nil when depth > maxRefHops, and pass depth+1 on the recursive calls. Callers in coerceValue (lines 64 and 74) pass 0. This mirrors what effectiveTypes already does at line 117.

Test to add: a schema of the form {"$defs": {"A": {"allOf": [{"$ref": "#/$defs/A"}]}}} referenced from a property, asserting the value is returned untouched rather than crashing.

---

P2 — Stop following a $ref whose siblings allow strings

runtime/pkg/jsonschemautil/coerce.go:101-108

resolveSchema replaces the schema wholesale when it sees a $ref, discarding sibling keywords. In JSON Schema 2020-12 $ref composes with its siblings, so {"$ref": "#/$defs/T", "type": "string"} requires a string — but the coercer sees only T's "type": "object" and rewrites the string into an object. This is the single direction where the coercion can turn a valid call into an invalid one; everywhere else it only relaxes.

Change: inside the resolveSchema loop, before following the ref, return nil, defs if the node's own Type/Types includes "string". Needs a slices.Contains(s.Types, "string") check alongside s.Type == "string".

Deliberately narrow: coercion can only ever replace a string, so a sibling type is harmful only when it permits strings. Blocking on any sibling type instead would disable coercion for where_per_metrics_view, whose additionalProperties is {"type": ["null"], "$ref": "#/$defs/Expression"} — a live and legitimate coercion target that jsonschema-go's own inference emits.

Test to add: {"$ref": "#/$defs/TimeRange", "type": "string"} on a property, with a JSON-text string value, asserting changed == false.

---

Apart from these, I am wondering if we should gate this behind an instance level flag which is enabled by default but can be disabled if needed to skip unnecessary computation and also to test out when the bug is fixed.

[nishantmonu51]

@pjain1 Addressed your review:

P1: Fixed in 924095d — both functions now take a depth parameter capped at maxRefHops, mirroring effectiveTypes, with a regression test using the cyclic allOf shape for both the property and items paths. Confirmed the test hits fatal error: stack overflow without the guard.

P2: I do not think this case can occur. In JSON Schema 2020-12, $ref composes conjunctively with its sibling keywords, so {"$ref": "#/$defs/T", "type": "string"} requires the instance to satisfy both the sibling type: string and T. The coercer only decodes when the followed T is object/array-only — and in that case the composition never accepted strings in the first place (it is contradictory), so the decode can only turn one invalid value into another invalid value, never a valid call into an invalid one. Given that, I would rather not special-case sibling types in resolveSchema.

Flag: Added in 712f9bc — a mcp_tolerant_args feature flag, enabled by default, disableable per project via features: {mcp_tolerant_args: false} in rill.yaml.

[pjain1]

P2 — Bail out of resolveSchema when a $ref node has a type-bearing sibling

runtime/pkg/jsonschemautil/coerce.go:104

Still the only path where coercion can turn a valid call into an invalid one. Inside the resolveSchema loop, before following the ref, return nil, defs if the node's own Type/Types includes "string". Deliberately narrow — coercion only ever replaces a string, so a sibling type is harmful only when it permits one. Blocking on any sibling type would disable coercion for where_per_metrics_view, whose additionalProperties is {"type": ["null"], "$ref": "#/$defs/Expression"}.

Test: {"$ref": "#/$defs/TimeRange", "type": "string"} on a property with a JSON-text string value, asserting changed == false.

P3 — Honour patternProperties in propertySchema

runtime/pkg/jsonschemautil/coerce.go:222

Between the Properties lookup and the AdditionalProperties fallback, return nil, nil if any PatternProperties regex matches the key. Fail-open is right here: matching the pattern and coercing against it would be more correct but adds regex compilation per key for no real gain.

P4 — Handle prefixItems in itemsSchema

runtime/pkg/jsonschemautil/coerce.go:255

Simplest correct fix: return nil, nil when PrefixItems is non-empty, matching how ItemsArray already fails open. Honouring it per index would mean threading the element index into itemsSchema, which isn't worth it for a form no Rill schema uses.

---

Also I think it would be better to have the flag as instance level config than a feature flag in rill.yaml as this would require committing change to project file. Instance level flag can be easily set as env var through UI or cli without editing project files.

[nishantmonu51]

@pjain1 Addressed the second round:

P2: Done in b52b44f. You are right that this is worth guarding: strictly per 2020-12 the sibling composes conjunctively, but jsonschema-go emits type + $ref sibling shapes in practice (e.g. where_per_metrics_view), so I would rather not depend on the validator's exact semantics there. resolveSchema now returns nil when the node's own Type/Types includes "string", with your suggested test; the {"type": ["null"], "$ref": ...} shape still coerces.

P3/P4: Done in the same commit — propertySchema fails open for patternProperties-matched keys before the additionalProperties fallback, and itemsSchema fails open when prefixItems is present.

Flag: Agreed — replaced the feature flag with a rill.ai.mcp_tolerant_args instance config option (default true), so it can be set via env or CLI without editing project files. It is resolved once at MCP server construction, and resolution errors are logged. Documented in the rill.yaml reference. Manually verified end-to-end: setting rill.ai.mcp_tolerant_args=false in the project .env makes stringified arguments fail validation on the next MCP session, and setting it back to true restores the tolerant decoding.

[begelundmuller]

Since the coercing is a little risky, maybe only do it as a fallback when the normal code actually fails to parse? E.g. could be something like this:

Suggested change

	if method == "tools/call" {
	s.coerceMCPToolArgs(req)
	}
	return next(ctx, method, req)
	res, err := next(ctx, method, req)
	if method != "tools/call" || err == nil {
	return res, err
	}
	var jsonrpcErr *jsonrpc.Error
	if !errors.As(err, &jsonrpcErr) || jsonrpcErr.Code != jsonrpc.CodeInvalidParams {
	return res, err
	}
	if !s.coerceMCPToolArgs(req) {
	return res, err
	}
	return next(ctx, method, req)