feat(composition): add composeDirective#2703
feat(composition): add composeDirective#27032xrJohann wants to merge 5 commits intowundergraph:mainfrom
Conversation
|
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 support for composing persisted directives via a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
composition/src/v1/federation/federation-factory.ts (1)
2021-2265:⚠️ Potential issue | 🟠 Major
on SCHEMAcomposed directives are still dropped.The new wiring passes
composedDirectiveNamesthrough the type/field serializers, but this factory still never emits a schema definition node with persisted directives. A composed directive used asschema@myDirective`` will therefore disappear from both router and client outputs even though its definition is added. Please add a schema-node serialization path for composed directives and cover it with a regression test.Also applies to: 2332-2356
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composition/src/v1/federation/federation-factory.ts` around lines 2021 - 2265, The loop never handles schema definition nodes so composed directives applied with "on SCHEMA" are dropped; update federation-factory.ts to detect schema definition data (Kind.SCHEMA_DEFINITION / the SchemaDefinition data object) and serialize a SchemaDefinition node with persisted directives using the same utilities used for types: call getClientPersistedDirectiveNodes(parentDefinitionData, composedDirectiveNames) for the client output and this.getNodeForRouterSchemaByData(parentDefinitionData) (or the equivalent router-node serializer) for the router output, push those nodes to this.routerDefinitions and this.clientDefinitions, handle inaccessible/schema-specific validation like the other cases, and add a regression test that composes a schema with a directive (schema `@myDirective`) and asserts the directive appears in both router and client outputs.
🧹 Nitpick comments (1)
composition/src/v1/normalization/normalization-factory.ts (1)
3906-3920: Consider consolidating the two-step data population pattern.The code creates the persisted directive data via
addPersistedDirectiveDefinitionDataByNode, then immediately setsdata.locations = allLocations. This works but could be cleaner by extendingaddPersistedDirectiveDefinitionDataByNodeto accept an optionallocationsparameter.♻️ Potential refactor to pass locations directly
An alternative approach would be to extend
addPersistedDirectiveDefinitionDataByNodewith an optionallocationsparameter, avoiding the immediate mutation after creation. However, this is purely a style preference and the current implementation is correct.this.addPersistedDirectiveDefinitionDataByNode( composedDirectiveDefinitionDataByDirectiveName, directiveDefinitionNode, executableLocations, + allLocations, // optional parameter for full location set ); -const data = composedDirectiveDefinitionDataByDirectiveName.get(directiveName)!; -data.locations = allLocations;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composition/src/v1/normalization/normalization-factory.ts` around lines 3906 - 3920, Consolidate the two-step population by extending addPersistedDirectiveDefinitionDataByNode to accept an optional locations parameter and pass the computed allLocations directly instead of mutating data afterwards: compute allLocations (as currently done from directiveDefinitionNode.locations), call addPersistedDirectiveDefinitionDataByNode(composedDirectiveDefinitionDataByDirectiveName, directiveDefinitionNode, executableLocations, allLocations), then remove the immediate data.locations = allLocations assignment; update addPersistedDirectiveDefinitionDataByNode’s signature and any other callers to preserve backward compatibility (make the new parameter optional and set locations inside the function when provided).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@composition/src/v1/federation/federation-factory.ts`:
- Around line 1627-1648: The current pre-pass only accumulates provenance for
composed directives and keeps the first subgraph's
PersistedDirectiveDefinitionData; update the loop over
internalSubgraph.composedDirectiveDefinitionDataByDirectiveName to compare the
full directive definitions (use the node from
internalSubgraph.directiveDefinitionByName and the existing entry in
composedDirectiveDefinitionDataByDirectiveName and
persistedDirectiveDefinitionByDirectiveName) and either merge/validate all
definition fields (arguments, locations, repeatability, description, and any
other definition metadata) or throw a composition error when they conflict;
specifically, when an existing composedDirectiveDefinitionDataByDirectiveName
entry exists, check that node/definition shapes match (or are safely mergeable),
merge/subsumed fields into the existing definition and update
persistedDirectiveDefinitionByDirectiveName as needed, otherwise raise a clear
composition error indicating the conflicting directive and subgraphs involved.
---
Outside diff comments:
In `@composition/src/v1/federation/federation-factory.ts`:
- Around line 2021-2265: The loop never handles schema definition nodes so
composed directives applied with "on SCHEMA" are dropped; update
federation-factory.ts to detect schema definition data (Kind.SCHEMA_DEFINITION /
the SchemaDefinition data object) and serialize a SchemaDefinition node with
persisted directives using the same utilities used for types: call
getClientPersistedDirectiveNodes(parentDefinitionData, composedDirectiveNames)
for the client output and
this.getNodeForRouterSchemaByData(parentDefinitionData) (or the equivalent
router-node serializer) for the router output, push those nodes to
this.routerDefinitions and this.clientDefinitions, handle
inaccessible/schema-specific validation like the other cases, and add a
regression test that composes a schema with a directive (schema `@myDirective`)
and asserts the directive appears in both router and client outputs.
---
Nitpick comments:
In `@composition/src/v1/normalization/normalization-factory.ts`:
- Around line 3906-3920: Consolidate the two-step population by extending
addPersistedDirectiveDefinitionDataByNode to accept an optional locations
parameter and pass the computed allLocations directly instead of mutating data
afterwards: compute allLocations (as currently done from
directiveDefinitionNode.locations), call
addPersistedDirectiveDefinitionDataByNode(composedDirectiveDefinitionDataByDirectiveName,
directiveDefinitionNode, executableLocations, allLocations), then remove the
immediate data.locations = allLocations assignment; update
addPersistedDirectiveDefinitionDataByNode’s signature and any other callers to
preserve backward compatibility (make the new parameter optional and set
locations inside the function when provided).
🪄 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: 25fb18a6-0579-4276-839c-05aec23166cf
📒 Files selected for processing (10)
composition/src/errors/errors.tscomposition/src/normalization/types.tscomposition/src/schema-building/types.tscomposition/src/schema-building/utils.tscomposition/src/subgraph/types.tscomposition/src/v1/constants/directive-definitions.tscomposition/src/v1/federation/federation-factory.tscomposition/src/v1/normalization/normalization-factory.tscomposition/tests/v1/directives/compose-directive.test.tscomposition/tests/v1/utils/utils.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@composition/src/v1/normalization/normalization-factory.ts`:
- Around line 3913-3920: When merging composed directives in
normalization-factory.ts, do not blindly accept the first-seen full metadata; in
the block that calls addPersistedDirectiveDefinitionDataByNode and then reads
composedDirectiveDefinitionDataByDirectiveName.get(directiveName) (the code that
sets data.locations = allLocations), add validation that compares existing
stored metadata for that directive (argumentDataByName, repeatable,
executableLocations, locations, etc.) against the incoming
directiveDefinitionNode/subgraph payload; if any incompatible differences are
found, either raise a deterministic error or apply a well-documented
reconciliation rule (e.g., require exact match for argument shapes and
repeatable, union only for locations/executableLocations), and only then merge
subgraphNames and update locations — do this inside the same merge path so
conflicts are detected before accepting first-seen metadata.
🪄 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: 535031ac-a219-4fb1-b643-ce9b15ae886a
📒 Files selected for processing (2)
composition/src/errors/errors.tscomposition/src/v1/normalization/normalization-factory.ts
| this.addPersistedDirectiveDefinitionDataByNode( | ||
| composedDirectiveDefinitionDataByDirectiveName, | ||
| directiveDefinitionNode, | ||
| executableLocations, | ||
| ); | ||
| const data = composedDirectiveDefinitionDataByDirectiveName.get(directiveName)!; | ||
| data.locations = allLocations; | ||
| } |
There was a problem hiding this comment.
Validate cross-subgraph composed directive definition consistency before accepting first-seen metadata.
This stores full directive metadata (argumentDataByName, repeatable, executableLocations, locations, etc.) from one subgraph. Downstream federation merge currently only unions subgraphNames and preserves the first payload, so conflicting definitions across subgraphs can be silently accepted and produce order-dependent results.
Please add conflict detection (or deterministic reconciliation) for composed directives during merge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@composition/src/v1/normalization/normalization-factory.ts` around lines 3913
- 3920, When merging composed directives in normalization-factory.ts, do not
blindly accept the first-seen full metadata; in the block that calls
addPersistedDirectiveDefinitionDataByNode and then reads
composedDirectiveDefinitionDataByDirectiveName.get(directiveName) (the code that
sets data.locations = allLocations), add validation that compares existing
stored metadata for that directive (argumentDataByName, repeatable,
executableLocations, locations, etc.) against the incoming
directiveDefinitionNode/subgraph payload; if any incompatible differences are
found, either raise a deterministic error or apply a well-documented
reconciliation rule (e.g., require exact match for argument shapes and
repeatable, union only for locations/executableLocations), and only then merge
subgraphNames and update locations — do this inside the same merge path so
conflicts are detected before accepting first-seen metadata.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@composition/src/v1/federation/federation-factory.ts`:
- Around line 1654-1662: The intersection step can leave existing.locations
empty but still keep the directive; modify the logic around existing.locations
(the loop that checks data.locations) and the subsequent call to
setMutualExecutableLocations so that if existing.locations becomes empty you
either remove the directive from the composition set or throw a composition
error: after intersecting (the block referencing existing.locations and
data.locations) check existing.locations.size === 0 and then (a) drop the
directive from whatever map/array holds it (so it won't be serialized), or (b)
raise a composition error with a clear message indicating the directive lost all
mutual locations; ensure this check occurs before calling
setMutualExecutableLocations so executable locations aren't processed for a
directive with no locations.
🪄 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: 63e8cf3e-57c8-476f-9eee-3063315fe6cd
📒 Files selected for processing (1)
composition/src/v1/federation/federation-factory.ts
| // Intersect locations so only mutually supported locations are emitted | ||
| if (existing.locations && data.locations) { | ||
| for (const loc of existing.locations) { | ||
| if (!data.locations.has(loc)) { | ||
| existing.locations.delete(loc); | ||
| } | ||
| } | ||
| } | ||
| setMutualExecutableLocations(existing, data.executableLocations); |
There was a problem hiding this comment.
Reject directives whose mutual location set goes empty.
This intersection can leave existing.locations empty and still keep the directive definition alive. composition/src/schema-building/utils.ts:559-576 then serializes that set verbatim, so both schemas can end up with a directive definition that has no locations. Please raise a composition error or drop the directive once the mutual location set reaches zero.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@composition/src/v1/federation/federation-factory.ts` around lines 1654 -
1662, The intersection step can leave existing.locations empty but still keep
the directive; modify the logic around existing.locations (the loop that checks
data.locations) and the subsequent call to setMutualExecutableLocations so that
if existing.locations becomes empty you either remove the directive from the
composition set or throw a composition error: after intersecting (the block
referencing existing.locations and data.locations) check existing.locations.size
=== 0 and then (a) drop the directive from whatever map/array holds it (so it
won't be serialized), or (b) raise a composition error with a clear message
indicating the directive lost all mutual locations; ensure this check occurs
before calling setMutualExecutableLocations so executable locations aren't
processed for a directive with no locations.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
composition/src/v1/federation/federation-factory.ts (1)
1655-1663:⚠️ Potential issue | 🟠 MajorReject composed directives whose mutual location set goes empty.
This merge can leave
existing.locations/existing.executableLocationsempty and still keep the directive alive.buildValidPersistedDirectiveDefinitionNode()later serializesdata.locations ?? data.executableLocations, so the supergraph can end up with a directive definition that has no valid locations. Please drop the directive or raise a composition error as soon as the mutual set reaches zero.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composition/src/v1/federation/federation-factory.ts` around lines 1655 - 1663, When intersecting directive locations (the block that loops over existing.locations vs data.locations) and after calling setMutualExecutableLocations(existing, data.executableLocations), check whether the mutual sets have become empty (i.e., existing.locations is empty AND existing.executableLocations is empty, or if you prefer that persisted serialization uses data you can check data.locations/data.executableLocations similarly), and if so reject the directive immediately by removing it from the directives collection or throwing a composition error (so it never reaches buildValidPersistedDirectiveDefinitionNode). Update the logic around existing.locations, data.locations, and setMutualExecutableLocations to perform this empty-check and fail-fast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@composition/src/v1/federation/federation-factory.ts`:
- Line 2047: The code is passing the composedDirectiveNames set into
client-schema helper calls which causes getClientPersistedDirectiveNodes and
getClientSchemaFieldNodeByFieldData to filter out actual `@composeDirective`
usages; remove the composedDirectiveNames filter from those calls so the helpers
preserve directive applications. Specifically, in federation-factory.ts stop
passing the composedDirectiveNames variable into
getClientPersistedDirectiveNodes(...) and
getClientSchemaFieldNodeByFieldData(...), or pass undefined/empty set instead,
and apply the same change to the other call sites where composedDirectiveNames
is currently forwarded so directive usages are retained.
- Around line 1669-1672: When reporting repeatability conflicts use the combined
set of subgraphs instead of only existing.subgraphNames: merge
data.subgraphNames into existing.subgraphNames (the same way addIterableToSet
does) before calling composeDirectiveRepeatableConflictError so the error
created by composeDirectiveRepeatableConflictError(directiveName, ...) includes
both the original and incoming subgraph names; i.e., perform the union of
data.subgraphNames and existing.subgraphNames first and then push the composed
error into this.errors.
---
Duplicate comments:
In `@composition/src/v1/federation/federation-factory.ts`:
- Around line 1655-1663: When intersecting directive locations (the block that
loops over existing.locations vs data.locations) and after calling
setMutualExecutableLocations(existing, data.executableLocations), check whether
the mutual sets have become empty (i.e., existing.locations is empty AND
existing.executableLocations is empty, or if you prefer that persisted
serialization uses data you can check data.locations/data.executableLocations
similarly), and if so reject the directive immediately by removing it from the
directives collection or throwing a composition error (so it never reaches
buildValidPersistedDirectiveDefinitionNode). Update the logic around
existing.locations, data.locations, and setMutualExecutableLocations to perform
this empty-check and fail-fast.
🪄 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: c77f03bf-f72e-49db-8579-cba191d9cd60
📒 Files selected for processing (3)
composition/src/errors/errors.tscomposition/src/v1/federation/federation-factory.tscomposition/tests/v1/directives/compose-directive.test.ts
| } | ||
|
|
||
| pushParentDefinitionDataToDocumentDefinitions(interfaceImplementations: InterfaceImplementationData[]) { | ||
| const composedDirectiveNames = new Set(this.composedDirectiveDefinitionDataByDirectiveName.keys()); |
There was a problem hiding this comment.
These client-schema helper calls currently strip composed directive usages.
getClientPersistedDirectiveNodes() in composition/src/schema-building/utils.ts:447-476 skips any directive whose name is present in composedDirectiveNames, and getClientSchemaFieldNodeByFieldData() forwards the same filter. Passing this set at all of these call sites means the client SDL keeps the directive definitions you add later, but drops the actual @composeDirective applications from types, fields, enum values, and arguments. That breaks the main client-schema path for this feature.
Also applies to: 2066-2066, 2113-2113, 2137-2137, 2183-2183, 2209-2209, 2253-2253, 2271-2271, 2290-2290, 2358-2358, 2379-2379
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@composition/src/v1/federation/federation-factory.ts` at line 2047, The code
is passing the composedDirectiveNames set into client-schema helper calls which
causes getClientPersistedDirectiveNodes and getClientSchemaFieldNodeByFieldData
to filter out actual `@composeDirective` usages; remove the composedDirectiveNames
filter from those calls so the helpers preserve directive applications.
Specifically, in federation-factory.ts stop passing the composedDirectiveNames
variable into getClientPersistedDirectiveNodes(...) and
getClientSchemaFieldNodeByFieldData(...), or pass undefined/empty set instead,
and apply the same change to the other call sites where composedDirectiveNames
is currently forwarded so directive usages are retained.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
composition/src/v1/federation/federation-factory.ts (1)
1629-1641:⚠️ Potential issue | 🟠 MajorNon-repeatable composed directive applications can still duplicate across subgraphs.
Registering custom directives here makes them flow through
extractPersistedDirectives(), but that helper still only treatsNON_REPEATABLE_PERSISTED_DIRECTIVESas singletons. If the same non-repeatable composed directive is applied to the same coordinate in two subgraphs, both nodes are retained; the router path then hitsinvalidRepeatedFederatedDirectiveErrorMessage, and the client path can emit duplicate applications. Please dedupe/merge those usages from the merged directive definition’srepeatableflag instead of the static built-in set.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composition/src/v1/federation/federation-factory.ts` around lines 1629 - 1641, The current registration keeps multiple applications of the same non-repeatable composed directive across subgraphs because deduping relies on the static NON_REPEATABLE_PERSISTED_DIRECTIVES instead of the merged directive definition; update the logic that registers and/or extracts persisted directives (around persistedDirectiveDefinitionByDirectiveName and extractPersistedDirectives) to consult the merged directive definition's repeatable flag (from the composed/merged directive data you get via internalSubgraph.composedDirectiveDefinitionDataByDirectiveName) and, if the merged directive is non-repeatable, collapse duplicate applications at the same coordinate into a single retained node (so you no longer rely on NON_REPEATABLE_PERSISTED_DIRECTIVES and avoid triggering invalidRepeatedFederatedDirectiveErrorMessage or emitting duplicates).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@composition/src/v1/federation/federation-factory.ts`:
- Around line 1660-1680: The directive-definition location intersection must
also prune stored directive usages/usages collections so no stale usages remain;
after you intersect existing.locations with data.locations (and call
setMutualExecutableLocations), iterate the accumulated usages for this
directiveName (the structure that stores per-directive usages—e.g., the usages
collection attached to the existing directive entry in
composedDirectiveDefinitionDataByDirectiveName) and remove any usage whose
location is not present in existing.locations (or existing.executableLocations
as appropriate); if that removal leaves no usages and/or no valid locations
remain, reuse composeDirectiveNoMutualLocationsError, remove the
composedDirectiveDefinitionDataByDirectiveName entry and add directiveName to
rejectedComposedDirectiveNames as done below. Ensure the pruning logic
references existing.locations, existing.executableLocations,
composedDirectiveDefinitionDataByDirectiveName and
rejectedComposedDirectiveNames so emitted SDL cannot contain usages outside the
mutually supported locations.
---
Duplicate comments:
In `@composition/src/v1/federation/federation-factory.ts`:
- Around line 1629-1641: The current registration keeps multiple applications of
the same non-repeatable composed directive across subgraphs because deduping
relies on the static NON_REPEATABLE_PERSISTED_DIRECTIVES instead of the merged
directive definition; update the logic that registers and/or extracts persisted
directives (around persistedDirectiveDefinitionByDirectiveName and
extractPersistedDirectives) to consult the merged directive definition's
repeatable flag (from the composed/merged directive data you get via
internalSubgraph.composedDirectiveDefinitionDataByDirectiveName) and, if the
merged directive is non-repeatable, collapse duplicate applications at the same
coordinate into a single retained node (so you no longer rely on
NON_REPEATABLE_PERSISTED_DIRECTIVES and avoid triggering
invalidRepeatedFederatedDirectiveErrorMessage or emitting duplicates).
🪄 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: a8a70fcd-cc7d-419a-81f4-a132ff882391
📒 Files selected for processing (3)
composition/src/errors/errors.tscomposition/src/v1/federation/federation-factory.tscomposition/tests/v1/directives/compose-directive.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- composition/src/errors/errors.ts
| // Intersect locations so only mutually supported locations are emitted | ||
| if (existing.locations && data.locations) { | ||
| for (const loc of existing.locations) { | ||
| if (!data.locations.has(loc)) { | ||
| existing.locations.delete(loc); | ||
| } | ||
| } | ||
| } | ||
| setMutualExecutableLocations(existing, data.executableLocations); | ||
| // Reject if no locations remain after intersection | ||
| const effectiveLocationsEmpty = | ||
| existing.locations !== undefined | ||
| ? existing.locations.size === 0 | ||
| : existing.executableLocations.size === 0; | ||
| if (effectiveLocationsEmpty) { | ||
| addIterableToSet({ source: data.subgraphNames, target: existing.subgraphNames }); | ||
| this.errors.push(composeDirectiveNoMutualLocationsError(directiveName, existing.subgraphNames)); | ||
| this.composedDirectiveDefinitionDataByDirectiveName.delete(directiveName); | ||
| rejectedComposedDirectiveNames.add(directiveName); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Shrinking the definition locations here can leave stale directive usages behind.
This only narrows the directive definition. The accumulated directive nodes on types/fields/input values/enum values are never filtered against the reduced location set, so OBJECT | FIELD_DEFINITION vs FIELD_DEFINITION can still produce directive @myDirective ... on FIELD_DEFINITION while an object-level @myDirective usage remains in the emitted router/client SDL. Because both schemas are built with assumeValidSDL, that invalid combination will not be caught locally. Please reject or strip usages on locations that fall out of the intersection.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@composition/src/v1/federation/federation-factory.ts` around lines 1660 -
1680, The directive-definition location intersection must also prune stored
directive usages/usages collections so no stale usages remain; after you
intersect existing.locations with data.locations (and call
setMutualExecutableLocations), iterate the accumulated usages for this
directiveName (the structure that stores per-directive usages—e.g., the usages
collection attached to the existing directive entry in
composedDirectiveDefinitionDataByDirectiveName) and remove any usage whose
location is not present in existing.locations (or existing.executableLocations
as appropriate); if that removal leaves no usages and/or no valid locations
remain, reuse composeDirectiveNoMutualLocationsError, remove the
composedDirectiveDefinitionDataByDirectiveName entry and add directiveName to
rejectedComposedDirectiveNames as done below. Ensure the pruning logic
references existing.locations, existing.executableLocations,
composedDirectiveDefinitionDataByDirectiveName and
rejectedComposedDirectiveNames so emitted SDL cannot contain usages outside the
mutually supported locations.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
1 participant
I have read contributing.md and have not raised an issue yet - I will get there just experimenting for now
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Checklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.