diff --git a/codeframe/core/adapters/subprocess_adapter.py b/codeframe/core/adapters/subprocess_adapter.py index e3be6d4a..8106e041 100644 --- a/codeframe/core/adapters/subprocess_adapter.py +++ b/codeframe/core/adapters/subprocess_adapter.py @@ -153,12 +153,16 @@ def _drain_stderr() -> None: stderr_output = "".join(stderr_chunks) - return self._map_result( + modified_files = self._detect_modified_files(workspace_path) + + result = self._map_result( exit_code=process.returncode, stdout="\n".join(stdout_lines), stderr=stderr_output, workspace_path=workspace_path, ) + result.modified_files = modified_files + return result def _map_result( self, @@ -196,6 +200,44 @@ def _map_result( error=stderr or f"Process exited with code {exit_code}", ) + def _detect_modified_files(self, workspace_path: Path) -> list[str]: + """Detect files modified by the subprocess via git diff. + + Combines modified, staged, and untracked files. Returns an empty list + if git is unavailable or the workspace is not a git repo. + """ + try: + result = subprocess.run( + ["git", "diff", "--name-only", "HEAD"], + cwd=str(workspace_path), + capture_output=True, + text=True, + timeout=10, + ) + if result.returncode != 0: + # Also covers repos with no commits (HEAD does not exist) + return [] + + files = [f for f in result.stdout.strip().splitlines() if f] + + # Also pick up untracked files + untracked = subprocess.run( + ["git", "ls-files", "--others", "--exclude-standard"], + cwd=str(workspace_path), + capture_output=True, + text=True, + timeout=10, + ) + if untracked.returncode == 0: + files.extend( + f for f in untracked.stdout.strip().splitlines() if f + ) + + # Deduplicate while preserving order + return list(dict.fromkeys(files)) + except (FileNotFoundError, OSError, subprocess.TimeoutExpired): + return [] + def _extract_blocker_question(self, output: str) -> str: """Extract a meaningful blocker question from output.""" lines = [line.strip() for line in output.splitlines() if line.strip()] diff --git a/tests/core/adapters/test_claude_code.py b/tests/core/adapters/test_claude_code.py index 5affaaa2..def67c1b 100644 --- a/tests/core/adapters/test_claude_code.py +++ b/tests/core/adapters/test_claude_code.py @@ -12,6 +12,12 @@ class TestClaudeCodeAdapter: """Unit tests for ClaudeCodeAdapter.""" + @pytest.fixture(autouse=True) + def _no_git(self): + """Prevent _detect_modified_files from calling real git.""" + with patch.object(ClaudeCodeAdapter, "_detect_modified_files", return_value=[]): + yield + def test_name(self) -> None: with patch("shutil.which", return_value="/usr/bin/claude"): adapter = ClaudeCodeAdapter() diff --git a/tests/core/adapters/test_opencode.py b/tests/core/adapters/test_opencode.py index 89518e51..ac4c36e1 100644 --- a/tests/core/adapters/test_opencode.py +++ b/tests/core/adapters/test_opencode.py @@ -12,6 +12,12 @@ class TestOpenCodeAdapter: """Unit tests for OpenCodeAdapter.""" + @pytest.fixture(autouse=True) + def _no_git(self): + """Prevent _detect_modified_files from calling real git.""" + with patch.object(OpenCodeAdapter, "_detect_modified_files", return_value=[]): + yield + def test_name(self) -> None: with patch("shutil.which", return_value="/usr/bin/opencode"): adapter = OpenCodeAdapter() diff --git a/tests/core/adapters/test_subprocess_adapter.py b/tests/core/adapters/test_subprocess_adapter.py index b998aa32..5c9d3032 100644 --- a/tests/core/adapters/test_subprocess_adapter.py +++ b/tests/core/adapters/test_subprocess_adapter.py @@ -42,6 +42,12 @@ def test_init_stores_resolved_path(self): class TestSubprocessAdapterRun: """Tests for subprocess execution.""" + @pytest.fixture(autouse=True) + def _no_git(self): + """Prevent _detect_modified_files from calling real git.""" + with patch.object(SubprocessAdapter, "_detect_modified_files", return_value=[]): + yield + @pytest.fixture def adapter(self): with patch("shutil.which", return_value="/usr/bin/test-agent"): @@ -222,6 +228,119 @@ def test_default_returns_empty_prompt(self): assert adapter.get_stdin("") == "" +class TestSubprocessAdapterModifiedFiles: + """Tests for git diff file detection after execution.""" + + @pytest.fixture + def adapter(self): + with patch("shutil.which", return_value="/usr/bin/test-agent"): + return SubprocessAdapter("test-agent") + + def _make_mock_process( + self, stdout_lines=None, stderr_text="", returncode=0 + ): + mock = MagicMock() + mock.stdout = iter(stdout_lines or []) + mock.stderr = MagicMock() + mock.stderr.read.return_value = stderr_text + mock.stdin = MagicMock() + mock.returncode = returncode + mock.wait.return_value = None + return mock + + def test_populates_modified_files_on_success(self, adapter, tmp_path): + """After successful execution, modified_files should list changed files.""" + mock_process = self._make_mock_process( + stdout_lines=["done\n"], returncode=0 + ) + with ( + patch("subprocess.Popen", return_value=mock_process), + patch( + "subprocess.run", + side_effect=[ + MagicMock(returncode=0, stdout="src/main.py\ntests/test_main.py\n"), + MagicMock(returncode=0, stdout=""), # no untracked files + ], + ), + ): + result = adapter.run("task-1", "fix", tmp_path) + + assert result.status == "completed" + assert result.modified_files == ["src/main.py", "tests/test_main.py"] + + def test_empty_modified_files_when_no_changes(self, adapter, tmp_path): + mock_process = self._make_mock_process( + stdout_lines=["done\n"], returncode=0 + ) + with ( + patch("subprocess.Popen", return_value=mock_process), + patch( + "subprocess.run", + side_effect=[ + MagicMock(returncode=0, stdout=""), + MagicMock(returncode=0, stdout=""), + ], + ), + ): + result = adapter.run("task-1", "fix", tmp_path) + + assert result.modified_files == [] + + def test_detects_files_even_on_failure(self, adapter, tmp_path): + """Failed execution should still detect modified files.""" + mock_process = self._make_mock_process( + stderr_text="error", returncode=1 + ) + with ( + patch("subprocess.Popen", return_value=mock_process), + patch( + "subprocess.run", + side_effect=[ + MagicMock(returncode=0, stdout="src/broken.py\n"), + MagicMock(returncode=0, stdout=""), + ], + ), + ): + result = adapter.run("task-1", "fix", tmp_path) + + assert result.status == "failed" + assert "src/broken.py" in result.modified_files + + def test_graceful_when_not_git_repo(self, adapter, tmp_path): + """Should return empty modified_files if git is unavailable.""" + mock_process = self._make_mock_process( + stdout_lines=["done\n"], returncode=0 + ) + with ( + patch("subprocess.Popen", return_value=mock_process), + patch( + "subprocess.run", + side_effect=FileNotFoundError("git not found"), + ), + ): + result = adapter.run("task-1", "fix", tmp_path) + + assert result.status == "completed" + assert result.modified_files == [] + + def test_graceful_when_git_fails(self, adapter, tmp_path): + """Should return empty modified_files if git diff fails.""" + mock_process = self._make_mock_process( + stdout_lines=["done\n"], returncode=0 + ) + with ( + patch("subprocess.Popen", return_value=mock_process), + patch( + "subprocess.run", + return_value=MagicMock(returncode=128, stdout=""), + ), + ): + result = adapter.run("task-1", "fix", tmp_path) + + assert result.status == "completed" + assert result.modified_files == [] + + class TestSubprocessAdapterBlockerExtraction: """Tests for blocker question extraction."""