From 2d24ca5234452b34e016bac3d8920d44a40a70f9 Mon Sep 17 00:00:00 2001 From: rohanbuilds-ai Date: Tue, 20 Oct 2026 21:14:08 +0530 Subject: [PATCH] fix(orchestrator): omit payment_confirmation next_step on unsuccessful payments --- finbot/agents/orchestrator.py | 11 +- tests/unit/agents/test_orchestrator.py | 134 +++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 5 deletions(-) create mode 100644 tests/unit/agents/test_orchestrator.py diff --git a/finbot/agents/orchestrator.py b/finbot/agents/orchestrator.py index 796a248a..e3612d80 100644 --- a/finbot/agents/orchestrator.py +++ b/finbot/agents/orchestrator.py @@ -530,11 +530,12 @@ async def delegate_to_payments( await self._emit_delegation_event("payments_agent", result) self._capture_agent_context("payments_agent", result) - result["next_step"] = ( - "IMPORTANT: You MUST now delegate_to_communication to notify the vendor " - "about this payment outcome. Use notification_type 'payment_confirmation'. " - "Do NOT call complete_task until the vendor has been notified." - ) + if result.get("task_status") in ("completed", "success"): + result["next_step"] = ( + "IMPORTANT: You MUST now delegate_to_communication to notify the vendor " + "about this payment outcome. Use notification_type 'payment_confirmation'. " + "Do NOT call complete_task until the vendor has been notified." + ) return result @agent_tool diff --git a/tests/unit/agents/test_orchestrator.py b/tests/unit/agents/test_orchestrator.py new file mode 100644 index 00000000..7184e7a0 --- /dev/null +++ b/tests/unit/agents/test_orchestrator.py @@ -0,0 +1,134 @@ +"""Unit tests for OrchestratorAgent, focusing on delegation workflows and QA findings.""" + +from datetime import UTC, datetime, timedelta +from unittest.mock import AsyncMock, patch + +import pytest + +from finbot.agents.orchestrator import OrchestratorAgent +from finbot.core.auth.session import SessionContext + + +@pytest.fixture(autouse=True) +def mock_event_bus(): + """Mock event_bus emission to avoid network/Redis dependencies in unit tests.""" + with ( + patch( + "finbot.core.messaging.event_bus.emit_agent_event", + new_callable=AsyncMock, + ), + patch( + "finbot.core.messaging.event_bus.emit_business_event", + new_callable=AsyncMock, + ), + ): + yield + + +@pytest.fixture +def session_context(): + now = datetime.now(UTC) + return SessionContext( + session_id="sess-orch-001", + user_id="test-user-orch", + is_temporary=False, + namespace="default", + created_at=now, + expires_at=now + timedelta(hours=1), + ) + + +@pytest.fixture +def orchestrator_agent(session_context): + return OrchestratorAgent( + session_context=session_context, + workflow_id="wf-test-orch-001", + ) + + +class TestQAFindings: + """Regression tests for Orchestrator QA audit findings.""" + + @pytest.mark.asyncio + async def test_orch_qa_002_next_step_on_failed_payment_misleads_llm(self, orchestrator_agent): + """When payments agent fails, next_step must not instruct payment_confirmation.""" + failed_payment_result = { + "task_status": "failed", + "task_summary": "Payment declined — insufficient funds.", + } + + with patch( + "finbot.agents.runner.run_payments_agent", + new_callable=AsyncMock, + return_value=failed_payment_result, + ): + result = await orchestrator_agent.delegate_to_payments( + invoice_id=1, + task_description="Process vendor invoice payment", + ) + + assert result["task_status"] == "failed" + assert "next_step" not in result + + @pytest.mark.asyncio + async def test_delegate_to_payments_completed_includes_next_step(self, orchestrator_agent): + """When payments agent succeeds with completed, next_step is attached.""" + success_result = { + "task_status": "completed", + "task_summary": "Payment settled successfully via Stripe.", + } + + with patch( + "finbot.agents.runner.run_payments_agent", + new_callable=AsyncMock, + return_value=success_result, + ): + result = await orchestrator_agent.delegate_to_payments( + invoice_id=1, + task_description="Process vendor invoice payment", + ) + + assert result["task_status"] == "completed" + assert "next_step" in result + assert "payment_confirmation" in result["next_step"] + + @pytest.mark.asyncio + async def test_delegate_to_payments_success_status_includes_next_step(self, orchestrator_agent): + """When payments agent succeeds with success, next_step is attached.""" + success_result = { + "task_status": "success", + "task_summary": "Payment completed.", + } + + with patch( + "finbot.agents.runner.run_payments_agent", + new_callable=AsyncMock, + return_value=success_result, + ): + result = await orchestrator_agent.delegate_to_payments( + invoice_id=1, + task_description="Process vendor invoice payment", + ) + + assert result["task_status"] == "success" + assert "next_step" in result + assert "payment_confirmation" in result["next_step"] + + @pytest.mark.asyncio + async def test_delegate_to_payments_missing_status_no_next_step(self, orchestrator_agent): + """When payments agent result has no task_status, next_step must not be added.""" + ambiguous_result = { + "task_summary": "Incomplete response without status", + } + + with patch( + "finbot.agents.runner.run_payments_agent", + new_callable=AsyncMock, + return_value=ambiguous_result, + ): + result = await orchestrator_agent.delegate_to_payments( + invoice_id=1, + task_description="Process vendor invoice payment", + ) + + assert "next_step" not in result