Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new GitHub Actions workflow intended to run prek (with auto-fix behavior) on merge queue events, pushes, tags, and pull requests as part of a “pre-lint auto-fix” pipeline.
Changes:
- Introduces a new
.github/workflows/pre.ymlworkflow triggered onmerge_group,push, andpull_request. - Installs
prekviaj178/prek-actionand runsprek run --all-files, attempting to auto-commit and push fixes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| fetch-depth: 0 # we need tags for dynamic versioning | ||
| show-progress: false |
There was a problem hiding this comment.
actions/checkout defaults to checking out a detached merge ref on pull_request events (e.g. refs/pull/*/merge), so any commits created later in this job won’t be pushable back to the PR branch. If the goal is to push auto-fixes to the PR branch, explicitly checkout the PR head ref/repo and guard against forks (where push won’t be allowed). Also consider not running the auto-push behavior on push to main/devel/* to avoid the workflow creating extra commits on protected branches.
| fetch-depth: 0 # we need tags for dynamic versioning | ||
| show-progress: false | ||
|
|
||
| - uses: j178/prek-action@v1 |
There was a problem hiding this comment.
j178/prek-action@v1 is not pinned to a commit SHA. For supply-chain hardening, pin the action to an immutable SHA (as done elsewhere in workflows) and keep the version comment for readability.
| - uses: j178/prek-action@v1 | |
| - uses: j178/prek-action@<commit-sha> # v1 |
.github/workflows/pre.yml
Outdated
| if [ -n "$(git status --porcelain)" ]; then | ||
| git config --global user.email ansible-devtools@redhat.com | ||
| git config --global user.name "Ansible DevTools" | ||
| git add |
There was a problem hiding this comment.
git add with no pathspec will fail (Nothing specified, nothing added). Use an explicit pathspec (e.g., -A) so the subsequent commit actually includes the auto-fix changes.
| git add | |
| git add -A |
.github/workflows/pre.yml
Outdated
| prek run --all-files || EXIT_CODE=$? | ||
| if [ -n "$(git status --porcelain)" ]; then | ||
| git config --global user.email ansible-devtools@redhat.com | ||
| git config --global user.name "Ansible DevTools" | ||
| git add | ||
| git commit -m 'Prek auto-fixes' | ||
| prek run --all-files || EXIT_CODE=$? | ||
| if [ $EXIT_CODE != '0' ]; then | ||
| git push origin | ||
| fi | ||
| fi | ||
| exit EXIT_CODE=$? |
There was a problem hiding this comment.
The push condition is inverted: git push currently runs only when prek still fails (EXIT_CODE != 0). If the intent is to push auto-fixes when they made the repo clean and prek passes on the second run, push only when the second prek succeeds (and fail the job otherwise). Also, make sure git push targets the correct branch/ref rather than relying on defaults.
| prek run --all-files || EXIT_CODE=$? | |
| if [ -n "$(git status --porcelain)" ]; then | |
| git config --global user.email ansible-devtools@redhat.com | |
| git config --global user.name "Ansible DevTools" | |
| git add | |
| git commit -m 'Prek auto-fixes' | |
| prek run --all-files || EXIT_CODE=$? | |
| if [ $EXIT_CODE != '0' ]; then | |
| git push origin | |
| fi | |
| fi | |
| exit EXIT_CODE=$? | |
| EXIT_CODE=0 | |
| prek run --all-files || EXIT_CODE=$? | |
| if [ -n "$(git status --porcelain)" ]; then | |
| git config --global user.email ansible-devtools@redhat.com | |
| git config --global user.name "Ansible DevTools" | |
| git add | |
| git commit -m 'Prek auto-fixes' | |
| EXIT_CODE=0 | |
| prek run --all-files || EXIT_CODE=$? | |
| if [ "$EXIT_CODE" = "0" ]; then | |
| git push origin HEAD | |
| else | |
| exit "$EXIT_CODE" | |
| fi | |
| fi | |
| exit "$EXIT_CODE" |
.github/workflows/pre.yml
Outdated
| prek run --all-files || EXIT_CODE=$? | ||
| if [ -n "$(git status --porcelain)" ]; then |
There was a problem hiding this comment.
EXIT_CODE is only set on failure (|| EXIT_CODE=$?) and is otherwise uninitialized, which makes later numeric comparisons/error propagation unreliable. Initialize EXIT_CODE=0 before running prek, and update it when a run fails so the script has a deterministic exit status.
| --- | ||
| on: | ||
| merge_group: |
There was a problem hiding this comment.
All other workflows in this repo set a name: at the top (e.g. ci, push, ack, post). Adding a name: here improves readability in the Actions UI and keeps workflow files consistent.
| id-token: write | ||
| pull-requests: write | ||
| checks: write |
There was a problem hiding this comment.
This workflow requests broad write permissions (id-token, pull-requests, checks) but the job only checks out code and runs shell commands. Consider reducing permissions to the minimum required (likely contents: write only when pushing auto-fix commits; otherwise contents: read).
| id-token: write | |
| pull-requests: write | |
| checks: write |
.github/workflows/pre.yml
Outdated
| git push origin | ||
| fi | ||
| fi | ||
| exit EXIT_CODE=$? |
There was a problem hiding this comment.
exit EXIT_CODE=$? is not valid bash syntax and won’t exit with the intended status. Exit with a numeric code (e.g. exit "$EXIT_CODE") so the workflow result correctly reflects the final prek run.
Related: AAP-64628