[rohanbuilds-ai]

Summary

Fixes #460: Guard next_step in OrchestratorAgent.delegate_to_payments to only instruct sending a payment_confirmation notification when the payment was actually successful (completed or success).

Problem

In finbot/agents/orchestrator.py (delegate_to_payments), the orchestrator unconditionally appended a next_step directive to the returned delegation dictionary:

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

When run_payments_agent returned task_status="failed" (e.g. Insufficient funds, declined card, invalid payment parameters, or retry exhaustion), this directive still told the orchestrator LLM:

"IMPORTANT: You MUST now delegate_to_communication to notify the vendor about this payment outcome. Use notification_type 'payment_confirmation'."

This actively misled the LLM into notifying the vendor of a successful payment that never occurred — creating data integrity, customer confusion, and compliance risks in a financial processing workflow.

Solution

• In OrchestratorAgent.delegate_to_payments:
  • Check if result.get("task_status") in ("completed", "success"): before attaching the payment_confirmation next_step.
  • On failed, declined, missing, or error statuses, omit the instruction so the LLM handles the failure without sending an inaccurate confirmation.
• Add focused regression unit tests in tests/unit/agents/test_orchestrator.py.

Verification & Test Evidence

1. Baseline Reproduction on origin/main:
   Running the regression tests against unmodified origin/main confirmed the bug:

   FAILED tests/unit/agents/test_orchestrator.py::TestQAFindings::test_orch_qa_002_next_step_on_failed_payment_misleads_llm
   E   assert 'next_step' not in {'task_status': 'failed', 'task_summary': 'Payment declined — insufficient funds.', '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."}
   FAILED tests/unit/agents/test_orchestrator.py::TestQAFindings::test_delegate_to_payments_missing_status_no_next_step
   E   assert 'next_step' not in {'task_summary': 'Incomplete response without status', 'next_step': "IMPORTANT: ..."}
   

2. Post-Fix Focused Tests:
   All 4 tests in tests/unit/agents/test_orchestrator.py pass:

   tests/unit/agents/test_orchestrator.py::TestQAFindings::test_orch_qa_002_next_step_on_failed_payment_misleads_llm PASSED [ 25%]
   tests/unit/agents/test_orchestrator.py::TestQAFindings::test_delegate_to_payments_completed_includes_next_step PASSED [ 50%]
   tests/unit/agents/test_orchestrator.py::TestQAFindings::test_delegate_to_payments_success_status_includes_next_step PASSED [ 75%]
   tests/unit/agents/test_orchestrator.py::TestQAFindings::test_delegate_to_payments_missing_status_no_next_step PASSED [100%]
   ============================== 4 passed in 0.33s ==============================
   

3. Full Agent Suite:
   Reran full tests/unit/agents/ suite: 55 passed, 15 skipped, 3 failed.

   • The 3 failures (test_sai_von_005_onboarding_tracking, test_sai_edge_001_invoice_agent_empty_task_data_prompt, test_sai_edge_002_vendor_agent_empty_task_data_prompt) were independently verified on origin/main to be pre-existing upstream issues (addressed in PR fix(agents): handle unresolvable task_data IDs #595).

4. Formatting & Diff:

   • tests/unit/agents/test_orchestrator.py is formatted with black and isort.
   • git diff --check passes with zero whitespace errors.
   • Diff is strictly confined to finbot/agents/orchestrator.py and tests/unit/agents/test_orchestrator.py.

[rohanbuilds-ai]

Closing this PR because I found that #574, opened earlier by @Deez-Automations, addresses the same issue. Thank you for the prior implementation and for maintaining the project. I'll focus on distinct security improvements instead.