From bd67660955bdd0f70027a92382c03ae6cebbdcd3 Mon Sep 17 00:00:00 2001 From: Sagar Gupta Date: Sat, 4 Jul 2026 20:32:51 +0530 Subject: [PATCH] fix(git): normalize git_log schema across filtered and unfiltered branches git_log emitted two different string shapes depending on whether start_timestamp or end_timestamp was passed: Unfiltered branch used commit.hexsha!r / commit.author!r, producing repr()-quoted values with leading single quotes and Actor angle brackets, and included the full commit body via commit.message. Filtered branch used git log --format=%H%n%an%n%ad%n%s%n, producing raw unquoted values, and dropped the commit body entirely (%s is subject only). Downstream parsers that split on "\n" and ":" have to special-case which branch produced the entry. Reported in #4469. Collapse both branches onto repo.iter_commits() with since/until kwargs, so there is exactly one code path. Drop the repr() formatting so both branches emit raw values (bare commit hash, author name/email, full message). Add a regression test asserting the two branches produce the same key order and neither emits repr artefacts. Closes #4469 --- src/git/src/mcp_server_git/server.py | 60 ++++++++++------------------ src/git/tests/test_server.py | 22 ++++++++++ 2 files changed, 43 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..26470dbced 100644 --- a/src/git/src/mcp_server_git/server.py +++ b/src/git/src/mcp_server_git/server.py @@ -157,45 +157,27 @@ 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 '-'") + + rev_kwargs: dict[str, str] = {} + if start_timestamp: + rev_kwargs["since"] = start_timestamp + if end_timestamp: + rev_kwargs["until"] = end_timestamp + + log = [] + for commit in repo.iter_commits(max_count=max_count, **rev_kwargs): + log.append( + f"Commit: {commit.hexsha}\n" + f"Author: {commit.author}\n" + f"Date: {commit.authored_datetime}\n" + f"Message: {commit.message}\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..0533ec27f4 100644 --- a/src/git/tests/test_server.py +++ b/src/git/tests/test_server.py @@ -234,6 +234,28 @@ def test_git_log_default(test_repository): assert len(result) >= 1 assert "initial commit" in result[0] + +def test_git_log_schema_matches_across_filter_branches(test_repository): + """git_log emits the same string schema whether or not timestamp filters are supplied.""" + for i in range(2): + file_path = Path(test_repository.working_dir) / f"schema_test_{i}.txt" + file_path.write_text(f"content {i}") + test_repository.index.add([f"schema_test_{i}.txt"]) + test_repository.index.commit(f"schema commit {i}") + + unfiltered = git_log(test_repository, max_count=1) + filtered = git_log(test_repository, max_count=1, start_timestamp="1970-01-01") + + assert len(unfiltered) == 1 + assert len(filtered) == 1 + # Same keys, same order — parsers that split on "\n" and ":" must not care which branch produced the entry. + assert [line.split(":", 1)[0] for line in unfiltered[0].splitlines()] == \ + [line.split(":", 1)[0] for line in filtered[0].splitlines()] + # And no repr() artefacts (leading quotes, angle brackets) that used to appear only in the unfiltered branch. + for entry in (unfiltered[0], filtered[0]): + commit_line = entry.splitlines()[0] + assert not commit_line.startswith("Commit: '"), f"Commit hash should not be repr-quoted: {commit_line!r}" + def test_git_create_branch(test_repository): result = git_create_branch(test_repository, "new-feature-branch")