feat(releases): require approval for package releases #2753
Conversation
|
|
Warning Rate limit exceeded@myftija has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis PR adds a new workflow Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
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 (3)
.github/workflows/changeset-pr.yml (2)
13-15: Consider cancellation impact on changeset commits.Concurrency is set to cancel in-progress runs. While this reduces noise, it can leave changesets uncommitted if a second push arrives during changesets/action execution. Per the PR objectives, publish runs explicitly disable cancellation to prevent partial failures. Consider whether the same reasoning applies here to avoid orphaned changeset PRs.
78-89: Cache configuration inconsistency with release-pr job.The release-pr job (line 38) specifies
cache: "pnpm"during node setup, but the update-lockfile job omits it (line 84-86). While update-lockfile may have lighter cache needs, consider whether consistency or explicit pnpm download is preferred for this secondary job..github/workflows/release.yml (1)
100-100: Inconsistent step names between release and prerelease jobs.The release job uses updated names ("Install dependencies", "Generate Prisma client"), but the prerelease job still uses old names ("Download deps", "Generate Prisma Client"). Consider updating prerelease step names (lines 137-141) to match the release job for consistency.
Apply this diff to align naming:
- - name: Download deps + - name: Install dependencies run: pnpm install --frozen-lockfile - - name: Generate Prisma Client + - name: Generate Prisma client run: pnpm run generateAlso applies to: 137-141
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/changeset-pr.yml(1 hunks).github/workflows/release.yml(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-27T16:27:48.109Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T16:27:48.109Z
Learning: pnpm version `10.23.0` and Node.js version `20.11.1` are required for development
Applied to files:
.github/workflows/release.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
.github/workflows/changeset-pr.yml (2)
43-64: Changeset PR creation and versioning logic looks sound.The release-pr job correctly uses
changesets/action@v1with dynamic title updates that extract the version from the release branch. The conditional onpublished != 'true'properly handles the case where the action skips PR creation (likely when changesets are empty or already published).
91-102: Lockfile commit and push logic is sound.The use of
set -e, silent diff check, and proper git config for the bot user ensures safe, idempotent updates. Explicit success messaging before pushing is clear and maintainable..github/workflows/release.yml (5)
1-22: Workflow restructuring aligns with PR objectives.The trigger change from push to pull_request [closed] correctly gates releases to merged PRs. The concurrency group simplification and
cancel-in-progress: falseprevent partial publish failures as intended. Workflow name clarifies this is for npm packages, not changesets themselves.
25-41: Environment-based approval gating properly configured.The
environment: npm-publish(line 28) enables GitHub's required approval step, hardening the npm OIDC setup as described in PR objectives. The tightened gating conditions (lines 33-37) require the PR to be merged and the branch to start withchangeset-release/, ensuring only changesets-generated PRs trigger releases. The newpublished_package_versionoutput (line 41) is useful for downstream jobs referencing package versions.
30-32: Permission hardening reduces attack surface.Changing
contents: writetocontents: readwhile keepingpackages: writeandid-token: writefollows the principle of least privilege. The workflow no longer needs to commit directly—lockfile updates are now in the separate changeset-pr workflow.
59-96: Release job refactoring is sound.Removal of update-lockfile logic aligns with the new changeset-pr workflow design. Step name updates ("Download deps" → "Install dependencies", "Generate Prisma Client" → "Generate Prisma client") improve consistency. The Docker tag creation step properly uses the new
published_package_versionoutput.
98-106: Prerelease job correctly gated to workflow_dispatch with approval environment.The prerelease job is appropriately restricted to manual workflow_dispatch triggers and also gets the
environment: npm-publish(line 102) for approval gating. The comment (line 98) correctly explains why both jobs remain in the same file—npm OIDC verification requires it. This maintains security while enabling both automated (merged PR) and manual (prerelease) publishing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
134-134: Standardize step names across release and prerelease jobs.Step naming is inconsistent between the two jobs:
- Release job:
"Install dependencies"(line 59) and"Generate Prisma client"(line 62)- Prerelease job:
"Download deps"(line 134) and"Generate Prisma Client"(line 137)Apply this diff to align naming:
# npm v11.5.1 or newer is required for OIDC support # https://github.blog/changelog/2025-07-31-npm-trusted-publishing-with-oidc-is-generally-available/#whats-new - - name: Download deps + - name: Install dependencies run: pnpm install --frozen-lockfile - - name: Generate Prisma Client + - name: Generate Prisma client run: pnpm run generateAlso applies to: 137-137
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-09-23T12:51:42.019Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 1306
File: .github/actions/get-image-tag/action.yml:51-62
Timestamp: 2024-09-23T12:51:42.019Z
Learning: In the 'get-image-tag' GitHub Action, prefer dependent workflows to fail immediately when the tag is invalid, without outputting the validity status as an output.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2025-11-27T16:27:48.109Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T16:27:48.109Z
Learning: pnpm version `10.23.0` and Node.js version `20.11.1` are required for development
Applied to files:
.github/workflows/release.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
.github/workflows/release.yml (3)
3-41: Verify the workflow trigger, gating, and environment approval setup.The trigger has been refactored from
pushtopull_request(closed), and the release job now gates on:
- Merged PRs with head ref starting
changeset-release/- Explicitly excludes
workflow_dispatchevents (which trigger prerelease instead)- Requires
environment: npm-publishfor approval enforcementConfirm that:
- The changeset PR workflow (referenced in PR description) is creating PRs with the
changeset-release/prefix so they match the gating condition.- The GitHub
npm-publishenvironment is configured with required approval(s) in the repository settings.- The
id-token: writepermission in the release job is correctly provisioned for npm OIDC under thenpm-publishenvironment.
20-22: Workflow separation and approval enforcement look solid.The refactoring correctly:
- Separates release (on merged
changeset-release/*PRs) from prerelease (onworkflow_dispatch)- Sets
cancel-in-progress: falseto prevent partial publish failures- Adds
environment: npm-publishto both jobs to enable approval gating- Tightens permissions (
id-token: writefor npm OIDC,packages: writeonly in release job)- Validates prerelease ref is on main (lines 111–116)
This aligns with the PR objectives and hardens the npm OIDC setup via environment-enforced approval.
Also applies to: 28-28, 99-99, 33-37, 103-103
132-132: npm version 11.6.4 meets OIDC requirements.Line 132 pins npm to 11.6.4, which exceeds the minimum version requirement of 11.5.1 for npm's Trusted Publishing (OIDC) support. No changes needed.
Background
Currently the changeset PR creation and the publishing is handled by the same workflow. This is not ideal:
Changes in this PR
These changes also enable hardening the npm OIDC setup by tying it to a GH environment that requires approval.