feat: add cli commands to create,publish and delete grpc subgraphs#2179
feat: add cli commands to create,publish and delete grpc subgraphs#2179JivusAyrus merged 14 commits intomainfrom
Conversation
JivusAyrus
requested review from
StarpTech,
endigma,
jensneuse and
wilsonrivera
as code owners
WalkthroughAdds CLI commands for managing gRPC service subgraphs, updates control-plane publish logic to treat GRPC_SERVICE as a first-class subgraph type (including proto handling and validation), and introduces tests and test-data fixtures for gRPC service, feature-subgraph, and feature-flag flows. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
✨ Finishing Touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (28)
controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts (5)
192-195: Good type-mismatch guidance; minor casing nit.Message is correct and actionable. Prefer “gRPC service” capitalization for consistency across CLI/docs.
Apply:
-? `Subgraph ${subgraph.name} is a grpc service. Please use the 'wgc grpc-service publish' command to publish the grpc service.` +? `Subgraph ${subgraph.name} is a gRPC service. Please use the 'wgc grpc-service publish' command to publish the gRPC service.`
204-206: Fix typo in comment.“He flags” → “The flags”.
- * If he flags do not match, the database is the source of truth, so return an appropriate error. + * If the flags do not match, the database is the source of truth, so return an appropriate error.
469-481: Clarify/capitalize user-facing error text.Capitalize “gRPC” to align with CLI help.
- details: `The proto is required for plugin and grpc subgraphs.`, + details: `The proto is required for plugin and gRPC subgraphs.`,
511-522: Clarify/capitalize user-facing error text.Same here—use “gRPC”.
- details: `The schema, mappings, and lock are required for plugin and grpc subgraphs.`, + details: `The schema, mappings, and lock are required for plugin and gRPC subgraphs.`,
535-553: Avoid accidentally clearing stored proto on future calls without proto.Current code builds a proto object with empty strings if
req.protois absent (even though today you gate byreq.type). Guard the assignment to only includeprotowhen provided; keeps server resilient to future client changes.- : subgraph.type === 'grpc_service' - ? { - schema: req.proto?.schema || '', - mappings: req.proto?.mappings || '', - lock: req.proto?.lock || '', - } - : undefined, + : subgraph.type === 'grpc_service' + ? (req.proto + ? { + schema: req.proto.schema, + mappings: req.proto.mappings, + lock: req.proto.lock, + } + : undefined) + : undefined,cli/src/commands/grpc-service/commands/create.ts (3)
15-30: Unify terminology: use “gRPC” in help text.Consistent casing improves UX and searchability.
- command.description('Creates a federated grpc subgraph on the control plane.'); + command.description('Creates a federated gRPC subgraph on the control plane.'); ... - 'The name of the grpc subgraph to create. It is usually in the format of <org>.<service.name> and is used to uniquely identify your grpc subgraph.', + 'The name of the gRPC subgraph to create. It is usually in the format of <org>.<service.name> and is used to uniquely identify your gRPC subgraph.', ... - command.option('-n, --namespace [string]', 'The namespace of the grpc subgraph.'); + command.option('-n, --namespace [string]', 'The namespace of the gRPC subgraph.'); ... - 'The labels to apply to the subgraph. The labels are passed in the format <key>=<value> <key>=<value>.', + 'The labels to apply to the subgraph. Use the format <key>=<value> <key>=<value>.',
44-66: Use consistent wording and stderr for errors.
- Prefer “gRPC” casing.
- Print errors to stderr.
- const spinner = ora('GRPC Subgraph is being created...').start(); + const spinner = ora('gRPC subgraph is being created...').start(); ... - spinner.succeed('GRPC subgraph was created successfully.'); + spinner.succeed('gRPC subgraph was created successfully.'); ... - spinner.fail('Failed to create grpc subgraph.'); + spinner.fail('Failed to create gRPC subgraph.'); - if (resp.response?.details) { - console.log(pc.red(pc.bold(resp.response?.details))); - } + if (resp.response?.details) { + console.error(pc.red(pc.bold(resp.response?.details))); + }
21-24: Optional: client-side URL validation for faster feedback.Validate
--routing-urlwith the WHATWG URL API before making the API call; fail early with a clear message.cli/src/commands/grpc-service/commands/delete.ts (4)
30-47: Unify casing in spinner/messages.[gRPC] casing for consistency.
- const spinner = ora(`The grpc subgraph "${name}" is being deleted...`).start(); + const spinner = ora(`The gRPC subgraph "${name}" is being deleted...`).start(); ... - spinner.succeed(`The grpc subgraph "${name}" was deleted successfully.`); + spinner.succeed(`The gRPC subgraph "${name}" was deleted successfully.`);
57-90: Severity consistency: use warn (not fail) when deletion succeeded but composition has errors.Match publish.ts behavior and avoid implying the operation failed.
- spinner.fail(`The grpc subgraph "${name}" was deleted but with composition errors.`); + spinner.warn(`The gRPC subgraph "${name}" was deleted but with composition errors.`); ... - `There were composition errors when composing at least one federated graph related to the` + - ` grpc subgraph "${name}".\nThe router will continue to work with the latest valid schema.` + + `There were composition errors when composing at least one federated graph related to the` + + ` gRPC subgraph "${name}".\nThe router will continue to work with the latest valid schema.` +
91-119: Unify casing in deployment warning.- `The grpc subgraph "${name}" was deleted, but the updated composition could not be deployed.` + + `The gRPC subgraph "${name}" was deleted, but the updated composition could not be deployed.` +
121-127: Write errors to stderr.- if (resp.response?.details) { - console.log(pc.red(pc.bold(resp.response?.details))); - } + if (resp.response?.details) { + console.error(pc.red(pc.bold(resp.response?.details))); + }cli/src/commands/grpc-service/commands/publish.ts (3)
16-31: Casing nits in description/help.Use “gRPC” consistently.
- command.description( - "Publishes a grpc subgraph on the control plane. If the grpc subgraph doesn't exists, it will be created.\nIf the publication leads to composition errors, the errors will be visible in the Studio.\nThe router will continue to work with the latest valid schema.\nConsider using the 'wgc subgraph check' command to check for composition errors before publishing.", - ); + command.description( + "Publishes a gRPC subgraph on the control plane. If the gRPC subgraph doesn't exist, it will be created.\nIf the publication leads to composition errors, the errors will be visible in the Studio.\nThe router will continue to work with the latest valid schema.\nConsider using the 'wgc subgraph check' command to check for composition errors before publishing.", + ); ... - command.option('--name [string]', 'The name of the grpc subgraph.'); + command.option('--name [string]', 'The name of the gRPC subgraph.'); - command.option('-n, --namespace [string]', 'The namespace of the grpc subgraph.'); + command.option('-n, --namespace [string]', 'The namespace of the gRPC subgraph.'); - 'The routing URL of the grpc subgraph. This is the URL at which the grpc subgraph will be accessible.' + + 'The routing URL of the gRPC subgraph. This is the URL at which the gRPC subgraph will be accessible.' + ... - 'The labels to apply to the grpc subgraph. The labels are passed in the format <key>=<value> <key>=<value>.' + + 'The labels to apply to the gRPC subgraph. Use the format <key>=<value> <key>=<value>.' +
45-55: Casing nit in directory error.- `The grpc subgraph directory '${pc.bold(grpcSubgraphDir)}' does not exist. Please check the path and try again.`, + `The gRPC subgraph directory '${pc.bold(grpcSubgraphDir)}' does not exist. Please check the path and try again.`,
133-134: Spinner text casing.- const spinner = ora('GRPC Subgraph is being published...').start(); + const spinner = ora('gRPC subgraph is being published...').start();controlplane/test/subgraph/create-subgraph.test.ts (2)
881-911: Use try/finally to always close the server in testsIf an assertion throws, server.close() won’t run, leaking a listener. Wrap body in try/finally.
- const { client, server } = await SetupTest({ - dbname, - }); - - const grpcServiceName = genID('grpc-service'); - const grpcServiceLabel = genUniqueLabel('service'); - - const createGrpcServiceSubgraphResp = await client.createFederatedSubgraph({ - name: grpcServiceName, - namespace: DEFAULT_NAMESPACE, - type: SubgraphType.GRPC_SERVICE, - routingUrl: DEFAULT_SUBGRAPH_URL_ONE, - labels: [grpcServiceLabel], - }); - - expect(createGrpcServiceSubgraphResp.response?.code).toBe(EnumStatusCode.OK); - // ... - await server.close(); + const { client, server } = await SetupTest({ dbname }); + try { + const grpcServiceName = genID('grpc-service'); + const grpcServiceLabel = genUniqueLabel('service'); + const createGrpcServiceSubgraphResp = await client.createFederatedSubgraph({ + name: grpcServiceName, + namespace: DEFAULT_NAMESPACE, + type: SubgraphType.GRPC_SERVICE, + routingUrl: DEFAULT_SUBGRAPH_URL_ONE, + labels: [grpcServiceLabel], + }); + expect(createGrpcServiceSubgraphResp.response?.code).toBe(EnumStatusCode.OK); + // ... + } finally { + await server.close(); + }
879-957: Factor repeated setup into a helper (createGrpcServiceSubgraph) to reduce duplicationThe gRPC creation flow repeats across tests (name/labels/type/routingUrl + asserts). Consider extracting into test-util to keep tests concise.
If you want, I can generate a createGrpcServiceSubgraph helper and update these tests in a follow-up.
Also applies to: 958-1033, 1034-1096, 1098-1134, 1135-1157
controlplane/test/feature-subgraph/create-feature-subgraph.test.ts (3)
568-611: Ensure server is closed even on failureSame pattern: wrap SetupTest usage in try/finally to avoid port leaks.
- const { client, server } = await SetupTest({ dbname }); - // ... - await server.close(); + const { client, server } = await SetupTest({ dbname }); + try { + // ... + } finally { + await server.close(); + }
650-704: Avoid hardcoded URL; reuse shared constantsUse a shared DEFAULT_SUBGRAPH_URL_THREE (in test-util) instead of 'http://localhost:4003' inline for consistency.
- routingUrl: 'http://localhost:4003', + routingUrl: DEFAULT_SUBGRAPH_URL_THREE,Additional change (test-util.ts):
export const DEFAULT_SUBGRAPH_URL_THREE = 'http://localhost:4003';
613-648: Add a publish-path test asserting proto is required for gRPC-based feature subgraphsYou cover creation errors; consider a targeted publish test that omits proto and expects an ERR to guard regressions in publishFederatedSubgraph.
I can add a small test case next to this block if you’d like.
controlplane/test/feature-flag/feature-flag-with-grpc-service-fs.test.ts (3)
9-16: Remove unused importtoggleFeatureFlag is imported but not used.
- DEFAULT_SUBGRAPH_URL_TWO, - SetupTest, - toggleFeatureFlag, + DEFAULT_SUBGRAPH_URL_TWO, + SetupTest,
18-23: Make test-data path resolution robust (avoid process.cwd() reliance)CWD can vary under different runners. Resolve relative to this file instead.
-import path from 'node:path'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); -const testDataPath = path.join(process.cwd(), 'test/test-data/grpc-service'); +const testDataPath = path.join(__dirname, '..', 'test-data', 'grpc-service');
36-265: Close server with try/finallyApply try/finally to guarantee cleanup on assertion failures.
- const { client, server } = await SetupTest({ - dbname, - }); - // ... - await server.close(); + const { client, server } = await SetupTest({ dbname }); + try { + // ... + } finally { + await server.close(); + }controlplane/test/feature-subgraph/publish-feature-subgraph.test.ts (3)
39-44: Use the grpc-service fixtures instead of plugin fixturesThese tests are for gRPC service subgraphs but load proto/mapping/lock from test/test-data/plugin. Point them to test/test-data/grpc-service to avoid fixture drift and make intent explicit.
-// Read the test proto data for gRPC service tests -const testDataPath = path.join(process.cwd(), 'test/test-data/plugin'); +// Read the test proto data for gRPC service tests +const testDataPath = path.join(process.cwd(), 'test/test-data/grpc-service'); const grpcProtoSchema = readFileSync(path.join(testDataPath, 'service.proto'), 'utf8'); const grpcProtoMappings = readFileSync(path.join(testDataPath, 'mapping.json'), 'utf8'); const grpcProtoLock = readFileSync(path.join(testDataPath, 'service.proto.lock.json'), 'utf8');Optional: make this path robust against cwd changes:
import { fileURLToPath } from 'node:url'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const testDataPath = path.join(__dirname, '../test-data/grpc-service');
29-37: Remove unused helperenableProposalsForNamespace() is defined but never used in this file.
-// Helper function to enable proposals for namespace -async function enableProposalsForNamespace(client: any, namespace = 'default') { - const enableResponse = await client.enableProposalsForNamespace({ - namespace, - enableProposals: true, - }); - - return enableResponse; -}
517-553: Fix misleading comment (test is expected to pass)The inline comment says “should fail” but the test correctly expects OK and asserts GRPC_SERVICE inheritance. Update the comment (and optionally the test title) to match behavior.
-test('that a feature subgraph can be created and published with a gRPC service base subgraph using wgc fs publish with STANDARD type', async () => { +test('that a feature subgraph can be created via wgc fs publish when base is GRPC_SERVICE (STANDARD param ignored)', async () => { ... - // Try to create and publish feature subgraph based on gRPC service with STANDARD type - should fail (replicating CLI call) + // Create and publish with type=STANDARD provided — this is ignored and inherits GRPC_SERVICE from the base (replicating CLI call)controlplane/test/subgraph/publish-subgraph.test.ts (2)
800-812: Use grpc-service fixtures for gRPC tests (don’t reuse plugin fixtures)These GRPC tests should use the grpc-service proto/mapping/lock to avoid conflating plugin and GRPC fixtures.
- const validGrpcProtoRequest = { - schema: pluginSchema, - mappings: pluginMappings, - lock: pluginLock, - }; + const grpcTestDataPath = path.join(process.cwd(), 'test/test-data/grpc-service'); + const grpcServiceSchema = readFileSync(path.join(grpcTestDataPath, 'service.proto'), 'utf8'); + const grpcServiceMappings = readFileSync(path.join(grpcTestDataPath, 'mapping.json'), 'utf8'); + const grpcServiceLock = readFileSync(path.join(grpcTestDataPath, 'service.proto.lock.json'), 'utf8'); + + const validGrpcProtoRequest = { + schema: grpcServiceSchema, + mappings: grpcServiceMappings, + lock: grpcServiceLock, + };Optional: resolve path from import.meta.url as in previous comment for cwd-agnostic tests.
818-846: Prefer DEFAULT_SUBGRAPH_URL_ constants over literal URLs*Replace hard-coded 'http://localhost:4001' with DEFAULT_SUBGRAPH_URL_ONE (and likewise for 4002/4003) from test-util for consistency and easier changes.
-import { ... DEFAULT_NAMESPACE, ... } from '../test-util.js'; +import { ... DEFAULT_NAMESPACE, DEFAULT_SUBGRAPH_URL_ONE, DEFAULT_SUBGRAPH_URL_TWO, ... } from '../test-util.js'; ... -const routingUrl = 'http://localhost:4001'; +const routingUrl = DEFAULT_SUBGRAPH_URL_ONE; ... - routingUrl: 'http://localhost:4002', + routingUrl: DEFAULT_SUBGRAPH_URL_TWO,Apply similarly to other occurrences in this block.
Also applies to: 848-881, 909-935, 937-963, 987-1010, 1012-1034, 1036-1066, 1067-1102, 1104-1135
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
cli/src/commands/grpc-service/commands/create.ts(1 hunks)cli/src/commands/grpc-service/commands/delete.ts(1 hunks)cli/src/commands/grpc-service/commands/publish.ts(1 hunks)cli/src/commands/grpc-service/index.ts(1 hunks)controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts(4 hunks)controlplane/test/feature-flag/feature-flag-with-grpc-service-fs.test.ts(1 hunks)controlplane/test/feature-subgraph/create-feature-subgraph.test.ts(1 hunks)controlplane/test/feature-subgraph/publish-feature-subgraph.test.ts(4 hunks)controlplane/test/subgraph/create-subgraph.test.ts(1 hunks)controlplane/test/subgraph/publish-subgraph.test.ts(4 hunks)controlplane/test/test-data/grpc-service/mapping.json(1 hunks)controlplane/test/test-data/grpc-service/service.proto(1 hunks)controlplane/test/test-data/grpc-service/service.proto.lock.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T10:28:04.730Z
Learnt from: JivusAyrus
PR: wundergraph/cosmo#2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.730Z
Learning: In the controlplane codebase, authentication and authorization checks (including organization scoping) are handled at the service layer in files like unlinkSubgraph.ts before calling repository methods. Repository methods like unlinkSubgraph() in SubgraphRepository.ts can focus purely on data operations without redundant security checks.
Applied to files:
controlplane/test/subgraph/create-subgraph.test.ts
🧬 Code graph analysis (9)
controlplane/test/feature-flag/feature-flag-with-grpc-service-fs.test.ts (3)
controlplane/src/core/test-util.ts (4)
beforeAllSetup(38-45)afterAllSetup(47-51)genID(53-55)genUniqueLabel(57-59)controlplane/test/test-util.ts (5)
SetupTest(66-417)createThenPublishSubgraph(550-573)DEFAULT_SUBGRAPH_URL_ONE(49-49)DEFAULT_SUBGRAPH_URL_TWO(50-50)DEFAULT_ROUTER_URL(48-48)shared/src/utils/util.ts (1)
joinLabel(17-19)
cli/src/commands/grpc-service/commands/publish.ts (6)
cli/src/commands/grpc-service/commands/create.ts (1)
opts(13-73)cli/src/commands/grpc-service/commands/delete.ts (1)
opts(10-156)cli/src/commands/grpc-service/index.ts (1)
opts(10-21)cli/src/core/types/types.ts (1)
BaseCommandOptions(8-10)shared/src/utils/util.ts (1)
splitLabel(9-15)cli/src/core/config.ts (1)
getBaseHeaders(40-46)
cli/src/commands/grpc-service/index.ts (4)
cli/src/commands/grpc-service/commands/create.ts (1)
opts(13-73)cli/src/commands/grpc-service/commands/delete.ts (1)
opts(10-156)cli/src/commands/grpc-service/commands/publish.ts (1)
opts(14-287)cli/src/core/types/types.ts (1)
BaseCommandOptions(8-10)
cli/src/commands/grpc-service/commands/create.ts (4)
cli/src/commands/grpc-service/index.ts (1)
opts(10-21)cli/src/core/types/types.ts (1)
BaseCommandOptions(8-10)shared/src/utils/util.ts (1)
splitLabel(9-15)cli/src/core/config.ts (1)
getBaseHeaders(40-46)
cli/src/commands/grpc-service/commands/delete.ts (5)
cli/src/commands/grpc-service/commands/create.ts (1)
opts(13-73)cli/src/commands/grpc-service/commands/publish.ts (1)
opts(14-287)cli/src/commands/grpc-service/index.ts (1)
opts(10-21)cli/src/core/types/types.ts (1)
BaseCommandOptions(8-10)cli/src/core/config.ts (1)
getBaseHeaders(40-46)
controlplane/test/subgraph/publish-subgraph.test.ts (2)
controlplane/src/core/test-util.ts (4)
genUniqueLabel(57-59)genID(53-55)createTestRBACEvaluator(173-175)createTestGroup(181-198)controlplane/test/test-util.ts (3)
SetupTest(66-417)createSubgraph(704-716)subgraphSDL(754-758)
controlplane/test/subgraph/create-subgraph.test.ts (2)
controlplane/test/test-util.ts (4)
SetupTest(66-417)DEFAULT_NAMESPACE(52-52)DEFAULT_SUBGRAPH_URL_ONE(49-49)DEFAULT_SUBGRAPH_URL_TWO(50-50)controlplane/src/core/test-util.ts (4)
genID(53-55)genUniqueLabel(57-59)createTestRBACEvaluator(173-175)createTestGroup(181-198)
controlplane/test/feature-subgraph/create-feature-subgraph.test.ts (2)
controlplane/test/test-util.ts (3)
SetupTest(66-417)DEFAULT_SUBGRAPH_URL_ONE(49-49)DEFAULT_SUBGRAPH_URL_TWO(50-50)controlplane/src/core/test-util.ts (2)
genID(53-55)genUniqueLabel(57-59)
controlplane/test/feature-subgraph/publish-feature-subgraph.test.ts (2)
controlplane/test/test-util.ts (3)
SetupTest(66-417)DEFAULT_SUBGRAPH_URL_ONE(49-49)DEFAULT_SUBGRAPH_URL_TWO(50-50)controlplane/src/core/test-util.ts (2)
genID(53-55)genUniqueLabel(57-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build_test
- GitHub Check: build_push_image
🔇 Additional comments (19)
cli/src/commands/grpc-service/commands/publish.ts (4)
57-58: Name defaulting is sensible.Using directory basename when --name is omitted is pragmatic and matches other commands.
80-131: Input validation looks solid.Existence and non-empty checks for schema/proto/mapping/lock are thorough.
261-283: Warnings table UX is consistent.Good parity with other commands.
1-13: Import set is minimal and appropriate.All imports are used; no redundancy observed.
cli/src/commands/grpc-service/index.ts (2)
6-8: Wiring of new commands looks good.create/publish/delete are correctly registered under grpc-service.
11-18: Description aligns with command set.No issues found.
controlplane/test/feature-flag/feature-flag-with-grpc-service-fs.test.ts (2)
126-139: Proto lock/mapping consistency: QueryUsersRequest missingvalidProtoRequest references mapping that declares QueryUsersRequest, but the lock file doesn’t define it. Publishing may fail until the lock is fixed (see separate comment on the lock file).
Would you like me to open a quick PR update to the lock fixture after you confirm?
176-213: usersByStatus field lacks a mapping in the gRPC proto mappingFeature SDL adds Query.usersByStatus, but mapping.json doesn’t map it to an RPC. If the server validates coverage, this could fail.
Two options:
- Add a usersByStatus operation/req/resp in service.proto and mapping.json.
- Or remove the field from this test if not needed.
controlplane/test/test-data/grpc-service/mapping.json (1)
44-164: Consider mapping coverage for all GraphQL operations used in testsFeature test adds Query.usersByStatus, which has no entry here. If validation enforces coverage, add a mapping block; otherwise, ignore.
I can draft the mapping once you confirm the intended RPC name and request/response messages.
controlplane/test/feature-subgraph/publish-feature-subgraph.test.ts (6)
462-515: LGTM: Feature FS inherits GRPC_SERVICE type from baseGood coverage ensuring type inheritance and routing URL on the created feature subgraph.
568-608: LGTM: Routing URL required validation for GRPC_SERVICE feature FSSolid negative test asserting proper error when routingUrl is omitted.
610-649: LGTM: Invalid routing URL validationError path for malformed routingUrl is asserted correctly.
651-721: LGTM: Multiple gRPC feature subgraphs from the same baseGood to validate type, isFeatureSubgraph, and distinct routing URLs across multiple features.
722-758: LGTM: Proto requirement for gRPC feature subgraphsNegative case correctly checks proto requirement for GRPC_SERVICE.
798-840: Error string is consistent across code and tests: The message “Subgraph ${name} is a grpc service. Please use the 'wgc grpc-service publish' command to publish the grpc service.” is defined inline in publishFederatedSubgraph.ts (line 193) and referenced verbatim in both tests; casing and wording are stable.controlplane/test/subgraph/publish-subgraph.test.ts (2)
44-55: LGTM: Helper for creating GRPC_SERVICE subgraphsNice focused helper; keeps tests concise and asserts OK on creation.
606-607: LGTM: Unified proto requirement messageExpectation aligns with server-side rule: proto must be present for plugin and grpc subgraphs.
controlplane/test/test-data/grpc-service/service.proto (2)
1-98: LGTM: gRPC test fixture is well-structuredClear service and messages with wrappers; suitable for end-to-end tests.
1-98: No changes needed: grpc-service fixture is referenced
The fixture under test/test-data/grpc-service is loaded by controlplane/test/feature-flag/feature-flag-with-grpc-service-fs.test.ts (lines 17–22).
There was a problem hiding this comment.
Actionable comments posted: 0
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/subgraph/publishFederatedSubgraph.ts (1)
185-201: Fix type-mismatch guard to respect caller intent; also polish gRPC wordingDefaulting
req.typeearlier makes this check always run and can incorrectly block updates to existing subgraphs. Gate the comparison on whether the client actually provided a type, and capitalize “gRPC” in the message.Apply within this hunk:
- if (req.type !== undefined && subgraph.type !== formatSubgraphType(req.type)) { + if (clientSuppliedType && subgraph.type !== formatSubgraphType(req.type)) { return { response: { code: EnumStatusCode.ERR, details: subgraph.type === 'grpc_plugin' ? `Subgraph ${subgraph.name} is a plugin. Please use the 'wgc router plugin publish' command to publish the plugin.` - : subgraph.type === 'grpc_service' - ? `Subgraph ${subgraph.name} is a grpc service. Please use the 'wgc grpc-service publish' command to publish the grpc service.` + : subgraph.type === 'grpc_service' + ? `Subgraph ${subgraph.name} is a gRPC service. Please use the 'wgc grpc-service publish' command to publish the gRPC service.` : `Subgraph ${subgraph.name} is not of type ${formatSubgraphType(req.type)}.`, },And add near the defaults (above Line 65):
// Preserve whether caller explicitly set a type and avoid clobbering enum zero-values. const clientSuppliedType = req.type !== undefined; req.type = req.type ?? SubgraphType.STANDARD;
🧹 Nitpick comments (3)
controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts (3)
469-481: Clarify error copy: explicitly name both gRPC flavors“The proto is required for plugin and grpc subgraphs.” → be explicit to reduce ambiguity in UX.
- details: `The proto is required for plugin and grpc subgraphs.`, + details: `The proto is required for gRPC plugin and gRPC service subgraphs.`,
511-522: Match copy with above: explicitly list gRPC plugin/service- details: `The schema, mappings, and lock are required for plugin and grpc subgraphs.`, + details: `The schema, mappings, and lock are required for gRPC plugin and gRPC service subgraphs.`,
546-553: Avoid silently wiping stored proto on service updatesGiven the earlier validations require
req.proto.schema/mappings/lock, using|| ''increases risk of overwriting persisted values if validations change or regress. Assert non-null and pass through.: subgraph.type === 'grpc_service' ? { - schema: req.proto?.schema || '', - mappings: req.proto?.mappings || '', - lock: req.proto?.lock || '', + schema: req.proto!.schema, + mappings: req.proto!.mappings, + lock: req.proto!.lock, } : undefined,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts (1)
controlplane/src/core/util.ts (1)
formatSubgraphType(606-621)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build_test
- GitHub Check: Analyze (go)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
cli/src/commands/router/commands/plugin/commands/delete.ts (1)
120-126: Log errors to stderr for tooling compatibilityUse console.error for error details so scripts piping stdout aren’t polluted by error text.
- if (resp.response?.details) { - console.log(pc.red(pc.bold(resp.response?.details))); - } + if (resp.response?.details) { + console.error(pc.red(pc.bold(resp.response?.details))); + }cli/src/commands/grpc-service/commands/publish.ts (9)
16-18: Grammar: "doesn't exists" → "doesn't exist"Minor user-facing copy fix.
- "Publishes a gRPC subgraph on the control plane. If the gRPC subgraph doesn't exists, it will be created.\nIf the publication leads to composition errors, the errors will be visible in the Studio.\nThe router will continue to work with the latest valid schema.\nConsider using the 'wgc subgraph check' command to check for composition errors before publishing.", + "Publishes a gRPC subgraph on the control plane. If the gRPC subgraph doesn't exist, it will be created.\nIf the publication leads to composition errors, the errors will be visible in the Studio.\nThe router will continue to work with the latest valid schema.\nConsider using the 'wgc subgraph check' command to check for composition errors before publishing.",
133-134: Consistent casing: “gRPC subgraph”Align spinner text casing with the rest of the file and other commands.
- const spinner = ora('GRPC Subgraph is being published...').start(); + const spinner = ora('The gRPC subgraph is being published...').start();
140-143: Fix misleading comment about routingUrl requirementThe comment says optional “when the subgraph does not exist,” but it’s actually required on first publish and ignored on updates. Correct the comment to avoid confusion.
- // Optional when subgraph does not exist yet + // Ignored if the subgraph already exists; required on first publish routingUrl: options.routingUrl,
72-79: Prefer readFile with encoding over TextDecoderSimpler, fewer allocations, and clearer intent.
- const schemaBuffer = await readFile(schemaFile); - const schema = new TextDecoder().decode(schemaBuffer); + const schema = await readFile(schemaFile, 'utf8'); @@ - const protoSchemaBuffer = await readFile(protoSchemaFile); - const protoSchema = new TextDecoder().decode(protoSchemaBuffer); + const protoSchema = await readFile(protoSchemaFile, 'utf8'); @@ - const protoMappingBuffer = await readFile(protoMappingFile); - const protoMapping = new TextDecoder().decode(protoMappingBuffer); + const protoMapping = await readFile(protoMappingFile, 'utf8'); @@ - const protoLockBuffer = await readFile(protoLockFile); - const protoLock = new TextDecoder().decode(protoLockBuffer); + const protoLock = await readFile(protoLockFile, 'utf8');Also applies to: 89-95, 106-114, 125-131
106-114: Fail fast on invalid JSON for mapping/lockValidate JSON to provide immediate, actionable errors before calling the API.
Add after reading each file:
// After reading protoMapping try { JSON.parse(protoMapping); } catch { program.error(pc.red(pc.bold(`The proto mapping file '${pc.bold(protoMappingFile)}' is not valid JSON.`))); } // After reading protoLock try { JSON.parse(protoLock); } catch { program.error(pc.red(pc.bold(`The proto lock file '${pc.bold(protoLockFile)}' is not valid JSON.`))); }Also applies to: 125-131
135-153: Wrap network call in try/catch to handle transport errors cleanlyUncaught network errors will crash the CLI with an unhelpful stack trace. Wrap the publish call and exit via program.error.
- const resp = await opts.client.platform.publishFederatedSubgraph( + try { + const resp = await opts.client.platform.publishFederatedSubgraph( { name: grpcSubgraphName, namespace: options.namespace, schema, // Ignored if the subgraph already exists; required on first publish routingUrl: options.routingUrl, labels: options.label.map((label: string) => splitLabel(label)), type: SubgraphType.GRPC_SERVICE, proto: { schema: protoSchema, mappings: protoMapping, lock: protoLock, }, }, { headers: getBaseHeaders(), }, - ); + ); - switch (resp.response?.code) { + switch (resp.response?.code) { case EnumStatusCode.OK: { spinner.succeed( resp?.hasChanged === false ? 'No new changes to publish.' : `The gRPC subgraph ${pc.bold(grpcSubgraphName)} published successfully.`, ); @@ - if (!options.suppressWarnings && resp.compositionWarnings.length > 0) { + if (!options.suppressWarnings && resp.compositionWarnings.length > 0) { const compositionWarningsTable = new Table({ head: [ pc.bold(pc.white('FEDERATED_GRAPH_NAME')), pc.bold(pc.white('NAMESPACE')), pc.bold(pc.white('FEATURE_FLAG')), pc.bold(pc.white('WARNING_MESSAGE')), ], colWidths: [30, 30, 30, 120], wordWrap: true, }); console.log(pc.yellow(`The following warnings were produced while composing the federated graph:`)); for (const compositionWarning of resp.compositionWarnings) { compositionWarningsTable.push([ compositionWarning.federatedGraphName, compositionWarning.namespace, compositionWarning.featureFlag || '-', compositionWarning.message, ]); } console.log(compositionWarningsTable.toString()); } + } catch (err) { + spinner.fail('Network error while publishing gRPC subgraph.'); + const msg = err instanceof Error ? err.message : String(err); + program.error(pc.red(pc.bold(msg))); + }Also applies to: 284-287
172-176: Write error details to stderrKeep failures script-friendly: errors to stderr.
- console.log(pc.red(`Error: Proposal match failed`)); - console.log(pc.red(resp.proposalMatchMessage)); + console.error(pc.red(`Error: Proposal match failed`)); + console.error(pc.red(resp.proposalMatchMessage));
57-58: Optional: validate label format before publishingPrevent surprising server-side errors; fail early if any label isn’t key=value.
Add after computing grpcSubgraphName:
const badLabel = (options.label ?? []).find((l: string) => !/^[^=\s]+=[^=\s]+$/.test(l)); if (badLabel) { program.error(pc.red(pc.bold(`Invalid label '${badLabel}'. Expected format: key=value`))); }
155-171: Success copy tweak (optional)Slightly clearer, active voice.
- 'To apply any new changes after this publication, update your gRPC subgraph by modifying your schema (remember to generate), updating your implementation and then publishing again.', + 'To apply new changes, modify your schema (remember to generate), update your implementation, and publish again.',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
cli/src/commands/grpc-service/commands/delete.ts(1 hunks)cli/src/commands/grpc-service/commands/publish.ts(1 hunks)cli/src/commands/router/commands/plugin/commands/delete.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cli/src/commands/grpc-service/commands/delete.ts
🧰 Additional context used
🧬 Code graph analysis (1)
cli/src/commands/grpc-service/commands/publish.ts (6)
cli/src/commands/grpc-service/commands/delete.ts (1)
opts(10-156)cli/src/commands/grpc-service/index.ts (1)
opts(10-21)cli/src/commands/grpc-service/commands/create.ts (1)
opts(13-73)cli/src/core/types/types.ts (1)
BaseCommandOptions(8-10)shared/src/utils/util.ts (1)
splitLabel(9-15)cli/src/core/config.ts (1)
getBaseHeaders(40-46)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
cli/src/commands/router/commands/plugin/commands/delete.ts (1)
15-15: Commander short/long flag binding LGTMSwitching to "-f, --force" correctly binds short and long forms to one boolean. Matches Commander conventions and other commands.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
cli/src/commands/grpc-service/commands/create.ts (3)
15-29: Unify “gRPC” casing and fix minor grammar in user-facing text.Use “gRPC” consistently and fix “doesn’t exists” → “doesn’t exist”. Also prefer consistent spinner phrasing.
- command.description('Creates a federated grpc subgraph on the control plane.'); + command.description('Creates a federated gRPC subgraph on the control plane.'); @@ - 'The name of the grpc subgraph to create. It is used to uniquely identify your grpc subgraph.', + 'The name of the gRPC subgraph to create. It is used to uniquely identify your gRPC subgraph.', @@ - command.option('-n, --namespace [string]', 'The namespace of the grpc subgraph.'); + command.option('-n, --namespace [string]', 'The namespace of the gRPC subgraph.'); @@ - 'The routing URL of your subgraph. This is the url at which the subgraph will be accessible.', + 'The routing URL of your subgraph. This is the URL at which the subgraph will be accessible.', @@ - command.option('--readme <path-to-readme>', 'The markdown file which describes the subgraph.'); + command.option('--readme <path-to-readme>', 'The Markdown file that describes the subgraph.'); @@ - const spinner = ora('GRPC Subgraph is being created...').start(); + const spinner = ora('The gRPC subgraph is being created...').start(); @@ - spinner.succeed('GRPC subgraph was created successfully.'); + spinner.succeed('The gRPC subgraph was created successfully.'); @@ - spinner.fail('Failed to create grpc subgraph.'); + spinner.fail('Failed to create gRPC subgraph.');Also applies to: 44-62
31-43: Optionally pre-validate the routing URL client-side (same rule as server).Fail early if the URL is invalid; mirrors server-side isValidUrl check and improves UX.
-import { splitLabel } from '@wundergraph/cosmo-shared'; +import { splitLabel, isValidUrl } from '@wundergraph/cosmo-shared'; @@ command.requiredOption( '-r, --routing-url <url>', 'The routing URL of your subgraph. This is the URL at which the subgraph will be accessible.', ); @@ command.action(async (name, options) => { let readmeFile; if (options.readme) { @@ } + + if (!isValidUrl(options.routingUrl)) { + program.error( + pc.red(pc.bold(`Routing URL "${pc.bold(options.routingUrl || '')}" is not a valid URL.`)), + ); + }Also applies to: 20-24, 5-5
63-66: Write error details to stderr.Use console.error for error details to respect stdout/stderr separation in CLI pipelines.
- if (resp.response?.details) { - console.log(pc.red(pc.bold(resp.response?.details))); - } + if (resp.response?.details) { + console.error(pc.red(pc.bold(resp.response?.details))); + }controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts (1)
192-195: Standardize “gRPC” casing in server messages.Minor copy nits only; improves consistency with CLI.
- ? `Subgraph ${subgraph.name} is a grpc service. Please use the 'wgc grpc-service publish' command to publish the grpc service.` + ? `Subgraph ${subgraph.name} is a gRPC service. Please use the 'wgc grpc-service publish' command to publish the gRPC service.` @@ - if (baseSubgraph.type === 'grpc_service') { + if (baseSubgraph.type === 'grpc_service') { return { response: { code: EnumStatusCode.ERR, - details: `Cannot create a feature subgraph with a grpc service base subgraph using this command. Since the base subgraph "${req.baseSubgraphName}" is a grpc service, please use the 'wgc feature-subgraph create' command to create the feature subgraph first, then publish it using the 'wgc grpc-service publish' command.`, + details: `Cannot create a feature subgraph with a gRPC service base subgraph using this command. Since the base subgraph "${req.baseSubgraphName}" is a gRPC service, please use the 'wgc feature-subgraph create' command to create the feature subgraph first, then publish it using the 'wgc grpc-service publish' command.`, }, @@ - details: `The proto is required for plugin and grpc subgraphs.`, + details: `The proto is required for plugin and gRPC subgraphs.`, @@ - details: `The schema, mappings, and lock are required for plugin and grpc subgraphs.`, + details: `The schema, mappings, and lock are required for plugin and gRPC subgraphs.`,Also applies to: 272-283, 487-489, 528-529
cli/src/commands/grpc-service/commands/publish.ts (4)
59-66: Simplify file reads: read as UTF-8 strings instead of Buffer+TextDecoder.Reduces boilerplate and allocations.
- const schemaBuffer = await readFile(schemaFile); - const schema = new TextDecoder().decode(schemaBuffer); + const schema = await readFile(schemaFile, 'utf8'); @@ - const protoSchemaBuffer = await readFile(protoSchemaFile); - const protoSchema = new TextDecoder().decode(protoSchemaBuffer); + const protoSchema = await readFile(protoSchemaFile, 'utf8'); @@ - const protoMappingBuffer = await readFile(protoMappingFile); - const protoMapping = new TextDecoder().decode(protoMappingBuffer); + const protoMapping = await readFile(protoMappingFile, 'utf8'); @@ - const protoLockBuffer = await readFile(protoLockFile); - const protoLock = new TextDecoder().decode(protoLockBuffer); + const protoLock = await readFile(protoLockFile, 'utf8');Also applies to: 91-97, 108-116, 127-133
17-18: Polish copy: fix grammar and casing in description and spinner.“doesn’t exists” → “doesn’t exist”; use “gRPC” consistently.
- command.description( - "Publishes a gRPC subgraph on the control plane. If the gRPC subgraph doesn't exists, it will be created.\nIf the publication leads to composition errors, the errors will be visible in the Studio.\nThe router will continue to work with the latest valid schema.\nConsider using the 'wgc subgraph check' command to check for composition errors before publishing.", + command.description( + "Publishes a gRPC subgraph on the control plane. If the gRPC subgraph doesn't exist, it will be created.\nIf the publication leads to composition errors, the errors will be visible in the Studio.\nThe router will continue to work with the latest valid schema.\nConsider using the 'wgc subgraph check' command to check for composition errors before publishing.", ); @@ - 'The routing URL of the gRPC subgraph. This is the URL at which the gRPC subgraph will be accessible.' + + 'The routing URL of the gRPC subgraph. This is the URL at which the gRPC subgraph will be accessible.' + ' This parameter is always ignored if the subgraph has already been created.', @@ - const spinner = ora('GRPC Subgraph is being published...').start(); + const spinner = ora('The gRPC subgraph is being published...').start();Also applies to: 27-30, 135-136
175-179: Send error details to stderr in failure branches.Minor consistency: prefer console.error for errors.
- console.log(pc.red(`Error: Proposal match failed`)); - console.log(pc.red(resp.proposalMatchMessage)); + console.error(pc.red(`Error: Proposal match failed`)); + console.error(pc.red(resp.proposalMatchMessage));
108-116: Optionally validate JSON shape for mapping/lock files.Early JSON.parse helps catch malformed files before API call.
- const protoMapping = await readFile(protoMappingFile, 'utf8'); + const protoMapping = await readFile(protoMappingFile, 'utf8'); + try { JSON.parse(protoMapping); } catch { program.error(pc.red(pc.bold(`The proto mapping file '${pc.bold(protoMappingFile)}' is not valid JSON.`))); } @@ - const protoLock = await readFile(protoLockFile, 'utf8'); + const protoLock = await readFile(protoLockFile, 'utf8'); + try { JSON.parse(protoLock); } catch { program.error(pc.red(pc.bold(`The proto lock file '${pc.bold(protoLockFile)}' is not valid JSON.`))); }Also applies to: 127-133
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
cli/src/commands/grpc-service/commands/create.ts(1 hunks)cli/src/commands/grpc-service/commands/publish.ts(1 hunks)controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts(6 hunks)controlplane/test/feature-subgraph/publish-feature-subgraph.test.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- controlplane/test/feature-subgraph/publish-feature-subgraph.test.ts
🧰 Additional context used
🧬 Code graph analysis (3)
cli/src/commands/grpc-service/commands/create.ts (5)
cli/src/commands/grpc-service/commands/publish.ts (1)
opts(14-289)cli/src/commands/grpc-service/index.ts (1)
opts(10-21)cli/src/core/types/types.ts (1)
BaseCommandOptions(8-10)shared/src/utils/util.ts (1)
splitLabel(9-15)cli/src/core/config.ts (1)
getBaseHeaders(40-46)
cli/src/commands/grpc-service/commands/publish.ts (6)
cli/src/commands/grpc-service/commands/create.ts (1)
opts(13-73)cli/src/commands/grpc-service/index.ts (1)
opts(10-21)cli/src/commands/grpc-service/commands/delete.ts (1)
opts(10-156)cli/src/core/types/types.ts (1)
BaseCommandOptions(8-10)shared/src/utils/util.ts (1)
splitLabel(9-15)cli/src/core/config.ts (1)
getBaseHeaders(40-46)
controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts (1)
router/gen/proto/wg/cosmo/common/common.pb.go (4)
EnumStatusCode(23-23)EnumStatusCode(103-105)EnumStatusCode(107-109)EnumStatusCode(116-118)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts (2)
482-536: Good: Proto is now required for gRPC service and plugin subgraphs.This aligns server validation with the new CLI behavior and prevents incomplete publications.
559-565: Good: Include proto payload on updates for grpc_service.Ensures schema/mapping/lock are persisted for gRPC services on publish.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cli/src/commands/feature-subgraph/commands/publish.ts (2)
27-30: Flag alias collision: -r is used for both --routing-url and --rawCommander will treat the later definition as winning; one of these flags will misbehave. Drop the short flag from
--raw(you already have-j, --json) or change it to a different alias.Apply this minimal fix:
- command.option('-r, --raw', 'Prints to the console in json format instead of table'); + command.option('--raw', 'Prints to the console in json format instead of table');Also applies to: 61-62
71-72: Make JSON/raw handling consistentYou compute
shouldOutputJson = options.json || options.raw, but pass onlyoptions.jsontohandleCompositionResult. With--raw, spinner is suppressed but output won’t be JSON.Apply:
- const shouldOutputJson = options.json || options.raw; + const shouldOutputJson = options.json || options.raw; ... - shouldOutputJson: options.json, + shouldOutputJson,Also applies to: 141-141
🧹 Nitpick comments (3)
cli/src/commands/feature-subgraph/commands/publish.ts (3)
98-123: Pass headers to publishFederatedSubgraph — confirm SDK signature and avoid label clobbering
- Good: adding
{ headers: getBaseHeaders() }to align with other publish commands.- Caution:
labels: []may unintentionally clear existing labels on re-publish. If the backend treats empty array as “set to empty,” prefer omitting the field when no labels are provided.Apply if labels are optional:
{ baseSubgraphName: options.subgraph, disableResolvabilityValidation: options.disableResolvabilityValidation, isFeatureSubgraph: true, - labels: [], name, namespace: options.namespace,If the request type requires
labels, consider threading a--labeloption (parity with standard subgraph publish) and only sending when provided.
18-18: Nit: grammar in help text“doesn't exists” → “doesn't exist”.
- "Publishes a feature subgraph on the control plane. If the feature subgraph doesn't exists, it will be created.\nIf the publication leads to composition errors, the errors will be visible in the Studio.\nThe router will continue to work with the latest valid schema.", + "Publishes a feature subgraph on the control plane. If the feature subgraph doesn't exist, it will be created.\nIf the publication leads to composition errors, the errors will be visible in the Studio.\nThe router will continue to work with the latest valid schema.",
32-35: Nit: wording/consistencyUse “URL” and simplify the sentence.
- 'The url used for subscriptions. If empty, it defaults to same url used for routing.' + + 'The URL used for subscriptions. If omitted, it defaults to the routing URL.' +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
cli/src/commands/feature-subgraph/commands/publish.ts(2 hunks)cli/src/commands/grpc-service/commands/publish.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cli/src/commands/grpc-service/commands/publish.ts
🧰 Additional context used
🧬 Code graph analysis (1)
cli/src/commands/feature-subgraph/commands/publish.ts (2)
cli/src/commands/subgraph/commands/publish.ts (1)
opts(20-252)cli/src/core/config.ts (1)
getBaseHeaders(40-46)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build_push_image
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
cli/src/commands/feature-subgraph/commands/publish.ts (1)
13-13: LGTM: centralizing auth/telemetry headers via getBaseHeadersImport is consistent with other commands and keeps header construction in one place.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts (4)
192-194: Capitalize “gRPC” in user-facing error textAlign with product terminology and other docs.
- : subgraph.type === 'grpc_service' - ? `Subgraph ${subgraph.name} is a grpc service. Please use the 'wgc grpc-service publish' command to publish the grpc service.` + : subgraph.type === 'grpc_service' + ? `Subgraph ${subgraph.name} is a gRPC service. Please use the 'wgc grpc-service publish' command to publish the gRPC service.`
272-283: Capitalize “gRPC” and tighten guidanceConsistent casing and clearer directive.
- if (baseSubgraph.type === 'grpc_service') { + if (baseSubgraph.type === 'grpc_service') { return { response: { code: EnumStatusCode.ERR, - details: `Cannot create a feature subgraph with a grpc service base subgraph using this command. Since the base subgraph "${req.baseSubgraphName}" is a grpc service, please use the 'wgc feature-subgraph create' command to create the feature subgraph first, then publish it using the 'wgc grpc-service publish' command.`, + details: `Cannot create a feature subgraph with a gRPC service base using this command. Since the base subgraph "${req.baseSubgraphName}" is a gRPC service, first create the feature subgraph with 'wgc feature-subgraph create', then publish it with 'wgc grpc-service publish'.`, },
487-487: Clarify scope: “plugin and gRPC service subgraphs”Be explicit; “grpc subgraphs” could be ambiguous.
- details: `The proto is required for plugin and grpc subgraphs.`, + details: `The proto is required for plugin and gRPC service subgraphs.`,
528-529: Clarify scope: “plugin and gRPC service subgraphs”Match the terminology used elsewhere.
- details: `The schema, mappings, and lock are required for plugin and grpc subgraphs.`, + details: `The schema, mappings, and lock are required for plugin and gRPC service subgraphs.`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts (1)
controlplane/src/core/util.ts (1)
formatSubgraphType(606-621)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build_test
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts (1)
559-566: gRPC proto persistence and routing URL validation verified — no changes required
- Proto: SubgraphRepository.update persists { schema, mappings, lock } for both grpc_service and grpc_plugin; pluginData is only used for grpc_plugin. (controlplane/src/core/repositories/SubgraphRepository.ts:563-577; controlplane/src/types/index.ts:107-110)
- Routing URL: create/publish paths validate routingUrl for non-GRPC_PLUGIN (GRPC_SERVICE must provide a routingUrl). isValidUrl uses the URL constructor (shared/src/utils/util.ts:isValidUrl) and will accept grpc://—exempting GRPC_SERVICE or loosening validation would be a behavioral change. (controlplane/src/core/bufservices/subgraph/createFederatedSubgraph.ts:148-151; controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts:372-374; shared/src/utils/util.ts:51-54)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
cli/src/commands/grpc-service/commands/publish.ts (5)
59-65: Read files as UTF-8 strings directly; drop TextDecoder.Simplifies code and avoids extra allocations.
- const schemaBuffer = await readFile(schemaFile); - const schema = new TextDecoder().decode(schemaBuffer); + const schema = await readFile(schemaFile, 'utf8'); @@ - const protoSchemaBuffer = await readFile(protoSchemaFile); - const protoSchema = new TextDecoder().decode(protoSchemaBuffer); + const protoSchema = await readFile(protoSchemaFile, 'utf8'); @@ - const protoMappingBuffer = await readFile(protoMappingFile); - const protoMapping = new TextDecoder().decode(protoMappingBuffer); + const protoMapping = await readFile(protoMappingFile, 'utf8'); @@ - const protoLockBuffer = await readFile(protoLockFile); - const protoLock = new TextDecoder().decode(protoLockBuffer); + const protoLock = await readFile(protoLockFile, 'utf8');Also applies to: 91-97, 108-116, 127-133
110-116: Validate JSON for mapping and lock early.Catch invalid JSON before sending to the API; improves UX.
if (protoMapping.trim().length === 0) { program.error( pc.red( pc.bold(`The proto mapping file '${protoMappingFile}' is empty. Please provide a valid mapping.`), ), ); } + try { + JSON.parse(protoMapping); + } catch { + program.error(pc.red(pc.bold(`The proto mapping file '${protoMappingFile}' is not valid JSON.`))); + } @@ if (protoLock.trim().length === 0) { program.error( pc.red(pc.bold(`The proto lock file '${protoLockFile}' is empty. Please provide a valid lock.`)), ); } + try { + JSON.parse(protoLock); + } catch { + program.error(pc.red(pc.bold(`The proto lock file '${protoLockFile}' is not valid JSON.`))); + }Also applies to: 129-133
1-1: Ensure the generated path is a directory (not just exists).Prevents confusing errors when a file is passed instead of a folder.
-import { existsSync } from 'node:fs'; +import { existsSync, statSync } from 'node:fs';if (!existsSync(grpcSubgraphGeneratedDir)) { program.error( pc.red( pc.bold( `The gRPC subgraph generated directory '${grpcSubgraphGeneratedDir}' does not exist. Please check the path and try again.`, ), ), ); } + const dirStat = statSync(grpcSubgraphGeneratedDir); + if (!dirStat.isDirectory()) { + program.error( + pc.red( + pc.bold( + `The gRPC subgraph generated path '${grpcSubgraphGeneratedDir}' is not a directory. Please provide the directory created by 'wgc grpc-service generate'.`, + ), + ), + ); + }Also applies to: 67-76
263-263: Guard optional warnings array.Avoids a potential runtime error if the API returns warnings as undefined.
- if (!options.suppressWarnings && resp.compositionWarnings.length > 0) { + if (!options.suppressWarnings && resp.compositionWarnings?.length > 0) {
135-135: Polish user-facing copy and casing (gRPC).Consistent casing and a slightly clearer spinner/warn text.
- const spinner = ora('GRPC Subgraph is being published...').start(); + const spinner = ora('The gRPC subgraph is being published...').start();- spinner.warn( - "The gRPC subgraph was published, but the updated composition hasn't been deployed, so it's not accessible to the router. Check the errors listed below for details.", - ); + spinner.warn( + "The gRPC subgraph was published, but the updated composition hasn't been deployed, so it's not accessible to the router. See the errors below for details.", + );Also applies to: 223-225
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cli/src/commands/grpc-service/commands/publish.ts(1 hunks)controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts
🧰 Additional context used
🧬 Code graph analysis (1)
cli/src/commands/grpc-service/commands/publish.ts (6)
cli/src/commands/grpc-service/commands/create.ts (1)
opts(13-73)cli/src/commands/grpc-service/commands/delete.ts (1)
opts(10-156)cli/src/commands/grpc-service/index.ts (1)
opts(10-21)cli/src/core/types/types.ts (1)
BaseCommandOptions(8-10)shared/src/utils/util.ts (1)
splitLabel(9-15)cli/src/core/config.ts (1)
getBaseHeaders(40-46)
🪛 GitHub Actions: wgc CI
cli/src/commands/grpc-service/commands/publish.ts
[error] 1-1: Prettier formatting check failed. Code style issues found in this file. Run 'prettier --write' to fix.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
cli/src/commands/grpc-service/commands/publish.ts (1)
119-140: Guard the publish call with try/catch to avoid unhandled rejections (and fix “gRPC” casing).Network/API failures will currently crash the command with a stack trace. Fail gracefully and keep output consistent.
- const spinner = ora('GRPC Subgraph is being published...').start(); - - const resp = await opts.client.platform.publishFederatedSubgraph( + const spinner = ora('gRPC subgraph is being published...').start(); + let resp: any; + try { + resp = await opts.client.platform.publishFederatedSubgraph( { name, namespace: options.namespace, schema, // Optional when subgraph does not exist yet routingUrl: options.routingUrl, labels: options.label.map((label: string) => splitLabel(label)), type: SubgraphType.GRPC_SERVICE, proto: { schema: protoSchema, mappings: protoMapping, lock: protoLock, }, }, { headers: getBaseHeaders(), }, - ); + ); + } catch (err) { + spinner.fail(`Failed to publish gRPC subgraph "${name}".`); + console.error(pc.red(pc.bold(String((err as Error)?.message ?? err)))); + process.exitCode = 1; + return; + }
🧹 Nitpick comments (6)
cli/src/commands/grpc-service/commands/publish.ts (6)
143-147: Grammar: add “was”.Reads better and matches style used elsewhere.
- : `The gRPC subgraph ${pc.bold(name)} published successfully.`, + : `The gRPC subgraph ${pc.bold(name)} was published successfully.`,
100-106: Validate JSON in mapping/lock early for clearer errors.Fail fast if files contain invalid JSON.
const protoMappingBuffer = await readFile(protoMappingFile); const protoMapping = new TextDecoder().decode(protoMappingBuffer); if (protoMapping.trim().length === 0) { program.error( pc.red(pc.bold(`The proto mapping file '${protoMappingFile}' is empty. Please provide a valid mapping.`)), ); } + try { + JSON.parse(protoMapping); + } catch { + program.error(pc.red(pc.bold(`The proto mapping file '${protoMappingFile}' contains invalid JSON.`))); + } @@ const protoLockBuffer = await readFile(protoLockFile); const protoLock = new TextDecoder().decode(protoLockBuffer); if (protoLock.trim().length === 0) { program.error(pc.red(pc.bold(`The proto lock file '${protoLockFile}' is empty. Please provide a valid lock.`))); } + try { + JSON.parse(protoLock); + } catch { + program.error(pc.red(pc.bold(`The proto lock file '${protoLockFile}' contains invalid JSON.`))); + }Also applies to: 113-117
31-36: Validate label format before mapping.Prevent sending malformed labels like “foo” (missing “=”).
command.option( '--label [labels...]', 'The labels to apply to the gRPC subgraph. The labels are passed in the format <key>=<value> <key>=<value>.' + ' This parameter is always ignored if the gRPC subgraph has already been created.', [], ); @@ - labels: options.label.map((label: string) => splitLabel(label)), + labels: options.label.map((label: string) => splitLabel(label)),Add right before starting the spinner:
+ // Validate labels early + const invalidLabel = options.label.find((l: string) => !l.includes('=')); + if (invalidLabel) { + program.error(pc.red(pc.bold(`Invalid label '${invalidLabel}'. Expected format <key>=<value>.`))); + }Also applies to: 128-129
159-162: Use stderr/stdout channels consistently (errors → error, warnings → warn).Improves CLI ergonomics and tooling integration.
- console.log(pc.red(`Error: Proposal match failed`)); - console.log(pc.red(resp.proposalMatchMessage)); + console.error(pc.red(`Error: Proposal match failed`)); + console.error(pc.red(resp.proposalMatchMessage)); @@ - console.log(pc.yellow(`Warning: Proposal match failed`)); - console.log(pc.yellow(resp.proposalMatchMessage)); + console.warn(pc.yellow(`Warning: Proposal match failed`)); + console.warn(pc.yellow(resp.proposalMatchMessage)); @@ - console.log(pc.yellow(`The following warnings were produced while composing the federated graph:`)); + console.warn(pc.yellow(`The following warnings were produced while composing the federated graph:`));Also applies to: 166-169, 259-259
182-186: Minor copy edit: remove stray comma.Cleaner sentence.
- `We found composition errors, while composing the federated graph.\nThe router will continue to work with the latest valid schema.\n${pc.bold( + `We found composition errors while composing the federated graph.\nThe router will continue to work with the latest valid schema.\n${pc.bold(
141-245: De-duplicate composition/deployment table rendering via a shared helper.Create a small utility (e.g., printCompositionOutcome) reused by grpc-service create/publish/delete and feature-subgraph publish for consistent UX and less code drift.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cli/src/commands/grpc-service/commands/publish.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cli/src/commands/grpc-service/commands/publish.ts (7)
cli/src/commands/feature-subgraph/commands/publish.ts (1)
opts(15-156)cli/src/commands/grpc-service/commands/create.ts (1)
opts(13-73)cli/src/commands/grpc-service/commands/delete.ts (1)
opts(10-156)cli/src/commands/grpc-service/index.ts (1)
opts(10-21)cli/src/core/types/types.ts (1)
BaseCommandOptions(8-10)shared/src/utils/util.ts (1)
splitLabel(9-15)cli/src/core/config.ts (1)
getBaseHeaders(40-46)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build_test
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
cli/src/commands/grpc-service/commands/publish.ts (1)
49-117: Solid input validation — nice coverage of existence/emptiness checks.Checks for schema, proto, mapping, and lock files are thorough and user-friendly.
4 participants
Summary by CodeRabbit
New Features
Control Plane
Bug Fixes
Chores
Tests
Checklist