fix: avoid duplicate errors for external fields

.changeset/spotty-ducks-repeat.md

@@ -0,0 +1,8 @@
+---
+"@theguild/federation-composition": patch
+---
+
+Stop reporting a duplicate `FIELD_TYPE_MISMATCH` next to `EXTERNAL_TYPE_MISMATCH`. An `@external`
+declaration mirrors a field it does not own, so it no longer takes part in the field type merge
+check - `ExternalTypeMismatchRule` already compares it against the merged type. Key fields are
+excepted, since that rule skips them.

__tests__/supergraph/errors/EXTERNAL_TYPE_MISMATCH.spec.ts

@@ -237,7 +237,8 @@ testVersions((api, version) => {
       ]),
     ).toEqual(
       expect.objectContaining({
-        errors: expect.arrayContaining([
+        // exactly one error: the @external declaration does not also produce a FIELD_TYPE_MISMATCH
+        errors: [
           expect.objectContaining({
             message: expect.stringContaining(
               `Type of field "Product.variant" is incompatible across subgraphs (where marked @external): it has type "VariantV1!" in subgraph "products" but type "VariantV2" in subgraph "pricing"`,
@@ -246,7 +247,7 @@ testVersions((api, version) => {
               code: "EXTERNAL_TYPE_MISMATCH",
             }),
           }),
-        ]),
+        ],
       }),
     );
   });

src/supergraph/composition/object-type.ts

@@ -62,6 +62,20 @@ export function isFederationExtension(
   return false;
 }
 
+/**
+ * A field is only truly @external in Federation v1 when its type is an extension,
+ * as v1 subgraphs mark every field of an extended type as @external.
+ */
+export function isExternalInGraph(
+  external: boolean,
+  meta: ObjectTypeStateInGraph,
+  version: FederationVersion,
+) {
+  return version === "v1.0"
+    ? external && isFederationExtension(meta, version)
+    : external;
+}
+
 export function objectTypeBuilder(): TypeBuilder<ObjectType, ObjectTypeState> {
   let requiresUsageIndex: RequiresFieldUsageIndex | null = null;
 
@@ -172,11 +186,11 @@ export function objectTypeBuilder(): TypeBuilder<ObjectType, ObjectTypeState> {
         }
 
         // It's the first time we visited a non-external field, we should force the type on that field to match the local type
-        const isExternal =
-          graph.version === "v1.0"
-            ? field.external &&
-              isFederationExtension(typeInGraph, graph.version)
-            : field.external;
+        const isExternal = isExternalInGraph(
+          field.external,
+          typeInGraph,
+          graph.version,
+        );
         const shouldForceType =
           // If it's not an external field and it's first time we visited a non-external field,
           // we should force the type but only if it's not used as a key

src/supergraph/validation/rules/external-type-mismatch-rule.ts

@@ -1,6 +1,6 @@
 import { GraphQLError } from "graphql";
 import { andList } from "../../../utils/format.js";
-import { isFederationExtension } from "../../composition/object-type.js";
+import { isExternalInGraph } from "../../composition/object-type.js";
 import type { SupergraphVisitorMap } from "../../composition/visitor.js";
 import type { SupergraphValidationContext } from "../validation-context.js";
 
@@ -18,16 +18,11 @@ export function ExternalTypeMismatchRule(
       const graphsWithEqualType: string[] = [];
 
       for (const [graphId, field] of fieldState.byGraph) {
-        const graphVersion =
-          context.subgraphStates.get(graphId)!.federation.version;
-        const isExternal =
-          graphVersion === "v1.0"
-            ? field.external &&
-              isFederationExtension(
-                objectTypeState.byGraph.get(graphId)!,
-                graphVersion,
-              )
-            : field.external;
+        const isExternal = isExternalInGraph(
+          field.external,
+          objectTypeState.byGraph.get(graphId)!,
+          context.subgraphStates.get(graphId)!.federation.version,
+        );
         if (!isExternal) {
           graphsWithEqualType.push(graphId);
 

src/supergraph/validation/rules/fields-of-the-same-type-rule.ts

@@ -1,5 +1,6 @@
 import { GraphQLError } from "graphql";
 import { TypeKind } from "../../../subgraph/state.js";
+import { isExternalInGraph } from "../../composition/object-type.js";
 import { SupergraphVisitorMap } from "../../composition/visitor.js";
 import { SupergraphValidationContext } from "../validation-context.js";
 
@@ -80,6 +81,20 @@ export function FieldsOfTheSameTypeRule(
       const typeNameToPossibleTypeNames = new Map<string, Set<string>>();
 
       fieldState.byGraph.forEach((field, graphName) => {
+        // An @external declaration only mirrors the field it points at, it does not take part in
+        // merging its type. ExternalTypeMismatchRule compares those against the merged type and
+        // reports EXTERNAL_TYPE_MISMATCH - except for key fields, which it skips, so we keep those.
+        if (
+          !fieldState.usedAsKey &&
+          isExternalInGraph(
+            field.external,
+            objectTypeState.byGraph.get(graphName)!,
+            context.subgraphStates.get(graphName)!.federation.version,
+          )
+        ) {
+          return;
+        }
+
         const typeName = field.type
           .replaceAll("!", "")
           .replaceAll("[", "")