Skip to content

fix(git): refuse to commit when nothing is staged - #4761

Open
ConnorMoss02 wants to merge 3 commits into
modelcontextprotocol:mainfrom
ConnorMoss02:fix/git-commit-empty
Open

fix(git): refuse to commit when nothing is staged#4761
ConnorMoss02 wants to merge 3 commits into
modelcontextprotocol:mainfrom
ConnorMoss02:fix/git-commit-empty

Conversation

@ConnorMoss02

@ConnorMoss02 ConnorMoss02 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Closes #4762

git_commit returns Changes committed successfully with hash <sha> even when nothing is staged.

repo.index.commit() writes a tree from the index unconditionally — GitPython has no --allow-empty gate — and the return string cannot be false:

def git_commit(repo: git.Repo, message: str) -> str:
    commit = repo.index.commit(message)
    return f"Changes committed successfully with hash {commit.hexsha}"

An agent edits files, calls git_commit without git_add (or after a git_add that matched nothing), and gets a hash. It reports the work as committed. The working tree is still dirty, HEAD still holds the old content, and the repo now carries an empty commit. Nothing in the response distinguishes that from a real commit.

status before:  M a.txt
returns:        Changes committed successfully with hash efd05335b0ab
empty commit:   True        # parent tree == new tree
status after:   M a.txt
a.txt in HEAD:  v1          # the edit was never committed

git commit itself refuses this: "no changes added to commit".

Fix

Raise when the index matches HEAD, matching git's default. Two cases git does allow are preserved:

  • the first commit on an unborn branch, where there is no HEAD to compare against
  • an empty merge commit, which git permits while MERGE_HEAD exists — a conflict resolved back to HEAD's content leaves the index matching HEAD, and that commit is legitimate

No new parameter: git requires --allow-empty explicitly, so refusing by default matches it.

Tests

Six, covering both directions. Three assert the refusal (unstaged edit, clean tree, untracked-only) and fail with the source change reverted. Three assert it does not over-refuse (staged deletion, unborn branch, empty merge commit) and pass either way.

53 passed in src/git.

repo.index.commit() writes a tree from the index unconditionally, so
git_commit returned "Changes committed successfully with hash ..." even
when the index matched HEAD. A caller that edited files and skipped
git_add got a hash back, reported the work as committed, and left the
working tree dirty with an empty commit on top.

The message could not be false, so nothing downstream could tell a real
commit from an empty one.

Mirrors git commit, which refuses this without --allow-empty: raises when
the index matches HEAD, while still allowing the first commit on an
unborn branch and an empty merge commit when MERGE_HEAD is present.
The test provoked a merge conflict and asserted only that some
GitCommandError was raised, so a merge that failed for an unrelated
reason satisfied it and left no MERGE_HEAD. It passed locally and failed
on CI, where the fixture's missing user identity makes `git merge` refuse
to run at all.

Uses --no-commit --no-ff instead, which sets MERGE_HEAD without depending
on how a git version reports conflicts, sets an explicit identity because
`git merge` shells out to git, and asserts MERGE_HEAD exists so a failure
says what actually went wrong.
They differed only in starting state, and the repo's python tests use no
parametrize, so one test walking clean tree -> untracked -> unstaged edit
covers the same ground in a third of the lines.
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.

git_commit reports success and creates an empty commit when nothing is staged

1 participant