release: adopt npm trusted publishing#270
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughRestructured GitHub Actions release workflow from a single Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (merge -> main)
participant GH as GitHub Actions (prepare-release)
participant PR as Version PR (Changesets)
participant GH2 as GitHub Actions (publish)
participant NPM as npm Registry (OIDC)
Dev->>GH: push to main -> trigger prepare-release
GH->>PR: run changesets/action -> open/update version PR (has_changesets)
PR-->>Dev: Version PR created/updated
Dev->>PR: merge version PR when ready
PR->>GH2: trigger publish job (condition: has_changesets == 'false')
GH2->>NPM: request OIDC id-token
GH2->>NPM: npx changeset publish (OIDC-authenticated)
NPM-->>GH2: publish result
GH2-->>Dev: publish status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
|
Should we separate the prettier task again from linting/TSC-check? So linting can performed independently and parallel again. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
README.md (1)
57-58: Table row formatting is inconsistent with other entries.Line 58 uses the command-style format (
$ \Release workflow``) but "Release workflow" is not an executable command like the other rows. Consider restructuring this row to better fit the table's command/effect pattern or moving this information to a separate "Release" section.📝 Suggested fix
| $ `npx changeset add` | Add a changeset for package behavior changes in your PR | -| $ `Release workflow` | On merge to `main`, CI opens or updates a version PR and publishes from GitHub Actions via npm trusted publishing | +| *Release workflow* | On merge to `main`, CI opens or updates a version PR and publishes from GitHub Actions via npm trusted publishing |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 57 - 58, The table row for "`Release workflow`" is incorrectly formatted as a command; update the README table so the first-column cell is plain descriptive text (e.g., "Release workflow" without backticks) and the second column explains the behavior ("On merge to main, CI opens/updates a version PR and publishes via GitHub Actions/npm"), or remove this row and move that explanation into a separate "Release" section instead; locate the row containing "`npx changeset add`" and "`Release workflow`" and adjust the formatting accordingly so all table entries follow the same command/effect pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 32-36: Update the GitHub Actions workflow to annotate the pinned
SHA usages with their corresponding tagged versions for clarity: add inline
comments next to the uses entries for actions/checkout (currently pinned to a
SHA) noting "checkout v6.0.2", for actions/setup-node noting "setup-node
v6.3.0", and for changesets/action noting "changesets/action v1.5.3"; ensure you
place these comments directly beside the existing uses lines (e.g., the uses:
actions/checkout@<sha> line) so future maintainers can see the tagged release
that corresponds to each SHA.
---
Nitpick comments:
In `@README.md`:
- Around line 57-58: The table row for "`Release workflow`" is incorrectly
formatted as a command; update the README table so the first-column cell is
plain descriptive text (e.g., "Release workflow" without backticks) and the
second column explains the behavior ("On merge to main, CI opens/updates a
version PR and publishes via GitHub Actions/npm"), or remove this row and move
that explanation into a separate "Release" section instead; locate the row
containing "`npx changeset add`" and "`Release workflow`" and adjust the
formatting accordingly so all table entries follow the same command/effect
pattern.
🪄 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: 7a05d2b3-d9c6-49ac-8015-bd47ad4579d1
📒 Files selected for processing (5)
.github/workflows/release.ymlAGENTS.mdREADME.mdplans/build-and-release-certainty.mdplans/trusted-publishing.md
There was a problem hiding this comment.
Pull request overview
This PR migrates npm publishing for TKO to GitHub Actions OIDC “trusted publishing”, restructuring the release workflow to reduce permissions during the publish step and updating repo documentation/plans to match the new process.
Changes:
- Updates
.github/workflows/release.ymlto split “prepare release PR” vs “publish to npm” and remove token-based npm auth in favor of OIDC. - Switches the release workflow’s install step to
npm ciand pins key GitHub Actions by commit SHA (in the release workflow). - Updates maintainer/contributor docs and adds a plan documenting the trusted publishing migration.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Replaces the old lerna publish guidance with Changesets + release workflow guidance. |
| plans/trusted-publishing.md | Adds an implementation/verification plan for migrating to npm trusted publishing. |
| plans/build-and-release-certainty.md | Updates the release workflow description to reflect OIDC trusted publishing and clarifies “original” state/gaps. |
| AGENTS.md | Updates maintainer release instructions to reflect OIDC trusted publishing and discourages manual token-based publishing. |
| .github/workflows/release.yml | Implements the least-privilege split workflow and removes token-based npm publish configuration. |
Good question. I’d keep that separate from this PR so we can land the trusted publishing change cleanly first. If we want to split Prettier/lint/typecheck back out for CI parallelism, I’d prefer to do that in a follow-up workflow PR. |
Summary
Details
npm installwithnpm ciin the release flowmainTesting
Follow-up
knockout/tkousingrelease.yml@tko/*packagesSummary by CodeRabbit
Chores
Documentation