Skip to content

fix: show feature flags for a federated graph under split config - #3129

Open
JivusAyrus wants to merge 16 commits into
mainfrom
suvij/cosmo-315-controlplane-feature-flags-not-shown-for-a-federated-graph
Open

fix: show feature flags for a federated graph under split config#3129
JivusAyrus wants to merge 16 commits into
mainfrom
suvij/cosmo-315-controlplane-feature-flags-not-shown-for-a-federated-graph

Conversation

@JivusAyrus

@JivusAyrus JivusAyrus commented Jul 30, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features

    • Added stale-composition indicators for feature flags, including warnings when the latest composition fails.
    • Added links from stale warnings to the feature flag’s composition history.
    • Exposed latest composition failure status with feature-flag data.
  • Bug Fixes

    • Improved feature-flag retrieval for split configuration loading and failed compositions.
    • Federated graph details now correctly return “not found” when the graph cannot be resolved.
  • Tests

    • Expanded coverage for split-config, legacy behavior, failed compositions, SDL retrieval, and federated graph lookups.

Checklist

image image image

In case the ff composition fails
image

@JivusAyrus
JivusAyrus requested a review from a team as a code owner July 30, 2026 09:16
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Feature-flag resolution now uses the latest valid composition for a federated graph and target. Split-config-loading retains flags from previous valid compositions and reports failed latest compositions. API contracts, integration tests, and Studio stale-composition indicators were updated.

Changes

Feature-flag composition scoping

Layer / File(s) Summary
Repository query scoping
controlplane/src/core/repositories/FeatureFlagRepository.ts
Selects feature-flag schema versions by federated graph and target. Split-config-loading compares latest and latest-valid compositions. Legacy mode uses the latest valid base schema version.
Handler integration and unresolved graph handling
controlplane/src/core/bufservices/feature-flag/getFeatureFlagsInLatestCompositionByFederatedGraph.ts, controlplane/src/core/bufservices/federated-graph/getFederatedGraphById.ts, controlplane/src/core/bufservices/federated-graph/getCompositionDetails.ts
Handlers use latest-composition lookups and return hasFailedLatestComposition. Composition details returns ERR_NOT_FOUND when the target graph cannot be resolved.
Feature-flag stale-status contract
controlplane/src/types/index.ts, proto/wg/cosmo/platform/v1/platform.proto, connect/src/wg/cosmo/platform/v1/platform_pb.ts
Adds optional stale-composition status to the DTO and platform FeatureFlag message.
Split-config-loading validation
controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts, controlplane/test/feature-flag/get-federated-graph-sdl-by-name.test.ts, controlplane/test/federated-graph/get-federated-graph-by-id.test.ts
Tests latest valid composition selection, failed latest and base compositions, failed initial compositions, SDL fallback, and federated-graph retrieval.
Studio stale-composition indicators
studio/src/components/schema/stale-composition-warning.tsx, studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/schema/index.tsx, studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/schema/sdl.tsx
Displays stale icons beside affected flags and a warning banner for the active stale flag. Links point to the graph compositions page.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: displaying feature flags for federated graphs when split configuration is enabled.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
controlplane/test/composition/getCompositionDetails.test.ts (1)

597-600: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test federated-graph isolation, not only positive retrieval. Each test uses one graph, so removing federatedGraphId filtering would still pass.

  • controlplane/test/composition/getCompositionDetails.test.ts#L597-L600: add another graph and assert its flag composition is excluded.
  • controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts#L121-L135: assert only the requested graph’s flags are returned.
  • controlplane/test/federated-graph/get-federated-graph-by-id.test.ts#L243-L245: assert the response excludes a flag composed for another graph.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@controlplane/test/composition/getCompositionDetails.test.ts` around lines 597
- 600, Add federated-graph isolation assertions at all three sites: in
controlplane/test/composition/getCompositionDetails.test.ts:597-600, create a
flag composition for another graph and assert it is excluded; in
controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts:121-135,
assert only flags from the requested graph are returned; and in
controlplane/test/federated-graph/get-federated-graph-by-id.test.ts:243-245,
assert the response excludes a flag composed for another graph.
controlplane/test/federated-graph/get-federated-graph-by-id.test.ts (1)

195-224: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add explicit callback annotations.

The newly added test, cleanup, and labels.map callbacks rely on inferred parameter and return types. Annotate them to follow the TypeScript guideline.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@controlplane/test/federated-graph/get-federated-graph-by-id.test.ts` around
lines 195 - 224, Add explicit parameter and return type annotations to the
callbacks introduced in the test, including the test callback,
testContext.onTestFinished cleanup callback, and labels.map callback. Keep the
existing callback behavior unchanged while following the repository’s TypeScript
typing guideline.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@controlplane/test/composition/getCompositionDetails.test.ts`:
- Around line 597-600: Add federated-graph isolation assertions at all three
sites: in controlplane/test/composition/getCompositionDetails.test.ts:597-600,
create a flag composition for another graph and assert it is excluded; in
controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts:121-135,
assert only flags from the requested graph are returned; and in
controlplane/test/federated-graph/get-federated-graph-by-id.test.ts:243-245,
assert the response excludes a flag composed for another graph.

In `@controlplane/test/federated-graph/get-federated-graph-by-id.test.ts`:
- Around line 195-224: Add explicit parameter and return type annotations to the
callbacks introduced in the test, including the test callback,
testContext.onTestFinished cleanup callback, and labels.map callback. Keep the
existing callback behavior unchanged while following the repository’s TypeScript
typing guideline.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b82d47b6-fa3c-4d4e-ac52-1899e0edd73a

📥 Commits

Reviewing files that changed from the base of the PR and between b64f0d4 and b72648a.

📒 Files selected for processing (7)
  • controlplane/src/core/bufservices/feature-flag/getFeatureFlagsInLatestCompositionByFederatedGraph.ts
  • controlplane/src/core/bufservices/federated-graph/getCompositionDetails.ts
  • controlplane/src/core/bufservices/federated-graph/getFederatedGraphById.ts
  • controlplane/src/core/repositories/FeatureFlagRepository.ts
  • controlplane/test/composition/getCompositionDetails.test.ts
  • controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts
  • controlplane/test/federated-graph/get-federated-graph-by-id.test.ts

@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 51.68539% with 86 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.94%. Comparing base (64eaf60) to head (1fef1c0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
[...ionSlug]/[namespace]/graph/[slug]/schema/index.tsx](https://app.codecov.io/gh/wundergraph/cosmo/pull/3129?src=pr&el=tree&filepath=studio%2Fsrc%2Fpages%2F%5BorganizationSlug%5D%2F%5Bnamespace%5D%2Fgraph%2F%5Bslug%5D%2Fschema%2Findex.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=wundergraph#diff-c3R1ZGlvL3NyYy9wYWdlcy9bb3JnYW5pemF0aW9uU2x1Z10vW25hbWVzcGFjZV0vZ3JhcGgvW3NsdWddL3NjaGVtYS9pbmRleC50c3g=) 0.00% 32 Missing ⚠️
[...ationSlug]/[namespace]/graph/[slug]/schema/sdl.tsx](https://app.codecov.io/gh/wundergraph/cosmo/pull/3129?src=pr&el=tree&filepath=studio%2Fsrc%2Fpages%2F%5BorganizationSlug%5D%2F%5Bnamespace%5D%2Fgraph%2F%5Bslug%5D%2Fschema%2Fsdl.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=wundergraph#diff-c3R1ZGlvL3NyYy9wYWdlcy9bb3JnYW5pemF0aW9uU2x1Z10vW25hbWVzcGFjZV0vZ3JhcGgvW3NsdWddL3NjaGVtYS9zZGwudHN4) 0.00% 24 Missing ⚠️
...tudio/src/components/composition-errors-banner.tsx 0.00% 10 Missing ⚠️
...rc/components/schema/stale-composition-warning.tsx 0.00% 9 Missing and 1 partial ⚠️
...fservices/federated-graph/getCompositionDetails.ts 30.76% 9 Missing ⚠️
studio/src/components/ui/button.tsx 0.00% 1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (51.68%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #3129       +/-   ##
===========================================
- Coverage   62.37%   43.94%   -18.44%     
===========================================
  Files         262     1072      +810     
  Lines       31003   140358   +109355     
  Branches        0     7383     +7383     
===========================================
+ Hits        19337    61674    +42337     
- Misses      10158    76831    +66673     
- Partials     1508     1853      +345     
Files with missing lines Coverage Δ
...FeatureFlagsInLatestCompositionByFederatedGraph.ts 83.82% <100.00%> (ø)
...fservices/federated-graph/getFederatedGraphById.ts 91.09% <100.00%> (ø)
...ices/federated-graph/getFederatedGraphSDLByName.ts 83.15% <100.00%> (ø)
...ane/src/core/repositories/FeatureFlagRepository.ts 94.35% <100.00%> (ø)
controlplane/src/types/index.ts 100.00% <ø> (ø)
studio/src/components/ui/button.tsx 0.00% <0.00%> (ø)
...fservices/federated-graph/getCompositionDetails.ts 86.17% <30.76%> (ø)
...tudio/src/components/composition-errors-banner.tsx 0.00% <0.00%> (ø)
...rc/components/schema/stale-composition-warning.tsx 0.00% <0.00%> (ø)
[...ationSlug]/[namespace]/graph/[slug]/schema/sdl.tsx](https://app.codecov.io/gh/wundergraph/cosmo/pull/3129?src=pr&el=tree&filepath=studio%2Fsrc%2Fpages%2F%5BorganizationSlug%5D%2F%5Bnamespace%5D%2Fgraph%2F%5Bslug%5D%2Fschema%2Fsdl.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=wundergraph#diff-c3R1ZGlvL3NyYy9wYWdlcy9bb3JnYW5pemF0aW9uU2x1Z10vW25hbWVzcGFjZV0vZ3JhcGgvW3NsdWddL3NjaGVtYS9zZGwudHN4) 0.00% <0.00%> (ø)
... and 1 more

... and 802 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
controlplane/src/core/repositories/FeatureFlagRepository.ts (1)

1488-1494: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Declare the explicit return type as Promise<Array<{ id: string; featureFlagId: string | null }> | undefined>. The database column permits null.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@controlplane/src/core/repositories/FeatureFlagRepository.ts` around lines
1488 - 1494, Add an explicit return type annotation to the
getFeatureFlagSchemaVersionsByBaseSchemaVersion method. The return type should
be Promise<Array<{ id: string; featureFlagId: string | null }> | undefined> to
properly account for the fact that the database column permits null values for
the featureFlagId field.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@controlplane/src/core/repositories/FeatureFlagRepository.ts`:
- Around line 1488-1494: Add an explicit return type annotation to the
getFeatureFlagSchemaVersionsByBaseSchemaVersion method. The return type should
be Promise<Array<{ id: string; featureFlagId: string | null }> | undefined> to
properly account for the fact that the database column permits null values for
the featureFlagId field.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 36fed3a3-7f8b-4340-bd59-771e59bab8db

📥 Commits

Reviewing files that changed from the base of the PR and between b72648a and c81127e.

📒 Files selected for processing (3)
  • controlplane/src/core/bufservices/federated-graph/getCompositionDetails.ts
  • controlplane/src/core/repositories/FeatureFlagRepository.ts
  • controlplane/test/composition/getCompositionDetails.test.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
controlplane/src/core/bufservices/federated-graph/getFederatedGraphById.ts (1)

76-89: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Filter out disabled feature flags from featureFlagsInLatestValidComposition.

This loop pushes any flag whose ID appears in ffsInLatestValidComposition, without checking flag.isEnabled. featureFlags includes disabled flags because getFeatureFlagsByFederatedGraph calls getFeatureFlagsByBaseSubgraphIdAndLabelMatchers with excludeDisabled: false.

The sibling handler getFeatureFlagsInLatestCompositionByFederatedGraph.ts filters the same repository result by flag.isEnabled and documents why: a disabled flag's router config is removed without recomposing, so its schema version rows still exist but should not be reported as part of the latest composition. Before this PR's fix, split-config mode likely returned no rows here, so this gap stayed hidden. Now that the fix makes the repository method return real rows under split config, disabled flags can appear in featureFlagsInLatestValidComposition here, inconsistent with the sibling handler.

Apply the same isEnabled check in this loop.

🐛 Proposed fix to exclude disabled flags
       if (ffsInLatestValidComposition) {
         for (const ff of ffsInLatestValidComposition) {
           const flag = featureFlags.find((f) => f.id === ff.featureFlagId);
-          if (flag) {
+          // A disabled feature flag is no longer served in the latest composition (its router config is
+          // removed without recomposing), so exclude it even though its schema version rows still exist.
+          if (flag && flag.isEnabled) {
             featureFlagsInLatestValidComposition.push(flag);
           }
         }
       }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@controlplane/src/core/bufservices/federated-graph/getFederatedGraphById.ts`
around lines 76 - 89, Update the loop in getFederatedGraphById that builds
featureFlagsInLatestValidComposition to append a matched flag only when it
exists and flag.isEnabled is true. Preserve the existing featureFlags lookup and
skip disabled flags consistently with
getFeatureFlagsInLatestCompositionByFederatedGraph.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@controlplane/src/core/bufservices/federated-graph/getFederatedGraphById.ts`:
- Around line 76-89: Update the loop in getFederatedGraphById that builds
featureFlagsInLatestValidComposition to append a matched flag only when it
exists and flag.isEnabled is true. Preserve the existing featureFlags lookup and
skip disabled flags consistently with
getFeatureFlagsInLatestCompositionByFederatedGraph.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6921879b-ba0f-445d-92ed-3df3a8fe331b

📥 Commits

Reviewing files that changed from the base of the PR and between c81127e and 5bb5a70.

📒 Files selected for processing (3)
  • controlplane/src/core/bufservices/feature-flag/getFeatureFlagsInLatestCompositionByFederatedGraph.ts
  • controlplane/src/core/bufservices/federated-graph/getFederatedGraphById.ts
  • controlplane/src/core/repositories/FeatureFlagRepository.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts`:
- Around line 340-342: Update the assertion to use the same comparator on both
sides of the sort. The resp.featureFlags array is sorted using the default
comparator, while the expected array is sorted using localeCompare, which can
produce different orders for mixed-case alphanumeric names. Either apply
localeCompare to both arrays or use an order-independent comparison approach
like Set equality to ensure consistent results.
🪄 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: 6adac01c-c789-4af2-9bbe-5925e09c4718

📥 Commits

Reviewing files that changed from the base of the PR and between 5bb5a70 and 516d932.

📒 Files selected for processing (1)
  • controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts

…for-a-federated-graph' and 'suvij/cosmo-315-controlplane-feature-flags-not-shown-for-a-federated-graph' of github.com:wundergraph/cosmo into suvij/cosmo-315-controlplane-feature-flags-not-shown-for-a-federated-graph
@JivusAyrus
JivusAyrus requested a review from a team as a code owner August 4, 2026 12:27

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

Tip: disable this comment in your organization's Code Review settings.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Router image scan passed

✅ No security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-e74f61af829b79adb2e4ace7971bfee678266d17

Comment thread studio/src/components/schema/stale-composition-warning.tsx Fixed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
proto/wg/cosmo/platform/v1/platform.proto (1)

2612-2622: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Add a doc comment for has_failed_latest_composition to fix the generated-file CI check.

This field has no comment in the .proto source. connect/src/wg/cosmo/platform/v1/platform_pb.ts carries a doc comment for the generated hasFailedLatestComposition field, but a generator cannot produce that comment from a source field with no comment. When CI regenerates the TypeScript file from this .proto file, the comment is missing, and the generated-file check fails in multiple pipelines.

Add the doc comment above has_failed_latest_composition in this file. Then regenerate platform_pb.ts and commit the exact output.

🛠️ Proposed fix
   string updated_at = 8;
+  // Whether this flag's latest composition failed, meaning the composition being reported for it is the
+  // last successful one rather than the latest. Only populated by RPCs that resolve a flag against a
+  // federated graph's composition (currently GetFeatureFlagsInLatestCompositionByFederatedGraph); left
+  // unset elsewhere, so treat unset as "not computed" rather than "not stale".
   optional bool has_failed_latest_composition = 9;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@proto/wg/cosmo/platform/v1/platform.proto` around lines 2612 - 2622, Add a
documentation comment immediately above the has_failed_latest_composition field
in the FeatureFlag message, matching the intended generated
hasFailedLatestComposition documentation; then regenerate platform_pb.ts so its
output includes that comment exactly.

Source: Pipeline failures

🧹 Nitpick comments (5)
controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts (1)

200-232: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider extracting the shared test setup.

The tests at Lines 200-343 and Lines 344-476 build the same fixture: namespace, users subgraph, products-standalone subgraph, a users-ff feature subgraph, a products-standalone-feature feature subgraph, and the federated graph. Only the flag names differ. A local helper that returns { namespace, labels, federatedGraphName } would remove about 60 duplicated lines and keep each test focused on its assertions.

Also applies to: 344-410

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts`
around lines 200 - 232, Extract the duplicated fixture creation from the tests
around the split-config cases into a local helper that creates the namespace,
all four subgraphs, and the federated graph, then returns namespace, labels, and
federatedGraphName. Update both tests to call this helper and retain only their
differing feature-flag names and assertions.
controlplane/src/core/repositories/FeatureFlagRepository.ts (3)

1534-1542: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider adding the organization scope to both queries.

Sibling queries in this file constrain schemaVersion.organizationId (see getLatestValidFlagCompositionClientSchema at Line 1608 and getLatestValidFeatureFlagSchemaVersion at Line 1706). The two queries here rely only on federatedGraphId, which the current callers resolve org-scoped. Adding the predicate keeps the scoping consistent and protects against a future caller that passes an unresolved id.

♻️ Proposed change
       .where(
         and(
           baseLinkageCondition,
+          eq(schemaVersion.organizationId, this.organizationId),
           isNotNull(federatedGraphsToFeatureFlagSchemaVersions.featureFlagId),

Apply the same predicate to the latestSchemaVersions query at Line 1568 so both sides of the comparison use the same scope.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@controlplane/src/core/repositories/FeatureFlagRepository.ts` around lines
1534 - 1542, Add the schema version organization scope predicate to both queries
in the relevant repository methods, including the `latestSchemaVersions` query
and the preceding composition query. Match the existing
`schemaVersion.organizationId` constraint used by
`getLatestValidFlagCompositionClientSchema` and
`getLatestValidFeatureFlagSchemaVersion`, ensuring both comparison sides remain
organization-scoped.

1499-1520: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Annotate baseLinkageCondition with an explicit type.

let baseLinkageCondition; has no type annotation and no initializer. TypeScript treats it as an evolving any, so a wrong condition type in either branch is not caught at compile time. Annotate it as SQL<unknown> | undefined.

♻️ Proposed annotation
-    let baseLinkageCondition;
+    let baseLinkageCondition: SQL<unknown> | undefined;

As per coding guidelines: "Avoid any type in TypeScript; use specific types or generics" and "Use explicit type annotations for function parameters and return types in TypeScript".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@controlplane/src/core/repositories/FeatureFlagRepository.ts` around lines
1499 - 1520, Add an explicit type annotation to the baseLinkageCondition
variable declaration. Change let baseLinkageCondition; to explicitly type it as
SQL<unknown> | undefined so that TypeScript can properly validate the condition
expressions assigned in both the if and else branches instead of treating the
variable as an evolving any type.

Source: Coding guidelines


1654-1669: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Extract the composition-validity predicate into a helper.

The same three conditions now appear four times in this file: Lines 1538-1540, Lines 1609-1611, Lines 1667-1669, and Lines 1710-1712. A single private helper keeps the definition of a "valid composition" in one place, so a future change (for example, a new failure column) cannot be applied inconsistently.

♻️ Proposed helper
private compositionSucceededConditions(): SQL<unknown>[] {
  return [
    eq(graphCompositions.isComposable, true),
    or(isNull(graphCompositions.deploymentError), eq(graphCompositions.deploymentError, ''))!,
    or(isNull(graphCompositions.admissionError), eq(graphCompositions.admissionError, ''))!,
  ];
}

Then spread it into each and(...).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@controlplane/src/core/repositories/FeatureFlagRepository.ts` around lines
1654 - 1669, Create a private helper method compositionSucceededConditions() in
the FeatureFlagRepository class that returns an array of the three SQL
conditions: eq(graphCompositions.isComposable, true),
or(isNull(graphCompositions.deploymentError),
eq(graphCompositions.deploymentError, '')), and
or(isNull(graphCompositions.admissionError),
eq(graphCompositions.admissionError, '')). Replace all four occurrences of these
three conditions at lines 1538-1540, 1609-1611, 1667-1669, and 1710-1712 by
spreading the helper method's return value into the and(...) calls.
studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/schema/index.tsx (1)

1252-1267: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Avoid fetching feature-flag composition status twice on this page.

GraphSelector already calls getFeatureFlagsInLatestCompositionByFederatedGraph at lines 885-894. SchemaExplorerPage calls the same RPC again here to compute activeFeatureFlagIsStale. GraphSelector renders inside this page's toolbar, so the page issues this query twice and duplicates the hasFailedLatestComposition mapping logic.

Compute activeFeatureFlagIsStale once and pass it to GraphSelector, or move the query into one shared location, so the page fetches this data once.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@studio/src/pages/`[organizationSlug]/[namespace]/graph/[slug]/schema/index.tsx
around lines 1252 - 1267, Remove the duplicate
getFeatureFlagsInLatestCompositionByFederatedGraph query from SchemaExplorerPage
and consolidate activeFeatureFlagIsStale computation with GraphSelector’s
existing query, passing the resolved value through the GraphSelector props or
another shared path. Preserve the feature-flag name matching and
hasFailedLatestComposition behavior while ensuring the page fetches composition
status only once.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@controlplane/test/feature-flag/get-federated-graph-sdl-by-name.test.ts`:
- Around line 33-35: Update the parameterized test title in the test.each block
to use neutral wording that accurately covers both splitConfigEnabled=true and
false outcomes, including the successful fallback and ERR_NOT_FOUND cases.

---

Outside diff comments:
In `@proto/wg/cosmo/platform/v1/platform.proto`:
- Around line 2612-2622: Add a documentation comment immediately above the
has_failed_latest_composition field in the FeatureFlag message, matching the
intended generated hasFailedLatestComposition documentation; then regenerate
platform_pb.ts so its output includes that comment exactly.

---

Nitpick comments:
In `@controlplane/src/core/repositories/FeatureFlagRepository.ts`:
- Around line 1534-1542: Add the schema version organization scope predicate to
both queries in the relevant repository methods, including the
`latestSchemaVersions` query and the preceding composition query. Match the
existing `schemaVersion.organizationId` constraint used by
`getLatestValidFlagCompositionClientSchema` and
`getLatestValidFeatureFlagSchemaVersion`, ensuring both comparison sides remain
organization-scoped.
- Around line 1499-1520: Add an explicit type annotation to the
baseLinkageCondition variable declaration. Change let baseLinkageCondition; to
explicitly type it as SQL<unknown> | undefined so that TypeScript can properly
validate the condition expressions assigned in both the if and else branches
instead of treating the variable as an evolving any type.
- Around line 1654-1669: Create a private helper method
compositionSucceededConditions() in the FeatureFlagRepository class that returns
an array of the three SQL conditions: eq(graphCompositions.isComposable, true),
or(isNull(graphCompositions.deploymentError),
eq(graphCompositions.deploymentError, '')), and
or(isNull(graphCompositions.admissionError),
eq(graphCompositions.admissionError, '')). Replace all four occurrences of these
three conditions at lines 1538-1540, 1609-1611, 1667-1669, and 1710-1712 by
spreading the helper method's return value into the and(...) calls.

In
`@controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts`:
- Around line 200-232: Extract the duplicated fixture creation from the tests
around the split-config cases into a local helper that creates the namespace,
all four subgraphs, and the federated graph, then returns namespace, labels, and
federatedGraphName. Update both tests to call this helper and retain only their
differing feature-flag names and assertions.

In
`@studio/src/pages/`[organizationSlug]/[namespace]/graph/[slug]/schema/index.tsx:
- Around line 1252-1267: Remove the duplicate
getFeatureFlagsInLatestCompositionByFederatedGraph query from SchemaExplorerPage
and consolidate activeFeatureFlagIsStale computation with GraphSelector’s
existing query, passing the resolved value through the GraphSelector props or
another shared path. Preserve the feature-flag name matching and
hasFailedLatestComposition behavior while ensuring the page fetches composition
status only once.
🪄 Autofix

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: 26d64e1b-3a05-4942-864f-a7fad6469d1e

📥 Commits

Reviewing files that changed from the base of the PR and between 516d932 and a2e8028.

📒 Files selected for processing (11)
  • connect/src/wg/cosmo/platform/v1/platform_pb.ts
  • controlplane/src/core/bufservices/feature-flag/getFeatureFlagsInLatestCompositionByFederatedGraph.ts
  • controlplane/src/core/bufservices/federated-graph/getFederatedGraphById.ts
  • controlplane/src/core/repositories/FeatureFlagRepository.ts
  • controlplane/src/types/index.ts
  • controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts
  • controlplane/test/feature-flag/get-federated-graph-sdl-by-name.test.ts
  • proto/wg/cosmo/platform/v1/platform.proto
  • studio/src/components/schema/stale-composition-warning.tsx
  • studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/schema/index.tsx
  • studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/schema/sdl.tsx

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

…ompositions link and update usage in schema pages
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

{errors && <CompositionErrorsDialog errors={errors} />}
{viewCompositionsHref && (
<Button variant="destructive" size="xs" asChild>
<Link href={viewCompositionsHref}>View compositions</Link>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arent the values still fromn the dom even in the suggested fix?

…r-a-federated-graph' of github.com:wundergraph/cosmo into suvij/cosmo-315-controlplane-feature-flags-not-shown-for-a-federated-graph
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants