Skip to content

[Fusion] Fix composition crash on unimplemented merged interface field - #9821

Merged
glen-84 merged 2 commits into
mainfrom
gai/implemented-by-inaccessible-missing-field
Jun 2, 2026
Merged

glen-84 merged 2 commits into
mainfrom
gai/implemented-by-inaccessible-missing-field

Conversation

@glen-84

@glen-84 glen-84 commented Jun 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Composing two source schemas that declare the same-named interface with different fields crashed with KeyNotFoundException: The given key '…' was not present in the dictionary during post-merge validation.
  • ImplementedByInaccessibleRule indexed type.Fields[interfaceField.Name] directly, assuming every implementing type provides every field of the merged interface. When a type implements the interface but is missing a field contributed by another subgraph, the lookup threw.
  • Guard the lookup with TryGetField and skip absent fields; the unimplemented-field case is already reported by InterfaceFieldNoImplementationRule, which now surfaces a clear INTERFACE_FIELD_NO_IMPLEMENTATION error instead of an opaque crash.

Test plan

  • Added ImplementedByInaccessibleRuleTests.Validate_MergedInterfaceFieldNotOnAllImplementers_Succeeds, which merges two subgraphs whose shared interface has disjoint fields (previously crashed).
  • All 7 ImplementedByInaccessibleRuleTests pass, including the 6 existing @inaccessible detection cases.

Copilot AI review requested due to automatic review settings June 2, 2026 12:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a post-merge validation crash in Fusion composition when merged interface fields are not implemented by every implementing type, replacing a failing dictionary index with a guarded lookup so validation can continue and surface the intended INTERFACE_FIELD_NO_IMPLEMENTATION error from the appropriate rule.

Changes:

  • Updated ImplementedByInaccessibleRule to use TryGetField and skip interface fields that aren’t present on the implementing type (instead of throwing KeyNotFoundException).
  • Added a regression test ensuring ImplementedByInaccessibleRule validation does not crash when merged interfaces contain disjoint fields across subgraphs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/HotChocolate/Fusion/src/Fusion.Composition/PostMergeValidationRules/ImplementedByInaccessibleRule.cs Prevents crash by guarding field lookup on implementing types during interface-field iteration.
src/HotChocolate/Fusion/test/Fusion.Composition.Tests/PostMergeValidationRules/ImplementedByInaccessibleRuleTests.cs Adds coverage for merged interface unions where implementers don’t all define every interface field.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@glen-84
glen-84 merged commit a2a4897 into main Jun 2, 2026
275 of 281 checks passed
@glen-84
glen-84 deleted the gai/implemented-by-inaccessible-missing-field branch June 2, 2026 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants