Unify git_log output format between filtered and unfiltered paths

src/git/src/mcp_server_git/server.py

@@ -157,45 +157,29 @@ def git_reset(repo: git.Repo) -> str:
     return "All staged changes reset"
 
 def git_log(repo: git.Repo, max_count: int = 10, start_timestamp: Optional[str] = None, end_timestamp: Optional[str] = None) -> list[str]:
-    if start_timestamp or end_timestamp:
-        # Defense in depth: reject timestamps starting with '-' to prevent flag injection
-        if start_timestamp and start_timestamp.startswith("-"):
-            raise ValueError(f"Invalid start_timestamp: '{start_timestamp}' - cannot start with '-'")
-        if end_timestamp and end_timestamp.startswith("-"):
-            raise ValueError(f"Invalid end_timestamp: '{end_timestamp}' - cannot start with '-'")
-        # Use git log command with date filtering
-        args = []
-        if start_timestamp:
-            args.extend(['--since', start_timestamp])
-        if end_timestamp:
-            args.extend(['--until', end_timestamp])
-        args.extend(['--format=%H%n%an%n%ad%n%s%n'])
-
-        log_output = repo.git.log(*args).split('\n')
-
-        log = []
-        # Process commits in groups of 4 (hash, author, date, message)
-        for i in range(0, len(log_output), 4):
-            if i + 3 < len(log_output) and len(log) < max_count:
-                log.append(
-                    f"Commit: {log_output[i]}\n"
-                    f"Author: {log_output[i+1]}\n"
-                    f"Date: {log_output[i+2]}\n"
-                    f"Message: {log_output[i+3]}\n"
-                )
-        return log
-    else:
-        # Use existing logic for simple log without date filtering
-        commits = list(repo.iter_commits(max_count=max_count))
-        log = []
-        for commit in commits:
-            log.append(
-                f"Commit: {commit.hexsha!r}\n"
-                f"Author: {commit.author!r}\n"
-                f"Date: {commit.authored_datetime}\n"
-                f"Message: {commit.message!r}\n"
-            )
-        return log
+    # Defense in depth: reject timestamps starting with '-' to prevent flag injection
+    if start_timestamp and start_timestamp.startswith("-"):
+        raise ValueError(f"Invalid start_timestamp: '{start_timestamp}' - cannot start with '-'")
+    if end_timestamp and end_timestamp.startswith("-"):
+        raise ValueError(f"Invalid end_timestamp: '{end_timestamp}' - cannot start with '-'")
+
+    # Build kwargs for iter_commits - unified path for both filtered and unfiltered
+    kwargs: dict = {"max_count": max_count}
+    if start_timestamp:
+        kwargs["after"] = start_timestamp
+    if end_timestamp:
+        kwargs["before"] = end_timestamp
+
+    commits = list(repo.iter_commits(**kwargs))
+    log = []
+    for commit in commits:
+        log.append(
+            f"Commit: {commit.hexsha}\n"
+            f"Author: {commit.author.name} <{commit.author.email}>\n"
+            f"Date: {commit.authored_datetime}\n"
+            f"Message: {commit.message.strip()}\n"
+        )
+    return log
 
 def git_create_branch(repo: git.Repo, branch_name: str, base_branch: str | None = None) -> str:
     # Defense in depth: reject names starting with '-' to prevent flag injection

src/git/tests/test_server.py

@@ -234,6 +234,41 @@ def test_git_log_default(test_repository):
     assert len(result) >= 1
     assert "initial commit" in result[0]
 
+def test_git_log_consistent_format(test_repository):
+    """git_log output format should be identical between filtered and unfiltered calls."""
+    # Create a commit so we have something to compare
+    file_path = Path(test_repository.working_dir) / "format_test.txt"
+    file_path.write_text("format test content")
+    test_repository.index.add(["format_test.txt"])
+    test_repository.index.commit("format consistency test")
+
+    # Get unfiltered result
+    unfiltered = git_log(test_repository, max_count=1)
+    # Get filtered result (broad enough to include the commit)
+    filtered = git_log(test_repository, max_count=1, start_timestamp="1 year ago")
+
+    assert len(unfiltered) == 1
+    assert len(filtered) == 1
+
+    # Both should have the same format: no repr quotes, clean author, full message
+    for entry in [unfiltered[0], filtered[0]]:
+        # Should NOT contain repr-style quotes around the hash
+        assert "Commit: '" not in entry
+        # Should NOT contain git.Actor repr
+        assert "git.Actor" not in entry
+        # Should have clean Author format with angle brackets
+        assert "<" in entry and ">" in entry
+        # Should contain all four fields
+        assert "Commit:" in entry
+        assert "Author:" in entry
+        assert "Date:" in entry
+        assert "Message:" in entry
+
+    # The commit hash should be the same in both
+    unfiltered_hash = unfiltered[0].split("\n")[0].split("Commit: ")[1]
+    filtered_hash = filtered[0].split("\n")[0].split("Commit: ")[1]
+    assert unfiltered_hash == filtered_hash
+
 def test_git_create_branch(test_repository):
     result = git_create_branch(test_repository, "new-feature-branch")