Skip to content

Fix pruning of list-wrapped @require input types during composition - #10346

Merged
michaelstaib merged 9 commits into
mainfrom
mst/fix-fusion-list-shaped-require
Sep 4, 2026
Merged

michaelstaib merged 9 commits into
mainfrom
mst/fix-fusion-list-shaped-require

Conversation

@michaelstaib

Copy link
Copy Markdown
Member

No description provided.

@github-actions

github-actions Bot commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Patch coverage

95.5% of changed lines covered (148/155)

File Covered Changed Patch %
…/Planning/OperationPlanner.BuildExecutionTree.cs 1 3 33.3% 🔴
…/src/Fusion.Execution/Execution/Results/ResultDataMapper.cs 7 9 77.8% 🔴
…/Satisfiability/SatisfiabilityFactsBuilder.cs 22 24 91.7% 🟡
…/Fusion/src/Fusion.Execution/Planning/OperationPlanner.cs 13 14 92.9% 🟡
…/Extensions/MutableSchemaDefinitionExtensions.cs 3 3 100.0% 🟢
…/Fusion.Composition/Satisfiability/RequirementsValidator.cs 90 90 100.0% 🟢
…/Fusion/src/Fusion.Composition/SatisfiabilityValidator.cs 12 12 100.0% 🟢
Uncovered changed lines (JSON)
{
  "sha": "162e2c178d02745133f141be64bb1ad0412fff6b",
  "files": [
    { "path": "src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.BuildExecutionTree.cs", "ranges": [[2311, 2312]] },
    { "path": "src/HotChocolate/Fusion/src/Fusion.Execution/Execution/Results/ResultDataMapper.cs", "ranges": [[229, 230]] },
    { "path": "src/HotChocolate/Fusion/src/Fusion.Composition/Satisfiability/SatisfiabilityFactsBuilder.cs", "ranges": [[120, 121]] },
    { "path": "src/HotChocolate/Fusion/src/Fusion.Execution/Planning/OperationPlanner.cs", "ranges": [[2966, 2966]] }
  ]
}

Project coverage: 57.8% (287523/497449 lines)

GetPreservedTypeNames unwrapped @require and lookup argument types with
InnerType(), which strips a single wrapper. Argument types such as
[Foo!]! therefore lost their protection and the input type was removed
from the composed schema as unreferenced. At runtime the planner then
failed with a KeyNotFoundException in DoVariablesContainUploadScalar
whenever the composite schema also contained an Upload scalar.

Use NamedType() so the named type survives any list or non-null
wrapping, and make the upload check resolve variable types without
throwing so a missing or inaccessible type never aborts planning.
… null

ResultDataMapper.VisitPathObject returned false for a null value
regardless of the declared requirement type, so a requirement such as
promoCode.{ discountPercent isExpired } into a nullable input object
made the dependent operation node skip entirely. The resolver was never
invoked and a non-null field came back null without an error.

Pass the requirement type into VisitPathObject and write null for a
nullable position, matching VisitPath and VisitPathList.
@michaelstaib
michaelstaib force-pushed the mst/fix-fusion-list-shaped-require branch from 9c1335a to 0ee5a06 Compare September 3, 2026 13:58
… lookups

When the leftover of a requirement was planned as a lookup and part of
that lookup could not be resolved by the same source schema, the child
lookups were pushed to the backlog without the dependents of the parent
work item. The same happened for the unresolvable remainder of a
requirement that was partially inlined into an existing step. The step
consuming the requirement therefore depended only on the step producing
the list, not on the steps producing the leaf values, and could execute
before they existed. For a requirement such as
items(first: 50).nodes[product.discountedPrice] the consumer was skipped
and the field came back null without an error.

Forward the dependents to the unresolvable children in both places.
…uired parent

When a second field at the same path carried the same requirement, the
planner tried to inline it into the re-entrant lookup step created for
the first field. Three things went wrong on that path: the merge into
the nested selection set used the consuming step's root type instead of
the requiring field's declaring type, which threw once a sibling such
as product { id } had a sub-selection; the merge path evaluated the
carrier checks and the dependency against the root step instead of the
merged lookup step, producing plans in which a step consumed a value
from a step that depended on it; and a step of the requiring schema was
accepted as a carrier although it can never resolve the required data,
adding a dead selection and a spurious edge.

Use the declaring type for the merge, target the merged step in the
merge path, and skip same-schema carrier candidates. Existing plans
with two requirements now fetch the shared requirement once.
…equirement

A field with @require whose data comes from another schema is resolved
in a second call to the declaring schema, which needs a lookup for the
field's type whose key is available from the first call. The
satisfiability validator only checked that the required data itself was
reachable, so a schema such as CartItem.unitPrice requiring
product.discountedPrice without any lookup for CartItem composed
successfully and every query failed at plan time with "No possible plan
was found".

Require that the declaring schema can be re-entered at the type through
one of its lookups, or that the requirement is resolvable without the
declaring schema at all, and report a nested satisfiability error that
names the field, the schema and the missing lookup otherwise.
…ments

The ancestor fallback of requirement inlining, taken when the requiring
field sits under an inline fragment, still accepted a step of the
requiring schema as a carrier. The plan did not fail but carried a dead
product { __typename } selection and a spurious dependency. Apply the
same skip as in the candidate loop.
@michaelstaib michaelstaib changed the title Fix pruning of list-wrapped @require input types during composition Fix pruning of list-wrapped @require input types during composition Sep 3, 2026
@michaelstaib
michaelstaib merged commit fe28fcd into main Sep 4, 2026
6 of 7 checks passed
@michaelstaib
michaelstaib deleted the mst/fix-fusion-list-shaped-require branch September 4, 2026 11:44
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.

1 participant