diff --git a/core/worktree.py b/core/worktree.py index 30c2c0b..7964992 100644 --- a/core/worktree.py +++ b/core/worktree.py @@ -10,6 +10,7 @@ All functions are defensive: never raise, always log warnings on error. import logging import shutil import subprocess +import time from pathlib import Path _logger = logging.getLogger("kin.worktree") @@ -59,12 +60,15 @@ def create_worktree(project_path: str, task_id: str, step_name: str = "step") -> return None -def merge_worktree(worktree_path: str, project_path: str) -> dict: +def merge_worktree(worktree_path: str, project_path: str, max_retries: int = 0, retry_delay_s: int = 15) -> dict: """Merge the worktree branch back into current HEAD of project_path. Branch name is derived from the worktree directory name. On conflict: aborts merge and returns success=False with conflict list. + max_retries: number of retry attempts after the first failure (default 0 = no retry). + retry_delay_s: seconds to wait between retry attempts. + Returns {success: bool, conflicts: list[str], merged_files: list[str]} """ git = _git(project_path) @@ -88,50 +92,60 @@ def merge_worktree(worktree_path: str, project_path: str) -> dict: ) commit_had_changes = commit_result.returncode == 0 - merge_result = subprocess.run( - [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, "diff", "HEAD~1", "HEAD", "--name-only"], + for attempt in range(max_retries + 1): + merge_result = subprocess.run( + [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, "diff", "HEAD~1", "HEAD", "--name-only"], + cwd=project_path, + capture_output=True, + text=True, + timeout=10, + ) + merged_files = [ + f.strip() for f in diff_result.stdout.splitlines() if f.strip() + ] + _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, "diff", "--name-only", "--diff-filter=U"], cwd=project_path, capture_output=True, text=True, timeout=10, ) - merged_files = [ - f.strip() for f in diff_result.stdout.splitlines() if f.strip() - ] - _logger.info("Merged worktree %s: %d files", branch_name, len(merged_files)) - return {"success": True, "conflicts": [], "merged_files": merged_files} + conflicts = [f.strip() for f in conflict_result.stdout.splitlines() if f.strip()] - # 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": []} + subprocess.run( + [git, "merge", "--abort"], + cwd=project_path, + capture_output=True, + timeout=10, + ) - # Merge failed — collect conflicts and abort - conflict_result = subprocess.run( - [git, "diff", "--name-only", "--diff-filter=U"], - cwd=project_path, - capture_output=True, - text=True, - timeout=10, - ) - conflicts = [f.strip() for f in conflict_result.stdout.splitlines() if f.strip()] + if attempt < max_retries: + _logger.warning( + "KIN-139: merge conflict in %s (attempt %d/%d), retrying in %ds", + branch_name, attempt + 1, max_retries + 1, retry_delay_s, + ) + time.sleep(retry_delay_s) + continue - subprocess.run( - [git, "merge", "--abort"], - cwd=project_path, - capture_output=True, - timeout=10, - ) - _logger.warning("Merge conflict in worktree %s: %s", branch_name, conflicts) - return {"success": False, "conflicts": conflicts, "merged_files": []} + _logger.warning("Merge conflict in worktree %s: %s", branch_name, conflicts) + return {"success": False, "conflicts": conflicts, "merged_files": []} except Exception as exc: _logger.warning("merge_worktree error for %s: %s", branch_name, exc) diff --git a/tests/test_KIN-145_regression.py b/tests/test_KIN-145_regression.py new file mode 100644 index 0000000..26de613 --- /dev/null +++ b/tests/test_KIN-145_regression.py @@ -0,0 +1,255 @@ +"""KIN-145 — Regression tests for merge_worktree retry/sleep blocking fix. + +Root cause: agents/runner.py was calling merge_worktree(worktree_path, p_path, +max_retries=3, retry_delay_s=15). Git conflicts are deterministic and cannot +be resolved by retrying. Each retry causes time.sleep(15) → 3 retries = 45s +blocking sleep. The pipeline thread blocks, the parent web-server times out, +_check_parent_alive catches ESRCH → all active pipelines marked as failed → +mass failure across all projects. + +Fix: Removed max_retries=3, retry_delay_s=15 kwargs from runner.py:2076. +merge_worktree now uses default max_retries=0 → conflict returns immediately. + +Tests verify: + A. merge_worktree with default max_retries=0 does NOT call time.sleep on conflict + B. merge_worktree with default args returns {success: False} immediately on conflict + C. merge_worktree with explicit max_retries=N DOES sleep N times (feature intact) + D. Old kwargs (max_retries=3, retry_delay_s=15) would produce 45s total blocking + E. runner.py source code does NOT pass max_retries or retry_delay_s (static guard) +""" + +import re +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + + +# ───────────────────────────────────────────────────────────────────────────── +# Helpers +# ───────────────────────────────────────────────────────────────────────────── + +def _ok_run(): + m = MagicMock() + m.returncode = 0 + m.stdout = "" + m.stderr = "" + return m + + +def _fail_run(stderr="CONFLICT (content): merge conflict in src/app.py"): + m = MagicMock() + m.returncode = 1 + m.stdout = "" + m.stderr = stderr + return m + + +def _conflict_diff(): + m = MagicMock() + m.returncode = 0 + m.stdout = "src/conflict.py\n" + m.stderr = "" + return m + + +def _side_effects_for_n_conflict_attempts(n_attempts: int) -> list: + """Build subprocess.run side_effects for n conflict attempts. + + Structure: git-add, git-commit, then per-attempt: merge-fail + diff + abort. + """ + effects = [_ok_run(), _ok_run()] # git add -A, git commit + for _ in range(n_attempts): + effects.extend([_fail_run(), _conflict_diff(), _ok_run()]) # merge-fail, diff, abort + return effects + + +# ───────────────────────────────────────────────────────────────────────────── +# A & B. Default max_retries=0: no sleep, immediate failure return +# ───────────────────────────────────────────────────────────────────────────── + +class TestMergeWorktreeNoRetryDefault: + @patch("core.worktree.time.sleep") + @patch("subprocess.run") + def test_merge_worktree_default_args_does_not_sleep_on_conflict( + self, mock_run, mock_sleep, tmp_path + ): + """KIN-145 fix A: merge_worktree с аргументами по умолчанию НЕ вызывает sleep при конфликте.""" + from core.worktree import merge_worktree + + worktree_path = tmp_path / ".kin_worktrees" / "T-145-dev" + worktree_path.mkdir(parents=True) + mock_run.side_effect = _side_effects_for_n_conflict_attempts(1) + + merge_worktree(str(worktree_path), str(tmp_path)) + + mock_sleep.assert_not_called() + + @patch("core.worktree.time.sleep") + @patch("subprocess.run") + def test_merge_worktree_default_args_returns_failure_immediately_on_conflict( + self, mock_run, mock_sleep, tmp_path + ): + """KIN-145 fix B: merge_worktree без retry немедленно возвращает success=False при конфликте.""" + from core.worktree import merge_worktree + + worktree_path = tmp_path / ".kin_worktrees" / "T-145-dev" + worktree_path.mkdir(parents=True) + mock_run.side_effect = _side_effects_for_n_conflict_attempts(1) + + result = merge_worktree(str(worktree_path), str(tmp_path)) + + assert result["success"] is False + assert "src/conflict.py" in result["conflicts"] + + @patch("core.worktree.time.sleep") + @patch("subprocess.run") + def test_merge_worktree_explicit_max_retries_0_no_sleep( + self, mock_run, mock_sleep, tmp_path + ): + """KIN-145: explicit max_retries=0 тоже не вызывает sleep при конфликте.""" + from core.worktree import merge_worktree + + worktree_path = tmp_path / ".kin_worktrees" / "T-145-b-dev" + worktree_path.mkdir(parents=True) + mock_run.side_effect = _side_effects_for_n_conflict_attempts(1) + + merge_worktree(str(worktree_path), str(tmp_path), max_retries=0) + + mock_sleep.assert_not_called() + + +# ───────────────────────────────────────────────────────────────────────────── +# C. Retry feature still works when explicitly requested +# ───────────────────────────────────────────────────────────────────────────── + +class TestMergeWorktreeRetryBehaviorIntact: + @patch("core.worktree.time.sleep") + @patch("subprocess.run") + def test_merge_worktree_max_retries_2_sleeps_twice( + self, mock_run, mock_sleep, tmp_path + ): + """KIN-145 fix C: retry-функциональность не сломана — max_retries=2 вызывает sleep 2 раза.""" + from core.worktree import merge_worktree + + worktree_path = tmp_path / ".kin_worktrees" / "T-145-retry-dev" + worktree_path.mkdir(parents=True) + # max_retries=2 → 3 total attempts (attempt 0,1,2) + # sleep happens after attempt 0 and 1 (not after final attempt 2) + mock_run.side_effect = _side_effects_for_n_conflict_attempts(3) + + merge_worktree(str(worktree_path), str(tmp_path), max_retries=2, retry_delay_s=5) + + assert mock_sleep.call_count == 2 + for c in mock_sleep.call_args_list: + assert c.args[0] == 5 + + @patch("core.worktree.time.sleep") + @patch("subprocess.run") + def test_merge_worktree_retry_with_eventual_success_no_failure( + self, mock_run, mock_sleep, tmp_path + ): + """KIN-145: retry завершается успехом если второй attempt проходит.""" + from core.worktree import merge_worktree + + worktree_path = tmp_path / ".kin_worktrees" / "T-145-retry-ok" + worktree_path.mkdir(parents=True) + + diff_ok = MagicMock() + diff_ok.returncode = 0 + diff_ok.stdout = "src/fixed.py\n" + diff_ok.stderr = "" + + mock_run.side_effect = [ + _ok_run(), # git add -A + _ok_run(), # git commit + _fail_run(), # merge attempt 0: fail + _conflict_diff(), # git diff --diff-filter=U + _ok_run(), # git merge --abort + _ok_run(), # merge attempt 1: success + diff_ok, # git diff HEAD~1 HEAD --name-only + ] + + result = merge_worktree(str(worktree_path), str(tmp_path), max_retries=1, retry_delay_s=1) + + assert result["success"] is True + assert mock_sleep.call_count == 1 + + +# ───────────────────────────────────────────────────────────────────────────── +# D. Old buggy kwargs would have caused 45s total blocking sleep +# ───────────────────────────────────────────────────────────────────────────── + +class TestOldBuggyBehaviorDocumented: + @patch("core.worktree.time.sleep") + @patch("subprocess.run") + def test_old_kwargs_max_retries_3_delay_15_sleeps_45s_total( + self, mock_run, mock_sleep, tmp_path + ): + """KIN-145 doc D: старые kwargs max_retries=3, retry_delay_s=15 = 45s суммарного blocking. + + Этот тест документирует удалённое поведение. + Если этот тест когда-либо упадёт (sleep_count != 3), значит поведение изменилось. + """ + from core.worktree import merge_worktree + + worktree_path = tmp_path / ".kin_worktrees" / "T-145-old-dev" + worktree_path.mkdir(parents=True) + # max_retries=3 → 4 total attempts → 3 sleeps + mock_run.side_effect = _side_effects_for_n_conflict_attempts(4) + + merge_worktree(str(worktree_path), str(tmp_path), max_retries=3, retry_delay_s=15) + + assert mock_sleep.call_count == 3 + total_sleep_s = sum(c.args[0] for c in mock_sleep.call_args_list) + assert total_sleep_s == 45 + + +# ───────────────────────────────────────────────────────────────────────────── +# E. Static guard: runner.py must not pass retry kwargs to merge_worktree +# ───────────────────────────────────────────────────────────────────────────── + +class TestRunnerCallSiteStaticGuard: + def test_runner_merge_worktree_call_has_no_max_retries_kwarg(self): + """KIN-145 guard E: runner.py НЕ передаёт max_retries в merge_worktree.""" + runner_path = Path(__file__).parent.parent / "agents" / "runner.py" + source = runner_path.read_text() + + calls = re.findall(r"merge_worktree\([^)]*\)", source, re.DOTALL) + assert calls, "merge_worktree не найден в agents/runner.py — проверь путь" + + for call_src in calls: + assert "max_retries" not in call_src, ( + f"runner.py вызывает merge_worktree с max_retries: {call_src!r}\n" + "Это баг KIN-145 — retry kwargs вызывают 45s blocking sleep!" + ) + + def test_runner_merge_worktree_call_has_no_retry_delay_kwarg(self): + """KIN-145 guard E: runner.py НЕ передаёт retry_delay_s в merge_worktree.""" + runner_path = Path(__file__).parent.parent / "agents" / "runner.py" + source = runner_path.read_text() + + calls = re.findall(r"merge_worktree\([^)]*\)", source, re.DOTALL) + assert calls, "merge_worktree не найден в agents/runner.py — проверь путь" + + for call_src in calls: + assert "retry_delay_s" not in call_src, ( + f"runner.py вызывает merge_worktree с retry_delay_s: {call_src!r}\n" + "Это баг KIN-145 — retry kwargs вызывают 45s blocking sleep!" + ) + + def test_runner_merge_worktree_call_uses_exactly_two_positional_args(self): + """KIN-145 guard E: вызов merge_worktree в runner.py имеет ровно 2 аргумента.""" + runner_path = Path(__file__).parent.parent / "agents" / "runner.py" + source = runner_path.read_text() + + calls = re.findall(r"merge_worktree\(([^)]*)\)", source, re.DOTALL) + assert calls, "merge_worktree не найден в agents/runner.py" + + for call_args in calls: + args = [a.strip() for a in call_args.split(",") if a.strip()] + assert len(args) == 2, ( + f"runner.py вызывает merge_worktree с {len(args)} аргументами: {call_args!r}\n" + f"Ожидается ровно 2 (worktree_path, p_path). " + f"Лишние kwargs — источник бага KIN-145!" + ) diff --git a/web/frontend/src/views/Dashboard.vue b/web/frontend/src/views/Dashboard.vue index 8aa2393..8d2794e 100644 --- a/web/frontend/src/views/Dashboard.vue +++ b/web/frontend/src/views/Dashboard.vue @@ -1,5 +1,5 @@