diff --git a/src/auto_commit_service/git/__init__.py b/src/auto_commit_service/git/__init__.py index 5bfbab8..ca0ebe6 100644 --- a/src/auto_commit_service/git/__init__.py +++ b/src/auto_commit_service/git/__init__.py @@ -9,8 +9,6 @@ from .operations import ( git_push, git_pull_rebase, git_fetch, - git_stash, - git_stash_pop, pre_cycle_sync, ) from .diff_parser import summarize_diff, DiffSummary @@ -25,8 +23,6 @@ __all__ = [ "git_push", "git_pull_rebase", "git_fetch", - "git_stash", - "git_stash_pop", "pre_cycle_sync", "summarize_diff", "DiffSummary", diff --git a/src/auto_commit_service/git/operations.py b/src/auto_commit_service/git/operations.py index 76ca06d..00fb2e0 100644 --- a/src/auto_commit_service/git/operations.py +++ b/src/auto_commit_service/git/operations.py @@ -412,25 +412,6 @@ async def git_fetch( return False -async def git_stash(repo_path: Path) -> bool: - """Stash uncommitted changes. Returns True if something was stashed.""" - stdout, _, returncode = await _run_git_command( - "stash", cwd=repo_path, check=False - ) - return returncode == 0 and "No local changes" not in stdout - - -async def git_stash_pop(repo_path: Path) -> bool: - """Pop stashed changes. Returns True on success.""" - _, stderr, returncode = await _run_git_command( - "stash", "pop", cwd=repo_path, check=False - ) - if returncode != 0: - logger.warning(f"git stash pop failed in {repo_path}: {stderr}") - return False - return True - - async def pre_cycle_sync( repo_path: Path, remote: str = "origin", @@ -438,16 +419,17 @@ async def pre_cycle_sync( ) -> dict: """Sync repo with remote before a commit cycle. - Sequence: fetch -> check if behind -> stash -> pull --rebase -> stash pop. + Sequence: fetch -> check if behind -> skip if dirty -> pull --rebase. + Never stashes — dirty trees are left alone and committed normally. All failures are non-fatal (logged and returned in result dict). - Returns dict with keys: fetched, pulled, behind_count, stashed, error + Returns dict with keys: fetched, pulled, behind_count, skipped_dirty, error """ result = { "fetched": False, "pulled": False, "behind_count": 0, - "stashed": False, + "skipped_dirty": False, "error": None, } @@ -468,14 +450,18 @@ async def pre_cycle_sync( if status.behind == 0: return result # Nothing to pull - # Step 3: Stash if dirty - has_changes = status.has_changes - stashed = False - if has_changes: - stashed = await git_stash(repo_path) - result["stashed"] = stashed + # Step 3: Skip pull if working tree is dirty — multiple agents may be + # working concurrently, so stashing is unsafe. The dirty changes will + # get committed this cycle, pushed, and the next cycle will pull clean. + if status.has_changes: + result["skipped_dirty"] = True + logger.debug( + f"Skipping pull for {repo_path}: dirty working tree " + f"({status.behind} commits behind, will sync after commit)" + ) + return result - # Step 4: Pull --rebase + # Step 4: Pull --rebase (clean tree only) try: await git_pull_rebase(repo_path, remote, branch) result["pulled"] = True @@ -486,17 +472,6 @@ async def pre_cycle_sync( logger.warning(f"Pull failed in {repo_path}: {e}") result["error"] = f"pull_failed: {e}" - # Step 5: Unstash if we stashed - if stashed: - pop_ok = await git_stash_pop(repo_path) - if not pop_ok: - logger.warning( - f"Stash pop failed in {repo_path}. " - f"Changes are in git stash; will be picked up next cycle." - ) - error_prefix = result.get("error") or "" - result["error"] = f"{error_prefix}; stash_pop_failed".lstrip("; ") - return result diff --git a/tests/test_git_operations.py b/tests/test_git_operations.py index 19d6f22..faa4937 100644 --- a/tests/test_git_operations.py +++ b/tests/test_git_operations.py @@ -11,8 +11,6 @@ from auto_commit_service.git import ( git_add_all, git_commit, git_fetch, - git_stash, - git_stash_pop, pre_cycle_sync, ) from auto_commit_service.git.repository import Repository @@ -160,42 +158,6 @@ class TestGitFetch: assert result is False -class TestGitStash: - """Tests for git_stash and git_stash_pop.""" - - async def test_stash_dirty_tree(self, temp_git_repo: Path) -> None: - """Test stashing with uncommitted changes.""" - (temp_git_repo / "README.md").write_text("# Modified\n") - stashed = await git_stash(temp_git_repo) - assert stashed is True - - # Working tree should be clean now - status = await git_status(temp_git_repo) - assert not status.has_changes - - async def test_stash_clean_tree(self, temp_git_repo: Path) -> None: - """Test stashing with no changes returns False.""" - stashed = await git_stash(temp_git_repo) - assert stashed is False - - async def test_stash_pop_restores(self, temp_git_repo: Path) -> None: - """Test stash pop restores changes.""" - (temp_git_repo / "README.md").write_text("# Modified\n") - await git_stash(temp_git_repo) - - popped = await git_stash_pop(temp_git_repo) - assert popped is True - - status = await git_status(temp_git_repo) - assert status.has_changes - assert "README.md" in status.modified - - async def test_stash_pop_empty(self, temp_git_repo: Path) -> None: - """Test stash pop with nothing stashed returns False.""" - popped = await git_stash_pop(temp_git_repo) - assert popped is False - - class TestPreCycleSync: """Tests for pre_cycle_sync function.""" @@ -237,8 +199,8 @@ class TestPreCycleSync: # Verify the file arrived assert (temp_git_repo / "other.txt").exists() - async def test_behind_dirty_tree(self, temp_git_repo: Path, tmp_path: Path) -> None: - """Test sync when behind with dirty working tree — stash, pull, pop.""" + async def test_behind_dirty_tree_skips_pull(self, temp_git_repo: Path, tmp_path: Path) -> None: + """Test sync when behind with dirty working tree — skips pull entirely.""" bare = _make_bare_remote(tmp_path, temp_git_repo) # Push a commit from another clone @@ -254,18 +216,19 @@ class TestPreCycleSync: cwd=other, check=True, capture_output=True, ) - # Dirty the local working tree (different file to avoid conflicts) + # Dirty the local working tree (temp_git_repo / "local_wip.txt").write_text("work in progress") result = await pre_cycle_sync(temp_git_repo, "origin", "master") assert result["fetched"] is True - assert result["pulled"] is True - assert result["stashed"] is True + assert result["pulled"] is False + assert result["skipped_dirty"] is True + assert result["behind_count"] == 1 assert result["error"] is None - # Remote file pulled - assert (temp_git_repo / "other.txt").exists() - # Local WIP restored from stash + # Remote file NOT pulled (skipped due to dirty tree) + assert not (temp_git_repo / "other.txt").exists() + # Local WIP untouched assert (temp_git_repo / "local_wip.txt").exists() async def test_fetch_failure(self, temp_git_repo: Path) -> None: