From bfc8f1c0bb2a1b774cdb99781bd7c4090865a2e5 Mon Sep 17 00:00:00 2001 From: Gros Frumos Date: Mon, 16 Mar 2026 15:48:09 +0200 Subject: [PATCH] =?UTF-8?q?kin:=20KIN-083=20Healthcheck=20claude=20CLI=20a?= =?UTF-8?q?uth:=20=D0=BF=D0=B5=D1=80=D0=B5=D0=B4=20=D0=B7=D0=B0=D0=BF?= =?UTF-8?q?=D1=83=D1=81=D0=BA=D0=BE=D0=BC=20pipeline=20=D0=BF=D1=80=D0=BE?= =?UTF-8?q?=D0=B2=D0=B5=D1=80=D1=8F=D1=82=D1=8C=20=D1=87=D1=82=D0=BE=20cla?= =?UTF-8?q?ude=20=D0=B7=D0=B0=D0=BB=D0=BE=D0=B3=D0=B8=D0=BD=D0=B5=D0=BD=20?= =?UTF-8?q?(=D0=B1=D1=8B=D1=81=D1=82=D1=80=D1=8B=D0=B9=20claude=20-p=20'ok?= =?UTF-8?q?'=20--output-format=20json,=20=D0=BF=D1=80=D0=BE=D0=B2=D0=B5?= =?UTF-8?q?=D1=80=D0=B8=D1=82=D1=8C=20is=5Ferror=20=D0=B8=20'Not=20logged?= =?UTF-8?q?=20in').=20=D0=95=D1=81=D0=BB=D0=B8=20=D0=BD=D0=B5=20=D0=B7?= =?UTF-8?q?=D0=B0=D0=BB=D0=BE=D0=B3=D0=B8=D0=BD=D0=B5=D0=BD=20=E2=80=94=20?= =?UTF-8?q?=D0=BD=D0=B5=20=D0=B7=D0=B0=D0=BF=D1=83=D1=81=D0=BA=D0=B0=D1=82?= =?UTF-8?q?=D1=8C=20pipeline,=20=D0=B0=20=D0=BF=D0=BE=D0=BA=D0=B0=D0=B7?= =?UTF-8?q?=D0=B0=D1=82=D1=8C=20=D0=BE=D1=88=D0=B8=D0=B1=D0=BA=D1=83=20'Cl?= =?UTF-8?q?aude=20CLI=20requires=20login'=20=D0=B2=20GUI=20=D1=81=20=D0=B8?= =?UTF-8?q?=D0=BD=D1=81=D1=82=D1=80=D1=83=D0=BA=D1=86=D0=B8=D0=B5=D0=B9.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- agents/bootstrap.py | 2 +- agents/runner.py | 109 +++++- core/db.py | 34 +- core/hooks.py | 16 +- core/models.py | 28 +- scripts/rebuild-frontend.sh | 13 +- tests/conftest.py | 20 + tests/test_api.py | 112 ++++++ tests/test_auto_mode.py | 6 +- tests/test_bootstrap.py | 20 + tests/test_hooks.py | 254 +++++++++++- tests/test_models.py | 81 ++++ tests/test_runner.py | 370 +++++++++++++++++- web/api.py | 35 +- .../src/__tests__/claude-auth.test.ts | 277 +++++++++++++ web/frontend/src/api.ts | 26 +- web/frontend/src/views/ProjectView.vue | 21 +- web/frontend/src/views/TaskDetail.vue | 23 +- 18 files changed, 1390 insertions(+), 57 deletions(-) create mode 100644 tests/conftest.py create mode 100644 web/frontend/src/__tests__/claude-auth.test.ts diff --git a/agents/bootstrap.py b/agents/bootstrap.py index ecd79d7..3d6430e 100644 --- a/agents/bootstrap.py +++ b/agents/bootstrap.py @@ -213,7 +213,7 @@ def detect_modules(project_path: Path) -> list[dict]: if not child.is_dir() or child.name in _SKIP_DIRS or child.name.startswith("."): continue mod = _analyze_module(child, project_path) - key = (mod["name"], mod["path"]) + key = mod["name"] if key not in seen: seen.add(key) modules.append(mod) diff --git a/agents/runner.py b/agents/runner.py index c3d500f..2c2f66e 100644 --- a/agents/runner.py +++ b/agents/runner.py @@ -68,6 +68,54 @@ from core.context_builder import build_context, format_prompt from core.hooks import run_hooks +class ClaudeAuthError(Exception): + """Raised when Claude CLI is not authenticated or not available.""" + pass + + +def check_claude_auth(timeout: int = 10) -> None: + """Check that claude CLI is authenticated before running a pipeline. + + Runs: claude -p 'ok' --output-format json --no-verbose with timeout. + Returns None if auth is confirmed. + Raises ClaudeAuthError if: + - claude CLI not found in PATH (FileNotFoundError) + - stdout/stderr contains 'not logged in' (case-insensitive) + - returncode != 0 + - is_error=true in parsed JSON output + Returns silently on TimeoutExpired (ambiguous — don't block pipeline). + """ + claude_cmd = _resolve_claude_cmd() + env = _build_claude_env() + try: + proc = subprocess.run( + [claude_cmd, "-p", "ok", "--output-format", "json", "--no-verbose"], + capture_output=True, + text=True, + timeout=timeout, + env=env, + stdin=subprocess.DEVNULL, + ) + except FileNotFoundError: + raise ClaudeAuthError("claude CLI not found in PATH. Install it or add to PATH.") + except subprocess.TimeoutExpired: + return # Ambiguous — don't block pipeline on timeout + + stdout = proc.stdout or "" + stderr = proc.stderr or "" + combined = stdout + stderr + + if "not logged in" in combined.lower(): + raise ClaudeAuthError("Claude CLI requires login. Run: claude login") + + if proc.returncode != 0: + raise ClaudeAuthError("Claude CLI requires login. Run: claude login") + + parsed = _try_parse_json(stdout) + if isinstance(parsed, dict) and parsed.get("is_error"): + raise ClaudeAuthError("Claude CLI requires login. Run: claude login") + + def run_agent( conn: sqlite3.Connection, role: str, @@ -467,6 +515,37 @@ def _is_permission_error(result: dict) -> bool: # Autocommit: git add -A && git commit after successful pipeline # --------------------------------------------------------------------------- +def _get_changed_files(project_path: str) -> list[str]: + """Return files changed in the current pipeline run. + + Combines unstaged changes, staged changes, and the last commit diff + to cover both autocommit-on and autocommit-off scenarios. + Returns [] on any git error (e.g. no git repo, first commit). + """ + env = _build_claude_env() + git_cmd = shutil.which("git", path=env["PATH"]) or "git" + files: set[str] = set() + for git_args in ( + ["diff", "--name-only"], # unstaged tracked changes + ["diff", "--cached", "--name-only"], # staged changes + ["diff", "HEAD~1", "HEAD", "--name-only"], # last commit (post-autocommit) + ): + try: + r = subprocess.run( + [git_cmd] + git_args, + cwd=project_path, + capture_output=True, + text=True, + timeout=10, + env=env, + ) + if r.returncode == 0: + files.update(f.strip() for f in r.stdout.splitlines() if f.strip()) + except Exception: + pass + return list(files) + + def _run_autocommit( conn: sqlite3.Connection, task_id: str, @@ -582,7 +661,7 @@ def _save_sysadmin_output( if not m_name: continue try: - models.add_module( + m = models.add_module( conn, project_id=project_id, name=m_name, @@ -591,7 +670,10 @@ def _save_sysadmin_output( description=item.get("description"), owner_role="sysadmin", ) - modules_added += 1 + if m.get("_created", True): + modules_added += 1 + else: + modules_skipped += 1 except Exception: modules_skipped += 1 @@ -739,6 +821,18 @@ def run_pipeline( Returns {success, steps_completed, total_cost, total_tokens, total_duration, results} """ + # Auth check — skip for dry_run (dry_run never calls claude CLI) + if not dry_run: + try: + check_claude_auth() + except ClaudeAuthError as exc: + return { + "success": False, + "error": "claude_auth_required", + "message": str(exc), + "instructions": "Run: claude login", + } + task = models.get_task(conn, task_id) if not task: return {"success": False, "error": f"Task '{task_id}' not found"} @@ -979,6 +1073,14 @@ def run_pipeline( task_modules = models.get_modules(conn, project_id) + # Compute changed files for hook filtering (frontend build trigger) + changed_files: list[str] | None = None + project = models.get_project(conn, project_id) + if project and project.get("path"): + p_path = Path(project["path"]).expanduser() + if p_path.is_dir(): + changed_files = _get_changed_files(str(p_path)) + last_role = steps[-1].get("role", "") if steps else "" auto_eligible = last_role in {"tester", "reviewer"} @@ -1018,7 +1120,8 @@ def run_pipeline( # Run post-pipeline hooks (failures don't affect pipeline status) try: run_hooks(conn, project_id, task_id, - event="pipeline_completed", task_modules=task_modules) + event="pipeline_completed", task_modules=task_modules, + changed_files=changed_files) except Exception: pass # Hook errors must never block pipeline completion diff --git a/core/db.py b/core/db.py index 19fe3b3..8a782ca 100644 --- a/core/db.py +++ b/core/db.py @@ -457,13 +457,20 @@ def _seed_default_hooks(conn: sqlite3.Connection): Creates rebuild-frontend hook only when: - project 'kin' exists in the projects table - the hook doesn't already exist (no duplicate) + + Also updates existing hooks to the correct command/config if outdated. """ - kin_exists = conn.execute( - "SELECT 1 FROM projects WHERE id = 'kin'" + kin_row = conn.execute( + "SELECT path FROM projects WHERE id = 'kin'" ).fetchone() - if not kin_exists: + if not kin_row or not kin_row["path"]: return + _PROJECT_PATH = kin_row["path"].rstrip("/") + _REBUILD_SCRIPT = f"{_PROJECT_PATH}/scripts/rebuild-frontend.sh" + _REBUILD_TRIGGER = "web/frontend/*" + _REBUILD_WORKDIR = _PROJECT_PATH + exists = conn.execute( "SELECT 1 FROM hooks" " WHERE project_id = 'kin'" @@ -472,12 +479,25 @@ def _seed_default_hooks(conn: sqlite3.Connection): ).fetchone() if not exists: conn.execute( - """INSERT INTO hooks (project_id, name, event, command, enabled) + """INSERT INTO hooks + (project_id, name, event, trigger_module_path, command, + working_dir, timeout_seconds, enabled) VALUES ('kin', 'rebuild-frontend', 'pipeline_completed', - 'cd /Users/grosfrumos/projects/kin/web/frontend && npm run build', - 1)""" + ?, ?, ?, 300, 1)""", + (_REBUILD_TRIGGER, _REBUILD_SCRIPT, _REBUILD_WORKDIR), ) - conn.commit() + else: + # Migrate existing hook: set trigger_module_path, correct command, working_dir + conn.execute( + """UPDATE hooks + SET trigger_module_path = ?, + command = ?, + working_dir = ?, + timeout_seconds = 300 + WHERE project_id = 'kin' AND name = 'rebuild-frontend'""", + (_REBUILD_TRIGGER, _REBUILD_SCRIPT, _REBUILD_WORKDIR), + ) + conn.commit() # Enable autocommit for kin project (opt-in, idempotent) conn.execute( diff --git a/core/hooks.py b/core/hooks.py index c68df47..b4adc2b 100644 --- a/core/hooks.py +++ b/core/hooks.py @@ -115,9 +115,14 @@ def run_hooks( task_id: str | None, event: str, task_modules: list[dict], + changed_files: list[str] | None = None, ) -> list[HookResult]: """Run matching hooks for the given event and module list. + If changed_files is provided, trigger_module_path is matched against + the actual git-changed file paths (more precise than task_modules). + Falls back to task_modules matching when changed_files is None. + Never raises — hook failures are logged but don't affect the pipeline. """ hooks = get_hooks(conn, project_id, event=event) @@ -125,10 +130,13 @@ def run_hooks( for hook in hooks: if hook["trigger_module_path"] is not None: pattern = hook["trigger_module_path"] - matched = any( - fnmatch.fnmatch(m.get("path", ""), pattern) - for m in task_modules - ) + if changed_files is not None: + matched = any(fnmatch.fnmatch(f, pattern) for f in changed_files) + else: + matched = any( + fnmatch.fnmatch(m.get("path", ""), pattern) + for m in task_modules + ) if not matched: continue diff --git a/core/models.py b/core/models.py index ceb1672..cbdeb72 100644 --- a/core/models.py +++ b/core/models.py @@ -99,6 +99,15 @@ def get_project(conn: sqlite3.Connection, id: str) -> dict | None: return _row_to_dict(row) +def delete_project(conn: sqlite3.Connection, id: str) -> None: + """Delete a project and all its related data (modules, decisions, tasks).""" + # Delete tables that have FK references to tasks BEFORE deleting tasks + for table in ("modules", "agent_logs", "decisions", "pipelines", "tasks"): + conn.execute(f"DELETE FROM {table} WHERE project_id = ?", (id,)) + conn.execute("DELETE FROM projects WHERE id = ?", (id,)) + conn.commit() + + def get_effective_mode(conn: sqlite3.Connection, project_id: str, task_id: str) -> str: """Return effective execution mode: 'auto' or 'review'. @@ -381,17 +390,26 @@ def add_module( ) -> dict: """Register a project module.""" cur = conn.execute( - """INSERT INTO modules (project_id, name, type, path, description, + """INSERT OR IGNORE INTO modules (project_id, name, type, path, description, owner_role, dependencies) VALUES (?, ?, ?, ?, ?, ?, ?)""", (project_id, name, type, path, description, owner_role, _json_encode(dependencies)), ) + created = cur.rowcount > 0 conn.commit() - row = conn.execute( - "SELECT * FROM modules WHERE id = ?", (cur.lastrowid,) - ).fetchone() - return _row_to_dict(row) + if cur.lastrowid: + row = conn.execute( + "SELECT * FROM modules WHERE id = ?", (cur.lastrowid,) + ).fetchone() + else: + row = conn.execute( + "SELECT * FROM modules WHERE project_id = ? AND name = ?", + (project_id, name), + ).fetchone() + result = _row_to_dict(row) + result["_created"] = created + return result def get_modules(conn: sqlite3.Connection, project_id: str) -> list[dict]: diff --git a/scripts/rebuild-frontend.sh b/scripts/rebuild-frontend.sh index 19b9ea6..b21ae1c 100755 --- a/scripts/rebuild-frontend.sh +++ b/scripts/rebuild-frontend.sh @@ -19,20 +19,13 @@ npm run build echo "[rebuild-frontend] Build complete." # Restart API server if it's currently running. +# API is managed by launchctl with KeepAlive=true — just kill it, launchctl restarts it. # pgrep returns 1 if no match; || true prevents set -e from exiting. API_PID=$(pgrep -f "uvicorn web.api" 2>/dev/null || true) if [ -n "$API_PID" ]; then - echo "[rebuild-frontend] Stopping API server (PID: $API_PID) ..." + echo "[rebuild-frontend] Restarting API server (PID: $API_PID) — launchctl will auto-restart ..." kill "$API_PID" 2>/dev/null || true - # Wait for port 8420 to free up (up to 5 s) - for i in $(seq 1 5); do - pgrep -f "uvicorn web.api" > /dev/null 2>&1 || break - sleep 1 - done - echo "[rebuild-frontend] Starting API server ..." - cd "$PROJECT_ROOT" - nohup python -m uvicorn web.api:app --port 8420 >> /tmp/kin-api.log 2>&1 & - echo "[rebuild-frontend] API server started (PID: $!)." + echo "[rebuild-frontend] API server restarted (launchctl KeepAlive=true)." else echo "[rebuild-frontend] API server not running; skipping restart." fi diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..ff03a3b --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,20 @@ +"""Shared pytest fixtures for Kin test suite.""" + +import pytest +from unittest.mock import patch + + +@pytest.fixture(autouse=True) +def _mock_check_claude_auth(): + """Авто-мок agents.runner.check_claude_auth для всех тестов. + + run_pipeline() вызывает check_claude_auth() перед запуском агентов. + Без мока тесты, использующие side_effect-очереди для subprocess.run, + ломаются: первый вызов (auth-check) потребляет элемент очереди. + + Тесты TestCheckClaudeAuth (test_runner.py) НЕ затрагиваются: + они вызывают check_claude_auth через напрямую импортированную ссылку + (bound at module load time), а не через agents.runner.check_claude_auth. + """ + with patch("agents.runner.check_claude_auth"): + yield diff --git a/tests/test_api.py b/tests/test_api.py index e1c92f5..3503e9a 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -2,11 +2,13 @@ import pytest from pathlib import Path +from unittest.mock import patch, MagicMock from fastapi.testclient import TestClient # Patch DB_PATH before importing app import web.api as api_module + @pytest.fixture def client(tmp_path): db_path = tmp_path / "test.db" @@ -224,6 +226,30 @@ def test_run_not_found(client): assert r.status_code == 404 +def test_run_returns_503_when_claude_not_authenticated(client): + """KIN-083: /run возвращает 503 с claude_auth_required если claude не залогинен.""" + from agents.runner import ClaudeAuthError + with patch("agents.runner.check_claude_auth", side_effect=ClaudeAuthError("Claude CLI requires login. Run: claude login")): + r = client.post("/api/tasks/P1-001/run") + assert r.status_code == 503 + body = r.json() + assert body["detail"]["error"] == "claude_auth_required" + assert body["detail"]["instructions"] == "Run: claude login" + assert "login" in body["detail"]["message"].lower() + + +def test_start_phase_returns_503_when_claude_not_authenticated(client): + """KIN-083: /phases/start возвращает 503 с claude_auth_required если claude не залогинен.""" + from agents.runner import ClaudeAuthError + with patch("agents.runner.check_claude_auth", side_effect=ClaudeAuthError("Claude CLI requires login. Run: claude login")): + r = client.post("/api/projects/p1/phases/start") + assert r.status_code == 503 + body = r.json() + assert body["detail"]["error"] == "claude_auth_required" + assert body["detail"]["instructions"] == "Run: claude login" + assert "login" in body["detail"]["message"].lower() + + def test_run_kin_038_without_allow_write(client): """Регрессионный тест KIN-038: allow_write удалён из схемы, эндпоинт принимает запросы с пустым телом без этого параметра.""" @@ -1583,3 +1609,89 @@ def test_kin_arch_003_deploy_operations_project_null_path_uses_cwd_none(client): assert call_kwargs.get("cwd") is None, ( "KIN-ARCH-003: для operations-проектов без path, cwd должен быть None" ) + + +# --------------------------------------------------------------------------- +# Bootstrap endpoint — KIN-081 +# --------------------------------------------------------------------------- + +@pytest.fixture +def bootstrap_client(tmp_path): + """TestClient без seed-данных, с отдельным DB_PATH.""" + db_path = tmp_path / "bs_test.db" + api_module.DB_PATH = db_path + from web.api import app + return TestClient(app), tmp_path + + +def test_bootstrap_endpoint_invalid_path_returns_400(bootstrap_client): + """KIN-081: bootstrap возвращает 400 если путь не существует.""" + client, _ = bootstrap_client + r = client.post("/api/bootstrap", json={ + "id": "newproj", "name": "New Project", "path": "/nonexistent/path/that/does/not/exist" + }) + assert r.status_code == 400 + assert "not a directory" in r.json()["detail"].lower() + + +def test_bootstrap_endpoint_duplicate_id_returns_409(bootstrap_client, tmp_path): + """KIN-081: bootstrap возвращает 409 если проект с таким ID уже существует.""" + client, _ = bootstrap_client + proj_dir = tmp_path / "myproj" + proj_dir.mkdir() + # Create project first + client.post("/api/projects", json={"id": "existing", "name": "Existing", "path": str(proj_dir)}) + # Try bootstrap with same ID + r = client.post("/api/bootstrap", json={ + "id": "existing", "name": "Same ID", "path": str(proj_dir) + }) + assert r.status_code == 409 + assert "already exists" in r.json()["detail"] + + +def test_bootstrap_endpoint_rollback_on_save_error(bootstrap_client, tmp_path): + """KIN-081: при ошибке в save_to_db проект удаляется (rollback), возвращается 500.""" + client, _ = bootstrap_client + proj_dir = tmp_path / "rollbackproj" + proj_dir.mkdir() + + from core.db import init_db + from core import models as _models + + def _save_create_then_fail(conn, project_id, name, path, *args, **kwargs): + # Simulate partial write: project row created, then error + _models.create_project(conn, project_id, name, path) + raise RuntimeError("simulated DB error after project created") + + with patch("web.api.save_to_db", side_effect=_save_create_then_fail): + r = client.post("/api/bootstrap", json={ + "id": "rollbackproj", "name": "Rollback Test", "path": str(proj_dir) + }) + + assert r.status_code == 500 + assert "Bootstrap failed" in r.json()["detail"] + + # Project must NOT remain in DB (rollback was executed) + conn = init_db(api_module.DB_PATH) + assert _models.get_project(conn, "rollbackproj") is None + conn.close() + + +def test_bootstrap_endpoint_success(bootstrap_client, tmp_path): + """KIN-081: успешный bootstrap возвращает 200 с project и counts.""" + client, _ = bootstrap_client + proj_dir = tmp_path / "goodproj" + proj_dir.mkdir() + (proj_dir / "requirements.txt").write_text("fastapi\n") + + with patch("web.api.find_vault_root", return_value=None): + r = client.post("/api/bootstrap", json={ + "id": "goodproj", "name": "Good Project", "path": str(proj_dir) + }) + + assert r.status_code == 200 + data = r.json() + assert data["project"]["id"] == "goodproj" + assert "modules_count" in data + assert "decisions_count" in data + assert "tasks_count" in data diff --git a/tests/test_auto_mode.py b/tests/test_auto_mode.py index eb73463..2799d6a 100644 --- a/tests/test_auto_mode.py +++ b/tests/test_auto_mode.py @@ -179,12 +179,13 @@ class TestAutoApprove: class TestAutoRerunOnPermissionDenied: """Runner повторяет шаг при permission issues, останавливается по лимиту (1 retry).""" + @patch("agents.runner._get_changed_files", return_value=[]) @patch("agents.runner._run_autocommit") @patch("agents.runner._run_learning_extraction") @patch("core.followup.generate_followups") @patch("agents.runner.run_hooks") @patch("agents.runner.subprocess.run") - def test_auto_mode_retries_on_permission_error(self, mock_run, mock_hooks, mock_followup, mock_learn, mock_autocommit, conn): + def test_auto_mode_retries_on_permission_error(self, mock_run, mock_hooks, mock_followup, mock_learn, mock_autocommit, mock_changed_files, conn): """Auto-режим: при permission denied runner делает 1 retry с allow_write=True.""" mock_run.side_effect = [ _mock_permission_denied(), # 1-й вызов: permission error @@ -261,12 +262,13 @@ class TestAutoRerunOnPermissionDenied: task = models.get_task(conn, "VDOL-001") assert task["status"] == "blocked" + @patch("agents.runner._get_changed_files", return_value=[]) @patch("agents.runner._run_autocommit") @patch("agents.runner._run_learning_extraction") @patch("core.followup.generate_followups") @patch("agents.runner.run_hooks") @patch("agents.runner.subprocess.run") - def test_subsequent_steps_use_allow_write_after_retry(self, mock_run, mock_hooks, mock_followup, mock_learn, mock_autocommit, conn): + def test_subsequent_steps_use_allow_write_after_retry(self, mock_run, mock_hooks, mock_followup, mock_learn, mock_autocommit, mock_changed_files, conn): """После успешного retry все следующие шаги тоже используют allow_write.""" mock_run.side_effect = [ _mock_permission_denied(), # Шаг 1: permission error diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py index 20dc5ea..ee61ceb 100644 --- a/tests/test_bootstrap.py +++ b/tests/test_bootstrap.py @@ -114,6 +114,26 @@ def test_detect_modules_empty(tmp_path): assert detect_modules(tmp_path) == [] +def test_detect_modules_deduplication_by_name(tmp_path): + """KIN-081: detect_modules дедуплицирует по имени (не по имени+путь). + + Если два разных scan_dir дают одноимённые модули (например, frontend/src/components + и backend/src/components), результат содержит только первый. + Это соответствует UNIQUE constraint (project_id, name) в таблице modules. + """ + fe_comp = tmp_path / "frontend" / "src" / "components" + fe_comp.mkdir(parents=True) + (fe_comp / "App.vue").write_text("") + + be_comp = tmp_path / "backend" / "src" / "components" + be_comp.mkdir(parents=True) + (be_comp / "Service.ts").write_text("export class Service {}") + + modules = detect_modules(tmp_path) + names = [m["name"] for m in modules] + assert names.count("components") == 1 + + def test_detect_modules_backend_pg(tmp_path): """Test detection in backend-pg/src/ pattern (like vdolipoperek).""" src = tmp_path / "backend-pg" / "src" / "services" diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 6ce38f7..226437c 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -540,6 +540,7 @@ class TestKIN052RebuildFrontendCommand: """Хук должен сохраняться в файловой БД и быть доступен после пересоздания соединения. Симулирует рестарт: создаём хук, закрываем соединение, открываем новое — хук на месте. + Используем проект НЕ 'kin', чтобы _seed_default_hooks не мигрировал хук. """ with tempfile.NamedTemporaryFile(suffix=".db", delete=False) as f: db_path = f.name @@ -547,16 +548,17 @@ class TestKIN052RebuildFrontendCommand: # Первое соединение — создаём проект и хук conn1 = init_db(db_path) from core import models as _models - _models.create_project(conn1, "kin", "Kin", "/projects/kin", tech_stack=["vue3"]) - cmd = "cd /Users/grosfrumos/projects/kin/web/frontend && npm run build" - hook = create_hook(conn1, "kin", "rebuild-frontend", "pipeline_completed", cmd, + _models.create_project(conn1, "kin-test", "KinTest", "/projects/kin-test", + tech_stack=["vue3"]) + cmd = "cd /projects/kin-test/web/frontend && npm run build" + hook = create_hook(conn1, "kin-test", "rebuild-frontend", "pipeline_completed", cmd, trigger_module_path=None) hook_id = hook["id"] conn1.close() # Второе соединение — «рестарт», хук должен быть на месте conn2 = init_db(db_path) - hooks = get_hooks(conn2, "kin", event="pipeline_completed", enabled_only=True) + hooks = get_hooks(conn2, "kin-test", event="pipeline_completed", enabled_only=True) conn2.close() assert len(hooks) == 1, "После пересоздания соединения хук должен оставаться в БД" @@ -595,6 +597,7 @@ class TestKIN053SeedDefaultHooks: """_seed_default_hooks создаёт rebuild-frontend хук при наличии проекта 'kin'. Порядок: init_db → create_project('kin') → повторный init_db → хук есть. + KIN-003: команда теперь scripts/rebuild-frontend.sh, не cd && npm run build. """ with tempfile.NamedTemporaryFile(suffix=".db", delete=False) as f: db_path = f.name @@ -609,28 +612,33 @@ class TestKIN053SeedDefaultHooks: assert len(hooks) == 1 assert hooks[0]["name"] == "rebuild-frontend" - assert "npm run build" in hooks[0]["command"] - assert "web/frontend" in hooks[0]["command"] + assert "rebuild-frontend.sh" in hooks[0]["command"] finally: os.unlink(db_path) def test_seed_hook_has_correct_command(self): - """Команда хука — точная строка с cd && npm run build.""" + """Команда хука использует динамический путь из projects.path (KIN-BIZ-004). + + KIN-003: хук мигрирован на скрипт scripts/rebuild-frontend.sh + с trigger_module_path='web/frontend/*' для точного git-фильтра. + KIN-BIZ-004: путь берётся из projects.path, не захардкожен. + """ + project_path = "/projects/kin" with tempfile.NamedTemporaryFile(suffix=".db", delete=False) as f: db_path = f.name try: conn1 = init_db(db_path) - models.create_project(conn1, "kin", "Kin", "/projects/kin") + models.create_project(conn1, "kin", "Kin", project_path) conn1.close() conn2 = init_db(db_path) hooks = get_hooks(conn2, "kin", event="pipeline_completed", enabled_only=False) conn2.close() - assert hooks[0]["command"] == ( - "cd /Users/grosfrumos/projects/kin/web/frontend && npm run build" - ) - assert hooks[0]["trigger_module_path"] is None + assert hooks[0]["command"] == f"{project_path}/scripts/rebuild-frontend.sh" + assert hooks[0]["trigger_module_path"] == "web/frontend/*" + assert hooks[0]["working_dir"] == project_path + assert hooks[0]["timeout_seconds"] == 300 finally: os.unlink(db_path) @@ -672,3 +680,225 @@ class TestKIN053SeedDefaultHooks: assert other_hooks == [] finally: os.unlink(db_path) + + def test_seed_hook_migration_updates_existing_hook(self): + """_seed_default_hooks мигрирует существующий хук используя динамический путь (KIN-BIZ-004). + + Если rebuild-frontend уже существует со старой командой (cd && npm run build), + повторный init_db должен обновить его на scripts/rebuild-frontend.sh + с путём из projects.path. + """ + project_path = "/projects/kin" + with tempfile.NamedTemporaryFile(suffix=".db", delete=False) as f: + db_path = f.name + try: + conn1 = init_db(db_path) + models.create_project(conn1, "kin", "Kin", project_path) + # Вставляем старый хук вручную (имитация состояния до KIN-003) + old_cmd = f"cd {project_path}/web/frontend && npm run build" + conn1.execute( + """INSERT INTO hooks (project_id, name, event, trigger_module_path, command, + working_dir, timeout_seconds, enabled) + VALUES ('kin', 'rebuild-frontend', 'pipeline_completed', + NULL, ?, NULL, 120, 1)""", + (old_cmd,), + ) + conn1.commit() + conn1.close() + + # Повторный init_db запускает _seed_default_hooks с миграцией + conn2 = init_db(db_path) + hooks = get_hooks(conn2, "kin", event="pipeline_completed", enabled_only=False) + conn2.close() + + assert len(hooks) == 1 + assert hooks[0]["command"] == f"{project_path}/scripts/rebuild-frontend.sh" + assert hooks[0]["trigger_module_path"] == "web/frontend/*" + assert hooks[0]["working_dir"] == project_path + assert hooks[0]["timeout_seconds"] == 300 + finally: + os.unlink(db_path) + + def test_seed_hook_uses_dynamic_path_not_hardcoded(self): + """Команда хука содержит путь из projects.path, а не захардкоженный /Users/grosfrumos/... (KIN-BIZ-004). + + Создаём проект с нестандартным путём и проверяем, + что хук использует именно этот путь. + """ + custom_path = "/srv/custom/kin-deployment" + with tempfile.NamedTemporaryFile(suffix=".db", delete=False) as f: + db_path = f.name + try: + conn1 = init_db(db_path) + models.create_project(conn1, "kin", "Kin", custom_path) + conn1.close() + + conn2 = init_db(db_path) + hooks = get_hooks(conn2, "kin", event="pipeline_completed", enabled_only=False) + conn2.close() + + assert len(hooks) == 1 + assert hooks[0]["command"] == f"{custom_path}/scripts/rebuild-frontend.sh", ( + "Команда должна использовать путь из projects.path, не захардкоженный" + ) + assert hooks[0]["working_dir"] == custom_path, ( + "working_dir должен совпадать с projects.path" + ) + assert "/Users/grosfrumos" not in hooks[0]["command"], ( + "Захардкоженный путь /Users/grosfrumos не должен присутствовать в команде" + ) + finally: + os.unlink(db_path) + + +# --------------------------------------------------------------------------- +# KIN-003: changed_files — точный git-фильтр для trigger_module_path +# --------------------------------------------------------------------------- + +class TestChangedFilesMatching: + """Тесты для нового параметра changed_files в run_hooks() (KIN-003). + + Когда changed_files передан — trigger_module_path матчится по реальным + git-изменённым файлам, а не по task_modules из БД. + """ + + def _make_proc(self, returncode=0, stdout="ok", stderr=""): + m = MagicMock() + m.returncode = returncode + m.stdout = stdout + m.stderr = stderr + return m + + @pytest.fixture + def frontend_trigger_hook(self, conn): + """Хук с trigger_module_path='web/frontend/*'.""" + return create_hook( + conn, "vdol", "rebuild-frontend", "pipeline_completed", + "scripts/rebuild-frontend.sh", + trigger_module_path="web/frontend/*", + working_dir="/tmp", + ) + + @patch("core.hooks.subprocess.run") + def test_hook_fires_when_frontend_file_in_changed_files( + self, mock_run, conn, frontend_trigger_hook + ): + """Хук срабатывает, если среди changed_files есть файл в web/frontend/.""" + mock_run.return_value = self._make_proc() + results = run_hooks( + conn, "vdol", "VDOL-001", + event="pipeline_completed", + task_modules=[], + changed_files=["web/frontend/App.vue", "core/models.py"], + ) + assert len(results) == 1 + assert results[0].name == "rebuild-frontend" + mock_run.assert_called_once() + + @patch("core.hooks.subprocess.run") + def test_hook_skipped_when_no_frontend_file_in_changed_files( + self, mock_run, conn, frontend_trigger_hook + ): + """Хук НЕ срабатывает, если changed_files не содержит web/frontend/* файлов.""" + mock_run.return_value = self._make_proc() + results = run_hooks( + conn, "vdol", "VDOL-001", + event="pipeline_completed", + task_modules=[], + changed_files=["core/models.py", "web/api.py", "agents/runner.py"], + ) + assert len(results) == 0 + mock_run.assert_not_called() + + @patch("core.hooks.subprocess.run") + def test_hook_skipped_when_changed_files_is_empty_list( + self, mock_run, conn, frontend_trigger_hook + ): + """Пустой changed_files [] — хук с trigger_module_path не срабатывает.""" + mock_run.return_value = self._make_proc() + results = run_hooks( + conn, "vdol", "VDOL-001", + event="pipeline_completed", + task_modules=[{"path": "web/frontend/App.vue", "name": "App"}], + changed_files=[], # git говорит: ничего не изменилось + ) + assert len(results) == 0 + mock_run.assert_not_called() + + @patch("core.hooks.subprocess.run") + def test_changed_files_overrides_task_modules_match( + self, mock_run, conn, frontend_trigger_hook + ): + """Если changed_files передан, task_modules игнорируется для фильтрации. + + task_modules содержит frontend-файл, но changed_files — нет. + Хук не должен сработать: changed_files имеет приоритет. + """ + mock_run.return_value = self._make_proc() + results = run_hooks( + conn, "vdol", "VDOL-001", + event="pipeline_completed", + task_modules=[{"path": "web/frontend/App.vue", "name": "App"}], + changed_files=["core/models.py"], # нет frontend-файлов + ) + assert len(results) == 0, ( + "changed_files должен иметь приоритет над task_modules" + ) + mock_run.assert_not_called() + + @patch("core.hooks.subprocess.run") + def test_fallback_to_task_modules_when_changed_files_is_none( + self, mock_run, conn, frontend_trigger_hook + ): + """Если changed_files=None — используется старое поведение через task_modules.""" + mock_run.return_value = self._make_proc() + results = run_hooks( + conn, "vdol", "VDOL-001", + event="pipeline_completed", + task_modules=[{"path": "web/frontend/App.vue", "name": "App"}], + changed_files=None, # не передан — fallback + ) + assert len(results) == 1 + assert results[0].name == "rebuild-frontend" + mock_run.assert_called_once() + + @patch("core.hooks.subprocess.run") + def test_hook_without_trigger_fires_regardless_of_changed_files( + self, mock_run, conn + ): + """Хук без trigger_module_path всегда срабатывает, даже если changed_files=[]. + + Используется для хуков, которые должны запускаться после каждого pipeline. + """ + mock_run.return_value = self._make_proc() + create_hook( + conn, "vdol", "always-run", "pipeline_completed", + "echo always", + trigger_module_path=None, + working_dir="/tmp", + ) + results = run_hooks( + conn, "vdol", "VDOL-001", + event="pipeline_completed", + task_modules=[], + changed_files=[], # пусто — но хук без фильтра всегда запустится + ) + assert len(results) == 1 + assert results[0].name == "always-run" + mock_run.assert_called_once() + + @patch("core.hooks.subprocess.run") + def test_deep_frontend_path_matches_glob( + self, mock_run, conn, frontend_trigger_hook + ): + """Вложенные пути web/frontend/src/components/Foo.vue матчатся по 'web/frontend/*'.""" + mock_run.return_value = self._make_proc() + results = run_hooks( + conn, "vdol", "VDOL-001", + event="pipeline_completed", + task_modules=[], + changed_files=["web/frontend/src/components/TaskCard.vue"], + ) + assert len(results) == 1, ( + "fnmatch должен рекурсивно матчить 'web/frontend/*' на вложенные пути" + ) diff --git a/tests/test_models.py b/tests/test_models.py index 95284e8..32dc7cc 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -280,6 +280,87 @@ def test_add_and_get_modules(conn): assert len(mods) == 1 +def test_add_module_created_true_for_new_module(conn): + """KIN-081: add_module возвращает _created=True для нового модуля (INSERT).""" + models.create_project(conn, "p1", "P1", "/p1") + m = models.add_module(conn, "p1", "api", "backend", "src/api/") + assert m["_created"] is True + assert m["name"] == "api" + + +def test_add_module_created_false_for_duplicate_name(conn): + """KIN-081: add_module возвращает _created=False при дублировании по имени (INSERT OR IGNORE). + + UNIQUE constraint — (project_id, name). Второй INSERT с тем же name игнорируется, + возвращается существующая запись с _created=False. + """ + models.create_project(conn, "p1", "P1", "/p1") + m1 = models.add_module(conn, "p1", "api", "backend", "src/api/") + assert m1["_created"] is True + + # Same name, different path — should be ignored + m2 = models.add_module(conn, "p1", "api", "frontend", "src/api-v2/") + assert m2["_created"] is False + assert m2["name"] == "api" + # Only one module in DB + assert len(models.get_modules(conn, "p1")) == 1 + + +def test_add_module_duplicate_returns_original_row(conn): + """KIN-081: при дублировании add_module возвращает оригинальную запись (не новые данные).""" + models.create_project(conn, "p1", "P1", "/p1") + m1 = models.add_module(conn, "p1", "api", "backend", "src/api/", + description="original desc") + m2 = models.add_module(conn, "p1", "api", "frontend", "src/api-v2/", + description="new desc") + # Should return original row, not updated one + assert m2["type"] == "backend" + assert m2["description"] == "original desc" + assert m2["id"] == m1["id"] + + +def test_add_module_same_name_different_projects_are_independent(conn): + """KIN-081: два проекта могут иметь одноимённые модули — UNIQUE per project_id.""" + models.create_project(conn, "p1", "P1", "/p1") + models.create_project(conn, "p2", "P2", "/p2") + m1 = models.add_module(conn, "p1", "api", "backend", "src/api/") + m2 = models.add_module(conn, "p2", "api", "backend", "src/api/") + assert m1["_created"] is True + assert m2["_created"] is True + assert m1["id"] != m2["id"] + + +# -- delete_project -- + +def test_delete_project_removes_project_record(conn): + """KIN-081: delete_project удаляет запись из таблицы projects.""" + models.create_project(conn, "p1", "P1", "/p1") + assert models.get_project(conn, "p1") is not None + models.delete_project(conn, "p1") + assert models.get_project(conn, "p1") is None + + +def test_delete_project_cascades_to_related_tables(conn): + """KIN-081: delete_project удаляет связанные modules, decisions, tasks, agent_logs.""" + models.create_project(conn, "p1", "P1", "/p1") + models.add_module(conn, "p1", "api", "backend", "src/api/") + models.add_decision(conn, "p1", "gotcha", "Bug X", "desc") + models.create_task(conn, "P1-001", "p1", "Task") + models.log_agent_run(conn, "p1", "developer", "implement", task_id="P1-001") + + models.delete_project(conn, "p1") + + assert conn.execute("SELECT COUNT(*) FROM modules WHERE project_id='p1'").fetchone()[0] == 0 + assert conn.execute("SELECT COUNT(*) FROM decisions WHERE project_id='p1'").fetchone()[0] == 0 + assert conn.execute("SELECT COUNT(*) FROM tasks WHERE project_id='p1'").fetchone()[0] == 0 + assert conn.execute("SELECT COUNT(*) FROM agent_logs WHERE project_id='p1'").fetchone()[0] == 0 + + +def test_delete_project_nonexistent_does_not_raise(conn): + """KIN-081: delete_project на несуществующий проект не бросает исключение.""" + models.delete_project(conn, "nonexistent") + + # -- Agent Logs -- def test_log_agent_run(conn): diff --git a/tests/test_runner.py b/tests/test_runner.py index 0a0c023..06530f6 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -9,7 +9,8 @@ from core import models from agents.runner import ( run_agent, run_pipeline, run_audit, _try_parse_json, _run_learning_extraction, _build_claude_env, _resolve_claude_cmd, _EXTRA_PATH_DIRS, _run_autocommit, - _parse_agent_blocked, + _parse_agent_blocked, _get_changed_files, _save_sysadmin_output, + check_claude_auth, ClaudeAuthError, ) @@ -400,10 +401,11 @@ class TestAutoMode: class TestRetryOnPermissionError: @patch("agents.runner._run_autocommit") @patch("agents.runner._run_learning_extraction") + @patch("agents.runner._get_changed_files") # KIN-003: prevents git subprocess calls @patch("core.followup.generate_followups") @patch("agents.runner.run_hooks") @patch("agents.runner.subprocess.run") - def test_retry_on_permission_error_auto_mode(self, mock_run, mock_hooks, mock_followup, mock_learn, mock_autocommit, conn): + def test_retry_on_permission_error_auto_mode(self, mock_run, mock_hooks, mock_followup, mock_get_files, mock_learn, mock_autocommit, conn): """Auto mode: retry при permission error должен срабатывать.""" permission_fail = _mock_claude_failure("permission denied: cannot write file") retry_success = _mock_claude_success({"result": "fixed"}) @@ -412,6 +414,7 @@ class TestRetryOnPermissionError: mock_hooks.return_value = [] mock_followup.return_value = {"created": [], "pending_actions": []} mock_learn.return_value = {"added": 0, "skipped": 0} + mock_get_files.return_value = [] models.update_project(conn, "vdol", execution_mode="auto_complete") steps = [{"role": "debugger", "brief": "find"}] @@ -2026,3 +2029,366 @@ class TestSaveSysadminOutput: } result = _save_sysadmin_output(ops_conn, "srv", "SRV-001", {"raw_output": json.dumps(output)}) assert result["modules_added"] == 0 + +# --------------------------------------------------------------------------- +# KIN-003: _get_changed_files — вычисление изменённых git-файлов +# --------------------------------------------------------------------------- + +class TestGetChangedFiles: + """Тесты для _get_changed_files(project_path) из agents/runner.py (KIN-003).""" + + @patch("agents.runner.subprocess.run") + def test_returns_files_from_git_diff(self, mock_run): + """Возвращает список файлов из git diff --name-only.""" + proc = MagicMock() + proc.returncode = 0 + proc.stdout = "web/frontend/App.vue\ncore/models.py\n" + mock_run.return_value = proc + + result = _get_changed_files("/tmp/fake-project") + + assert isinstance(result, list) + assert "web/frontend/App.vue" in result + assert "core/models.py" in result + + @patch("agents.runner.subprocess.run") + def test_returns_empty_list_on_exception(self, mock_run): + """При ошибке git (не найден, не репозиторий) возвращает [].""" + mock_run.side_effect = Exception("git not found") + + result = _get_changed_files("/tmp/fake-project") + + assert result == [] + + @patch("agents.runner.subprocess.run") + def test_deduplicates_files_from_multiple_git_commands(self, mock_run): + """Один файл из нескольких git-команд появляется в результате только один раз.""" + proc = MagicMock() + proc.returncode = 0 + proc.stdout = "web/frontend/App.vue\n" + mock_run.return_value = proc # все 3 git-команды возвращают одно и то же + + result = _get_changed_files("/tmp/fake-project") + + assert result.count("web/frontend/App.vue") == 1, ( + "Дубликаты из разных git-команд должны дедуплицироваться" + ) + + @patch("agents.runner.subprocess.run") + def test_combines_files_from_different_git_commands(self, mock_run): + """Файлы из трёх разных git-команд объединяются в один список.""" + mock_run.side_effect = [ + MagicMock(returncode=0, stdout="web/frontend/App.vue\n"), + MagicMock(returncode=0, stdout="core/models.py\n"), + MagicMock(returncode=0, stdout="agents/runner.py\n"), + ] + + result = _get_changed_files("/tmp/fake-project") + + assert "web/frontend/App.vue" in result + assert "core/models.py" in result + assert "agents/runner.py" in result + + @patch("agents.runner.subprocess.run") + def test_skips_failed_git_command_and_continues(self, mock_run): + """Упавшая git-команда (returncode != 0) не блокирует остальные.""" + fail_proc = MagicMock(returncode=1, stdout="") + success_proc = MagicMock(returncode=0, stdout="core/models.py\n") + mock_run.side_effect = [fail_proc, success_proc, fail_proc] + + result = _get_changed_files("/tmp/fake-project") + + assert "core/models.py" in result + + @patch("agents.runner.subprocess.run") + def test_strips_whitespace_from_file_paths(self, mock_run): + """Пробелы и переносы вокруг имён файлов обрезаются.""" + proc = MagicMock() + proc.returncode = 0 + proc.stdout = " web/frontend/App.vue \n core/models.py \n" + mock_run.return_value = proc + + result = _get_changed_files("/tmp/fake-project") + + assert "web/frontend/App.vue" in result + assert "core/models.py" in result + assert " web/frontend/App.vue " not in result + + +# --------------------------------------------------------------------------- +# KIN-003: run_pipeline — передача changed_files в run_hooks +# --------------------------------------------------------------------------- + +class TestPipelineChangedFiles: + """Интеграционные тесты: pipeline вычисляет changed_files и передаёт в run_hooks.""" + + @patch("agents.runner._get_changed_files") + @patch("agents.runner.run_hooks") + @patch("agents.runner.subprocess.run") + def test_pipeline_passes_changed_files_to_run_hooks( + self, mock_run, mock_hooks, mock_get_files + ): + """run_pipeline передаёт changed_files в run_hooks(event='pipeline_completed'). + + Используем проект с path='/tmp' (реальная директория), чтобы + _get_changed_files был вызван. + """ + c = init_db(":memory:") + models.create_project(c, "kin-tmp", "KinTmp", "/tmp", tech_stack=["vue3"]) + models.create_task(c, "KT-001", "kin-tmp", "Fix bug") + + mock_run.return_value = _mock_claude_success({"result": "done"}) + mock_hooks.return_value = [] + mock_get_files.return_value = ["web/frontend/App.vue", "core/models.py"] + + steps = [{"role": "debugger", "brief": "find bug"}] + result = run_pipeline(c, "KT-001", steps) + c.close() + + assert result["success"] is True + mock_get_files.assert_called_once_with("/tmp") + + # pipeline_completed call должен содержать changed_files + pipeline_calls = [ + call for call in mock_hooks.call_args_list + if call.kwargs.get("event") == "pipeline_completed" + ] + assert len(pipeline_calls) >= 1 + kw = pipeline_calls[0].kwargs + assert kw.get("changed_files") == ["web/frontend/App.vue", "core/models.py"] + + @patch("agents.runner._run_autocommit") + @patch("core.hooks.subprocess.run") + @patch("agents.runner._run_claude") + def test_pipeline_completes_when_frontend_hook_build_fails( + self, mock_run_claude, mock_hook_run, mock_autocommit + ): + """Ошибка сборки фронтенда (exitcode=1) не роняет pipeline (AC #3 KIN-003). + + Хук выполняется и возвращает failure, но pipeline.status = 'completed' + и результат run_pipeline['success'] = True. + + Примечание: патчим _run_claude (не subprocess.run) чтобы не конфликтовать + с core.hooks.subprocess.run — оба ссылаются на один и тот же subprocess.run. + """ + from core.hooks import create_hook + + c = init_db(":memory:") + models.create_project(c, "kin-build", "KinBuild", "/tmp", tech_stack=["vue3"]) + models.create_task(c, "KB-001", "kin-build", "Add feature") + create_hook( + c, "kin-build", "rebuild-frontend", "pipeline_completed", + "/tmp/rebuild.sh", + trigger_module_path=None, + working_dir="/tmp", + ) + + mock_run_claude.return_value = { + "output": "done", "returncode": 0, "error": None, + "empty_output": False, "tokens_used": None, "cost_usd": None, + } + # npm run build завершается с ошибкой + fail_proc = MagicMock() + fail_proc.returncode = 1 + fail_proc.stdout = "" + fail_proc.stderr = "Error: Cannot find module './App'" + mock_hook_run.return_value = fail_proc + + steps = [{"role": "tester", "brief": "test feature"}] + result = run_pipeline(c, "KB-001", steps) + + assert result["success"] is True, ( + "Ошибка сборки хука не должна ронять pipeline" + ) + pipe = c.execute( + "SELECT status FROM pipelines WHERE task_id='KB-001'" + ).fetchone() + assert pipe["status"] == "completed" + c.close() + + @patch("agents.runner._run_autocommit") + @patch("agents.runner.subprocess.run") + def test_pipeline_changed_files_is_none_when_project_path_missing( + self, mock_run, mock_autocommit, conn + ): + """Если путь проекта не существует, changed_files=None передаётся в run_hooks. + + Хуки по-прежнему запускаются, но без git-фильтра (task_modules fallback). + """ + # vdol path = ~/projects/vdolipoperek (не существует в CI) + # Хук без trigger_module_path должен сработать + from core.hooks import create_hook, get_hook_logs + + create_hook(conn, "vdol", "always", "pipeline_completed", + "echo ok", trigger_module_path=None, working_dir="/tmp") + + mock_run.return_value = _mock_claude_success({"result": "done"}) + build_proc = MagicMock(returncode=0, stdout="ok", stderr="") + + with patch("core.hooks.subprocess.run", return_value=build_proc): + steps = [{"role": "tester", "brief": "test"}] + result = run_pipeline(conn, "VDOL-001", steps) + + assert result["success"] is True + # Хук без фильтра должен был выполниться + logs = get_hook_logs(conn, project_id="vdol") + assert len(logs) >= 1 + + +# --------------------------------------------------------------------------- +# _save_sysadmin_output — KIN-081 +# --------------------------------------------------------------------------- + +class TestSaveSysadminOutput: + def test_modules_added_count_for_new_modules(self, conn): + """KIN-081: _save_sysadmin_output считает modules_added правильно через _created.""" + result = { + "raw_output": json.dumps({ + "modules": [ + {"name": "nginx", "type": "infra", "path": "/etc/nginx", + "description": "Web server"}, + {"name": "postgres", "type": "infra", "path": "/var/lib/postgresql", + "description": "Database"}, + ], + "decisions": [], + }) + } + counts = _save_sysadmin_output(conn, "vdol", "VDOL-001", result) + assert counts["modules_added"] == 2 + assert counts["modules_skipped"] == 0 + + def test_modules_skipped_count_for_duplicate_names(self, conn): + """KIN-081: повторный вызов с теми же модулями: added=0, skipped=2.""" + raw = json.dumps({ + "modules": [ + {"name": "nginx", "type": "infra", "path": "/etc/nginx"}, + {"name": "postgres", "type": "infra", "path": "/var/lib/postgresql"}, + ], + "decisions": [], + }) + result = {"raw_output": raw} + # First call — adds + _save_sysadmin_output(conn, "vdol", "VDOL-001", result) + # Second call — all duplicates + counts = _save_sysadmin_output(conn, "vdol", "VDOL-001", result) + assert counts["modules_added"] == 0 + assert counts["modules_skipped"] == 2 + + def test_empty_output_returns_zeros(self, conn): + """_save_sysadmin_output с не-JSON строкой возвращает нули.""" + counts = _save_sysadmin_output(conn, "vdol", "VDOL-001", + {"raw_output": "Agent completed the task."}) + assert counts == { + "decisions_added": 0, "decisions_skipped": 0, + "modules_added": 0, "modules_skipped": 0, + } + + def test_decisions_added_and_skipped(self, conn): + """_save_sysadmin_output дедуплицирует decisions через add_decision_if_new.""" + raw = json.dumps({ + "modules": [], + "decisions": [ + {"type": "convention", "title": "Use WAL mode", + "description": "PRAGMA journal_mode=WAL for SQLite"}, + ], + }) + result = {"raw_output": raw} + counts1 = _save_sysadmin_output(conn, "vdol", "VDOL-001", result) + assert counts1["decisions_added"] == 1 + assert counts1["decisions_skipped"] == 0 + + counts2 = _save_sysadmin_output(conn, "vdol", "VDOL-001", result) + assert counts2["decisions_added"] == 0 + assert counts2["decisions_skipped"] == 1 + + +# --------------------------------------------------------------------------- +# check_claude_auth +# --------------------------------------------------------------------------- + +class TestCheckClaudeAuth: + """Tests for check_claude_auth() — Claude CLI login healthcheck.""" + + @patch("agents.runner.subprocess.run") + def test_ok_when_returncode_zero(self, mock_run): + """Не бросает исключение при returncode=0 и корректном JSON.""" + mock = MagicMock() + mock.stdout = json.dumps({"result": "ok"}) + mock.stderr = "" + mock.returncode = 0 + mock_run.return_value = mock + + check_claude_auth() # должна вернуть None без исключений + + @patch("agents.runner.subprocess.run") + def test_not_logged_in_via_string_in_stdout(self, mock_run): + """Бросает ClaudeAuthError при 'Not logged in' в stdout.""" + mock = MagicMock() + mock.stdout = "Not logged in" + mock.stderr = "" + mock.returncode = 1 + mock_run.return_value = mock + + with pytest.raises(ClaudeAuthError) as exc_info: + check_claude_auth() + assert "login" in str(exc_info.value).lower() + + @patch("agents.runner.subprocess.run") + def test_not_logged_in_case_insensitive(self, mock_run): + """Бросает ClaudeAuthError при 'not logged in' в любом регистре.""" + mock = MagicMock() + mock.stdout = "" + mock.stderr = "Error: NOT LOGGED IN to Claude" + mock.returncode = 1 + mock_run.return_value = mock + + with pytest.raises(ClaudeAuthError): + check_claude_auth() + + @patch("agents.runner.subprocess.run") + def test_not_logged_in_via_string_in_stderr(self, mock_run): + """Бросает ClaudeAuthError при 'Not logged in' в stderr.""" + mock = MagicMock() + mock.stdout = "" + mock.stderr = "Error: Not logged in to Claude" + mock.returncode = 1 + mock_run.return_value = mock + + with pytest.raises(ClaudeAuthError): + check_claude_auth() + + @patch("agents.runner.subprocess.run") + def test_not_logged_in_via_nonzero_returncode(self, mock_run): + """Бросает ClaudeAuthError при ненулевом returncode (без 'Not logged in' текста).""" + mock = MagicMock() + mock.stdout = "" + mock.stderr = "Some other error" + mock.returncode = 1 + mock_run.return_value = mock + + with pytest.raises(ClaudeAuthError): + check_claude_auth() + + @patch("agents.runner.subprocess.run") + def test_not_logged_in_via_is_error_in_json(self, mock_run): + """Бросает ClaudeAuthError при is_error=true в JSON даже с returncode=0.""" + mock = MagicMock() + mock.stdout = json.dumps({"is_error": True, "result": "authentication required"}) + mock.stderr = "" + mock.returncode = 0 + mock_run.return_value = mock + + with pytest.raises(ClaudeAuthError): + check_claude_auth() + + @patch("agents.runner.subprocess.run", side_effect=FileNotFoundError) + def test_raises_when_cli_not_found(self, mock_run): + """При FileNotFoundError бросает ClaudeAuthError с понятным сообщением.""" + with pytest.raises(ClaudeAuthError) as exc_info: + check_claude_auth() + assert "PATH" in str(exc_info.value) or "not found" in str(exc_info.value).lower() + + @patch("agents.runner.subprocess.run", side_effect=subprocess.TimeoutExpired(cmd="claude", timeout=10)) + def test_ok_when_timeout(self, mock_run): + """При TimeoutExpired не бросает исключение (не блокируем на timeout).""" + check_claude_auth() # должна вернуть None без исключений diff --git a/web/api.py b/web/api.py index b958af9..10c8d28 100644 --- a/web/api.py +++ b/web/api.py @@ -426,6 +426,16 @@ def start_project_phase(project_id: str): in_progress, spawns a background subprocess (same as /api/tasks/{id}/run), and returns immediately so the HTTP request doesn't block on agent execution. """ + from agents.runner import check_claude_auth, ClaudeAuthError + try: + check_claude_auth() + except ClaudeAuthError: + raise HTTPException(503, detail={ + "error": "claude_auth_required", + "message": "Claude CLI requires login", + "instructions": "Run: claude login", + }) + conn = get_conn() p = models.get_project(conn, project_id) if not p: @@ -454,8 +464,9 @@ def start_project_phase(project_id: str): conn.close() kin_root = Path(__file__).parent.parent - cmd = [sys.executable, "-m", "cli.main", "--db", str(DB_PATH), "run", task_id] - cmd.append("--allow-write") + cmd = [sys.executable, "-m", "cli.main", "--db", str(DB_PATH), + "run", task_id] + cmd.append("--allow-write") # always required: subprocess runs non-interactively (stdin=DEVNULL) import os env = os.environ.copy() @@ -776,6 +787,16 @@ def is_task_running(task_id: str): @app.post("/api/tasks/{task_id}/run") def run_task(task_id: str): """Launch pipeline for a task in background. Returns 202.""" + from agents.runner import check_claude_auth, ClaudeAuthError + try: + check_claude_auth() + except ClaudeAuthError: + raise HTTPException(503, detail={ + "error": "claude_auth_required", + "message": "Claude CLI requires login", + "instructions": "Run: claude login", + }) + conn = get_conn() t = models.get_task(conn, task_id) if not t: @@ -965,8 +986,14 @@ def bootstrap(body: BootstrapRequest): if obs["tasks"] or obs["decisions"]: obsidian = obs - save_to_db(conn, body.id, body.name, str(project_path), - tech_stack, modules, decisions, obsidian) + try: + save_to_db(conn, body.id, body.name, str(project_path), + tech_stack, modules, decisions, obsidian) + except Exception as e: + if models.get_project(conn, body.id): + models.delete_project(conn, body.id) + conn.close() + raise HTTPException(500, f"Bootstrap failed: {e}") p = models.get_project(conn, body.id) conn.close() return { diff --git a/web/frontend/src/__tests__/claude-auth.test.ts b/web/frontend/src/__tests__/claude-auth.test.ts new file mode 100644 index 0000000..9fc7b56 --- /dev/null +++ b/web/frontend/src/__tests__/claude-auth.test.ts @@ -0,0 +1,277 @@ +/** + * KIN-083: Тесты healthcheck Claude CLI auth — frontend баннеры + * + * Проверяет: + * 1. TaskDetail.vue: при ошибке claude_auth_required от runTask — показывает баннер + * 2. TaskDetail.vue: баннер закрывается кнопкой ✕ + * 3. TaskDetail.vue: happy path — баннер не появляется при успешном runTask + * 4. ProjectView.vue: при ошибке claude_auth_required от startPhase — показывает баннер + * 5. ProjectView.vue: happy path — баннер не появляется при успешном startPhase + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { mount, flushPromises } from '@vue/test-utils' +import { createRouter, createMemoryHistory } from 'vue-router' +import TaskDetail from '../views/TaskDetail.vue' +import ProjectView from '../views/ProjectView.vue' + +// importOriginal сохраняет реальный ApiError — нужен для instanceof-проверки в компоненте +vi.mock('../api', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + api: { + project: vi.fn(), + taskFull: vi.fn(), + runTask: vi.fn(), + startPhase: vi.fn(), + getPhases: vi.fn(), + patchTask: vi.fn(), + patchProject: vi.fn(), + auditProject: vi.fn(), + createTask: vi.fn(), + deployProject: vi.fn(), + notifications: vi.fn(), + }, + } +}) + +import { api, ApiError } from '../api' + +const Stub = { template: '
' } + +const localStorageMock = (() => { + let store: Record = {} + return { + getItem: (k: string) => store[k] ?? null, + setItem: (k: string, v: string) => { store[k] = v }, + removeItem: (k: string) => { delete store[k] }, + clear: () => { store = {} }, + } +})() +Object.defineProperty(globalThis, 'localStorage', { value: localStorageMock, configurable: true }) + +function makeRouter() { + return createRouter({ + history: createMemoryHistory(), + routes: [ + { path: '/', component: Stub }, + { path: '/project/:id', component: ProjectView, props: true }, + { path: '/task/:id', component: TaskDetail, props: true }, + ], + }) +} + +const MOCK_TASK = { + id: 'KIN-001', + project_id: 'KIN', + title: 'Тестовая задача', + status: 'pending', + priority: 5, + assigned_role: null, + parent_task_id: null, + brief: null, + spec: null, + execution_mode: null, + blocked_reason: null, + dangerously_skipped: null, + category: null, + acceptance_criteria: null, + created_at: '2024-01-01', + updated_at: '2024-01-01', + pipeline_steps: [], + related_decisions: [], + pending_actions: [], +} + +const MOCK_PROJECT = { + id: 'KIN', + name: 'Kin', + path: '/projects/kin', + status: 'active', + priority: 5, + tech_stack: ['python', 'vue'], + execution_mode: 'review', + autocommit_enabled: 0, + obsidian_vault_path: null, + deploy_command: null, + created_at: '2024-01-01', + total_tasks: 1, + done_tasks: 0, + active_tasks: 1, + blocked_tasks: 0, + review_tasks: 0, + project_type: 'development', + ssh_host: null, + ssh_user: null, + ssh_key_path: null, + ssh_proxy_jump: null, + description: null, + tasks: [], + decisions: [], + modules: [], +} + +const MOCK_ACTIVE_PHASE = { + id: 1, + project_id: 'KIN', + role: 'pm', + phase_order: 1, + status: 'active', + task_id: 'KIN-R-001', + revise_count: 0, + revise_comment: null, + created_at: '2024-01-01', + updated_at: '2024-01-01', + task: { + id: 'KIN-R-001', + status: 'pending', + title: 'Research', + priority: 5, + assigned_role: 'pm', + parent_task_id: null, + brief: null, + spec: null, + execution_mode: null, + blocked_reason: null, + dangerously_skipped: null, + category: null, + acceptance_criteria: null, + project_id: 'KIN', + created_at: '2024-01-01', + updated_at: '2024-01-01', + }, +} + +beforeEach(() => { + localStorageMock.clear() + vi.clearAllMocks() + vi.mocked(api.project).mockResolvedValue(MOCK_PROJECT as any) + vi.mocked(api.taskFull).mockResolvedValue(MOCK_TASK as any) + vi.mocked(api.runTask).mockResolvedValue({ status: 'started' } as any) + vi.mocked(api.startPhase).mockResolvedValue({ status: 'started', phase_id: 1, task_id: 'KIN-R-001' }) + vi.mocked(api.getPhases).mockResolvedValue([]) + vi.mocked(api.notifications).mockResolvedValue([]) +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +// ───────────────────────────────────────────────────────────── +// TaskDetail: баннер при claude_auth_required +// ───────────────────────────────────────────────────────────── + +describe('KIN-083: TaskDetail — claude auth banner', () => { + async function mountTaskDetail() { + const router = makeRouter() + await router.push('/task/KIN-001') + const wrapper = mount(TaskDetail, { + props: { id: 'KIN-001' }, + global: { plugins: [router] }, + }) + await flushPromises() + return wrapper + } + + it('показывает баннер "Claude CLI requires login" при ошибке claude_auth_required от runTask', async () => { + vi.mocked(api.runTask).mockRejectedValue( + new ApiError('claude_auth_required', 'Claude CLI requires login. Run: claude login'), + ) + + const wrapper = await mountTaskDetail() + + const runBtn = wrapper.findAll('button').find(b => b.text().includes('Run Pipeline')) + expect(runBtn?.exists(), 'Кнопка Run Pipeline должна быть видна для pending задачи').toBe(true) + + await runBtn!.trigger('click') + await flushPromises() + + expect(wrapper.text(), 'Баннер должен содержать текст ошибки аутентификации').toContain('Claude CLI requires login') + }) + + it('баннер закрывается кнопкой ✕', async () => { + vi.mocked(api.runTask).mockRejectedValue( + new ApiError('claude_auth_required', 'Claude CLI requires login. Run: claude login'), + ) + + const wrapper = await mountTaskDetail() + + const runBtn = wrapper.findAll('button').find(b => b.text().includes('Run Pipeline')) + await runBtn!.trigger('click') + await flushPromises() + + expect(wrapper.text()).toContain('Claude CLI requires login') + + const closeBtn = wrapper.findAll('button').find(b => b.text().trim() === '✕') + expect(closeBtn?.exists(), 'Кнопка ✕ должна быть видна').toBe(true) + await closeBtn!.trigger('click') + await flushPromises() + + expect(wrapper.text(), 'После закрытия баннер не должен быть виден').not.toContain('Claude CLI requires login') + }) + + it('не показывает баннер когда runTask успешен (happy path)', async () => { + const wrapper = await mountTaskDetail() + + const runBtn = wrapper.findAll('button').find(b => b.text().includes('Run Pipeline')) + if (runBtn?.exists()) { + await runBtn.trigger('click') + await flushPromises() + } + + expect(wrapper.text(), 'Баннер не должен появляться при успешном запуске').not.toContain('Claude CLI requires login') + }) +}) + +// ───────────────────────────────────────────────────────────── +// ProjectView: баннер при claude_auth_required +// ───────────────────────────────────────────────────────────── + +describe('KIN-083: ProjectView — claude auth banner', () => { + async function mountOnPhases() { + vi.mocked(api.getPhases).mockResolvedValue([MOCK_ACTIVE_PHASE] as any) + + const router = makeRouter() + await router.push('/project/KIN') + const wrapper = mount(ProjectView, { + props: { id: 'KIN' }, + global: { plugins: [router] }, + }) + await flushPromises() + + const phasesTab = wrapper.findAll('button').find(b => b.text().includes('Phases')) + await phasesTab!.trigger('click') + await flushPromises() + + return wrapper + } + + it('показывает баннер "Claude CLI requires login" при ошибке claude_auth_required от startPhase', async () => { + vi.mocked(api.startPhase).mockRejectedValue( + new ApiError('claude_auth_required', 'Claude CLI requires login. Run: claude login'), + ) + + const wrapper = await mountOnPhases() + + const startBtn = wrapper.findAll('button').find(b => b.text().includes('Start Research')) + expect(startBtn?.exists(), 'Кнопка Start Research должна быть видна').toBe(true) + + await startBtn!.trigger('click') + await flushPromises() + + expect(wrapper.text(), 'Баннер должен содержать текст ошибки аутентификации').toContain('Claude CLI requires login') + }) + + it('не показывает баннер когда startPhase успешен (happy path)', async () => { + const wrapper = await mountOnPhases() + + const startBtn = wrapper.findAll('button').find(b => b.text().includes('Start Research')) + if (startBtn?.exists()) { + await startBtn.trigger('click') + await flushPromises() + } + + expect(wrapper.text(), 'Баннер не должен появляться при успешном запуске фазы').not.toContain('Claude CLI requires login') + }) +}) diff --git a/web/frontend/src/api.ts b/web/frontend/src/api.ts index ff9e353..85994ea 100644 --- a/web/frontend/src/api.ts +++ b/web/frontend/src/api.ts @@ -1,8 +1,28 @@ const BASE = '/api' +export class ApiError extends Error { + code: string + constructor(code: string, message: string) { + super(message) + this.name = 'ApiError' + this.code = code + } +} + +async function throwApiError(res: Response): Promise { + let code = '' + let msg = `${res.status} ${res.statusText}` + try { + const data = await res.json() + if (data.error) code = data.error + if (data.message) msg = data.message + } catch {} + throw new ApiError(code, msg) +} + async function get(path: string): Promise { const res = await fetch(`${BASE}${path}`) - if (!res.ok) throw new Error(`${res.status} ${res.statusText}`) + if (!res.ok) await throwApiError(res) return res.json() } @@ -12,7 +32,7 @@ async function patch(path: string, body: unknown): Promise { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), }) - if (!res.ok) throw new Error(`${res.status} ${res.statusText}`) + if (!res.ok) await throwApiError(res) return res.json() } @@ -22,7 +42,7 @@ async function post(path: string, body: unknown): Promise { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), }) - if (!res.ok) throw new Error(`${res.status} ${res.statusText}`) + if (!res.ok) await throwApiError(res) return res.json() } diff --git a/web/frontend/src/views/ProjectView.vue b/web/frontend/src/views/ProjectView.vue index b4bed48..0ea8a60 100644 --- a/web/frontend/src/views/ProjectView.vue +++ b/web/frontend/src/views/ProjectView.vue @@ -1,7 +1,7 @@