diff --git a/agents/runner.py b/agents/runner.py index bbae90d..9aaa7da 100644 --- a/agents/runner.py +++ b/agents/runner.py @@ -259,10 +259,9 @@ def _run_claude( "--output-format", "json", "--model", model, ] - if allow_write: - cmd.append("--dangerously-skip-permissions") - is_noninteractive = noninteractive or os.environ.get("KIN_NONINTERACTIVE") == "1" + if allow_write or noninteractive: + cmd.append("--dangerously-skip-permissions") if timeout is None: env_timeout = os.environ.get("KIN_AGENT_TIMEOUT") if env_timeout: @@ -824,6 +823,7 @@ def _is_test_failure(result: dict) -> bool: # Roles that trigger auto-test when project.auto_test_enabled is set _AUTO_TEST_ROLES = {"backend_dev", "frontend_dev"} _WORKTREE_ROLES = {"backend_dev", "frontend_dev", "debugger"} +_DEV_GUARD_ROLES = {"backend_dev", "frontend_dev", "debugger"} def _detect_test_command(project_path: str) -> str | None: @@ -1632,6 +1632,7 @@ def run_pipeline( dry_run=False, allow_write=True, noninteractive=noninteractive, + working_dir_override=worktree_path, ) allow_write = True # subsequent steps also with allow_write total_cost += retry.get("cost_usd") or 0 @@ -1712,6 +1713,21 @@ def run_pipeline( "total_duration_seconds": total_duration, "pipeline_id": pipeline["id"] if pipeline else None, } + # Fix D: suspicious guard — empty merged_files means agent made no changes + if not merge_result.get("merged_files"): + result["suspicious"] = True + result["suspicious_reason"] = "agent reported success but no file changes detected" + _logger.warning("KIN-117: suspicious step %s — no merged files after worktree merge", role) + try: + if pipeline: + models.write_log( + conn, pipeline["id"], + f"Step {i+1}/{len(steps)} suspicious: {role} reported success but merged no files", + level="WARN", + extra={"role": role}, + ) + except Exception: + pass cleanup_worktree(worktree_path, p_path) except Exception: pass # Worktree errors must never block pipeline @@ -2028,6 +2044,20 @@ def run_pipeline( if p_path.is_dir(): changed_files = _get_changed_files(str(p_path)) + # Fix D: retroactive suspicious guard — if no files changed after pipeline, + # mark successful dev-agent steps as suspicious (agent may have not written files) + if changed_files is not None and not changed_files: + for _r in results: + if (_r.get("role") in _DEV_GUARD_ROLES + and _r.get("success") + and not _r.get("suspicious")): + _r["suspicious"] = True + _r["suspicious_reason"] = "agent reported success but no file changes detected" + _logger.warning( + "KIN-117: suspicious step %s — no file changes after pipeline", + _r.get("role"), + ) + last_role = steps[-1].get("role", "") if steps else "" # For dept pipelines: if last step is a _head, check the last worker in its sub-pipeline effective_last_role = _last_sub_role if (_is_department_head(last_role) and _last_sub_role) else last_role diff --git a/core/worktree.py b/core/worktree.py index 1062766..30c2c0b 100644 --- a/core/worktree.py +++ b/core/worktree.py @@ -71,15 +71,34 @@ def merge_worktree(worktree_path: str, project_path: str) -> dict: branch_name = Path(worktree_path).name try: + # Stage all changes in the worktree before merge + subprocess.run( + [git, "-C", worktree_path, "add", "-A"], + capture_output=True, + text=True, + timeout=30, + ) + + # Commit staged changes; ignore failure (nothing to commit = returncode 1) + commit_result = subprocess.run( + [git, "-C", worktree_path, "commit", "-m", f"kin: {branch_name}"], + capture_output=True, + text=True, + timeout=30, + ) + commit_had_changes = commit_result.returncode == 0 + merge_result = subprocess.run( - [git, "-C", project_path, "merge", "--no-ff", branch_name], + [git, "merge", "--no-ff", branch_name], + cwd=project_path, capture_output=True, text=True, timeout=60, ) if merge_result.returncode == 0: diff_result = subprocess.run( - [git, "-C", project_path, "diff", "HEAD~1", "HEAD", "--name-only"], + [git, "diff", "HEAD~1", "HEAD", "--name-only"], + cwd=project_path, capture_output=True, text=True, timeout=10, @@ -90,9 +109,15 @@ def merge_worktree(worktree_path: str, project_path: str) -> dict: _logger.info("Merged worktree %s: %d files", branch_name, len(merged_files)) return {"success": True, "conflicts": [], "merged_files": merged_files} + # Merge failed — if commit was also empty (nothing to commit), treat as success + if not commit_had_changes: + _logger.info("Worktree %s: nothing to commit, skipping merge", branch_name) + return {"success": True, "conflicts": [], "merged_files": []} + # Merge failed — collect conflicts and abort conflict_result = subprocess.run( - [git, "-C", project_path, "diff", "--name-only", "--diff-filter=U"], + [git, "diff", "--name-only", "--diff-filter=U"], + cwd=project_path, capture_output=True, text=True, timeout=10, @@ -100,7 +125,8 @@ def merge_worktree(worktree_path: str, project_path: str) -> dict: conflicts = [f.strip() for f in conflict_result.stdout.splitlines() if f.strip()] subprocess.run( - [git, "-C", project_path, "merge", "--abort"], + [git, "merge", "--abort"], + cwd=project_path, capture_output=True, timeout=10, ) diff --git a/tests/test_kin_091_regression.py b/tests/test_kin_091_regression.py index 6729f9f..d23f22b 100644 --- a/tests/test_kin_091_regression.py +++ b/tests/test_kin_091_regression.py @@ -507,7 +507,11 @@ class TestMergeWorktree: merge_ok = MagicMock(returncode=0, stdout="", stderr="") diff_ok = MagicMock(returncode=0, stdout="src/api.py\nsrc/models.py\n", stderr="") - with patch("core.worktree.subprocess.run", side_effect=[merge_ok, diff_ok]): + add_ok = MagicMock(returncode=0, stdout="", stderr="") + commit_ok = MagicMock(returncode=0, stdout="", stderr="") + # Fix B adds git add -A + git commit before merge + with patch("core.worktree.subprocess.run", + side_effect=[add_ok, commit_ok, merge_ok, diff_ok]): result = merge_worktree(worktree, str(tmp_path)) assert result["success"] is True @@ -523,8 +527,11 @@ class TestMergeWorktree: conflict_files = MagicMock(returncode=0, stdout="src/models.py\n", stderr="") abort = MagicMock(returncode=0) + add_ok = MagicMock(returncode=0, stdout="", stderr="") + commit_ok = MagicMock(returncode=0, stdout="", stderr="") + # Fix B adds git add -A + git commit before merge with patch("core.worktree.subprocess.run", - side_effect=[merge_fail, conflict_files, abort]): + side_effect=[add_ok, commit_ok, merge_fail, conflict_files, abort]): result = merge_worktree(worktree, str(tmp_path)) assert result["success"] is False diff --git a/tests/test_kin_103_worktrees.py b/tests/test_kin_103_worktrees.py index 0af8d4d..61a93a2 100644 --- a/tests/test_kin_103_worktrees.py +++ b/tests/test_kin_103_worktrees.py @@ -144,8 +144,10 @@ class TestMergeWorktree: worktree_path = tmp_path / ".kin_worktrees" / "WT1-001-debugger" worktree_path.mkdir(parents=True) - # First call: merge succeeds; second: diff + # Fix B adds git add -A + git commit before merge mock_run.side_effect = [ + _ok_run(), # git add -A + _ok_run(), # git commit -m _ok_run(), # git merge --no-ff _ok_run(), # git diff HEAD~1 HEAD --name-only ] @@ -172,7 +174,8 @@ class TestMergeWorktree: merge_fail = _fail_run("CONFLICT (content)") - mock_run.side_effect = [merge_fail, conflict_diff, abort_result] + # Fix B adds git add -A + git commit before merge + mock_run.side_effect = [_ok_run(), _ok_run(), merge_fail, conflict_diff, abort_result] result = merge_worktree(str(worktree_path), str(tmp_path))