diff --git a/.agents/skills/create-commit/SKILL.md b/.agents/skills/create-commit/SKILL.md index f0a612c17e..bf0667ca65 100644 --- a/.agents/skills/create-commit/SKILL.md +++ b/.agents/skills/create-commit/SKILL.md @@ -62,5 +62,6 @@ subject/body limits, and the `AI-assisted-by` trailer. Follow it exactly. - Do not pass `--author`. Author identity comes from the local git config. - Do not amend a commit you did not create in this session, unless explicitly asked. - Do not bypass pre-commit hooks (`--no-verify`) unless explicitly asked. +- Never add a `Co-authored-by:` trailer. It is forbidden and fails `./scripts/check-commit`. - Do not add untracked files that were not created in this session. - Do not skip the `scripts/check-commit` verification step (Iron Rule 4). diff --git a/.github/workflows/commit-checker.yml b/.github/workflows/commit-checker.yml index 29d9c79d60..0de7fe8975 100644 --- a/.github/workflows/commit-checker.yml +++ b/.github/workflows/commit-checker.yml @@ -47,4 +47,45 @@ jobs: # pattern: '^.+(Resolves|Fixes): \#[0-9]+$' # error: 'You need at least one "Resolves|Fixes: #" line.' + check-commit-guidelines: + if: ${{ !github.event.pull_request.draft }} + name: Check Commit Guidelines + runs-on: ubuntu-latest + steps: + # On PR events check out the base ref, so the script below is + # always the trusted version: PR commits are only read, never + # executed. On push events this resolves to the pushed sha. + - name: Checkout trusted code + uses: actions/checkout@v6 + with: + fetch-depth: 0 + ref: ${{ github.event.pull_request.base.sha || github.sha }} + + - name: Fetch PR commits + if: ${{ github.event_name == 'pull_request' || github.event_name == 'pull_request_target' }} + run: git fetch --no-tags origin "pull/${{ github.event.pull_request.number }}/head:pr-head" + + - name: Run scripts/check-commit on every new commit + run: | + set -euo pipefail + if [ "${{ github.event_name }}" = "push" ]; then + if [ "${{ github.event.before }}" = "0000000000000000000000000000000000000000" ]; then + RANGE="${{ github.sha }}" + else + RANGE="${{ github.event.before }}..${{ github.sha }}" + fi + else + RANGE="${{ github.event.pull_request.base.sha }}..pr-head" + fi + COMMITS="$(git rev-list --reverse "$RANGE")" + if [ -z "$COMMITS" ]; then + echo "No new commits to check." + exit 0 + fi + FAILED=0 + for COMMIT in $COMMITS; do + ./scripts/check-commit -c "$COMMIT" || FAILED=1 + done + exit "$FAILED" + diff --git a/.serena/memories/workflow/creating-commits.md b/.serena/memories/workflow/creating-commits.md index d632c9a471..a60234819f 100644 --- a/.serena/memories/workflow/creating-commits.md +++ b/.serena/memories/workflow/creating-commits.md @@ -37,8 +37,10 @@ that breaks them is wrong, even if the code is right. - For another commit: `./scripts/check-commit -c ` - **NEVER** hand-wave the body as "one long line". If a line exceeds 76, break it at a space. +- **NEVER add a `Co-authored-by:` trailer.** It is forbidden; `scripts/check-commit` fails the commit when any line matches it (case-insensitive). +- CI runs `scripts/check-commit` on every new commit (`mem:workflow/creating-commits` is enforced by `.github/workflows/commit-checker.yml`), so a forbidden trailer fails the build. - Exceptions inside the body (do not wrap these): `Signed-off-by:`, - `Co-authored-by:`, `AI-assisted-by:` trailers, and lines carrying a URL. + `AI-assisted-by:` trailers, and lines carrying a URL. **AI-assisted-by trailer rules:** - Use only the model name, e.g. `mimo-v2.5`, `deepseek-v4-flash` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9da08257f7..2d665b0715 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -223,8 +223,12 @@ Commit messages must follow this format: - Keep the subject to **70 characters** or fewer - **Wrap body lines at 76 characters or fewer** (trailers and URLs excepted) - Separate the subject from the body with a **blank line** +- Never add a `Co-authored-by` trailer — it is forbidden and fails + `./scripts/check-commit` You can check a commit against these rules with `./scripts/check-commit`. +CI runs the same script on every new commit in +`.github/workflows/commit-checker.yml`, so violations fail the build. ### Examples diff --git a/scripts/check-commit b/scripts/check-commit index 3f2792552e..d0e9f59899 100755 --- a/scripts/check-commit +++ b/scripts/check-commit @@ -47,10 +47,12 @@ MERGE_PATTERN = re.compile(r"^(Merge|Revert|Reapply).+[^.]$") MAX_BODY_LINE = 76 TRAILER_PATTERN = re.compile( - r"^(Signed-off-by|Co-authored-by|Co-developed-by|Reviewed-by|" + r"^(Signed-off-by|Co-developed-by|Reviewed-by|" r"Acked-by|Tested-by|Reported-by|Suggested-by|AI-assisted-by):" ) +FORBIDDEN_TRAILER_PATTERN = re.compile(r"^\s*Co-authored-by\s*:", re.IGNORECASE) + URL_PATTERN = re.compile(r"https?://\S+") # ═══════════════════════════════════════════════════════════════════════════════ @@ -209,6 +211,25 @@ def check_signed_off_by(message): return True, None +def check_no_co_authored_by(message): + """Check that the forbidden Co-authored-by trailer is absent.""" + offenders = [ + (line_number, line) + for line_number, line in enumerate(message.split("\n"), start=1) + if FORBIDDEN_TRAILER_PATTERN.match(line) + ] + if not offenders: + return True, None + details = "\n".join( + f" line {line_number}: {line!r}" + for line_number, line in offenders + ) + return False, ( + "The 'Co-authored-by:' trailer is forbidden and must not appear " + "in the commit message. Offending line(s):\n" + details + ) + + # ═══════════════════════════════════════════════════════════════════════════════ def main(): @@ -234,6 +255,7 @@ def main(): ("Subject capitalized", check_subject_capitalized), ("Blank line after subject", check_body_blank_line), ("Body wrapped at 76 chars", check_body_line_length), + ("No Co-authored-by trailer", check_no_co_authored_by), ] all_ok = True diff --git a/scripts/test_check_commit.py b/scripts/test_check_commit.py index 4e4940429e..a3a7a1c2b9 100644 --- a/scripts/test_check_commit.py +++ b/scripts/test_check_commit.py @@ -106,6 +106,35 @@ class BodyLineLengthTests(unittest.TestCase): self.assertIn("line 4", error) +class ForbiddenTrailerTests(unittest.TestCase): + def assert_ok(self, message): + ok, error = check_commit.check_no_co_authored_by(message) + self.assertTrue(ok, error) + self.assertIsNone(error) + + def assert_fail(self, message): + ok, error = check_commit.check_no_co_authored_by(message) + self.assertFalse(ok) + self.assertIsNotNone(error) + return error + + def test_body_without_trailer_passes(self): + self.assert_ok(":bug: Fix crash\n\nBody.\n\nSigned-off-by: Someone \n") + + def test_co_authored_by_fails(self): + error = self.assert_fail(":bug: Fix crash\n\nBody.\n\nCo-authored-by: Someone \n") + self.assertIn("Co-authored-by", error) + + def test_co_authored_by_case_insensitive_fails(self): + self.assert_fail(":bug: Fix crash\n\nBody.\n\nco-authored-by: Someone \n") + + def test_co_authored_by_in_body_fails(self): + self.assert_fail(":bug: Fix crash\n\nCo-authored-by: Someone \nBody.\n") + + def test_similar_trailer_passes(self): + self.assert_ok(":bug: Fix crash\n\nBody.\n\nCo-developed-by: Someone \n") + + class SubjectRulesRegressionTests(unittest.TestCase): """Guard the pre-existing validators against accidental breakage."""