Skip to content

ci(plugin-build): grant pull-requests: write so auto-PR can be created - #154

Closed
PrestaEdit wants to merge 1 commit into
NativePHP:mainfrom
PrestaEdit:fix/workflow-pr-permissions
Closed

PrestaEdit wants to merge 1 commit into
NativePHP:mainfrom
PrestaEdit:fix/workflow-pr-permissions

Conversation

@PrestaEdit

Copy link
Copy Markdown
Contributor

Context

The plugin-build workflow compiles the TypeScript sources in resources/electron/electron-plugin/ and is supposed to open an auto-PR (Auto: Build plugin assets) with the fresh dist/ output whenever main moves. That auto-PR never lands because the workflow's GITHUB_TOKEN is missing pull-requests: write.

See run 33985077914:

##[error]Resource not accessible by integration - https://docs.github.com/rest/pulls/pulls#create-a-pull-request

The build step itself succeeds and the branch build-plugin-update is force-pushed, but peter-evans/create-pull-request@v8 cannot open the PR, so the compiled assets never reach main. That is the underlying reason files like pdfPageSize.js were missing from the repo and had to be committed manually in #153.

Fix

Add pull-requests: write to the job's permissions: block so the token can create the auto-PR.

Note for maintainers

For this to fully work, the repository setting Settings → Actions → General → Workflow permissions → "Allow GitHub Actions to create and approve pull requests" must also be enabled — otherwise even a well-scoped token is refused.

Follow-up on #153 (per @SRWieZ's comment).

🤖 Generated with Claude Code

@gwleuverink gwleuverink left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved! With one tiny suggestion that is not blocking

The reason the dist file was ignored in #153 is because we want to exclude it from the diff when reviewing PR's. CI should catch it, and this will fix the failing PR workflow

It needs one more thing though. A new compiled file still won't make it into the auto PR, only modified ones. Could you add this before the create-pull-request step?

- run: git add -f resources/electron/electron-plugin/dist

The action picks up staged changes, so adding this line should fire proof the workflow so we wont run into this issue again

@gwleuverink

Copy link
Copy Markdown
Collaborator

Sorry to flip on this right after approving. Simon and I talked it over and we'd rather not run the plugin build from fork branches at all, it's too easy to miss something malicious in a PR that touches package scripts.

We're dropping the pull_request trigger and only build on main after a merge

Thanks for chasing this down, you got 2.3.1 out the door 👍🏻 I tagged it this morning so things should work again after a composer update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants