kin/tests/test_kin_111_regression.py

"""Regression tests for KIN-111 — two separate bug batches.

Batch A (empty/null pipeline):
Root cause: PM returns {"pipeline": null} or {"pipeline": []} — crashes or hangs.

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.

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
(4) CLI run: PM returns {"pipeline": null} → exit(1) with error, not TypeError crash
(5) CLI run: PM returns {"pipeline": []} → exit(1) with error, run_pipeline not called
(6) run_pipeline with steps=[] — task status stays unchanged (not mutated to any other status)
(7) generate_followups: agent returns "[]" → {created: [], pending_actions: []}
(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

import pytest
from click.testing import CliRunner
from unittest.mock import patch, MagicMock

from core.db import init_db
from core import models


# ---------------------------------------------------------------------------
# Shared fixture
# ---------------------------------------------------------------------------

@pytest.fixture
def conn():
    c = init_db(":memory:")
    models.create_project(c, "proj", "Proj", "/tmp/proj", tech_stack=["python"])
    models.create_task(c, "PROJ-001", "proj", "Fix bug", brief={"route_type": "debug"})
    yield c
    c.close()


# ---------------------------------------------------------------------------
# (1/2/3) run_pipeline with steps=[] — early return, no DB side effects
# ---------------------------------------------------------------------------

class TestRunPipelineEmptySteps:

    @patch("agents.runner.check_claude_auth")
    def test_empty_steps_returns_error_dict(self, mock_auth, conn):
        """run_pipeline with steps=[] must return {success: False, error: 'empty_pipeline'}."""
        from agents.runner import run_pipeline
        result = run_pipeline(conn, "PROJ-001", [])
        assert result["success"] is False
        assert result.get("error") == "empty_pipeline", (
            f"Expected error='empty_pipeline', got: {result}"
        )

    @patch("agents.runner.check_claude_auth")
    def test_empty_steps_does_not_set_task_in_progress(self, mock_auth, conn):
        """run_pipeline with steps=[] must NOT transition task to in_progress."""
        from agents.runner import run_pipeline
        run_pipeline(conn, "PROJ-001", [])
        task = models.get_task(conn, "PROJ-001")
        assert task["status"] != "in_progress", (
            "Task must not be set to in_progress when pipeline has no steps"
        )

    @patch("agents.runner.check_claude_auth")
    def test_empty_steps_does_not_create_pipeline_record(self, mock_auth, conn):
        """run_pipeline with steps=[] must NOT create any pipeline record in DB."""
        from agents.runner import run_pipeline
        run_pipeline(conn, "PROJ-001", [])
        # No pipeline record must exist for this task
        row = conn.execute(
            "SELECT COUNT(*) FROM pipelines WHERE task_id = 'PROJ-001'"
        ).fetchone()
        assert row[0] == 0, (
            f"Expected 0 pipeline records, found {row[0]}. "
            "run_pipeline must not persist to DB when steps=[]."
        )


# ---------------------------------------------------------------------------
# (4/5) CLI run_task: PM returns null or empty pipeline
# ---------------------------------------------------------------------------

def _seed_db(tmp_path):
    """Create a real on-disk DB with test data and return its path string."""
    db_path = tmp_path / "test.db"
    c = init_db(str(db_path))
    models.create_project(c, "proj", "Proj", str(tmp_path), tech_stack=["python"])
    models.create_task(c, "PROJ-001", "proj", "Fix bug", brief={"route_type": "debug"})
    c.close()
    return str(db_path)


class TestCliRunTaskNullPipeline:
    """PM returns {"pipeline": null} — CLI must exit(1), not crash with TypeError."""

    @patch("agents.runner.run_pipeline")
    @patch("agents.runner.run_agent")
    def test_exit_code_is_1_not_exception(self, mock_run_agent, mock_run_pipeline, tmp_path):
        """PM returning pipeline=null → CLI exits with code 1 (not unhandled exception)."""
        from cli.main import cli as kin_cli
        db_path = _seed_db(tmp_path)
        mock_run_agent.return_value = {
            "success": True,
            "output": json.dumps({"pipeline": None, "analysis": "nothing"}),
        }
        runner = CliRunner()
        result = runner.invoke(kin_cli, ["--db", db_path, "run", "PROJ-001"])
        assert result.exit_code == 1, (
            f"Expected exit_code=1 for null pipeline, got {result.exit_code}"
        )

    @patch("agents.runner.run_pipeline")
    @patch("agents.runner.run_agent")
    def test_no_typeerror_on_null_pipeline(self, mock_run_agent, mock_run_pipeline, tmp_path):
        """PM returning pipeline=null must not crash with TypeError (len(None))."""
        from cli.main import cli as kin_cli
        db_path = _seed_db(tmp_path)
        mock_run_agent.return_value = {
            "success": True,
            "output": json.dumps({"pipeline": None, "analysis": "nothing"}),
        }
        runner = CliRunner()
        result = runner.invoke(kin_cli, ["--db", db_path, "run", "PROJ-001"])
        # If a TypeError was raised, result.exception will contain it
        if result.exception is not None:
            assert not isinstance(result.exception, TypeError), (
                "CLI crashed with TypeError when PM returned pipeline=null. "
                "Missing validation in cli/main.py."
            )

    @patch("agents.runner.run_pipeline")
    @patch("agents.runner.run_agent")
    def test_run_pipeline_not_called_on_null(self, mock_run_agent, mock_run_pipeline, tmp_path):
        """run_pipeline must NOT be called when PM returns pipeline=null."""
        from cli.main import cli as kin_cli
        db_path = _seed_db(tmp_path)
        mock_run_agent.return_value = {
            "success": True,
            "output": json.dumps({"pipeline": None, "analysis": "nothing"}),
        }
        runner = CliRunner()
        runner.invoke(kin_cli, ["--db", db_path, "run", "PROJ-001"])
        mock_run_pipeline.assert_not_called()


class TestCliRunTaskEmptyPipeline:
    """PM returns {"pipeline": []} — CLI must exit(1), not create empty pipeline."""

    @patch("agents.runner.run_pipeline")
    @patch("agents.runner.run_agent")
    def test_exit_code_is_1(self, mock_run_agent, mock_run_pipeline, tmp_path):
        """PM returning pipeline=[] → CLI exits with code 1."""
        from cli.main import cli as kin_cli
        db_path = _seed_db(tmp_path)
        mock_run_agent.return_value = {
            "success": True,
            "output": json.dumps({"pipeline": [], "analysis": "nothing to do"}),
        }
        runner = CliRunner()
        result = runner.invoke(kin_cli, ["--db", db_path, "run", "PROJ-001"])
        assert result.exit_code == 1, (
            f"Expected exit_code=1 for empty pipeline, got {result.exit_code}"
        )

    @patch("agents.runner.run_pipeline")
    @patch("agents.runner.run_agent")
    def test_run_pipeline_not_called_on_empty(self, mock_run_agent, mock_run_pipeline, tmp_path):
        """run_pipeline must NOT be called when PM returns pipeline=[]."""
        from cli.main import cli as kin_cli
        db_path = _seed_db(tmp_path)
        mock_run_agent.return_value = {
            "success": True,
            "output": json.dumps({"pipeline": [], "analysis": "nothing to do"}),
        }
        runner = CliRunner()
        runner.invoke(kin_cli, ["--db", db_path, "run", "PROJ-001"])
        mock_run_pipeline.assert_not_called()


# ---------------------------------------------------------------------------
# (6) run_pipeline with steps=[] — task status stays at original value
# ---------------------------------------------------------------------------

class TestRunPipelineEmptyStepsStatusUnchanged:

    @patch("agents.runner.check_claude_auth")
    def test_empty_steps_task_status_stays_todo(self, mock_auth, conn):
        """run_pipeline(steps=[]) must leave task.status unchanged (stays 'todo')."""
        from agents.runner import run_pipeline
        before = models.get_task(conn, "PROJ-001")["status"]
        run_pipeline(conn, "PROJ-001", [])
        after = models.get_task(conn, "PROJ-001")["status"]
        assert after == before, (
            f"Task status changed from '{before}' to '{after}' after empty pipeline. "
            "run_pipeline must not mutate task status when steps=[]."
        )


# ---------------------------------------------------------------------------
# (7/8/9) generate_followups: agent returns "[]"
# ---------------------------------------------------------------------------

class TestGenerateFollowupsEmptyArray:
    """Edge cases when the followup agent returns an empty JSON array '[]'."""

    @patch("agents.runner._run_claude")
    def test_agent_returns_empty_array_gives_empty_result(self, mock_claude, conn):
        """generate_followups: agent returning '[]' → {created: [], pending_actions: []}."""
        from core.followup import generate_followups
        mock_claude.return_value = {"output": "[]", "returncode": 0}
        result = generate_followups(conn, "PROJ-001")
        assert result["created"] == [], (
            f"Expected created=[], got: {result['created']}"
        )
        assert result["pending_actions"] == [], (
            f"Expected pending_actions=[], got: {result['pending_actions']}"
        )

    @patch("agents.runner._run_claude")
    def test_agent_returns_empty_array_creates_no_tasks_in_db(self, mock_claude, conn):
        """generate_followups: agent returning '[]' must not create any task in DB."""
        from core.followup import generate_followups
        mock_claude.return_value = {"output": "[]", "returncode": 0}
        tasks_before = conn.execute("SELECT COUNT(*) FROM tasks").fetchone()[0]
        generate_followups(conn, "PROJ-001")
        tasks_after = conn.execute("SELECT COUNT(*) FROM tasks").fetchone()[0]
        assert tasks_after == tasks_before, (
            f"Expected no new tasks, but count went from {tasks_before} to {tasks_after}. "
            "generate_followups must not create tasks when agent returns []."
        )

    @patch("agents.runner._run_claude")
    def test_no_pipeline_history_still_calls_claude(self, mock_claude, conn):
        """generate_followups: task with no agent_logs must still invoke Claude (no early bail)."""
        from core.followup import generate_followups
        # Verify there are no agent_logs for this task
        log_count = conn.execute(
            "SELECT COUNT(*) FROM agent_logs WHERE task_id = 'PROJ-001'"
        ).fetchone()[0]
        assert log_count == 0, "Precondition: no agent logs must exist"
        mock_claude.return_value = {"output": "[]", "returncode": 0}
        generate_followups(conn, "PROJ-001")
        mock_claude.assert_called_once(), (
            "Claude must be called even when there is no prior pipeline history. "
            "The early-return 'if not pipeline_output' must be removed."
        )


# ---------------------------------------------------------------------------
# (10) API /followup: needs_decision=False when agent returns []
# ---------------------------------------------------------------------------

class TestApiFollowupEmptyArrayNeedsDecision:
    """POST /api/tasks/{id}/followup: needs_decision must be False when agent returns []."""

    @patch("agents.runner._run_claude")
    def test_needs_decision_false_when_empty_array(self, mock_claude, tmp_path):
        """API: agent returning '[]' → needs_decision is False in response."""
        import web.api as api_module
        db_path = tmp_path / "test.db"
        api_module.DB_PATH = db_path
        from web.api import app
        from fastapi.testclient import TestClient

        mock_claude.return_value = {"output": "[]", "returncode": 0}
        c = TestClient(app)
        c.post("/api/projects", json={"id": "p1", "name": "P1", "path": "/p1"})
        c.post("/api/tasks", json={"project_id": "p1", "title": "Fix bug"})

        r = c.post("/api/tasks/P1-001/followup", json={})
        assert r.status_code == 200
        data = r.json()
        assert data["needs_decision"] is False, (
            f"Expected needs_decision=False when agent returns [], got: {data['needs_decision']}"
        )
        assert data["created"] == []
        assert data["pending_actions"] == []


# ---------------------------------------------------------------------------
# Additional edge cases — deeper investigation
# ---------------------------------------------------------------------------

class TestRunPipelineNoneSteps:
    """run_pipeline with steps=None — must also return empty_pipeline, not crash."""

    @patch("agents.runner.check_claude_auth")
    def test_none_steps_returns_empty_pipeline_error(self, mock_auth, conn):
        """run_pipeline(steps=None) must return {success: False, error: 'empty_pipeline'}."""
        from agents.runner import run_pipeline
        result = run_pipeline(conn, "PROJ-001", None)
        assert result["success"] is False
        assert result.get("error") == "empty_pipeline", (
            f"Expected error='empty_pipeline' for None steps, got: {result}"
        )

    @patch("agents.runner.check_claude_auth")
    def test_none_steps_does_not_mutate_task(self, mock_auth, conn):
        """run_pipeline(steps=None) must not change task status."""
        from agents.runner import run_pipeline
        before = models.get_task(conn, "PROJ-001")["status"]
        run_pipeline(conn, "PROJ-001", None)
        after = models.get_task(conn, "PROJ-001")["status"]
        assert after == before, (
            f"Task status changed from '{before}' to '{after}' after None steps"
        )


class TestRunPipelineEmptyStepsDryRun:
    """run_pipeline(steps=[], dry_run=True) — must bail before auth check."""

    def test_empty_steps_dry_run_returns_error_without_auth(self, conn):
        """run_pipeline(steps=[], dry_run=True) must return early without auth check."""
        from agents.runner import run_pipeline
        # No @patch for check_claude_auth — if auth is called, it may raise; empty guard must fire first
        result = run_pipeline(conn, "PROJ-001", [], dry_run=True)
        assert result["success"] is False
        assert result.get("error") == "empty_pipeline"


class TestCliRunTaskNonListPipeline:
    """PM returns pipeline as a non-list non-null value (dict or string) — CLI must exit(1)."""

    @patch("agents.runner.run_pipeline")
    @patch("agents.runner.run_agent")
    def test_dict_pipeline_exits_1(self, mock_run_agent, mock_run_pipeline, tmp_path):
        """PM returning pipeline={} (dict) → CLI exits 1."""
        from cli.main import cli as kin_cli
        db_path = _seed_db(tmp_path)
        mock_run_agent.return_value = {
            "success": True,
            "output": json.dumps({"pipeline": {"steps": []}, "analysis": "..."}),
        }
        runner = CliRunner()
        result = runner.invoke(kin_cli, ["--db", db_path, "run", "PROJ-001"])
        assert result.exit_code == 1, (
            f"Expected exit_code=1 for dict pipeline, got {result.exit_code}"
        )
        mock_run_pipeline.assert_not_called()

    @patch("agents.runner.run_pipeline")
    @patch("agents.runner.run_agent")
    def test_string_pipeline_exits_1(self, mock_run_agent, mock_run_pipeline, tmp_path):
        """PM returning pipeline='[]' (JSON-string-encoded) → CLI exits 1."""
        from cli.main import cli as kin_cli
        db_path = _seed_db(tmp_path)
        mock_run_agent.return_value = {
            "success": True,
            "output": json.dumps({"pipeline": "[]", "analysis": "..."}),
        }
        runner = CliRunner()
        result = runner.invoke(kin_cli, ["--db", db_path, "run", "PROJ-001"])
        assert result.exit_code == 1, (
            f"Expected exit_code=1 for string pipeline, got {result.exit_code}"
        )
        mock_run_pipeline.assert_not_called()


class TestGenerateFollowupsNullAndDict:
    """Additional generate_followups edge cases: null output, dict with empty tasks."""

    @patch("agents.runner._run_claude")
    def test_agent_returns_null_gives_empty_result(self, mock_claude, conn):
        """generate_followups: agent returning 'null' → {created: [], pending_actions: []}."""
        from core.followup import generate_followups
        mock_claude.return_value = {"output": "null", "returncode": 0}
        result = generate_followups(conn, "PROJ-001")
        assert result["created"] == [], f"Expected created=[], got: {result['created']}"
        assert result["pending_actions"] == []

    @patch("agents.runner._run_claude")
    def test_agent_returns_null_creates_no_tasks(self, mock_claude, conn):
        """generate_followups: agent returning 'null' must not create any tasks."""
        from core.followup import generate_followups
        mock_claude.return_value = {"output": "null", "returncode": 0}
        tasks_before = conn.execute("SELECT COUNT(*) FROM tasks").fetchone()[0]
        generate_followups(conn, "PROJ-001")
        tasks_after = conn.execute("SELECT COUNT(*) FROM tasks").fetchone()[0]
        assert tasks_after == tasks_before

    @patch("agents.runner._run_claude")
    def test_agent_returns_dict_with_empty_tasks_list(self, mock_claude, conn):
        """generate_followups: agent returning {"tasks": []} → empty result, no tasks created."""
        from core.followup import generate_followups
        mock_claude.return_value = {"output": '{"tasks": []}', "returncode": 0}
        tasks_before = conn.execute("SELECT COUNT(*) FROM tasks").fetchone()[0]
        result = generate_followups(conn, "PROJ-001")
        tasks_after = conn.execute("SELECT COUNT(*) FROM tasks").fetchone()[0]
        assert result["created"] == []
        assert tasks_after == tasks_before

    @patch("agents.runner._run_claude")
    def test_agent_returns_empty_string_gives_empty_result(self, mock_claude, conn):
        """generate_followups: agent returning '' (empty string) → {created: [], pending_actions: []}."""
        from core.followup import generate_followups
        mock_claude.return_value = {"output": "", "returncode": 0}
        result = generate_followups(conn, "PROJ-001")
        assert result["created"] == [], (
            f"Expected created=[] for empty string output, got: {result['created']}"
        )
        assert result["pending_actions"] == []

    @patch("agents.runner._run_claude")
    def test_agent_returns_whitespace_wrapped_empty_array(self, mock_claude, conn):
        """generate_followups: agent returning '  []  ' (whitespace-wrapped) → no tasks created."""
        from core.followup import generate_followups
        mock_claude.return_value = {"output": "  []  ", "returncode": 0}
        result = generate_followups(conn, "PROJ-001")
        assert result["created"] == [], (
            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."
        )