Conversation
# Conflicts: # connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.go # controlplane/CHANGELOG.md # controlplane/migrations/meta/0132_snapshot.json # controlplane/migrations/meta/_journal.json # controlplane/package.json # controlplane/src/types/index.ts
# Conflicts: # controlplane/src/core/sentry.config.ts
WalkthroughAdds Subgraph Check Extensions end-to-end: DB migration and schema, protobuf/RPCs and connect bindings, control-plane handlers/repositories, webhook delivery flow and persistence, CDN route for extension blobs, Studio UI/pages/components, type and lint-rule refactors, CLI/result plumbing, and tests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas to pay extra attention:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Tip ✨ Issue Enrichment is now available for GitHub issues!CodeRabbit can now help you manage issues more effectively:
Disable automatic issue enrichmentTo disable automatic issue enrichment, add the following to your issue_enrichment:
auto_enrich:
enabled: falseComment |
Router-nonroot image scan passed✅ No security vulnerabilities found in image: |
…tensions # 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 # controlplane/src/core/bufservices/PlatformService.ts
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
studio/src/components/checks/subgraph-check-extension.tsx (1)
24-47: Error messages may be masked when deliveryId is missing.The early return on line 24 prevents displaying any
errorMessagewhendeliveryIdis falsy. Since both fields are independent optional props, the backend might set an error without creating a delivery record (e.g., configuration validation failures), and those errors would be hidden by the "Skipped / Not configured" EmptyState.The past review comment noted this issue and was marked as addressed, but the logic flow problem persists—only the UI text was updated.
Consider restructuring the logic to check for
errorMessagefirst:- if (!deliveryId) { + if (errorMessage) { + return ( + <> + <Alert variant="destructive"> + <CrossCircledIcon className="h-4 w-4" /> + <AlertTitle>Subgraph Check Extension failed</AlertTitle> + <AlertDescription> + {errorMessage} + {deliveryId && ( + <p className="mt-4"> + <Button + variant="link" + className="p-0 h-auto" + onClick={() => setSelectedDeliveryId(deliveryId)} + > + View delivery details + </Button> + </p> + )} + </AlertDescription> + </Alert> + {deliveryId && ( + <WebhookDeliveryDetails + deliveryId={selectedDeliveryId} + onOpenChange={(isOpen) => { + if (!isOpen) { + setSelectedDeliveryId(undefined); + } + }} + refreshDeliveries={() => {}} + /> + )} + </> + ); + } + + if (!deliveryId) { return ( <EmptyState
🧹 Nitpick comments (1)
studio/src/components/checks/subgraph-check-extension.tsx (1)
49-77: Redundant deliveryId checks after early return.Lines 57 and 75 conditionally check
deliveryIdeven though the early return on line 24 guarantees it exists at this point in the code.Simplify by removing the redundant checks:
{errorMessage} - {deliveryId && ( <p className="mt-4"> <Button variant="link" className="p-0 h-auto" onClick={() => setSelectedDeliveryId(deliveryId)} > View delivery details </Button> </p> - )} </AlertDescription> </Alert> ) : ( <EmptyState icon={<CheckCircleIcon className="text-success" />} title="Subgraph Check Extension Successful" description="The subgraph check extension completed successfully." - actions={deliveryId && (<Button onClick={() => setSelectedDeliveryId(deliveryId)}>View delivery details</Button>)} + actions={<Button onClick={() => setSelectedDeliveryId(deliveryId)}>View delivery details</Button>} /> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
studio/src/components/checks/subgraph-check-extension.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
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.
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
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.
📚 Learning: 2025-09-08T20:57:07.946Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.
Applied to files:
studio/src/components/checks/subgraph-check-extension.tsx
📚 Learning: 2025-08-29T10:28:04.846Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
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:
studio/src/components/checks/subgraph-check-extension.tsx
📚 Learning: 2025-08-31T18:51:32.185Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/bufservices/check/getCheckSummary.ts:0-0
Timestamp: 2025-08-31T18:51:32.185Z
Learning: In the SchemaCheckRepository.getLinkedSchemaCheck method, organization-level security is implemented through post-query validation by checking `check.subgraphs[0].namespace.organizationId !== organizationId` and returning undefined if the linked check doesn't belong to the caller's organization, preventing cross-tenant data leakage.
Applied to files:
studio/src/components/checks/subgraph-check-extension.tsx
⏰ 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). (14)
- GitHub Check: build-router
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_push_image (nonroot)
- GitHub Check: build_push_image
- GitHub Check: image_scan (nonroot)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: image_scan
- GitHub Check: build_test
- GitHub Check: integration_test (./events)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
studio/src/pages/[organizationSlug]/check-extensions.tsx (1)
41-56: Config object includes all response fields when building mutation payload.The spread
...resinto config includes non-config fields (response,isSecretKeyAssigned,isLintingEnabledForNamespace, etc.) that are then passed to the mutation. This couples the mutation input to the query response structure.
🧹 Nitpick comments (1)
cdn-server/cdn/test/cdn.test.ts (1)
557-647: Schema check extensions tests are solid; consider aligning test names with expected status codesThe new
schema check extensions handlersuite nicely mirrors the other CDN handler tests and, importantly, covers:
- auth failures (missing/invalid/expired token),
- org-id mismatch returning
400(good for cross-tenant isolation, consistent with earlier schema-check behavior — based on learnings),- happy-path fetch and 404 for missing blobs.
Two minor nits you may want to clean up:
- The test named
it returns a 401 if the organization id does not match with the JWT payloadactually asserts400. Renaming the test (or adjusting the expectation) would avoid confusion.- For consistency with other suites, you might choose to reuse the same
requestPathshape (e.g. using thecheckIdin the mismatch cases too), though behavior is correct either way.No blockers; tests look functionally correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cdn-server/cdn/test/cdn.test.ts(2 hunks)controlplane/src/core/bufservices/check-extensions/configureSubgraphCheckExtensions.ts(1 hunks)controlplane/src/core/repositories/OrganizationRepository.ts(2 hunks)controlplane/src/core/repositories/SubgraphCheckExtensionsRepository.ts(1 hunks)controlplane/test/subgraph-check-extensions.test.ts(1 hunks)studio/src/components/check-extensions/check-extensions-config.tsx(1 hunks)studio/src/pages/[organizationSlug]/check-extensions.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- controlplane/src/core/repositories/SubgraphCheckExtensionsRepository.ts
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
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.
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
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.
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2155
File: router/core/router.go:1857-1866
Timestamp: 2025-08-20T10:08:17.857Z
Learning: router/pkg/config/config.schema.json forbids null values for traffic_shaping.subgraphs: additionalProperties references $defs.traffic_shaping_subgraph_request_rule with type "object". Therefore, in core.NewSubgraphTransportOptions, dereferencing each subgraph rule pointer is safe under schema-validated configs, and a nil-check is unnecessary.
📚 Learning: 2025-08-29T10:28:04.846Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
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/src/core/bufservices/check-extensions/configureSubgraphCheckExtensions.tscontrolplane/test/subgraph-check-extensions.test.tscdn-server/cdn/test/cdn.test.tscontrolplane/src/core/repositories/OrganizationRepository.ts
📚 Learning: 2025-09-17T20:55:39.456Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2172
File: router/core/graph_server.go:0-0
Timestamp: 2025-09-17T20:55:39.456Z
Learning: The Initialize method in router/internal/retrytransport/manager.go has been updated to properly handle feature-flag-only subgraphs by collecting subgraphs from both routerConfig.GetSubgraphs() and routerConfig.FeatureFlagConfigs.ConfigByFeatureFlagName, ensuring all subgraphs receive retry configuration.
Applied to files:
controlplane/src/core/bufservices/check-extensions/configureSubgraphCheckExtensions.ts
📚 Learning: 2025-09-08T20:57:07.946Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.
Applied to files:
controlplane/src/core/bufservices/check-extensions/configureSubgraphCheckExtensions.tscontrolplane/test/subgraph-check-extensions.test.tsstudio/src/pages/[organizationSlug]/check-extensions.tsxcdn-server/cdn/test/cdn.test.ts
📚 Learning: 2025-08-20T10:08:17.857Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2155
File: router/core/router.go:1857-1866
Timestamp: 2025-08-20T10:08:17.857Z
Learning: router/pkg/config/config.schema.json forbids null values for traffic_shaping.subgraphs: additionalProperties references $defs.traffic_shaping_subgraph_request_rule with type "object". Therefore, in core.NewSubgraphTransportOptions, dereferencing each subgraph rule pointer is safe under schema-validated configs, and a nil-check is unnecessary.
Applied to files:
controlplane/src/core/bufservices/check-extensions/configureSubgraphCheckExtensions.ts
📚 Learning: 2025-08-28T09:17:49.477Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2141
File: router-tests/http_subscriptions_test.go:17-55
Timestamp: 2025-08-28T09:17:49.477Z
Learning: The Cosmo router uses a custom, intentionally rigid multipart implementation for GraphQL subscriptions. The multipart parsing in test files should remain strict and not be made more tolerant, as this rigidity is by design.
Applied to files:
controlplane/test/subgraph-check-extensions.test.ts
📚 Learning: 2025-09-03T21:12:18.149Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: cli/src/commands/subgraph/commands/link.ts:21-28
Timestamp: 2025-09-03T21:12:18.149Z
Learning: Subgraph names in the Cosmo platform can contain slashes, so when parsing formats like `<namespace>/<subgraph-name>`, only the first slash should be treated as the delimiter between namespace and subgraph name. The subgraph name portion can contain additional slashes.
Applied to files:
studio/src/pages/[organizationSlug]/check-extensions.tsx
📚 Learning: 2025-07-30T15:23:03.295Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2079
File: router/pkg/config/config.schema.json:2942-2954
Timestamp: 2025-07-30T15:23:03.295Z
Learning: OCI (Open Container Initiative) registry URLs in the Cosmo router project should not include HTTP/HTTPS schemas. They are specified as hostnames only (e.g., "registry.example.com" or "registry.example.com/namespace"). The JSON schema validation should use plain "string" type without "http-url" format for plugin registry URLs.
Applied to files:
studio/src/components/check-extensions/check-extensions-config.tsx
📚 Learning: 2025-09-22T11:13:45.617Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2224
File: router/pkg/authentication/keyfunc/keyfunc_test.go:142-154
Timestamp: 2025-09-22T11:13:45.617Z
Learning: When reviewing forked code, especially test files with test fixtures like JWT tokens, avoid suggesting modifications to maintain alignment with upstream and preserve the original author's structure. Test fixtures that are clearly marked as such (not real secrets) should generally be left unchanged in forked implementations.
Applied to files:
cdn-server/cdn/test/cdn.test.ts
📚 Learning: 2025-08-31T18:51:32.185Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/bufservices/check/getCheckSummary.ts:0-0
Timestamp: 2025-08-31T18:51:32.185Z
Learning: In the SchemaCheckRepository.getLinkedSchemaCheck method, organization-level security is implemented through post-query validation by checking `check.subgraphs[0].namespace.organizationId !== organizationId` and returning undefined if the linked check doesn't belong to the caller's organization, preventing cross-tenant data leakage.
Applied to files:
cdn-server/cdn/test/cdn.test.tscontrolplane/src/core/repositories/OrganizationRepository.ts
🧬 Code graph analysis (5)
controlplane/src/core/bufservices/check-extensions/configureSubgraphCheckExtensions.ts (6)
controlplane/src/core/routes.ts (1)
RouterOptions(25-54)connect/src/wg/cosmo/platform/v1/platform_pb.ts (2)
ConfigureSubgraphCheckExtensionsRequest(21174-21254)ConfigureSubgraphCheckExtensionsResponse(21259-21291)controlplane/src/core/util.ts (3)
getLogger(92-94)handleError(50-88)enrichLogger(96-113)controlplane/src/core/repositories/OrganizationRepository.ts (1)
OrganizationRepository(50-1674)controlplane/src/core/repositories/NamespaceRepository.ts (1)
NamespaceRepository(9-188)controlplane/src/core/repositories/SubgraphCheckExtensionsRepository.ts (1)
SubgraphCheckExtensionsRepository(5-49)
controlplane/test/subgraph-check-extensions.test.ts (2)
controlplane/src/core/test-util.ts (2)
beforeAllSetup(38-45)afterAllSetup(47-51)controlplane/test/test-util.ts (1)
SetupTest(66-418)
studio/src/pages/[organizationSlug]/check-extensions.tsx (6)
studio/src/hooks/use-check-user-access.ts (1)
useCheckUserAccess(9-37)studio/src/hooks/use-workspace.ts (1)
useWorkspace(4-11)connect/src/wg/cosmo/platform/v1/platform-PlatformService_connectquery.ts (2)
configureSubgraphCheckExtensions(2376-2385)getSubgraphCheckExtensionsConfig(2362-2371)studio/src/components/check-extensions/check-extensions-config.tsx (2)
SubgraphCheckExtensionsConfig(24-24)CheckExtensionsConfig(94-388)studio/src/components/layout/dashboard-layout.tsx (1)
getDashboardLayout(312-337)studio/src/components/dashboard/workspace-selector.tsx (1)
WorkspaceSelector(16-58)
studio/src/components/check-extensions/check-extensions-config.tsx (3)
studio/src/hooks/use-workspace.ts (1)
useWorkspace(4-11)studio/src/lib/constants.ts (1)
docsBaseURL(1-1)playground/src/components/ui/checkbox.tsx (1)
Checkbox(26-26)
cdn-server/cdn/test/cdn.test.ts (1)
controlplane/test/test-util.ts (1)
InMemoryBlobStorage(641-693)
🪛 GitHub Actions: CDN CI
cdn-server/cdn/test/cdn.test.ts
[warning] 1-1: Code style issues found in test/cdn.test.ts. Run Prettier with --write to fix.
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues in this file.
⏰ 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). (15)
- GitHub Check: build-router
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_push_image (nonroot)
- GitHub Check: integration_test (./events)
- GitHub Check: build_push_image
- GitHub Check: image_scan (nonroot)
- GitHub Check: image_scan
- GitHub Check: integration_test (./telemetry)
- GitHub Check: build_test
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (10)
cdn-server/cdn/test/cdn.test.ts (1)
10-15: WidenedgenerateTokensignature for optionalfederated_graph_idlooks goodAllowing
federatedGraphId: string | undefinedmatches the new subgraph-checks CDN route, where JWTs are scoped only byorganization_id. Existing call sites that pass a concrete string remain valid, and the new tests confirm the org-only JWT usage.controlplane/src/core/repositories/OrganizationRepository.ts (2)
1007-1010: LGTM! Proper cleanup of subgraph checks blob storage during organization deletion.The addition follows the existing pattern of cleaning up blob storage directories for graphs and ensures no orphaned data remains after organization deletion.
1393-1393: LGTM! New feature flag follows established pattern.The
subgraph-check-extensionsfeature flag is correctly initialized tofalseby default, consistent with other enterprise-gated features in the map.controlplane/src/core/bufservices/check-extensions/configureSubgraphCheckExtensions.ts (1)
15-107: Well-structured handler with proper feature gating and validation.The implementation correctly:
- Gates the feature behind enterprise plan
- Validates namespace existence and write access
- Enforces https for non-localhost endpoints while allowing http/https for localhost
- Uses the pre-update
namespace.enableSubgraphCheckExtensionsvalue intentionally to force all include flags to true on first enablementcontrolplane/test/subgraph-check-extensions.test.ts (1)
1-270: Comprehensive test coverage for the new feature.The test suite thoroughly covers:
- Feature flag gating (enterprise vs non-enterprise plans)
- Authorization checks (write access requirements)
- Full configuration lifecycle
- Endpoint validation rules (localhost http/https, non-localhost https-only, invalid schemes)
studio/src/components/check-extensions/check-extensions-config.tsx (2)
26-71: LGTM! Endpoint validation correctly handles localhost and non-localhost URLs.The Zod schema properly validates:
- Localhost: allows both
http://andhttps://- Non-localhost: requires
https://only- Invalid URLs: rejects malformed input
This aligns with the backend validation in
configureSubgraphCheckExtensions.ts.
94-387: Well-structured form component with good UX patterns.The component handles:
- Secret key management with confirmation dialog for removal
- Conditional rendering based on feature enablement
- Proper form validation and submission
- Dynamic disabling of fields based on namespace settings
studio/src/pages/[organizationSlug]/check-extensions.tsx (3)
98-132: LGTM! Proper error and loading state handling.The component correctly handles:
- Loading state with fullscreen loader
- Error states with retry action
- Upgrade plan gating with navigation to billing
143-152: LGTM! Switch has appropriate guard conditions.The Switch is correctly disabled when:
- Mutation or refetch is pending
- Feature flag is not enabled
- User lacks admin/developer role
173-182: LGTM! Layout setup follows established patterns.The page correctly uses
getDashboardLayoutwithWorkspaceSelectorin breadcrumbs, consistent with other namespace-scoped pages.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cdn-server/cdn/test/cdn.test.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.
📚 Learning: 2025-09-22T11:13:45.617Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2224
File: router/pkg/authentication/keyfunc/keyfunc_test.go:142-154
Timestamp: 2025-09-22T11:13:45.617Z
Learning: When reviewing forked code, especially test files with test fixtures like JWT tokens, avoid suggesting modifications to maintain alignment with upstream and preserve the original author's structure. Test fixtures that are clearly marked as such (not real secrets) should generally be left unchanged in forked implementations.
Applied to files:
cdn-server/cdn/test/cdn.test.ts
📚 Learning: 2025-08-31T18:51:32.185Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/bufservices/check/getCheckSummary.ts:0-0
Timestamp: 2025-08-31T18:51:32.185Z
Learning: In the SchemaCheckRepository.getLinkedSchemaCheck method, organization-level security is implemented through post-query validation by checking `check.subgraphs[0].namespace.organizationId !== organizationId` and returning undefined if the linked check doesn't belong to the caller's organization, preventing cross-tenant data leakage.
Applied to files:
cdn-server/cdn/test/cdn.test.ts
📚 Learning: 2025-08-29T10:28:04.846Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
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:
cdn-server/cdn/test/cdn.test.ts
📚 Learning: 2025-09-08T20:57:07.946Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.
Applied to files:
cdn-server/cdn/test/cdn.test.ts
🧬 Code graph analysis (1)
cdn-server/cdn/test/cdn.test.ts (1)
controlplane/test/test-util.ts (1)
InMemoryBlobStorage(641-693)
⏰ 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). (15)
- GitHub Check: build-router
- GitHub Check: integration_test (./telemetry)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: build_push_image (nonroot)
- GitHub Check: integration_test (./events)
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: image_scan
- GitHub Check: image_scan (nonroot)
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
cdn-server/cdn/test/cdn.test.ts (2)
10-15: LGTM!The signature change to accept
federatedGraphId: string | undefinedcorrectly accommodates organization-scoped operations like subgraph check extensions where a federated graph ID is not required.
616-644: LGTM!The tests for successful file retrieval and 404 on missing files correctly validate the schema check extensions handler behavior.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
cdn-server/cdn/test/cdn.test.ts (1)
557-645: Schema check extensions tests are solid; consider isolating the expired‑token caseThe new
schema check extensions handlersuite nicely covers:
- 401 for missing and malformed Authorization headers,
- 400 when the path organization ID doesn’t match the JWT payload,
- 200 and 404 for existing/missing blobs.
One remaining clarity issue is the expired‑token test (lines 599‑614): it still calls
await app.request(`/foo/subgraph_checks/operations.json`, ...)even though the JWT’s
organization_idisorganizationId. That means the scenario combines “expired token” with an org‑ID mismatch in the path. Middleware will still return 401 for the expired token, but the test’s intent is cleaner if only expiration is varied:- const res = await app.request(`/foo/subgraph_checks/operations.json`, { + const res = await app.request(`/${organizationId}/subgraph_checks/${checkId}.json`, { method: 'GET', headers: { Authorization: `Bearer ${token}`, }, });This keeps the path consistent with the valid/200 and 404 cases, and makes the failure mode unambiguously “token expired”.
🧹 Nitpick comments (3)
studio/src/components/check-extensions/check-extensions-config.tsx (1)
148-161: Simplify nested fragments.The description contains nested empty fragments that can be simplified.
Apply this diff:
- : ( - <> - <> - You must{" "} - <Link - href={`/${organizationSlug}/policies?namespace=${namespace.name}`} - className="text-primary" - > - enable the linter - </Link> - {" "}for the namespace to be able to receive lint warnings and errors. - </> - </> - ), + : ( + <> + You must{" "} + <Link + href={`/${organizationSlug}/policies?namespace=${namespace.name}`} + className="text-primary" + > + enable the linter + </Link> + {" "}for the namespace to be able to receive lint warnings and errors. + </> + ),controlplane/src/core/repositories/SubgraphRepository.ts (1)
1794-1819: Subgraph check extension webhook integration is well-placed; consider isolating failuresThe new
performSchemaCheckparameters (actorId, blobStorage, admissionConfig, webhookService) and the call towebhookService.sendSubgraphCheckExtensionare threaded cleanly, and the resulting delivery info is correctly persisted and exposed via both the DB update and response (isCheckExtensionSkipped,checkExtensionErrorMessage).One thing to consider:
sendSubgraphCheckExtensioncan still throw for infrastructure reasons (blob storage, JWT signing, DB), which would currently abort the entire schema check. If you want extensions to be strictly best‑effort, you could wrap this call in atry/catch, log the error, and treat it as a skipped extension while keeping the main check result intact.Also applies to: 1836-1837, 2121-2141, 2160-2168, 2190-2194
cdn-server/cdn/test/cdn.test.ts (1)
89-97: Aligning JWT/path mismatch tests to 400 across handlers is reasonableUpdating these tests to expect
400for organization/graph ID mismatches (while keeping401for missing/invalid/expired tokens elsewhere) matches the “validation error vs authentication error” split and keeps behavior consistent across operations, router config, draft router config, and cache warmup flows.One minor nuance: the mismatch tests in the ready-router, draft-router, and cache-warmup suites still hit the persisted-operations-style route shape (
/foo/bar/operations/...) rather than each handler’s own path. Because the same JWT middleware enforces the org/graph claim checks, this still exercises the validation logic, but if you want route-specific coverage you could point those tests at the respective handler paths instead.Also applies to: 150-158, 211-219, 492-500
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cdn-server/cdn/test/cdn.test.ts(7 hunks)controlplane/src/core/repositories/SchemaCheckRepository.ts(10 hunks)controlplane/src/core/repositories/SubgraphRepository.ts(11 hunks)studio/src/components/check-extensions/check-extensions-config.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.
📚 Learning: 2025-09-22T11:13:45.617Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2224
File: router/pkg/authentication/keyfunc/keyfunc_test.go:142-154
Timestamp: 2025-09-22T11:13:45.617Z
Learning: When reviewing forked code, especially test files with test fixtures like JWT tokens, avoid suggesting modifications to maintain alignment with upstream and preserve the original author's structure. Test fixtures that are clearly marked as such (not real secrets) should generally be left unchanged in forked implementations.
Applied to files:
cdn-server/cdn/test/cdn.test.ts
📚 Learning: 2025-08-31T18:51:32.185Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/bufservices/check/getCheckSummary.ts:0-0
Timestamp: 2025-08-31T18:51:32.185Z
Learning: In the SchemaCheckRepository.getLinkedSchemaCheck method, organization-level security is implemented through post-query validation by checking `check.subgraphs[0].namespace.organizationId !== organizationId` and returning undefined if the linked check doesn't belong to the caller's organization, preventing cross-tenant data leakage.
Applied to files:
cdn-server/cdn/test/cdn.test.tscontrolplane/src/core/repositories/SubgraphRepository.tscontrolplane/src/core/repositories/SchemaCheckRepository.ts
📚 Learning: 2025-08-29T10:28:04.846Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
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:
cdn-server/cdn/test/cdn.test.tscontrolplane/src/core/repositories/SubgraphRepository.tscontrolplane/src/core/repositories/SchemaCheckRepository.ts
📚 Learning: 2025-09-08T20:57:07.946Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.
Applied to files:
cdn-server/cdn/test/cdn.test.tscontrolplane/src/core/repositories/SubgraphRepository.tscontrolplane/src/core/repositories/SchemaCheckRepository.ts
📚 Learning: 2025-07-30T15:23:03.295Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2079
File: router/pkg/config/config.schema.json:2942-2954
Timestamp: 2025-07-30T15:23:03.295Z
Learning: OCI (Open Container Initiative) registry URLs in the Cosmo router project should not include HTTP/HTTPS schemas. They are specified as hostnames only (e.g., "registry.example.com" or "registry.example.com/namespace"). The JSON schema validation should use plain "string" type without "http-url" format for plugin registry URLs.
Applied to files:
studio/src/components/check-extensions/check-extensions-config.tsx
📚 Learning: 2025-08-29T10:10:06.969Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/checks/[checkId]/index.tsx:0-0
Timestamp: 2025-08-29T10:10:06.969Z
Learning: The isCheckSuccessful function in both controlplane/src/core/util.ts and studio/src/components/check-badge-icon.tsx includes early exits for linked check validation. It returns false immediately if either isLinkedTrafficCheckFailed or isLinkedPruningCheckFailed is true, which means linked check failures automatically cause the overall check to fail.
Applied to files:
controlplane/src/core/repositories/SubgraphRepository.tscontrolplane/src/core/repositories/SchemaCheckRepository.ts
📚 Learning: 2025-09-10T11:15:52.157Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/ProposalRepository.ts:562-572
Timestamp: 2025-09-10T11:15:52.157Z
Learning: The getLatestCheckForProposal function in controlplane/src/core/repositories/ProposalRepository.ts is only called during proposal creation or updates, so proposal match error checking (hasProposalMatchError) is not needed since the proposal is being modified itself rather than being matched against.
Applied to files:
controlplane/src/core/repositories/SchemaCheckRepository.ts
📚 Learning: 2025-09-10T11:31:32.502Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/ProposalRepository.ts:0-0
Timestamp: 2025-09-10T11:31:32.502Z
Learning: When traffic checks or pruning checks are skipped in schema validation, the corresponding hasClientTraffic and hasGraphPruningErrors flags are automatically set to false by default. This means that checking clientTrafficCheckSkipped or graphPruningCheckSkipped flags in linked check failure conditions is redundant, as skipped checks will naturally have false values for their respective has* flags.
Applied to files:
controlplane/src/core/repositories/SchemaCheckRepository.ts
📚 Learning: 2025-09-10T09:59:17.257Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts:417-419
Timestamp: 2025-09-10T09:59:17.257Z
Learning: When skipTrafficCheck is true in schema checks, the traffic inspection loop is completely bypassed (as shown in line 2040 of SubgraphRepository.ts with the continue statement), which means hasClientTraffic will be false. The traffic analysis doesn't run at all when skipTrafficCheck is true, unlike the previous understanding that it always ran to collect usage data.
Applied to files:
controlplane/src/core/repositories/SchemaCheckRepository.ts
🧬 Code graph analysis (3)
studio/src/components/check-extensions/check-extensions-config.tsx (4)
connect/src/wg/cosmo/platform/v1/platform_pb.ts (1)
ConfigureSubgraphCheckExtensionsRequest(21174-21254)connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.go (3)
ConfigureSubgraphCheckExtensionsRequest(25790-25804)ConfigureSubgraphCheckExtensionsRequest(25819-25819)ConfigureSubgraphCheckExtensionsRequest(25834-25836)studio/src/hooks/use-workspace.ts (1)
useWorkspace(4-11)studio/src/lib/constants.ts (1)
docsBaseURL(1-1)
controlplane/src/core/repositories/SubgraphRepository.ts (4)
controlplane/src/db/schema.ts (1)
schemaChecks(786-832)controlplane/src/types/index.ts (2)
SchemaCheckDTO(189-223)SchemaLintIssues(697-700)controlplane/src/core/webhooks/OrganizationWebhookService.ts (1)
OrganizationWebhookService(119-779)connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.go (4)
LintSeverity(27-27)LintSeverity(56-58)LintSeverity(60-62)LintSeverity(69-71)
controlplane/src/core/repositories/SchemaCheckRepository.ts (1)
controlplane/src/core/webhooks/OrganizationWebhookService.ts (1)
OrganizationWebhookService(119-779)
⏰ 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). (15)
- GitHub Check: build-router
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: image_scan (nonroot)
- GitHub Check: integration_test (./events)
- GitHub Check: image_scan
- GitHub Check: build_test
- GitHub Check: build_push_image (nonroot)
- GitHub Check: build_push_image
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: build_test
- GitHub Check: build_push_image
🔇 Additional comments (9)
studio/src/components/check-extensions/check-extensions-config.tsx (1)
346-347: Verify checkbox checked state when feature is disabled.When
enableSubgraphCheckExtensionsisfalse, the checked logic forces all non-disabled checkboxes to appear checked:checked={!item.isDisabled && (!enableSubgraphCheckExtensions || field.value === true)}This means that when the feature is disabled, all available options show as checked (but disabled), regardless of their actual
field.value. Please confirm this is the intended UX behavior.If the intent is to display the actual saved configuration values even when the feature is disabled, the logic should be:
checked={!item.isDisabled && field.value === true}If the intent is to show all options as checked by default when the feature is disabled (to indicate what will be enabled), then the current logic is correct, but this UX pattern may be confusing to users.
controlplane/src/core/repositories/SubgraphRepository.ts (3)
1-10: Imports for LintSeverity/SchemaCheckDTO/OrganizationWebhookService are consistent with later usageThe new imports line up with how LintSeverity, SchemaCheckDTO, and OrganizationWebhookService are used further down in this file; nothing to change here.
Also applies to: 35-50, 68-68
1158-1160: checkExtensionDeliveryId / checkExtensionErrorMessage propagation looks correctThe additional fields are selected from
schemaChecksand propagated into bothSchemaCheckDTOandSchemaCheckSummaryDTOas optional values (|| undefined), matching the DB schema and the updated DTO definitions. This should surface extension metadata cleanly to callers.Also applies to: 1217-1219, 1302-1303
2142-2157: Verify severity enum alignment for extension‑provided lint issuesFiltering
sceResult.additionalLintIssuesbyissue.severity === LintSeverity.warn/LintSeverity.errorassumes the extension response uses the same enum values asLintSeverityfromplatform_pb. If the JSON schema uses different casing or string values, these filters will silently drop extension issues fromlintWarnings/lintErrors(though they’re still stored viaaddSchemaCheckLintIssues).Please double‑check that
subgraphCheckExtensionSchemadefinesseverityin terms of this exact enum so additional issues contribute correctly tohasLintErrorsand the response arrays; if not, a small mapping layer here would be safer.controlplane/src/core/repositories/SchemaCheckRepository.ts (4)
14-15: New imports match existing usageAdding
iliketo the drizzle import and bringing inOrganizationWebhookService/BlobStoragematches the usages later in the file, so this resolves dependencies cleanly.Also applies to: 53-55
99-115: Extendingupdateto persist extension metadata is consistent with schemaThe extra optional fields
checkExtensionDeliveryIdandcheckExtensionErrorMessageare threaded into theupdatepayload in the same pattern as the existing optional flags. This aligns with the new columns onschema_checksand with howperformSchemaChecknow callsupdate, so the persistence side looks good.Also applies to: 131-133
245-246: Clarified comment around override set operationsThe updated comment more clearly describes the relationship between incoming vs stored changes and the
EXCEPT/INTERSECTqueries. No behavioral change here, and the explanation now matches the SQL below.
608-611: checkMultipleSchemas: new parameters and linked‑check invocation are wired correctlyThe additions of
actorId,blobStorage,admissionConfig, andwebhookServicetocheckMultipleSchemasand the forwarding of these intosubgraphRepo.performSchemaCheckfor linked subgraphs look coherent:
admissionConfigshape matches whatperformSchemaCheckandsendSubgraphCheckExtensionexpect (jwtSecret/cdnBaseUrl).- The same
actorIdandwebhookServiceare reused, so extension deliveries for linked checks are attributed consistently.- Other existing fields (organizationSlug, targetNamespace, targetSubgraph, limit, chClient, etc.) are preserved.
Assuming all external call sites now pass these new arguments, this path should behave as intended.
Also applies to: 627-631, 649-650, 1141-1143, 1157-1158
cdn-server/cdn/test/cdn.test.ts (1)
1-15: Import andgenerateTokensignature change look consistentUsing
randomUUIDand allowingfederatedGraphIdto bestring | undefinedmatches the new subgraph check route where no federated graph ID is present, while existing callers still pass a concrete string. No issues spotted here.
studio/src/components/check-extensions/check-extensions-config.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
studio/src/components/check-extensions/check-extensions-config.tsx (1)
268-269: Description doesn't document the localhost exception.The description states the endpoint "Must be a valid absolute URL starting with
https://", but the validation (lines 40-48) allowshttp://orhttps://for localhost. This can confuse developers testing with local endpoints.Apply this diff to document the exception:
<FormDescription> - The endpoint that will receive POST requests with event data. Must be a valid absolute{" "} - URL starting with <code className="font-mono">https://</code>. + The endpoint that will receive POST requests with event data. Must be a valid absolute{" "} + URL starting with <code className="font-mono">https://</code> (<code className="font-mono">http://</code>{" "} + is allowed for <code className="font-mono">localhost</code>). </FormDescription>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
studio/src/components/check-extensions/check-extensions-config.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
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.
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
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.
📚 Learning: 2025-07-30T15:23:03.295Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2079
File: router/pkg/config/config.schema.json:2942-2954
Timestamp: 2025-07-30T15:23:03.295Z
Learning: OCI (Open Container Initiative) registry URLs in the Cosmo router project should not include HTTP/HTTPS schemas. They are specified as hostnames only (e.g., "registry.example.com" or "registry.example.com/namespace"). The JSON schema validation should use plain "string" type without "http-url" format for plugin registry URLs.
Applied to files:
studio/src/components/check-extensions/check-extensions-config.tsx
🧬 Code graph analysis (1)
studio/src/components/check-extensions/check-extensions-config.tsx (2)
studio/src/hooks/use-workspace.ts (1)
useWorkspace(4-11)studio/src/lib/constants.ts (1)
docsBaseURL(1-1)
⏰ 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). (15)
- GitHub Check: build-router
- GitHub Check: image_scan (nonroot)
- GitHub Check: build_push_image
- GitHub Check: integration_test (./telemetry)
- GitHub Check: build_push_image (nonroot)
- GitHub Check: integration_test (./events)
- GitHub Check: build_test
- GitHub Check: image_scan
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (3)
studio/src/components/check-extensions/check-extensions-config.tsx (3)
1-24: LGTM! Clean imports and type definition.The imports are well-organized and the
SubgraphCheckExtensionsConfigtype properly omits the namespace field from the protobuf message type.
26-71: LGTM! URL validation logic is sound.The endpoint validation correctly handles both production HTTPS requirements and localhost development scenarios. The use of
superRefinewith custom error messages provides clear feedback to users.
282-381: LGTM! Form rendering logic is well-structured.The conditional rendering for the secret key input and the dynamic checkbox list are implemented correctly. The checkbox
checkedstate logic (line 346) appropriately handles the interaction between item-specific disable state, global enable/disable toggle, and individual field values.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
studio/src/components/check-extensions/check-extensions-config.tsx (2)
114-118: Stale closure acknowledged but still a risk.The intentional partial dependency preserves user edits but creates a sync issue: external changes to other config fields won't reflect until
enableSubgraphCheckExtensionschanges. Consider using a ref to capture the latest config while keeping the selective trigger.
267-270: Description doesn't match validation rules for localhost.The description states the URL must start with
https://, but the validation permitshttp://for localhost endpoints. This mismatch may confuse developers during local testing.<FormDescription> - The endpoint that will receive POST requests with event data. Must be a valid absolute{" "} - URL starting with <code className="font-mono">https://</code>. + The endpoint that will receive POST requests with event data. Must be a valid absolute{" "} + URL starting with <code className="font-mono">https://</code> (<code className="font-mono">http://</code> allowed for localhost). </FormDescription>
🧹 Nitpick comments (1)
studio/src/components/check-extensions/check-extensions-config.tsx (1)
202-226: Consider wrapping title/description in AlertDialogHeader for semantic structure.The AlertDialog works but adding
AlertDialogHeaderimproves semantic HTML structure and accessibility:+import { + AlertDialog, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle +} from "@/components/ui/alert-dialog";<AlertDialogContent> + <AlertDialogHeader> <AlertDialogTitle>Secret key removed</AlertDialogTitle> <AlertDialogDescription> You are about to update the webhook configuration and remove the secret key.{" "} Are you sure you want to do this? </AlertDialogDescription> + </AlertDialogHeader> <AlertDialogFooter>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
studio/src/components/check-extensions/check-extensions-config.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
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.
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
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.
📚 Learning: 2025-07-30T15:23:03.295Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2079
File: router/pkg/config/config.schema.json:2942-2954
Timestamp: 2025-07-30T15:23:03.295Z
Learning: OCI (Open Container Initiative) registry URLs in the Cosmo router project should not include HTTP/HTTPS schemas. They are specified as hostnames only (e.g., "registry.example.com" or "registry.example.com/namespace"). The JSON schema validation should use plain "string" type without "http-url" format for plugin registry URLs.
Applied to files:
studio/src/components/check-extensions/check-extensions-config.tsx
🧬 Code graph analysis (1)
studio/src/components/check-extensions/check-extensions-config.tsx (3)
connect/src/wg/cosmo/platform/v1/platform_pb.ts (1)
ConfigureSubgraphCheckExtensionsRequest(21174-21254)studio/src/hooks/use-workspace.ts (1)
useWorkspace(4-11)studio/src/lib/constants.ts (1)
docsBaseURL(1-1)
⏰ 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). (15)
- GitHub Check: build-router
- GitHub Check: build_push_image (nonroot)
- GitHub Check: build_test
- GitHub Check: integration_test (./events)
- GitHub Check: build_push_image
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: image_scan (nonroot)
- GitHub Check: image_scan
- GitHub Check: integration_test (./telemetry)
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (5)
studio/src/components/check-extensions/check-extensions-config.tsx (5)
1-24: LGTM!Imports are well-organized and the type alias correctly derives from the protobuf message, omitting the
namespacefield that's handled separately.
26-71: LGTM!The validation schema correctly handles the URL validation with appropriate relaxation for localhost development environments while enforcing HTTPS for production endpoints.
73-92: LGTM!Interface definitions are well-structured with appropriate typing. The callback pattern in
onSaveChangeselegantly handles post-update form synchronization.
134-197: LGTM!Well-structured toggleable options with feature-specific documentation links, appropriate disabled states, and helpful inline guidance for enabling required namespace features.
345-350: Verify: Checkbox appears checked when main feature is disabled.The expression
!item.isDisabled && (!enableSubgraphCheckExtensions || field.value === true)causes all non-disabled checkboxes to appear checked whenenableSubgraphCheckExtensionsisfalse.If this is intentional (to show default selections before enabling), consider adding a visual hint or tooltip explaining "these options will be active once subgraph check extensions are enabled." If unintentional, the logic should only reflect
field.value:- checked={!item.isDisabled && (!enableSubgraphCheckExtensions || field.value === true)} + checked={!item.isDisabled && field.value === true}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
controlplane/src/core/repositories/OrganizationRepository.ts (1)
1007-1016: Subgraph check blob cleanup is consistent with existing patternAdding
blobPromises.push(blobStorage.removeDirectory({ key: \${organizationId}/subgraph_checks` }));reuses the existing cleanup approach (collect promises,Promise.allSettled`) and won’t block org deletion on best-effort blob removal. This looks correct for removing org‑scoped subgraph check artifacts.If more org‑level blob directories appear over time, consider centralizing the list of directories to delete into a small helper to avoid future drift, but that’s optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
controlplane/src/core/repositories/OrganizationRepository.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
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.
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
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.
📚 Learning: 2025-08-29T10:28:04.846Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
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/src/core/repositories/OrganizationRepository.ts
📚 Learning: 2025-08-31T18:51:32.185Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/bufservices/check/getCheckSummary.ts:0-0
Timestamp: 2025-08-31T18:51:32.185Z
Learning: In the SchemaCheckRepository.getLinkedSchemaCheck method, organization-level security is implemented through post-query validation by checking `check.subgraphs[0].namespace.organizationId !== organizationId` and returning undefined if the linked check doesn't belong to the caller's organization, preventing cross-tenant data leakage.
Applied to files:
controlplane/src/core/repositories/OrganizationRepository.ts
⏰ 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). (15)
- GitHub Check: build-router
- GitHub Check: build_push_image
- GitHub Check: build_push_image (nonroot)
- GitHub Check: integration_test (./events)
- GitHub Check: image_scan (nonroot)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: build_test
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: image_scan
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: build_test
- GitHub Check: Analyze (go)
- GitHub Check: build_push_image
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
controlplane/src/core/repositories/OrganizationRepository.ts (1)
1378-1403: New 'subgraph-check-extensions' feature default is wired correctly; just confirm type/plan coverageAdding
'subgraph-check-extensions': falseto the defaultlistkeepsgetOrganizationFeatures’s “full list of features with default values” invariant and lets the merge logic below treat it as a boolean flag.Please double‑check that:
FeatureIdsincludes'subgraph-check-extensions', and- Relevant billing plans (and any feature gating code) have been updated to include this feature where needed.
2 participants
Summary by CodeRabbit
New Features
Behavior Changes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist