Skip to content

Validate constant directive values on variable definitions (#3927)#4271

Open
andimarek wants to merge 2 commits intomasterfrom
claude/fix-issue-3927-mrgkP
Open

Validate constant directive values on variable definitions (#3927)#4271
andimarek wants to merge 2 commits intomasterfrom
claude/fix-issue-3927-mrgkP

Conversation

@andimarek
Copy link
Member

@andimarek andimarek commented Feb 25, 2026

Per the GraphQL spec, directives applied to variable definitions accept
only constant values. Variable references like $v are not allowed in
directive arguments on variable definitions. For example, the query
query ($v:Int @dir(arg:$v)) { ... } should be rejected.

Add a new validation rule VARIABLES_NOT_ALLOWED_IN_DIRECTIVES_ON_VARIABLE_DEFINITIONS
that detects variable references inside directive arguments on variable
definitions and reports a VariableNotAllowed error.

https://claude.ai/code/session_01QH4Ce5Fs9zfEjktZV4j1i2

@claude
Validate constant directive values on variable definitions (#3927)
8068a76 
Per the GraphQL spec, directives applied to variable definitions accept
only constant values. Variable references like $v are not allowed in
directive arguments on variable definitions. For example, the query
`query ($v:Int @dir(arg:$v)) { ... }` should be rejected.

Add a new validation rule VARIABLES_NOT_ALLOWED_IN_DIRECTIVES_ON_VARIABLE_DEFINITIONS
that detects variable references inside directive arguments on variable
definitions and reports a VariableNotAllowed error.

https://claude.ai/code/session_01QH4Ce5Fs9zfEjktZV4j1i2
@github-actions
Copy link
Contributor

github-actions bot commented Feb 25, 2026 •
edited
Loading

Test Results

  336 files  +1    336 suites  +1   5m 5s ⏱️ -8s
5 385 tests +7  5 377 ✅ +7  8 💤 ±0  0 ❌ ±0 
5 474 runs  +7  5 466 ✅ +7  8 💤 ±0  0 ❌ ±0 

Results for commit 2f019be. ± Comparison against base commit 9116159.

This pull request removes 196 and adds 179 tests. Note that renamed tests count towards both. 
	?

	, expected: combo-\"\\\b\f\n\r\t, #4]
                __schema { types { fields { args { type { name fields { name }}}}}}
                __schema { types { fields { type { name fields { name }}}}}
                __schema { types { inputFields { type { inputFields { name }}}}}
                __schema { types { interfaces { fields { type { interfaces { name } } } } } }
                __schema { types { name} }
                __type(name : "t") { name }
                a1: __schema { types { name} }
                a1: __type(name : "t") { name }
…

graphql.AssertTest ‑ assertFalse with different number of error args but false does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_21prov0_closure23@717d7587 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
graphql.AssertTest ‑ assertFalse with different number of error args but false does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_21prov0_closure24@2489e84a delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2, #1]
graphql.AssertTest ‑ assertFalse with different number of error args but false does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_21prov0_closure25@2fd954f delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2 arg3, #2]
graphql.AssertTest ‑ assertFalse with different number of error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_20prov0_closure20@7c51782d delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
graphql.AssertTest ‑ assertFalse with different number of error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_20prov0_closure21@4fe533ff delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2, #1]
graphql.AssertTest ‑ assertFalse with different number of error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_20prov0_closure22@5d5b9ecb delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2 arg3, #2]
graphql.AssertTest ‑ assertNotNull with different number of  error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_5prov0_closure3@6468a7b6 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
graphql.AssertTest ‑ assertNotNull with different number of  error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_5prov0_closure4@2121d1f9 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2, #1]
graphql.AssertTest ‑ assertNotNull with different number of  error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_5prov0_closure5@46e64760 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2 arg3, #2]
graphql.AssertTest ‑ assertNotNull with different number of error args with non null does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_6prov0_closure6@60c73e58 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
…

♻️ This comment has been updated with latest results.

@andimarek andimarek mentioned this pull request Feb 25, 2026
Open
bbakerman
bbakerman approved these changes Feb 28, 2026
View reviewed changes
Copy link
Member

@bbakerman bbakerman left a comment

Choose a reason for hiding this comment

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

Thanks Claude

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bbakerman bbakerman bbakerman approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@andimarek @bbakerman @claude