feat: add support for config splitting - #2893
Merged
wilsonrivera merged 40 commits intoJul 1, 2026
Merged
Conversation
![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
|
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:
WalkthroughThis PR implements split-config-loading: controlplane includes feature IDs in router JWTs, CLI fetch utilities read that claim to optionally fetch a manifest and per-feature router configs, and CLI compose/fetch commands can write configs split by feature flag. ChangesSplit-config-loading feature implementation
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. Comment |
![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: 4
🧹 Nitpick comments (3)
controlplane/src/core/repositories/OrganizationRepository.ts (1)
1709-1720: ⚡ Quick winUse the matched feature ID instead of a hardcoded value.
getOrganizationGraphTokenFeaturespushes'split-config-loading'for every match, which will drift iffeaturesToSurfaceWithGraphTokengains additional IDs. Pushfeature.idand keep the return type aligned toFeatureIds[].Proposed fix
- async getOrganizationGraphTokenFeatures(organizationId: string): Promise<string[]> { - const features: string[] = []; + async getOrganizationGraphTokenFeatures(organizationId: string): Promise<FeatureIds[]> { + const features: FeatureIds[] = []; const orgFeatures = await this.getFeatures({ organizationId }); for (const feature of orgFeatures) { if (featuresToSurfaceWithGraphToken.includes(feature.id) && feature.enabled) { - features.push('split-config-loading'); + features.push(feature.id); } } return features; }🤖 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/OrganizationRepository.ts` around lines 1709 - 1720, The function getOrganizationGraphTokenFeatures currently hardcodes 'split-config-loading' into the features array; change it to push the matched feature.id instead and update the function signature/return type to Promise<FeatureIds[]> (or the appropriate FeatureIds type alias) so the method returns the actual Feature IDs from featuresToSurfaceWithGraphToken; update any imports or type references for FeatureIds and ensure the features array is typed as FeatureIds[] and you still filter by feature.enabled and membership in featuresToSurfaceWithGraphToken.cli/src/commands/router/utils.ts (2)
103-104: 💤 Low valueRemove orphan comment.
Line 103 contains an empty comment that appears to be leftover code.
🧹 Proposed fix
- // return result;🤖 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/src/commands/router/utils.ts` around lines 103 - 104, Remove the stray orphan comment ("//") immediately before the return statement: find the function or block that ends with "return result;" and delete the empty comment line so only the return remains; no behavioral changes are required beyond removing this leftover comment.
90-101: ⚡ Quick winConsider parallel fetching for feature flags.
Feature flag configs are fetched sequentially, which could be slow when there are many flags. Parallel fetching would improve performance.
⚡ Proposed parallel fetch
// Fetch the latest router configuration for each feature flag - result.featureFlags = new Map<string, string>(); - for (const [featureFlagName] of mapper) { - result.featureFlags.set( - featureFlagName, - await fetchFileContentFromCdn( - new URL(`manifest/feature-flags/${featureFlagName}.json`, baseUrl), - resp.token, - graphSignKey, - ), - ); - } + const featureFlagEntries = await Promise.all( + [...mapper.keys()].map(async (featureFlagName) => { + const content = await fetchFileContentFromCdn( + new URL(`manifest/feature-flags/${featureFlagName}.json`, baseUrl), + resp.token, + graphSignKey, + ); + return [featureFlagName, content] as const; + }), + ); + result.featureFlags = new Map(featureFlagEntries);🤖 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/src/commands/router/utils.ts` around lines 90 - 101, The loop that populates result.featureFlags iterates over mapper entries sequentially causing slow IO; change it to fire all fetchFileContentFromCdn calls in parallel (use Promise.all over Array.from(mapper) or [...mapper]) and then populate result.featureFlags with the resolved contents while preserving keys; keep using the same arguments (new URL(`manifest/feature-flags/${featureFlagName}.json`, baseUrl), resp.token, graphSignKey) and ensure result.featureFlags is a Map<string,string> filled after the promises resolve.
🤖 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/graph/federated-graph/commands/fetch.ts`:
- Around line 60-61: The loop writing feature flag files uses untrusted
featureFlagName in the filesystem path (routerConfig.featureFlags,
featureFlagName, writeFileSync, featureFlagsPath); sanitize the name before
building the filename by rejecting or normalizing unsafe characters (strip path
separators and traversal tokens like "../", or take path.basename) and/or
replace non-alphanumerics with a safe token (e.g., hyphen) and validate
non-empty, then use the sanitized name when calling
writeFileSync(join(featureFlagsPath, `${sanitizedName}.json`)) to ensure no path
escape or unexpected overwrites occur.
In `@cli/src/commands/router/commands/compose.ts`:
- Around line 192-197: The current logic always mkdirs options.out
(resolve/existsSync/mkdir), turning a file path into a directory and causing
EISDIR when writeFile later writes that path; change it so that when running in
split mode (the flag that causes multiple files to be emitted) you create
options.out as a directory, but in non‑split mode do not mkdir the final
path—instead ensure the parent directory exists by calling
mkdir(path.dirname(options.out), { recursive: true }) before writeFile; apply
this fix to both occurrences handling options.out (the block using
resolve/existsSync/mkdir and the similar block at the later location).
- Around line 277-295: When options.splitConfigsEnabled is true but options.out
is not provided, feature-flag configs (ffConfigs) are currently dropped — update
the compose logic in compose.ts (the block handling options.splitConfigsEnabled,
ffConfigs, routerConfig.featureFlagConfigs and writing files to outDir) to
detect this case and fail fast: if options.splitConfigsEnabled && !options.out,
throw or return an explicit error (or process exit) explaining that --out is
required in split mode so feature-flag configs won’t be lost; alternatively, if
you prefer automatic behavior, embed ffConfigs into
routerConfig.featureFlagConfigs when options.out is omitted. Ensure the change
affects the conditional around ffConfigs.configByFeatureFlagName and the code
path that creates outDir/writeFile so the missing-out case is handled
deterministically.
In `@cli/src/commands/router/commands/fetch.ts`:
- Around line 65-67: There’s a duplicate signature verification log:
handleOutput already prints the signature message, so remove the extra
console.log inside the fetch command’s options.graphSignKey conditional (the
block that prints pc.green('The signature of the router config matches the local
computed signature.')); keep the handleOutput implementation as the single
source of truth for this message and delete the redundant conditional/log in the
fetch command.
---
Nitpick comments:
In `@cli/src/commands/router/utils.ts`:
- Around line 103-104: Remove the stray orphan comment ("//") immediately before
the return statement: find the function or block that ends with "return result;"
and delete the empty comment line so only the return remains; no behavioral
changes are required beyond removing this leftover comment.
- Around line 90-101: The loop that populates result.featureFlags iterates over
mapper entries sequentially causing slow IO; change it to fire all
fetchFileContentFromCdn calls in parallel (use Promise.all over
Array.from(mapper) or [...mapper]) and then populate result.featureFlags with
the resolved contents while preserving keys; keep using the same arguments (new
URL(`manifest/feature-flags/${featureFlagName}.json`, baseUrl), resp.token,
graphSignKey) and ensure result.featureFlags is a Map<string,string> filled
after the promises resolve.
In `@controlplane/src/core/repositories/OrganizationRepository.ts`:
- Around line 1709-1720: The function getOrganizationGraphTokenFeatures
currently hardcodes 'split-config-loading' into the features array; change it to
push the matched feature.id instead and update the function signature/return
type to Promise<FeatureIds[]> (or the appropriate FeatureIds type alias) so the
method returns the actual Feature IDs from featuresToSurfaceWithGraphToken;
update any imports or type references for FeatureIds and ensure the features
array is typed as FeatureIds[] and you still filter by feature.enabled and
membership in featuresToSurfaceWithGraphToken.
🪄 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: 28b22444-4fb3-4510-a057-c65dce6da9f5
📒 Files selected for processing (11)
cli/src/commands/auth/utils.tscli/src/commands/graph/federated-graph/commands/fetch.tscli/src/commands/router/commands/compose.tscli/src/commands/router/commands/fetch.tscli/src/commands/router/utils.tscontrolplane/src/core/bufservices/federated-graph/createFederatedGraphToken.tscontrolplane/src/core/bufservices/federated-graph/generateRouterToken.tscontrolplane/src/core/constants.tscontrolplane/src/core/repositories/FederatedGraphRepository.tscontrolplane/src/core/repositories/OrganizationRepository.tscontrolplane/src/core/services/CompositionService.ts
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (74.81%) 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 #2893 +/- ##
==========================================
- Coverage 61.38% 60.92% -0.46%
==========================================
Files 259 482 +223
Lines 30054 62700 +32646
Branches 0 6237 +6237
==========================================
+ Hits 18449 38201 +19752
- Misses 10119 24471 +14352
+ Partials 1486 28 -1458
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/src/commands/router/commands/compose.ts (1)
298-305:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLog the actual written file path in split mode.
When
--split-configs-enabledis set, the file is written torouter-config.json, but the success log prints only the output directory path.Suggested patch
- if (options.out) { - await writeFile( - options.splitConfigsEnabled ? join(options.out, 'router-config.json') : options.out, - routerConfig.toJsonString(), - ); - - console.log(pc.green(`Router config successfully written to ${pc.bold(options.out)}`)); + if (options.out) { + const outputPath = options.splitConfigsEnabled ? join(options.out, 'router-config.json') : options.out; + await writeFile(outputPath, routerConfig.toJsonString()); + + console.log(pc.green(`Router config successfully written to ${pc.bold(outputPath)}`)); } else {🤖 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/src/commands/router/commands/compose.ts` around lines 298 - 305, When writing the router config handle the split-configs-enabled branch by computing the actual output path into a variable (e.g., writtenPath = options.splitConfigsEnabled ? join(options.out, 'router-config.json') : options.out), use that variable in the writeFile call (instead of recomputing), and change the console.log to print the actual writtenPath (pc.bold(writtenPath)) so the success message shows the real file location when options.splitConfigsEnabled is true; refer to options.out, options.splitConfigsEnabled, the writeFile call and the console.log currently printing pc.bold(options.out).
🤖 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 `@cli/src/commands/router/commands/compose.ts`:
- Around line 298-305: When writing the router config handle the
split-configs-enabled branch by computing the actual output path into a variable
(e.g., writtenPath = options.splitConfigsEnabled ? join(options.out,
'router-config.json') : options.out), use that variable in the writeFile call
(instead of recomputing), and change the console.log to print the actual
writtenPath (pc.bold(writtenPath)) so the success message shows the real file
location when options.splitConfigsEnabled is true; refer to options.out,
options.splitConfigsEnabled, the writeFile call and the console.log currently
printing pc.bold(options.out).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: caa61f0b-291e-451e-bf39-6386196d55ee
📒 Files selected for processing (1)
cli/src/commands/router/commands/compose.ts
Aenimus
reviewed
May 26, 2026
Aenimus
reviewed
May 26, 2026
Aenimus
reviewed
May 26, 2026
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
cli/src/commands/router/utils.ts (1)
67-67:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVerify mapper integrity with the same signature key path.
At Line 67,
manifest/mapper.jsonis fetched withoutgraphSignKey, so mapper tampering is not detected even when signature verification is enabled for config files.Suggested fix
- const mapperTextContent = await fetchFileContentFromCdn(new URL('manifest/mapper.json', baseUrl), resp.token); + const mapperTextContent = await fetchFileContentFromCdn( + new URL('manifest/mapper.json', baseUrl), + resp.token, + graphSignKey, + );🤖 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/src/commands/router/utils.ts` at line 67, The manifest/mapper.json is fetched without using the same signature key, so tampering isn't detected; update the fetch call that sets mapperTextContent (the line calling fetchFileContentFromCdn with new URL('manifest/mapper.json', baseUrl) and resp.token) to use the same graphSignKey path used for config files — i.e., pass the graphSignKey (or call the same signed-fetch helper) into fetchFileContentFromCdn (or replace with the signature-verified fetch used for configs) so mapper.json is verified with the same key as other config artifacts.cli/src/commands/router/commands/fetch.ts (1)
26-28:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSanitize feature flag names before writing files.
At Line 27,
featureFlagNameis used directly in a path. Reject unsafe names (path separators / traversal) beforewriteFileto prevent writing outsidefeature-flags/.Suggested fix
+const SAFE_FEATURE_FLAG_NAME = /^[A-Za-z0-9_-]+$/; + for (const [featureFlagName, featureFlagRouterConfig] of config.featureFlags) { - await writeFile(resolve(directory, `${featureFlagName}.json`), featureFlagRouterConfig); + if (!SAFE_FEATURE_FLAG_NAME.test(featureFlagName)) { + throw new Error(`Invalid feature flag name: ${featureFlagName}`); + } + await writeFile(resolve(directory, `${featureFlagName}.json`), featureFlagRouterConfig); }🤖 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/src/commands/router/commands/fetch.ts` around lines 26 - 28, Sanitize the feature flag names before using them in the path: inside the loop that iterates config.featureFlags, validate featureFlagName (the variable used with resolve, writeFile and directory) to reject any names containing path separators or traversal segments (e.g., '/' '\' or '..') or any characters you don't allow; if a name fails validation, skip or throw/log an error instead of calling writeFile; alternatively normalize to a safe basename (using path.basename semantics) and then call resolve(directory, `${safeName}.json`) with writeFile. Ensure this check is applied to the loop that references config.featureFlags, featureFlagName, resolve and writeFile so no file can be written outside the feature-flags directory.cli/src/commands/router/commands/compose.ts (1)
289-299:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate feature flag names before using them as filenames.
At Line 298,
featureFlagNameis written directly into a file path. Add a strict allowlist check and fail fast on invalid names to avoid traversal/overwrite risk.Suggested fix
+const SAFE_FEATURE_FLAG_NAME = /^[A-Za-z0-9_-]+$/; + for (const [featureFlagName, featureFlagConfig] of Object.entries(ffConfigs.configByFeatureFlagName)) { + if (!SAFE_FEATURE_FLAG_NAME.test(featureFlagName)) { + program.error(`Invalid feature flag name: ${featureFlagName}`); + } const ffRouterConfig = new RouterConfig({ engineConfig: featureFlagConfig.engineConfig, version: featureFlagConfig.version, subgraphs: featureFlagConfig.subgraphs, compatibilityVersion: routerConfig.compatibilityVersion, });🤖 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/src/commands/router/commands/compose.ts` around lines 289 - 299, featureFlagName from ffConfigs.configByFeatureFlagName is used directly in join(outDir, `${featureFlagName}.json`) which allows path traversal or unsafe filenames; before creating ffRouterConfig and writing the file, validate featureFlagName against a strict allowlist/regex (e.g., only lowercase letters, numbers, dashes/underscores) and throw/reject if it fails, then proceed to call writeFile, createHash, and mapper.set only for valid names; reference the loop over ffConfigs.configByFeatureFlagName, the featureFlagName variable, the writeFile(join(...)) invocation, and mapper.set(...) when adding the validation and failing fast on invalid names.
🤖 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 `@cli/src/commands/router/commands/compose.ts`:
- Around line 289-299: featureFlagName from ffConfigs.configByFeatureFlagName is
used directly in join(outDir, `${featureFlagName}.json`) which allows path
traversal or unsafe filenames; before creating ffRouterConfig and writing the
file, validate featureFlagName against a strict allowlist/regex (e.g., only
lowercase letters, numbers, dashes/underscores) and throw/reject if it fails,
then proceed to call writeFile, createHash, and mapper.set only for valid names;
reference the loop over ffConfigs.configByFeatureFlagName, the featureFlagName
variable, the writeFile(join(...)) invocation, and mapper.set(...) when adding
the validation and failing fast on invalid names.
In `@cli/src/commands/router/commands/fetch.ts`:
- Around line 26-28: Sanitize the feature flag names before using them in the
path: inside the loop that iterates config.featureFlags, validate
featureFlagName (the variable used with resolve, writeFile and directory) to
reject any names containing path separators or traversal segments (e.g., '/' '\'
or '..') or any characters you don't allow; if a name fails validation, skip or
throw/log an error instead of calling writeFile; alternatively normalize to a
safe basename (using path.basename semantics) and then call resolve(directory,
`${safeName}.json`) with writeFile. Ensure this check is applied to the loop
that references config.featureFlags, featureFlagName, resolve and writeFile so
no file can be written outside the feature-flags directory.
In `@cli/src/commands/router/utils.ts`:
- Line 67: The manifest/mapper.json is fetched without using the same signature
key, so tampering isn't detected; update the fetch call that sets
mapperTextContent (the line calling fetchFileContentFromCdn with new
URL('manifest/mapper.json', baseUrl) and resp.token) to use the same
graphSignKey path used for config files — i.e., pass the graphSignKey (or call
the same signed-fetch helper) into fetchFileContentFromCdn (or replace with the
signature-verified fetch used for configs) so mapper.json is verified with the
same key as other config artifacts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1f97832f-8cdb-4616-871f-b0cded053d52
📒 Files selected for processing (4)
cli/src/commands/graph/federated-graph/commands/fetch.tscli/src/commands/router/commands/compose.tscli/src/commands/router/commands/fetch.tscli/src/commands/router/utils.ts
Aenimus
reviewed
May 28, 2026
Aenimus
reviewed
May 28, 2026
Aenimus
reviewed
May 28, 2026
Aenimus
reviewed
May 28, 2026
Aenimus
reviewed
May 28, 2026
Aenimus
reviewed
May 28, 2026
Contributor
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
…h-router-compose-and-router
…h-router-compose-and-router
Aenimus
reviewed
Jun 18, 2026
…h-router-compose-and-router
…h-router-compose-and-router
…h-router-compose-and-router
wilsonrivera
deleted the
wilson/eng-9577-cli-wgc-federated-graph-fetch-router-compose-and-router
branch
July 1, 2026 17:27
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
2 participants
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
New Features
Improvements
Documentation
Checklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.