256 lines
12 KiB
Python
256 lines
12 KiB
Python
|
|
"""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!"
|
|||
|
|
)
|