diff --git a/sova/dashboard/routers/spec.py b/sova/dashboard/routers/spec.py index 2171ce01..3c2dcc11 100644 --- a/sova/dashboard/routers/spec.py +++ b/sova/dashboard/routers/spec.py @@ -58,6 +58,11 @@ async def approve_spec(issue_number: str, req: ApproveRequest | None = None) -> if "error" in result: raise HTTPException(status_code=404, detail=result["error"]) + # Transition the researcher's TaskRun from awaiting_approval to done + updated_run_id = await control_service.complete_awaiting_approval_by_issue(issue_number, "done") + if updated_run_id: + log.info("spec.approve.taskrun_completed", issue=issue_number, run_id=updated_run_id) + # Spawn developer agent, then clear handoff only on success try: agent_result = await control_service.start_agent(issue_number, role="developer") @@ -75,6 +80,11 @@ async def approve_spec(issue_number: str, req: ApproveRequest | None = None) -> @router.post("/{issue_number}/revise") async def revise_spec(issue_number: str) -> dict: """Re-run spec generation. Respawns researcher to run /spec from scratch.""" + # Transition the researcher's TaskRun from awaiting_approval to rejected (spec sent back) + updated_run_id = await control_service.complete_awaiting_approval_by_issue(issue_number, "rejected") + if updated_run_id: + log.info("spec.revise.taskrun_rejected", issue=issue_number, run_id=updated_run_id) + # Respawn researcher to re-run /spec, then clear handoff on success try: agent_result = await control_service.start_agent(issue_number, role="researcher") @@ -91,6 +101,11 @@ async def revise_spec(issue_number: str) -> dict: @router.post("/{issue_number}/skip") async def skip_spec(issue_number: str) -> dict: """Skip spec review and proceed to development.""" + # Transition the researcher's TaskRun from awaiting_approval to done + updated_run_id = await control_service.complete_awaiting_approval_by_issue(issue_number, "done") + if updated_run_id: + log.info("spec.skip.taskrun_completed", issue=issue_number, run_id=updated_run_id) + # Spawn developer without spec, then clear handoff on success try: agent_result = await control_service.start_agent(issue_number, role="developer") @@ -111,5 +126,10 @@ async def reject_spec(issue_number: str) -> dict: if "error" in result: raise HTTPException(status_code=404, detail=result["error"]) + # Transition the researcher's TaskRun from awaiting_approval to rejected + updated_run_id = await control_service.complete_awaiting_approval_by_issue(issue_number, "rejected") + if updated_run_id: + log.info("spec.reject.taskrun_rejected", issue=issue_number, run_id=updated_run_id) + handoff_service.clear_handoff(issue=issue_number) return result diff --git a/sova/dashboard/services/agent_lifecycle.py b/sova/dashboard/services/agent_lifecycle.py index f41d42f4..0064b7f8 100644 --- a/sova/dashboard/services/agent_lifecycle.py +++ b/sova/dashboard/services/agent_lifecycle.py @@ -1327,6 +1327,54 @@ async def resume_from_approval(run_id: int) -> dict: } +async def complete_awaiting_approval_by_issue(issue_number: str, target_status: str = "done") -> int | None: + """Find and transition the most recent awaiting_approval TaskRun for an issue. + + Returns the run ID that was updated, or None if no matching run was found. + Non-fatal: logs warnings on errors but never raises. + """ + from datetime import datetime, timezone + + from sqlalchemy import select, update + + from sova.core.state import TaskStatus + from sova.db.models import TaskRun + from sova.db.session import get_session + + try: + async with await get_session() as session, session.begin(): + stmt = ( + select(TaskRun.id) + .where( + TaskRun.issue_number == issue_number.lstrip("#").strip(), + TaskRun.status == TaskStatus.AWAITING_APPROVAL, + ) + .order_by(TaskRun.started_at.desc()) + .limit(1) + ) + result = await session.execute(stmt) + run_id = result.scalar_one_or_none() + + if run_id is None: + return None + + async with await get_session() as session, session.begin(): + cas = await session.execute( + update(TaskRun) + .where(TaskRun.id == run_id, TaskRun.status == TaskStatus.AWAITING_APPROVAL) + .values(status=target_status, ended_at=datetime.now(timezone.utc)) + ) + if cas.rowcount == 0: + log.debug("complete_awaiting_approval.cas_failed", run_id=run_id, issue=issue_number) + return None + + log.info("complete_awaiting_approval.done", run_id=run_id, issue=issue_number, target=target_status) + return run_id + except Exception: + log.warning("complete_awaiting_approval.failed", issue=issue_number, exc_info=True) + return None + + async def reject_spec(run_id: int) -> dict: """Reject a spec and mark the awaiting_approval run as rejected. diff --git a/sova/dashboard/services/control_service.py b/sova/dashboard/services/control_service.py index ddd61ffb..cc8b874d 100644 --- a/sova/dashboard/services/control_service.py +++ b/sova/dashboard/services/control_service.py @@ -49,6 +49,9 @@ from sova.dashboard.services.agent_lifecycle import ( _wait_and_finalize as _wait_and_finalize, ) +from sova.dashboard.services.agent_lifecycle import ( + complete_awaiting_approval_by_issue as complete_awaiting_approval_by_issue, +) from sova.dashboard.services.agent_lifecycle import ( get_all_agents as get_all_agents, ) diff --git a/tests/test_spec.py b/tests/test_spec.py index 55413c22..b4d78f7b 100644 --- a/tests/test_spec.py +++ b/tests/test_spec.py @@ -1510,3 +1510,219 @@ def test_list_pending_specs_none_project_dir(self) -> None: with patch("sova.dashboard.services.spec_service.get_project_dir", return_value=None): assert list_pending_specs(project_dir=None) == [] + + +# --------------------------------------------------------------------------- +# complete_awaiting_approval_by_issue +# --------------------------------------------------------------------------- + + +class TestCompleteAwaitingApprovalByIssue: + """Tests for the function that transitions awaiting_approval TaskRuns.""" + + async def _create_awaiting_run(self, issue: str, role: str = "researcher") -> int: + from sova.db.models import TaskRun + from sova.db.session import get_session + + async with await get_session() as session, session.begin(): + run = TaskRun(issue_number=issue, role=role, status="awaiting_approval", current_step="spec") + session.add(run) + await session.flush() + return run.id + + async def test_transitions_to_done(self) -> None: + from sova.dashboard.services.agent_lifecycle import complete_awaiting_approval_by_issue + from sova.db.models import TaskRun + from sova.db.session import get_session + + run_id = await self._create_awaiting_run("42") + + result = await complete_awaiting_approval_by_issue("42", "done") + + assert result == run_id + async with await get_session() as session: + run = await session.get(TaskRun, run_id) + assert run.status == "done" + assert run.ended_at is not None + + async def test_transitions_to_rejected(self) -> None: + from sova.dashboard.services.agent_lifecycle import complete_awaiting_approval_by_issue + from sova.db.models import TaskRun + from sova.db.session import get_session + + run_id = await self._create_awaiting_run("42") + + result = await complete_awaiting_approval_by_issue("42", "rejected") + + assert result == run_id + async with await get_session() as session: + run = await session.get(TaskRun, run_id) + assert run.status == "rejected" + assert run.ended_at is not None + + async def test_returns_none_when_no_matching_run(self) -> None: + from sova.dashboard.services.agent_lifecycle import complete_awaiting_approval_by_issue + + result = await complete_awaiting_approval_by_issue("999", "done") + + assert result is None + + async def test_strips_hash_prefix(self) -> None: + from sova.dashboard.services.agent_lifecycle import complete_awaiting_approval_by_issue + from sova.db.models import TaskRun + from sova.db.session import get_session + + run_id = await self._create_awaiting_run("42") + + result = await complete_awaiting_approval_by_issue("#42", "done") + + assert result == run_id + async with await get_session() as session: + run = await session.get(TaskRun, run_id) + assert run.status == "done" + + async def test_picks_most_recent_run(self) -> None: + from sova.dashboard.services.agent_lifecycle import complete_awaiting_approval_by_issue + from sova.db.models import TaskRun + from sova.db.session import get_session + + old_id = await self._create_awaiting_run("42") + new_id = await self._create_awaiting_run("42") + assert new_id > old_id + + result = await complete_awaiting_approval_by_issue("42", "done") + + assert result == new_id + async with await get_session() as session: + new_run = await session.get(TaskRun, new_id) + old_run = await session.get(TaskRun, old_id) + assert new_run.status == "done" + assert old_run.status == "awaiting_approval" + + async def test_ignores_non_awaiting_runs(self) -> None: + from sova.dashboard.services.agent_lifecycle import complete_awaiting_approval_by_issue + from sova.db.models import TaskRun + from sova.db.session import get_session + + async with await get_session() as session, session.begin(): + run = TaskRun(issue_number="42", role="researcher", status="done", current_step="spec") + session.add(run) + + result = await complete_awaiting_approval_by_issue("42", "done") + assert result is None + + async def test_nonfatal_on_db_error(self) -> None: + from sova.dashboard.services.agent_lifecycle import complete_awaiting_approval_by_issue + + with patch("sova.db.session.get_session", side_effect=RuntimeError("DB down")): + result = await complete_awaiting_approval_by_issue("42", "done") + + assert result is None + + +# --------------------------------------------------------------------------- +# Spec router -- TaskRun transition on approve/reject/skip +# --------------------------------------------------------------------------- + + +class TestSpecRouterTaskRunTransition: + """Tests that spec router endpoints transition the awaiting_approval TaskRun.""" + + async def _create_awaiting_run(self, issue: str) -> int: + from sova.db.models import TaskRun + from sova.db.session import get_session + + async with await get_session() as session, session.begin(): + run = TaskRun(issue_number=issue, role="researcher", status="awaiting_approval", current_step="spec") + session.add(run) + await session.flush() + return run.id + + async def test_approve_transitions_taskrun_to_done(self) -> None: + from sova.dashboard.routers.spec import approve_spec + from sova.dashboard.services import control_service, handoff_service, spec_service + from sova.db.models import TaskRun + from sova.db.session import get_session + + run_id = await self._create_awaiting_run("42") + + with ( + patch.object(spec_service, "approve_spec", return_value={"status": "approved"}), + patch.object(spec_service, "write_answers"), + patch.object(control_service, "start_agent", new_callable=AsyncMock, return_value={"pid": 123}), + patch.object(handoff_service, "clear_handoff"), + ): + await approve_spec("42") + + async with await get_session() as session: + run = await session.get(TaskRun, run_id) + assert run.status == "done" + + async def test_reject_transitions_taskrun_to_rejected(self) -> None: + from sova.dashboard.routers.spec import reject_spec + from sova.dashboard.services import handoff_service, spec_service + from sova.db.models import TaskRun + from sova.db.session import get_session + + run_id = await self._create_awaiting_run("42") + + with ( + patch.object(spec_service, "reject_spec", return_value={"status": "rejected", "issue_number": "42"}), + patch.object(handoff_service, "clear_handoff"), + ): + await reject_spec("42") + + async with await get_session() as session: + run = await session.get(TaskRun, run_id) + assert run.status == "rejected" + + async def test_skip_transitions_taskrun_to_done(self) -> None: + from sova.dashboard.routers.spec import skip_spec + from sova.dashboard.services import control_service, handoff_service + from sova.db.models import TaskRun + from sova.db.session import get_session + + run_id = await self._create_awaiting_run("42") + + with ( + patch.object(control_service, "start_agent", new_callable=AsyncMock, return_value={"pid": 456}), + patch.object(handoff_service, "clear_handoff"), + ): + await skip_spec("42") + + async with await get_session() as session: + run = await session.get(TaskRun, run_id) + assert run.status == "done" + + async def test_revise_transitions_taskrun_to_rejected(self) -> None: + from sova.dashboard.routers.spec import revise_spec + from sova.dashboard.services import control_service, handoff_service + from sova.db.models import TaskRun + from sova.db.session import get_session + + run_id = await self._create_awaiting_run("42") + + with ( + patch.object(control_service, "start_agent", new_callable=AsyncMock, return_value={"pid": 789}), + patch.object(handoff_service, "clear_handoff"), + ): + await revise_spec("42") + + async with await get_session() as session: + run = await session.get(TaskRun, run_id) + assert run.status == "rejected" + + async def test_approve_succeeds_without_awaiting_run(self) -> None: + """Approve works even if no awaiting_approval TaskRun exists (non-fatal).""" + from sova.dashboard.routers.spec import approve_spec + from sova.dashboard.services import control_service, handoff_service, spec_service + + with ( + patch.object(spec_service, "approve_spec", return_value={"status": "approved"}), + patch.object(spec_service, "write_answers"), + patch.object(control_service, "start_agent", new_callable=AsyncMock, return_value={"pid": 123}), + patch.object(handoff_service, "clear_handoff"), + ): + result = await approve_spec("42") + + assert result["status"] == "approved"