fix(composition): resolve @composeDirective definitions per-directive instead of per-spec (#3391)

## Summary

Fixes a bug in `ComposeDirectiveManager` where `@composeDirective` would
fail
with a false `DIRECTIVE_COMPOSITION_ERROR` when two subgraphs linked the
same
custom spec but imported different subsets of its directives.

Closes FED-849

---

## Problem

`getLatestDirectiveDefinition()` resolved a directive's definition
through a two-hop chain:

```
directiveName → spec identity → "latest" subgraph for that spec → schema.directive(...)
```

The "latest" subgraph was determined solely by its spec version (highest
minor).
This meant that if the latest-version subgraph had not imported the
specific directive
being resolved, the lookup would silently fail and produce a false error
— even though
another subgraph had a perfectly valid definition for it.

**Failing scenarios:**

```graphql
# SG1 — imports @foo AND @bar, composes both
@link(url: "https://myorg.dev/myspec/v1.0", import: ["@foo", "@bar"])
@composeDirective(name: "@foo") @composeDirective(name: "@bar")

# SG2 — imports only @foo from the same spec
@link(url: "https://myorg.dev/myspec/v1.0", import: ["@foo"])
@composeDirective(name: "@foo")
```

If the iterator elected SG2 as "latest", resolving `@bar` would fail:

```
DIRECTIVE_COMPOSITION_ERROR: Core feature "https://myorg.dev/myspec" in subgraph "SG2"
does not have a directive definition for "@bar"
```

The bug was order-dependent: it succeeded only when the elected subgraph
happened to be
a superset of all other subgraphs' imports. Completely disjoint imports
(SG1 has `@foo`,
SG2 has `@bar`) always failed regardless of order.

---

## Fix

Added a `directiveDefinitionMap: Map<string, DirectiveDefinition>` built
during `validate()`.
For each composed directive, it finds the highest-minor-version subgraph
that **actually
imported** that specific directive, and stores its definition directly.
`getLatestDirectiveDefinition()` now does a single lookup on this map
instead of the
indirect two-hop chain.

**Before:**
```
directiveName → spec identity → latestSubgraph (may not have the directive!) → definition
```

**After:**
```
directiveName → definition  (sourced from the subgraph that actually imported it)
```

---

## Changes

| File | Description |
|---|---|
| `composition-js/src/composeDirectiveManager.ts` | New
`directiveDefinitionMap`, populated in `validate()`,
`getLatestDirectiveDefinition()` simplified to a direct lookup |
| `composition-js/src/__tests__/compose.composeDirective.test.ts` | Two
new regression tests covering the subset and fully-disjoint import
scenarios; one existing test updated that was asserting the previous
(buggy) error behavior |

---

## Testing

```bash
# Regression tests for FED-849
node_modules/.bin/jest --projects composition-js --no-coverage --testNamePattern="FED-849"

# Full composition suite
node_modules/.bin/jest --projects composition-js --no-coverage
```

All 15 test suites pass (400 tests, 0 failures).

---------

Co-authored-by: Dariusz Kuc <9501705+dariuszkuc@users.noreply.github.com>

Author: apollo-mateuswgoettems

.changeset/fix-compose-directive-subset-imports.md

@@ -0,0 +1,29 @@
+---
+"@apollo/composition": minor
+---
+
+Update `@composeDirective` merge logic to allow subgraphs to import different directives from the same spec. Previously, all subgraphs had to import ALL (exactly the same) custom directives (even if they did not use them) as otherwise composition would fail with a false `DIRECTIVE_COMPOSITION_ERROR`.
+
+**Affected scenario:** two or more subgraphs `@link` the same custom spec URL but import different directives from it:
+
+```graphql
+# subgraph A — imports @foo AND @bar, composes both
+@link(url: "https://myorg.dev/myspec/v1.0", import: ["@foo", "@bar"])
+@composeDirective(name: "@foo") @composeDirective(name: "@bar")
+
+# subgraph B — imports only @foo from the same spec
+@link(url: "https://myorg.dev/myspec/v1.0", import: ["@foo"])
+@composeDirective(name: "@foo")
+```
+
+This would produce a false error:
+```
+DIRECTIVE_COMPOSITION_ERROR: Core feature "https://myorg.dev/myspec" in subgraph "B"
+does not have a directive definition for "@foo"
+```
+
+The bug was order-dependent (failed when the subgraph elected as "latest" for the spec was not a superset of all other subgraphs' imports) and always failed for fully disjoint import sets (e.g. subgraph A imports only `@foo`, subgraph B imports only `@bar`).
+
+**Root cause:** `ComposeDirectiveManager.getLatestDirectiveDefinition()` resolved definitions through a two-hop chain — `directiveName → spec identity → "latest" subgraph for that spec → schema.directive(...)`. The "latest" subgraph was determined by spec version alone, without checking whether it had actually imported the directive being resolved.
+
+**Fix:** a `directiveDefinitionMap` (`directiveName → DirectiveDefinition`) is now built during `validate()`, sourcing each entry from the highest-minor-version subgraph that actually imported that specific directive. `getLatestDirectiveDefinition()` does a direct lookup on this map instead of the indirect chain.

composition-js/src/__tests__/compose.composeDirective.test.ts

@@ -156,6 +156,115 @@ describe('composing custom core directives', () => {
     expectDirectiveOnElement(schema, 'User.a', 'foo', { name: 'a' });
   });
 
+  // FED-849: composition should succeed when subgraphs import different subsets
+  // of directives from the same custom spec (e.g. SG1 imports [@foo, @bar], SG2 imports [@foo]).
+  it('two subgraphs with same spec but disjoint directive imports (FED-849)', () => {
+    // subgraphA imports both @foo and @bar from the spec and requests both to be composed
+    const subgraphA = generateSubgraph({
+      name: 'subgraphA',
+      linkText: '@link(url: "https://specs.apollo.dev/foo/v1.0", import: ["@foo", "@bar"])',
+      composeText: '@composeDirective(name: "@foo") @composeDirective(name: "@bar")',
+      directiveText: `
+        directive @foo(name: String!) on FIELD_DEFINITION
+        directive @bar(tag: String!) on FIELD_DEFINITION
+      `,
+      usage: '@foo(name: "a") @bar(tag: "t")',
+    });
+
+    // subgraphB imports only @foo from the same spec and requests only @foo to be composed
+    const subgraphB = generateSubgraph({
+      name: 'subgraphB',
+      linkText: '@link(url: "https://specs.apollo.dev/foo/v1.0", import: ["@foo"])',
+      composeText: '@composeDirective(name: "@foo")',
+      directiveText: 'directive @foo(name: String!) on FIELD_DEFINITION',
+      usage: '@foo(name: "b")',
+    });
+
+    const result = composeServices([subgraphA, subgraphB]);
+    const schema = expectNoErrors(result);
+
+    // both directives must be present in the supergraph
+    expectDirectiveDefinition(schema, 'foo', [DirectiveLocation.FIELD_DEFINITION], ['name']);
+    expectDirectiveDefinition(schema, 'bar', [DirectiveLocation.FIELD_DEFINITION], ['tag']);
+
+    // usages must be carried over correctly
+    expectDirectiveOnElement(schema, 'User.subgraphA', 'foo', { name: 'a' });
+    expectDirectiveOnElement(schema, 'User.subgraphA', 'bar', { tag: 't' });
+    expectDirectiveOnElement(schema, 'User.subgraphB', 'foo', { name: 'b' });
+  });
+
+  // FED-849 (disjoint variant): one subgraph imports only @foo, the other imports only @bar.
+  // The resolve path must not go through the "latest feature" subgraph which may not have imported
+  // the directive being resolved.
+  it('two subgraphs with same spec importing completely disjoint directives (FED-849)', () => {
+    // subgraphA imports only @foo
+    const subgraphA = generateSubgraph({
+      name: 'subgraphA',
+      linkText: '@link(url: "https://specs.apollo.dev/foo/v1.0", import: ["@foo"])',
+      composeText: '@composeDirective(name: "@foo")',
+      directiveText: 'directive @foo(name: String!) on FIELD_DEFINITION',
+      usage: '@foo(name: "a")',
+    });
+
+    // subgraphB imports only @bar from the same spec
+    const subgraphB = generateSubgraph({
+      name: 'subgraphB',
+      linkText: '@link(url: "https://specs.apollo.dev/foo/v1.0", import: ["@bar"])',
+      composeText: '@composeDirective(name: "@bar")',
+      directiveText: 'directive @bar(tag: String!) on FIELD_DEFINITION',
+      usage: '@bar(tag: "t")',
+    });
+
+    const result = composeServices([subgraphA, subgraphB]);
+    const schema = expectNoErrors(result);
+
+    expectDirectiveDefinition(schema, 'foo', [DirectiveLocation.FIELD_DEFINITION], ['name']);
+    expectDirectiveDefinition(schema, 'bar', [DirectiveLocation.FIELD_DEFINITION], ['tag']);
+
+    expectDirectiveOnElement(schema, 'User.subgraphA', 'foo', { name: 'a' });
+    expectDirectiveOnElement(schema, 'User.subgraphB', 'bar', { tag: 't' });
+  });
+
+  // FED-849 (cross-version variant): subgraphA is at foo/v1.0 and imports both @foo and @bar;
+  // subgraphB is at foo/v1.1 (newer minor) but only imports @foo.
+  // Without the fix, latestFeatureMap elects subgraphB (higher minor version) as the canonical
+  // source for the entire spec, so resolving @bar fails because subgraphB never imported it.
+  // With the fix, @bar is resolved directly from subgraphA — the only subgraph that imported it.
+  it('older subgraph imports superset of directives, newer subgraph imports subset (FED-849)', () => {
+    // subgraphA is on the older spec version but imports both directives
+    const subgraphA = generateSubgraph({
+      name: 'subgraphA',
+      linkText: '@link(url: "https://specs.apollo.dev/foo/v1.0", import: ["@foo", "@bar"])',
+      composeText: '@composeDirective(name: "@foo") @composeDirective(name: "@bar")',
+      directiveText: `
+        directive @foo(name: String!) on FIELD_DEFINITION
+        directive @bar(tag: String!) on FIELD_DEFINITION
+      `,
+      usage: '@foo(name: "a") @bar(tag: "t")',
+    });
+
+    // subgraphB is on the newer spec version but only imports @foo — does NOT import @bar
+    const subgraphB = generateSubgraph({
+      name: 'subgraphB',
+      linkText: '@link(url: "https://specs.apollo.dev/foo/v1.1", import: ["@foo"])',
+      composeText: '@composeDirective(name: "@foo")',
+      directiveText: 'directive @foo(name: String!) on FIELD_DEFINITION',
+      usage: '@foo(name: "b")',
+    });
+
+    // @bar must be resolved from subgraphA, not from subgraphB (which has the newer spec version
+    // but never imported @bar). Composition should succeed.
+    const result = composeServices([subgraphA, subgraphB]);
+    const schema = expectNoErrors(result);
+
+    expectDirectiveDefinition(schema, 'foo', [DirectiveLocation.FIELD_DEFINITION], ['name']);
+    expectDirectiveDefinition(schema, 'bar', [DirectiveLocation.FIELD_DEFINITION], ['tag']);
+
+    expectDirectiveOnElement(schema, 'User.subgraphA', 'foo', { name: 'a' });
+    expectDirectiveOnElement(schema, 'User.subgraphA', 'bar', { tag: 't' });
+    expectDirectiveOnElement(schema, 'User.subgraphB', 'foo', { name: 'b' });
+  });
+
   it('different major versions of core feature results in hint if not composed', () => {
     const subgraphA = generateSubgraph({
       name: 'subgraphA',
@@ -549,7 +658,11 @@ describe('composing custom core directives', () => {
     expectDirectiveOnElement(schema, 'User.subgraphB', 'bar', { name: 'b' });
   });
 
-  it('exported directive not imported everywhere. no definition', () => {
+  // FED-849 (version mismatch variant): subgraphA is at foo/v1.1 and only imports @foo;
+  // subgraphB is at foo/v1.0 and only imports @bar. The "latest" subgraph (A, v1.1) does not
+  // have @bar in its schema, but the fix resolves @bar's definition directly from subgraphB,
+  // so composition must succeed instead of raising a false "no definition" error.
+  it('exported directive not imported everywhere. no definition in latest-version subgraph should still compose (FED-849)', () => {
     const subgraphA = generateSubgraph({
       name: 'subgraphA',
       linkText: '@link(url: "https://specs.apollo.dev/foo/v1.1", import: ["@foo"])',
@@ -570,14 +683,14 @@ describe('composing custom core directives', () => {
       usage: '@bar(name: "b")',
     });
 
+    // @bar's definition is resolved from subgraphB (the only subgraph that imported it),
+    // so composition should succeed without errors.
     const result = composeServices([subgraphA, subgraphB]);
-    expect(errors(result)).toStrictEqual([
-      [
-      'DIRECTIVE_COMPOSITION_ERROR',
-      'Core feature "https://specs.apollo.dev/foo" in subgraph "subgraphA" does not have a directive definition for "@bar"',
-      ]
-    ]);
-    expect(hints(result)).toStrictEqual([]);
+    const schema = expectNoErrors(result);
+    expectDirectiveDefinition(schema, 'foo', [DirectiveLocation.FIELD_DEFINITION], ['name']);
+    expectDirectiveDefinition(schema, 'bar', [DirectiveLocation.FIELD_DEFINITION, DirectiveLocation.OBJECT], ['name', 'address']);
+    expectDirectiveOnElement(schema, 'User.subgraphA', 'foo', { name: 'a' });
+    expectDirectiveOnElement(schema, 'User.subgraphB', 'bar', { name: 'b' });
   });
 
   it.todo('composing same major version, but incompatible directives results in error');

composition-js/src/composeDirectiveManager.ts

@@ -80,6 +80,11 @@ export class ComposeDirectiveManager {
   // map of directive names to identity,origName
   directiveIdentityMap: Map<string, [string,string]>;
 
+  // map of directive names to a resolved DirectiveDefinition, sourced from the subgraph
+  // that actually imported each directive (not necessarily the "latest" subgraph for that spec).
+  // This avoids the FED-849 bug where the latest-spec subgraph may not have imported the directive.
+  directiveDefinitionMap: Map<string, DirectiveDefinition>;
+
   mismatchReporter: MismatchReporter;
 
   constructor(
@@ -90,6 +95,7 @@ export class ComposeDirectiveManager {
     this.mergeDirectiveMap = new Map();
     this.latestFeatureMap = new Map();
     this.directiveIdentityMap = new Map();
+    this.directiveDefinitionMap = new Map();
     this.mismatchReporter = new MismatchReporter(subgraphs.names(), pushError, pushHint);
   }
 
@@ -436,6 +442,33 @@ export class ComposeDirectiveManager {
       this.mergeDirectiveMap.set(subgraph, directivesForSubgraph);
     }
 
+    // Build a direct mapping from directive name to its definition, sourced from the subgraph
+    // with the highest minor version that actually imported it. This is the fix for FED-849:
+    // instead of resolving through latestFeatureMap (spec → one subgraph that may not have
+    // imported the directive), we track exactly which subgraph imported each directive.
+    for (const [directiveName, items] of itemsByDirectiveName.entries()) {
+      if (wontMergeDirectiveNames.has(directiveName)) {
+        continue;
+      }
+      // Sort by minor version descending so we pick the highest compatible version first.
+      const candidates = items
+        .filter(item => !wontMergeFeatures.has(item.feature.url.identity))
+        .sort((a, b) => b.feature.url.version.minor - a.feature.url.version.minor);
+
+      for (const item of candidates) {
+        const sg = this.subgraphs.get(item.sgName);
+        assert(sg, `subgraph "${item.sgName}" does not exist`);
+        const nameInSchema = sg.schema.coreFeatures
+          ?.getByIdentity(item.feature.url.identity)
+          ?.directiveNameInSchema(item.directiveName);
+        const def = nameInSchema ? sg.schema.directive(nameInSchema) : undefined;
+        if (def) {
+          this.directiveDefinitionMap.set(directiveName, def);
+          break;
+        }
+      }
+    }
+
     return {
       errors,
       hints,
@@ -455,33 +488,11 @@ export class ComposeDirectiveManager {
   }
 
   getLatestDirectiveDefinition(directiveName: string): DirectiveDefinition | undefined {
-    const val = this.directiveIdentityMap.get(directiveName);
-    if (val) {
-      const [identity, origName] = val;
-      const entry = this.latestFeatureMap.get(identity);
-      assert(entry, 'core feature identity must exist in map');
-      const [feature, subgraphName] = entry;
-      const subgraph = this.subgraphs.get(subgraphName);
-      assert(subgraph, `subgraph "${subgraphName}" does not exist`);
-
-      // we need to convert from the name that is used in the schemas that export the directive
-      // to the name used in the schema that is the latest version, which may or may not export
-      // See test "exported directive not imported everywhere. imported with different name"
-      const nameInSchema = subgraph.schema.coreFeatures?.getByIdentity(identity)?.directiveNameInSchema(origName);
-      if (nameInSchema) {
-        const directive = subgraph.schema.directive(nameInSchema);
-        if (!directive) {
-          this.pushError(ERRORS.DIRECTIVE_COMPOSITION_ERROR.err(
-            `Core feature "${identity}" in subgraph "${subgraphName}" does not have a directive definition for "@${directiveName}"`,
-            {
-              nodes: feature.directive.sourceAST,
-            },
-          ));
-        }
-        return directive;
-      }
-    }
-    return undefined;
+    // Resolve directly from the per-directive map that was populated in validate().
+    // Each entry is sourced from the highest-minor-version subgraph that actually imported
+    // the directive, avoiding the FED-849 bug where the "latest spec" subgraph may not
+    // have imported every directive from that spec.
+    return this.directiveDefinitionMap.get(directiveName);
   }
 
   private directivesForFeature(identity: string): [string,string][] {