fix: strip [skip ci] tokens and prefer newest commit in squash titles

This commit is contained in:
Emil Simeonov
2026-09-28 16:55:10 +02:00
parent ae0be43368
commit 213831f77b
3 changed files with 100 additions and 8 deletions
+49
View File
@@ -0,0 +1,49 @@
# DEVX-179: auto-merge must never select `[skip ci]` squash titles
## Problem
PR #351 squash-merged as `DEVX-178: chore: update badge URLs to commit
4422c70b [skip ci]`. `extract_conventional_msg` picked a commit subject
carrying `[skip ci]` — a master badge commit surfaced in the PR's commit
list after force-push/amend rewrote branch history. The resulting merge
commit suppressed the post-merge push run: no release, no publish, no
wiki sync, no Vikunja close.
PR #352 shows the secondary defect: two `ci:` commits on the branch and
the older one won the tie, so the squash title described a throwaway
retrigger commit instead of the real change.
## Approach
REQ-1: Subjects containing `[skip ci]`, `[ci skip]`, or `[skip actions]`
(case-insensitive) are ineligible merge titles. They are excluded before
priority scoring, so a badge/chore/`[skip ci]` commit can never suppress
the post-merge pipeline again.
REQ-2: Equal-priority ties resolve to the newest commit in the returned
list (Gitea returns PR commits newest-first; iterate in returned order
and keep the first best candidate).
## Test Plan
- `test_auto_merge.py`: commits `[ci retrigger, badge-chore-skipci]` →
the `ci` subject wins; commits `[older-ci, newer-ci]` → newest wins;
all commits `[skip ci]` → falls back to newest non-skipped subject.
- `make pytest-cov` (100% gate), `make lint-all`.
## Deploy Plan
Merge as `fix:` → post-merge cuts a release that also ships DEVX-178's
PLAYBOOK_ROLE_MAP entry.
## Rollback Plan
Revert the squash commit; extract behavior returns to the previous
(unhardened) selection.
## Acceptance Criteria
- [x] REQ-1: `[skip ci]`/`[ci skip]`/`[skip actions]` subjects are never
selected.
- [x] REQ-2: Equal-priority ties pick the newest commit.
- [x] Unit tests cover both; 100% coverage maintained.
+17 -8
View File
@@ -184,22 +184,31 @@ def validate_pr_title_matches_vikunja(pr_title: str, task_id: str) -> None:
)
# Subjects carrying these tokens suppress post-merge CI entirely — strip them
# before a commit message can become the squash title (see DEVX-179).
_SKIP_CI_RE = re.compile(r"\s*\[(?:ci skip|skip ci|skip actions|actions skip|skip)\]", re.IGNORECASE)
def extract_conventional_msg(commits: list[dict[str, Any]]) -> str:
"""Extract the conventional commit message from PR commits.
Picks the highest-priority conventional commit message from the PR.
Priority: feat > fix > refactor > docs > chore > other.
Falls back to the newest commit message if none match.
Priority: feat > fix > refactor > docs > chore > other. The Gitea
``/pulls/{n}/commits`` endpoint returns commits newest-first, so the
first best-scoring subject wins equal-priority ties.
``[skip ci]``-style tokens are stripped from every candidate so the
merge title can never suppress the post-merge release pipeline.
"""
priority = {"feat": 5, "fix": 4, "refactor": 3, "docs": 2, "chore": 1, "ci": 1, "style": 1, "test": 1}
best_msg = ""
best_score = 0
for commit in reversed(commits):
for commit in commits:
commit_info = commit.get("commit", {})
message = str(commit_info.get("message", "") if isinstance(commit_info, dict) else "").split("\n")[0]
# Strip any leading task ID prefix (e.g. "OBL-INFRA-364: fix: ...") so
# conventional commit matching works on the remainder.
stripped = _TASK_ID_PREFIX_RE.sub("", message)
# conventional commit matching works on the remainder; drop CI-skip
# tokens so they never reach the merge title.
stripped = _SKIP_CI_RE.sub("", _TASK_ID_PREFIX_RE.sub("", message)).strip()
m = CONVENTIONAL_RE.match(stripped)
if m:
prefix = m.group(1).split("(")[0].strip() # e.g. "feat" from "feat(scope)"
@@ -209,11 +218,11 @@ def extract_conventional_msg(commits: list[dict[str, Any]]) -> str:
best_msg = stripped
if best_msg:
return best_msg
# Fallback: use the newest commit's first line (strip task ID prefix if present)
# Fallback: the newest commit's first line (newest-first API order).
if commits:
commit_info = commits[-1].get("commit", {})
commit_info = commits[0].get("commit", {})
raw = str(commit_info.get("message", "") if isinstance(commit_info, dict) else "").split("\n")[0]
return _TASK_ID_PREFIX_RE.sub("", raw)
return _SKIP_CI_RE.sub("", _TASK_ID_PREFIX_RE.sub("", raw)).strip()
return ""
+34
View File
@@ -232,6 +232,40 @@ class TestExtractConventionalMsg:
]
assert extract_conventional_msg(commits) == "random message"
def test_skip_ci_subjects_are_ineligible(self) -> None:
"""[skip ci] commits must never become the merge title (REQ-1)."""
commits = [
{"commit": {"message": "ci: retrigger validation"}},
{"commit": {"message": "chore: update badge URLs to commit abc123 [skip ci]"}},
]
assert extract_conventional_msg(commits) == "ci: retrigger validation"
def test_skip_ci_variants_excluded(self) -> None:
"""All skip-token spellings are ineligible."""
for token in ("[skip ci]", "[ci skip]", "[skip actions]", "[actions skip]", "[SKIP CI]"):
commits = [
{"commit": {"message": "feat: real change"}},
{"commit": {"message": f"chore: noise {token}"}},
]
assert extract_conventional_msg(commits) == "feat: real change"
def test_tie_prefers_newest_commit(self) -> None:
"""Equal-priority ties resolve to the newest commit (REQ-2); the
API returns commits newest-first."""
commits = [
{"commit": {"message": "ci: add workflow_dispatch trigger"}},
{"commit": {"message": "ci: retrigger validation"}},
]
assert extract_conventional_msg(commits) == "ci: add workflow_dispatch trigger"
def test_all_skip_ci_fallback_strips_token(self) -> None:
"""When every commit carries [skip ci], the token is stripped so
the merge still triggers post-merge."""
commits = [
{"commit": {"message": "chore: noise [skip ci]"}},
]
assert extract_conventional_msg(commits) == "chore: noise"
# -- run_cmd --