Skip to content

Commit cc46adb

Browse files
committed
fix(git): refuse to commit when nothing is staged
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.
1 parent d73f99e commit cc46adb

2 files changed

Lines changed: 96 additions & 0 deletions

File tree

src/git/src/mcp_server_git/server.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,28 @@ def git_diff(repo: git.Repo, target: str, context_lines: int = DEFAULT_CONTEXT_L
125125
repo.rev_parse(target) # Validates target is a real git ref, throws BadName if not
126126
return repo.git.diff(f"--unified={context_lines}", target)
127127

128+
def _has_staged_changes(repo: git.Repo) -> bool:
129+
"""Whether the index holds anything git would record as a commit.
130+
131+
Mirrors `git commit`, which refuses to create an empty commit unless
132+
--allow-empty is given, but permits one while a merge is in progress.
133+
"""
134+
if (Path(repo.git_dir) / "MERGE_HEAD").exists():
135+
return True
136+
if not repo.head.is_valid():
137+
# Unborn branch: the first commit, so anything in the index counts.
138+
return bool(repo.index.entries)
139+
return bool(repo.index.diff(repo.head.commit))
140+
128141
def git_commit(repo: git.Repo, message: str) -> str:
142+
# repo.index.commit() writes a tree unconditionally, so without this check
143+
# a caller that forgot to stage gets a hash back for an empty commit and no
144+
# way to tell it apart from a real one.
145+
if not _has_staged_changes(repo):
146+
raise ValueError(
147+
"No changes staged for commit. Use git_add to stage changes first; "
148+
"git_status shows what is currently staged."
149+
)
129150
commit = repo.index.commit(message)
130151
return f"Changes committed successfully with hash {commit.hexsha}"
131152

src/git/tests/test_server.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,81 @@ def test_git_commit(test_repository):
198198
latest_commit = test_repository.head.commit
199199
assert latest_commit.message.strip() == "test commit message"
200200

201+
def test_git_commit_refuses_when_nothing_is_staged(test_repository):
202+
"""repo.index.commit() writes a tree unconditionally, so an unstaged edit
203+
used to come back as a hash for an empty commit while the working tree
204+
stayed dirty and the edit stayed uncommitted."""
205+
head_before = test_repository.head.commit.hexsha
206+
file_path = Path(test_repository.working_dir) / "test.txt"
207+
file_path.write_text("edited but never staged")
208+
209+
with pytest.raises(ValueError, match="No changes staged for commit"):
210+
git_commit(test_repository, "should not be created")
211+
212+
assert test_repository.head.commit.hexsha == head_before
213+
assert test_repository.is_dirty()
214+
215+
def test_git_commit_refuses_on_a_clean_tree(test_repository):
216+
head_before = test_repository.head.commit.hexsha
217+
218+
with pytest.raises(ValueError, match="No changes staged for commit"):
219+
git_commit(test_repository, "nothing to record")
220+
221+
assert test_repository.head.commit.hexsha == head_before
222+
223+
def test_git_commit_refuses_when_only_untracked_files_exist(test_repository):
224+
head_before = test_repository.head.commit.hexsha
225+
Path(test_repository.working_dir, "untracked.txt").write_text("never added")
226+
227+
with pytest.raises(ValueError, match="No changes staged for commit"):
228+
git_commit(test_repository, "should not be created")
229+
230+
assert test_repository.head.commit.hexsha == head_before
231+
232+
def test_git_commit_records_a_staged_deletion(test_repository):
233+
"""A deletion leaves no file behind, so it must not read as an empty index."""
234+
test_repository.git.rm("test.txt")
235+
236+
result = git_commit(test_repository, "remove test.txt")
237+
238+
assert "Changes committed successfully with hash" in result
239+
assert "test.txt" not in test_repository.head.commit.tree
240+
241+
def test_git_commit_allows_the_first_commit_on_an_unborn_branch(tmp_path: Path):
242+
repo = git.Repo.init(tmp_path / "unborn")
243+
Path(repo.working_dir, "first.txt").write_text("first")
244+
repo.index.add(["first.txt"])
245+
246+
result = git_commit(repo, "initial commit")
247+
248+
assert "Changes committed successfully with hash" in result
249+
assert repo.head.commit.message.strip() == "initial commit"
250+
251+
def test_git_commit_allows_an_empty_merge_commit(test_repository):
252+
"""git itself permits an empty commit while a merge is in progress, so a
253+
conflict resolved back to HEAD's content must still be committable."""
254+
test_repository.git.checkout("-b", "side")
255+
Path(test_repository.working_dir, "test.txt").write_text("side")
256+
test_repository.git.add("test.txt")
257+
test_repository.index.commit("side change")
258+
259+
test_repository.git.checkout("-")
260+
Path(test_repository.working_dir, "test.txt").write_text("mainline")
261+
test_repository.git.add("test.txt")
262+
test_repository.index.commit("mainline change")
263+
264+
with pytest.raises(git.GitCommandError):
265+
test_repository.git.merge("side")
266+
267+
# Resolve to HEAD's own content, so the index matches HEAD exactly.
268+
Path(test_repository.working_dir, "test.txt").write_text("mainline")
269+
test_repository.git.add("test.txt")
270+
assert not test_repository.index.diff(test_repository.head.commit)
271+
272+
result = git_commit(test_repository, "merge side")
273+
274+
assert "Changes committed successfully with hash" in result
275+
201276
def test_git_reset(test_repository):
202277
file_path = Path(test_repository.working_dir) / "reset_test.txt"
203278
file_path.write_text("content to reset")

0 commit comments

Comments
 (0)