From fca689c55436124e31adbc5edc754cd80943f6a9 Mon Sep 17 00:00:00 2001 From: David Phillips Date: Tue, 28 Jul 2026 14:14:11 -0700 Subject: [PATCH] Add commit message check --- .github/workflows/ci.yml | 27 ++ .gitignore | 2 + README.md | 11 + check-commit-messages/README.md | 34 +++ check-commit-messages/action.yml | 22 ++ check-commit-messages/check.py | 370 ++++++++++++++++++++++++++++ check-commit-messages/test_check.py | 150 +++++++++++ 7 files changed, 616 insertions(+) create mode 100644 .github/workflows/ci.yml create mode 100644 .gitignore create mode 100644 README.md create mode 100644 check-commit-messages/README.md create mode 100644 check-commit-messages/action.yml create mode 100755 check-commit-messages/check.py create mode 100755 check-commit-messages/test_check.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..234f9db --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,27 @@ +name: ci + +on: + pull_request: + merge_group: + push: + branches: + - main + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7.0.0 + with: + fetch-depth: 0 + persist-credentials: false + - name: Test commit message checker + run: python3 check-commit-messages/test_check.py + - name: Check commit messages + if: github.event_name == 'pull_request' + uses: ./check-commit-messages + with: + base_ref: ${{ github.event.pull_request.base.ref }} diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..7a60b85 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +__pycache__/ +*.pyc diff --git a/README.md b/README.md new file mode 100644 index 0000000..5673398 --- /dev/null +++ b/README.md @@ -0,0 +1,11 @@ +# GitHub Actions + +Shared GitHub Actions for Airlift projects. + +## Available actions + +### Check commit messages + +Checks pull request commit titles, description wrapping, and attribution +trailers. See [check-commit-messages](check-commit-messages/README.md) for +usage and policy details. diff --git a/check-commit-messages/README.md b/check-commit-messages/README.md new file mode 100644 index 0000000..d262656 --- /dev/null +++ b/check-commit-messages/README.md @@ -0,0 +1,34 @@ +# Check commit messages + +Checks every non-merge commit in a pull request. + +## Policy + +- Commit titles should be at most 50 characters and must not exceed 60. +- Commit descriptions should wrap at 72 characters. Ordinary text must not + exceed 79 characters. +- `Assisted-by` and `Co-authored-by` trailers must not credit common AI models + or coding tools. Human attribution remains allowed. + +Long URLs, recognized trailers, quoted text, fenced code blocks, and long +unwrappable tokens are exempt from description wrapping. + +## Usage + +The action requires a checkout with complete history: + +```yaml +check-commit-messages: + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7.0.0 + with: + fetch-depth: 0 + persist-credentials: false + - uses: airlift/github-actions/check-commit-messages@ # v1 + with: + base_ref: ${{ github.event.pull_request.base.ref }} +``` + +Consumers should pin the action to a complete commit SHA. diff --git a/check-commit-messages/action.yml b/check-commit-messages/action.yml new file mode 100644 index 0000000..a1cd2dd --- /dev/null +++ b/check-commit-messages/action.yml @@ -0,0 +1,22 @@ +name: Check commit messages +description: Check commit title length, description wrapping, and attribution trailers + +inputs: + base_ref: + description: Pull request base branch + required: true + +runs: + using: composite + steps: + - name: Check commit messages + shell: bash + env: + BASE_REF: ${{ inputs.base_ref }} + run: | + if [[ ! "$BASE_REF" =~ ^[A-Za-z0-9._/-]+$ ]]; then + echo "Invalid pull request base ref: $BASE_REF" >&2 + exit 1 + fi + python3 "${{ github.action_path }}/check.py" \ + "origin/${BASE_REF}..HEAD" diff --git a/check-commit-messages/check.py b/check-commit-messages/check.py new file mode 100755 index 0000000..ef845da --- /dev/null +++ b/check-commit-messages/check.py @@ -0,0 +1,370 @@ +#!/usr/bin/env python3 + +import argparse +import re +import subprocess +import sys +from dataclasses import dataclass + +RECOMMENDED_SUBJECT_LENGTH = 50 +MAX_SUBJECT_LENGTH = 60 +RECOMMENDED_DESCRIPTION_LINE_LENGTH = 72 +MAX_DESCRIPTION_LINE_LENGTH = 79 + +REVISION_RANGE_PATTERN = re.compile(r"^origin/[A-Za-z0-9._/-]+\.\.HEAD$") +URL_PATTERN = re.compile(r"(?:https?://|ssh://|git@|www\.)\S+") +TRAILER_PATTERN = re.compile( + r"^(?:" + r"Signed-off-by|Co-authored-by|Assisted-by|Reviewed-by|Acked-by|" + r"Tested-by|Reported-by|Fixes|Refs|Relates-to|Change-Id" + r"):\s+\S.+$", + re.IGNORECASE, +) +ATTRIBUTION_PATTERN = re.compile( + r"^(?:Assisted-by|Co-authored-by):\s*\S.*$", + re.IGNORECASE, +) +PROHIBITED_ATTRIBUTION_MARKERS = ( + "aider", + "claude", + "cline", + "codex", + "copilot", + "cursor", + "devin", + "gemini", + "gpt", + "windsurf", +) + + +@dataclass(frozen=True) +class CommitSubjectViolation: + commit: str + subject: str + length: int + + +@dataclass(frozen=True) +class CommitDescriptionViolation: + commit: str + subject: str + line_number: int + length: int + line: str + + +@dataclass(frozen=True) +class CommitAttributionViolation: + commit: str + subject: str + line_number: int + line: str + + +def run_git(arguments: list[str]) -> str: + try: + result = subprocess.run( + ["git", *arguments], + check=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + except subprocess.CalledProcessError as exception: + print(f"git {' '.join(arguments)} failed:", file=sys.stderr) + print(exception.stderr, file=sys.stderr) + raise SystemExit(exception.returncode) from exception + + return result.stdout + + +def get_commits(revision_range: str) -> list[str]: + if REVISION_RANGE_PATTERN.fullmatch(revision_range) is None: + print( + "Revision range must match origin/..HEAD with a safe base ref.", + file=sys.stderr, + ) + raise SystemExit(1) + + output = run_git(["rev-list", "--reverse", "--no-merges", revision_range]) + return [line for line in output.splitlines() if line] + + +def get_commit_message(commit: str) -> str: + return run_git(["show", "-s", "--format=%B", commit]) + + +def get_subject_violations( + commit: str, message: str +) -> list[CommitSubjectViolation]: + lines = message.splitlines() + if not lines or len(lines[0]) <= MAX_SUBJECT_LENGTH: + return [] + + return [ + CommitSubjectViolation( + commit=commit, + subject=lines[0], + length=len(lines[0]), + ) + ] + + +def is_wrapping_exempt(line: str, in_code_block: bool) -> bool: + stripped = line.strip() + + if in_code_block: + return True + if not stripped: + return True + if stripped.startswith(">"): + return True + if TRAILER_PATTERN.fullmatch(stripped): + return True + + tokens = stripped.split() + if any(URL_PATTERN.search(token) for token in tokens): + return is_wrapped_after_removing_unwrappable_tokens(tokens) + if any(len(token) > MAX_DESCRIPTION_LINE_LENGTH for token in tokens): + return is_wrapped_after_removing_unwrappable_tokens(tokens) + + return False + + +def is_wrapped_after_removing_unwrappable_tokens(tokens: list[str]) -> bool: + wrappable_tokens = [ + token + for token in tokens + if ( + not URL_PATTERN.search(token) + and len(token) <= MAX_DESCRIPTION_LINE_LENGTH + ) + ] + return len(" ".join(wrappable_tokens)) <= MAX_DESCRIPTION_LINE_LENGTH + + +def get_description_violations( + commit: str, message: str +) -> list[CommitDescriptionViolation]: + lines = message.splitlines() + if not lines: + return [] + + subject = lines[0] + in_code_block = False + violations = [] + + for line_number, line in enumerate(lines[1:], start=2): + stripped = line.strip() + starts_code_fence = stripped.startswith("```") or stripped.startswith("~~~") + + if ( + len(line) > MAX_DESCRIPTION_LINE_LENGTH + and not starts_code_fence + and not is_wrapping_exempt(line, in_code_block) + ): + violations.append( + CommitDescriptionViolation( + commit=commit, + subject=subject, + line_number=line_number, + length=len(line), + line=line, + ) + ) + + if starts_code_fence: + in_code_block = not in_code_block + + return violations + + +def get_attribution_violations( + commit: str, message: str +) -> list[CommitAttributionViolation]: + lines = message.splitlines() + if not lines: + return [] + + subject = lines[0] + in_code_block = False + violations = [] + + for line_number, line in enumerate(lines[1:], start=2): + stripped = line.strip() + starts_code_fence = stripped.startswith("```") or stripped.startswith("~~~") + + if starts_code_fence: + in_code_block = not in_code_block + continue + if in_code_block or stripped.startswith(">"): + continue + + if ATTRIBUTION_PATTERN.fullmatch(stripped) is None: + continue + + normalized_line = stripped.casefold() + if any( + marker in normalized_line + for marker in PROHIBITED_ATTRIBUTION_MARKERS + ): + violations.append( + CommitAttributionViolation( + commit=commit, + subject=subject, + line_number=line_number, + line=line, + ) + ) + + return violations + + +def check_commit_messages( + revision_range: str, +) -> tuple[ + list[str], + list[CommitSubjectViolation], + list[CommitDescriptionViolation], + list[CommitAttributionViolation], +]: + commits = get_commits(revision_range) + subject_violations = [] + description_violations = [] + attribution_violations = [] + + for commit in commits: + message = get_commit_message(commit) + subject_violations.extend(get_subject_violations(commit, message)) + description_violations.extend(get_description_violations(commit, message)) + attribution_violations.extend(get_attribution_violations(commit, message)) + + return ( + commits, + subject_violations, + description_violations, + attribution_violations, + ) + + +def print_subject_violations( + violations: list[CommitSubjectViolation], +) -> None: + print( + f"Commit subjects should be at most {RECOMMENDED_SUBJECT_LENGTH} " + f"characters; this check fails subjects over {MAX_SUBJECT_LENGTH} " + "characters.", + file=sys.stderr, + ) + print(file=sys.stderr) + + for violation in violations: + print_commit_header(violation.commit, violation.subject) + print( + f" subject: {violation.length} characters", + file=sys.stderr, + ) + print(file=sys.stderr) + + +def print_description_violations( + violations: list[CommitDescriptionViolation], +) -> None: + print( + "Commit descriptions should wrap at " + f"{RECOMMENDED_DESCRIPTION_LINE_LENGTH} characters; this check fails " + f"ordinary text over {MAX_DESCRIPTION_LINE_LENGTH} characters.", + file=sys.stderr, + ) + print( + "Long URLs, trailers, quoted text, code blocks, and long unwrappable " + "tokens are allowed.", + file=sys.stderr, + ) + print(file=sys.stderr) + + for violation in violations: + print_commit_header(violation.commit, violation.subject) + print( + f" line {violation.line_number}: {violation.length} characters", + file=sys.stderr, + ) + print(f" {violation.line}", file=sys.stderr) + print(file=sys.stderr) + + +def print_attribution_violations( + violations: list[CommitAttributionViolation], +) -> None: + print( + "AI models and coding tools must not be credited with Assisted-by or " + "Co-authored-by trailers.", + file=sys.stderr, + ) + print("Human attributions using these trailers are allowed.", file=sys.stderr) + print(file=sys.stderr) + + for violation in violations: + print_commit_header(violation.commit, violation.subject) + print( + f" line {violation.line_number}: prohibited AI/tool attribution", + file=sys.stderr, + ) + print(f" {violation.line}", file=sys.stderr) + print(file=sys.stderr) + + +def print_commit_header(commit: str, subject: str) -> None: + print(f"{format_commit_reference(commit)} {subject}", file=sys.stderr) + + +def format_commit_reference(commit: str) -> str: + if re.fullmatch(r"[0-9a-fA-F]{40}", commit): + return commit[:12] + return commit + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser( + description=( + "Check commit subject length, description wrapping, " + "and attribution trailers." + ) + ) + parser.add_argument( + "revision_range", + help="Git revision range to check, such as origin/main..HEAD.", + ) + return parser.parse_args() + + +def main() -> int: + args = parse_args() + ( + commits, + subject_violations, + description_violations, + attribution_violations, + ) = check_commit_messages(args.revision_range) + commit_count = len(commits) + + if subject_violations: + print_subject_violations(subject_violations) + if description_violations: + print_description_violations(description_violations) + if attribution_violations: + print_attribution_violations(attribution_violations) + if subject_violations or description_violations or attribution_violations: + return 1 + + noun = "message" if commit_count == 1 else "messages" + print( + f"Checked {commit_count} commit {noun}; subjects and descriptions " + "meet length limits and no prohibited attributions were found." + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/check-commit-messages/test_check.py b/check-commit-messages/test_check.py new file mode 100755 index 0000000..ddbbf5d --- /dev/null +++ b/check-commit-messages/test_check.py @@ -0,0 +1,150 @@ +#!/usr/bin/env python3 + +import unittest + +from check import ( + PROHIBITED_ATTRIBUTION_MARKERS, + get_attribution_violations, + get_description_violations, + get_subject_violations, +) + + +class TestCommitMessages(unittest.TestCase): + def test_subject_length(self) -> None: + cases = [ + ("60 characters", "x" * 60, []), + ("61 characters", "x" * 61, [61]), + ] + + for name, subject, expected_lengths in cases: + with self.subTest(name=name): + violations = get_subject_violations( + "abc123", + f"{subject}\n\nThis line is wrapped.", + ) + self.assertEqual( + [violation.length for violation in violations], + expected_lengths, + ) + + def test_description_wrapping(self) -> None: + cases = [ + ( + "wrapped description", + "This line is wrapped.\n" + "This line is also wrapped before it gets too wide.", + [], + ), + ("79 characters", "x" * 79, []), + ( + "80 characters", + "This line is exactly eighty characters long and it should " + "fail as written here.!", + [3], + ), + ( + "long URL", + "See https://example.com/path/that/is/long/enough/to/exceed/" + "the/wrap/limit for context.", + [], + ), + ( + "unwrapped text around a URL", + "This surrounding prose is still far too long and should not " + "be hidden behind a long URL. https://example.com/long/url", + [3], + ), + ( + "long unwrappable token", + "Use com.example.really.long.package.name.with.enough.parts." + "to.exceed.the.wrap.limit.without.spaces.", + [], + ), + ( + "code block", + "```\n" + "This line is ordinary prose in a code block but should not " + "be checked by wrapping rules.\n" + "```", + [], + ), + ( + "block quote", + "> This quoted body line can exceed seventy-nine characters " + "because wrapping it would alter quoted text.", + [], + ), + ( + "trailer", + "Signed-off-by: Example Person With A Very Long Name " + "", + [], + ), + ] + + for name, body, expected_lines in cases: + with self.subTest(name=name): + self.assertEqual( + description_violating_lines(commit_message(body)), + expected_lines, + ) + + def test_rejects_prohibited_attribution_markers(self) -> None: + for marker in PROHIBITED_ATTRIBUTION_MARKERS: + with self.subTest(marker=marker): + self.assertEqual( + attribution_violating_lines( + commit_message( + f"Co-authored-by: {marker} " + ) + ), + [3], + ) + + def test_rejects_assisted_by_case_insensitively(self) -> None: + self.assertEqual( + attribution_violating_lines( + commit_message( + "assisted-BY: Internal CoDeX helper " + ) + ), + [3], + ) + + def test_accepts_human_attributions(self) -> None: + message = commit_message( + "Assisted-by: Alex Example \n" + "Co-authored-by: Taylor Example " + ) + self.assertEqual(attribution_violating_lines(message), []) + + def test_accepts_examples_that_are_not_attribution_trailers(self) -> None: + messages = [ + commit_message( + "```\nCo-authored-by: ChatGPT \n```" + ), + commit_message("> Co-authored-by: ChatGPT "), + ] + + for message in messages: + with self.subTest(message=message): + self.assertEqual(attribution_violating_lines(message), []) + + +def commit_message(body: str) -> str: + return f"Add useful check\n\n{body}" + + +def description_violating_lines(message: str) -> list[int]: + violations = get_description_violations("abc123", message) + return [violation.line_number for violation in violations] + + +def attribution_violating_lines(message: str) -> list[int]: + violations = get_attribution_violations("abc123", message) + return [violation.line_number for violation in violations] + + +if __name__ == "__main__": + unittest.main()