From 12d95b2e137b767bd35bb8dd77ebf63e10f26b13 Mon Sep 17 00:00:00 2001 From: Gros Frumos Date: Tue, 17 Mar 2026 16:02:47 +0200 Subject: [PATCH] kin: auto-commit after pipeline --- core/db.py | 8 + tests/test_kin_arch_014_regression.py | 288 +++++++++++++++++++++++++ tests/test_qa_gaps.py | 15 +- web/frontend/src/views/ProjectView.vue | 16 ++ 4 files changed, 318 insertions(+), 9 deletions(-) create mode 100644 tests/test_kin_arch_014_regression.py diff --git a/core/db.py b/core/db.py index cebb6f0..74d01df 100644 --- a/core/db.py +++ b/core/db.py @@ -660,6 +660,14 @@ def _migrate(conn: sqlite3.Connection): ) conn.commit() + # Fix orphaned dept_sub pipelines left in 'running' state due to double-create bug + # (KIN-ARCH-012): before the fix, _execute_department_head_step created a pipeline + # that was never updated, leaving ghost 'running' records in prod DB. + conn.execute( + "UPDATE pipelines SET status = 'failed' WHERE route_type = 'dept_sub' AND status = 'running'" + ) + conn.commit() + def _seed_default_hooks(conn: sqlite3.Connection): """Seed default hooks for the kin project (idempotent). diff --git a/tests/test_kin_arch_014_regression.py b/tests/test_kin_arch_014_regression.py new file mode 100644 index 0000000..bf98785 --- /dev/null +++ b/tests/test_kin_arch_014_regression.py @@ -0,0 +1,288 @@ +""" +Regression tests for KIN-ARCH-014: +blocked_reason из dept_result должен корректно пробрасываться в БД при неуспешном sub-pipeline. + +Два уровня цепочки (convention #305 — отдельный тест на каждый уровень): + +Уровень 1: _execute_department_head_step должен включать blocked_reason из sub_result + в возвращаемый dict. Без фикса — dict возвращался без этого поля. + +Уровень 2: run_pipeline должен передавать dept_result.blocked_reason в update_task + (а не generic 'Department X sub-pipeline failed'). +""" + +import json +import pytest +from unittest.mock import patch, MagicMock + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + +@pytest.fixture +def conn(): + from core.db import init_db + from core import models + c = init_db(":memory:") + models.create_project(c, "proj1", "Project1", "/tmp/proj1", tech_stack=["python"]) + models.create_task(c, "PROJ1-001", "proj1", "Parent task", + brief={"route_type": "department"}) + yield c + c.close() + + +def _dept_head_result_with_sub_pipeline(): + """Валидный вывод dept head с sub_pipeline — используется в обоих уровнях.""" + return { + "raw_output": json.dumps({ + "sub_pipeline": [ + {"role": "backend_dev", "brief": "implement feature"} + ], + "artifacts": {}, + "handoff_notes": "proceed", + }) + } + + +def _failing_sub_result(blocked_reason=None, error=None): + return { + "success": False, + "blocked_reason": blocked_reason, + "error": error, + "total_cost_usd": 0, + "total_tokens": 0, + "total_duration_seconds": 0, + "steps_completed": 0, + "results": [], + "pipeline_id": None, + } + + +def _mock_agent_success(output="done"): + """Мок успешного subprocess.run для run_agent.""" + m = MagicMock() + m.stdout = json.dumps({"result": output}) + m.stderr = "" + m.returncode = 0 + return m + + +# --------------------------------------------------------------------------- +# Уровень 1: _execute_department_head_step propagates blocked_reason +# Convention #304: имя теста описывает сломанное поведение +# --------------------------------------------------------------------------- + +class TestDeptHeadStepBlockedReasonPropagation: + """Уровень 1 цепочки: _execute_department_head_step → sub_result → returned dict.""" + + def test_blocked_reason_dropped_from_returned_dict_when_sub_pipeline_fails(self, conn): + """Broken behavior: _execute_department_head_step отбрасывал blocked_reason + из sub_result — возвращаемый dict не содержал этого поля. + + Fixed (KIN-ARCH-014): blocked_reason из sub_result прокидывается в dict. + """ + from agents.runner import _execute_department_head_step + + failing = _failing_sub_result( + blocked_reason="Конкретная причина: tester заблокировал задачу", + error="TestError: endpoint not found", + ) + + with patch("agents.runner.run_pipeline", return_value=failing), \ + patch("agents.runner.models.create_pipeline", return_value={"id": 99}), \ + patch("agents.runner.models.create_handoff"): + result = _execute_department_head_step( + conn=conn, + task_id="PROJ1-001", + project_id="proj1", + parent_pipeline_id=None, + step={"role": "backend_head", "brief": "plan"}, + dept_head_result=_dept_head_result_with_sub_pipeline(), + ) + + assert result["success"] is False + assert "blocked_reason" in result, ( + "blocked_reason должен быть в возвращаемом dict " + "— без фикса KIN-ARCH-014 это поле отсутствовало" + ) + assert result["blocked_reason"] == "Конкретная причина: tester заблокировал задачу" + + def test_error_field_used_as_fallback_when_blocked_reason_is_none(self, conn): + """Если sub_result.blocked_reason is None, но есть error — + error должен использоваться как blocked_reason (fallback). + """ + from agents.runner import _execute_department_head_step + + failing = _failing_sub_result( + blocked_reason=None, + error="RuntimeError: Claude CLI exited with code 1", + ) + + with patch("agents.runner.run_pipeline", return_value=failing), \ + patch("agents.runner.models.create_pipeline", return_value={"id": 99}), \ + patch("agents.runner.models.create_handoff"): + result = _execute_department_head_step( + conn=conn, + task_id="PROJ1-001", + project_id="proj1", + parent_pipeline_id=None, + step={"role": "backend_head", "brief": "plan"}, + dept_head_result=_dept_head_result_with_sub_pipeline(), + ) + + assert result["success"] is False + assert result.get("blocked_reason") == "RuntimeError: Claude CLI exited with code 1" + + def test_success_result_does_not_inject_blocked_reason(self, conn): + """При успешном sub_result blocked_reason не добавляется в dict.""" + from agents.runner import _execute_department_head_step + + success_sub_result = { + "success": True, + "total_cost_usd": 0.01, + "total_tokens": 100, + "total_duration_seconds": 5.0, + "steps_completed": 1, + "results": [{"role": "backend_dev", "output": "done"}], + "pipeline_id": 99, + } + + with patch("agents.runner.run_pipeline", return_value=success_sub_result), \ + patch("agents.runner.models.create_pipeline", return_value={"id": 99}), \ + patch("agents.runner.models.create_handoff"): + result = _execute_department_head_step( + conn=conn, + task_id="PROJ1-001", + project_id="proj1", + parent_pipeline_id=None, + step={"role": "backend_head", "brief": "plan"}, + dept_head_result=_dept_head_result_with_sub_pipeline(), + ) + + assert result["success"] is True + assert not result.get("blocked_reason"), ( + "blocked_reason не должен появляться при успешном sub_result" + ) + + +# --------------------------------------------------------------------------- +# Уровень 2: run_pipeline passes dept_result.blocked_reason to update_task +# Convention #305: отдельный класс для каждого уровня +# --------------------------------------------------------------------------- + +class TestRunPipelineDeptBlockedReasonSavedToDb: + """Уровень 2 цепочки: run_pipeline → dept_result → models.update_task → БД.""" + + @patch("agents.runner._run_autocommit") + @patch("agents.runner._execute_department_head_step") + @patch("agents.runner._is_department_head") + @patch("agents.runner.subprocess.run") + @patch("agents.runner.check_claude_auth") + def test_generic_error_msg_saved_instead_of_dept_blocked_reason( + self, mock_auth, mock_run, mock_is_dept, mock_dept_step, mock_autocommit, conn + ): + """Broken behavior: run_pipeline сохранял в БД generic строку + 'Department X sub-pipeline failed' вместо реального blocked_reason из dept_result. + + Fixed (KIN-ARCH-014): в БД сохраняется dept_result["blocked_reason"]. + """ + from agents.runner import run_pipeline + from core import models + + mock_run.return_value = _mock_agent_success() + mock_is_dept.return_value = True + mock_dept_step.return_value = { + "success": False, + "blocked_reason": "Специфическая причина: backend_dev упал с OOM", + "error": "OOMError", + "output": "", + "cost_usd": 0, + "tokens_used": 0, + "duration_seconds": 0, + } + + steps = [{"role": "backend_head", "brief": "plan the work"}] + result = run_pipeline(conn, "PROJ1-001", steps) + + assert result["success"] is False + task = models.get_task(conn, "PROJ1-001") + assert task["status"] == "blocked" + assert task["blocked_reason"] == "Специфическая причина: backend_dev упал с OOM", ( + f"Ожидался реальный blocked_reason из dept_result, " + f"получено: {task['blocked_reason']!r}" + ) + + @patch("agents.runner._run_autocommit") + @patch("agents.runner._execute_department_head_step") + @patch("agents.runner._is_department_head") + @patch("agents.runner.subprocess.run") + @patch("agents.runner.check_claude_auth") + def test_generic_string_not_saved_when_specific_reason_available( + self, mock_auth, mock_run, mock_is_dept, mock_dept_step, mock_autocommit, conn + ): + """run_pipeline НЕ должен сохранять generic 'Department X sub-pipeline failed' + в blocked_reason когда dept_result содержит конкретный blocked_reason. + """ + from agents.runner import run_pipeline + from core import models + + specific_reason = "Tester blocked: API endpoint /api/users не реализован" + mock_run.return_value = _mock_agent_success() + mock_is_dept.return_value = True + mock_dept_step.return_value = { + "success": False, + "blocked_reason": specific_reason, + "error": specific_reason, + "output": "", + "cost_usd": 0, + "tokens_used": 0, + "duration_seconds": 0, + } + + steps = [{"role": "backend_head", "brief": "plan"}] + run_pipeline(conn, "PROJ1-001", steps) + + task = models.get_task(conn, "PROJ1-001") + # Generic строка из old code не должна быть в БД + assert "sub-pipeline failed" not in (task["blocked_reason"] or ""), ( + f"Найдена generic строка в blocked_reason: {task['blocked_reason']!r}. " + f"Ожидался специфичный blocked_reason из dept_result." + ) + assert task["blocked_reason"] == specific_reason + + @patch("agents.runner._run_autocommit") + @patch("agents.runner._execute_department_head_step") + @patch("agents.runner._is_department_head") + @patch("agents.runner.subprocess.run") + @patch("agents.runner.check_claude_auth") + def test_fallback_to_generic_when_dept_result_has_no_blocked_reason( + self, mock_auth, mock_run, mock_is_dept, mock_dept_step, mock_autocommit, conn + ): + """Если dept_result не содержит ни blocked_reason, ни error — + используется fallback generic строка (не KeyError, задача блокируется). + """ + from agents.runner import run_pipeline + from core import models + + mock_run.return_value = _mock_agent_success() + mock_is_dept.return_value = True + mock_dept_step.return_value = { + "success": False, + "blocked_reason": None, + "error": None, + "output": "", + "cost_usd": 0, + "tokens_used": 0, + "duration_seconds": 0, + } + + steps = [{"role": "backend_head", "brief": "plan"}] + result = run_pipeline(conn, "PROJ1-001", steps) + + assert result["success"] is False + task = models.get_task(conn, "PROJ1-001") + assert task["status"] == "blocked" + # Должен быть хоть какой-то blocked_reason (fallback, не None) + assert task["blocked_reason"] is not None + assert len(task["blocked_reason"]) > 0 diff --git a/tests/test_qa_gaps.py b/tests/test_qa_gaps.py index bfce568..1b8ed77 100644 --- a/tests/test_qa_gaps.py +++ b/tests/test_qa_gaps.py @@ -246,11 +246,8 @@ class TestMissingDeptRoutes: route a task exclusively to the infra or research department via route template. """ - def test_infra_head_has_no_dedicated_dept_route(self): - """Confirms infra_head is NOT reachable via any standalone dept_infra route. - - This test documents the gap. It will FAIL once a dept_infra route is added. - """ + def test_infra_head_has_dedicated_dept_route(self): + """dept_infra route exists in specialists.yaml with steps: [infra_head].""" import yaml with open("agents/specialists.yaml") as f: data = yaml.safe_load(f) @@ -258,15 +255,15 @@ class TestMissingDeptRoutes: routes = data["routes"] dept_routes = {k: v for k, v in routes.items() if k.startswith("dept_")} - # Check no route exclusively using infra_head as its only step infra_only_routes = [ name for name, route in dept_routes.items() if route["steps"] == ["infra_head"] ] - assert len(infra_only_routes) == 0, ( - f"Found unexpected dept_infra route(s): {infra_only_routes}. " - "Update this test if route was intentionally added." + assert len(infra_only_routes) == 1, ( + f"Expected exactly one dept_infra route with steps=[infra_head], " + f"found: {infra_only_routes}" ) + assert "dept_infra" in infra_only_routes def test_research_head_has_no_dedicated_dept_route(self): """Confirms research_head is NOT reachable via any standalone dept_research route. diff --git a/web/frontend/src/views/ProjectView.vue b/web/frontend/src/views/ProjectView.vue index 6cde750..0326d52 100644 --- a/web/frontend/src/views/ProjectView.vue +++ b/web/frontend/src/views/ProjectView.vue @@ -774,6 +774,14 @@ async function addDecision() { :title="autocommit ? 'Autocommit: on — git commit after pipeline' : 'Autocommit: off'"> {{ autocommit ? '✓ Autocommit' : 'Autocommit' }} + +