Skip to content

Commit 044459e

Browse files
ruby-automationruby-automation
authored
fix(ci): preserve GitHub Update branch merge exception (#220)
EXT-29 (PR #218) replaced the metadata-aware commit-range validator with a subject-only traversal, losing the exception for GitHub's trusted "Update branch" auto-merge commits. Restore it in validate-commit-range using a three-part predicate: a commit is skipped only when it has exactly two parents, the committer is GitHub <noreply@github.com>, and the subject matches `Merge branch '<base>' into <head>`. The git log format is expanded from `%s` to `%P%x01%cn%x01%ce%x01%s` (SOH-delimited) so parent hashes, committer name, and committer email are available in the loop alongside the subject. Add five new ExUnit tests covering the skip itself and each predicate variation (wrong name, wrong email, single parent, non-matching subject). Add documents/github-update-branch-validation-decision.adoc recording the rationale and alternatives considered. Refs: EXT-35 Co-authored-by: bougyman's bot <ruby-automation@users.noreply.github.com>
1 parent db08f0e commit 044459e

3 files changed

Lines changed: 176 additions & 2 deletions

File tree

‎documents/github-update-branch-validation-decision.adoc‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
= GitHub "Update branch" Merge Commit Validation Exception
2+
:toc:
3+
:toc-placement: preamble
4+
5+
== Context
6+
7+
When a contributor clicks GitHub's **Update branch** button on a pull request,
8+
GitHub creates an automatic merge commit that incorporates the latest commits
9+
from the base branch into the PR branch. This merge commit has a
10+
non-Conventional-Commit subject:
11+
12+
----
13+
Merge branch 'main' into <feature-branch>
14+
----
15+
16+
The `git-hooks/validate-commit-range` script validates every commit subject in
17+
the PR range against the Conventional Commits specification. Without a
18+
targeted exception, GitHub's auto-generated merge commit causes CI to fail and
19+
blocks PRs that would otherwise be mergeable.
20+
21+
This issue was first introduced by EXT-29 (PR #218), which replaced a
22+
metadata-aware validator with a subject-only traversal, removing the exception
23+
that had previously been in place.
24+
25+
== Decision
26+
27+
Restore a narrowly scoped exception inside `validate-commit-range`. A commit
28+
is skipped **only when all three conditions hold simultaneously**:
29+
30+
1. *Exactly two parents* — the commit is a merge commit (not a regular commit
31+
disguised with a merge-style subject).
32+
33+
2. *Committer is `GitHub <noreply@github.com>`* — the commit was produced by
34+
the trusted GitHub bot, not by a contributor.
35+
36+
3. *Subject matches `^Merge branch '[^']+' into .+`* — the subject is
37+
GitHub's canonical "Update branch" format, with the base branch name
38+
enclosed in single quotes.
39+
40+
If any predicate does not match, the commit subject is validated normally.
41+
42+
== Implementation
43+
44+
`git log` is invoked with `--format='%P%x01%cn%x01%ce%x01%s'` so that parent
45+
hashes, committer name, committer email, and the subject are all available
46+
within the validation loop. Records are NUL-delimited (`-z`); fields within
47+
each record are separated by SOH (ASCII 0x01), a character that cannot appear
48+
in committer metadata or commit subjects in practice.
49+
50+
== Rationale for each predicate
51+
52+
*Parent count* prevents a regular commit from bypassing validation simply by
53+
having a subject that starts with `Merge branch '`. The contributor subject
54+
guard in `validate-conventional-subject` already catches this pattern and
55+
rejects it with a helpful remediation hint; the exception must not interfere
56+
with that guidance.
57+
58+
*Committer identity* binds the exception to the specific GitHub bot account
59+
that creates "Update branch" commits. Contributor-authored merge commits
60+
(e.g., `git merge --no-ff`) use the contributor's own identity and are
61+
therefore still validated.
62+
63+
*Subject pattern* ensures only the exact auto-generated format is exempted.
64+
A two-parent commit from the GitHub committer with a different subject (e.g.,
65+
a revert, or a merge of a submodule) still goes through validation.
66+
67+
== Alternatives considered
68+
69+
*Exempt all merge commits* — rejected. Contributors can create merge commits
70+
with arbitrary subjects; blanket exemption would create a loophole.
71+
72+
*Reword the commit before pushing* — not feasible. The commit is created by
73+
GitHub's server-side automation after the PR branch is pushed; there is no
74+
hook opportunity to intercept or rewrite it before it lands.
75+
76+
*Allow `Merge branch '...' into ...'` as a valid Conventional Commits type* —
77+
rejected. This would widen the allowed format for all contributors rather than
78+
targeting the specific trusted-bot case.

‎git-hooks/validate-commit-range‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,41 @@ base_sha=$(git_or_die merge-base HEAD "$base_ref")
8080

8181
validation_status=0
8282

83-
while IFS= read -r -d '' subject
83+
# A commit is exempt from subject validation only when ALL three conditions hold:
84+
# 1. It has exactly two parents (is a merge commit).
85+
# 2. Its committer is GitHub <noreply@github.com> (the trusted bot identity).
86+
# 3. Its subject matches the canonical "Update branch" pattern.
87+
# Ordinary contributor-created merge commits (different committer, or a
88+
# subject that doesn't match the pattern) still go through subject validation.
89+
github_merge_pattern="^Merge branch '[^']+' into .+"
90+
91+
while IFS= read -r -d '' entry
8492
do
93+
IFS=$'\x01' read -r parents committer_name committer_email subject <<< "$entry"
94+
95+
if [ -n "$parents" ]
96+
then
97+
IFS=' ' read -ra parents_array <<< "$parents"
98+
parent_count=${#parents_array[@]}
99+
else
100+
parent_count=0
101+
fi
102+
103+
if [ "$parent_count" -eq 2 ] \
104+
&& [ "$committer_name" = "GitHub" ] \
105+
&& [ "$committer_email" = "noreply@github.com" ] \
106+
&& [[ "$subject" =~ $github_merge_pattern ]]
107+
then
108+
continue
109+
fi
110+
85111
"$validator" --subject "$subject"
86112
status=$?
87113

88114
if [ "$status" -ne 0 ]
89115
then
90116
validation_status=$status
91117
fi
92-
done < <(git log -z --format='%s' "$base_sha..HEAD")
118+
done < <(git log -z --format='%P%x01%cn%x01%ce%x01%s' "$base_sha..HEAD")
93119

94120
exit "$validation_status"

‎test/git_hooks_test.exs‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,76 @@ defmodule GitHooksTest do
133133
refute output =~ "feat: valid commit"
134134
end
135135

136+
test "the range guard skips a GitHub Update-branch merge commit matching all three predicates" do
137+
{worktree, hooks_dir} = setup_hooks_worktree!()
138+
add_github_merge!(worktree)
139+
140+
assert {"", 0} = run(Path.join(hooks_dir, "validate-commit-range"), [], cd: worktree)
141+
end
142+
143+
test "the range guard validates when the committer name is not GitHub" do
144+
{worktree, hooks_dir} = setup_hooks_worktree!()
145+
add_github_merge!(worktree, committer_name: "Not GitHub")
146+
147+
assert {output, 1} = run(Path.join(hooks_dir, "validate-commit-range"), [], cd: worktree)
148+
assert output =~ "Merge branch 'main' into feature"
149+
end
150+
151+
test "the range guard validates when the committer email is not noreply@github.com" do
152+
{worktree, hooks_dir} = setup_hooks_worktree!()
153+
add_github_merge!(worktree, committer_email: "not@github.com")
154+
155+
assert {output, 1} = run(Path.join(hooks_dir, "validate-commit-range"), [], cd: worktree)
156+
assert output =~ "Merge branch 'main' into feature"
157+
end
158+
159+
test "the range guard validates a single-parent commit whose subject matches the GitHub pattern" do
160+
{worktree, hooks_dir} = setup_hooks_worktree!()
161+
162+
git!(worktree, ["commit", "--allow-empty", "-m", "Merge branch 'main' into feature"])
163+
164+
assert {output, 1} = run(Path.join(hooks_dir, "validate-commit-range"), [], cd: worktree)
165+
assert output =~ "Merge branch 'main' into feature"
166+
end
167+
168+
test "the range guard validates a two-parent GitHub-committer merge with a non-matching subject" do
169+
{worktree, hooks_dir} = setup_hooks_worktree!()
170+
add_github_merge!(worktree, subject: "Merge feature into main")
171+
172+
assert {output, 1} = run(Path.join(hooks_dir, "validate-commit-range"), [], cd: worktree)
173+
assert output =~ "Merge feature into main"
174+
end
175+
176+
# Creates a no-fast-forward merge commit on `main` from a throwaway `feature`
177+
# branch. Defaults simulate GitHub's "Update branch" committer identity and
178+
# subject so the predicate in validate-commit-range matches.
179+
defp add_github_merge!(worktree, opts \\ []) do
180+
committer_name = Keyword.get(opts, :committer_name, "GitHub")
181+
committer_email = Keyword.get(opts, :committer_email, "noreply@github.com")
182+
subject = Keyword.get(opts, :subject, "Merge branch 'main' into feature")
183+
184+
git!(worktree, ["checkout", "-b", "feature"])
185+
File.write!(Path.join(worktree, "feature_file"), "feature content")
186+
git!(worktree, ["add", "feature_file"])
187+
git!(worktree, ["commit", "-m", "feat: add feature"])
188+
git!(worktree, ["checkout", "main"])
189+
190+
git_with_env!(worktree, ["merge", "--no-ff", "-m", subject, "feature"], [
191+
{"GIT_COMMITTER_NAME", committer_name},
192+
{"GIT_COMMITTER_EMAIL", committer_email}
193+
])
194+
end
195+
196+
defp git_with_env!(directory, args, extra_env) do
197+
case System.cmd("git", args, cd: directory, stderr_to_stdout: true, env: extra_env) do
198+
{_output, 0} ->
199+
:ok
200+
201+
{output, status} ->
202+
flunk("git #{Enum.join(args, " ")} failed (#{status}):\n#{output}")
203+
end
204+
end
205+
136206
# Creates a git repo in a temp dir with origin set up and the guard scripts
137207
# copied in. Uses a cryptographic nonce so collisions cannot occur across
138208
# BEAM VM restarts (unlike System.unique_integer/1 which resets each run).

0 commit comments

Comments
 (0)