Skip to content

Change event of create_release workflow - #612

Draft
urfeex wants to merge 1 commit into
UniversalRobots:masterfrom
urfeex:fix_release_workflow
Draft

urfeex wants to merge 1 commit into
UniversalRobots:masterfrom
urfeex:fix_release_workflow

Conversation

@urfeex

@urfeex urfeex commented Nov 3, 2026

Copy link
Copy Markdown
Member

If we act on the pull_request event from a forked source (which is how we usually do release PRs), the workflow's permissions are downgraded to read-only. Changing that to the pull_request_target event should fix that.

If we act on the pull_request event from a forked source (which is how
we usually do release PRs), the workflow's permissions are downgraded to
read-only. Changing that to the pull_request_target event should fix
that.
@urfeex
urfeex requested a review from urrsk November 3, 2026 09:15
@urfeex urfeex added github_actions Pull requests that update GitHub Actions code CI labels Nov 3, 2026
Copilot AI balanced review requested due to automatic review settings November 3, 2026 09:15
@cursor

cursor Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Switching to pull_request_target elevates workflow privileges; mitigated by running only on merged, labeled PRs and checking out the merge commit SHA.

Overview
Release creation on merged release PRs now uses pull_request_target instead of pull_request, so the job keeps contents: write when the PR comes from a fork (where pull_request downgrades the token to read-only).

Checkout is pinned to github.event.pull_request.merge_commit_sha so the workflow runs against the merge commit on the base branch after merge, not an arbitrary fork ref.

Reviewed by Cursor Bugbot for commit a9b3876. Bugbot is set up for automated code reviews on this repo. Configure here.

@codecov

codecov Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.78%. Comparing base (7057991) to head (a9b3876).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #612      +/-   ##
==========================================
+ Coverage   83.39%   83.78%   +0.39%     
==========================================
  Files         127      127              
  Lines        8388     8388              
  Branches     3829     3829              
==========================================
+ Hits         6995     7028      +33     
+ Misses        981      953      -28     
+ Partials      412      407       -5     
Flag Coverage Δ
python_scripts 49.69% <ø> (ø)
start_ursim 91.08% <ø> (-1.12%) ⬇️
unit 61.62% <ø> (+0.01%) ⬆️
ur20-latest 81.06% <ø> (-0.02%) ⬇️
ur5-3.14.3 80.78% <ø> (-0.02%) ⬇️
ur5e-10.11.0 76.60% <ø> (ø)
ur5e-10.12.0 77.67% <ø> (+0.07%) ⬆️
ur5e-10.7.0 76.10% <ø> (-0.03%) ⬇️
ur5e-5.9.4 81.02% <ø> (+0.03%) ⬆️
ur7e-10.13.0 78.13% <ø> (-0.03%) ⬇️
ur7e-10.14.0 78.44% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are recorded for the release-workflow changes.

No open issues 🎉

What changed in this PR

Updates the release workflow so fork-based release PRs can use the write permissions needed to create releases while checking out the merged commit.

Changes:

  • Switches from pull_request to pull_request_target, retaining the closed-event and master branch filters.
  • Explicitly checks out the pull request’s merge commit.
  • Adds a comment explaining the fork-token permission limitation.
File Description
.github/​workflows/​create_release.yml Changes the event trigger and pins checkout to the merged PR commit.

Review effort: Balanced

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit a9b3876. Configure here.

steps:
- uses: actions/checkout@v7
with:
ref: ${{ github.event.pull_request.merge_commit_sha }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fork release checkout will fail

High Severity

actions/checkout@v7 refuses ref: ${{ github.event.pull_request.merge_commit_sha }} on pull_request_target for fork PRs unless allow-unsafe-pr-checkout is set. The checkout step fails for the fork-based release PRs this change is meant to support, so the draft release is never created.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a9b3876. Configure here.

on:
pull_request:
# pull_request tokens are read-only for fork-based release PRs.
pull_request_target:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR title has grammar issues

Low Severity

The PR title Change event of create_release workflow omits articles and can be read as the noun phrase change event rather than a verb phrase. This violates the PR title rule requiring a meaningful title without grammar or spelling errors. A clearer title is Switch create_release workflow to pull_request_target.

Fix in Cursor Fix in Web

Triggered by team rule: PR title

Reviewed by Cursor Bugbot for commit a9b3876. Configure here.

@urfeex
urfeex marked this pull request as draft November 4, 2026 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI github_actions Pull requests that update GitHub Actions code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants