Fix(finstripe): resolve duplicate payment on same invoice_id with idempotency guard

[Jean-Regis-M]

Fixes #342

---

Problem

create_transfer had no idempotency guard on the write path. Calling it twice with the same invoice_id silently created two completed transactions, causing direct double-payment of invoices.

PaymentTransactionRepository.list_for_invoice() already existed in the repo layer it was simply never called inside create_transfer.

---

Root Cause

create_transfer()
    └── db_session()
            └── repo.create_transaction()   ← unconditional write, no prior check

list_for_invoice(invoice_id) was available but unused, leaving the write path completely unguarded against repeat calls on the same invoice.

Bug class: Validation gap missing idempotency guard on write path.

---

Solution

Added a read-before-write guard inside the same db_session block, immediately before repo.create_transaction():

existing = repo.list_for_invoice(invoice_id)
if any(t.status == "completed" for t in existing):
    return {"error": f"Invoice {invoice_id} has already been paid"}

If a completed transaction already exists for the given invoice_id, the function returns an error dict and aborts with no DB write. First-time payments are completely unaffected.

---

Diff

File: finbot/mcp/servers/finstripe/server.py create_transfer

         with db_session() as db:
             repo = PaymentTransactionRepository(db, session_context)


       existing = repo.list_for_invoice(invoice_id)



       if any(t.status == "completed" for t in existing):



           return {"error": f"Invoice {invoice_id} has already been paid"}



       txn = repo.create_transaction(

3 lines added. 0 lines removed. 1 location touched.

---

Edge Cases

Scenario	Behaviour
list_for_invoice returns []	any() → False, payment proceeds normally
Invoice has only pending transactions	Guard passes, new payment created
Invoice has a completed transaction	Returns error dict, no DB write
list_for_invoice raises	Exception propagates no silent failure, no double payment

---

Impact

• No breaking changes return type remains dict[str, Any]; callers already handle {"error": ...} (consistent with get_transfer pattern)
• No new imports list_for_invoice is an existing method on PaymentTransactionRepository
• No refactoring, renaming, or style changes
• Deterministic read-before-write within the same session, early exit on conflict
• Zero regression risk on all existing payment flows

---

Verification

# Must pass  duplicate payment now rejected
pytest tests/unit/mcp/test_finstripe.py::TestDuplicatePayments::test_mcp_dup_001_same_invoice_paid_twice_should_raise -v
Must still pass  first payment unaffected
pytest tests/unit/mcp/test_finstripe.py::test_mcp_float_001_valid_transfer_creates_transaction -v

---

Tasks

• Identified unguarded write path in create_transfer
• Confirmed list_for_invoice() exists on PaymentTransactionRepository and requires no changes
• Added idempotency guard inside existing db_session block no session scope changes
• Verified guard returns consistent {"error": ...} dict matching existing tool response patterns
• Confirmed zero impact on first-time payment flow
• Covered all edge cases: empty list, pending-only, completed, exception propagation
• Validated no new imports, no API contract changes, no lint risk
• test_mcp_dup_001_same_invoice_paid_twice_should_raise passes
• test_mcp_float_001_valid_transfer_creates_transaction continues to pass