fix: guard against double-approve/reject on resolved approvals

backend/app/api/approvals.py

@@ -417,6 +417,7 @@ async def create_approval(
         confidence=payload.confidence,
         rubric_scores=payload.rubric_scores,
         status=payload.status,
+        resolved_at=utcnow() if payload.status != "pending" else None,
     )
     session.add(approval)
     await session.flush()
@@ -443,29 +444,36 @@ async def update_approval(
     session: AsyncSession = SESSION_DEP,
 ) -> ApprovalRead:
     """Update an approval's status and resolution timestamp."""
-    approval = await Approval.objects.by_id(approval_id).first(session)
+    try:
+        approval_lookup_id = UUID(approval_id)
+    except ValueError as exc:
+        raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) from exc
+
+    approval = await Approval.objects.by_id(approval_lookup_id).first(session)
     if approval is None or approval.board_id != board.id:
         raise HTTPException(status_code=status.HTTP_404_NOT_FOUND)
     updates = payload.model_dump(exclude_unset=True)
     prior_status = approval.status
     if "status" in updates:
         target_status = updates["status"]
-        if target_status == "pending" and prior_status != "pending":
-            task_ids_by_approval = await load_task_ids_by_approval(
-                session, approval_ids=[approval.id]
-            )
-            approval_task_ids = task_ids_by_approval.get(approval.id)
-            if not approval_task_ids and approval.task_id is not None:
-                approval_task_ids = [approval.task_id]
-            await _ensure_no_pending_approval_conflicts(
-                session,
-                board_id=board.id,
-                task_ids=approval_task_ids or [],
-                exclude_approval_id=approval.id,
-            )
-        approval.status = target_status
-        if approval.status != "pending":
-            approval.resolved_at = utcnow()
+        if prior_status in ("approved", "rejected"):
+            # Legacy rows may be resolved without a timestamp; permit same-status backfill only.
+            if target_status == prior_status and approval.resolved_at is None:
+                approval.resolved_at = utcnow()
+            else:
+                raise HTTPException(
+                    status_code=status.HTTP_409_CONFLICT,
+                    detail={
+                        "message": f"Approval is already {prior_status}.",
+                        "approval_id": str(approval.id),
+                        "current_status": prior_status,
+                        "requested_status": target_status,
+                    },
+                )
+        else:
+            approval.status = target_status
+            if approval.status != "pending":
+                approval.resolved_at = utcnow()
     session.add(approval)
     await session.commit()
     await session.refresh(approval)

backend/tests/test_approvals_pending_conflicts.py

@@ -9,6 +9,7 @@
 from sqlmodel.ext.asyncio.session import AsyncSession
 
 from app.api import approvals as approvals_api
+from app.models.approvals import Approval
 from app.models.boards import Board
 from app.models.organizations import Organization
 from app.models.tasks import Task
@@ -131,23 +132,183 @@ async def test_create_approval_rejects_pending_conflict_from_linked_task_ids() -
 
 
 @pytest.mark.asyncio
-async def test_update_approval_rejects_reopening_to_pending_with_existing_pending() -> None:
+async def test_create_approval_sets_resolved_at_for_resolved_status() -> None:
     engine = await _make_engine()
     try:
         async with await _make_session(engine) as session:
             board, task_ids = await _seed_board_with_tasks(session, task_count=1)
             task_id = task_ids[0]
-            pending = await approvals_api.create_approval(
+            created = await approvals_api.create_approval(
                 payload=ApprovalCreate(
                     action_type="task.execute",
                     task_id=task_id,
-                    payload={"reason": "Primary pending approval is active."},
-                    confidence=83,
-                    status="pending",
+                    payload={"reason": "Resolved before persistence."},
+                    confidence=88,
+                    status="approved",
+                ),
+                board=board,
+                session=session,
+            )
+
+            assert created.status == "approved"
+            assert created.resolved_at is not None
+    finally:
+        await engine.dispose()
+
+
+@pytest.mark.asyncio
+async def test_update_approval_backfills_resolved_at_for_legacy_resolved_approval() -> None:
+    engine = await _make_engine()
+    try:
+        async with await _make_session(engine) as session:
+            board, task_ids = await _seed_board_with_tasks(session, task_count=1)
+            task_id = task_ids[0]
+            approval = Approval(
+                board_id=board.id,
+                task_id=task_id,
+                action_type="task.execute",
+                payload={"reason": "Legacy resolved approval."},
+                confidence=80,
+                status="approved",
+            )
+            session.add(approval)
+            await session.commit()
+
+            updated = await approvals_api.update_approval(
+                approval_id=str(approval.id),
+                payload=ApprovalUpdate(status="approved"),
+                board=board,
+                session=session,
+            )
+
+            assert updated.status == "approved"
+            assert updated.resolved_at is not None
+    finally:
+        await engine.dispose()
+
+
+@pytest.mark.asyncio
+async def test_update_approval_rejects_double_approve() -> None:
+    """Re-approving an already-approved approval must return 409."""
+    engine = await _make_engine()
+    try:
+        async with await _make_session(engine) as session:
+            board, task_ids = await _seed_board_with_tasks(session, task_count=1)
+            task_id = task_ids[0]
+            approval = await approvals_api.create_approval(
+                payload=ApprovalCreate(
+                    action_type="task.execute",
+                    task_id=task_id,
+                    payload={"reason": "Needs sign-off."},
+                    confidence=80,
+                    status="approved",
+                ),
+                board=board,
+                session=session,
+            )
+
+            with pytest.raises(HTTPException) as exc:
+                await approvals_api.update_approval(
+                    approval_id=str(approval.id),
+                    payload=ApprovalUpdate(status="approved"),
+                    board=board,
+                    session=session,
+                )
+
+            assert exc.value.status_code == 409
+            detail = exc.value.detail
+            assert isinstance(detail, dict)
+            assert detail["message"] == "Approval is already approved."
+            assert detail["current_status"] == "approved"
+            assert detail["requested_status"] == "approved"
+    finally:
+        await engine.dispose()
+
+
+@pytest.mark.asyncio
+async def test_update_approval_rejects_double_reject() -> None:
+    """Re-rejecting an already-rejected approval must return 409."""
+    engine = await _make_engine()
+    try:
+        async with await _make_session(engine) as session:
+            board, task_ids = await _seed_board_with_tasks(session, task_count=1)
+            task_id = task_ids[0]
+            approval = await approvals_api.create_approval(
+                payload=ApprovalCreate(
+                    action_type="task.execute",
+                    task_id=task_id,
+                    payload={"reason": "Needs sign-off."},
+                    confidence=80,
+                    status="rejected",
+                ),
+                board=board,
+                session=session,
+            )
+
+            with pytest.raises(HTTPException) as exc:
+                await approvals_api.update_approval(
+                    approval_id=str(approval.id),
+                    payload=ApprovalUpdate(status="rejected"),
+                    board=board,
+                    session=session,
+                )
+
+            assert exc.value.status_code == 409
+            detail = exc.value.detail
+            assert isinstance(detail, dict)
+            assert detail["message"] == "Approval is already rejected."
+            assert detail["current_status"] == "rejected"
+            assert detail["requested_status"] == "rejected"
+    finally:
+        await engine.dispose()
+
+
+@pytest.mark.asyncio
+async def test_update_approval_rejects_approving_rejected() -> None:
+    """Approving a rejected approval must return 409 (resolved approvals are immutable)."""
+    engine = await _make_engine()
+    try:
+        async with await _make_session(engine) as session:
+            board, task_ids = await _seed_board_with_tasks(session, task_count=1)
+            task_id = task_ids[0]
+            approval = await approvals_api.create_approval(
+                payload=ApprovalCreate(
+                    action_type="task.execute",
+                    task_id=task_id,
+                    payload={"reason": "Needs sign-off."},
+                    confidence=80,
+                    status="rejected",
                 ),
                 board=board,
                 session=session,
             )
+
+            with pytest.raises(HTTPException) as exc:
+                await approvals_api.update_approval(
+                    approval_id=str(approval.id),
+                    payload=ApprovalUpdate(status="approved"),
+                    board=board,
+                    session=session,
+                )
+
+            assert exc.value.status_code == 409
+            detail = exc.value.detail
+            assert isinstance(detail, dict)
+            assert detail["message"] == "Approval is already rejected."
+            assert detail["current_status"] == "rejected"
+            assert detail["requested_status"] == "approved"
+    finally:
+        await engine.dispose()
+
+
+@pytest.mark.asyncio
+async def test_update_approval_rejects_reopening_resolved_to_pending() -> None:
+    """Resolved approvals are immutable — cannot reopen to pending."""
+    engine = await _make_engine()
+    try:
+        async with await _make_session(engine) as session:
+            board, task_ids = await _seed_board_with_tasks(session, task_count=1)
+            task_id = task_ids[0]
             resolved = await approvals_api.create_approval(
                 payload=ApprovalCreate(
                     action_type="task.review",
@@ -162,7 +323,7 @@ async def test_update_approval_rejects_reopening_to_pending_with_existing_pendin
 
             with pytest.raises(HTTPException) as exc:
                 await approvals_api.update_approval(
-                    approval_id=resolved.id,  # type: ignore[arg-type]
+                    approval_id=str(resolved.id),
                     payload=ApprovalUpdate(status="pending"),
                     board=board,
                     session=session,
@@ -171,12 +332,8 @@ async def test_update_approval_rejects_reopening_to_pending_with_existing_pendin
             assert exc.value.status_code == 409
             detail = exc.value.detail
             assert isinstance(detail, dict)
-            assert detail["message"] == "Each task can have only one pending approval."
-            assert detail["conflicts"] == [
-                {
-                    "task_id": str(task_id),
-                    "approval_id": str(pending.id),
-                },
-            ]
+            assert detail["message"] == "Approval is already approved."
+            assert detail["current_status"] == "approved"
+            assert detail["requested_status"] == "pending"
     finally:
         await engine.dispose()