Skip to content

Fix: CS-Fixer workflow fails on pull_request events (fork PRs and same-repo alike) - #212

Merged
skerbis merged 1 commit into
FriendsOfREDAXO:mainfrom
skerbis:fix/cs-fixer-fork-pr-workflow
Sep 17, 2026
Merged

skerbis merged 1 commit into
FriendsOfREDAXO:mainfrom
skerbis:fix/cs-fixer-fork-pr-workflow

Conversation

@skerbis

@skerbis skerbis commented Sep 17, 2026

Copy link
Copy Markdown
Member

Summary

The PHP-CS-Fixer workflow currently fails on every pull request with:

fatal: invalid reference: <branch-name>
Error: Invalid status code: 128

(observed on #209, but this affects any PR, not just that one)

Root cause: actions/checkout leaves a detached HEAD for pull_request-triggered runs — there is no local branch named e.g. feature/x to push to, only the checked-out merge commit. stefanzweifel/git-auto-commit-action@v4 then fails trying to resolve/push that branch name.

Even once that ref issue is fixed, the workflow still can't succeed for PRs from forks: the GITHUB_TOKEN used for a pull_request run from a fork has no write access to the fork's branch, so an auto-commit step there would just fail differently (permission denied) instead of with a ref error. This is a hard GitHub Actions security boundary, not something fixable via workflow config alone.

Fix

Split the single job into two:

  • check-style (runs on pull_request, any source including forks): php-cs-fixer fix --dry-run --diff --verbose — fails the check if style violations exist, never attempts to write/push anything. Contributors fix locally via the already-defined composer cs-fix script.
  • fix-style (runs on push to main/master, always same-repo by definition): keeps the previous auto-fix-and-commit behavior, now pinned to the real branch via an explicit ref/branch instead of relying on default ref detection.

Also bumps actions/checkout v3 → v4 and git-auto-commit-action v4 → v7 (current Node-runtime versions), which incidentally addresses the "Node 20 is being deprecated" warning also visible in the failing run — that warning comes from the older action versions' own bundled runtime, not from anything configurable in the workflow YAML itself.

Test plan

  • YAML validated (python3 -c "import yaml; yaml.safe_load(...)")
  • --dry-run confirmed supported by the pinned friendsofphp/php-cs-fixer ^3.0
  • Will be exercised live once this PR itself runs CI (pull_request path) and once something is next pushed directly to main (push path) — happy to adjust if either behaves unexpectedly

🤖 Generated with Claude Code

actions/checkout leaves a detached HEAD for pull_request-triggered
runs (not a branch named e.g. "feature/x"), so
stefanzweifel/git-auto-commit-action failed with
"fatal: invalid reference" trying to push to that name. Worse, for
PRs from forks the workflow token has no write access to the fork's
branch at all, so even a fixed ref would still fail on permission.

Split into two jobs:
- pull_request (any branch, incl. forks): dry-run check only, fails
  the job on style violations without attempting to push anything.
  Contributors fix locally via `composer cs-fix`.
- push to main/master (always same-repo): keeps the previous
  auto-fix-and-commit behavior, now pinned to the real branch name
  via an explicit ref/branch instead of relying on default detection.

Also bumps actions/checkout v3 -> v4 and git-auto-commit-action
v4 -> v7 (current Node-runtime versions, addressing the Node 20
deprecation warning surfaced in the failing run).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@skerbis
skerbis merged commit 76db18e into FriendsOfREDAXO:main Sep 17, 2026
2 checks passed
skerbis added a commit to skerbis/quick_navigation that referenced this pull request Sep 17, 2026
Resolves conflicts in package.yml/CHANGELOG.md from FriendsOfREDAXO#209/FriendsOfREDAXO#210/FriendsOfREDAXO#212
having merged first: bumps the version to 9.2.1 (patch on top of
the now-released 9.2.0) and keeps both changelog entries.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.

1 participant