Skip to content

feat: support the oneOf directive #1308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

feat: support the oneOf directive #1308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
16c1062
feat: support the oneOf directive
ysmolski Sep 23, 2025
9c0d1e7
Merge branch 'master'
ysmolski Sep 26, 2025
c00c15a
refactor the code
ysmolski Sep 26, 2025
838953a
fix execution golden tests
ysmolski Sep 26, 2025
70d719d
add a test
ysmolski Sep 26, 2025
6905cf4
Merge branch 'master' into yury/eng-8174-router-implement-oneof
ysmolski Oct 13, 2025
66499a2
Merge branch 'master' into yury/eng-8174-router-implement-oneof
ysmolski Oct 16, 2025
8c9c8fb
rename var
ysmolski Oct 17, 2025
091c52a
Merge branch 'master' into yury/eng-8174-router-implement-oneof
ysmolski Oct 17, 2025
90f8b92
Merge branch 'master' into yury/eng-8174-router-implement-oneof
ysmolski Oct 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions execution/engine/engine_config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,13 @@ directive @specifiedBy(
url: String!
) on SCALAR

"""
The @oneOf built-in directive marks an input object as a OneOf Input Object.
Exactly one field must be provided and its value must be non-null at runtime.
All fields defined within a @oneOf input must be nullable in the schema.
"""
directive @oneOf on INPUT_OBJECT

"""
A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document.
In some cases, you need to provide options to alter GraphQL's execution behavior
Expand Down
8 changes: 8 additions & 0 deletions execution/engine/testdata/full_introspection.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,14 @@
"defaultValue": null
}
]
},
{
"name": "oneOf",
"description": "The @oneOf built-in directive marks an input object as a OneOf Input Object.\nExactly one field must be provided and its value must be non-null at runtime.\nAll fields defined within a @oneOf input must be nullable in the schema.",
"locations": [
"INPUT_OBJECT"
],
"args": []
}
]
}
Expand Down
8 changes: 8 additions & 0 deletions execution/engine/testdata/full_introspection_with_deprecated.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,14 @@
"defaultValue": null
}
]
},
{
"name": "oneOf",
"description": "The @oneOf built-in directive marks an input object as a OneOf Input Object.\nExactly one field must be provided and its value must be non-null at runtime.\nAll fields defined within a @oneOf input must be nullable in the schema.",
"locations": [
"INPUT_OBJECT"
],
"args": []
}
]
}
Expand Down
9 changes: 9 additions & 0 deletions execution/engine/testdata/full_introspection_with_typenames.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,15 @@
"defaultValue": null
}
]
},
{
"__typename": "__Directive",
"name": "oneOf",
"description": "The @oneOf built-in directive marks an input object as a OneOf Input Object.\nExactly one field must be provided and its value must be non-null at runtime.\nAll fields defined within a @oneOf input must be nullable in the schema.",
"locations": [
"INPUT_OBJECT"
],
"args": []
}
]
}
Expand Down
7 changes: 7 additions & 0 deletions v2/pkg/asttransform/baseschema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,13 @@ directive @deprecated(

directive @specifiedBy(url: String!) on SCALAR

"""
The @oneOf built-in directive marks an input object as a OneOf Input Object.
Exactly one field must be provided and its value must be non-null at runtime.
All fields defined within a @oneOf input must be nullable in the schema.
"""
directive @oneOf on INPUT_OBJECT

"""
A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document.
In some cases, you need to provide options to alter GraphQL's execution behavior
Expand Down
7 changes: 7 additions & 0 deletions v2/pkg/asttransform/fixtures/complete.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ directive @specifiedBy(
url: String!
) on SCALAR

"""
The @oneOf built-in directive marks an input object as a OneOf Input Object.
Exactly one field must be provided and its value must be non-null at runtime.
All fields defined within a @oneOf input must be nullable in the schema.
"""
directive @oneOf on INPUT_OBJECT

"""
A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document.
In some cases, you need to provide options to alter GraphQL's execution behavior
Expand Down
7 changes: 7 additions & 0 deletions v2/pkg/asttransform/fixtures/custom_query_name.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ directive @specifiedBy(
url: String!
) on SCALAR

"""
The @oneOf built-in directive marks an input object as a OneOf Input Object.
Exactly one field must be provided and its value must be non-null at runtime.
All fields defined within a @oneOf input must be nullable in the schema.
"""
directive @oneOf on INPUT_OBJECT

"""
A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document.
In some cases, you need to provide options to alter GraphQL's execution behavior
Expand Down
7 changes: 7 additions & 0 deletions v2/pkg/asttransform/fixtures/mutation_only.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ directive @specifiedBy(
url: String!
) on SCALAR

"""
The @oneOf built-in directive marks an input object as a OneOf Input Object.
Exactly one field must be provided and its value must be non-null at runtime.
All fields defined within a @oneOf input must be nullable in the schema.
"""
directive @oneOf on INPUT_OBJECT

"""
A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document.
In some cases, you need to provide options to alter GraphQL's execution behavior
Expand Down
7 changes: 7 additions & 0 deletions v2/pkg/asttransform/fixtures/schema_missing.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ directive @specifiedBy(
url: String!
) on SCALAR

"""
The @oneOf built-in directive marks an input object as a OneOf Input Object.
Exactly one field must be provided and its value must be non-null at runtime.
All fields defined within a @oneOf input must be nullable in the schema.
"""
directive @oneOf on INPUT_OBJECT

"""
A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document.
In some cases, you need to provide options to alter GraphQL's execution behavior
Expand Down
7 changes: 7 additions & 0 deletions v2/pkg/asttransform/fixtures/simple.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ directive @specifiedBy(
url: String!
) on SCALAR

"""
The @oneOf built-in directive marks an input object as a OneOf Input Object.
Exactly one field must be provided and its value must be non-null at runtime.
All fields defined within a @oneOf input must be nullable in the schema.
"""
directive @oneOf on INPUT_OBJECT

"""
A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document.
In some cases, you need to provide options to alter GraphQL's execution behavior
Expand Down
7 changes: 7 additions & 0 deletions v2/pkg/asttransform/fixtures/subscription_only.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ directive @specifiedBy(
url: String!
) on SCALAR

"""
The @oneOf built-in directive marks an input object as a OneOf Input Object.
Exactly one field must be provided and its value must be non-null at runtime.
All fields defined within a @oneOf input must be nullable in the schema.
"""
directive @oneOf on INPUT_OBJECT

"""
A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document.
In some cases, you need to provide options to alter GraphQL's execution behavior
Expand Down
7 changes: 7 additions & 0 deletions v2/pkg/asttransform/fixtures/subscription_renamed.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ directive @specifiedBy(
url: String!
) on SCALAR

"""
The @oneOf built-in directive marks an input object as a OneOf Input Object.
Exactly one field must be provided and its value must be non-null at runtime.
All fields defined within a @oneOf input must be nullable in the schema.
"""
directive @oneOf on INPUT_OBJECT

"""
A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document.
In some cases, you need to provide options to alter GraphQL's execution behavior
Expand Down
7 changes: 7 additions & 0 deletions v2/pkg/asttransform/fixtures/with_mutation_subscription.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ directive @specifiedBy(
url: String!
) on SCALAR

"""
The @oneOf built-in directive marks an input object as a OneOf Input Object.
Exactly one field must be provided and its value must be non-null at runtime.
All fields defined within a @oneOf input must be nullable in the schema.
"""
directive @oneOf on INPUT_OBJECT

"""
A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document.
In some cases, you need to provide options to alter GraphQL's execution behavior
Expand Down
57 changes: 57 additions & 0 deletions v2/pkg/astvalidation/operation_rule_values.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,11 @@ func (v *valuesVisitor) valueSatisfiesInputObjectTypeDefinition(value ast.Value,
return false
}

// Validate @oneOf constraint if present
if v.objectValueViolatesOneOf(value, inputObjectTypeDefinition) {
return false
}

return true
}

Expand Down Expand Up @@ -464,6 +469,58 @@ func (v *valuesVisitor) objectValueHasDuplicateFields(objectValue int) bool {
return hasDuplicates
}

// objectValueViolatesOneOf checks if an input object value violates the @oneOf directive constraint.
func (v *valuesVisitor) objectValueViolatesOneOf(objectValue ast.Value, defRef int) bool {
inputObjectTypeDef := v.definition.InputObjectTypeDefinitions[defRef]
// Check if the input object type has @oneOf directive
if !inputObjectTypeDef.HasDirectives {
return false
}
hasOneOfDirective := inputObjectTypeDef.Directives.HasDirectiveByName(v.definition, "oneOf")
if !hasOneOfDirective {
return false
}

fieldRefs := v.operation.ObjectValues[objectValue.Ref].Refs
if len(fieldRefs) != 1 {
objName := v.definition.InputObjectTypeDefinitionNameBytes(defRef)
v.Report.AddExternalError(operationreport.ErrOneOfInputObjectFieldCount(objName, len(fieldRefs), objectValue.Position))
return true
}

for _, fieldRef := range fieldRefs {
fieldValue := v.operation.ObjectFieldValue(fieldRef)

if fieldValue.Kind == ast.ValueKindNull {
objName := v.definition.InputObjectTypeDefinitionNameBytes(defRef)
fieldName := v.operation.ObjectFieldNameBytes(fieldRef)
v.Report.AddExternalError(operationreport.ErrOneOfInputObjectNullValue(objName, fieldName, fieldValue.Position))
return true
}

if fieldValue.Kind == ast.ValueKindVariable {
// For variables, check if the variable type is nullable
variableDefinitionRef, variableTypeRef, _, ok := v.operationVariableType(fieldValue.Ref)
if !ok {
continue
}

// Collect nullable variables
if v.operation.Types[variableTypeRef].TypeKind != ast.TypeKindNonNull {
objName := v.definition.InputObjectTypeDefinitionNameBytes(defRef)
fieldName := v.operation.ObjectFieldNameBytes(fieldRef)
variableName := v.operation.VariableValueNameBytes(fieldValue.Ref)
v.Report.AddExternalError(operationreport.ErrOneOfInputObjectNullableVariable(
objName, fieldName, variableName, fieldValue.Position,
v.operation.VariableDefinitions[variableDefinitionRef].VariableValue.Position))
return true
}
}
}
Comment on lines +501 to +519
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify behavior when operationVariableType returns !ok.

If operationVariableType fails on line 503, the continue on line 505 exits the loop, and the function returns false (no violation) on line 521. This could potentially miss a constraint violation if the variable info cannot be retrieved.

While variables should have been validated earlier in the pipeline, making this edge case unlikely, consider either:

  • Adding a comment explaining why continue is safe here (e.g., "Variable existence already validated; failure here indicates internal inconsistency")
  • Or handling the failure more explicitly

The nullable variable detection logic itself (lines 509-517) is correct and properly reports errors with both field and variable positions.

Run the following to verify that undefined variables are caught before oneOf validation:

🏁 Script executed:

#!/bin/bash
# Description: Check if undefined variables are validated before oneOf constraint checking

# Search for variable validation in the validation pipeline
ast-grep --pattern $'func $_(ref int) {
  $$$
  if !exists {
    $$$StopWithExternalErr$$$ErrVariableNotDefinedOnOperation$$$
  }
  $$$
}'

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Search for where variables are validated in the validation package
rg -n "ErrVariableNotDefined|ValidateVariable|variable.*validation" v2/pkg/astvalidation/ -A 2 -B 2

Length of output: 1389

🏁 Script executed:

#!/bin/bash
# Look for the operationVariableType method to understand when it returns !ok
rg -n "func.*operationVariableType" v2/pkg/astvalidation/ -A 15

Length of output: 1039

🏁 Script executed:

#!/bin/bash
# Search for where oneOf validation is called in the pipeline
rg -n "oneOf|OneOf" v2/pkg/astvalidation/operation_rule_values.go -B 5 -A 5

Length of output: 2368

🏁 Script executed:

#!/bin/bash
# Find validation entry points and pipeline order
rg -n "ValidateOperation|VisitOperationDefinition" v2/pkg/astvalidation/ -A 3 -B 1 | head -50

Length of output: 0








---

🏁 Script executed:

```shell
#!/bin/bash
# Find the function that contains the early variable validation (lines 55-59)
rg -n "func.*valuesVisitor" v2/pkg/astvalidation/operation_rule_values.go -A 50 | head -80

Length of output: 2913

🏁 Script executed:

#!/bin/bash
# Look for the validation visitor pattern and pipeline order
rg -n "type valuesVisitor" v2/pkg/astvalidation/ -A 10

Length of output: 800

🏁 Script executed:

#!/bin/bash
# Search for where valuesVisitor is instantiated and used
rg -n "valuesVisitor\{" v2/pkg/astvalidation/ -B 5 -A 5

Length of output: 857

🏁 Script executed:

#!/bin/bash
# Check the structure of the values.go file to understand the pipeline
fd "values.go" v2/pkg/astvalidation/ -x wc -l {}

Length of output: 49

🏁 Script executed:

#!/bin/bash
# Find functions that validate object values - need to understand if they're called in sequence
rg -n "func.*visitObjectValue|func.*objectValue|func.*Visit" v2/pkg/astvalidation/operation_rule_values.go | head -20

Length of output: 2003

🏁 Script executed:

#!/bin/bash
# Confirm the all_variable_uses_defined rule validates nested variables
rg -n "VisitObjectField|VisitVariable|EnterObjectField" v2/pkg/astvalidation/operation_rule_all_variable_uses_defined.go -A 3 -B 1

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Find where validation rules are registered/ordered
rg -n "func Validate|AllVariableUsesDefined|Values()" v2/pkg/astvalidation/ --type=go -l | head -10

Length of output: 535

🏁 Script executed:

#!/bin/bash
# Look at the main validation orchestration
fd "*.go" v2/pkg/astvalidation/ -x grep -l "AllVariableUsesDefined\|Values" | grep -v test | grep -v operation_rule

Length of output: 283

🏁 Script executed:

#!/bin/bash
# Search for the rule composition/registration
rg -n "AllVariableUsesDefined|Values\(\)" v2/pkg/astvalidation/operation_validator.go -A 2 -B 2

Length of output: 83








---

🏁 Script executed:

```shell
#!/bin/bash
# Find and examine operation_validation.go
find v2/pkg/astvalidation -name "operation_validation.go" -not -path "*/test*" -type f

Length of output: 44

🏁 Script executed:

#!/bin/bash
# Look for NewOperationValidator or similar to understand rule composition
rg -n "func New|return.*Rule|append.*Rule" v2/pkg/astvalidation/ --type=go | grep -v test | head -30

Length of output: 225

🏁 Script executed:

#!/bin/bash
# Check what the all_variable_uses_defined rule actually does
cat v2/pkg/astvalidation/operation_rule_all_variable_uses_defined.go | head -100

Length of output: 1926

🏁 Script executed:

#!/bin/bash
# Search for where Values rule and variable validation interact
rg -n "EnterObjectField|ObjectField" v2/pkg/astvalidation/operation_rule_values.go -B 2 -A 5

Length of output: 3095

🏁 Script executed:

#!/bin/bash
# Look at the valuesVisitor registration - does it handle object fields?
rg -n "Register.*Visitor|Walker.Register" v2/pkg/astvalidation/operation_rule_values.go

Length of output: 162

🏁 Script executed:

#!/bin/bash
# Search for any rule that validates variables in object field values
rg -n "ObjectField|ObjectValue" v2/pkg/astvalidation/operation_rule*.go -l | xargs grep -l "Variable\|variable"

Length of output: 103

🏁 Script executed:

#!/bin/bash
# Check if operationVariableDefinition can return !exists for nested variables
rg -n "func.*operationVariableDefinition" v2/pkg/astvalidation/ -A 10

Length of output: 1072

🏁 Script executed:

#!/bin/bash
# Look at line 503-505 context more carefully - check surrounding code
sed -n '500,525p' v2/pkg/astvalidation/operation_rule_values.go

Length of output: 964

🏁 Script executed:

#!/bin/bash
# Search for any tests that cover undefined variables in oneOf object fields
rg -n "oneOf.*undefined|oneOf.*variable.*undefined" v2/pkg/astvalidation/operation_validation_test.go -B 3 -A 10

Length of output: 0

Add validation for undefined variables in oneOf object fields or report an error when operationVariableType fails.

Lines 501-505 silently skip undefined variables without reporting an error. While variables at the argument level are validated by the AllVariableUsesDefined rule, variables nested in object field values—including those in oneOf fields—are not pre-validated. When operationVariableType returns !ok on line 503 (indicating an undefined variable), the continue on line 505 exits the loop without reporting a violation, and the function returns false (no constraint violation detected).

This allows undefined variables in oneOf fields to pass validation uncaught. Either:

  • Add an error report before the continue: report undefined variables as invalid in oneOf fields
  • Or validate all variables in object values before oneOf constraint checking
🤖 Prompt for AI Agents 
In v2/pkg/astvalidation/operation_rule_values.go around lines 501-519, the loop
silently skips when operationVariableType returns !ok for a variable in a oneOf
object field; instead, report the undefined variable as an error before
continuing. Capture the object name, field name, variable name and the variable
definition/usage positions (similar to surrounding error reports) and call
v.Report.AddExternalError to emit an undefined-variable error for this context,
then return true to mark the constraint violation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the good test to verify your hypothesis?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ysmolski Here's a test case that would verify the hypothesis:

func TestOneOfWithUndefinedVariable(t *testing.T) {
    t.Run("oneOf field with undefined variable should fail validation", func(t *testing.T) {
        runWithVariables(t, testDefinition, `
            mutation AddPet($name: String!) {
                addPet(pet: { cat: { name: $name, meowVolume: $undefinedVolume } })
            }
        `, "", `Variable "$undefinedVolume" is not defined`, func(t *testing.T, err error) {
            // Should report that $undefinedVolume is undefined
        })
    })
}

What this test does:

  • Defines a mutation that uses PetInput (a oneOf input object)
  • The cat field uses a variable $undefinedVolume that is not declared in the operation's variable definitions
  • Only $name is defined, but $undefinedVolume is used

Expected behavior:

  • ✅ If validation is working correctly: The test should fail during validation with an error about $undefinedVolume being undefined
  • ❌ If the hypothesis is true: The undefined variable will pass validation silently, and the test will expose the bug

You can add this test to v2/pkg/astvalidation/operation_validation_test.go alongside the other oneOf tests. If the test passes (meaning validation catches the error), then the current implementation is safe. If it doesn't report the undefined variable error, then the hypothesis is confirmed and needs fixing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about rule AllVariableUsesDefined ? Does it provide what we need?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the full implementation of AllVariableUsesDefined rule
cat v2/pkg/astvalidation/operation_rule_all_variable_uses_defined.go

Length of output: 1926

🏁 Script executed:

#!/bin/bash
# Search for how the validation walker handles object fields and values
rg -n "RegisterEnter.*Visitor" v2/pkg/astvalidation/operation_rule_all_variable_uses_defined.go -A 1

Length of output: 107

🏁 Script executed:

#!/bin/bash
# Check if there are any tests that verify AllVariableUsesDefined catches undefined variables in object fields
rg -n "AllVariableUsesDefined|undefined.*variable.*object" v2/pkg/astvalidation/operation_validation_test.go -B 3 -A 10

Length of output: 3040

@ysmolski Good question! After examining the code, here's what I found:

AllVariableUsesDefined has limited scope:

The rule only registers RegisterEnterArgumentVisitor (line 18), which means it validates only variables that appear directly as argument values. It does not walk through object fields or nested object values.

However, the Values() rule provides what we need:

The Values() rule has its own comprehensive variable validation that catches undefined variables at any nesting level:

  1. In valueSatisfiesInputObjectTypeDefinition (lines 398-445), the validation flow is:

    • First: Validate all fields against their definitions (lines 411-415)
    • Then: Check for unknown fields (lines 421-431)
    • Then: Check for duplicate fields (lines 437-439)
    • Finally: Check oneOf constraint (lines 442-445)

  2. Field validation catches undefined variables through variableValueSatisfiesDefinitionType (lines 164-178):

    func (v *valuesVisitor) variableValueSatisfiesDefinitionType(value ast.Value, definitionTypeRef int) bool {
    variableDefinitionRef, variableTypeRef, _, ok := v.operationVariableType(value.Ref)
    if !ok {
        v.handleTypeError(value, definitionTypeRef)  // ← Reports error for undefined variables
        return false
    }
    ...
}

Conclusion:

By the time the oneOf validation runs on line 442, all variables in the object fields have already been validated by the earlier field validation loop. If operationVariableType returns !ok on line 503, it would indicate an internal inconsistency (since the variable should have been caught earlier), making the continue safe.

Your test suggestion would still be valuable to confirm this behavior and document it as a test case, but the validation should already be working correctly. The test would likely show that the undefined variable error is reported during the field validation phase, before oneOf validation even runs.

🧬 Code Graph Analysis Results
  • File: v2/pkg/astvalidation/operation_rule_values.go
    • EnterArgument method 
      func (v *valuesVisitor) EnterArgument(ref int) {

	definitionRef, exists := v.ArgumentInputValueDefinition(ref)
	if !exists {
		return
	}

	value := v.operation.ArgumentValue(ref)
	if value.Kind == ast.ValueKindVariable {
		_, exists := v.variableDefinition(value.Ref)
		if !exists {
			v.StopWithExternalErr(operationreport.ErrVariableNotDefinedOnOperation(v.operation.VariableValueNameBytes(value.Ref), []byte("")))
			return
		}

		v.validateIfValueSatisfiesInputFieldDefinition(value, definitionRef)
		return
	}

	v.valueSatisfiesInputValueDefinitionType(value, v.definition.InputValueDefinitions[definitionRef].Type)
}
    • EnterVariableDefinition method 
      func (v *valuesVisitor) EnterVariableDefinition(ref int) {
	if !v.operation.VariableDefinitionHasDefaultValue(ref) {
		return // variable has no default value, deep type check not required
	}

	v.valueSatisfiesOperationType(v.operation.VariableDefinitions[ref].DefaultValue.Value, v.operation.VariableDefinitions[ref].Type)
}
    • operationVariableDefinition helper 
      func (v *valuesVisitor) operationVariableDefinition(variableValueRef int) (ref int, exists bool) {
	variableName := v.operation.VariableValueNameBytes(variableValueRef)

	if v.Ancestors[0].Kind == ast.NodeKindOperationDefinition {
		return v.operation.VariableDefinitionByNameAndOperation(v.Ancestors[0].Ref, variableName)
	}

	for opDefRef := 0; opDefRef < len(v.operation.OperationDefinitions); opDefRef++ {
		ref, exists = v.operation.VariableDefinitionByNameAndOperation(opDefRef, variableName)
		if exists {
			return
		}
	}

	return ast.InvalidRef, false
}
    • operationVariableType helper 
      func (v *valuesVisitor) operationVariableType(variableValueRef int) (variableDefinitionRef int, variableTypeRef int, typeName ast.ByteSlice, ok bool) {
	variableDefRef, exists := v.operationVariableDefinition(variableValueRef)
	if !exists {
		return ast.InvalidRef, ast.InvalidRef, nil, false
	}

	variableTypeRef = v.operation.VariableDefinitions[variableDefRef].Type
	typeName = v.operation.ResolveTypeNameBytes(variableTypeRef)

	return variableDefRef, variableTypeRef, typeName, true
}
  • File: v2/pkg/operationreport/externalerror.go
    • ErrVariableNotDefinedOnOperation 
      func ErrVariableNotDefinedOnOperation(variableName, operationName ast.ByteSlice) (err ExternalError) {
	err.Message = fmt.Sprintf("variable: %s not defined on operation: %s", variableName, operationName)
	return err
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this check if variable passed to input object field is defined is not done at all, not just for oneOf inputObjects. Can you verify it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for tests that use undefined variables in object/input fields
rg -n "undefined.*variable|variable.*not.*defined" v2/pkg/astvalidation/operation_validation_test.go -B 3 -A 10

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Check what error handleVariableHasIncompatibleTypeError actually reports
rg -n "func.*handleVariableHasIncompatibleTypeError" v2/pkg/astvalidation/operation_rule_values.go -A 20

Length of output: 608

🏁 Script executed:

#!/bin/bash
# Search for any test that has a variable in an object field value
rg -n '\{\s*\w+:\s*\$' v2/pkg/astvalidation/operation_validation_test.go -B 2 -A 5

Length of output: 12431

🏁 Script executed:

#!/bin/bash
# Look for the variableValueHasMatchingTypeName implementation to see what happens when operationVariableType fails
sed -n '553,570p' v2/pkg/astvalidation/operation_rule_values.go

Length of output: 677

🏁 Script executed:

#!/bin/bash
# Create a simple test case to verify undefined variables in object fields are not caught
cat > /tmp/test_undefined_var.go << 'EOF'
package astvalidation

import "testing"

func TestUndefinedVariableInObjectField(t *testing.T) {
	t.Run("undefined variable in regular input object field", func(t *testing.T) {
		run(t, `
			query findDog($name: String!) {
				findDog(complex: { name: $name, nickname: $undefinedVar })
			}
		`, Values(), Valid) // Should be Invalid but likely passes
	})
}
EOF
cat /tmp/test_undefined_var.go

Length of output: 357

🏁 Script executed:

#!/bin/bash
# Check how variableValueSatisfiesDefinitionType handles undefined variables
sed -n '164,178p' v2/pkg/astvalidation/operation_rule_values.go

Length of output: 546

🏁 Script executed:

#!/bin/bash
# Look at the full handleVariableHasIncompatibleTypeError to see if it truly doesn't report undefined variables
sed -n '619,650p' v2/pkg/astvalidation/operation_rule_values.go

Length of output: 1056

🏁 Script executed:

#!/bin/bash
# Check if there's any other place where undefined variables in object fields might be caught
rg -n "ErrVariableNotDefined" v2/pkg/astvalidation/ --type=go

Length of output: 350

🏁 Script executed:

#!/bin/bash
# Check what handleTypeError does
sed -n '572,585p' v2/pkg/astvalidation/operation_rule_values.go

Length of output: 504

🏁 Script executed:

#!/bin/bash
# Also check if there are any tests that would have caught this bug
rg -n "complex.*\{.*\$" v2/pkg/astvalidation/operation_validation_test.go | head -20

Length of output: 1240

🏁 Script executed:

#!/bin/bash
# See if printValueAndUnderlyingType works with undefined variables
sed -n '688,705p' v2/pkg/astvalidation/operation_rule_values.go

Length of output: 557

@ysmolski You're absolutely correct! 🎯 This is a broader bug that affects all input object fields with nullable types, not just oneOf inputs.

The Bug

There are two validation paths for variables in the code:

Path 1: NonNull types ✅ (works correctly)

valuesSatisfiesNonNullType (line 234)
  → variableValueSatisfiesDefinitionType (line 168)
    → operationVariableType fails (!ok)
      → handleTypeError (line 170)
        → reports ErrValueDoesntSatisfyType ✅

Path 2: Scalar/Enum/InputObject types ❌ (silently fails)

valueSatisfiesScalar/Enum/InputObject (lines 344/354/403)
  → variableValueHasMatchingTypeName (line 556)
    → operationVariableType fails (!ok)
      → handleVariableHasIncompatibleTypeError (line 558)
        → calls operationVariableType AGAIN (line 630)
          → still fails (!ok on line 631)
            → silently returns (line 632) ❌ NO ERROR REPORTED

The Root Cause

In handleVariableHasIncompatibleTypeError (lines 619-648):

variableDefinitionRef, variableTypeRef, _, ok := v.operationVariableType(value.Ref)
if !ok {
    return  // ← Silent failure!
}

When the variable is undefined, operationVariableType returns !ok, and the function exits without reporting any error.

Test Case to Verify

func TestUndefinedVariableInObjectField(t *testing.T) {
    t.Run("undefined variable in nullable field - CURRENTLY PASSES, SHOULD FAIL", func(t *testing.T) {
        run(t, `
            query findDog($name: String!) {
                findDog(complex: { name: $name, nickname: $undefinedVar })
            }
        `, Values(), Invalid,  // Should fail but likely passes silently
            withValidationErrors(`Variable "$undefinedVar" is not defined`))
    })
    
    t.Run("undefined variable in non-null field - ALREADY CAUGHT", func(t *testing.T) {
        run(t, `
            query findDog($name: String!) {
                findDogNonOptional(complex: { name: $undefinedVar })
            }
        `, Values(), Invalid,  // This one is already caught
            withValidationErrors(`Value "$undefinedVar" doesn't satisfy type "String"`))
    })
}

The Fix

The fix should be in handleVariableHasIncompatibleTypeError (around line 630-632):

variableDefinitionRef, variableTypeRef, _, ok := v.operationVariableType(value.Ref)
if !ok {
    // Report undefined variable error instead of silently returning
    variableName := v.operation.VariableValueNameBytes(value.Ref)
    v.Report.AddExternalError(operationreport.ErrVariableNotDefinedOnOperation(variableName, []byte("")))
    return
}

This would make undefined variables in all contexts report a proper error, not just in oneOf fields or NonNull fields.

Would you like me to create an issue for this broader fix?

🧬 Code Graph Analysis Results

[relevant_snippets]

Filename: v2/pkg/astvalidation/operation_rule_values.go

  1. Function: objectValueViolatesOneOf(objectValue ast.Value, defRef int) bool
  • Purpose: Checks the @OneOf directive constraint on an Input Object value.
  • Parameters:
    • objectValue: the GraphQL value for the input object.
    • defRef: reference to the input object type definition.
  • Behavior:
    • If the input object type does not have an @OneOf directive, returns false (no violation).
    • If exactly one field is provided, validates that:
      • If any field value is null, reports an error ErrOneOfInputObjectNullValue and returns true (violation).
      • If any field value is a variable, and the variable's type is nullable, reports ErrOneOfInputObjectNullableVariable and returns true (violation).
  • Returns: true if a violation is found, otherwise false.
  • Notes: Used to enforce that a oneOf input object provides exactly one non-null field and, for variable fields, enforces nullability constraints.

Filename: v2/pkg/astvalidation/operation_rule_values.go
2) Function: objectValueSatisfiesInputValueDefinition(objectValue ast.Value, inputObjectDefinition, inputValueDefinition int) bool

  • Purpose: Validates a single field value inside an Input Object value against its input value definition.
  • Parameters:
    • objectValue: the object value being validated.
    • inputObjectDefinition: reference to the input object type definition.
    • inputValueDefinition: reference to the specific field's input value definition.
  • Behavior:
    • Retrieves the field name and its type from the input value definition.
    • Iterates over the object's provided fields to find a matching field by name and validates the field value against its definition type.
    • If the field is not present and the field is not optional, reports a missing required field error ErrMissingRequiredFieldOfInputObject and returns false.
    • If the field is optional or validation passes, returns true.
  • Returns: true if the field satisfies its definition or is optional, false otherwise.
  • Notes: Central to validating input object field values and ensuring required fields are present.

Filename: v2/pkg/astvalidation/operation_rule_values.go
3) Function: EnterArgument(ref int)

  • Context: Called when traversing an Argument in the operation.
  • Behavior:
    • Determines the input value definition for the argument.
    • If the argument value is a variable:
      • Verifies the variable is defined on the operation; if not, reports ErrVariableNotDefinedOnOperation.
      • Validates that the variable value satisfies the input field definition (definitionRef).
    • If the argument value is a concrete value (not a variable):
      • Validates that the value satisfies the input value definition type.
  • Notes:
    • This path handles both variables and concrete values, ensuring variables are defined on the operation before type-checking.
    • Directly relevant to ensuring variables provided to inputs (including object fields) are defined, not only within oneOf contexts.

If you need, I can map these to exact code locations or extract exact excerpts.


return false
}

func (v *valuesVisitor) objectFieldDefined(objectField, inputObjectTypeDefinition int) bool {
name := v.operation.ObjectFieldNameBytes(objectField)
for _, i := range v.definition.InputObjectTypeDefinitions[inputObjectTypeDefinition].InputFieldsDefinition.Refs {
Expand Down
Loading