fix: check actual PR commits instead of merge commit HEAD#186
fix: check actual PR commits instead of merge commit HEAD#186shenxianpeng wants to merge 2 commits intomainfrom
Conversation
When actions/checkout runs on a pull_request event, it checks out an
auto-generated merge commit at refs/pull/{N}/merge as HEAD. The commit
message of this merge commit is 'Merge {sha} into {base}', which:
- Matches the (Merge).* alternative in the conventional commits regex
- Is allowed by allow_merge_commits = true
This caused the action to always pass on PRs regardless of the actual
commit messages, since only the merge commit HEAD was ever checked.
Fix by adding get_pr_commit_messages() which, when GITHUB_BASE_REF is
set (indicating a PR context), retrieves the real PR commits via:
git log --no-merges origin/{base_ref}..HEAD --format=%B%x00 --reverse
Each commit message is then piped individually to commit-check --message
via stdin so the actual PR commits are validated.
In non-PR contexts (push, manual dispatch, etc.) the existing behaviour
is preserved: commit-check determines what to check from git itself.
Also fixes stderr being silently discarded (subprocess.PIPE) by using
subprocess.STDOUT so errors are captured in result.txt and surfaced in
the job summary and PR comments.
Fixes #184
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip Migrating from UI to YAML configuration.Use the |
Commit-Check ✔️ |
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where, on GitHub Actions pull_request events, the action validated only the auto-generated PR merge commit at HEAD instead of the actual PR commits—allowing non-conforming commit messages to pass.
Changes:
- Adds PR-aware commit message retrieval using
git log origin/{base_ref}..HEADand validates each commit message individually viacommit-check --messageover stdin. - Preserves existing non-PR behavior by letting
commit-checkinfer what to validate from the repository state. - Redirects
commit-checkstderr to stdout so failures are captured inresult.txtand surfaced via summaries/comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| "git", | ||
| "log", | ||
| "--no-merges", | ||
| f"origin/{base_ref}..HEAD", | ||
| "--format=%B%x00", | ||
| "--reverse", |
| if MESSAGE == "true": | ||
| commit_messages = get_pr_commit_messages() | ||
| if commit_messages: | ||
| # PR context: check each commit message individually to avoid | ||
| # only checking the auto-generated merge commit at HEAD. | ||
| for msg in commit_messages: | ||
| command = ["commit-check", "--message"] | ||
| print(" ".join(command)) | ||
| result = subprocess.run( | ||
| command, | ||
| input=msg, | ||
| text=True, | ||
| stdout=result_file, | ||
| stderr=subprocess.STDOUT, | ||
| check=False, | ||
| ) | ||
| ret_code += result.returncode | ||
| else: | ||
| # Non-PR context: let commit-check determine what to check from git. | ||
| command = ["commit-check", "--message"] | ||
| print(" ".join(command)) | ||
| ret_code += subprocess.run( | ||
| command, | ||
| stdout=result_file, | ||
| stderr=subprocess.STDOUT, | ||
| text=True, | ||
| check=False, | ||
| ).returncode |
| try: | ||
| result = subprocess.run( | ||
| [ | ||
| MESSAGE, | ||
| BRANCH, | ||
| AUTHOR_NAME, | ||
| AUTHOR_EMAIL, | ||
| "git", | ||
| "log", | ||
| "--no-merges", | ||
| f"origin/{base_ref}..HEAD", | ||
| "--format=%B%x00", | ||
| "--reverse", | ||
| ], | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| text=True, | ||
| check=False, | ||
| ) | ||
| if value == "true" | ||
| if result.returncode != 0 or not result.stdout.strip(): | ||
| return None | ||
| messages = [m.strip() for m in result.stdout.split("\x00") if m.strip()] | ||
| return messages if messages else None | ||
| except Exception: | ||
| return None |
| ret_code += result.returncode | ||
| else: | ||
| # Non-PR context: let commit-check determine what to check from git. | ||
| command = ["commit-check", "--message"] | ||
| print(" ".join(command)) | ||
| ret_code += subprocess.run( | ||
| command, | ||
| stdout=result_file, | ||
| stderr=subprocess.STDOUT, | ||
| text=True, | ||
| check=False, | ||
| ).returncode | ||
|
|
||
| if other_args: | ||
| command = ["commit-check"] + other_args | ||
| print(" ".join(command)) | ||
| ret_code += subprocess.run( | ||
| command, | ||
| stdout=result_file, | ||
| stderr=subprocess.STDOUT, | ||
| text=True, | ||
| check=False, | ||
| ).returncode | ||
|
|
When actions/checkout runs on a pull_request event, it checks out an auto-generated merge commit at refs/pull/{N}/merge as HEAD. The commit message of this merge commit is 'Merge {sha} into {base}', which:
This caused the action to always pass on PRs regardless of the actual commit messages, since only the merge commit HEAD was ever checked.
Fix by adding get_pr_commit_messages() which, when GITHUB_BASE_REF is set (indicating a PR context), retrieves the real PR commits via:
git log --no-merges origin/{base_ref}..HEAD --format=%B%x00 --reverse
Each commit message is then piped individually to commit-check --message via stdin so the actual PR commits are validated.
In non-PR contexts (push, manual dispatch, etc.) the existing behaviour is preserved: commit-check determines what to check from git itself.
Also fixes stderr being silently discarded (subprocess.PIPE) by using subprocess.STDOUT so errors are captured in result.txt and surfaced in the job summary and PR comments.
Fixes #184