"""Regression tests for KIN-117 — dev-agent silent success detection. Root cause: dev-agents периодически заявляют об успешной реализации, но файлы не записываются на диск. Три причины: (1) --dangerously-skip-permissions не добавляется когда stdin=DEVNULL (noninteractive) (2) worktree создаётся, но merge не происходит (uncommitted files) (3) permission-retry вызывает run_agent без working_dir_override=worktree_path Fix D добавляет guard: после успешного шага dev-агента проверяет git diff — если изменений нет, помечает шаг suspicious=True. Acceptance criteria: (1) Fix A: noninteractive=True → --dangerously-skip-permissions в subprocess cmd (2) Fix D: dev-агент success + пустой git diff → step.suspicious=True (3) Fix D: dev-агент success + непустой git diff → step.suspicious НЕ выставлен (нет false positive) (4) Fix C: permission retry в auto_complete режиме → subprocess cwd=worktree_path (5) Fix B: merge_worktree запускает git add -A + git commit перед git merge --no-ff """ import json from pathlib import Path from unittest.mock import MagicMock, patch import pytest from core.db import init_db from core import models # ─── Helpers ──────────────────────────────────────────────────────────────── def _ok(stdout=""): m = MagicMock() m.returncode = 0 m.stdout = stdout m.stderr = "" return m def _fail(stdout="", stderr="error"): m = MagicMock() m.returncode = 1 m.stdout = stdout m.stderr = stderr return m def _claude_success(): return _ok(json.dumps({"result": "all done, implementation complete"})) def _permission_denied(): """Subprocess result that triggers _is_permission_error.""" m = MagicMock() m.returncode = 1 m.stdout = "" m.stderr = "permission denied to write file" return m # ─── Fixtures ──────────────────────────────────────────────────────────────── @pytest.fixture def conn_basic(tmp_path): """Project with path=tmp_path (real dir), review mode, no worktrees.""" c = init_db(":memory:") models.create_project(c, "prj1", "Test Project", str(tmp_path), tech_stack=["python"]) models.create_task(c, "TSK-001", "prj1", "Fix something", brief={"route_type": "debug"}) yield c, str(tmp_path) c.close() @pytest.fixture def conn_auto_wt(tmp_path): """Project with auto_complete mode and worktrees enabled, real path.""" c = init_db(":memory:") models.create_project(c, "prj2", "Auto WT Project", str(tmp_path), tech_stack=["python"]) c.execute( "UPDATE projects SET execution_mode='auto_complete', worktrees_enabled=1 WHERE id='prj2'" ) c.commit() models.create_task(c, "TSK-002", "prj2", "Fix with worktree", brief={"route_type": "debug"}) yield c, str(tmp_path) c.close() # ─── Fix A: noninteractive mode добавляет --dangerously-skip-permissions ───── class TestFixA_NoninteractiveSkipPermissions: """Fix A: is_noninteractive вычисляется ДО проверки allow_write.""" @patch("agents.runner.subprocess.run") def test_noninteractive_adds_dangerously_skip_permissions(self, mock_run): """noninteractive=True, allow_write=False → --dangerously-skip-permissions в cmd.""" from agents.runner import _run_claude mock_run.return_value = _claude_success() _run_claude("test prompt", noninteractive=True, allow_write=False) cmd = mock_run.call_args[0][0] assert "--dangerously-skip-permissions" in cmd, ( "--dangerously-skip-permissions must be added when noninteractive=True, " f"got cmd: {cmd}" ) @patch("agents.runner.subprocess.run") def test_allow_write_adds_dangerously_skip_permissions(self, mock_run): """allow_write=True → --dangerously-skip-permissions в cmd (существующее поведение).""" from agents.runner import _run_claude mock_run.return_value = _claude_success() _run_claude("test prompt", allow_write=True, noninteractive=False) cmd = mock_run.call_args[0][0] assert "--dangerously-skip-permissions" in cmd @patch("agents.runner.subprocess.run") def test_interactive_no_allow_write_no_skip_permissions(self, mock_run): """allow_write=False + noninteractive=False → --dangerously-skip-permissions НЕ в cmd.""" from agents.runner import _run_claude mock_run.return_value = _claude_success() _run_claude("test prompt", allow_write=False, noninteractive=False) cmd = mock_run.call_args[0][0] assert "--dangerously-skip-permissions" not in cmd, ( "--dangerously-skip-permissions must NOT be added in interactive mode without allow_write" ) # ─── Fix D: suspicious guard (non-worktree path) ───────────────────────────── class TestFixD_SuspiciousGuardNoWorktree: """Fix D: guard проверяет git diff после успешного шага dev-агента.""" @patch("agents.runner.check_claude_auth") @patch("agents.runner.subprocess.run") def test_success_empty_git_diff_marks_step_suspicious(self, mock_run, mock_auth, conn_basic): """dev-агент success + пустой git diff → result['suspicious']=True.""" from agents.runner import run_pipeline conn, proj_path = conn_basic def side_effect(cmd, **kwargs): cmd_str = " ".join(str(c) for c in cmd) if "diff" in cmd_str: return _ok("") # empty diff → no file changes return _claude_success() # claude call mock_run.side_effect = side_effect mock_auth.return_value = None result = run_pipeline(conn, "TSK-001", [{"role": "debugger", "brief": "fix it"}]) assert result["success"] is True step_result = result["results"][0] assert step_result.get("suspicious") is True, ( "Step must be marked suspicious=True when agent claims success " f"but git diff is empty. Got result: {step_result}" ) @patch("agents.runner.check_claude_auth") @patch("agents.runner.subprocess.run") def test_success_nonempty_git_diff_not_suspicious(self, mock_run, mock_auth, conn_basic): """dev-агент success + непустой git diff → suspicious НЕ выставляется.""" from agents.runner import run_pipeline conn, proj_path = conn_basic def side_effect(cmd, **kwargs): cmd_str = " ".join(str(c) for c in cmd) if "diff" in cmd_str: return _ok("src/app.py\nsrc/utils.py\n") # files changed return _claude_success() mock_run.side_effect = side_effect mock_auth.return_value = None result = run_pipeline(conn, "TSK-001", [{"role": "debugger", "brief": "fix it"}]) assert result["success"] is True step_result = result["results"][0] assert not step_result.get("suspicious"), ( "Step must NOT be suspicious when git diff shows changed files. " f"Got result: {step_result}" ) @patch("agents.runner.check_claude_auth") @patch("agents.runner.subprocess.run") def test_suspicious_step_does_not_block_pipeline(self, mock_run, mock_auth, conn_basic): """suspicious шаг не останавливает pipeline — только выставляет флаг.""" from agents.runner import run_pipeline conn, proj_path = conn_basic def side_effect(cmd, **kwargs): cmd_str = " ".join(str(c) for c in cmd) if "diff" in cmd_str: return _ok("") # empty diff → suspicious, but pipeline continues return _claude_success() mock_run.side_effect = side_effect mock_auth.return_value = None result = run_pipeline(conn, "TSK-001", [{"role": "debugger", "brief": "fix"}]) # Pipeline must succeed even when step is suspicious assert result["success"] is True assert result["steps_completed"] == 1 # ─── Fix D: suspicious guard (worktree path) ───────────────────────────────── class TestFixD_SuspiciousGuardWithWorktree: """Fix D: guard проверяет merged_files после worktree merge.""" @patch("agents.runner.check_claude_auth") @patch("core.worktree.cleanup_worktree") @patch("core.worktree.merge_worktree", return_value={"success": True, "conflicts": [], "merged_files": []}) @patch("core.worktree.create_worktree", return_value="/tmp/WT-suspicious-debugger") @patch("core.worktree.ensure_gitignore") @patch("agents.runner.subprocess.run") def test_worktree_empty_merged_files_marks_suspicious( self, mock_run, mock_gitignore, mock_create, mock_merge, mock_cleanup, mock_auth, conn_auto_wt ): """worktree merge возвращает merged_files=[] → шаг помечается suspicious=True.""" from agents.runner import run_pipeline conn, proj_path = conn_auto_wt mock_run.return_value = _claude_success() mock_auth.return_value = None result = run_pipeline(conn, "TSK-002", [{"role": "debugger", "brief": "fix it"}]) assert result["success"] is True step_result = result["results"][0] assert step_result.get("suspicious") is True, ( "Step must be marked suspicious when worktree merge returns merged_files=[]. " f"Got result: {step_result}" ) @patch("agents.runner.check_claude_auth") @patch("core.worktree.cleanup_worktree") @patch("core.worktree.merge_worktree", return_value={"success": True, "conflicts": [], "merged_files": ["src/app.py"]}) @patch("core.worktree.create_worktree", return_value="/tmp/WT-ok-debugger") @patch("core.worktree.ensure_gitignore") @patch("agents.runner.subprocess.run") def test_worktree_nonempty_merged_files_not_suspicious( self, mock_run, mock_gitignore, mock_create, mock_merge, mock_cleanup, mock_auth, conn_auto_wt ): """worktree merge возвращает merged_files=['src/app.py'] → шаг НЕ suspicious.""" from agents.runner import run_pipeline conn, proj_path = conn_auto_wt mock_run.return_value = _claude_success() mock_auth.return_value = None result = run_pipeline(conn, "TSK-002", [{"role": "debugger", "brief": "fix it"}]) assert result["success"] is True step_result = result["results"][0] assert not step_result.get("suspicious"), ( "Step must NOT be suspicious when worktree has merged files. " f"Got result: {step_result}" ) # ─── Fix C: permission retry использует worktree_path ───────────────────────── class TestFixC_PermissionRetryWorktreePath: """Fix C: retry после permission error передаёт working_dir_override=worktree_path.""" FAKE_WT = "/tmp/fake-worktree-kin117" @patch("agents.runner.check_claude_auth") @patch("core.worktree.cleanup_worktree") @patch("core.worktree.merge_worktree", return_value={"success": True, "conflicts": [], "merged_files": ["fix.py"]}) @patch("core.worktree.create_worktree", return_value=FAKE_WT) @patch("core.worktree.ensure_gitignore") @patch("agents.runner.subprocess.run") def test_permission_retry_uses_worktree_cwd( self, mock_run, mock_gitignore, mock_create, mock_merge, mock_cleanup, mock_auth, conn_auto_wt ): """При permission error в auto_complete режиме retry запускается в cwd=worktree_path.""" from agents.runner import run_pipeline conn, proj_path = conn_auto_wt claude_calls = [] def side_effect(cmd, **kwargs): if "--output-format" in cmd: cwd = kwargs.get("cwd") claude_calls.append({"cmd": list(cmd), "cwd": cwd}) # First claude call → permission error triggers retry if len(claude_calls) == 1: return _permission_denied() return _claude_success() # Non-claude subprocess calls (git diff, etc.) return _ok("") mock_run.side_effect = side_effect mock_auth.return_value = None result = run_pipeline(conn, "TSK-002", [{"role": "debugger", "brief": "implement feature"}]) assert len(claude_calls) >= 2, ( f"Expected at least 2 claude calls (first + retry), got {len(claude_calls)}" ) retry_call = claude_calls[1] assert retry_call["cwd"] == self.FAKE_WT, ( f"Retry must use worktree_path={self.FAKE_WT!r} as cwd, " f"got {retry_call['cwd']!r}. " "Fix: add working_dir_override=worktree_path to permission retry run_agent call." ) # ─── Fix B: merge_worktree auto-commits перед merge ────────────────────────── class TestFixB_MergeWorktreePreCommit: """Fix B: merge_worktree выполняет git add -A + git commit перед git merge --no-ff.""" @patch("subprocess.run") def test_merge_worktree_runs_git_add_before_merge(self, mock_run, tmp_path): """git add -A вызывается ДО git merge --no-ff.""" from core.worktree import merge_worktree worktree_path = tmp_path / ".kin_worktrees" / "TASK-001-debugger" worktree_path.mkdir(parents=True) commands_seen = [] def run_side_effect(cmd, **kwargs): commands_seen.append(list(cmd)) return _ok() mock_run.side_effect = run_side_effect merge_worktree(str(worktree_path), str(tmp_path)) flat = [" ".join(c) for c in commands_seen] add_idx = next( (i for i, c in enumerate(flat) if "add" in c and "-A" in c), None ) merge_idx = next( (i for i, c in enumerate(flat) if "merge" in c and "--no-ff" in c), None ) assert add_idx is not None, ( "git add -A must be called before merge. " f"Commands seen: {flat}" ) assert merge_idx is not None, "git merge --no-ff must be called" assert add_idx < merge_idx, ( f"git add -A (idx={add_idx}) must be BEFORE git merge --no-ff (idx={merge_idx})" ) @patch("subprocess.run") def test_merge_worktree_runs_git_commit_before_merge(self, mock_run, tmp_path): """git commit вызывается ДО git merge --no-ff.""" from core.worktree import merge_worktree worktree_path = tmp_path / ".kin_worktrees" / "TASK-001-debugger" worktree_path.mkdir(parents=True) commands_seen = [] def run_side_effect(cmd, **kwargs): commands_seen.append(list(cmd)) return _ok() mock_run.side_effect = run_side_effect merge_worktree(str(worktree_path), str(tmp_path)) flat = [" ".join(c) for c in commands_seen] commit_idx = next( (i for i, c in enumerate(flat) if "commit" in c and "-m" in c), None ) merge_idx = next( (i for i, c in enumerate(flat) if "merge" in c and "--no-ff" in c), None ) assert commit_idx is not None, ( "git commit -m must be called before merge. " f"Commands seen: {flat}" ) assert merge_idx is not None, "git merge --no-ff must be called" assert commit_idx < merge_idx, ( f"git commit (idx={commit_idx}) must be BEFORE git merge --no-ff (idx={merge_idx})" ) @patch("subprocess.run") def test_merge_worktree_commit_failure_does_not_block_merge(self, mock_run, tmp_path): """Если git commit возвращает non-zero (нечего коммитить) — merge всё равно запускается.""" from core.worktree import merge_worktree worktree_path = tmp_path / ".kin_worktrees" / "TASK-001-debugger" worktree_path.mkdir(parents=True) commands_seen = [] def run_side_effect(cmd, **kwargs): commands_seen.append(list(cmd)) cmd_str = " ".join(cmd) if "commit" in cmd_str: return _fail(stderr="nothing to commit") # nothing to commit return _ok() mock_run.side_effect = run_side_effect result = merge_worktree(str(worktree_path), str(tmp_path)) flat = [" ".join(c) for c in commands_seen] merge_calls = [c for c in flat if "merge" in c and "--no-ff" in c] assert len(merge_calls) > 0, ( "git merge --no-ff must still be called even when pre-commit fails. " f"Commands seen: {flat}" ) assert result["success"] is True