diff --git a/core/worktree.py b/core/worktree.py index 7964992..30c2c0b 100644 --- a/core/worktree.py +++ b/core/worktree.py @@ -10,7 +10,6 @@ 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") @@ -60,15 +59,12 @@ def create_worktree(project_path: str, task_id: str, step_name: str = "step") -> return None -def merge_worktree(worktree_path: str, project_path: str, max_retries: int = 0, retry_delay_s: int = 15) -> dict: +def merge_worktree(worktree_path: str, project_path: str) -> 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) @@ -92,60 +88,50 @@ def merge_worktree(worktree_path: str, project_path: str, max_retries: int = 0, ) commit_had_changes = commit_result.returncode == 0 - 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"], + 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, ) - conflicts = [f.strip() for f in conflict_result.stdout.splitlines() if f.strip()] + 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} - subprocess.run( - [git, "merge", "--abort"], - cwd=project_path, - capture_output=True, - timeout=10, - ) + # 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": []} - 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 + # 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()] - _logger.warning("Merge conflict in worktree %s: %s", branch_name, conflicts) - return {"success": False, "conflicts": conflicts, "merged_files": []} + 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": []} 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 deleted file mode 100644 index 26de613..0000000 --- a/tests/test_KIN-145_regression.py +++ /dev/null @@ -1,255 +0,0 @@ -"""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/components/EscalationBanner.vue b/web/frontend/src/components/EscalationBanner.vue index 09c9f9f..043f0ba 100644 --- a/web/frontend/src/components/EscalationBanner.vue +++ b/web/frontend/src/components/EscalationBanner.vue @@ -2,7 +2,7 @@ import { ref, computed, onMounted, onUnmounted } from 'vue' import { useI18n } from 'vue-i18n' import { useRouter } from 'vue-router' -import { api, ApiError, type EscalationNotification, type Task } from '../api' +import { api, type EscalationNotification, type Task } from '../api' const { t, locale } = useI18n() const router = useRouter() @@ -186,28 +186,6 @@ function cancelRevise() { reviseComment.value = '' } -const restartingTaskId = ref(null) -const restartError = ref(null) - -async function restartTask(taskId: string) { - if (!confirm(t('escalation.restart_confirm', { task_id: taskId }))) return - restartingTaskId.value = taskId - restartError.value = null - try { - await api.runTask(taskId) - dismiss(taskId) - } catch (e: any) { - if (e instanceof ApiError && e.code === 'task_already_running') { - restartError.value = t('escalation.restart_already_running') - } else { - restartError.value = e.message - } - setTimeout(() => { restartError.value = null }, 4000) - } finally { - restartingTaskId.value = null - } -} - onMounted(async () => { await load() pollTimer = setInterval(load, 10000) @@ -304,30 +282,15 @@ onUnmounted(() => {

{{ n.reason }}

{{ formatTime(n.blocked_at) }}

-
- - -
+ - -
- {{ restartError }} -
-
diff --git a/web/frontend/src/locales/en.json b/web/frontend/src/locales/en.json index 7bd5e04..bc034c1 100644 --- a/web/frontend/src/locales/en.json +++ b/web/frontend/src/locales/en.json @@ -243,10 +243,7 @@ "revise_comment_placeholder": "Revision comment...", "revise_send": "Send", "revise_cancel": "Cancel", - "revise_default_comment": "Sent for revision", - "restart": "▶ Restart", - "restart_confirm": "Restart pipeline for task {task_id}?", - "restart_already_running": "Pipeline is already running" + "revise_default_comment": "Sent for revision" }, "liveConsole": { "hide_log": "▲ Скрыть лог", diff --git a/web/frontend/src/locales/ru.json b/web/frontend/src/locales/ru.json index 2f6505d..a4967a7 100644 --- a/web/frontend/src/locales/ru.json +++ b/web/frontend/src/locales/ru.json @@ -243,10 +243,7 @@ "revise_comment_placeholder": "Комментарий к доработке...", "revise_send": "Отправить", "revise_cancel": "Отмена", - "revise_default_comment": "Отправлено на доработку", - "restart": "▶ Перезапустить", - "restart_confirm": "Перезапустить pipeline для задачи {task_id}?", - "restart_already_running": "Pipeline уже запущен" + "revise_default_comment": "Отправлено на доработку" }, "liveConsole": { "hide_log": "▲ Скрыть лог", diff --git a/web/frontend/src/views/Dashboard.vue b/web/frontend/src/views/Dashboard.vue index 8d2794e..1966342 100644 --- a/web/frontend/src/views/Dashboard.vue +++ b/web/frontend/src/views/Dashboard.vue @@ -1,5 +1,5 @@