merge queue: embarking main (504fb53) and #903 together

mergify_cli/stack/hooks/prepare-commit-msg

@@ -0,0 +1,87 @@
+#!/bin/sh
+#
+#  Copyright © 2021-2024 Mergify SAS
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+# This hook preserves Change-Id during amend operations where the message
+# is provided via -m or -F flags (which would otherwise lose the Change-Id).
+#
+# Arguments:
+#   $1 - Path to the commit message file
+#   $2 - Source of the commit message: message, template, merge, squash, or commit
+#   $3 - Commit SHA (only for squash or commit source)
+
+if test "$#" -lt 1; then
+    exit 0
+fi
+
+MSG_FILE="$1"
+SOURCE="${2:-}"
+
+# If source is "commit" (from --amend or -c/-C), git already preserves the
+# original message content including Change-Id, so we don't need to do anything.
+# This hook is specifically for the case where -m or -F is used with --amend,
+# which sets source to "message" and loses the original Change-Id.
+
+# Only act if we have a message file
+if test ! -f "$MSG_FILE"; then
+    exit 0
+fi
+
+# Check if HEAD exists (not initial commit)
+if ! git rev-parse --verify HEAD >/dev/null 2>&1; then
+    exit 0
+fi
+
+# Check if the current message already has a Change-Id
+if grep -q "^Change-Id: I[0-9a-f]\{40\}$" "$MSG_FILE"; then
+    exit 0
+fi
+
+# Get Change-Id from HEAD if it exists
+HEAD_CHANGEID=$(git log -1 --format=%B HEAD 2>/dev/null | grep "^Change-Id: I[0-9a-f]\{40\}$" | tail -1)
+if test -z "$HEAD_CHANGEID"; then
+    exit 0
+fi
+
+# Heuristic to detect amend: During --amend, git sets GIT_AUTHOR_DATE to preserve
+# the original author date. This date should exactly match HEAD's author date.
+# For a regular commit, GIT_AUTHOR_DATE is not set by git.
+# We also check that source is "message" (set by git for both -m and -F flags).
+if test "$SOURCE" = "message" && test -n "$GIT_AUTHOR_DATE"; then
+    # Get HEAD's author date in the same format git uses internally (raw format)
+    # The raw format is: seconds-since-epoch timezone (e.g., "1234567890 +0000")
+    HEAD_AUTHOR_DATE_RAW=$(git log -1 --format=%ad --date=raw HEAD 2>/dev/null)
+    if test -n "$HEAD_AUTHOR_DATE_RAW"; then
+        # Extract epoch from GIT_AUTHOR_DATE (handles various formats)
+        # During amend, GIT_AUTHOR_DATE is in format: "@epoch tz" (e.g., "@1234567890 +0000")
+        # Remove the @ prefix if present
+        GIT_AUTHOR_EPOCH=$(echo "$GIT_AUTHOR_DATE" | cut -d' ' -f1 | tr -d '@')
+        HEAD_AUTHOR_EPOCH=$(echo "$HEAD_AUTHOR_DATE_RAW" | cut -d' ' -f1)
+
+        # If the epoch timestamps match, this is likely an amend operation.
+        # Additional check: the author date should be at least 2 seconds in the past.
+        # This prevents false positives when commits happen in quick succession
+        # (e.g., in automated tests or scripts) where timestamps might match by coincidence.
+        if test "$GIT_AUTHOR_EPOCH" = "$HEAD_AUTHOR_EPOCH"; then
+            CURRENT_EPOCH=$(date +%s)
+            AGE=$((CURRENT_EPOCH - GIT_AUTHOR_EPOCH))
+            if test "$AGE" -ge 2; then
+                # This looks like an amend with -m flag - preserve the Change-Id
+                echo "" >> "$MSG_FILE"
+                echo "$HEAD_CHANGEID" >> "$MSG_FILE"
+            fi
+        fi
+    fi
+fi

mergify_cli/stack/setup.py

@@ -26,12 +26,11 @@
 from mergify_cli import utils
 
 
-async def stack_setup() -> None:
-    hooks_dir = pathlib.Path(await utils.git("rev-parse", "--git-path", "hooks"))
-    installed_hook_file = hooks_dir / "commit-msg"
+async def _install_hook(hooks_dir: pathlib.Path, hook_name: str) -> None:
+    installed_hook_file = hooks_dir / hook_name
 
     new_hook_file = str(
-        importlib.resources.files(__package__).joinpath("hooks/commit-msg"),
+        importlib.resources.files(__package__).joinpath(f"hooks/{hook_name}"),
     )
 
     if installed_hook_file.exists():
@@ -40,7 +39,7 @@ async def stack_setup() -> None:
         async with aiofiles.open(new_hook_file) as f:
             data_new = await f.read()
         if data_installed == data_new:
-            console.log("Git commit-msg hook is up to date")
+            console.log(f"Git {hook_name} hook is up to date")
         else:
             console.print(
                 f"error: {installed_hook_file} differ from mergify_cli hook",
@@ -49,6 +48,12 @@ async def stack_setup() -> None:
             sys.exit(1)
 
     else:
-        console.log("Installation of git commit-msg hook")
+        console.log(f"Installation of git {hook_name} hook")
         shutil.copy(new_hook_file, installed_hook_file)
         installed_hook_file.chmod(0o755)
+
+
+async def stack_setup() -> None:
+    hooks_dir = pathlib.Path(await utils.git("rev-parse", "--git-path", "hooks"))
+    await _install_hook(hooks_dir, "commit-msg")
+    await _install_hook(hooks_dir, "prepare-commit-msg")

mergify_cli/tests/stack/test_setup.py

@@ -1,15 +1,19 @@
 from __future__ import annotations
 
+import re
+import shutil
+import subprocess
 import typing
 
+import pytest
+
 from mergify_cli.stack import setup
+from mergify_cli.stack.changes import CHANGEID_RE
 
 
 if typing.TYPE_CHECKING:
     import pathlib
 
-    import pytest
-
     from mergify_cli.tests import utils as test_utils
 
 
@@ -22,5 +26,186 @@ async def test_setup(
     git_mock.mock("rev-parse", "--git-path", "hooks", output=str(hooks_dir))
     await setup.stack_setup()
 
-    hook = hooks_dir / "commit-msg"
-    assert hook.exists()
+    commit_msg_hook = hooks_dir / "commit-msg"
+    assert commit_msg_hook.exists()
+
+    prepare_commit_msg_hook = hooks_dir / "prepare-commit-msg"
+    assert prepare_commit_msg_hook.exists()
+
+
+@pytest.fixture
+def git_repo_with_hooks(tmp_path: pathlib.Path) -> pathlib.Path:
+    """Create a real git repo with the hooks installed."""
+    import importlib.resources
+
+    subprocess.run(
+        ["git", "init", "--initial-branch=main"],
+        check=True,
+        cwd=tmp_path,
+    )
+    subprocess.run(
+        ["git", "config", "user.email", "test@example.com"],
+        check=True,
+        cwd=tmp_path,
+    )
+    subprocess.run(
+        ["git", "config", "user.name", "Test User"],
+        check=True,
+        cwd=tmp_path,
+    )
+
+    # Install hooks
+    hooks_dir = tmp_path / ".git" / "hooks"
+    for hook_name in ("commit-msg", "prepare-commit-msg"):
+        hook_source = str(
+            importlib.resources.files("mergify_cli.stack").joinpath(
+                f"hooks/{hook_name}",
+            ),
+        )
+        hook_dest = hooks_dir / hook_name
+        shutil.copy(hook_source, hook_dest)
+        hook_dest.chmod(0o755)
+
+    return tmp_path
+
+
+def get_commit_message(repo_path: pathlib.Path) -> str:
+    """Get the current HEAD commit message."""
+    return subprocess.check_output(
+        ["git", "log", "-1", "--format=%B"],
+        text=True,
+        cwd=repo_path,
+    )
+
+
+def get_change_id(message: str) -> str | None:
+    """Extract Change-Id from a commit message."""
+    match = CHANGEID_RE.search(message)
+    return match.group(1) if match else None
+
+
+def test_commit_gets_change_id(git_repo_with_hooks: pathlib.Path) -> None:
+    """Test that a new commit gets a Change-Id from the commit-msg hook."""
+    # Create a file and commit
+    (git_repo_with_hooks / "file.txt").write_text("content")
+    subprocess.run(["git", "add", "file.txt"], check=True, cwd=git_repo_with_hooks)
+    subprocess.run(
+        ["git", "commit", "-m", "Initial commit"],
+        check=True,
+        cwd=git_repo_with_hooks,
+    )
+
+    message = get_commit_message(git_repo_with_hooks)
+    change_id = get_change_id(message)
+
+    assert change_id is not None, f"Expected Change-Id in message:\n{message}"
+    assert re.match(r"^I[0-9a-f]{40}$", change_id)
+
+
+def test_amend_with_m_flag_preserves_change_id(
+    git_repo_with_hooks: pathlib.Path,
+) -> None:
+    """Test that amending a commit with -m flag preserves the Change-Id.
+
+    This is the specific scenario where tools like Claude Code amend commits
+    by passing the message via -m flag, which would otherwise lose the Change-Id.
+    """
+    import time
+
+    # Create initial commit with Change-Id
+    (git_repo_with_hooks / "file.txt").write_text("content")
+    subprocess.run(["git", "add", "file.txt"], check=True, cwd=git_repo_with_hooks)
+    subprocess.run(
+        ["git", "commit", "-m", "Initial commit"],
+        check=True,
+        cwd=git_repo_with_hooks,
+    )
+
+    original_message = get_commit_message(git_repo_with_hooks)
+    original_change_id = get_change_id(original_message)
+    assert original_change_id is not None
+
+    # Wait a bit so the hook can detect this is an amend (author date will be old)
+    time.sleep(2)
+
+    # Amend with -m flag (this is what Claude Code does)
+    subprocess.run(
+        ["git", "commit", "--amend", "-m", "Amended commit"],
+        check=True,
+        cwd=git_repo_with_hooks,
+    )
+
+    amended_message = get_commit_message(git_repo_with_hooks)
+    amended_change_id = get_change_id(amended_message)
+
+    assert amended_change_id is not None, (
+        f"Expected Change-Id in amended message:\n{amended_message}"
+    )
+    assert amended_change_id == original_change_id, (
+        f"Change-Id should be preserved during amend.\n"
+        f"Original: {original_change_id}\n"
+        f"After amend: {amended_change_id}"
+    )
+
+
+def test_amend_without_m_flag_preserves_change_id(
+    git_repo_with_hooks: pathlib.Path,
+) -> None:
+    """Test that amending without -m flag also preserves the Change-Id."""
+    # Create initial commit with Change-Id
+    (git_repo_with_hooks / "file.txt").write_text("content")
+    subprocess.run(["git", "add", "file.txt"], check=True, cwd=git_repo_with_hooks)
+    subprocess.run(
+        ["git", "commit", "-m", "Initial commit"],
+        check=True,
+        cwd=git_repo_with_hooks,
+    )
+
+    original_message = get_commit_message(git_repo_with_hooks)
+    original_change_id = get_change_id(original_message)
+    assert original_change_id is not None
+
+    # Amend without changing message
+    subprocess.run(
+        ["git", "commit", "--amend", "--no-edit"],
+        check=True,
+        cwd=git_repo_with_hooks,
+    )
+
+    amended_message = get_commit_message(git_repo_with_hooks)
+    amended_change_id = get_change_id(amended_message)
+
+    assert amended_change_id is not None
+    assert amended_change_id == original_change_id
+
+
+def test_new_commit_after_amend_gets_new_change_id(
+    git_repo_with_hooks: pathlib.Path,
+) -> None:
+    """Test that a new commit (not an amend) gets a new Change-Id."""
+    # Create first commit
+    (git_repo_with_hooks / "file1.txt").write_text("content1")
+    subprocess.run(["git", "add", "file1.txt"], check=True, cwd=git_repo_with_hooks)
+    subprocess.run(
+        ["git", "commit", "-m", "First commit"],
+        check=True,
+        cwd=git_repo_with_hooks,
+    )
+
+    first_change_id = get_change_id(get_commit_message(git_repo_with_hooks))
+    assert first_change_id is not None
+
+    # Create second commit (should get a different Change-Id)
+    (git_repo_with_hooks / "file2.txt").write_text("content2")
+    subprocess.run(["git", "add", "file2.txt"], check=True, cwd=git_repo_with_hooks)
+    subprocess.run(
+        ["git", "commit", "-m", "Second commit"],
+        check=True,
+        cwd=git_repo_with_hooks,
+    )
+
+    second_change_id = get_change_id(get_commit_message(git_repo_with_hooks))
+    assert second_change_id is not None
+    assert second_change_id != first_change_id, (
+        "Each commit should have a unique Change-Id"
+    )