ci(workflows): add commit message and PR title quality checks

Add CI enforcement of conventional commit format for PR titles and
commit messages. Includes three Python scripts under Tools/ci/:

- conventional_commits.py: shared parsing/validation library
- check_pr_title.py: validates PR title format, suggests fixes
- check_commit_messages.py: checks commits for blocking errors
  (fixup/squash/WIP leftovers) and advisory warnings (review-response,
  formatter-only commits)

The workflow (.github/workflows/commit_checks.yml) posts concise
GitHub PR comments with actionable suggestions and auto-removes them
once issues are resolved.

Also updates CONTRIBUTING.md and docs with the conventional commits
convention.

Signed-off-by: Ramon Roche <mrpollo@gmail.com>
This commit is contained in:
Ramon Roche
2026-03-05 16:06:25 -08:00
parent 343fd01e19
commit 4da97eb4fd
8 changed files with 1036 additions and 48 deletions

331
Tools/ci/check_commit_messages.py Executable file
View File

@@ -0,0 +1,331 @@
#!/usr/bin/env python3
"""Validate commit messages in a PR against conventional commits format.
Reads a JSON array of GitHub commit objects from stdin (as returned by the
GitHub API's /pulls/{n}/commits endpoint) and checks each message for
blocking errors and advisory warnings.
With --markdown, outputs a formatted PR comment body instead of plain text.
"""
import json
import sys
from conventional_commits import (
EXEMPT_PREFIXES,
parse_header,
)
# Blocking: prefixes that indicate unsquashed fixup commits
FIXUP_PREFIXES = ('fixup!', 'squash!', 'amend!')
# Blocking: single-word throwaway messages (case-insensitive exact match)
THROWAWAY_WORDS = frozenset({
'fix', 'fixed', 'fixes',
'update', 'updated', 'updates',
'test', 'tests', 'testing',
'tmp', 'temp',
'oops', 'wip',
'debug', 'cleanup',
})
# Blocking: debug session leftovers
DEBUG_KEYWORDS = ('tmate',)
# Warning: review-response messages (case-insensitive substring match)
REVIEW_RESPONSE_PATTERNS = (
'address review',
'apply suggestions from code review',
'code review',
)
# Warning: formatter-only commits
FORMATTER_PATTERNS = (
'do make format',
'make format',
'run formatter',
'apply format',
)
MIN_MESSAGE_LENGTH = 5
def check_commit(message: str) -> tuple[list[str], list[str]]:
"""Return (errors, warnings) for a single commit message."""
errors: list[str] = []
warnings: list[str] = []
first_line = message.split('\n', 1)[0].strip()
lower = first_line.lower()
# --- Blocking checks ---
for prefix in FIXUP_PREFIXES:
if lower.startswith(prefix):
errors.append(f'Unsquashed commit: starts with "{prefix}"')
if lower == 'wip' or lower.startswith('wip ') or lower.startswith('wip:'):
errors.append('WIP commit should not be merged')
if len(first_line) < MIN_MESSAGE_LENGTH:
errors.append(f'Message too short ({len(first_line)} chars, minimum {MIN_MESSAGE_LENGTH})')
if first_line.strip() and first_line.strip().lower() in THROWAWAY_WORDS:
errors.append(f'Single-word throwaway message: "{first_line.strip()}"')
for kw in DEBUG_KEYWORDS:
if kw in lower:
errors.append(f'Debug session leftover: contains "{kw}"')
# --- Warning checks ---
for pattern in REVIEW_RESPONSE_PATTERNS:
if pattern in lower:
warnings.append('Review-response commit')
break
for pattern in FORMATTER_PATTERNS:
if pattern in lower:
warnings.append('Formatter-only commit')
break
if not parse_header(first_line):
# Exempt merge commits
for prefix in EXEMPT_PREFIXES:
if first_line.startswith(prefix):
break
else:
warnings.append(
'Missing conventional commit format '
'(e.g. "feat(ekf2): add something")'
)
return errors, warnings
def suggest_commit(message: str) -> str | None:
"""Suggest how to fix a bad commit message."""
first_line = message.split('\n', 1)[0].strip()
lower = first_line.lower()
for prefix in FIXUP_PREFIXES:
if lower.startswith(prefix):
return 'Squash this into the commit it fixes'
if lower == 'wip' or lower.startswith('wip ') or lower.startswith('wip:'):
return 'Reword with a descriptive message (e.g. "feat(scope): what changed")'
if len(first_line) < MIN_MESSAGE_LENGTH:
return 'Reword with a descriptive message (e.g. "feat(ekf2): what changed")'
if first_line.strip().lower() in THROWAWAY_WORDS:
return 'Reword with a descriptive message (e.g. "fix(scope): what changed")'
return None
def format_plain(data: list) -> tuple[bool, bool]:
"""Print plain text output. Returns (has_blocking, has_warnings)."""
has_blocking = False
has_warnings = False
for commit in data:
sha = commit.get('sha', '?')[:10]
message = commit.get('commit', {}).get('message', '')
first_line = message.split('\n', 1)[0].strip()
errors, warnings = check_commit(message)
if errors or warnings:
print(f"\n {sha} {first_line}")
for err in errors:
print(f" ERROR: {err}")
has_blocking = True
for warn in warnings:
print(f" WARNING: {warn}")
has_warnings = True
if has_blocking:
print(
"\n"
"ERROR = must fix before merging (CI will block the PR)\n"
"WARNING = advisory, not blocking, but recommended to fix\n"
"\n"
"See the contributing guide for details:\n"
" https://github.com/PX4/PX4-Autopilot/blob/main/CONTRIBUTING.md#commit-message-convention\n",
)
elif has_warnings:
print(
"\n"
"WARNING = advisory, not blocking, but recommended to fix\n"
"\n"
"See the contributing guide for details:\n"
" https://github.com/PX4/PX4-Autopilot/blob/main/CONTRIBUTING.md#commit-message-convention\n",
)
return has_blocking, has_warnings
def format_markdown_blocking(data: list) -> str:
"""Format a blocking error markdown comment."""
error_groups: dict[str, list[str]] = {}
unique_commits: list[tuple[str, str, list[str], str]] = []
for commit in data:
sha = commit.get('sha', '?')[:10]
message = commit.get('commit', {}).get('message', '')
first_line = message.split('\n', 1)[0].strip()
errors, _ = check_commit(message)
if not errors:
continue
suggestion = suggest_commit(message) or ''
unique_commits.append((sha, first_line, errors, suggestion))
for err in errors:
error_groups.setdefault(err, []).append(sha)
lines = [
"## \u274c Commit messages need attention before merging",
"",
]
has_large_group = any(len(shas) > 3 for shas in error_groups.values())
if has_large_group:
lines.extend([
"**Issues found:**",
"",
])
for err_msg, shas in error_groups.items():
if len(shas) > 3:
lines.append(f"- **{len(shas)} commits**: {err_msg} "
f"(`{shas[0]}`, `{shas[1]}`, ... `{shas[-1]}`)")
else:
sha_list = ', '.join(f'`{s}`' for s in shas)
lines.append(f"- {err_msg}: {sha_list}")
distinct_messages = {msg for _, msg, _, _ in unique_commits}
if len(distinct_messages) <= 5:
lines.extend(["", "**Affected commits:**", ""])
for sha, msg, errors, suggestion in unique_commits:
safe_msg = msg.replace('|', '\\|')
lines.append(f"- `{sha}` {safe_msg}")
else:
lines.extend([
"| Commit | Message | Issue | Suggested fix |",
"|--------|---------|-------|---------------|",
])
for sha, msg, errors, suggestion in unique_commits:
issues = '; '.join(errors)
safe_msg = msg.replace('|', '\\|')
lines.append(f"| `{sha}` | {safe_msg} | {issues} | {suggestion} |")
lines.extend([
"",
"See [CONTRIBUTING.md](https://github.com/PX4/PX4-Autopilot/blob/main/CONTRIBUTING.md#commit-message-convention) "
"for how to clean up commits.",
"",
"---",
"*This comment will be automatically removed once the issues are resolved.*",
])
return '\n'.join(lines)
def format_markdown_advisory(data: list) -> str:
"""Format an advisory warning markdown comment."""
lines = [
"## \U0001f4a1 Commit messages could be improved",
"",
"Not blocking, but these commit messages could use some cleanup.",
"",
"| Commit | Message | Suggestion |",
"|--------|---------|------------|",
]
for commit in data:
sha = commit.get('sha', '?')[:10]
message = commit.get('commit', {}).get('message', '')
first_line = message.split('\n', 1)[0].strip()
_, warnings = check_commit(message)
if not warnings:
continue
suggestion = '; '.join(warnings)
safe_msg = first_line.replace('|', '\\|')
lines.append(f"| `{sha}` | {safe_msg} | {suggestion} |")
lines.extend([
"",
"See the [commit message convention](https://github.com/PX4/PX4-Autopilot/blob/main/CONTRIBUTING.md#commit-message-convention) "
"for details.",
"",
"---",
"*This comment will be automatically removed once the issues are resolved.*",
])
return '\n'.join(lines)
def main() -> None:
markdown_stdout = '--markdown' in sys.argv
markdown_file = None
for i, a in enumerate(sys.argv):
if a == '--markdown-file' and i + 1 < len(sys.argv):
markdown_file = sys.argv[i + 1]
elif a.startswith('--markdown-file='):
markdown_file = a.split('=', 1)[1]
try:
data = json.load(sys.stdin)
except json.JSONDecodeError as exc:
print(f"Failed to parse JSON input: {exc}", file=sys.stderr)
sys.exit(2)
if not isinstance(data, list):
print("Expected a JSON array of commit objects.", file=sys.stderr)
sys.exit(2)
# Always compute blocking/warning state
has_blocking = False
has_warnings = False
for commit in data:
message = commit.get('commit', {}).get('message', '')
errors, warnings = check_commit(message)
if errors:
has_blocking = True
if warnings:
has_warnings = True
# Generate markdown if needed
md = None
if has_blocking:
md = format_markdown_blocking(data)
elif has_warnings:
md = format_markdown_advisory(data)
if md:
if markdown_stdout:
print(md)
if markdown_file:
with open(markdown_file, 'w') as f:
f.write(md + '\n')
elif markdown_file:
with open(markdown_file, 'w') as f:
pass
# Plain text output to stderr for CI logs (always, unless --markdown only)
if not markdown_stdout:
has_blocking, _ = format_plain(data)
sys.exit(1 if has_blocking else 0)
if __name__ == '__main__':
main()

163
Tools/ci/check_pr_title.py Executable file
View File

@@ -0,0 +1,163 @@
#!/usr/bin/env python3
"""Validate that a PR title follows conventional commits format.
Format: type(scope): description
Can output plain text for CI logs or markdown for PR comments.
"""
import re
import sys
from conventional_commits import (
CONVENTIONAL_TYPES,
EXEMPT_PREFIXES,
parse_header,
suggest_scope,
suggest_type,
)
def suggest_title(title: str) -> str | None:
"""Try to suggest a corrected title in conventional commits format."""
stripped = title.strip()
# Remove common bracket prefixes like [docs], [CI], etc.
bracket_match = re.match(r'^\[([^\]]+)\]\s*(.+)', stripped)
if bracket_match:
prefix = bracket_match.group(1).strip().lower()
rest = bracket_match.group(2).strip()
rest = re.sub(r'^[\-:]\s*', '', rest).strip()
if len(rest) >= 5:
# Try to map bracket content to a type
commit_type = prefix if prefix in CONVENTIONAL_TYPES else suggest_type(rest)
scope = suggest_scope(rest)
if scope:
return f"{commit_type}({scope}): {rest}"
# Already has old-style "subsystem: description" format - convert it
colon_match = re.match(r'^([a-zA-Z][a-zA-Z0-9_/\-\. ]*): (.+)$', stripped)
if colon_match:
old_subsystem = colon_match.group(1).strip()
desc = colon_match.group(2).strip()
if len(desc) >= 5:
commit_type = suggest_type(desc)
# Use the old subsystem as scope (clean it up)
scope = old_subsystem.lower().replace(' ', '_')
return f"{commit_type}({scope}): {desc}"
# No format at all - try to guess both type and scope
commit_type = suggest_type(stripped)
scope = suggest_scope(stripped)
if scope:
desc = stripped[0].lower() + stripped[1:] if stripped else stripped
return f"{commit_type}({scope}): {desc}"
return None
def check_title(title: str) -> bool:
title = title.strip()
if not title:
print("PR title is empty.", file=sys.stderr)
return False
for prefix in EXEMPT_PREFIXES:
if title.startswith(prefix):
return True
if parse_header(title):
return True
types_str = ', '.join(f'`{t}`' for t in CONVENTIONAL_TYPES.keys())
print(
f"PR title does not match conventional commits format.\n"
f"\n"
f" Title: {title}\n"
f"\n"
f"Expected format: type(scope): description\n"
f"\n"
f"Valid types: {types_str}\n"
f"\n"
f"Good examples:\n"
f" feat(ekf2): add height fusion timeout\n"
f" fix(mavlink): correct BATTERY_STATUS_V2 parsing\n"
f" ci(workflows): migrate to reusable workflows\n"
f" feat(boards/px4_fmu-v6x)!: remove deprecated driver API\n"
f"\n"
f"Bad examples:\n"
f" fix stuff\n"
f" Update file\n"
f" ekf2: fix something (missing type prefix)\n"
f"\n"
f"See the contributing guide for details:\n"
f" https://github.com/PX4/PX4-Autopilot/blob/main/CONTRIBUTING.md#commit-message-convention\n",
file=sys.stderr,
)
return False
def format_markdown(title: str) -> str:
"""Format a markdown PR comment body for a bad title."""
lines = [
"## \u274c PR title needs conventional commit format",
"",
"Expected format: `type(scope): description` "
"([conventional commits](https://www.conventionalcommits.org/)).",
"",
"**Your title:**",
f"> {title}",
"",
]
suggestion = suggest_title(title)
if suggestion:
lines.extend([
"**Suggested fix:**",
f"> {suggestion}",
"",
])
lines.extend([
"**To fix this:** click the ✏️ next to the PR title at the top "
"of this page and update it.",
"",
"See [CONTRIBUTING.md](https://github.com/PX4/PX4-Autopilot/blob/main/CONTRIBUTING.md#commit-message-convention) "
"for details.",
"",
"---",
"*This comment will be automatically removed once the issue is resolved.*",
])
return '\n'.join(lines)
def main() -> None:
import argparse
parser = argparse.ArgumentParser(description='Check PR title format')
parser.add_argument('title', help='The PR title to validate')
parser.add_argument('--markdown', action='store_true',
help='Output markdown to stdout on failure')
parser.add_argument('--markdown-file', metavar='FILE',
help='Write markdown to FILE on failure')
args = parser.parse_args()
passed = check_title(args.title)
if not passed:
md = format_markdown(args.title)
if args.markdown:
print(md)
if args.markdown_file:
with open(args.markdown_file, 'w') as f:
f.write(md + '\n')
elif args.markdown_file:
with open(args.markdown_file, 'w') as f:
pass
sys.exit(0 if passed else 1)
if __name__ == '__main__':
main()

View File

@@ -0,0 +1,146 @@
"""Shared constants and helpers for conventional commit validation.
Format: type(scope): description
Optional breaking change marker: type(scope)!: description
"""
import re
CONVENTIONAL_TYPES = {
'feat': 'A new feature',
'fix': 'A bug fix',
'docs': 'Documentation only changes',
'style': 'Formatting, whitespace, no code change',
'refactor': 'Code change that neither fixes a bug nor adds a feature',
'perf': 'Performance improvement',
'test': 'Adding or correcting tests',
'build': 'Build system or external dependencies',
'ci': 'CI configuration files and scripts',
'chore': 'Other changes that don\'t modify src or test files',
'revert': 'Reverts a previous commit',
}
# type(scope)[!]: description
# - type: one of CONVENTIONAL_TYPES keys
# - scope: required, alphanumeric with _/-/.
# - !: optional breaking change marker
# - description: at least 5 chars
HEADER_PATTERN = re.compile(
r'^(' + '|'.join(CONVENTIONAL_TYPES.keys()) + r')'
r'\(([a-zA-Z0-9_/\-\.]+)\)'
r'(!)?'
r': (.{5,})$'
)
EXEMPT_PREFIXES = ('Merge ',)
# Common PX4 subsystem scopes for suggestions
KNOWN_SCOPES = [
'ekf2', 'mavlink', 'commander', 'navigator', 'sensors',
'mc_att_control', 'mc_pos_control', 'mc_rate_control',
'fw_att_control', 'fw_pos_control', 'fw_rate_control',
'vtol', 'actuators', 'battery', 'param', 'logger',
'uorb', 'drivers', 'boards', 'simulation', 'sitl',
'gps', 'rc', 'safety', 'can', 'serial',
'ci', 'docs', 'build', 'cmake', 'tools',
'mixer', 'land_detector', 'airspeed', 'gyroscope',
'accelerometer', 'magnetometer', 'barometer',
]
# Keyword patterns to suggest scopes from description text
KEYWORD_SCOPES = [
(r'\b(ekf|estimator|height|fusion|imu|baro)\b', 'ekf2'),
(r'\b(mavlink|MAVLink|MAVLINK|command_int|heartbeat)\b', 'mavlink'),
(r'\b(uorb|orb|pub|sub|topic)\b', 'uorb'),
(r'\b(board|fmu|nuttx|stm32)\b', 'boards'),
(r'\b(mixer|actuator|motor|servo|pwm|dshot)\b', 'actuators'),
(r'\b(battery|power)\b', 'battery'),
(r'\b(param|parameter)\b', 'param'),
(r'\b(log|logger|sdlog)\b', 'logger'),
(r'\b(sensor|accel|gyro)\b', 'sensors'),
(r'\b(land|takeoff|rtl|mission|navigator|geofence)\b', 'navigator'),
(r'\b(position|velocity|attitude|rate)\s*(control|ctrl)\b', 'mc_att_control'),
(r'\b(mc|multicopter|quad)\b', 'mc_att_control'),
(r'\b(fw|fixedwing|fixed.wing|plane)\b', 'fw_att_control'),
(r'\b(vtol|transition)\b', 'vtol'),
(r'\b(ci|workflow|github.action|pipeline)\b', 'ci'),
(r'\b(doc|docs|documentation|readme)\b', 'docs'),
(r'\b(cmake|make|toolchain|compiler)\b', 'build'),
(r'\b(sitl|simulation|gazebo|jmavsim|sih)\b', 'simulation'),
(r'\b(can|uavcan|cyphal|dronecan)\b', 'can'),
(r'\b(serial|uart|spi|i2c)\b', 'serial'),
(r'\b(safety|failsafe|arm|disarm|kill)\b', 'safety'),
(r'\b(rc|radio|sbus|crsf|elrs|dsm)\b', 'rc'),
(r'\b(gps|gnss|rtk|ubx)\b', 'gps'),
(r'\b(optical.flow|flow|rangefinder|lidar|distance)\b', 'sensors'),
(r'\b(orbit|follow|offboard)\b', 'commander'),
(r'\b(driver)\b', 'drivers'),
]
# Verb patterns to suggest conventional commit type
VERB_TYPE_MAP = [
(r'^fix(e[ds])?[\s:]', 'fix'),
(r'^bug[\s:]', 'fix'),
(r'^add(s|ed|ing)?[\s:]', 'feat'),
(r'^implement', 'feat'),
(r'^introduce', 'feat'),
(r'^support', 'feat'),
(r'^enable', 'feat'),
(r'^update[ds]?[\s:]', 'feat'),
(r'^improv(e[ds]?|ing)', 'perf'),
(r'^optimi[zs](e[ds]?|ing)', 'perf'),
(r'^refactor', 'refactor'),
(r'^clean\s*up', 'refactor'),
(r'^restructure', 'refactor'),
(r'^simplif(y|ied)', 'refactor'),
(r'^remov(e[ds]?|ing)', 'refactor'),
(r'^delet(e[ds]?|ing)', 'refactor'),
(r'^deprecat', 'refactor'),
(r'^replac(e[ds]?|ing)', 'refactor'),
(r'^renam(e[ds]?|ing)', 'refactor'),
(r'^migrat', 'refactor'),
(r'^revert', 'revert'),
(r'^doc(s|ument)', 'docs'),
(r'^test', 'test'),
(r'^format', 'style'),
(r'^lint', 'style'),
(r'^whitespace', 'style'),
(r'^build', 'build'),
(r'^ci[\s:]', 'ci'),
]
def parse_header(text: str) -> dict | None:
"""Parse a conventional commit header into components.
Returns dict with keys {type, scope, breaking, subject} or None if
the text doesn't match conventional commits format.
"""
text = text.strip()
m = HEADER_PATTERN.match(text)
if not m:
return None
return {
'type': m.group(1),
'scope': m.group(2),
'breaking': m.group(3) == '!',
'subject': m.group(4),
}
def suggest_type(text: str) -> str:
"""Infer a conventional commit type from description text."""
lower = text.strip().lower()
for pattern, commit_type in VERB_TYPE_MAP:
if re.search(pattern, lower):
return commit_type
return 'feat'
def suggest_scope(text: str) -> str | None:
"""Infer a scope from keywords in the text."""
lower = text.strip().lower()
for pattern, scope in KEYWORD_SCOPES:
if re.search(pattern, lower, re.IGNORECASE):
return scope
return None

95
Tools/ci/pr_comment.py Executable file
View File

@@ -0,0 +1,95 @@
#!/usr/bin/env python3
"""Post, update, or delete a PR comment with deduplication.
Uses hidden HTML markers to find existing comments and avoid duplicates.
Reads comment body from stdin when posting or updating.
Usage:
echo "comment body" | python3 pr_comment.py --marker pr-title --pr 123 --result fail
python3 pr_comment.py --marker pr-title --pr 123 --result pass
Results:
fail - post/update comment with body from stdin
warn - post/update comment with body from stdin
pass - delete existing comment if any
Requires GH_TOKEN and GITHUB_REPOSITORY environment variables.
"""
import argparse
import json
import os
import subprocess
import sys
def gh_api(endpoint: str, method: str = 'GET', body: dict | None = None) -> str:
"""Call the GitHub API via gh cli."""
cmd = ['gh', 'api', endpoint, '-X', method]
if body:
for key, value in body.items():
cmd.extend(['-f', f'{key}={value}'])
result = subprocess.run(cmd, capture_output=True, text=True)
if result.returncode != 0 and method != 'DELETE':
print(f"gh api error: {result.stderr}", file=sys.stderr)
return result.stdout
def find_comment(repo: str, pr: int, marker: str) -> str | None:
"""Find an existing comment by its hidden marker. Returns comment ID or None."""
response = gh_api(f"repos/{repo}/issues/{pr}/comments?per_page=100")
try:
comments = json.loads(response)
except json.JSONDecodeError:
return None
if not isinstance(comments, list):
return None
for comment in comments:
if isinstance(comment, dict) and comment.get('body', '').startswith(marker):
return str(comment['id'])
return None
def main() -> None:
parser = argparse.ArgumentParser(description='Manage PR quality comments')
parser.add_argument('--marker', required=True,
help='Marker name (e.g. pr-title, commit-msgs, pr-body)')
parser.add_argument('--pr', required=True, type=int,
help='Pull request number')
parser.add_argument('--result', required=True, choices=['pass', 'fail', 'warn'],
help='Check result: pass deletes comment, fail/warn posts it')
args = parser.parse_args()
repo = os.environ.get('GITHUB_REPOSITORY', '')
if not repo:
print("GITHUB_REPOSITORY not set", file=sys.stderr)
sys.exit(2)
marker = f"<!-- commit-quality-{args.marker} -->"
existing_id = find_comment(repo, args.pr, marker)
if args.result == 'pass':
if existing_id:
gh_api(f"repos/{repo}/issues/comments/{existing_id}", method='DELETE')
return
# Read comment body from stdin
body_content = sys.stdin.read().strip()
if not body_content:
print("No comment body provided on stdin", file=sys.stderr)
sys.exit(2)
full_body = f"{marker}\n{body_content}"
if existing_id:
gh_api(f"repos/{repo}/issues/comments/{existing_id}", method='PATCH',
body={'body': full_body})
else:
gh_api(f"repos/{repo}/issues/{args.pr}/comments", method='POST',
body={'body': full_body})
if __name__ == '__main__':
main()