fix(release): publish-pipeline + Deploy-tab observability dependency#573
Conversation
kaviththiranga
requested review from
sameerajayasoma and
stefinie123
as code owners
📝 WalkthroughWalkthroughCI publish was changed from conditional Changesets-based steps to always build all workspaces then publish each non-private workspace topologically via ChangesRelease Publishing Strategy Refactor
Observability hooks: optional API handling
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
84-89: ⚡ Quick winGuard
latestdist-tag promotion withLATEST.On Line [84]-Line [89], all stable tags publish with
--tag latest. If an older stable tag is pushed, this can movelatestbackward. UseLATESTto gatelatestpromotion.Suggested patch
- TAG=latest - if [ "${IS_PRERELEASE}" = "true" ]; then - TAG=next - fi + if [ "${IS_PRERELEASE}" = "true" ]; then + TAG=next + elif [ "${LATEST}" = "true" ]; then + TAG=latest + else + echo "::error::Refusing to publish non-latest stable tag ${RELEASE_TAG} as latest" + exit 1 + fi🤖 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/release.yml around lines 84 - 89, The workflow currently always sets TAG=latest unless IS_PRERELEASE is true, which can accidentally move the latest dist-tag backwards; change the logic so TAG is only set to "latest" when the new LATEST environment variable is explicitly "true" (keep TAG="next" when IS_PRERELEASE is "true"), and only supply the --tag argument to the npm publish invocation when TAG is set (i.e., gate passing --tag "${TAG}" behind a conditional). Update the block that defines TAG and the yarn workspaces foreach npm publish invocation to reference TAG, IS_PRERELEASE and LATEST accordingly so latest promotion only happens when LATEST="true".
🤖 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 `@README.md`:
- Around line 523-524: Update the README sentence that currently claims "Yarn
Berry's `yarn npm publish` does not accept a `--dry-run` flag" to reflect the
correct behavior: change it to state that `yarn npm publish` supports a dry-run
via `-n, --dry-run` (or `--dry-run`) and can be used to preview what would be
published without uploading; optionally keep the `yarn pack` suggestion as an
alternative offline check but remove the definitive "does not accept" wording so
the README references the official CLI option (`yarn npm publish -n|--dry-run`)
instead.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 84-89: The workflow currently always sets TAG=latest unless
IS_PRERELEASE is true, which can accidentally move the latest dist-tag
backwards; change the logic so TAG is only set to "latest" when the new LATEST
environment variable is explicitly "true" (keep TAG="next" when IS_PRERELEASE is
"true"), and only supply the --tag argument to the npm publish invocation when
TAG is set (i.e., gate passing --tag "${TAG}" behind a conditional). Update the
block that defines TAG and the yarn workspaces foreach npm publish invocation to
reference TAG, IS_PRERELEASE and LATEST accordingly so latest promotion only
happens when LATEST="true".
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 385c60f9-8722-429d-88e3-6dd46ead7cc9
📒 Files selected for processing (3)
.github/workflows/release.ymlREADME.mdpackage.json
| Before the first real release, validate the publish path locally. Yarn Berry's `yarn npm publish` does not accept a `--dry-run` flag, so the equivalent offline check is `yarn pack` on every public workspace — `yarn pack` runs the same workspace-protocol rewriter that `yarn npm publish` does, just stopping before the upload: | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Yarn 4 yarn npm publishsupport a--dry-run flag? Please return the official Yarn CLI docs page and option list.
💡 Result:
Yes. Yarn 4’s yarn npm publish command supports a dry-run option. Official Yarn CLI docs page: https://yarnpkg.com/cli/npm/publish [1] Option list for yarn npm publish (as shown on that page), including the dry-run flag: - -n,--dry-run — “Show what would be published without actually publishing” [1] Other options listed on the same page: - --access [1] - --tag [1] - --tolerate-republish [1] - --otp [1] - --provenance [1] - --json [1] So you can run: yarn npm publish --dry-run (or -n) to see what would be published without publishing to the registry. [1]
Citations:
Update documentation to reflect yarn npm publish --dry-run support.
Line 523 incorrectly states that Yarn Berry's yarn npm publish does not accept a --dry-run flag. The official Yarn CLI documentation confirms the command supports -n,--dry-run to show what would be published without actually publishing.
Suggested fix:
Patch
-Before the first real release, validate the publish path locally. Yarn Berry's `yarn npm publish` does not accept a `--dry-run` flag, so the equivalent offline check is `yarn pack` on every public workspace — `yarn pack` runs the same workspace-protocol rewriter that `yarn npm publish` does, just stopping before the upload:
+Before the first real release, validate the publish path locally. While `yarn npm publish` supports `--dry-run`, this repo uses an offline `yarn pack` check across all public workspaces so the same workspace-protocol rewrite path is validated without any registry interaction:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Before the first real release, validate the publish path locally. Yarn Berry's `yarn npm publish` does not accept a `--dry-run` flag, so the equivalent offline check is `yarn pack` on every public workspace — `yarn pack` runs the same workspace-protocol rewriter that `yarn npm publish` does, just stopping before the upload: | |
| Before the first real release, validate the publish path locally. While `yarn npm publish` supports `--dry-run`, this repo uses an offline `yarn pack` check across all public workspaces so the same workspace-protocol rewrite path is validated without any registry interaction: |
🤖 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 `@README.md` around lines 523 - 524, Update the README sentence that currently
claims "Yarn Berry's `yarn npm publish` does not accept a `--dry-run` flag" to
reflect the correct behavior: change it to state that `yarn npm publish`
supports a dry-run via `-n, --dry-run` (or `--dry-run`) and can be used to
preview what would be published without uploading; optionally keep the `yarn
pack` suggestion as an alternative offline check but remove the definitive "does
not accept" wording so the README references the official CLI option (`yarn npm
publish -n|--dry-run`) instead.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
The release workflow used `changeset publish`, which shells out to `npm publish` for non-pnpm repos. `npm publish` does not understand Yarn Berry's `workspace:^` protocol and serialised the literal string into 13 of 17 @openchoreo/* tarballs at v1.1.0. External installs fail with `Workspace not found` on first dependency resolution. Replace the publish step with Yarn Berry's native `yarn workspaces foreach --no-private --topological npm publish`, which rewrites `workspace:^` to concrete versions at pack time — independent of `.npmrc:ignore-scripts=true`, which would otherwise also have blocked the prepack hook from running under npm publish. `--tolerate-republish` preserves the idempotent-on-retry behaviour of the previous setup. `changeset version` (in prepare-next-version.yml) is unaffected — only the publish step moves. Verified by running `yarn workspaces foreach --no-private --topological pack` across all 22 public workspaces and grepping the resulting tarballs: every `@openchoreo/*` dep was rewritten to `^1.1.0`, zero `workspace:` leaks. Signed-off-by: Kavith Lokuhewage <kaviththiranga@gmail.com>
kaviththiranga
force-pushed
the
fix-npm-deps
branch
from
7c89539 to
18b70ee
Compare
The Deploy and RuntimeHealthCard hooks resolve the observability API
via `useApi(observabilityApiRef)`, which throws NotImplementedError
when no factory is registered for the ref. When a host installs only
the Core plugin set (without @openchoreo/backstage-plugin-openchoreo-
observability), opening any component's Deploy tab crashes with:
NotImplementedError: No implementation available for apiRef{
plugin.openchoreo-observability.service
}
Both hooks already document the intent that observability should be
optional: useLogsSummary's existing comment notes "without requiring
a package dependency" and useIncidentsSummary already absorbs per-
call failures as zero incidents. The bug is just the choice of
Backstage API — useApi() throws when the ref is unregistered, while
useApiHolder().get() returns undefined.
Switch both hooks to useApiHolder().get() and short-circuit the
fetch when the api is missing:
- useIncidentsSummary now returns an empty Map (PipelineCanvas
already optional-chains incidentsSummaries.get(name)?.activeCount).
- useLogsSummary now sets observabilityDisabled: true, which routes
to the existing empty-state branch in RuntimeHealthCard (the same
branch the backend uses when the cluster has observability off).
No UI changes — only the throw is gone.
Signed-off-by: Kavith Lokuhewage <kaviththiranga@gmail.com>
kaviththiranga
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (1)
package.json (1)
30-30: 💤 Low valueInconsistency with workflow dist-tag handling.
The workflow dynamically sets the tag to
latestornextbased on prerelease detection, but this script hardcodes--tag latest. If someone manually runsyarn release:publishfor a prerelease version, it would incorrectly publish to thelatestdist-tag.Consider documenting this limitation (script is for stable releases only) or parameterizing the tag. Since CI is the primary release path, this is a minor concern.
🤖 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 `@package.json` at line 30, The "release:publish" npm script hardcodes --tag latest which conflicts with the workflow that may set "next" for prereleases; update the script (the "release:publish" entry) to accept a configurable dist-tag (e.g., read from an environment variable like DIST_TAG or npm_config_tag and default to "latest") or document in README that this script is intended for stable releases only so manual runs won't accidentally publish prereleases to "latest".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@package.json`:
- Line 30: The "release:publish" npm script hardcodes --tag latest which
conflicts with the workflow that may set "next" for prereleases; update the
script (the "release:publish" entry) to accept a configurable dist-tag (e.g.,
read from an environment variable like DIST_TAG or npm_config_tag and default to
"latest") or document in README that this script is intended for stable releases
only so manual runs won't accidentally publish prereleases to "latest".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5ad53294-68e0-4ec3-9e4d-e6261ae91748
📒 Files selected for processing (5)
.github/workflows/release.ymlREADME.mdpackage.jsonplugins/openchoreo/src/components/Environments/hooks/useIncidentsSummary.tsplugins/openchoreo/src/components/RuntimeLogs/OverviewCard/useLogsSummary.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
2 participants
Purpose
Two related bugs were blocking external installs of
@openchoreo/*@1.1.0per the docs:Publish pipeline leaked
workspace:^into tarballs.changeset publishshells out tonpm publishfor non-pnpm repos, andnpm publishdoes not rewrite Yarn Berry'sworkspace:protocol. 13 of 17 published
@openchoreo/*@1.1.0packages contained literalworkspace:^strings in
dependencies. Externalyarn add @openchoreo/backstage-plugin@^1.1.0failed withWorkspace not found.Deploy tab hard-required the observability plugin. Following the install guide through
Section 4 (Core) only, opening any component's Deploy tab threw
NotImplementedError: No implementation available for apiRef{plugin.openchoreo-observability.service}. The hooksconsuming observability already documented the intent that it be optional ("without requiring a
package dependency") but used
useApi()(throws) instead ofuseApiHolder().get()(returnsundefined).
Goals
@openchoreo/*package installable viayarn addagainst the published registry,with concrete version ranges in
dependencies.install guide's promise that "Section 4 is the only mandatory install."
Approach
Commit 1 —
fix(release): publish with yarn npm publish to rewrite workspace depsReplace
changeset publishwithyarn workspaces foreach --all --no-private --topological --verbose npm publish --tolerate-republish --access public --tag <latest|next>. Yarn Berry'snative publish rewrites
workspace:^to concrete versions at pack time, independent of.npmrc:ignore-scripts=true.--tolerate-republishpreserves the previous idempotent-on-retrybehaviour.
changeset version(inprepare-next-version.yml) is untouched — only the publishstep moves.
Files changed:
package.json—release:publishscript.github/workflows/release.yml— publish step + commentREADME.md— release-procedure docs (incl.yarn pack-based local dry-run, sinceyarn npm publish --dry-runis not supported)Verified locally:
yarn workspaces foreach packacross all 22 public workspaces produced 22clean tarballs with zero
workspace:leaks. The@openchoreo/*@1.1.0packages weresubsequently deleted from GitHub Packages and republished manually with the fixed command —
yarn add @openchoreo/backstage-plugin@^1.1.0now succeeds end-to-end.Commit 2 —
fix(openchoreo): make observability dependency optional in Deploy tabSwitch both consumers of
observabilityApiRefin@openchoreo/backstage-pluginfromuseApi()to
useApiHolder().get()and short-circuit each fetch when the API is undefined.Files changed:
plugins/openchoreo/src/components/Environments/hooks/useIncidentsSummary.ts— returns anempty
Mapwhen observability isn't registered.PipelineCanvasalready optional-chainsincidentsSummaries.get(name)?.activeCount, so the Deploy tab now renders environment tilescleanly without incident chips.
plugins/openchoreo/src/components/RuntimeLogs/OverviewCard/useLogsSummary.ts— setsobservabilityDisabled: true.RuntimeHealthCardalready routes that state to a friendly emptycard (the same branch it uses when the backend reports observability is off cluster-side).
No UI changes — only the throw is gone.
User stories
yarn add @openchoreo/backstage-plugin@^1.1.0and have it resolve.Deploy tab loads without runtime errors.
Release note
Fixed:
@openchoreo/*packages no longer publish withworkspace:^strings independencies.Fixed: Core Deploy tab no longer requires
@openchoreo/backstage-plugin-openchoreo-observability— installs of just the Core plugin setwork end-to-end.
Documentation
Release-procedure changes are in this PR's
README.mddiff. The OpenChoreo install guide isbeing updated separately to flag Section 4's previously-implicit dependency on Section 5
(resolved by this PR) and other gaps surfaced during install testing.
Summary by CodeRabbit
Chores
Documentation
Bug Fixes