fix: preserve composed directives with executable locations in supergraph

.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`.

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

__tests__/supergraph/base.spec.ts

@@ -1,5 +1,6 @@
 import { expect, test } from "vitest";
 import {
+  assertCompositionFailure,
   assertCompositionSuccess,
   graphql,
   testVersions,
@@ -247,6 +248,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 +483,272 @@ 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
+      `);
+    });
+
+    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',
+        );
+      }
+    });
   }
 });

src/supergraph/state.ts

@@ -301,9 +301,14 @@ 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 || !directiveState.byGraph.size) {
+        if (
+          !directiveState.isExecutable ||
+          !directiveState.byGraph.size ||
+          directiveState.composed
+        ) {
           continue;
         }
 
@@ -325,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;
           }