feat: local cosmo cloud registry#2778
Merged
comatory merged 6 commits intoMay 15, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds an ChangesInsecure OCI Plugin Registry Support
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2778 +/- ##
==========================================
- Coverage 58.96% 57.77% -1.19%
==========================================
Files 240 393 +153
Lines 26387 40082 +13695
Branches 0 661 +661
==========================================
+ Hits 15558 23157 +7599
- Misses 9314 15503 +6189
+ Partials 1515 1422 -93
🚀 New features to boost your workflow:
|
Router-nonroot image scan passed✅ No security vulnerabilities found in image: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-compose.yml`:
- Around line 382-383: The docker-compose ports mapping currently publishes the
registry on all interfaces via the line '- '5050:5000'', exposing an insecure
unauthenticated registry to the LAN; change that mapping to bind to loopback by
replacing the host portion with 127.0.0.1 (i.e., update the '- '5050:5000''
ports entry to use 127.0.0.1:5050:5000') so the registry listens only on
localhost.
In `@router/.env.example`:
- Around line 15-18: Dotenv-linter flags UnorderedKey because the plugin env
variables are not alphabetically ordered; reorder the three keys so they are
alphabetical (PLUGINS_ENABLED, PLUGINS_REGISTRY_INSECURE, PLUGINS_REGISTRY_URL)
while preserving their values (e.g., true/localhost:5050) so the dotenv-linter
no longer reports UnorderedKey for PLUGINS_REGISTRY_URL,
PLUGINS_REGISTRY_INSECURE, and PLUGINS_ENABLED.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eeebaee6-477f-40bb-bb58-0aa34c5e7a67
📒 Files selected for processing (7)
cli/.env.exampledocker-compose.ymlrouter-tests/testenv/testenv.gorouter/.env.examplerouter/core/graph_server.gorouter/pkg/config/config.gorouter/pkg/grpcconnector/grpcpluginoci/grpc_oci_plugin.go
comatory
requested review from
a team,
Aenimus,
JivusAyrus,
Noroth,
SkArchon,
StarpTech,
devsergiy,
endigma,
jensneuse and
wilsonrivera
as code owners
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
endigma
requested changes
May 15, 2026
comatory
requested review from
alepane21,
dkorittki,
pepol and
thisisnithin
as code owners
Dependency ReviewThe following issues were found:
|
comatory
force-pushed
the
ondrej/eng-9355-cosmo-run-local-instance-of-cosmo-cloud-registry
branch
from
f4e40ce to
4a19226
Compare
comatory
force-pushed
the
ondrej/eng-9355-cosmo-run-local-instance-of-cosmo-cloud-registry
branch
from
4a19226 to
17dcc34
Compare
github-actions
Bot
removed
studio
monorepo
controlplane
cli
protocol
connect
graphqlmetrics
docs-website
labels
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
.github/workflows/composition-ci.yaml (1)
45-52:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftAdd explicit
rootDirconfiguration to composition/tsconfig.json.The TypeScript compilation is failing with error TS5011 because
rootDiris not explicitly set. The composition package's tsconfig.json extends the base config but neither definesrootDir. Add"rootDir": "./src"to the compilerOptions to resolve this.Note: The CI job failures mentioned in the original comment ("wgc CI / build_bun_matrix", "build_test_default", etc.) do not exist in this workflow file. The actual job is
build_test, which will fail at the Build step (line 46) due to this TypeScript configuration issue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/composition-ci.yaml around lines 45 - 52, Add an explicit rootDir to the composition package TypeScript config by updating composition/tsconfig.json's compilerOptions to include "rootDir": "./src" so the TypeScript compiler has a defined source root (this fixes TS5011 during the Build step); edit the compilerOptions block in composition/tsconfig.json and add the rootDir entry without changing other inherited settings.cli/src/commands/subgraph/commands/publish.ts (1)
150-154:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
process.exitCode = 1for proposal match failure.The
ERR_SCHEMA_MISMATCH_WITH_APPROVED_PROPOSALcase was changed fromprocess.exitCode = 1; return;to justbreak;to allow truncation warnings to display. However, this error case should still set a non-zero exit code to signal command failure. Thedefaultcase (line 252) correctly setsprocess.exitCode = 1.🔧 Proposed fix
case EnumStatusCode.ERR_SCHEMA_MISMATCH_WITH_APPROVED_PROPOSAL: { spinner.fail(`Failed to publish subgraph "${name}".`); console.log(pc.red(`Error: Proposal match failed`)); console.log(pc.red(resp.proposalMatchMessage)); + process.exitCode = 1; break; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/src/commands/subgraph/commands/publish.ts` around lines 150 - 154, The ERR_SCHEMA_MISMATCH_WITH_APPROVED_PROPOSAL branch (EnumStatusCode.ERR_SCHEMA_MISMATCH_WITH_APPROVED_PROPOSAL) currently calls spinner.fail and prints resp.proposalMatchMessage but does not set a non-zero exit code; restore setting process.exitCode = 1 in this case after the logs (while keeping the break so truncation warnings can still display) so the CLI signals failure—update the case that calls spinner.fail(`Failed to publish subgraph "${name}".`) and prints pc.red messages to also set process.exitCode = 1.composition/src/v1/federation/federation-factory.ts (1)
2969-3174:⚠️ Potential issue | 🟠 Major | ⚡ Quick winContract-added
@inaccessiblenever gets registered for definition emission.These branches mutate
persistedDirectivesData.directivesByNamedirectly, but#addDirectiveDefinitionsAndDepsToRouterAndClientSchemas()only emits directive definitions fromreferencedPersistedDirectiveNames. On a v2 graph with no preexisting@inaccessibleusage, the contract can now write@inaccessibleinto the output without ever addingINACCESSIBLE_DEFINITION.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composition/src/v1/federation/federation-factory.ts` around lines 2969 - 3174, The contract logic sets INACCESSIBLE into persistedDirectivesData.directivesByName (in buildFederationContractResult and helper branches like handleChildTagInclusions/handleChildTagExclusions) but never registers that name for emission; update every code path that writes INACCESSIBLE to also register it with the router/client emission set (e.g., call the existing registration helper or do this.referencedPersistedDirectiveNames.add(INACCESSIBLE)) so addDirectiveDefinitionsAndDepsToRouterAndClientSchemas will emit INACCESSIBLE_DEFINITION on v2 graphs without preexisting usage.controlplane/src/core/bufservices/graph/setGraphRouterCompatibilityVersion.ts (2)
100-114:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPersist the compatibility version in the no-subgraph fast path.
This branch returns success without calling
updateRouterCompatibilityVersion(), so empty graphs keep the old compatibility version in storage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controlplane/src/core/bufservices/graph/setGraphRouterCompatibilityVersion.ts` around lines 100 - 114, The no-subgraph fast path returns success but never persists the new router compatibility version; call the same persistence used elsewhere (updateRouterCompatibilityVersion) before returning so storage reflects the new value. In setGraphRouterCompatibilityVersion, when subgraphs.length === 0, invoke updateRouterCompatibilityVersion with the graph id and the computed version (the same value used for newVersion), then return the response; ensure federatedGraph.routerCompatibilityVersion is updated in the returned previousVersion/newVersion fields consistently after persistence.
121-184:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn the transaction result instead of falling through to unconditional success.
The transaction callback builds error responses for composition and deployment failures, but nothing captures that return value. The outer handler still reaches the
OKresponse at Line 186, so callers cannot observe a failed compose/deploy.💡 Proposed fix
- await opts.db.transaction(async (tx) => { + const result = await opts.db.transaction(async (tx) => { const fedGraphRepo = new FederatedGraphRepository(logger, tx, authContext.organizationId); - - const auditLogRepo = new AuditLogRepository(opts.db); + const auditLogRepo = new AuditLogRepository(tx); await fedGraphRepo.updateRouterCompatibilityVersion(federatedGraph.id, version); await auditLogRepo.addAuditLog({ organizationId: authContext.organizationId, @@ if (composition.deploymentErrors.length > 0) { return { response: { code: EnumStatusCode.ERR_DEPLOYMENT_FAILED, @@ deploymentErrors: composition.deploymentErrors, }; } + + return { + response: { + code: EnumStatusCode.OK, + }, + previousVersion: federatedGraph.routerCompatibilityVersion, + newVersion: version, + compositionErrors: [], + compositionWarnings: [], + deploymentErrors: [], + }; }); - - return { - response: { - code: EnumStatusCode.OK, - }, - previousVersion: federatedGraph.routerCompatibilityVersion, - newVersion: version, - compositionErrors: [], - compositionWarnings: [], - deploymentErrors: [], - }; + return result;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controlplane/src/core/bufservices/graph/setGraphRouterCompatibilityVersion.ts` around lines 121 - 184, The transaction callback sometimes returns error response objects from composeAndDeployGraphs (calls to fedGraphRepo.composeAndDeployGraphs and auditLogRepo.addAuditLog), but that return value is never captured so the outer handler always proceeds to the unconditional OK; fix by capturing and propagating the transaction result: assign the result of opts.db.transaction(...) to a variable (e.g., txResult), ensure the transaction callback returns the error response objects it already constructs on composition/deployment failures, and then check txResult after the await; if txResult contains a non-OK response (e.g., response.code is an error) return it immediately instead of falling through to the OK response.controlplane/src/core/bufservices/monograph/updateMonograph.ts (1)
138-180:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPropagate composition/deployment results from the repository calls.
FederatedGraphRepository.update()andSubgraphRepository.update()return compose/deploy status elsewhere in this PR. Ignoring both results here means the handler still sendsMONOGRAPH_SCHEMA_UPDATEDand returnsOKwhen recomposition or deployment fails.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controlplane/src/core/bufservices/monograph/updateMonograph.ts` around lines 138 - 180, The calls to fedGraphRepo.update and subgraphRepo.update currently ignore their return values which carry compose/deploy status; capture each call's result (e.g., const fedRes = await fedGraphRepo.update(...); const subRes = await subgraphRepo.update(...)) and check for failure flags or error details returned by those methods, and if either indicates composition/deployment failed, stop emitting MONOGRAPH_SCHEMA_UPDATED and propagate a non-OK response (throw or return an error result) to the caller; only proceed to emit the event and return OK when both results indicate success.controlplane/src/core/repositories/FederatedGraphRepository.ts (1)
1526-1864:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftMove router upload and admission calls out of the SQL transaction.
This transaction persists schema versions and then performs blob uploads, admission-webhook calls, and cache-warmer work before commit. If the SQL transaction rolls back afterward, the CDN can already point at artifacts for a schema version that never committed, and the long network I/O keeps DB locks open much longer than necessary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controlplane/src/core/repositories/FederatedGraphRepository.ts` around lines 1526 - 1864, The transaction is doing long-running network work (composeAndUploadRouterConfig, blob uploads, admission-webhook calls) after saving schema versions which risks inconsistent state if the transaction rolls back; refactor db.transaction so that inside the transaction you only do reads/writes and call saveComposition (Composer.saveComposition) and collect the minimal upload tasks/results (e.g., federatedGraph.id/name/namespace, baseCompositionData, contractBaseCompositionDataByContractId entries, actorId, admissionConfig, blobStorage, featureFlagRouterExecutionConfig maps) into a plain array to return from the transaction, and then after the transaction completes perform the composer.composeAndUploadRouterConfig calls and related filtering (building allDeploymentErrors) using that returned array; keep composition error/warning accumulation (allCompositionErrors/allCompositionWarnings) inside the transaction but compute deploymentErrors after commit so composeAndUploadRouterConfig, Upload/Admission calls, and network I/O happen outside db.transaction.
🟡 Minor comments (6)
composition-go/shim/tsup.config.ts-45-45 (1)
45-45:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate reference to match corrected variable name.
If the typo in
stringHashPLuginis fixed on line 21, this reference will also need to be updated tostringHashPlugin.📝 Proposed fix
- esbuildPlugins: [polyfillNodePlugin, polyfillPlugin, stringHashPLugin], + esbuildPlugins: [polyfillNodePlugin, polyfillPlugin, stringHashPlugin],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composition-go/shim/tsup.config.ts` at line 45, The esbuildPlugins array references the misspelled identifier stringHashPLugin; change that entry to the corrected name stringHashPlugin so it matches the variable declared earlier (stringHashPlugin) and avoids a runtime/reference error in the esbuildPlugins assignment.composition-go/shim/tsup.config.ts-21-21 (1)
21-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix typo in variable name.
stringHashPLuginshould bestringHashPlugin(lowercase 'p').📝 Proposed fix
-const stringHashPLugin: Plugin = { +const stringHashPlugin: Plugin = {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composition-go/shim/tsup.config.ts` at line 21, Rename the misspelled variable stringHashPLugin to stringHashPlugin (lowercase 'p') wherever it's declared and referenced; update the const declaration (const stringHashPlugin: Plugin = { ... }) and any usages/imports/exports that reference stringHashPLugin so the symbol name is consistent throughout the module (e.g., in plugins array or default export).controlplane/src/core/bufservices/feature-flag/deleteFeatureFlag.ts-160-180 (1)
160-180:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent per-graph error reporting in webhook payload.
The
errorsfield uses a global check (compositionErrors.length > 0 || deploymentErrors.length > 0), butcreateFeatureFlag.tsandupdateFeatureFlag.tsuse per-graph checks like:compositionErrors.some((error) => error.federatedGraphName === graph.name) || deploymentErrors.some((error) => error.federatedGraphName === graph.name)This means if one graph fails, all graphs receive
errors: truein their webhook payloads, even those that succeeded.Proposed fix for per-graph error checking
for (const graph of federatedGraphs) { + const hasErrors = + compositionErrors.some((error) => error.federatedGraphName === graph.name) || + deploymentErrors.some((error) => error.federatedGraphName === graph.name); orgWebhooks.send( { eventName: OrganizationEventName.FEDERATED_GRAPH_SCHEMA_UPDATED, payload: { federated_graph: { id: graph.id, name: graph.name, namespace: graph.namespace, }, organization: { id: authContext.organizationId, slug: authContext.organizationSlug, }, - errors: compositionErrors.length > 0 || deploymentErrors.length > 0, + errors: hasErrors, actor_id: authContext.userId, }, }, authContext.userId, ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controlplane/src/core/bufservices/feature-flag/deleteFeatureFlag.ts` around lines 160 - 180, The webhook payload currently sets errors using a global check (compositionErrors.length > 0 || deploymentErrors.length > 0) inside the federatedGraphs loop; change this to a per-graph check so only the relevant graph includes errors. In the loop that calls orgWebhooks.send for OrganizationEventName.FEDERATED_GRAPH_SCHEMA_UPDATED, replace the errors value with a predicate that tests whether compositionErrors.some(e => e.federatedGraphName === graph.name) || deploymentErrors.some(e => e.federatedGraphName === graph.name), referencing the federatedGraphs iteration variable (graph), compositionErrors, deploymentErrors, and the orgWebhooks.send call to locate and update the code.controlplane/src/core/bufservices/feature-flag/enableFeatureFlag.ts-143-163 (1)
143-163:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent per-graph error reporting in webhook payload.
Same issue as in
deleteFeatureFlag.ts: theerrorsfield uses a global check rather than per-graph filtering. This is inconsistent withcreateFeatureFlag.tsandupdateFeatureFlag.tswhich correctly check errors per graph.Proposed fix for per-graph error checking
for (const graph of federatedGraphs) { + const hasErrors = + compositionErrors.some((error) => error.federatedGraphName === graph.name) || + deploymentErrors.some((error) => error.federatedGraphName === graph.name); orgWebhooks.send( { eventName: OrganizationEventName.FEDERATED_GRAPH_SCHEMA_UPDATED, payload: { federated_graph: { id: graph.id, name: graph.name, namespace: graph.namespace, }, organization: { id: authContext.organizationId, slug: authContext.organizationSlug, }, - errors: compositionErrors.length > 0 || deploymentErrors.length > 0, + errors: hasErrors, actor_id: authContext.userId, }, }, authContext.userId, ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controlplane/src/core/bufservices/feature-flag/enableFeatureFlag.ts` around lines 143 - 163, The webhook payload currently sets errors using global compositionErrors/deploymentErrors which misreports per-graph status; update the loop in enableFeatureFlag where orgWebhooks.send is called so that the errors boolean is computed for each graph separately by checking only errors related to that graph (e.g., filter compositionErrors and deploymentErrors for entries matching the current graph.id or graph.namespace/other identifying field) and use that per-graph result in the payload instead of the global arrays.composition/src/v1/normalization/params.ts-15-62 (1)
15-62:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
interfacefor exported object-shaped parameter types incomposition/src/v1/normalization/types/params.ts.Lines 8–49 export object-shaped
typealiases; convert these tointerfacedeclarations to align with TypeScript conventions used throughout the codebase (e.g.,JWTEncodeParams,MailerParams,PluginPublishParams).As per coding guidelines:
**/*.{ts,tsx}: Prefer interfaces over type aliases for object shapes in TypeScript.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composition/src/v1/normalization/params.ts` around lines 15 - 62, Replace the exported object-shaped type aliases with exported interface declarations: Convert ValidateOneOfDirectiveParams, HandleFieldInheritableDirectivesParams, HandleNonExternalConditionalFieldParams, BatchNormalizeParams, NormalizationFactoryParams, NormalizeSubgraphParams, NormalizeSubgraphFromStringParams, and SanitizeDefaultValueParams into interfaces that preserve the exact property names, types, optional markers (e.g., options?), and existing JSDoc/comments; keep the same exported identifiers so usages remain unchanged and ensure any references to InputObjectDefinitionData, DirectiveName, Subgraph, Graph, DocumentNode, etc., continue to resolve.composition/src/v1/federation/params.ts-23-62 (1)
23-62:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrefer
interfacefor exported object-shaped params.The object-shaped
typealiases in lines 30–71 should be converted tointerfacedeclarations to align with TypeScript guidelines. This includesValidateOneOfDirectiveParams,ExtractFederatedDirectivesParams,FederationParams,FederateSubgraphsWithContractsV1Params,FederateSubgraphsContractV1Params, andFederationFactoryParams.💡 Example refactor pattern
-export type FederationParams = { +export interface FederationParams { subgraphs: Array<Subgraph>; options?: CompositionOptions; -}; +}As per coding guidelines:
**/*.{ts,tsx}: Prefer interfaces over type aliases for object shapes in TypeScript.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composition/src/v1/federation/params.ts` around lines 23 - 62, Convert the exported object-shaped type aliases into exported interfaces: replace each "export type ValidateOneOfDirectiveParams = { ... }", "ExtractPersistedDirectivesParams", "FederationParams", "FederateSubgraphsWithContractsV1Params", "FederateSubgraphsContractV1Params", and "FederationFactoryParams" with "export interface <Name> { ... }" keeping the same property names and types (e.g., data: InputObjectDefinitionData; inputValueNodes: Array<InputValueDefinitionNode>; etc. and the maps/sets in FederationFactoryParams). Ensure the exported identifier names and member signatures are unchanged so callers of ValidateOneOfDirectiveParams, ExtractPersistedDirectivesParams, FederationParams, FederateSubgraphsWithContractsV1Params, FederateSubgraphsContractV1Params, and FederationFactoryParams continue to work.
🧹 Nitpick comments (4)
cli/src/commands/router/commands/plugin/commands/publish.ts (1)
16-58: ⚡ Quick winConsider simplifying platform detection logic.
The
getDefaultPlatforms()function uses a verbose switch statement to map Node.js platform/arch to Docker platform format. Consider using a lookup table for cleaner code:♻️ Proposed refactor using a platform map
function getDefaultPlatforms(): string[] { const supportedPlatforms = ['linux/amd64', 'linux/arm64', 'darwin/amd64', 'darwin/arm64', 'windows/amd64']; const defaultPlatforms = ['linux/amd64']; - // Get current OS and architecture const currentPlatform = platform(); const currentArch = arch(); - - // Map Node.js platform/arch to Docker platform format - let dockerPlatform: string | null = null; - - switch (currentPlatform) { - case 'linux': { - if (currentArch === 'x64') { - dockerPlatform = 'linux/amd64'; - } else if (currentArch === 'arm64') { - dockerPlatform = 'linux/arm64'; - } - break; - } - case 'darwin': { - if (currentArch === 'x64') { - dockerPlatform = 'darwin/amd64'; - } else if (currentArch === 'arm64') { - dockerPlatform = 'darwin/arm64'; - } - break; - } - case 'win32': { - if (currentArch === 'x64') { - dockerPlatform = 'windows/amd64'; - } - break; - } - } + + const platformMap: Record<string, Record<string, string>> = { + linux: { x64: 'linux/amd64', arm64: 'linux/arm64' }, + darwin: { x64: 'darwin/amd64', arm64: 'darwin/arm64' }, + win32: { x64: 'windows/amd64' }, + }; + + const dockerPlatform = platformMap[currentPlatform]?.[currentArch]; - // Add user's platform to defaults if supported and not already included if (dockerPlatform && supportedPlatforms.includes(dockerPlatform) && !defaultPlatforms.includes(dockerPlatform)) { defaultPlatforms.push(dockerPlatform); } return defaultPlatforms; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/src/commands/router/commands/plugin/commands/publish.ts` around lines 16 - 58, The getDefaultPlatforms() function uses a verbose switch to map Node.js platform/arch to Docker platform strings; replace that switch with a small lookup map keyed by platform+arch (e.g., `${platform()}|${arch()}`) that returns the dockerPlatform string, then use the existing supportedPlatforms and defaultPlatforms logic to push the mapped value only if it exists, is supported, and not already present; update references to dockerPlatform, supportedPlatforms, defaultPlatforms, platform(), and arch() accordingly while preserving the final return of defaultPlatforms.composition-go/README.md (1)
6-6: 💤 Low valueMarkdown heading style inconsistency flagged by linter.
The markdownlint tool expects setext-style headings (
====underlines) but ATX-style headings (##) are used. While this is a style preference and not a functional issue, consider making heading styles consistent across the repository's documentation.As per coding guidelines, this matches the static analysis hint from markdownlint-cli2.
Also applies to: 74-74, 140-140, 146-146
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composition-go/README.md` at line 6, Replace ATX-style headings in README.md with setext-style underlines to satisfy markdownlint; for example change the "## Composition" header (and the other flagged headings in the file) to the setext form by putting the heading text on one line and an underline of = (for top-level) or - (for second-level) on the next line. Update each flagged heading occurrence consistently so the repository uses setext-style headings across README.md.composition-go/generate.sh (1)
3-3: 💤 Low valueFix typo in comment.
"require" should be "required".
📝 Proposed fix
-# This scripts generates the files require for running +# This script generates the files required for running🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composition-go/generate.sh` at line 3, Update the top-line comment in generate.sh to correct the typo: change "This scripts generates the files require for running" to use the proper wording (e.g., "This script generates the files required for running") so the comment accurately reads that the script generates required files; edit the sentence around the existing header comment in the generate.sh file to singular "script" and "required".composition/src/federation/params.ts (1)
7-25: ⚡ Quick winModel these exported parameter contracts as interfaces.
These are public object shapes, so using interfaces keeps the package API aligned with the repo's TypeScript convention and makes future extension/declaration merging easier.
💡 Proposed fix
-export type FederateSubgraphsParams = { +export interface FederateSubgraphsParams { subgraphs: Array<Subgraph>; options?: CompositionOptions; version?: SupportedRouterCompatibilityVersion; -}; +} -export type FederateSubgraphsWithContractsParams = { +export interface FederateSubgraphsWithContractsParams { subgraphs: Array<Subgraph>; tagOptionsByContractName: Map<ContractName, ContractTagOptions>; options?: CompositionOptions; version?: SupportedRouterCompatibilityVersion; -}; +} -export type FederateSubgraphsContractParams = { +export interface FederateSubgraphsContractParams { contractTagOptions: ContractTagOptions; subgraphs: Array<Subgraph>; options?: CompositionOptions; version?: SupportedRouterCompatibilityVersion; -}; +}As per coding guidelines,
**/*.{ts,tsx}: Prefer interfaces over type aliases for object shapes in TypeScript.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composition/src/federation/params.ts` around lines 7 - 25, Convert the three exported object-shaped type aliases into exported interfaces: replace the type alias declarations for FederateSubgraphsParams, FederateSubgraphsWithContractsParams, and FederateSubgraphsContractParams with interface declarations that retain the same properties and optional modifiers (subgraphs: Array<Subgraph>, options?: CompositionOptions, version?: SupportedRouterCompatibilityVersion, and for the contracts variants include tagOptionsByContractName: Map<ContractName, ContractTagOptions> and contractTagOptions: ContractTagOptions respectively). Ensure the exported names remain identical and keep the existing referenced types (Subgraph, CompositionOptions, SupportedRouterCompatibilityVersion, ContractName, ContractTagOptions) so consumers and declaration merging work as expected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/composition-ci.yaml:
- Around line 65-67: The action metadata for the go-linter custom action
incorrectly includes a forbidden key "type: string" on the working-directory
input; edit the action metadata where the input "working-directory" is defined
in .github/actions/go-linter/action.yaml and remove the "type: string" line so
the input only uses supported keys (e.g., description, required, default,
deprecationMessage), then commit the change so the workflow using uses:
./.github/actions/go-linter with input working-directory: ./composition-go
passes actionlint validation.
In @.github/workflows/release.yaml:
- Around line 32-33: The workflow uses the undefined environment variable
env.WORKING_DIRECTORY in the step input working-directory, so either add a
WORKING_DIRECTORY entry to the workflow env block (set a default path string) or
remove the working-directory: ${{ env.WORKING_DIRECTORY }} line from the step so
the action uses its default working directory; update the env block or the step
that references working-directory to ensure the variable is defined or omitted
(symbols: env.WORKING_DIRECTORY, working-directory).
In `@cdn-server/package.json`:
- Line 3: The package.json "version" field was decreased to "0.19.0" which looks
like an unintentional downgrade; update the "version" value in package.json back
to "0.21.0" (or to the correct semver intended for this PR) so the package
version does not regress, or add a brief justification in the PR description
explaining this deliberate rollback; locate the "version" entry in package.json
to make the change.
In `@cli/package.json`:
- Line 3: The package.json "version" field was unintentionally downgraded from
0.118.1 to 0.114.2; restore the correct semver by updating the "version" value
back to the intended release (e.g., "0.118.1") in cli/package.json's "version"
property so it matches the expected release tag and CI/release tooling.
- Line 55: Update the axios dependency in package.json from "1.13.5" to a secure
version (at least 1.15.2; prefer "^1.16.1"), then reinstall and commit updated
lockfile; specifically edit the "axios" entry in package.json, run your package
manager (npm install or yarn install) to regenerate package-lock.json /
yarn.lock, run tests/lint, and include the updated lockfile in the PR so the
vulnerability is resolved.
In `@cli/src/commands/router/commands/plugin/commands/publish.ts`:
- Line 187: The duplicated array of platforms should be extracted to a single
module-level constant (e.g., SUPPORTED_PLATFORMS) and both getDefaultPlatforms()
and the validation logic currently declaring supportedPlatforms (used in
publish.ts) should reference that constant instead; remove the local
supportedPlatforms declarations and replace their uses with SUPPORTED_PLATFORMS,
and ensure any functions (getDefaultPlatforms, the publish validation block)
import or access the new constant within the same module.
In `@composition-go/composition.go`:
- Around line 69-98: The HTTP call in introspectSubgraph currently uses
http.DefaultClient which has no timeout; replace that usage by creating and
using a configured *http.Client* with a sensible Timeout (or use a context with
deadline and http.Client{Timeout: ...}) for the request in introspectSubgraph so
the call cannot hang indefinitely; update the code paths around the http
request/resp (the http.NewRequest/DefaultClient.Do sequence) to use the new
client (or context-aware Do) and ensure response.Body is still closed and errors
are wrapped as before.
In `@composition-go/polyfill.go`:
- Line 57: The Password field is being set from the username; update the
assignment so Password uses u.User.Password() instead of u.User.Username();
locate the struct literal where Password: u.User.Username() appears (in
composition-go/polyfill.go, the u.User reference) and replace it with Password:
u.User.Password() to correctly propagate the user's password.
In `@composition/package.json`:
- Line 3: The package.json "version" field was changed from the current
published 0.58.x line back to "0.56.0"; revert this rollback by updating the
"version" value in package.json to a monotonic version that is >= the published
0.58.x (or leave it to your release tooling to bump it forward), ensuring the
`@wundergraph/composition` package does not move to an earlier semver line.
In `@composition/src/schema-building/types.ts`:
- Around line 1-12: The import for Kind is currently using the `type` prefix but
`Kind` is a runtime constant; update the import list in
composition/src/schema-building/types.ts so that `Kind` is imported as a value
(remove the `type` before `Kind`) while keeping the other imports as type-only
(e.g., ConstDirectiveNode, ConstValueNode, DirectiveDefinitionNode,
DocumentNode, NamedTypeNode, OperationTypeDefinitionNode, OperationTypeNode,
StringValueNode, TypeNode) so runtime references like
`Kind.ENUM_TYPE_DEFINITION` and `Kind.FIELD_DEFINITION` work.
In `@composition/src/v1/federation/federation-factory.ts`:
- Around line 295-305: extractPersistedDirectives is only checking
persistedDirectiveDefinitionByDirectiveName which is seeded with built-ins, so
custom persisted directives collected in
potentialPersistedDirectiveDefinitionDataByDirectiveName never get emitted;
update the factory to merge entries from
potentialPersistedDirectiveDefinitionDataByDirectiveName into
persistedDirectiveDefinitionByDirectiveName (or make extractPersistedDirectives
consult the potential map as well) before calling extractPersistedDirectives so
custom directive definitions (and their names tracked via
referencedPersistedDirectiveNames) are preserved and emitted alongside
built-ins.
- Around line 1642-1658: When detecting that a later subgraph has no persisted
executable directives (the check on
internalSubgraph.persistedDirectiveDefinitionDataByDirectiveName.size === 0),
also clear the accumulated candidates in
potentialPersistedDirectiveDefinitionDataByDirectiveName before setting
shouldSkipPersistedExecutableDirectives and continuing; this ensures stale
entries aren't kept and later emitted by
buildFederationResult()/buildFederationContractResult(). Update the branch that
sets shouldSkipPersistedExecutableDirectives to call
potentialPersistedDirectiveDefinitionDataByDirectiveName.clear() (or equivalent
removal) so no old candidates survive.
In `@controlplane/src/core/bufservices/federated-graph/migrateFromApollo.ts`:
- Around line 156-170: After calling fedGraphRepo.composeAndDeployGraphs(...)
check its returned result for structured errors and handle them before
proceeding: if the result indicates composition or deployment failures, abort
the migration flow (return/throw), log or surface the errors, and do not call
subsequent steps that mint the graph token or emit the APOLLO_MIGRATE_SUCCESS
event; update the migrateFromApollo flow to inspect the composeAndDeployGraphs
return value and only continue to mint the token and emit APOLLO_MIGRATE_SUCCESS
when the compose/deploy result is successful.
In `@controlplane/src/core/bufservices/federated-graph/moveFederatedGraph.ts`:
- Around line 133-175: The webhook payload is still using the original
compositionErrors/deploymentErrors variables instead of the aggregated arrays
built while moving contracts; update the webhook error reporting to reference
the aggregated variables (allCompositionErrors, allDeploymentErrors,
allCompositionWarnings) so contract move failures collected in the loop that
pushes into those arrays are reflected in the emitted webhook; locate the
webhook loop that constructs the payload after the for-loop (the section that
currently reads compositionErrors/deploymentErrors) and replace those references
with the corresponding all* arrays populated during the fedGraphRepo.move loop.
In `@controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts`:
- Around line 759-783: The current early-return when compositionErrors.length >
0 clears deploymentErrors (and vice versa), losing valid failures; update the
return in the composition-failure branch inside publishFederatedSubgraph (where
compositionErrors, boundedCompositionErrors, boundedCompositionWarnings,
deploymentErrors, boundedDeploymentErrors and EnumStatusCode are used) to
include the boundedDeploymentErrors (and not an empty deploymentErrors array) so
both composition and deployment failures are preserved in the response, and make
the symmetric change in the deployment-failure branch to include
boundedCompositionErrors instead of clearing compositionErrors; keep the
response codes as-is but preserve both bounded arrays and
counts/proposalMatchMessage.
In `@controlplane/src/core/composition/composer.ts`:
- Around line 218-228: The early return when routerCompatibilityVersion is
invalid skips persisting the composition failure; instead of returning
immediately after pushing new RouterConfigUploadError into errors, call
graphCompositionRepository.updateComposition(...) to record the deployment
failure (e.g., set deployable=false or persist the errors) before returning;
locate the check around routerCompatibilityVersion/ROUTER_COMPATIBILITY_VERSIONS
and ensure the error handling path uses the same updateComposition call used
later to persist composition state so the saved composition reflects the failed
router-config upload.
In `@controlplane/src/core/repositories/FederatedGraphRepository.ts`:
- Around line 325-336: composeAndDeployGraphs is being invoked with
federatedGraph that still has the pre-update labelMatchers, so compositions use
stale feature-flag selections; update the federatedGraph instance to include the
new labelMatchers (or pass the updated DTO) before calling
composeAndDeployGraphs so it reads the current federatedGraph.labelMatchers;
locate the federatedGraph variable used in this call and ensure you replace or
mutate its labelMatchers with the post-update matcher set (the value produced by
your update flow) prior to invoking FedGraphRepository.composeAndDeployGraphs.
- Around line 420-431: The composeAndDeployGraphs call inside move() is executed
on the outer (non-transactional) repository, which can read stale state; change
the call to use the transactional repository instance so recomposition runs in
the same DB transaction as the namespace/mapping updates. Locate the move()
implementation in FederatedGraphRepository and replace the direct
this.composeAndDeployGraphs(...) invocation with the transactional repo's method
(e.g., this.transactional().composeAndDeployGraphs(...) or the repo instance
passed into the transaction callback), ensuring blobStorage, chClient,
compositionOptions and federatedGraphs (movedContractGraph) are forwarded
unchanged.
---
Outside diff comments:
In @.github/workflows/composition-ci.yaml:
- Around line 45-52: Add an explicit rootDir to the composition package
TypeScript config by updating composition/tsconfig.json's compilerOptions to
include "rootDir": "./src" so the TypeScript compiler has a defined source root
(this fixes TS5011 during the Build step); edit the compilerOptions block in
composition/tsconfig.json and add the rootDir entry without changing other
inherited settings.
In `@cli/src/commands/subgraph/commands/publish.ts`:
- Around line 150-154: The ERR_SCHEMA_MISMATCH_WITH_APPROVED_PROPOSAL branch
(EnumStatusCode.ERR_SCHEMA_MISMATCH_WITH_APPROVED_PROPOSAL) currently calls
spinner.fail and prints resp.proposalMatchMessage but does not set a non-zero
exit code; restore setting process.exitCode = 1 in this case after the logs
(while keeping the break so truncation warnings can still display) so the CLI
signals failure—update the case that calls spinner.fail(`Failed to publish
subgraph "${name}".`) and prints pc.red messages to also set process.exitCode =
1.
In `@composition/src/v1/federation/federation-factory.ts`:
- Around line 2969-3174: The contract logic sets INACCESSIBLE into
persistedDirectivesData.directivesByName (in buildFederationContractResult and
helper branches like handleChildTagInclusions/handleChildTagExclusions) but
never registers that name for emission; update every code path that writes
INACCESSIBLE to also register it with the router/client emission set (e.g., call
the existing registration helper or do
this.referencedPersistedDirectiveNames.add(INACCESSIBLE)) so
addDirectiveDefinitionsAndDepsToRouterAndClientSchemas will emit
INACCESSIBLE_DEFINITION on v2 graphs without preexisting usage.
In
`@controlplane/src/core/bufservices/graph/setGraphRouterCompatibilityVersion.ts`:
- Around line 100-114: The no-subgraph fast path returns success but never
persists the new router compatibility version; call the same persistence used
elsewhere (updateRouterCompatibilityVersion) before returning so storage
reflects the new value. In setGraphRouterCompatibilityVersion, when
subgraphs.length === 0, invoke updateRouterCompatibilityVersion with the graph
id and the computed version (the same value used for newVersion), then return
the response; ensure federatedGraph.routerCompatibilityVersion is updated in the
returned previousVersion/newVersion fields consistently after persistence.
- Around line 121-184: The transaction callback sometimes returns error response
objects from composeAndDeployGraphs (calls to
fedGraphRepo.composeAndDeployGraphs and auditLogRepo.addAuditLog), but that
return value is never captured so the outer handler always proceeds to the
unconditional OK; fix by capturing and propagating the transaction result:
assign the result of opts.db.transaction(...) to a variable (e.g., txResult),
ensure the transaction callback returns the error response objects it already
constructs on composition/deployment failures, and then check txResult after the
await; if txResult contains a non-OK response (e.g., response.code is an error)
return it immediately instead of falling through to the OK response.
In `@controlplane/src/core/bufservices/monograph/updateMonograph.ts`:
- Around line 138-180: The calls to fedGraphRepo.update and subgraphRepo.update
currently ignore their return values which carry compose/deploy status; capture
each call's result (e.g., const fedRes = await fedGraphRepo.update(...); const
subRes = await subgraphRepo.update(...)) and check for failure flags or error
details returned by those methods, and if either indicates
composition/deployment failed, stop emitting MONOGRAPH_SCHEMA_UPDATED and
propagate a non-OK response (throw or return an error result) to the caller;
only proceed to emit the event and return OK when both results indicate success.
In `@controlplane/src/core/repositories/FederatedGraphRepository.ts`:
- Around line 1526-1864: The transaction is doing long-running network work
(composeAndUploadRouterConfig, blob uploads, admission-webhook calls) after
saving schema versions which risks inconsistent state if the transaction rolls
back; refactor db.transaction so that inside the transaction you only do
reads/writes and call saveComposition (Composer.saveComposition) and collect the
minimal upload tasks/results (e.g., federatedGraph.id/name/namespace,
baseCompositionData, contractBaseCompositionDataByContractId entries, actorId,
admissionConfig, blobStorage, featureFlagRouterExecutionConfig maps) into a
plain array to return from the transaction, and then after the transaction
completes perform the composer.composeAndUploadRouterConfig calls and related
filtering (building allDeploymentErrors) using that returned array; keep
composition error/warning accumulation
(allCompositionErrors/allCompositionWarnings) inside the transaction but compute
deploymentErrors after commit so composeAndUploadRouterConfig, Upload/Admission
calls, and network I/O happen outside db.transaction.
---
Minor comments:
In `@composition-go/shim/tsup.config.ts`:
- Line 45: The esbuildPlugins array references the misspelled identifier
stringHashPLugin; change that entry to the corrected name stringHashPlugin so it
matches the variable declared earlier (stringHashPlugin) and avoids a
runtime/reference error in the esbuildPlugins assignment.
- Line 21: Rename the misspelled variable stringHashPLugin to stringHashPlugin
(lowercase 'p') wherever it's declared and referenced; update the const
declaration (const stringHashPlugin: Plugin = { ... }) and any
usages/imports/exports that reference stringHashPLugin so the symbol name is
consistent throughout the module (e.g., in plugins array or default export).
In `@composition/src/v1/federation/params.ts`:
- Around line 23-62: Convert the exported object-shaped type aliases into
exported interfaces: replace each "export type ValidateOneOfDirectiveParams = {
... }", "ExtractPersistedDirectivesParams", "FederationParams",
"FederateSubgraphsWithContractsV1Params", "FederateSubgraphsContractV1Params",
and "FederationFactoryParams" with "export interface <Name> { ... }" keeping the
same property names and types (e.g., data: InputObjectDefinitionData;
inputValueNodes: Array<InputValueDefinitionNode>; etc. and the maps/sets in
FederationFactoryParams). Ensure the exported identifier names and member
signatures are unchanged so callers of ValidateOneOfDirectiveParams,
ExtractPersistedDirectivesParams, FederationParams,
FederateSubgraphsWithContractsV1Params, FederateSubgraphsContractV1Params, and
FederationFactoryParams continue to work.
In `@composition/src/v1/normalization/params.ts`:
- Around line 15-62: Replace the exported object-shaped type aliases with
exported interface declarations: Convert ValidateOneOfDirectiveParams,
HandleFieldInheritableDirectivesParams, HandleNonExternalConditionalFieldParams,
BatchNormalizeParams, NormalizationFactoryParams, NormalizeSubgraphParams,
NormalizeSubgraphFromStringParams, and SanitizeDefaultValueParams into
interfaces that preserve the exact property names, types, optional markers
(e.g., options?), and existing JSDoc/comments; keep the same exported
identifiers so usages remain unchanged and ensure any references to
InputObjectDefinitionData, DirectiveName, Subgraph, Graph, DocumentNode, etc.,
continue to resolve.
In `@controlplane/src/core/bufservices/feature-flag/deleteFeatureFlag.ts`:
- Around line 160-180: The webhook payload currently sets errors using a global
check (compositionErrors.length > 0 || deploymentErrors.length > 0) inside the
federatedGraphs loop; change this to a per-graph check so only the relevant
graph includes errors. In the loop that calls orgWebhooks.send for
OrganizationEventName.FEDERATED_GRAPH_SCHEMA_UPDATED, replace the errors value
with a predicate that tests whether compositionErrors.some(e =>
e.federatedGraphName === graph.name) || deploymentErrors.some(e =>
e.federatedGraphName === graph.name), referencing the federatedGraphs iteration
variable (graph), compositionErrors, deploymentErrors, and the orgWebhooks.send
call to locate and update the code.
In `@controlplane/src/core/bufservices/feature-flag/enableFeatureFlag.ts`:
- Around line 143-163: The webhook payload currently sets errors using global
compositionErrors/deploymentErrors which misreports per-graph status; update the
loop in enableFeatureFlag where orgWebhooks.send is called so that the errors
boolean is computed for each graph separately by checking only errors related to
that graph (e.g., filter compositionErrors and deploymentErrors for entries
matching the current graph.id or graph.namespace/other identifying field) and
use that per-graph result in the payload instead of the global arrays.
---
Nitpick comments:
In `@cli/src/commands/router/commands/plugin/commands/publish.ts`:
- Around line 16-58: The getDefaultPlatforms() function uses a verbose switch to
map Node.js platform/arch to Docker platform strings; replace that switch with a
small lookup map keyed by platform+arch (e.g., `${platform()}|${arch()}`) that
returns the dockerPlatform string, then use the existing supportedPlatforms and
defaultPlatforms logic to push the mapped value only if it exists, is supported,
and not already present; update references to dockerPlatform,
supportedPlatforms, defaultPlatforms, platform(), and arch() accordingly while
preserving the final return of defaultPlatforms.
In `@composition-go/generate.sh`:
- Line 3: Update the top-line comment in generate.sh to correct the typo: change
"This scripts generates the files require for running" to use the proper wording
(e.g., "This script generates the files required for running") so the comment
accurately reads that the script generates required files; edit the sentence
around the existing header comment in the generate.sh file to singular "script"
and "required".
In `@composition-go/README.md`:
- Line 6: Replace ATX-style headings in README.md with setext-style underlines
to satisfy markdownlint; for example change the "## Composition" header (and the
other flagged headings in the file) to the setext form by putting the heading
text on one line and an underline of = (for top-level) or - (for second-level)
on the next line. Update each flagged heading occurrence consistently so the
repository uses setext-style headings across README.md.
In `@composition/src/federation/params.ts`:
- Around line 7-25: Convert the three exported object-shaped type aliases into
exported interfaces: replace the type alias declarations for
FederateSubgraphsParams, FederateSubgraphsWithContractsParams, and
FederateSubgraphsContractParams with interface declarations that retain the same
properties and optional modifiers (subgraphs: Array<Subgraph>, options?:
CompositionOptions, version?: SupportedRouterCompatibilityVersion, and for the
contracts variants include tagOptionsByContractName: Map<ContractName,
ContractTagOptions> and contractTagOptions: ContractTagOptions respectively).
Ensure the exported names remain identical and keep the existing referenced
types (Subgraph, CompositionOptions, SupportedRouterCompatibilityVersion,
ContractName, ContractTagOptions) so consumers and declaration merging work as
expected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@router/pkg/grpcconnector/grpcpluginoci/grpc_oci_plugin.go`:
- Around line 187-197: Before appending crane.Insecure and calling crane.Get in
grpc_oci_plugin.go, validate the registry host parsed from p.imgRef and
fail-fast if p.registryInsecure is true but the host is not an allowed local
value; only allow insecure mode for explicit local hosts (e.g., "localhost",
"127.0.0.1", "::1", "host.docker.internal"). Specifically, in the block that
currently checks p.registryInsecure and appends to opts (the code around the
crane.Insecure append and the subsequent crane.Get call), parse the image
reference to extract the registry hostname, compare it against an allowlist, and
if the host is not allowed return an error instead of enabling insecure pulls;
otherwise continue to append crane.Insecure and proceed to crane.Get.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a0eb1f25-a8c5-4f02-af95-c23bfb9eca75
📒 Files selected for processing (7)
cli/.env.exampledocker-compose.ymlrouter-tests/testenv/testenv.gorouter/.env.examplerouter/core/graph_server.gorouter/pkg/config/config.gorouter/pkg/grpcconnector/grpcpluginoci/grpc_oci_plugin.go
✅ Files skipped from review due to trivial changes (2)
- cli/.env.example
- router/pkg/config/config.go
🚧 Files skipped from review as they are similar to previous changes (3)
- docker-compose.yml
- router-tests/testenv/testenv.go
- router/core/graph_server.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli/.env.example`:
- Around line 4-5: dotenv key ordering is failing lint due to UnorderedKey; swap
the two entries so PLUGIN_REGISTRY_INSECURE appears before PLUGIN_REGISTRY_URL
in the .env example (i.e., alphabetically order the keys
PLUGIN_REGISTRY_INSECURE and PLUGIN_REGISTRY_URL) to satisfy dotenv-linter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 300f2d34-5baa-48a5-bdb2-a3680b651a5d
📒 Files selected for processing (5)
cli/.env.examplecli/src/commands/demo/util.tscli/src/core/config.tsdocker-compose.ymlrouter/.env.example
✅ Files skipped from review due to trivial changes (1)
- router/.env.example
🚧 Files skipped from review as they are similar to previous changes (1)
- docker-compose.yml
endigma
approved these changes
May 15, 2026
comatory
deleted the
ondrej/eng-9355-cosmo-run-local-instance-of-cosmo-cloud-registry
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When developing #2750, it was not straightforward to test the functionality of publishing gRPC plugins and have them loaded in local instance of the router. The work-around was to use non-public staging registry, which required modifications to docker compose files.
This change adds a new service to the compose file
registry-2which serves as mock docker registry. The goal is to simplify local development in two ways:pnpm wgc router plugin publish ...make start-router(given that correct router token is provided)Caveats
You must provide
PLUGIN_REGISTRY_URLaccording to environment example files. Insecure access to docker registry must be set in the docker configuration. Our setup assumes Orbstack, so this"insecure-registries": ["host.docker.internal:5050"]needs to be added to the docker config.Summary by CodeRabbit
How to test this change
make infra-downmake infra-uplocalhost:3002/graphqland--label-matcher graph=demo. Publish subgraphs associated with that graph (I'm using cosmo-onboarding repo, use the plugins inplugins/directory):pnpm wgc router plugin publish --name <products-2|reviews> <path-to-local-folder> --label graph=demo --platform linux/amd64 linux/arm64 darwin/arm64(I'm on mac so Darwin is required).GRAPH_API_TOKEN=<new-token> make start-routerChecklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.