Skip to content

fix(git): correct field misalignment in date-filtered git_log#4524

Open
itguruhaseeb wants to merge 1 commit into
modelcontextprotocol:mainfrom
itguruhaseeb:fix/git-log-timestamp-field-shift
Open

fix(git): correct field misalignment in date-filtered git_log#4524
itguruhaseeb wants to merge 1 commit into
modelcontextprotocol:mainfrom
itguruhaseeb:fix/git-log-timestamp-field-shift

Conversation

@itguruhaseeb

Copy link
Copy Markdown

Problem

git_log's date-filtered branch parses git log output into fixed groups of 4 lines (hash, author, date, subject), but the pretty-format string ends with a trailing %n:

args.extend(['--format=%H%n%an%n%ad%n%s%n'])

That trailing %n makes git emit a blank line after every commit, so each record occupies 5 slots after .split('\n') while the loop advances by 4. Only the first commit lands on a correct boundary. Every commit after it is shifted by one field, and a commit can be dropped or a spurious blank entry emitted.

Concretely, for a repo with an "initial commit" plus a few filtered commits, the second entry comes back as:

Commit: <author name>
Author: <date string>
Date:   initial commit
Message:

The unfiltered branch (iter_commits, used when no start_timestamp/end_timestamp is given) is correct and unaffected, which is why this went unnoticed.

Fix

Drop the trailing %n so each record is exactly 4 lines and the group-of-4 parser stays aligned:

args.extend(['--format=%H%n%an%n%ad%n%s'])

%s is the commit subject (first line only), so every field is guaranteed single-line and the 4-line grouping is safe.

Test

Adds test_git_log_with_timestamp_filter, which filters by start_timestamp and asserts every commit's fields map correctly across multiple commits. It fails on the old format (returns 5 entries with shifted fields) and passes with the fix. Full git-server suite: 44 passed.

The date-filtered branch of git_log used the pretty-format
'%H%n%an%n%ad%n%s%n'. The trailing %n makes git emit a blank line after
every commit, so each record occupies 5 slots after split('\n') while the
parser advances in groups of 4. Only the first commit lands on a correct
boundary; every subsequent commit is shifted by one field (the hash is
labeled Author, the date labeled Message) and a commit can be dropped or a
spurious blank entry emitted.

Drop the trailing %n so each record is exactly 4 lines and the group-of-4
parser stays aligned. The unfiltered branch (iter_commits) was already
correct and is unchanged.

Adds a regression test that filters by start_timestamp and asserts every
commit's fields map correctly across multiple commits.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant