Fix/1174 payment webhook state

backend/docs/ISSUE_1174_WEBHOOK_CORRECTNESS.md

@@ -0,0 +1,16 @@
+# Payment webhook correctness (#1174)
+
+This increment protects fiat payment state against duplicate and out-of-order bank webhook delivery.
+
+- Duplicate state notifications have no timestamp or event-history side effects.
+- Only `processing -> settled/failed/returned` and `settled -> returned` are accepted.
+- Provider events cannot bypass manual approval or compliance holds.
+- Invalid transitions raise `INVALID_PAYMENT_STATUS_TRANSITION`.
+
+Run regression tests from `backend`:
+
+```sh
+npx vitest run src/services/__tests__/fiat-webhook-correctness.test.ts
+```
+
+Operational note: The webhook HTTP handler should translate invalid transitions to a suitable permanent response or acknowledgement to avoid repeated provider redelivery.

backend/docs/ISSUE_1177_GATEWAY_ACCOUNTING.md

@@ -0,0 +1,16 @@
+# Gateway response accounting (#1177)
+
+This increment corrects the health metrics and circuit breaker behavior in `BaseGatewayAdapter`.
+
+- Returned payment responses with `success: false` or `status: failed` count as failures, even when the provider call does not throw.
+- Repeated failed responses open the circuit at the configured threshold.
+- A subsequent successful response resets the consecutive-failure count.
+- The adapter retains its existing response contract rather than throwing for returned payment failures.
+
+Run regression tests from `backend`:
+
+```sh
+npx vitest run src/integrations/__tests__/gateway-adapter-result.test.ts
+```
+
+The `GatewayManager` does not fail over for returned failed response objects; adjusting its retry/failover policy requires separate safeguards against ambiguous outcomes and duplicate charges.

backend/src/integrations/__tests__/gateway-adapter-result.test.ts

@@ -0,0 +1,46 @@
+import { describe, expect, it } from 'vitest';
+import {
+  BaseGatewayAdapter, type GatewayConfig, type GatewayType,
+  type PaymentRequest, type PaymentResponse,
+} from '../gateway-adapter.js';
+
+const config: GatewayConfig = {
+  gatewayType: 'stripe', enabled: true, priority: 1, maxRetries: 0,
+  timeoutMs: 1000, healthCheckIntervalMs: 60000,
+  circuitBreakerThreshold: 2, credentials: {},
+};
+
+class StubGateway extends BaseGatewayAdapter {
+  response: PaymentResponse = { success: false, transactionId: 'tx1', status: 'failed' };
+  constructor() { super(config); }
+  getType(): GatewayType { return 'stripe'; }
+  protected async executePayment(_request: PaymentRequest): Promise<PaymentResponse> {
+    return this.response;
+  }
+  protected async performHealthCheck(): Promise<void> {}
+}
+
+const request: PaymentRequest = {
+  amount: '1.00', currency: 'USD', recipient: 'test', idempotencyKey: 'key',
+};
+
+describe('gateway result accounting (#1177)', () => {
+  it('records returned failures and opens circuit at threshold', async () => {
+    const gateway = new StubGateway();
+    await gateway.processPayment(request);
+    expect(gateway.getHealthStatus().consecutiveFailures).toBe(1);
+    expect(gateway.getHealthStatus().successRate).toBe(0);
+    await gateway.processPayment(request);
+    expect(gateway.getHealthStatus().isHealthy).toBe(false);
+    await expect(gateway.processPayment(request)).rejects.toThrow('circuit is open');
+  });
+
+  it('resets failure streak after successful response', async () => {
+    const gateway = new StubGateway();
+    await gateway.processPayment(request);
+    gateway.response = { success: true, transactionId: 'tx2', status: 'completed' };
+    await gateway.processPayment(request);
+    expect(gateway.getHealthStatus().consecutiveFailures).toBe(0);
+    expect(gateway.getHealthStatus().successRate).toBe(0.5);
+  });
+});

backend/src/integrations/gateway-adapter.ts

@@ -124,6 +124,19 @@ export abstract class BaseGatewayAdapter implements IGatewayAdapter {
       const response = await this.executePayment(request);
       const processingTime = Date.now() - startTime;
 
+      // A gateway may resolve successfully at the transport layer while
+      // returning a failed payment. Such responses must count as failures.
+      if (!response.success || response.status === 'failed') {
+        this.recordFailure(processingTime);
+        if (
+          this.health.consecutiveFailures >= this.config.circuitBreakerThreshold ||
+          this.circuitState === CircuitState.HalfOpen
+        ) {
+          this.openCircuit();
+        }
+        return response;
+      }
+
       this.recordSuccess(processingTime);
 
       // Close circuit if half-open

backend/src/services/__tests__/fiat-webhook-correctness.test.ts

@@ -0,0 +1,49 @@
+import { beforeEach, describe, expect, it } from 'vitest';
+import { fiatPaymentsService } from '../fiat-payments.js';
+
+describe('fiat payment webhook correctness (#1174)', () => {
+  beforeEach(() => fiatPaymentsService.resetForTests());
+
+  function makePayment(amount = 100) {
+    const account = fiatPaymentsService.createBankAccount({
+      accountHolderName: 'Test', bankName: 'Test Bank',
+      accountNumberMasked: '****1111', routingNumber: '021000021',
+      verificationMethod: 'plaid', countryCode: 'US',
+    });
+    return fiatPaymentsService.createPayment({
+      method: 'ach', bankAccountId: account.id,
+      recipient: { name: 'Payee', accountNumberMasked: '****2222',
+        routingNumber: '021000021', bankName: 'Bank', countryCode: 'US' },
+      amount, currency: 'USD', isInternational: false,
+    });
+  }
+
+  it('prevents settlement from bypassing approval', () => {
+    const payment = makePayment(20000);
+    expect(payment.status).toBe('pending_approval');
+    expect(() => fiatPaymentsService.handleWebhook({ paymentId: payment.id, eventType: 'settled' }))
+      .toThrow('INVALID_PAYMENT_STATUS_TRANSITION');
+    expect(fiatPaymentsService.getPayment(payment.id)?.status).toBe('pending_approval');
+  });
+
+  it('deduplicates settlement and rejects stale processing notifications', () => {
+    const payment = makePayment();
+    fiatPaymentsService.handleWebhook({ paymentId: payment.id, eventType: 'settled' });
+    const count = fiatPaymentsService.getPaymentEvents(payment.id).length;
+    fiatPaymentsService.handleWebhook({ paymentId: payment.id, eventType: 'settled' });
+    expect(fiatPaymentsService.getPaymentEvents(payment.id)).toHaveLength(count);
+    expect(() => fiatPaymentsService.handleWebhook({ paymentId: payment.id, eventType: 'processing' }))
+      .toThrow('INVALID_PAYMENT_STATUS_TRANSITION');
+    expect(fiatPaymentsService.getPayment(payment.id)?.status).toBe('settled');
+  });
+
+  it('allows a bank return after settlement', () => {
+    const payment = makePayment();
+    fiatPaymentsService.handleWebhook({ paymentId: payment.id, eventType: 'settled' });
+    const returned = fiatPaymentsService.handleWebhook({
+      paymentId: payment.id, eventType: 'returned', reason: 'Bank return',
+    });
+    expect(returned?.status).toBe('returned');
+    expect(returned?.returnReason).toBe('Bank return');
+  });
+});

backend/src/services/fiat-payments.ts

@@ -352,6 +352,19 @@ class FiatPaymentsService {
       return undefined;
     }
 
+    // Bank webhook deliveries can be duplicated or arrive out of order.
+    if (payment.status === input.eventType) return payment;
+
+    // Do not allow provider notifications to bypass approval/compliance holds
+    // or regress terminal payment states. A bank return after settlement is valid.
+    const allowedTransitions: Partial<Record<FiatPaymentStatus, Array<typeof input.eventType>>> = {
+      processing: ['settled', 'failed', 'returned'],
+      settled: ['returned'],
+    };
+    if (!allowedTransitions[payment.status]?.includes(input.eventType)) {
+      throw new Error('INVALID_PAYMENT_STATUS_TRANSITION');
+    }
+
     payment.status = input.eventType;
     payment.updatedAt = this.nowIso();