fix(graphql): execute one operation per request, selected per spec (fixes #142) - #143
Merged
Merged
Conversation
) parse_document merged every operation's top-level selections into one flat dict, discarding operation ownership — with multiple operations selecting the same group, the later tree silently overwrote the earlier one on ≤6.1.2 and the executor serialized with the WRONG projection (field leakage; measured: a mutation declaring { id } returned email). Since specs/023 the same document raised ResponseKeyConflictError instead — visible, but rejecting legal GraphQL. - QueryParser.parse_operations(): per-operation parse trees carrying definition/operation/name; in-operation duplicate response keys still conflict; parse_document keeps its legacy flat behavior (documented) - execute_query(parsed_operations=...): GraphQL-spec selection — operationName matches a named operation, a single anonymous operation runs as-is, several operations without a name is an error (graphql-core wording). The legacy all-definitions path stays for direct callers - GraphQLHandler uses the spec path — the operationName it always accepted (and federation always passed) finally takes effect - compose_query: exactly one operation per document (no operationName channel on a bare MCP query string); multi-operation documents are rejected with a clear message Behavior change: multi-operation documents without a name used to execute ALL definitions (with cross-contaminated selections); they now error. Single-operation requests are unaffected. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #142.
parse_documentmerged every operation's top-level selections into one flat dict, discarding operation ownership:{ id }returnedemailtoo. Response keys overwrote each other as well, and mutations in both operations executed.ALIAS_CONFLICT— safer, but rejecting legal GraphQL (multi-operation documents +operationNameare spec-standard).This PR restores the operation dimension end to end: parse per operation, execute exactly one operation per request.
Changes
QueryParser.parse_operations()definition/operation/name/selections). Same-name groups in different operations coexist; in-operation duplicate response keys still conflict (specs/023 FR-007).parse_documentkeeps its legacy flat behavior, now documented with a CAUTION.execute_query(parsed_operations=...)operationNamematches a named operation; a single anonymous operation runs as-is; several operations without a name is an error (graphql-core wording: "Must provide operation name if query contains multiple operations."). The legacy all-definitions path remains for direct callers.GraphQLHandleroperation_nameit always accepted (and the federation transport always passed,introspect.py) finally takes effect.compose_queryoperationNamechannel); multi-operation documents are rejected with a clear message.Behavior change
Multi-operation documents without a name used to execute ALL definitions (with cross-contaminated selections); they now error per spec. Single-operation requests — the overwhelmingly common case — are completely unaffected (locked by tests).
Verification
emailleak), multi-op-without-name error, unknownoperationName, single-anonymous-op unchanged, legacy path unchanged, handler e2e (operationNameselection), compose multi-op rejection, parser unit tests.ruff check src/clean.🤖 Generated with Claude Code