ci(release): add canary npm publishing - #557
Conversation
Add an internal canary channel for branch-specific QA. Keep the stable Release Please flow unchanged. Derive immutable versions from npm latest and the workflow run number. Publish under the canary dist-tag and document retry and QA usage.
|
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9e35cf0. Configure here.
There was a problem hiding this comment.
Stale comment
Risk: high. Left a non-blocking comment; not approving because Cursor Bugbot completed as skipped with unresolved findings and Security Agent reported an unresolved high-severity issue on the canary publish path. Assigned reviewers for human review of this npm publishing change.
Sent by Cursor Approval Agent: Pull Request Router and Approver
There was a problem hiding this comment.
Stale comment
Risk: high. Left a non-blocking comment; not approving because this npm canary publish path exceeds the low-risk approval threshold and Security Agent has an unresolved medium-severity finding on trusted OIDC publishing without environment approval. Human reviewers are already assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
There was a problem hiding this comment.
Risk: high. Left a non-blocking comment; not approving because this npm canary publish path exceeds the low-risk approval threshold. Cursor Security Agent passed on this sync, Cursor Bugbot was not present, and human reviewers are already assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
jdomingos
left a comment
There was a problem hiding this comment.
Thorough review of the canary publishing path. Overall the design is sound: version derivation is guarded and unit-tested, the semver ordering keeps canaries below the next stable (0.0.283-canary.N < 0.0.283), nothing is committed or tagged, and I verified the fragile-looking guards work (npm view <pkg>@<missing-version> exits non-zero on npm 11.x; biome excludes **/package.json so the runner-side version rewrite won't fail the in-build lint; re-runs keep GITHUB_RUN_NUMBER so published versions are immutable).
Findings (inline below):
- P0 — the publish-authority model depends on
productionenvironment approval that is not actually configured (no required reviewers, no deployment branch policy). - P1 — the new version-derivation tests never run in PR CI; they only execute inside the canary job itself.
- P1 — re-running an older, never-published run moves the
canarydist-tag backwards; the retry docs overstate rerun safety.
Recommendation: hold merge until the production environment has required reviewers configured (a repo-settings change, not a code change). The two P1s are should-fix.
| permissions: | ||
| contents: read | ||
| id-token: write | ||
| environment: production |
There was a problem hiding this comment.
P0 — publish authority relies on environment approval that is not configured.
docs/releases.md says the production environment "must require approval from a release maintainer", but the environment currently has no protection rules and no deployment branch policy (gh api repos/stakekit/widget/environments/production returns {"deployment_branch_policy":null,"protection_rules":[]}).
Since workflow_dispatch executes the workflow file from the selected branch, any writer can dispatch a modified release.yml (e.g., npm publish --tag latest with an arbitrary version, or exfiltrate the OIDC token) from their own branch — with zero review. This job's id-token: write + environment: production then mints a valid npm trusted-publishing token.
Please configure required reviewers on the production environment (and confirm the npm trusted publisher is bound to this environment) before merging. Approvers must review the dispatched branch's workflow/script diff, since the environment gate is the only trust boundary. Optionally split canary into its own environment so stable can additionally get a main-only deployment branch policy.
| export default defineConfig({ | ||
| test: { | ||
| environment: "node", | ||
| include: ["scripts/**/*.test.ts"], |
There was a problem hiding this comment.
P1 — these tests never run in PR CI.
prepare-canary-release.test.ts only executes inside the canary workflow job. PR CI runs pnpm test → the widget test script → vite.config.dev.ts, whose include is tests/**/*.test.{ts,tsx} — so scripts/**/*.test.ts is never picked up. A regression in the release script keeps CI green and is only discovered when a canary publish fails.
Relatedly, turbo.json now adds scripts/** to the test task inputs, so the cache key covers files the task never actually tests.
Suggest wiring this config into the widget test script (or a test:scripts script run by the turbo test task) so PR CI exercises it.
|
|
||
| - name: Publish canary to npm | ||
| if: steps.publish.outputs.should_publish == 'true' | ||
| run: npm publish "./$PACKAGE_DIR" --access public --tag canary |
There was a problem hiding this comment.
P1 — re-running an older failed run can move the canary dist-tag backwards.
GITHUB_RUN_NUMBER is stable across re-runs, and the existence check only looks at this run's own version. Scenario: run 418 fails before publishing (e.g., flaky test) → run 420 later publishes 0.0.283-canary.420 → someone re-runs 418 per the retry docs → existence check passes (418 was never on npm) and --tag canary retags to 0.0.283-canary.418. QA installing @stakekit/widget@canary silently downgrades to older code.
The docs/releases.md claim that re-running the same run is safe only holds for post-publication reruns.
Suggest moving the canary dist-tag only when the new version is semver-greater than the current tag (publish under a placeholder tag, then a conditional npm dist-tag add), or documenting that runs superseded by a later canary must not be re-run.



Add an internal canary channel for branch-specific QA.
Keep the stable Release Please flow unchanged.
Derive immutable versions from npm latest and the workflow run number.
Publish under the canary dist-tag and document retry and QA usage.
Note
Medium Risk
Introduces a second npm publish path (writers can publish without production environment approval); mitigated by canary-only dist-tag, version guards, and tests, but still affects package distribution and trusted-publisher configuration.
Overview
Adds an internal canary npm path for
@stakekit/widgetwhile keeping the existing Release Please stable flow onmainpushes only.The Release workflow now supports workflow_dispatch. The stable job is gated to
pushevents; a new Publish canary job runs on manual dispatch from a chosen branch. It reads npmlatest, runsprepare-canary-release.tsto bump to{next patch}-canary.{GITHUB_RUN_NUMBER}(branchpackage.jsonmust matchlatest), builds and tests, skips if that version already exists on npm, then publishes with thecanarydist-tag—without git tags, GitHub releases, or committing the version.Supporting changes: docs/releases.md (stable vs canary, trusted publishing, QA/retry guidance), Vitest coverage for version derivation, a scripts Vitest config, rev-dep dev entry points, and turbo task inputs including
scripts/**.Reviewed by Cursor Bugbot for commit 9e35cf0. Configure here.