feat: add feature flag recomposition support#2853
Open
wilsonrivera wants to merge 2 commits into
Open
Conversation
wilsonrivera
requested review from
Aenimus,
JivusAyrus,
StarpTech,
endigma,
jensneuse and
thisisnithin
as code owners
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds RecomposeFeatureFlag end-to-end: proto messages and RPC, generated client bindings, server handler with validation and composition/deploy logic, router wiring and audit action, CLI command, and server/CLI tests. ChangesFeature Flag Recomposition Workflow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
5 tasks
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2853 +/- ##
===========================================
- Coverage 65.01% 40.93% -24.08%
===========================================
Files 573 1026 +453
Lines 71938 129306 +57368
Branches 4862 5997 +1135
===========================================
+ Hits 46767 52927 +6160
- Misses 23724 74635 +50911
- Partials 1447 1744 +297
🚀 New features to boost your workflow:
|
Router image scan passed✅ No security vulnerabilities found in image: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cli/test/feature-flag/recompose.test.ts (1)
52-60: ⚡ Quick winTighten thrown-error assertions to avoid false positives
At Line 53 and Line 67,
.rejects.toThrow()is too broad. Because Line 12 mocksprocess.exitto throw'process.exit', these tests can pass even if the command exits unexpectedly. Assert the expected error message/details (or assertprocess.exitwas not called) so these cases validate the intended failure mode.✅ Suggested test hardening
test('that recompose fails fails with exit code 1 if config splitting is not enabled', async () => { await expect( runRecompose({ response: { code: EnumStatusCode.ERR, details: 'Configuration splitting not enabled' }, compositionErrors: [], compositionWarnings: [], deploymentErrors: [], }), - ).rejects.toThrow(); + ).rejects.toThrow('Configuration splitting not enabled'); expect(process.exitCode).toBeUndefined(); }); test('that recompose fails but does not return exit code 1 if the feature flag is not found', async () => { await expect( runRecompose({ response: { code: EnumStatusCode.ERR_NOT_FOUND, details: 'The feature flag "feature-flag" was not found' }, compositionErrors: [], compositionWarnings: [], deploymentErrors: [], }), - ).rejects.toThrow(); + ).rejects.toThrow('was not found'); expect(process.exitCode).toBeUndefined(); });Also applies to: 65-73
🤖 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 `@cli/test/feature-flag/recompose.test.ts` around lines 52 - 60, The test using runRecompose currently uses .rejects.toThrow() which is too broad and can pass due to the mocked process.exit throwing 'process.exit'; update the assertions to verify the specific failure from runRecompose (e.g., .rejects.toThrow('Configuration splitting not enabled') or similar exact message coming from runRecompose) and/or assert that the mocked process.exit was not called; locate the failing tests by the names "that recompose fails fails with exit code 1 if config splitting is not enabled" and the other case around lines 65–73 and update their expectations to check the exact error message or add an assertion on the process.exit mock instead of a generic .toThrow().
🤖 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 `@cli/src/commands/feature-flag/commands/recompose.ts`:
- Around line 44-55: Wrap the call to
opts.client.platform.recomposeFeatureFlag(...) in a try/catch to prevent
unhandled promise rejections: call recomposeFeatureFlag (the RPC) inside the
try, on success stop the spinner with spinner.succeed(...) and proceed, and in
the catch call spinner.fail(...) with an informative message including the
caught error, then surface/exit appropriately (e.g., throw or process.exit with
non-zero). Ensure you still pass the same payload
(disableResolvabilityValidation, limit, name, namespace) and headers from
getBaseHeaders(), and avoid leaving the spinner running on transport/protocol
failures.
In `@controlplane/test/feature-flag/recompose-feature-flag.test.ts`:
- Around line 18-23: The vi.mock call is targeting the wrong module specifier so
the real ClickHouse implementation can leak into tests; update the string
argument passed to vi.mock so it exactly matches the module specifier used by
the test's import (the same relative path the test imports), and keep the mocked
exports as shown (mock ClickHouseClient and its prototype.queryPromise) so
ClickHouseClient and queryPromise are used by the test instead of the real
module.
---
Nitpick comments:
In `@cli/test/feature-flag/recompose.test.ts`:
- Around line 52-60: The test using runRecompose currently uses
.rejects.toThrow() which is too broad and can pass due to the mocked
process.exit throwing 'process.exit'; update the assertions to verify the
specific failure from runRecompose (e.g., .rejects.toThrow('Configuration
splitting not enabled') or similar exact message coming from runRecompose)
and/or assert that the mocked process.exit was not called; locate the failing
tests by the names "that recompose fails fails with exit code 1 if config
splitting is not enabled" and the other case around lines 65–73 and update their
expectations to check the exact error message or add an assertion on the
process.exit mock instead of a generic .toThrow().
🪄 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: dd7db1d7-5d85-424e-abf1-7f4defbb79ff
⛔ Files ignored due to path filters (2)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.gois excluded by!**/*.pb.go,!**/gen/**connect-go/gen/proto/wg/cosmo/platform/v1/platformv1connect/platform.connect.gois excluded by!**/gen/**
📒 Files selected for processing (12)
cli/src/commands/feature-flag/commands/recompose.tscli/src/commands/feature-flag/index.tscli/test/feature-flag/recompose.test.tscli/test/feature-flag/utils.tsconnect/src/wg/cosmo/platform/v1/platform-PlatformService_connectquery.tsconnect/src/wg/cosmo/platform/v1/platform_connect.tsconnect/src/wg/cosmo/platform/v1/platform_pb.tscontrolplane/src/core/bufservices/PlatformService.tscontrolplane/src/core/bufservices/feature-flag/recomposeFeatureFlag.tscontrolplane/src/db/models.tscontrolplane/test/feature-flag/recompose-feature-flag.test.tsproto/wg/cosmo/platform/v1/platform.proto
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Checklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.
The goal of this PR is to introduce support to recompose specific feature flags the same way
federated-graph recomposeandmonograph recomposeworks