From 5cae10bfa6b3f9000976b47766e760c5d9b914e8 Mon Sep 17 00:00:00 2001 From: Adam Benhassen Date: Fri, 10 Apr 2026 19:02:36 +0200 Subject: [PATCH 1/5] fix: preserve composed directives with executable locations in supergraph --- __tests__/supergraph/base.spec.ts | 146 ++++++++++++++++++++++++++++++ src/supergraph/state.ts | 6 +- 2 files changed, 151 insertions(+), 1 deletion(-) diff --git a/__tests__/supergraph/base.spec.ts b/__tests__/supergraph/base.spec.ts index a0f3f455..1b5d41b9 100644 --- a/__tests__/supergraph/base.spec.ts +++ b/__tests__/supergraph/base.spec.ts @@ -247,6 +247,38 @@ testVersions((api, version) => { expect(result.supergraphSdl).not.toContain("directive @a(n: Int)"); }); + test("non-composed executable directive with QUERY | MUTATION is omitted if only defined within a single subgraph", () => { + const result = api.composeServices([ + { + name: "a", + url: "http://a.com", + typeDefs: graphql` + extend schema + @link(url: "https://specs.apollo.dev/federation/${version}", import: ["@key"]) + + directive @a(n: Int) on QUERY | MUTATION + + type Query { + a: Int + } + `, + }, + { + name: "b", + url: "http://b.com", + typeDefs: graphql` + extend schema + @link(url: "https://specs.apollo.dev/federation/${version}", import: ["@key"]) + + type Query { + b: Int + } + `, + }, + ]); + expect(result.supergraphSdl).not.toContain("directive @a"); + }); + test("executable directive only contains locations shared between all subgraphs", () => { const result = api.composeServices([ { @@ -450,5 +482,119 @@ testVersions((api, version) => { } `); }); + + test("composed directive with only executable locations is preserved in supergraph", () => { + const result = api.composeServices([ + { + name: "a", + url: "http://a.com", + typeDefs: graphql` + extend schema + @link(url: "https://specs.apollo.dev/federation/${version}", import: ["@composeDirective"]) + @link(url: "https://a.dev/a/v1.0", import: ["@a"]) + @composeDirective(name: "@a") + + directive @a(name: String!) on QUERY | MUTATION + + type Query { + a: Int + } + `, + }, + { + name: "b", + url: "http://b.com", + typeDefs: graphql` + extend schema + @link(url: "https://specs.apollo.dev/federation/${version}", import: []) + + type Query { + b: Int + } + `, + }, + ]); + + assertCompositionSuccess(result); + expect(result.supergraphSdl).toContainGraphQL(graphql` + directive @a(name: String!) on QUERY | MUTATION + `); + }); + + test("composed directive with VARIABLE_DEFINITION and FIELD locations is preserved in supergraph", () => { + const result = api.composeServices([ + { + name: "a", + url: "http://a.com", + typeDefs: graphql` + extend schema + @link(url: "https://specs.apollo.dev/federation/${version}", import: ["@composeDirective"]) + @link(url: "https://a.dev/a/v1.0", import: ["@a"]) + @composeDirective(name: "@a") + + directive @a(provider: String!) on VARIABLE_DEFINITION | FIELD + + type Query { + a: Int + } + `, + }, + { + name: "b", + url: "http://b.com", + typeDefs: graphql` + extend schema + @link(url: "https://specs.apollo.dev/federation/${version}", import: []) + + type Query { + b: Int + } + `, + }, + ]); + + assertCompositionSuccess(result); + expect(result.supergraphSdl).toContainGraphQL(graphql` + directive @a(provider: String!) on VARIABLE_DEFINITION | FIELD + `); + }); + + test("composed directive with mixed schema and executable locations is preserved when only one subgraph defines it", () => { + const result = api.composeServices([ + { + name: "a", + url: "http://a.com", + typeDefs: graphql` + extend schema + @link(url: "https://specs.apollo.dev/federation/${version}", import: ["@composeDirective"]) + @link(url: "https://a.dev/a/v1.0", import: ["@a"]) + @composeDirective(name: "@a") + + directive @a(n: Int) on FIELD | FIELD_DEFINITION + + type Query { + a: Int + } + `, + }, + { + name: "b", + url: "http://b.com", + typeDefs: graphql` + extend schema + @link(url: "https://specs.apollo.dev/federation/${version}", import: []) + + type Query { + b: Int + } + `, + }, + ]); + + assertCompositionSuccess(result); + expect(result.supergraphSdl).toContainGraphQL(graphql` + directive @a(n: Int) on FIELD | FIELD_DEFINITION + `); + }); } }); diff --git a/src/supergraph/state.ts b/src/supergraph/state.ts index 133d5f7a..a6465060 100644 --- a/src/supergraph/state.ts +++ b/src/supergraph/state.ts @@ -303,7 +303,11 @@ export function createSupergraphStateBuilder() { // Strip out all executable directives that are not defined or identical every supergraph for (const directiveState of state.directives.values()) { - if (!directiveState.isExecutable || !directiveState.byGraph.size) { + if ( + !directiveState.isExecutable || + !directiveState.byGraph.size || + directiveState.composed + ) { continue; } From 928404574d0c58993619206ada5f30c11c5f0f6d Mon Sep 17 00:00:00 2001 From: Adam Benhassen Date: Mon, 13 Apr 2026 19:34:41 +0200 Subject: [PATCH 2/5] remove dead path --- src/supergraph/state.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/supergraph/state.ts b/src/supergraph/state.ts index a6465060..9c4de8d4 100644 --- a/src/supergraph/state.ts +++ b/src/supergraph/state.ts @@ -301,7 +301,8 @@ export function createSupergraphStateBuilder() { build() { const transformFields = createFieldsTransformer(state); - // Strip out all executable directives that are not defined or identical every supergraph + // Strip non-composed executable directives that are not defined identically in every subgraph. + // Composed directives are preserved unconditionally, they were explicitly requested via @composeDirective. for (const directiveState of state.directives.values()) { if ( !directiveState.isExecutable || @@ -329,11 +330,6 @@ export function createSupergraphStateBuilder() { directiveState.locations.forEach((location) => { // if it is not an executable location -> remove if (!isExecutableDirectiveLocation(location)) { - // If it is a compose directive we want to retain the schema location. - if (directiveState.composed) { - return; - } - directiveState.locations.delete(location); return; } From c1d89fe0a0e185a758af36e4310fce546082c4b2 Mon Sep 17 00:00:00 2001 From: Adam Benhassen Date: Mon, 13 Apr 2026 19:40:55 +0200 Subject: [PATCH 3/5] track behavior --- __tests__/supergraph/base.spec.ts | 154 ++++++++++++++++++++++++++++++ 1 file changed, 154 insertions(+) diff --git a/__tests__/supergraph/base.spec.ts b/__tests__/supergraph/base.spec.ts index 1b5d41b9..c64486cf 100644 --- a/__tests__/supergraph/base.spec.ts +++ b/__tests__/supergraph/base.spec.ts @@ -1,5 +1,6 @@ import { expect, test } from "vitest"; import { + assertCompositionFailure, assertCompositionSuccess, graphql, testVersions, @@ -596,5 +597,158 @@ testVersions((api, version) => { directive @a(n: Int) on FIELD | FIELD_DEFINITION `); }); + + test("composed directive with only executable locations is preserved when both subgraphs define it", () => { + const result = api.composeServices([ + { + name: "a", + url: "http://a.com", + typeDefs: graphql` + extend schema + @link(url: "https://specs.apollo.dev/federation/${version}", import: ["@composeDirective"]) + @link(url: "https://a.dev/a/v1.0", import: ["@a"]) + @composeDirective(name: "@a") + + directive @a(name: String!) on QUERY | MUTATION + + type Query { + a: Int + } + `, + }, + { + name: "b", + url: "http://b.com", + typeDefs: graphql` + extend schema + @link(url: "https://specs.apollo.dev/federation/${version}", import: ["@composeDirective"]) + @link(url: "https://a.dev/a/v1.0", import: ["@a"]) + @composeDirective(name: "@a") + + directive @a(name: String!) on QUERY | MUTATION + + type Query { + b: Int + } + `, + }, + ]); + + assertCompositionSuccess(result); + expect(result.supergraphSdl).toContainGraphQL(graphql` + directive @a(name: String!) on QUERY | MUTATION + `); + }); + + // Apollo silently picks a winning definition by reverse-alphabetical service name + // (i.e., "b" wins over "a") when composed directive definitions conflict across subgraphs. + // Guild raises FIELD_ARGUMENT_TYPE_MISMATCH, which is stricter and arguably more correct. + test("conflicting composed directive definitions across subgraphs", () => { + const result = api.composeServices([ + { + name: "a", + url: "http://a.com", + typeDefs: graphql` + extend schema + @link(url: "https://specs.apollo.dev/federation/${version}", import: ["@composeDirective"]) + @link(url: "https://a.dev/a/v1.0", import: ["@a"]) + @composeDirective(name: "@a") + + directive @a(name: String!) on QUERY | MUTATION + + type Query { + a: Int + } + `, + }, + { + name: "b", + url: "http://b.com", + typeDefs: graphql` + extend schema + @link(url: "https://specs.apollo.dev/federation/${version}", import: ["@composeDirective"]) + @link(url: "https://a.dev/a/v1.0", import: ["@a"]) + @composeDirective(name: "@a") + + directive @a(name: ID!) on QUERY | MUTATION + + type Query { + b: Int + } + `, + }, + ]); + + if (api.library === "apollo") { + // Apollo silently resolves the conflict by picking one definition + // by reverse-alphabetical service name. Service "b" wins over "a", + // so the ID! type from subgraph "b" is used. + assertCompositionSuccess(result); + expect(result.supergraphSdl).toContainGraphQL(graphql` + directive @a(name: ID!) on QUERY | MUTATION + `); + } else { + // Guild correctly rejects conflicting argument types + assertCompositionFailure(result); + expect(result.errors?.[0]?.message).toContain( + 'Type of argument "@a(name:)" is incompatible across subgraphs', + ); + } + }); + + // When the type definitions are swapped between services, Apollo picks a + // different winner, confirming the resolution is based on service name sort + // order, not schema correctness. + test("conflicting composed directive definitions; swapped types changes Apollo winner", () => { + const result = api.composeServices([ + { + name: "a", + url: "http://a.com", + typeDefs: graphql` + extend schema + @link(url: "https://specs.apollo.dev/federation/${version}", import: ["@composeDirective"]) + @link(url: "https://a.dev/a/v1.0", import: ["@a"]) + @composeDirective(name: "@a") + + directive @a(name: ID!) on QUERY | MUTATION + + type Query { + a: Int + } + `, + }, + { + name: "b", + url: "http://b.com", + typeDefs: graphql` + extend schema + @link(url: "https://specs.apollo.dev/federation/${version}", import: ["@composeDirective"]) + @link(url: "https://a.dev/a/v1.0", import: ["@a"]) + @composeDirective(name: "@a") + + directive @a(name: String!) on QUERY | MUTATION + + type Query { + b: Int + } + `, + }, + ]); + + if (api.library === "apollo") { + // Now service "b" has String! instead of ID!, and Apollo picks "b" again; + // so this time String! wins, proving the winner is name-sorted, not type-based. + assertCompositionSuccess(result); + expect(result.supergraphSdl).toContainGraphQL(graphql` + directive @a(name: String!) on QUERY | MUTATION + `); + } else { + // Guild rejects regardless of which service has which type + assertCompositionFailure(result); + expect(result.errors?.[0]?.message).toContain( + 'Type of argument "@a(name:)" is incompatible across subgraphs', + ); + } + }); } }); From 9d31b9fa52465c8cd134f79f3a606b3205065c86 Mon Sep 17 00:00:00 2001 From: Adam Benhassen Date: Mon, 13 Apr 2026 19:41:11 +0200 Subject: [PATCH 4/5] document behavior --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index cf22f195..e2d49eea 100644 --- a/README.md +++ b/README.md @@ -121,6 +121,12 @@ will continue to do so as we learn more about the Federation specification. Your feedback and bug reports are welcome and appreciated. +#### Known Behavioral Differences + +| Scenario | Apollo | Guild | +| --- | --- | --- | +| `@composeDirective` with conflicting argument types across subgraphs (e.g. `String!` vs `ID!`) | Silently picks one definition by reverse-alphabetical service name | Raises `FIELD_ARGUMENT_TYPE_MISMATCH` error | + ## Supergraph SDL Composition ✅ Done From 47a5f9a978af84306748fc0fc8cef3952339b995 Mon Sep 17 00:00:00 2001 From: Adam Benhassen Date: Mon, 13 Apr 2026 19:53:35 +0200 Subject: [PATCH 5/5] add changeset --- .changeset/fix-composed-executable-directives.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/fix-composed-executable-directives.md diff --git a/.changeset/fix-composed-executable-directives.md b/.changeset/fix-composed-executable-directives.md new file mode 100644 index 00000000..980603c6 --- /dev/null +++ b/.changeset/fix-composed-executable-directives.md @@ -0,0 +1,7 @@ +--- +"@theguild/federation-composition": patch +--- + +fix: preserve composed directives with executable locations in supergraph + +`@composeDirective` was stripping directive definitions that only had executable locations (`QUERY`, `MUTATION`, `FIELD`, `VARIABLE_DEFINITION`) from the supergraph. Directives with schema-level locations (`OBJECT`, `FIELD_DEFINITION`) were unaffected. This fix skips the executable directive stripping logic for composed directives entirely, since they were explicitly requested via `@composeDirective`.