feat(federation): correctness and parity fixes (composition + planner)#3025
Draft
jensneuse wants to merge 11 commits into
Draft
feat(federation): correctness and parity fixes (composition + planner)#3025jensneuse wants to merge 11 commits into
jensneuse wants to merge 11 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds ADRs plus composition and router regression coverage for compound-key routing, union/interface ChangesFederation planner and composition fixes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3025 +/- ##
===========================================
- Coverage 60.27% 46.58% -13.69%
===========================================
Files 477 1120 +643
Lines 62471 155243 +92772
Branches 6131 10172 +4041
===========================================
+ Hits 37656 72327 +34671
- Misses 24787 81070 +56283
- Partials 28 1846 +1818
🚀 New features to boost your workflow:
|
❌ Internal Query Planner CI checks failedThe Internal Query Planner CI checks failed in the celestial repository, and this is going to stop the merge of this PR. |
jensneuse
force-pushed
the
planner-improvements
branch
from
256e89f to
8d29088
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
router-tests/protocol/testdata/multihop_compound_key/list.graphqls (1)
13-15: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMake
pidrequired in this compound-key fixture.
pidis part of@key(fields: "id pid"), but this fixture makes it nullable. That weakens the entity identity contract for the regression and drifts from the composition-side fixture for the same scenario, which treats the compound-key member as required.Suggested change
type Product `@key`(fields: "id pid") { id: String! - pid: String + pid: String! }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@router-tests/protocol/testdata/multihop_compound_key/list.graphqls` around lines 13 - 15, Make the compound-key fixture consistent by changing the Product type so pid is required, since it is part of the `@key`(fields: "id pid") identity in this multihop_compound_key test data. Update the Product definition in list.graphqls to match the composition-side fixture and ensure pid is non-null alongside id.router-tests/protocol/multihop_compound_key_test.go (1)
67-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse semantic JSON comparison for the response body.
This exact string comparison will fail on harmless key-order or formatting changes in the router response.
require.JSONEqkeeps the regression focused on payload content.Suggested change
- require.Equal(t, `{"data":{"topProducts":{"products":[{"id":"1","pid":"p1","price":{"price":100},"category":{"mainProduct":{"id":"1"},"id":"c1","tag":"t1"}},{"id":"2","pid":"p2","price":{"price":200},"category":{"mainProduct":{"id":"2"},"id":"c2","tag":"t2"}}],"selected":{"id":"2"},"first":{"id":"1"}}}}`, res.Body) + require.JSONEq(t, `{"data":{"topProducts":{"products":[{"id":"1","pid":"p1","price":{"price":100},"category":{"mainProduct":{"id":"1"},"id":"c1","tag":"t1"}},{"id":"2","pid":"p2","price":{"price":200},"category":{"mainProduct":{"id":"2"},"id":"c2","tag":"t2"}}],"selected":{"id":"2"},"first":{"id":"1"}}}}`, res.Body)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@router-tests/protocol/multihop_compound_key_test.go` at line 67, The response assertion in multihop_compound_key_test.go is using a raw string equality check for the router payload, which is too brittle for harmless JSON formatting or key-order changes. Update the test in the relevant assertion near the topProducts response to use semantic JSON comparison instead of direct byte/string equality, keeping the check focused on the content of res.Body and the expected JSON structure.composition/tests/v1/resolvability.test.ts (1)
718-751: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid rebuilding expected errors with the same production helpers.
unresolvablePathError(generateResolvabilityErrorReasons(...))couples the expectation to the exact message-building code used by the failure path, so helper regressions can still pass this test. Prefer asserting on explicit message fragments or literals for the four unresolvable paths instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composition/tests/v1/resolvability.test.ts` around lines 718 - 751, The expectation in resolvability.test.ts is too tightly coupled to the production error builders because it uses unresolvablePathError with generateResolvabilityErrorReasons for each of firstFieldData, selectedFieldData, pidFieldData, and categoryFieldData. Replace those helper-based expected values with explicit assertions on the emitted error messages or stable literal fragments so the test validates the unresolvable paths without reusing the same message-generation logic from the implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@adr/abstract-empty-selection-set.md`:
- Around line 113-116: The fenced markdown block in
abstract-empty-selection-set.md is missing a language identifier, which causes
markdownlint to flag it. Update the fence around the printOperation planner
output to use a text language tag so the block is explicitly labeled; keep the
existing content unchanged and ensure the fenced section in the ADR is
consistently marked for plain text output.
In `@adr/provides-on-union-fieldsets.md`:
- Around line 237-239: The ADR is inconsistent about planner impact: this
section says there is no runtime or planner change, but earlier parts describe
planner-side datasource-selection changes and Celestial validation. Update the
wording in the affected ADR section to match the rest of the document by
accurately describing the planner impact and the purpose of the celestial
snapshot run, using the surrounding planner/datasource-selection terminology to
keep the regression surface consistent.
In `@composition/src/resolvability-graph/graph.ts`:
- Around line 218-223: The sibling availability check in graph.ts is incorrectly
treating merged key fields as already gathered output, which can make an
`@external` key member look resolvable before the sibling can actually provide it.
Update the logic around selectionTreeContains in the
availableChildTree/requiredChildTree path so merged keyTree entries from
siblingAvailableTree are not counted as gathered output for satisfiability;
ensure the resolvability decision in the keyTree merge section and the
child-field check stays tied to real gatherable output from the sibling, not
just merged key members.
In `@composition/tests/v1/directives/provides.test.ts`:
- Around line 849-860: The current test expectation is still tied to the old
`incompatibleTypeWithProvidesErrorMessage` wording for `enumProvides`, which now
risks locking in outdated “not an Object nor Interface” text. Update the
`provides.test.ts` assertion together with the shared helper used by
`invalidProvidesOrRequiresDirectivesError`/`incompatibleTypeWithProvidesErrorMessage`
so the expected compose error reflects the new accepted `@provides` parent types
and no longer hardcodes the legacy message. Keep the test focused on the invalid
enum response case while matching the revised error output from the
`normalizeSubgraphFailure` path.
In `@docs/planner-improvements.md`:
- Line 6: The markdown link for federation-gateway-audit is malformed because
the URL contains spaces, so fix the link target in the planner improvements doc
so it renders as a valid clickable URL. Update the reference where the
federation-gateway-audit link is defined, keeping the link text the same but
replacing the broken URL with the correct no-space repository URL.
In `@router-tests/protocol/provides_interface_test.go`:
- Around line 35-43: The provider query assertions in readGraphQLRequest should
also verify the requested id selections, not just media, animals, and name.
Update the provides_interface_test.go checks around req.Query to assert that the
upstream query includes the id fields for the provided entities so the test
fails if the planner stops selecting them.
In `@router-tests/protocol/provides_union_test.go`:
- Around line 25-36: The test in the middleware handler for provides_union_test
only verifies that _entities is absent, so it can still pass even if the
provider query never requests title. Update the handler to inspect the decoded
core.GraphQLRequest and add a positive assertion that the upstream selection
actually includes title before returning the mocked response, using the existing
Middleware closure, core.GraphQLRequest, and req.Query checks to locate the
change.
In `@router-tests/protocol/requires_argument_conflict_test.go`:
- Around line 24-37: The test stub for the Products middleware only checks that
the request does not contain _entities, so it can miss regressions in the
planned catalog fetch shape. Update the middleware in
requires_argument_conflict_test.go to assert the outgoing GraphQLRequest.Query
includes the argumented selections expected from the planner, or derive the
mocked response from req.Query so the response reflects what was actually
requested. Use the existing Products middleware and core.GraphQLRequest handling
to locate the fix.
In `@router-tests/protocol/split_union_test.go`:
- Around line 52-55: The empty-selection regression assertion in the split union
test only removes literal spaces, so other whitespace forms can hide the same
broken query shape. Update the check in the `split_union_test.go` assertion
around `req.Query` to normalize all whitespace before `require.NotContains`, or
switch to a regex-based match, so `items { __typename }` variants with
newlines/tabs are also rejected.
---
Nitpick comments:
In `@composition/tests/v1/resolvability.test.ts`:
- Around line 718-751: The expectation in resolvability.test.ts is too tightly
coupled to the production error builders because it uses unresolvablePathError
with generateResolvabilityErrorReasons for each of firstFieldData,
selectedFieldData, pidFieldData, and categoryFieldData. Replace those
helper-based expected values with explicit assertions on the emitted error
messages or stable literal fragments so the test validates the unresolvable
paths without reusing the same message-generation logic from the implementation.
In `@router-tests/protocol/multihop_compound_key_test.go`:
- Line 67: The response assertion in multihop_compound_key_test.go is using a
raw string equality check for the router payload, which is too brittle for
harmless JSON formatting or key-order changes. Update the test in the relevant
assertion near the topProducts response to use semantic JSON comparison instead
of direct byte/string equality, keeping the check focused on the content of
res.Body and the expected JSON structure.
In `@router-tests/protocol/testdata/multihop_compound_key/list.graphqls`:
- Around line 13-15: Make the compound-key fixture consistent by changing the
Product type so pid is required, since it is part of the `@key`(fields: "id pid")
identity in this multihop_compound_key test data. Update the Product definition
in list.graphqls to match the composition-side fixture and ensure pid is
non-null alongside id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 075ea6af-6297-4397-bd42-5fd1a10cd24f
⛔ Files ignored due to path filters (1)
router/go.sumis excluded by!**/*.sum
📒 Files selected for processing (25)
adr/abstract-empty-selection-set.mdadr/multi-hop-compound-key-resolvability.mdadr/provides-on-interface-fieldsets.mdadr/provides-on-union-fieldsets.mdadr/requires-field-argument-conflict.mdcomposition/src/resolvability-graph/graph.tscomposition/src/v1/federation/federation-factory.tscomposition/src/v1/normalization/normalization-factory.tscomposition/src/v1/normalization/types/types.tscomposition/tests/v1/directives/provides.test.tscomposition/tests/v1/resolvability.test.tsdocs/planner-improvements.mdrouter-tests/protocol/multihop_compound_key_test.gorouter-tests/protocol/provides_interface_test.gorouter-tests/protocol/provides_union_test.gorouter-tests/protocol/requires_argument_conflict_test.gorouter-tests/protocol/split_union_test.gorouter-tests/protocol/testdata/multihop_compound_key/compose.yamlrouter-tests/protocol/testdata/multihop_compound_key/link.graphqlsrouter-tests/protocol/testdata/multihop_compound_key/list.graphqlsrouter-tests/protocol/testdata/multihop_compound_key/price.graphqlsrouter-tests/protocol/testdata/multihop_compound_key/products.graphqlsrouter-tests/protocol/testdata/multihop_compound_key_config.jsonrouter-tests/protocol/testdata/provides_interface_config.jsonrouter/go.mod
Five Architecture Decision Records describing federation composition and query-planning gaps and the chosen fix approach: @provides over union-typed fields, @provides over interface-typed fields, multi-hop compound @key resolvability, abstract-typed field over partial union membership, and @requires field sets with conflicting field arguments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ce-typed fields Relax the field-set parent gate so a union return type is an accepted @provides parent, and resolve @external at the implementation level for an interface-typed @provides leaf (accepting only what an alternative composer accepts; a leaf not @external on every concrete implementation is still rejected). Pairs with the graphql-go-tools planner changes that honor these @provides at runtime. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ather Model key satisfaction as a recursive field-set selection-tree check and synthesize a multi-hop entity edge when a missing compound-@key member is gatherable through a single nested entity hop. Pairs with the graphql-go-tools planner fix (subset-to-compound-key jump, gated as a strict fallback). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Router e2e tests asserting the full response body and the exact subgraph request payloads (queries and entity representations) for each fix: @provides over a union-typed field, an abstract field whose union members are split across datasources (HTTP 200, was 500), @requires with conflicting field arguments (both correctly-argumented values), @provides over an interface-typed field, and a multi-hop compound @key (the multi-stage entity fetch). Requires the graphql-go-tools planner changes, pinned in go.mod. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ter-tests) Pin github.com/wundergraph/graphql-go-tools/v2 in both the router and router-tests modules to the planner-improvements commit carrying the five federation query-planner fixes (wundergraph/graphql-go-tools#1551), and tidy go.sum. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jensneuse
force-pushed
the
planner-improvements
branch
from
8d29088 to
88ab6b2
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
adr/abstract-empty-selection-set.md (1)
113-116: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language tag to this fenced block.
The planner output fence is still untyped here, so markdownlint MD040 will keep flagging the ADR. Use
textfor this block.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adr/abstract-empty-selection-set.md` around lines 113 - 116, The fenced planner output block is still missing a language tag, which keeps markdownlint MD040 failing. Update the fenced block in the ADR so the output is explicitly typed as text, keeping the content unchanged; this is the same fenced section containing the planner output string and should be easy to locate by the “printOperation planner id” text.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@adr/abstract-empty-selection-set.md`:
- Around line 113-116: The fenced planner output block is still missing a
language tag, which keeps markdownlint MD040 failing. Update the fenced block in
the ADR so the output is explicitly typed as text, keeping the content
unchanged; this is the same fenced section containing the planner output string
and should be easy to locate by the “printOperation planner id” text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac7d5b01-ee38-416e-8e14-dcf280d5c26f
⛔ Files ignored due to path filters (2)
router-tests/go.sumis excluded by!**/*.sumrouter/go.sumis excluded by!**/*.sum
📒 Files selected for processing (26)
adr/abstract-empty-selection-set.mdadr/multi-hop-compound-key-resolvability.mdadr/provides-on-interface-fieldsets.mdadr/provides-on-union-fieldsets.mdadr/requires-field-argument-conflict.mdcomposition/src/resolvability-graph/graph.tscomposition/src/v1/federation/federation-factory.tscomposition/src/v1/normalization/normalization-factory.tscomposition/src/v1/normalization/types/types.tscomposition/tests/v1/directives/provides.test.tscomposition/tests/v1/resolvability.test.tsdocs/planner-improvements.mdrouter-tests/go.modrouter-tests/protocol/multihop_compound_key_test.gorouter-tests/protocol/provides_interface_test.gorouter-tests/protocol/provides_union_test.gorouter-tests/protocol/requires_argument_conflict_test.gorouter-tests/protocol/split_union_test.gorouter-tests/protocol/testdata/multihop_compound_key/compose.yamlrouter-tests/protocol/testdata/multihop_compound_key/link.graphqlsrouter-tests/protocol/testdata/multihop_compound_key/list.graphqlsrouter-tests/protocol/testdata/multihop_compound_key/price.graphqlsrouter-tests/protocol/testdata/multihop_compound_key/products.graphqlsrouter-tests/protocol/testdata/multihop_compound_key_config.jsonrouter-tests/protocol/testdata/provides_interface_config.jsonrouter/go.mod
✅ Files skipped from review due to trivial changes (7)
- router-tests/protocol/testdata/multihop_compound_key/compose.yaml
- router-tests/protocol/testdata/multihop_compound_key/list.graphqls
- router-tests/protocol/testdata/multihop_compound_key/link.graphqls
- adr/requires-field-argument-conflict.md
- docs/planner-improvements.md
- adr/provides-on-union-fieldsets.md
- adr/provides-on-interface-fieldsets.md
🚧 Files skipped from review as they are similar to previous changes (12)
- router-tests/go.mod
- composition/src/v1/federation/federation-factory.ts
- router-tests/protocol/testdata/multihop_compound_key/products.graphqls
- router/go.mod
- router-tests/protocol/provides_interface_test.go
- composition/src/v1/normalization/types/types.ts
- composition/tests/v1/directives/provides.test.ts
- router-tests/protocol/testdata/multihop_compound_key/price.graphqls
- composition/tests/v1/resolvability.test.ts
- router-tests/protocol/multihop_compound_key_test.go
- composition/src/v1/normalization/normalization-factory.ts
- composition/src/resolvability-graph/graph.ts
- Include Union in the incompatible-@provides-response-type compose error so it reflects that union parents are now accepted (the assertion builds via the helper). - Add a negative regression test proving a compound @key whose missing member is @external on a sibling stays unresolvable (guards the multi-hop resolvability path against a false-positive). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add a language tag to a fenced block (abstract-empty-selection-set ADR). - Correct the provides-on-union ADR consequences to state the planner change is strictly gated (byte-identical for non-union-@provides graphs) rather than absent. - De-link the malformed audit reference in the reviewer guide. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ixes Bump github.com/wundergraph/graphql-go-tools/v2 (router + router-tests) to the planner-improvements commit that adds the four review-feedback planner fixes (variable-definition import on raw arg copy, interface-fragment @provides lookup, union-member merge, ignore orphaned provided suggestions). Celestial: 0 plan diffs across 220 graphs / 13,143 operations; the five router e2e pass against the new pin. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(B1 replacement) The B1 split-union fix is replaced (in graphql-go-tools) by source-subgraph / intersection semantics: a union field is resolved from the subgraph(s) that own the current path; foreign value-type members are absent (not gathered), and the resolving subgraph's own non-shared members are response-only nulls. This corrects the earlier gather approach, which returned the foreign subgraph's members (silently-wrong data). - e2e: rename TestPartiallySplitUnionOnShareableField -> TestPartialUnionIntersectionOnShareableField; assert one upstream fetch, zero sibling/products requests, and the intersection body. - ADR abstract-empty-selection-set + reviewer guide updated to the intersection semantics and the partial-union-complex 5/5 verification. Co-Authored-By: Ahmet Soormally <ahmet@mangomm.co.uk> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d-type-mismatch Pin github.com/wundergraph/graphql-go-tools/v2 (router + router-tests) to the planner-improvements commit that replaces the B1 split-union gather with source-subgraph intersection and adds the child-type-mismatch field-aliasing fix. Validated: audit partial-union-complex 5/5, child-type-mismatch 4/4; celestial 0-diff for the intersection change (the child-type-mismatch wire-format aliasing re-baselines two goldens). Co-Authored-By: Claude Opus 4.8 (1M context) <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
1 participant
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.
Federation correctness and parity fixes spanning the Cosmo composition library and the graphql-go-tools query planner. Pairs with wundergraph/graphql-go-tools#1551 (pinned in router/go.mod).
Federation planner-improvements — reviewer guide
Five federation correctness and parity fixes, each spanning the Cosmo
composition library (TypeScript) and/or the
graphql-go-toolsquery planner (Go).Every fix is grounded in a decision record under
adr/, validated against thepublic [federation-gateway-audit](https://github.com/the federation-gateway-audit project/federation-gateway-audit),
and gated for zero plan-snapshot regression.
This branch (
planner-improvements) pairs with theplanner-improvementsbranch ingraphql-go-tools. During development the two are linked with a localgo.work; thefinal commit pins the published
graphql-go-toolscommit inrouter/go.mod.What each fix does
@providesover a union-typed field was rejected at compose time; even composed, the planner did not honor it (it entity-fetched the owner instead of reading the provided value inline).@providesover an interface-typed field with implementation-level@externalwas rejected bywgc(the alternative implementation accepts it), and the planner did not honor it after abstract→concrete rewriting.@keymember from a third subgraph was declared unresolvable; the planner could not build the two-stage (gather → compound-key) entity fetch.@requiresconsumers requiring the same field with different arguments collapsed to one value → silently wrong data.Where the code lives (navigation)
Composition changes are in
composition/; planner changes are ingraphql-go-tools(
v2/pkg/engine/...). Each cluster has unit tests next to the code and an end-to-endtest under
router-tests/protocol/.A1 — provides-on-union
composition/src/v1/normalization/normalization-factory.ts(relax thefield-set parent gate to admit a union parent) +
types/types.ts.v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go(
isProvidedFieldfalls back to the fragment-stripped provides key, gated to unioninline fragments).
router-tests/protocol/provides_union_test.go.A2 — provides-on-interface
normalization-factory.ts(
#getImplementationExternalFieldDataForProvidesInterfaceLeaf— resolve@externalat the implementation level; accepts only what the alternative implementation accepts).
datasource_filter_collect_nodes_visitor.go,datasource_filter_visitor.go,path_builder.go(match interface-keyed provides against rewritten concrete fragmentpaths; the
isOnAbstractFragmentmechanism generalizes A1's).router-tests/protocol/provides_interface_test.go.A3 — multi-hop compound
@keycomposition/src/resolvability-graph/graph.ts(selection-tree multi-hopresolvability) +
composition/src/v1/federation/federation-factory.ts.source_connection_graph.go,node_selection_visitor.go,node_selection_builder.go,datasource_filter_node_suggestions.go,datasource_filter_visitor.go,datasource_filter_resolvable_visitor.go(subset-source-key → compound-target-key jump, gated as a strict fallback).
router-tests/protocol/multihop_compound_key_test.go.B1 — abstract empty-selection-set
datasource_filter_visitor.go(closest-datasource pass for split unionfields) +
abstract_selection_rewriter.go+node_selection_builder.go+node_selection_visitor.go.router-tests/protocol/split_union_test.go.B2 — @requires argument conflict
graphql_datasource/representation_variable.go&graphql_datasource.go(argument-fingerprint conflict detection + fetch split),plan/federation_metadata.go,plan/path_builder_visitor.go.router-tests/protocol/requires_argument_conflict_test.go.The load-bearing safety property
Every planner change is strictly gated so it can only affect the specific broken
shape; any plan that already succeeds is byte-identical. This is enforced by the
celestial plan-snapshot sweep: each cluster's planner change produced 0 diffs
across 220 real federated graphs / 13,143 operations versus the prior state. (For A3
this gate is essential — the un-gated synthesis drifts ~114 operations; gating it as a
strict fallback brings it to zero.)
Verification summary
to passing —
provides-on-union(2/2),provides-on-interface(2/2),complex-entity-call(1/1),requires-with-argument-conflict(1/1) — each verifiedby a clean single-suite run against the router built from all five fixes.
(Note: the audit's all-suites bash runner has a router-startup-timing flake that
yields false
fetch failedresults when suites are cycled rapidly; it is not used asevidence — every suite that flakes there passes in a clean single run, e.g.
abstract-types,enum-intersection, andcomplex-entity-callitself.)produced 0 plan diffs across 220 graphs / 13,143 operations, so the planner emits
byte-identical plans to stock and no other audit suite's runtime behavior can change.
Composition changes are additive / parity with the alternative composer and guarded by negative tests.
(1189 tests / 76 files; private tests run from a gitignored copy, never committed).
graphql-go-toolsplan+graphql_datasourcepackages: green.router-tests/protocol): all five new tests pass (combined router).accepts A1/A2/A3; B1 HTTP 500 → 200; B2 now returns spec-correct values matching the
spec-correct alternative implementation, where both stock Cosmo and the other alternative return wrong data).
Notes for reviewers
Among alternative implementations only one gets it right; with this fix Cosmo matches the spec-correct behavior
and is more correct than the other alternative (which collapses to the other argument).
stricter than the alternative implementation's here. The composition relaxation accepts only what the alternative implementation
accepts (resolve interface-
@provides@externalat the implementation level); agenuinely-invalid
@provides(no implementation marks the leaf@external) is stillrejected — covered by a negative test.
🤖 Generated with Claude Code
Summary by CodeRabbit
@provideson union/interface field sets, multi-hop compound keys,@requiresargument conflicts, and partially split unions.@providesand@requiresresolution behaviors across subgraphs, including union/interface cases and fetch correctness under conflicting arguments.@provides.@provides(union/interface),@requiresargument conflicts, and partially split unions.