From 72733dc9a3103b7b87507de42276287ccdbff300 Mon Sep 17 00:00:00 2001 From: Sachin Date: Fri, 21 Aug 2026 15:55:25 -0700 Subject: [PATCH] fix(git): normalize git_log output format between filtered and unfiltered calls The filtered branch (with start/end timestamps) used raw git log --format with %s (subject only), while the unfiltered branch used Python repr (!r) formatting on commit fields. This produced inconsistent output schemas: - Filtered: raw hash, author name only, subject only - Unfiltered: quoted hash, git.Actor repr, quoted message with escaped newlines Now both paths use repo.iter_commits() with after/before kwargs, producing a unified clean format: raw hash, "Name " author, full message body. Fixes #4469 --- src/git/src/mcp_server_git/server.py | 62 +++++++++++----------------- src/git/tests/test_server.py | 35 ++++++++++++++++ 2 files changed, 58 insertions(+), 39 deletions(-) diff --git a/src/git/src/mcp_server_git/server.py b/src/git/src/mcp_server_git/server.py index 84188d8fd7..ccf3551a21 100644 --- a/src/git/src/mcp_server_git/server.py +++ b/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 diff --git a/src/git/tests/test_server.py b/src/git/tests/test_server.py index 893195d414..a94aac643c 100644 --- a/src/git/tests/test_server.py +++ b/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")