From f10daa235925c341cd5f9058ff3a2baa20425588 Mon Sep 17 00:00:00 2001 From: Pawel Lebioda Date: Tue, 7 Jul 2026 11:43:33 +0200 Subject: [PATCH] Stage deleted conflict-resolution files with index.remove, not index.add A "deleted by us"/"deleted by them" conflict resolved by removing the file leaves nothing on disk for GitPython's index.add to read, so commit_solution and commit_ci_fix_solution crashed with FileNotFoundError instead of staging the removal. --- src/mergai/app.py | 10 ++++- src/mergai/ci/commit.py | 18 ++++++++- tests/test_ci_fix_commit.py | 69 +++++++++++++++++++++++++++++++++++ tests/test_commit_solution.py | 20 ++++++++++ 4 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 tests/test_ci_fix_commit.py diff --git a/src/mergai/app.py b/src/mergai/app.py index 49ffcf5..d806a1b 100644 --- a/src/mergai/app.py +++ b/src/mergai/app.py @@ -658,10 +658,16 @@ def commit_solution(self): for item in self.repo.index.diff(None): path = item.a_path if path in resolved: - self.repo.index.add([path]) + if item.deleted_file: + self.repo.index.remove([path]) + else: + self.repo.index.add([path]) elif path in declared_modified: modified_files.append(path) - self.repo.index.add([path]) + if item.deleted_file: + self.repo.index.remove([path]) + else: + self.repo.index.add([path]) else: message = ( f"Unstaged changes found in file {path}, which the " diff --git a/src/mergai/ci/commit.py b/src/mergai/ci/commit.py index 952052f..0d8c659 100644 --- a/src/mergai/ci/commit.py +++ b/src/mergai/ci/commit.py @@ -6,6 +6,7 @@ keeps a thin delegator. """ +import os from typing import Any from ..solution_types import CI_FIX @@ -67,10 +68,23 @@ def commit_ci_fix_solution(app: Any, solution_idx: int) -> None: # Stage every file the agent touched (resolved + modified). # Untracked files won't show up in index.diff(None), so add them # explicitly via git index.add — same approach commit_solution uses. + # A file the agent deleted to resolve a conflict no longer exists on + # disk, so it must be staged via index.remove instead — index.add + # would try to read it and raise FileNotFoundError. files_to_stage = list(response.get("resolved", {}).keys()) files_to_stage += list(response.get("modified", {}).keys()) - if files_to_stage: - app.repo.index.add(files_to_stage) + working_dir = app.repo.working_tree_dir or "" + files_to_add = [] + files_to_remove = [] + for f in files_to_stage: + if os.path.exists(os.path.join(working_dir, f)): + files_to_add.append(f) + else: + files_to_remove.append(f) + if files_to_add: + app.repo.index.add(files_to_add) + if files_to_remove: + app.repo.index.remove(files_to_remove) # Build commit message matching mergai's voice. The title is # configurable (commit.ci_fix_title_format) so it can mirror the PR diff --git a/tests/test_ci_fix_commit.py b/tests/test_ci_fix_commit.py new file mode 100644 index 0000000..a4614f7 --- /dev/null +++ b/tests/test_ci_fix_commit.py @@ -0,0 +1,69 @@ +"""Tests for commit_ci_fix_solution staging behavior. + +Regression coverage mirroring tests/test_commit_solution.py: when the agent +resolves a CI failure by deleting a file, staging must use index.remove +instead of index.add, since the file no longer exists on disk for add to +read. +""" + +from types import SimpleNamespace + +import git + +from mergai.ci.commit import commit_ci_fix_solution +from mergai.solution_types import CI_FIX + + +def _init_repo(path): + repo = git.Repo.init(path) + with repo.config_writer() as cw: + cw.set_value("user", "name", "test") + cw.set_value("user", "email", "test@example.com") + (path / "a.txt").write_text("base a\n") + repo.index.add(["a.txt"]) + repo.index.commit("init") + return repo + + +def _fake_app(repo, *, resolved): + head = repo.head.commit.hexsha + note = SimpleNamespace( + has_solutions=True, + solutions=[ + { + "type": CI_FIX, + "request": {"workflow": "format", "run_id": "1", "attempt_number": 1}, + "response": {"summary": "fix", "resolved": resolved, "modified": {}}, + } + ], + merge_info=SimpleNamespace(target_branch="master", merge_commit_sha=head), + ) + config = SimpleNamespace( + workflows={}, + commit=SimpleNamespace( + ci_fix_title_format="Fix '%(workflow)' failure for merge commit " + "'%(merge_commit_short_sha)' into '%(target_branch)'" + ), + ) + return SimpleNamespace( + note=note, + repo=repo, + config=config, + commit_footer="", + add_selective_note=lambda *a, **k: None, + ) + + +def test_commit_ci_fix_solution_stages_deleted_resolved_file(tmp_path): + repo = _init_repo(tmp_path) + + # The agent resolved the CI failure by deleting the offending file. + (tmp_path / "a.txt").unlink() + + app = _fake_app(repo, resolved={"a.txt": "removed stale file"}) + + commit_ci_fix_solution(app, 0) + + assert not repo.is_dirty() + assert "a.txt" not in [item[0] for item in repo.index.entries] + assert not (tmp_path / "a.txt").exists() diff --git a/tests/test_commit_solution.py b/tests/test_commit_solution.py index b0fa5e7..f4e26c1 100644 --- a/tests/test_commit_solution.py +++ b/tests/test_commit_solution.py @@ -68,6 +68,26 @@ def test_commit_solution_accepts_declared_modified_files(tmp_path, monkeypatch): assert (tmp_path / "version.h").read_text() == "adjusted v\n" +def test_commit_solution_stages_deleted_resolved_file(tmp_path, monkeypatch): + repo = _init_repo(tmp_path) + monkeypatch.chdir(tmp_path) + + # Conflict resolved by deleting the file (e.g. a "deleted by us" conflict + # where the deletion side wins) — commit_solution must stage the removal + # via index.remove instead of trying to index.add a path that no longer + # exists on disk. + (tmp_path / "a.txt").unlink() + + app = AppContext() + app._note = _note(repo, resolved={"a.txt": {}}, modified={}) + + app.commit_solution() + + assert not repo.is_dirty() + assert "a.txt" not in [item[0] for item in repo.index.entries] + assert not (tmp_path / "a.txt").exists() + + def test_commit_solution_rejects_undeclared_dirty_file(tmp_path, monkeypatch): repo = _init_repo(tmp_path) monkeypatch.chdir(tmp_path)