fix: update the registry url in the cli#2134
Conversation
WalkthroughUpdated the default plugin registry URL in config.ts to use cosmo-registry.wundergraph.com when PLUGIN_REGISTRY_URL is unset. No other logic or signatures changed. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
cli/src/core/config.ts (1)
40-46: Avoid sending Authorization: Bearer undefined when apiKey is absentWhen COSMO_API_KEY is unset, the header becomes a literal "Bearer undefined". That’s noisy, can confuse servers, and may leak into logs.
Refactor to include the header only when a key is present:
-export const getBaseHeaders = (): HeadersInit => { - return { - 'user-agent': `cosmo-cli/${info.version}`, - authorization: 'Bearer ' + config.apiKey, - 'cosmo-org-slug': getLoginDetails()?.organizationSlug || '', - }; -}; +export const getBaseHeaders = (): HeadersInit => { + const headers: Record<string, string> = { + 'user-agent': `cosmo-cli/${info.version}`, + 'cosmo-org-slug': getLoginDetails()?.organizationSlug || '', + }; + if (config.apiKey) { + headers.authorization = `Bearer ${config.apiKey}`; + } + return headers; +};
🧹 Nitpick comments (2)
cli/src/core/config.ts (2)
23-38: Optional: support COSMO_PLUGIN_REGISTRY_URL (and keep PLUGIN_REGISTRY_URL as a fallback) for env var consistencyMost envs here use a COSMO_* prefix. Consider supporting COSMO_PLUGIN_REGISTRY_URL first while remaining backward compatible.
You can implement this as:
- pluginRegistryURL: process.env.PLUGIN_REGISTRY_URL || 'https://cosmo-registry.wundergraph.com', + // Prefer COSMO_* naming; keep backward-compatible fallback + pluginRegistryURL: + process.env.COSMO_PLUGIN_REGISTRY_URL + || process.env.PLUGIN_REGISTRY_URL + || 'https://cosmo-registry.wundergraph.com',
1-47: Follow-up: Update tests and docs per the PR checklistThe change switches the default registry to production. Please add/update:
- A small unit/integration test that verifies the default resolves to the production registry when PLUGIN_REGISTRY_URL is unset.
- Docs in cosmo-docs outlining the new default and the env var overrides.
I can draft the tests and a short docs snippet if you confirm the preferred env var naming (keep PLUGIN_REGISTRY_URL only vs. adding COSMO_PLUGIN_REGISTRY_URL).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cli/src/core/config.ts(1 hunks)
🔇 Additional comments (1)
cli/src/core/config.ts (1)
37-37: Ignore scheme addition for pluginRegistryURL; keep host-only registry referenceThe default
pluginRegistryURLis only ever used to construct OCI image references and invoke Docker commands (login, logout). Docker registry references and image tags must be hostname (and optional namespace/path) without anhttp://orhttps://prefix—adding a scheme would break tag parsing anddocker login. A search also shows no usage ofnew URL(...)orfetch(...)with this value.• cli/src/core/config.ts: defines the default as
'cosmo-registry.wundergraph.com'
• cli/src/commands/router/commands/plugin/commands/publish.ts: usesconfig.pluginRegistryURLfor image tags and Docker CLI callsNo change required here.
Likely an incorrect or invalid review comment.
Summary by CodeRabbit
Checklist