feat(cli): add subgraph batch-publish command - #2899
Conversation
Add 'wgc subgraph batch-publish' to publish many existing subgraphs and feature subgraphs from a single config file via a new PublishFederatedSubgraphs RPC. All schema versions are written first, then each affected federated graph (and its contracts/feature flags) is composed exactly once instead of once per subgraph. Closes ENG-9668
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.
|
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 adds a batch-publish RPC and CLI to publish multiple existing subgraphs in one request, including proto contract and generated bindings, a backend handler that validates and persists schemas then composes/deploys affected graphs, a repository refactor for batch updates, CLI wiring to load config files, and comprehensive integration tests. ChangesBatch Publish Subgraphs Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2899 +/- ##
===========================================
- Coverage 66.26% 41.10% -25.17%
===========================================
Files 258 1045 +787
Lines 27309 132873 +105564
Branches 0 6338 +6338
===========================================
+ Hits 18097 54613 +36516
- Misses 7773 76475 +68702
- Partials 1439 1785 +346
🚀 New features to boost your workflow:
|
Router image scan passed✅ No security vulnerabilities found in image: |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
controlplane/src/core/repositories/SubgraphRepository.ts (1)
88-102: ⚡ Quick winUse an
interfacefor this exported payload.
UpdateSubgraphSchemaDatais an exported object shape, so it should follow the repo convention and be declared as aninterfaceinstead of atypealias.As per coding guidelines, `**/*.{ts,tsx}`: Prefer interfaces over type aliases for object shapes in TypeScript.♻️ Suggested change
-export type UpdateSubgraphSchemaData = { +export interface UpdateSubgraphSchemaData { targetId: string; labels: Label[]; updatedBy: string; namespaceId: string; unsetLabels: boolean; @@ isV2Graph?: boolean; readme?: string; proto?: ProtoSubgraph; -}; +}🤖 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/SubgraphRepository.ts` around lines 88 - 102, The exported object shape UpdateSubgraphSchemaData should be declared as an interface rather than a type alias to follow repository convention; change the declaration of UpdateSubgraphSchemaData from a "type" to an "interface" while preserving all properties (targetId, labels: Label[], updatedBy, namespaceId, unsetLabels, routingUrl, schemaSDL, subscriptionUrl, subscriptionProtocol: SubscriptionProtocol, websocketSubprotocol: WebsocketSubprotocol, isV2Graph, readme, proto: ProtoSubgraph) and keep all existing optional markers and imports intact so consumers of UpdateSubgraphSchemaData behave the same.cli/src/commands/subgraph/commands/batch-publish.ts (2)
35-36: ⚡ Quick winAdd explicit types for function boundaries in this command.
Line 69’s
optionsparameter is implicit; add explicit types (and return type on the exported factory) for safer maintenance.Suggested change
+type BatchPublishCommandOptions = { + config: string; + namespace?: string; + failOnCompositionError?: boolean; + failOnAdmissionWebhookError?: boolean; + suppressWarnings?: boolean; + disableResolvabilityValidation?: boolean; + limit: string; +}; + -export default (opts: BaseCommandOptions) => { +export default (opts: BaseCommandOptions): Command => { const command = new Command('batch-publish'); ... - command.action(async (options) => { + command.action(async (options: BatchPublishCommandOptions): Promise<void> => {As per coding guidelines:
Use explicit type annotations for function parameters and return types in TypeScript.Also applies to: 69-69
🤖 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/subgraph/commands/batch-publish.ts` around lines 35 - 36, The exported factory function currently has implicit function boundary types; add explicit TypeScript annotations: annotate the exported default function's parameter as (opts: BaseCommandOptions) and its return type as Command (or the specific Command type you use), and add an explicit type for the inner handler's options parameter (the `options` argument referenced on line 69) e.g. (options: YourOptionsType) — update the signature for the command action/handler where `options` is declared and ensure any custom option interface is defined/imported so all function parameters and the factory's return type are explicitly typed.
38-42: ⚡ Quick winUse a template literal for the multi-line description.
This keeps the string easier to read and follows the project style rule.
Suggested change
- command.description( - 'Publishes the schemas of multiple subgraphs at once using a config file.\n' + - 'All subgraphs and feature subgraphs listed in the config must already exist.\n' + - 'If the publication leads to composition errors, the errors will be visible in the Studio.\n' + - 'The router will continue to work with the latest valid schema.', - ); + command.description(`Publishes the schemas of multiple subgraphs at once using a config file. +All subgraphs and feature subgraphs listed in the config must already exist. +If the publication leads to composition errors, the errors will be visible in the Studio. +The router will continue to work with the latest valid schema.`);As per coding guidelines:
Use template literals instead of string concatenation.🤖 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/subgraph/commands/batch-publish.ts` around lines 38 - 42, Replace the concatenated multi-line string used for the command description with a single template literal: find the description passed to the command (the current concatenated string starting with 'Publishes the schemas of multiple subgraphs at once using a config file.\n' + ...) and rewrite it as a backtick (`) template literal preserving the same text and newlines; update the call to description(...) or the variable holding this text (in batch-publish command) to use the template literal for readability and to follow the project's style rule.controlplane/test/subgraph/batch-publish-subgraphs.test.ts (1)
32-36: ⚡ Quick winTighten test typings (
any+ implicit callback params).Use concrete/inferred client type and explicitly type callback params to keep test helpers type-safe.
Suggested change
const getCompositionCount = async ( - client: any, + client: Awaited<ReturnType<typeof SetupTest>>['client'], fedGraphName: string, - namespace = DEFAULT_NAMESPACE, + namespace: string = DEFAULT_NAMESPACE, ): Promise<number> => { ... - async (role) => { + async (role: string) => { ... - async (role) => { + async (role: string) => {As per coding guidelines:
Use explicit type annotations for function parameters and return types in TypeScriptandAvoid any type in TypeScript; use specific types or generics.Also applies to: 474-475, 509-510
🤖 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/subgraph/batch-publish-subgraphs.test.ts` around lines 32 - 36, Replace the liberal use of any and implicit callback params in the test helpers with concrete types: change getCompositionCount(client: any, fedGraphName: string, namespace = DEFAULT_NAMESPACE): Promise<number> to use the actual GraphQL/Apollo client type used in these tests (e.g., ApolloClient<NormalizedCacheObject> or the project's test client type) and explicitly type any callback parameters passed to client methods (e.g., response, err) so the helper is fully typed; apply the same fix to the other helper functions referenced in the comment (the helpers around lines noted 474-475 and 509-510) so all test helper signatures and their internal callbacks use explicit, specific types rather than any.
🤖 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/subgraph/commands/batch-publish.ts`:
- Around line 112-133: The readEntries mapping should wrap the per-entry file
read/decoding in a try/catch so I/O or decode errors produce a clear
command-scoped message instead of bubbling raw exceptions; inside the async map
for readEntries (handling BatchPublishEntry), catch errors around
readFile/TextDecoder.decode for schemaFile and call program.error with a
descriptive message that includes entry.name, schemaFile and the caught
error.message (or stack) so the CLI fails with context; ensure existing checks
for existsSync and empty schema remain, and return the { name, schema } only on
success.
- Around line 83-85: The catch block currently uses `catch (e: any)` and
accesses `e.message`; change it to `catch (e: unknown)` and narrow the error
before using its message when calling `program.error(pc.red(pc.bold(...)))`:
determine the message via an `instanceof Error` check (or `typeof` fallback) and
use that safe `message` string (or `String(e)`) so you don't assume shape of `e`
while still including `configFile` and the error text in the `program.error`
call.
In `@controlplane/src/core/bufservices/subgraph/publishFederatedSubgraphs.ts`:
- Around line 218-220: In the catch block inside publishFederatedSubgraphs
(where schemaErrors is pushed), change the clause from catch (e: any) to catch
(error: unknown) and narrow the value before accessing .message: check if error
is an instance of Error and use error.message, otherwise coerce to a safe string
(e.g., String(error) or a default) when building the string `Subgraph
"${subgraph.name}": ...` so you never assume properties on an unknown value.
In `@controlplane/src/core/repositories/SubgraphRepository.ts`:
- Around line 391-394: The code currently returns early with subgraph undefined
when subgraphRepo.byTargetId(data.targetId) misses, which allows batchUpdate to
continue and commit partial work; replace this silent return with throwing a
descriptive error (including the targetId) so batchUpdate aborts the entire
operation. Locate the lookup call to subgraphRepo.byTargetId(...) in
SubgraphRepository (the block that currently returns { subgraph: undefined,
affectedFederatedGraphById, affectedFeatureFlagIds, subgraphChanged,
labelChanged }) and instead throw an Error (or a domain-specific exception)
indicating the subgraph for that targetId was not found so callers like
batchUpdate will stop and not commit a partial batch.
- Around line 397-404: The current condition only forces addSchemaVersion when
schemaSDL changed or the subgraph is a grpc_plugin, which drops proto-only
updates for grpc_service; update the guard around addSchemaVersion (referencing
data.schemaSDL, subgraph.type, subgraph.schemaSDL, addSchemaVersion) to also
call addSchemaVersion for subgraph.type === 'grpc_service' when the
proto/mappings/lock payload has changed — e.g., detect changes by comparing
data.proto, data.mappings, or data.lock against subgraph.proto,
subgraph.mappings, subgraph.lock and treat any difference as subgraphChanged so
addSchemaVersion is invoked with the new proto/mappings/lock.
---
Nitpick comments:
In `@cli/src/commands/subgraph/commands/batch-publish.ts`:
- Around line 35-36: The exported factory function currently has implicit
function boundary types; add explicit TypeScript annotations: annotate the
exported default function's parameter as (opts: BaseCommandOptions) and its
return type as Command (or the specific Command type you use), and add an
explicit type for the inner handler's options parameter (the `options` argument
referenced on line 69) e.g. (options: YourOptionsType) — update the signature
for the command action/handler where `options` is declared and ensure any custom
option interface is defined/imported so all function parameters and the
factory's return type are explicitly typed.
- Around line 38-42: Replace the concatenated multi-line string used for the
command description with a single template literal: find the description passed
to the command (the current concatenated string starting with 'Publishes the
schemas of multiple subgraphs at once using a config file.\n' + ...) and rewrite
it as a backtick (`) template literal preserving the same text and newlines;
update the call to description(...) or the variable holding this text (in
batch-publish command) to use the template literal for readability and to follow
the project's style rule.
In `@controlplane/src/core/repositories/SubgraphRepository.ts`:
- Around line 88-102: The exported object shape UpdateSubgraphSchemaData should
be declared as an interface rather than a type alias to follow repository
convention; change the declaration of UpdateSubgraphSchemaData from a "type" to
an "interface" while preserving all properties (targetId, labels: Label[],
updatedBy, namespaceId, unsetLabels, routingUrl, schemaSDL, subscriptionUrl,
subscriptionProtocol: SubscriptionProtocol, websocketSubprotocol:
WebsocketSubprotocol, isV2Graph, readme, proto: ProtoSubgraph) and keep all
existing optional markers and imports intact so consumers of
UpdateSubgraphSchemaData behave the same.
In `@controlplane/test/subgraph/batch-publish-subgraphs.test.ts`:
- Around line 32-36: Replace the liberal use of any and implicit callback params
in the test helpers with concrete types: change getCompositionCount(client: any,
fedGraphName: string, namespace = DEFAULT_NAMESPACE): Promise<number> to use the
actual GraphQL/Apollo client type used in these tests (e.g.,
ApolloClient<NormalizedCacheObject> or the project's test client type) and
explicitly type any callback parameters passed to client methods (e.g.,
response, err) so the helper is fully typed; apply the same fix to the other
helper functions referenced in the comment (the helpers around lines noted
474-475 and 509-510) so all test helper signatures and their internal callbacks
use explicit, specific types rather than any.
🪄 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: c696e707-47d6-4f56-855d-ddcfa583854c
⛔ 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 (10)
cli/src/commands/subgraph/commands/batch-publish.tscli/src/commands/subgraph/index.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/subgraph/publishFederatedSubgraphs.tscontrolplane/src/core/repositories/SubgraphRepository.tscontrolplane/test/subgraph/batch-publish-subgraphs.test.tsproto/wg/cosmo/platform/v1/platform.proto
Drop the separate feature_subgraphs section from the batch publish request and config. Regular subgraphs and feature subgraphs cannot share a name within a namespace, so each entry's kind is resolved from the control plane instead. A mixed regular + feature subgraph batch is covered by tests.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
controlplane/test/subgraph/batch-publish-subgraphs.test.ts (1)
30-43: 💤 Low valueConsider typing the
clientparameter instead ofany.The
clientparameter is typed asany, which bypasses type checking. Consider using the actual client type for better type safety.const getCompositionCount = async ( - client: any, + client: PromiseClient<typeof PlatformService>, fedGraphName: string, namespace = DEFAULT_NAMESPACE, ): Promise<number> => {This would require importing
PromiseClientandPlatformServicefrom the appropriate modules.🤖 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/subgraph/batch-publish-subgraphs.test.ts` around lines 30 - 43, The helper getCompositionCount currently types its client parameter as any; change its signature to accept the concrete gRPC client type by importing and using PromiseClient from the PlatformService (e.g., PromiseClient<PlatformService> or the specific generated PromiseClient type) so client.getCompositions is type-checked; update the import statements to pull PromiseClient and PlatformService from the generated proto/SDK modules and replace the any in getCompositionCount(client: any, ...) with the proper PromiseClient<PlatformService> type.
🤖 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/subgraph/batch-publish-subgraphs.test.ts`:
- Around line 30-43: The helper getCompositionCount currently types its client
parameter as any; change its signature to accept the concrete gRPC client type
by importing and using PromiseClient from the PlatformService (e.g.,
PromiseClient<PlatformService> or the specific generated PromiseClient type) so
client.getCompositions is type-checked; update the import statements to pull
PromiseClient and PlatformService from the generated proto/SDK modules and
replace the any in getCompositionCount(client: any, ...) with the proper
PromiseClient<PlatformService> type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5d6a8166-25d2-4a5f-a522-89c6cd3f21b0
⛔ Files ignored due to path filters (1)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.gois excluded by!**/*.pb.go,!**/gen/**
📒 Files selected for processing (5)
cli/src/commands/subgraph/commands/batch-publish.tsconnect/src/wg/cosmo/platform/v1/platform_pb.tscontrolplane/src/core/bufservices/subgraph/publishFederatedSubgraphs.tscontrolplane/test/subgraph/batch-publish-subgraphs.test.tsproto/wg/cosmo/platform/v1/platform.proto
✅ Files skipped from review due to trivial changes (1)
- connect/src/wg/cosmo/platform/v1/platform_pb.ts
Split batchUpdate into batchWriteAndCollect (a short transaction that writes all schema versions and returns the deduplicated affected graphs/flags) and composition run by the handler afterwards on the plain db handle. Composition is long-running (worker compose, blob uploads, admission webhooks) and should not hold a DB transaction open across the whole batch.
…e flag Add a scenario where one batch publish touches two federated graphs, a contract, and an enabled feature flag, asserting each graph composes exactly once. Runs with split-config-loading both off and on.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
controlplane/test/subgraph/batch-publish-subgraphs.test.ts (1)
591-597: ⚡ Quick winTighten the fedGraphB fan-out count assertion to exact delta.
toBeGreaterThan(bTotalBefore + 1)won’t catch accidental extra recompositions. If this scenario expects one base + one feature-flag recomposition, assertbTotalBefore + 2exactly.Proposed patch
- expect(await getCompositionCount(client, fedGraphB, DEFAULT_NAMESPACE, false)).toBeGreaterThan(bTotalBefore + 1); + expect(await getCompositionCount(client, fedGraphB, DEFAULT_NAMESPACE, false)).toBe(bTotalBefore + 2);🤖 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/subgraph/batch-publish-subgraphs.test.ts` around lines 591 - 597, The test currently uses a loose assertion for fedGraphB's total compositions; replace the final expect call that checks getCompositionCount(client, fedGraphB, DEFAULT_NAMESPACE, false) from toBeGreaterThan(bTotalBefore + 1) to an exact equality check asserting toBe(bTotalBefore + 2) so the test enforces exactly one base recompose plus one feature-flag recompose; update only that assertion in batch-publish-subgraphs.test.ts referring to fedGraphB, getCompositionCount, DEFAULT_NAMESPACE and bTotalBefore.
🤖 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/subgraph/batch-publish-subgraphs.test.ts`:
- Around line 591-597: The test currently uses a loose assertion for fedGraphB's
total compositions; replace the final expect call that checks
getCompositionCount(client, fedGraphB, DEFAULT_NAMESPACE, false) from
toBeGreaterThan(bTotalBefore + 1) to an exact equality check asserting
toBe(bTotalBefore + 2) so the test enforces exactly one base recompose plus one
feature-flag recompose; update only that assertion in
batch-publish-subgraphs.test.ts referring to fedGraphB, getCompositionCount,
DEFAULT_NAMESPACE and bTotalBefore.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 236dbace-73c1-4b9f-88ed-280f5b9c543e
📒 Files selected for processing (1)
controlplane/test/subgraph/batch-publish-subgraphs.test.ts
…ubgraphs-in-a-single-command
…e methods @Traced only wraps prototype methods, which ES #private methods are not. Switch the extracted SubgraphRepository helpers to TypeScript private methods so they get their own spans. Add tracing tests covering both cases.
Document wgc subgraph batch-publish in docs-website (new page + navigation) and update the CLI --config help to say YAML.
comatory
left a comment
There was a problem hiding this comment.
I just have few suggestions.
…lish Address review feedback: add -j/--json and -r/--raw output flags (matching feature-subgraph publish), and replace sync existsSync with a shared async fileExists util in cli utils.
…ubgraphs-in-a-single-command # Conflicts: # connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.go # connect/src/wg/cosmo/platform/v1/platform-PlatformService_connectquery.ts # connect/src/wg/cosmo/platform/v1/platform_connect.ts
Summary by CodeRabbit
New Features
Behavior
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.