diff --git a/documents/github-update-branch-validation-decision.adoc b/documents/github-update-branch-validation-decision.adoc new file mode 100644 index 0000000..207d5da --- /dev/null +++ b/documents/github-update-branch-validation-decision.adoc @@ -0,0 +1,78 @@ += GitHub "Update branch" Merge Commit Validation Exception +:toc: +:toc-placement: preamble + +== Context + +When a contributor clicks GitHub's **Update branch** button on a pull request, +GitHub creates an automatic merge commit that incorporates the latest commits +from the base branch into the PR branch. This merge commit has a +non-Conventional-Commit subject: + +---- +Merge branch 'main' into +---- + +The `git-hooks/validate-commit-range` script validates every commit subject in +the PR range against the Conventional Commits specification. Without a +targeted exception, GitHub's auto-generated merge commit causes CI to fail and +blocks PRs that would otherwise be mergeable. + +This issue was first introduced by EXT-29 (PR #218), which replaced a +metadata-aware validator with a subject-only traversal, removing the exception +that had previously been in place. + +== Decision + +Restore a narrowly scoped exception inside `validate-commit-range`. A commit +is skipped **only when all three conditions hold simultaneously**: + +1. *Exactly two parents* — the commit is a merge commit (not a regular commit + disguised with a merge-style subject). + +2. *Committer is `GitHub `* — the commit was produced by + the trusted GitHub bot, not by a contributor. + +3. *Subject matches `^Merge branch '[^']+' into .+`* — the subject is + GitHub's canonical "Update branch" format, with the base branch name + enclosed in single quotes. + +If any predicate does not match, the commit subject is validated normally. + +== Implementation + +`git log` is invoked with `--format='%P%x01%cn%x01%ce%x01%s'` so that parent +hashes, committer name, committer email, and the subject are all available +within the validation loop. Records are NUL-delimited (`-z`); fields within +each record are separated by SOH (ASCII 0x01), a character that cannot appear +in committer metadata or commit subjects in practice. + +== Rationale for each predicate + +*Parent count* prevents a regular commit from bypassing validation simply by +having a subject that starts with `Merge branch '`. The contributor subject +guard in `validate-conventional-subject` already catches this pattern and +rejects it with a helpful remediation hint; the exception must not interfere +with that guidance. + +*Committer identity* binds the exception to the specific GitHub bot account +that creates "Update branch" commits. Contributor-authored merge commits +(e.g., `git merge --no-ff`) use the contributor's own identity and are +therefore still validated. + +*Subject pattern* ensures only the exact auto-generated format is exempted. +A two-parent commit from the GitHub committer with a different subject (e.g., +a revert, or a merge of a submodule) still goes through validation. + +== Alternatives considered + +*Exempt all merge commits* — rejected. Contributors can create merge commits +with arbitrary subjects; blanket exemption would create a loophole. + +*Reword the commit before pushing* — not feasible. The commit is created by +GitHub's server-side automation after the PR branch is pushed; there is no +hook opportunity to intercept or rewrite it before it lands. + +*Allow `Merge branch '...' into ...'` as a valid Conventional Commits type* — +rejected. This would widen the allowed format for all contributors rather than +targeting the specific trusted-bot case. diff --git a/git-hooks/validate-commit-range b/git-hooks/validate-commit-range index ebb9048..40432cf 100755 --- a/git-hooks/validate-commit-range +++ b/git-hooks/validate-commit-range @@ -80,8 +80,34 @@ base_sha=$(git_or_die merge-base HEAD "$base_ref") validation_status=0 -while IFS= read -r -d '' subject +# A commit is exempt from subject validation only when ALL three conditions hold: +# 1. It has exactly two parents (is a merge commit). +# 2. Its committer is GitHub (the trusted bot identity). +# 3. Its subject matches the canonical "Update branch" pattern. +# Ordinary contributor-created merge commits (different committer, or a +# subject that doesn't match the pattern) still go through subject validation. +github_merge_pattern="^Merge branch '[^']+' into .+" + +while IFS= read -r -d '' entry do + IFS=$'\x01' read -r parents committer_name committer_email subject <<< "$entry" + + if [ -n "$parents" ] + then + IFS=' ' read -ra parents_array <<< "$parents" + parent_count=${#parents_array[@]} + else + parent_count=0 + fi + + if [ "$parent_count" -eq 2 ] \ + && [ "$committer_name" = "GitHub" ] \ + && [ "$committer_email" = "noreply@github.com" ] \ + && [[ "$subject" =~ $github_merge_pattern ]] + then + continue + fi + "$validator" --subject "$subject" status=$? @@ -89,6 +115,6 @@ do then validation_status=$status fi -done < <(git log -z --format='%s' "$base_sha..HEAD") +done < <(git log -z --format='%P%x01%cn%x01%ce%x01%s' "$base_sha..HEAD") exit "$validation_status" diff --git a/test/git_hooks_test.exs b/test/git_hooks_test.exs index c375225..e08befa 100644 --- a/test/git_hooks_test.exs +++ b/test/git_hooks_test.exs @@ -133,6 +133,76 @@ defmodule GitHooksTest do refute output =~ "feat: valid commit" end + test "the range guard skips a GitHub Update-branch merge commit matching all three predicates" do + {worktree, hooks_dir} = setup_hooks_worktree!() + add_github_merge!(worktree) + + assert {"", 0} = run(Path.join(hooks_dir, "validate-commit-range"), [], cd: worktree) + end + + test "the range guard validates when the committer name is not GitHub" do + {worktree, hooks_dir} = setup_hooks_worktree!() + add_github_merge!(worktree, committer_name: "Not GitHub") + + assert {output, 1} = run(Path.join(hooks_dir, "validate-commit-range"), [], cd: worktree) + assert output =~ "Merge branch 'main' into feature" + end + + test "the range guard validates when the committer email is not noreply@github.com" do + {worktree, hooks_dir} = setup_hooks_worktree!() + add_github_merge!(worktree, committer_email: "not@github.com") + + assert {output, 1} = run(Path.join(hooks_dir, "validate-commit-range"), [], cd: worktree) + assert output =~ "Merge branch 'main' into feature" + end + + test "the range guard validates a single-parent commit whose subject matches the GitHub pattern" do + {worktree, hooks_dir} = setup_hooks_worktree!() + + git!(worktree, ["commit", "--allow-empty", "-m", "Merge branch 'main' into feature"]) + + assert {output, 1} = run(Path.join(hooks_dir, "validate-commit-range"), [], cd: worktree) + assert output =~ "Merge branch 'main' into feature" + end + + test "the range guard validates a two-parent GitHub-committer merge with a non-matching subject" do + {worktree, hooks_dir} = setup_hooks_worktree!() + add_github_merge!(worktree, subject: "Merge feature into main") + + assert {output, 1} = run(Path.join(hooks_dir, "validate-commit-range"), [], cd: worktree) + assert output =~ "Merge feature into main" + end + + # Creates a no-fast-forward merge commit on `main` from a throwaway `feature` + # branch. Defaults simulate GitHub's "Update branch" committer identity and + # subject so the predicate in validate-commit-range matches. + defp add_github_merge!(worktree, opts \\ []) do + committer_name = Keyword.get(opts, :committer_name, "GitHub") + committer_email = Keyword.get(opts, :committer_email, "noreply@github.com") + subject = Keyword.get(opts, :subject, "Merge branch 'main' into feature") + + git!(worktree, ["checkout", "-b", "feature"]) + File.write!(Path.join(worktree, "feature_file"), "feature content") + git!(worktree, ["add", "feature_file"]) + git!(worktree, ["commit", "-m", "feat: add feature"]) + git!(worktree, ["checkout", "main"]) + + git_with_env!(worktree, ["merge", "--no-ff", "-m", subject, "feature"], [ + {"GIT_COMMITTER_NAME", committer_name}, + {"GIT_COMMITTER_EMAIL", committer_email} + ]) + end + + defp git_with_env!(directory, args, extra_env) do + case System.cmd("git", args, cd: directory, stderr_to_stdout: true, env: extra_env) do + {_output, 0} -> + :ok + + {output, status} -> + flunk("git #{Enum.join(args, " ")} failed (#{status}):\n#{output}") + end + end + # Creates a git repo in a temp dir with origin set up and the guard scripts # copied in. Uses a cryptographic nonce so collisions cannot occur across # BEAM VM restarts (unlike System.unique_integer/1 which resets each run).