[libgraphql-core-v1] Task 16.6b: Implement type extension merging#106
Merged
Conversation
Previously SchemaBuilder silently dropped every TypeExtension and SchemaExtension definition -- schema data loss relative to v0, with ExtensionOfUndefinedType and InvalidExtensionTypeKind existing as dead error kinds. This implements full extension support per the September 2025 spec's Type System Extensions sections. Design: - All 6 type-extension kinds merge in place onto the registered type: Object/Interface append fields + implements entries, Enum appends values, Union appends members, InputObject appends input fields, Scalar appends directives; every kind appends extension directives. Duplicate contributions reuse the existing duplicate error kinds with "first defined here" + spec notes; per-item error accumulation merges the extension's remaining valid contributions (v1 style; v0 aborted on first duplicate). - Extension-before-definition defers via owned PendingTypeExtension entries (eagerly converted from borrowed AST via the existing builder/ast_helpers pieces), applied in arrival order when the target type is absorbed -- matching v0's deferred-extension behavior per the plan's Unresolved Question 3, now resolved. - Extensions still pending at build() each produce an ExtensionOfUndefinedType error; kind mismatches produce InvalidExtensionTypeKind, which now carries the actual vs extension GraphQLTypeKind for an informative message. - extend schema merges root operations through load_root_operations(), factored out of load_schema_definition so both paths share duplicate handling. Schema-level directives are not yet stored anywhere (true for schema definitions too) so extension directives merge root ops only -- recorded in the plan's completion notes. - Extension-contributed fields get the same __-prefix rejection as the definition path. Tests: 36 new (253 crate total) covering per-kind merge, extension- before-definition, undefined target, kind mismatch, and duplicate contributions, plus implements merging, dunder rejections, extend- schema merge + duplicate root op, spec-note assertions, and the 16.6a regression: `extend input X @oneOf` now triggers the oneOf constraints on X's fields. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TipQtpvuLuHjHmRsVEgwJw
Addresses the blocker from the pre-PR Opus review: September 2025 Input Object Extensions rule 5 states "The @OneOf directive must not be provided by an Input Object type extension." The initial 16.6b implementation (following 16.6a's completion-note framing) instead merged the directive and let the field-nullability constraints fire -- which produced the right error for the wrong reason on non-nullable fields, and silently ACCEPTED an all-nullable input becoming oneOf via extension (a spec-invalid schema). Now `extend input X @oneOf` produces the new OneOfDirectiveProvidedByInputObjectExtension error (span on the directive annotation, spec note attached) and the directive is not merged, so the constraints correctly do not fire from an unmerged directive. The mirror direction still works and is now locked in by a test: a non-nullable field CONTRIBUTED BY an extension to an already-@OneOf input is rejected by the 16.6a validator, because validation runs over the fully-merged type. Also adds the implicit-schema-definition extend-schema test (extend schema with no explicit `schema {}` block, matching graphql-js's permissive reading of the Schema Extension rules) and records a tracking note: non-repeatable-directive uniqueness across extensions belongs to Task 16.6f's directive-application validator, which must run over merged types. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TipQtpvuLuHjHmRsVEgwJw
Contributor
There was a problem hiding this comment.
Pull request overview
Implements GraphQL SDL schema/type extension support in libgraphql-core-v1’s SchemaBuilder, fixing prior behavior where all extensions were silently dropped and enabling v0-parity merging semantics (including extension-before-definition deferral) with error accumulation.
Changes:
- Add owned pending-extension infrastructure and merge logic for all type extension kinds (object/interface/enum/union/input/scalar), including duplicate detection and kind-mismatch/undefined-target errors.
- Implement
extend schema { ... }root operation merging by factoring shared root-op loading/duplicate handling. - Add a comprehensive new test suite covering merge behavior, ordering, error cases,
extend schema,__-name rejection, and@oneOf× extensions rule handling.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| libgraphql-core-v1-plan.md | Marks Task 16.6b complete and documents the final design/semantics (pending extensions, merge-before-validate, schema extension behavior, oneOf rule 5). |
| crates/libgraphql-core-v1/src/schema/tests/schema_builder_extension_tests.rs | Adds extensive tests for all extension kinds, ordering, duplicates, undefined targets, kind mismatch, schema extensions, and oneOf-related rules. |
| crates/libgraphql-core-v1/src/schema/tests/mod.rs | Registers the new extension test module. |
| crates/libgraphql-core-v1/src/schema/schema_builder.rs | Implements schema/type extension loading, pending extension application, build-time undefined-target errors, and merge helpers. |
| crates/libgraphql-core-v1/src/schema/schema_build_error.rs | Extends InvalidExtensionTypeKind diagnostics and adds OneOfDirectiveProvidedByInputObjectExtension. |
| crates/libgraphql-core-v1/src/schema/pending_type_extension.rs | Introduces owned PendingTypeExtension representations extracted from AST for deferral and in-place merging. |
| crates/libgraphql-core-v1/src/schema/mod.rs | Wires in the new pending-extension module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
Task 16.6b (second of the six schema-hardening PRs): implement type-extension and schema-extension support. Previously
SchemaBuildersilently dropped every extension (TypeExtension(_) | SchemaExtension(_) => {}) — schema data loss relative to v0, withExtensionOfUndefinedTypeandInvalidExtensionTypeKindsitting as dead error kinds.Design
implementsentries; Enum appends values; Union appends members; InputObject appends input fields; Scalar appends directives; every kind appends extension directives. Duplicate contributions reuse the existing duplicate error kinds with "first defined here" + spec notes. Per-item error accumulation: a duplicate is skipped, the extension's remaining valid contributions still merge (v1's error-accumulation style; v0 aborted on first duplicate).PendingTypeExtensionentries (newschema/pending_type_extension.rs; data extracted eagerly from the borrowed AST via the existingast_helpers/builder conversions), applied in arrival order when the target is absorbed — v0-parity behavior, resolving the plan's Unresolved Question 3. Extensions still pending atbuild()each produceExtensionOfUndefinedType. Kind mismatch producesInvalidExtensionTypeKind, which now carries actual-vs-extensionGraphQLTypeKindfor an informative message.extend schemamerges root operations viaload_root_operations(), factored out ofload_schema_definitionso both paths share duplicate handling. Works with implicitly-defined schemas (no explicitschema {}block), matching graphql-js. Schema-level directive annotations aren't stored onSchemaat all yet (true forschema {}definitions too) — recorded honestly in the plan notes.@oneOf× extensions, both directions (surfaced by the pre-PR Opus review, which caught my initial implementation violating Input Object Extensions rule 5):extend input X @oneOfis rejected with the newOneOfDirectiveProvidedByInputObjectExtensionerror and the directive is not merged — even an all-nullable input must not become oneOf via extension. Conversely, a non-nullable field contributed by an extension to an already-@oneOfinput is caught by the 16.6a constraints, because validation runs over the fully-merged type (merge-before-validate ordering, now locked in by test).__-prefix rejection as the definition path.Ordering-correctness notes (review-verified)
Pending extensions are consumed on the first successful type registration, and the duplicate-type check returns before consumption — so a duplicate second definition cannot double-apply extensions. Ext-vs-ext collisions are caught (the second extension sees the first's merged contribution). A mismatched-kind extension errors without affecting sibling extensions.
Tests
39 new tests (v1 crate: 217 → 256): per-kind merge / extension-before-definition / undefined target / kind mismatch / duplicate contribution; implements merge + duplicate; dunder rejections;
extend schemamerge + duplicate root-op + implicit-schema case; spec-note assertions; oneOf rule-5 rejection (all-nullable smoking-gun + not-merged-no-constraint-errors) and rule-6 merged-field constraint.Tracking
Non-repeatable-directive uniqueness across extensions ("must not already apply to the previous type", present in every extension section) is owned by Task 16.6f's directive-application validator, which the plan now requires to run over merged types.
Verification
cargo test --workspace— 1361 tests greencargo clippy --workspace --tests -- -Dwarnings— clean[x]boxes + Completion Notes per the Execution Protocol🤖 Generated with Claude Code
https://claude.ai/code/session_01TipQtpvuLuHjHmRsVEgwJw
Generated by Claude Code