diff --git a/docs/specs/DEVX-179.md b/docs/specs/DEVX-179.md new file mode 100644 index 0000000..230c88d --- /dev/null +++ b/docs/specs/DEVX-179.md @@ -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. diff --git a/src/devx/ci/auto_merge.py b/src/devx/ci/auto_merge.py index 636e1d6..8175c0c 100644 --- a/src/devx/ci/auto_merge.py +++ b/src/devx/ci/auto_merge.py @@ -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 "" diff --git a/tests/unit/test_auto_merge.py b/tests/unit/test_auto_merge.py index 07270c4..eead839 100644 --- a/tests/unit/test_auto_merge.py +++ b/tests/unit/test_auto_merge.py @@ -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 --