fix(jsonschema): emit $ref/$defs for recursive input types

[asoorm]

Problem

BuildJsonSchema inlines input types and truncates recursion at a fixed depth, dropping self-referential fields (e.g. left/right). With additionalProperties: false, valid nested payloads are then rejected: additional properties 'left', 'right' not allowed. The same input passes normal GraphQL; only the generated JSON Schema (MCP path) rejects it.

Also fixes a tracker leak: the early return nil at the depth limit skipped cleanup, leaking the recursion counter across sibling uses of a type.

Fix

Emit $ref/$defs for recursive input types: a first pass finds self-/mutually-recursive types; each is emitted once under root $defs and referenced via $ref. Recursion terminates by construction and nesting works to any depth. Non-recursive types are unchanged (still inlined). maxRecursionDepth is kept on the exported signatures for compatibility but is now a no-op.

Design note

Revisits #1124, which deliberately dropped $def for LLM compatibility. That holds for non-recursive inputs (kept inline); for recursive inputs the only alternative to $ref is a schema that rejects valid data, so $defs is reintroduced for recursive types only. The validator (santhosh-tekuri/jsonschema) supports it.

Tests

• New recursive_input_test.go: validates a nested recursive payload against the generated schema using santhosh-tekuri/jsonschema/v5.
• Updated the two tests that encoded the old truncation behaviour; non-recursive golden tests unchanged.

Closes ENG-9631

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e8742dcd-1d2c-40c4-aa80-cf3bacee55e6

📥 Commits

Reviewing files that changed from the base of the PR and between d727f4e and f42d4d9.

📒 Files selected for processing (1)

• v2/pkg/engine/jsonschema/variables_schema.go

🚧 Files skipped from review as they are similar to previous changes (1)

• v2/pkg/engine/jsonschema/variables_schema.go

---

📝 Walkthrough

Walkthrough

Refactor variable JSON Schema emission to detect recursive input-object types, emit reusable definitions under $defs, return $ref references for recursive types, update the builder state and build flow, and add tests (including compiled-schema validation of a nested recursive payload).

Changes

Recursive Input Types via $ref/$defs

Layer / File(s)	Summary
JSON Schema Contract - Add $ref/$defs Support v2/pkg/engine/jsonschema/schema.go	JsonSchema gains Ref and Defs fields. MarshalJSON conditionally emits $ref and $defs. NewRefSchema constructor creates nullable reference schemas pointing to #/$defs/<typeName>.
VariablesSchemaBuilder State Reorganization v2/pkg/engine/jsonschema/variables_schema.go	Builder replaces recursion-depth tracking with recursiveTypes and defs maps. Constructor initializes new state and per-build reset recomputes recursive types; small comment updates reflect unresolved-type skips.
Recursive Detection, Type Processing & Definition Caching v2/pkg/engine/jsonschema/variables_schema.go	Add dependency-graph-based detection of recursive input types; emit $ref for recursive named input objects; use ensureDef placeholder to generate/cache $defs entries and attach them to the root schema; update BuildJsonSchema to use the new builder.
Test updates and integration test v2/pkg/engine/jsonschema/variables_schema_test.go, v2/pkg/engine/jsonschema/recursive_input_test.go	Update recursive and mutually-recursive tests to assert $defs/$ref emission and JSON structure changes; add TestRecursiveInputAcceptsNestedPayload to validate nested recursive payloads are accepted by the compiled schema.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: emitting $ref/$defs for recursive input types instead of truncating recursion.
Description check	✅ Passed	The description clearly explains the problem, fix, and design rationale, directly relating to all changes in the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ahmet/eng-9631-json-schema-recursive-refs

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

_{Tip: disable this comment in your organization's Code Review settings.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

v2/pkg/engine/jsonschema/variables_schema.go (2)

124-127: 💤 Low value

Stale comment: recursion depth truncation no longer applies.

The comment references "maximum recursion depth" but that behavior was removed. The nil check is still valid for error cases (e.g., undefined types), but the comment should reflect the actual reason.

📝 Suggested comment update

-	// Skip this variable if we reached maximum recursion depth
+	// Skip this variable if type processing failed (e.g., undefined type)
 	if varSchema == nil {
 		return
 	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@v2/pkg/engine/jsonschema/variables_schema.go` around lines 124 - 127, The
existing comment above the nil-check for varSchema is stale (mentions "maximum
recursion depth"); update it to accurately state why varSchema can be nil and
why we return: replace the recursion-related text with a brief note like "skip
this variable if schema resolution failed or type is undefined" so readers know
varSchema == nil represents an unresolved/invalid schema/error case (refer to
the varSchema variable and the nil check in variables_schema.go).

---

394-397: 💤 Low value

Stale comment: recursion depth truncation no longer applies.

Same issue as line 124 - the comment references removed behavior.

📝 Suggested comment update

-	// Skip this field if we reached maximum recursion depth
+	// Skip this field if type processing failed
 	if fieldSchema == nil {
 		return
 	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@v2/pkg/engine/jsonschema/variables_schema.go` around lines 394 - 397, The
comment above the nil-check for fieldSchema is stale (it mentions recursion
depth truncation which no longer applies); update or remove that comment so it
accurately states why we return when fieldSchema == nil (e.g., "skip processing
when schema resolution returned nil / no schema available for this field") and
make the same change to the duplicate stale comment referenced earlier (the one
similar to line 124); ensure the comment references fieldSchema and the
nil-return behavior rather than recursion depth.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@v2/pkg/engine/jsonschema/variables_schema.go`:
- Around line 124-127: The existing comment above the nil-check for varSchema is
stale (mentions "maximum recursion depth"); update it to accurately state why
varSchema can be nil and why we return: replace the recursion-related text with
a brief note like "skip this variable if schema resolution failed or type is
undefined" so readers know varSchema == nil represents an unresolved/invalid
schema/error case (refer to the varSchema variable and the nil check in
variables_schema.go).
- Around line 394-397: The comment above the nil-check for fieldSchema is stale
(it mentions recursion depth truncation which no longer applies); update or
remove that comment so it accurately states why we return when fieldSchema ==
nil (e.g., "skip processing when schema resolution returned nil / no schema
available for this field") and make the same change to the duplicate stale
comment referenced earlier (the one similar to line 124); ensure the comment
references fieldSchema and the nil-return behavior rather than recursion depth.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f32b0f20-a5d9-4635-bbaa-bdc19d9fdba5

📥 Commits

Reviewing files that changed from the base of the PR and between 3db7728 and d727f4e.

📒 Files selected for processing (2)

• v2/pkg/engine/jsonschema/variables_schema.go
• v2/pkg/engine/jsonschema/variables_schema_test.go