288 lines
13 KiB
Python
288 lines
13 KiB
Python
|
"""Regression tests for KIN-111 — empty/null pipeline returned by PM agent.
|
||
|
|
|
||
|
|
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
|
||
|
|
|
||
|
|
Fixes required:
|
||
|
|
cli/main.py: validate pipeline_steps after extraction — non-empty list required
|
||
|
|
agents/runner.py: run_pipeline early-return when steps=[]
|
||
|
|
|
||
|
|
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
|
||
|
|
"""
|
||
|
|
|
||
|
|
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"] == []
|