kin: auto-commit after pipeline
This commit is contained in:
Gros Frumos
parent
4f1dfbf10f
commit
c54849cf20
3 changed files with 170 additions and 25 deletions
|
|
@ -1,15 +1,13 @@
|
|||
"""Regression tests for KIN-111 — empty/null pipeline returned by PM agent.
|
||||
"""Regression tests for KIN-111 — two separate bug batches.
|
||||
|
||||
Root cause (two scenarios):
|
||||
1. PM returns {"pipeline": null} → cli/main.py calls len(None) → TypeError (crash)
|
||||
2. PM returns {"pipeline": []} → run_pipeline creates empty DB record,
|
||||
task stuck in 'review' with 0 steps executed
|
||||
Batch A (empty/null pipeline):
|
||||
Root cause: PM returns {"pipeline": null} or {"pipeline": []} — crashes or hangs.
|
||||
|
||||
Fixes required:
|
||||
cli/main.py: validate pipeline_steps after extraction — non-empty list required
|
||||
agents/runner.py: run_pipeline early-return when steps=[]
|
||||
Batch B (Deploy button, Settings JSON, Worktrees toggle):
|
||||
Root cause: Vite dev server intercepts /api/* requests and returns HTML instead of
|
||||
proxying to FastAPI. Fix: add server.proxy in vite.config.ts.
|
||||
|
||||
Coverage:
|
||||
Batch A coverage:
|
||||
(1) run_pipeline with steps=[] returns {success: False, error: 'empty_pipeline'}
|
||||
(2) run_pipeline with steps=[] does NOT transition task to in_progress
|
||||
(3) run_pipeline with steps=[] does NOT create a pipeline record in DB
|
||||
|
|
@ -20,7 +18,12 @@ Coverage:
|
|||
(8) generate_followups: agent returns "[]" → no tasks created in DB
|
||||
(9) generate_followups: task has no prior agent_logs → Claude still called (no early bail)
|
||||
(10) API /followup: agent returns "[]" → needs_decision is False
|
||||
"""
|
||||
|
||||
Batch B coverage:
|
||||
(11) GET /api/projects returns Content-Type: application/json (not text/html)
|
||||
(12) PATCH /api/projects/{id} with worktrees_enabled=True → 200, not 400 Bad Request
|
||||
(13) POST /api/projects/{id}/deploy without deploy config → 400 (button blocked correctly)
|
||||
(14) vite.config.ts has server.proxy for /api → proxy to FastAPI (the actual fix)"""
|
||||
|
||||
import json
|
||||
|
||||
|
|
@ -420,3 +423,106 @@ class TestGenerateFollowupsNullAndDict:
|
|||
f"Expected created=[] for whitespace-wrapped '[]', got: {result['created']}"
|
||||
)
|
||||
assert result["pending_actions"] == []
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Batch B — (11/12/13/14) Deploy button, Settings JSON, Worktrees toggle
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.fixture
|
||||
def api_client(tmp_path):
|
||||
"""TestClient with isolated DB, seeded project."""
|
||||
import web.api as api_module
|
||||
api_module.DB_PATH = tmp_path / "test.db"
|
||||
from web.api import app
|
||||
from fastapi.testclient import TestClient
|
||||
c = TestClient(app)
|
||||
c.post("/api/projects", json={"id": "p1", "name": "P1", "path": "/p1"})
|
||||
return c
|
||||
|
||||
|
||||
class TestSettingsJsonResponse:
|
||||
"""(11) GET /api/projects must return JSON content-type, not HTML."""
|
||||
|
||||
def test_get_projects_content_type_is_json(self, api_client):
|
||||
"""GET /api/projects → Content-Type must be application/json."""
|
||||
r = api_client.get("/api/projects")
|
||||
assert r.status_code == 200, f"Expected 200, got {r.status_code}"
|
||||
ct = r.headers.get("content-type", "")
|
||||
assert "application/json" in ct, (
|
||||
f"Expected content-type=application/json, got: {ct!r}. "
|
||||
"If Vite is serving HTML for /api/* requests, the proxy is not configured."
|
||||
)
|
||||
|
||||
def test_get_projects_returns_list_not_html(self, api_client):
|
||||
"""GET /api/projects → body must be a JSON list, not an HTML string."""
|
||||
r = api_client.get("/api/projects")
|
||||
assert r.status_code == 200
|
||||
data = r.json() # raises JSONDecodeError if HTML was returned
|
||||
assert isinstance(data, list), f"Expected list, got: {type(data).__name__}"
|
||||
|
||||
|
||||
class TestWorktreesTogglePatch:
|
||||
"""(12) PATCH /api/projects/{id} with worktrees_enabled=True must return 200."""
|
||||
|
||||
def test_patch_worktrees_enabled_true_returns_200(self, api_client):
|
||||
"""PATCH worktrees_enabled=True → 200, not 400 Bad Request."""
|
||||
r = api_client.patch("/api/projects/p1", json={"worktrees_enabled": True})
|
||||
assert r.status_code == 200, (
|
||||
f"Expected 200 for worktrees_enabled patch, got {r.status_code}: {r.text}"
|
||||
)
|
||||
|
||||
def test_patch_worktrees_enabled_true_persists(self, api_client):
|
||||
"""PATCH worktrees_enabled=True → project reflects the change."""
|
||||
api_client.patch("/api/projects/p1", json={"worktrees_enabled": True})
|
||||
r = api_client.get("/api/projects/p1")
|
||||
assert r.status_code == 200
|
||||
assert r.json()["worktrees_enabled"], (
|
||||
"worktrees_enabled must be truthy after PATCH"
|
||||
)
|
||||
|
||||
def test_patch_worktrees_enabled_false_returns_200(self, api_client):
|
||||
"""PATCH worktrees_enabled=False → 200, not 400 Bad Request."""
|
||||
r = api_client.patch("/api/projects/p1", json={"worktrees_enabled": False})
|
||||
assert r.status_code == 200, (
|
||||
f"Expected 200 for worktrees_enabled=False patch, got {r.status_code}: {r.text}"
|
||||
)
|
||||
|
||||
|
||||
class TestDeployButtonBackend:
|
||||
"""(13) Deploy endpoint must return 400 when no deploy config is set."""
|
||||
|
||||
def test_deploy_without_config_returns_400(self, api_client):
|
||||
"""POST /deploy on unconfigured project → 400 (Deploy button correctly blocked)."""
|
||||
r = api_client.post("/api/projects/p1/deploy")
|
||||
assert r.status_code == 400, (
|
||||
f"Expected 400 when neither deploy_runtime nor deploy_command is set, got {r.status_code}"
|
||||
)
|
||||
|
||||
def test_deploy_not_found_returns_404(self, api_client):
|
||||
"""POST /deploy on unknown project → 404."""
|
||||
r = api_client.post("/api/projects/NOPE/deploy")
|
||||
assert r.status_code == 404
|
||||
|
||||
|
||||
class TestViteProxyConfig:
|
||||
"""(14) vite.config.ts must have server.proxy configured for /api.
|
||||
|
||||
This is the actual fix for KIN-111: without the proxy, Vite serves its
|
||||
own HTML for /api/* requests in dev mode, causing JSON parse errors.
|
||||
"""
|
||||
|
||||
def test_vite_config_has_api_proxy(self):
|
||||
"""vite.config.ts must define server.proxy that includes '/api'."""
|
||||
import pathlib
|
||||
config_path = pathlib.Path(__file__).parent.parent / "web" / "frontend" / "vite.config.ts"
|
||||
assert config_path.exists(), f"vite.config.ts not found at {config_path}"
|
||||
content = config_path.read_text()
|
||||
assert "proxy" in content, (
|
||||
"vite.config.ts has no 'proxy' config. "
|
||||
"Add server: { proxy: { '/api': 'http://localhost:8000' } } to fix "
|
||||
"the Unexpected token '<' error in Settings and the Bad Request on Worktrees toggle."
|
||||
)
|
||||
assert "/api" in content or "'/api'" in content or '"/api"' in content, (
|
||||
"vite.config.ts proxy must include '/api' route to FastAPI backend."
|
||||
)
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ 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, _get_changed_files, _save_sysadmin_output,
|
||||
check_claude_auth, ClaudeAuthError,
|
||||
check_claude_auth, ClaudeAuthError, _MODEL_TIMEOUTS,
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -603,7 +603,7 @@ class TestNonInteractive:
|
|||
mock_run.return_value = _mock_claude_success({"result": "ok"})
|
||||
run_agent(conn, "debugger", "VDOL-001", "vdol", noninteractive=True)
|
||||
call_kwargs = mock_run.call_args[1]
|
||||
assert call_kwargs.get("timeout") == 1200 # sonnet default
|
||||
assert call_kwargs.get("timeout") == 2400 # sonnet default
|
||||
|
||||
@patch.dict("os.environ", {"KIN_NONINTERACTIVE": ""})
|
||||
@patch("agents.runner.subprocess.run")
|
||||
|
|
@ -611,7 +611,7 @@ class TestNonInteractive:
|
|||
mock_run.return_value = _mock_claude_success({"result": "ok"})
|
||||
run_agent(conn, "debugger", "VDOL-001", "vdol", noninteractive=False)
|
||||
call_kwargs = mock_run.call_args[1]
|
||||
assert call_kwargs.get("timeout") == 1200 # sonnet default
|
||||
assert call_kwargs.get("timeout") == 2400 # sonnet default
|
||||
|
||||
@patch.dict("os.environ", {"KIN_NONINTERACTIVE": ""})
|
||||
@patch("agents.runner.subprocess.run")
|
||||
|
|
@ -630,7 +630,7 @@ class TestNonInteractive:
|
|||
run_agent(conn, "debugger", "VDOL-001", "vdol", noninteractive=False)
|
||||
call_kwargs = mock_run.call_args[1]
|
||||
assert call_kwargs.get("stdin") == subprocess.DEVNULL
|
||||
assert call_kwargs.get("timeout") == 1200 # sonnet default
|
||||
assert call_kwargs.get("timeout") == 2400 # sonnet default
|
||||
|
||||
@patch.dict("os.environ", {"KIN_AGENT_TIMEOUT": "900"})
|
||||
@patch("agents.runner.subprocess.run")
|
||||
|
|
@ -643,21 +643,21 @@ class TestNonInteractive:
|
|||
|
||||
@patch.dict("os.environ", {"KIN_AGENT_TIMEOUT": ""})
|
||||
@patch("agents.runner.subprocess.run")
|
||||
def test_opus_timeout_1800(self, mock_run, conn):
|
||||
"""Opus model gets 1800s (30 min) timeout."""
|
||||
def test_opus_timeout_2400(self, mock_run, conn):
|
||||
"""Opus model gets 2400s (40 min) timeout."""
|
||||
mock_run.return_value = _mock_claude_success({"result": "ok"})
|
||||
run_agent(conn, "debugger", "VDOL-001", "vdol", model="opus")
|
||||
call_kwargs = mock_run.call_args[1]
|
||||
assert call_kwargs.get("timeout") == 1800
|
||||
assert call_kwargs.get("timeout") == 2400
|
||||
|
||||
@patch.dict("os.environ", {"KIN_AGENT_TIMEOUT": ""})
|
||||
@patch("agents.runner.subprocess.run")
|
||||
def test_haiku_timeout_600(self, mock_run, conn):
|
||||
"""Haiku model gets 600s (10 min) timeout."""
|
||||
def test_haiku_timeout_1200(self, mock_run, conn):
|
||||
"""Haiku model gets 1200s (20 min) timeout."""
|
||||
mock_run.return_value = _mock_claude_success({"result": "ok"})
|
||||
run_agent(conn, "debugger", "VDOL-001", "vdol", model="haiku")
|
||||
call_kwargs = mock_run.call_args[1]
|
||||
assert call_kwargs.get("timeout") == 600
|
||||
assert call_kwargs.get("timeout") == 1200
|
||||
|
||||
@patch.dict("os.environ", {"KIN_AGENT_TIMEOUT": "999"})
|
||||
@patch("agents.runner.subprocess.run")
|
||||
|
|
@ -1264,11 +1264,11 @@ class TestRegressionKIN056:
|
|||
@patch.dict("os.environ", {"KIN_NONINTERACTIVE": "1", "KIN_AGENT_TIMEOUT": ""})
|
||||
@patch("agents.runner.subprocess.run")
|
||||
def test_web_noninteractive_timeout_uses_model_default(self, mock_run, conn):
|
||||
"""Web путь: KIN_NONINTERACTIVE=1 → timeout = model default (sonnet=1200s)."""
|
||||
"""Web путь: KIN_NONINTERACTIVE=1 → timeout = model default (sonnet=2400s)."""
|
||||
mock_run.return_value = _mock_claude_success({"result": "ok"})
|
||||
run_agent(conn, "debugger", "VDOL-001", "vdol")
|
||||
call_kwargs = mock_run.call_args[1]
|
||||
assert call_kwargs.get("timeout") == 1200
|
||||
assert call_kwargs.get("timeout") == 2400
|
||||
|
||||
@patch("agents.runner.subprocess.run")
|
||||
def test_web_and_cli_paths_use_same_timeout(self, mock_run, conn):
|
||||
|
|
@ -1301,6 +1301,45 @@ class TestRegressionKIN056:
|
|||
assert call_kwargs.get("timeout") == 900
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# KIN-TIMEOUT: Регрессия — таймауты были слишком короткими (opus 1800s, sonnet 1200s, haiku 600s)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestRegressionModelTimeouts:
|
||||
"""Регрессионные тесты: увеличенные таймауты для opus/sonnet/haiku.
|
||||
|
||||
Старые значения: opus=1800s, sonnet=1200s, haiku=600s.
|
||||
Новые значения: opus=2400s, sonnet=2400s, haiku=1200s.
|
||||
|
||||
Каждый тест ПАДАЛ бы со старыми значениями и ПРОХОДИТ после фикса.
|
||||
"""
|
||||
|
||||
def test_opus_model_timeout_is_2400(self):
|
||||
"""Opus: таймаут должен быть 2400s (40 мин), а не старый 1800s."""
|
||||
assert _MODEL_TIMEOUTS["opus"] == 2400, (
|
||||
f"Регрессия: ожидалось 2400s для opus, получено {_MODEL_TIMEOUTS['opus']}s"
|
||||
)
|
||||
|
||||
def test_sonnet_model_timeout_is_2400(self):
|
||||
"""Sonnet: таймаут должен быть 2400s (40 мин), а не старый 1200s."""
|
||||
assert _MODEL_TIMEOUTS["sonnet"] == 2400, (
|
||||
f"Регрессия: ожидалось 2400s для sonnet, получено {_MODEL_TIMEOUTS['sonnet']}s"
|
||||
)
|
||||
|
||||
def test_haiku_model_timeout_is_1200(self):
|
||||
"""Haiku: таймаут должен быть 1200s (20 мин), а не старый 600s."""
|
||||
assert _MODEL_TIMEOUTS["haiku"] == 1200, (
|
||||
f"Регрессия: ожидалось 1200s для haiku, получено {_MODEL_TIMEOUTS['haiku']}s"
|
||||
)
|
||||
|
||||
def test_all_models_have_minimum_timeout_20min(self):
|
||||
"""Все модели должны иметь таймаут не менее 1200s (20 мин)."""
|
||||
for model, timeout in _MODEL_TIMEOUTS.items():
|
||||
assert timeout >= 1200, (
|
||||
f"Регрессия: модель '{model}' имеет таймаут {timeout}s < 1200s"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# KIN-057: claude CLI в PATH при запуске через launchctl
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue