Fix/payment state machine enforcement

BackendAcademy/src/ai/prompt-template.service.spec.ts

@@ -22,7 +22,7 @@ describe('PromptTemplateService reloads', () => {
   it('keeps the last valid templates when a later reload is malformed', () => {
     writeFileSync(configPath, JSON.stringify({
       schemaVersion: '1.0.0',
-      templates: { chat_tutor: [{ version: '2.0.0', description: 'Test', systemPrompt: 'Use the reloaded prompt.' }] },
+      templates: { chat_tutor: [{ version: '2.0.0', description: 'Test', systemPrompt: 'Use the reloaded prompt.', approval: { status: 'approved' } }] },
     }));
 
     expect(service.reloadTemplates()).toBe(true);

BackendAcademy/src/database/database.service.ts

@@ -43,6 +43,20 @@ export interface RedemptionRecord {
  */
 export type PaymentStatus = 'pending' | 'processing' | 'succeeded' | 'failed' | 'refunded';
 
+/**
+ * Every payment status, in declaration order. Exported so transition tests
+ * and callers can enumerate the full state space without re-listing it.
+ *
+ * #661 (BA-093): used by the exhaustive state-transition test suite.
+ */
+export const PAYMENT_STATUSES: PaymentStatus[] = [
+  'pending',
+  'processing',
+  'succeeded',
+  'failed',
+  'refunded',
+];
+
 export interface PaymentRecord {
   id: string;
   orderId: string;
@@ -152,15 +166,42 @@ export class DatabaseService implements OnModuleInit, OnApplicationShutdown {
   /**
    * Explicit state machine for payment status transitions. A status that
    * does not appear as a key has no legal outgoing transitions (terminal).
+   *
+   * #661 (BA-093): this is the single, centralized source of truth for legal
+   * transitions. Both the webhook ingress path
+   * ({@link PaymentsService.processPaymentWebhookEvent}) and any internal
+   * status update route through {@link updatePaymentStatus}, which consults
+   * these rules, so illegal regressions and terminal-state changes are
+   * rejected everywhere.
    */
-  private static readonly ALLOWED_TRANSITIONS: Record<PaymentStatus, PaymentStatus[]> = {
+  static readonly ALLOWED_TRANSITIONS: Record<PaymentStatus, PaymentStatus[]> = {
     pending: ['processing', 'succeeded', 'failed'],
     processing: ['succeeded', 'failed'],
     succeeded: ['refunded'],
     failed: [],
     refunded: [],
   };
 
+  /**
+   * Returns whether transitioning from `from` to `to` is legal under the
+   * centralized payment state machine (#661 / BA-093). A no-op
+   * (`from === to`) is considered legal because callers treat it as an
+   * idempotent no-op rather than a transition.
+   */
+  static isLegalPaymentTransition(from: PaymentStatus, to: PaymentStatus): boolean {
+    if (from === to) return true;
+    const allowed = DatabaseService.ALLOWED_TRANSITIONS[from] ?? [];
+    return allowed.includes(to);
+  }
+
+  /**
+   * Returns whether a status is terminal (has no legal outgoing transitions),
+   * so any further change from it must be rejected (#661 / BA-093).
+   */
+  static isTerminalPaymentStatus(status: PaymentStatus): boolean {
+    return (DatabaseService.ALLOWED_TRANSITIONS[status] ?? []).length === 0;
+  }
+
   onModuleInit() {
     this.seedSampleCoupons();
     this.ensureMigrationTracking();

BackendAcademy/src/payments/payment-state-machine.spec.ts

@@ -0,0 +1,106 @@
+import { DatabaseService, PAYMENT_STATUSES, PaymentStatus } from '../database/database.service';
+import { TransactionManagerService } from '../common/transaction-manager.service';
+
+/**
+ * Exhaustive payment state-machine coverage for BA-093 / #661.
+ *
+ * These tests assert that the centralized transition rules reject illegal
+ * status regressions and terminal-state changes for *every* status, and that
+ * duplicate provider events are treated as safe no-ops. Both the webhook
+ * ingress path and any internal status update route through
+ * `DatabaseService.updatePaymentStatus`, so exercising it here covers the
+ * internal-update requirement directly.
+ */
+describe('Payment state machine (BA-093 / #661)', () => {
+  describe('isLegalPaymentTransition', () => {
+    it('matches the declared allowed transitions for every status pair', () => {
+      for (const from of PAYMENT_STATUSES) {
+        for (const to of PAYMENT_STATUSES) {
+          const expected =
+            from === to || DatabaseService.ALLOWED_TRANSITIONS[from].includes(to);
+          expect(DatabaseService.isLegalPaymentTransition(from, to)).toBe(expected);
+        }
+      }
+    });
+
+    it('rejects regressions and terminal-state changes', () => {
+      // succeeded -> pending is an illegal regression.
+      expect(DatabaseService.isLegalPaymentTransition('succeeded', 'pending')).toBe(false);
+      // failed is terminal: any onward change is illegal.
+      expect(DatabaseService.isLegalPaymentTransition('failed', 'succeeded')).toBe(false);
+      // refunded is terminal: any onward change is illegal.
+      expect(DatabaseService.isLegalPaymentTransition('refunded', 'pending')).toBe(false);
+      expect(DatabaseService.isLegalPaymentTransition('refunded', 'succeeded')).toBe(false);
+    });
+  });
+
+  describe('isTerminalPaymentStatus', () => {
+    it('marks failed and refunded as terminal', () => {
+      expect(DatabaseService.isTerminalPaymentStatus('failed')).toBe(true);
+      expect(DatabaseService.isTerminalPaymentStatus('refunded')).toBe(true);
+    });
+
+    it('marks pending, processing and succeeded as non-terminal', () => {
+      expect(DatabaseService.isTerminalPaymentStatus('pending')).toBe(false);
+      expect(DatabaseService.isTerminalPaymentStatus('processing')).toBe(false);
+      expect(DatabaseService.isTerminalPaymentStatus('succeeded')).toBe(false);
+    });
+  });
+
+  describe('updatePaymentStatus enforcement (internal updates)', () => {
+    let db: DatabaseService;
+
+    beforeEach(() => {
+      db = new DatabaseService(new TransactionManagerService());
+    });
+
+    const seed = (id: string, status: PaymentStatus): void => {
+      db.createPayment({
+        id,
+        orderId: 'ord-1',
+        userId: 'user-1',
+        status,
+        amount: 100,
+        assetCode: 'XLM',
+        provider: 'stellar',
+      });
+    };
+
+    PAYMENT_STATUSES.forEach((from) => {
+      PAYMENT_STATUSES.forEach((to) => {
+        it(`rejects illegal transition from ${from} -> ${to} via updatePaymentStatus`, async () => {
+          const id = `pay-${from}-${to}`;
+          seed(id, from);
+
+          const result = await db.updatePaymentStatus(id, to, `evt-${from}-${to}`);
+
+          if (from === to) {
+            // Idempotent no-op: safe, but not a state change.
+            expect(result.success).toBe(true);
+            expect(result.transitioned).toBe(false);
+          } else if (DatabaseService.ALLOWED_TRANSITIONS[from].includes(to)) {
+            expect(result.success).toBe(true);
+            expect(result.transitioned).toBe(true);
+          } else {
+            // Illegal regression / terminal change must be refused.
+            expect(result.success).toBe(false);
+            expect(result.transitioned).toBe(false);
+            expect(result.reason).toMatch(/Illegal transition/i);
+          }
+        });
+      });
+    });
+
+    it('treats a repeated provider event id as a safe no-op', async () => {
+      seed('pay-dup', 'pending');
+      const first = await db.updatePaymentStatus('pay-dup', 'succeeded', 'evt-dup');
+      expect(first.success).toBe(true);
+      expect(first.transitioned).toBe(true);
+
+      const second = await db.updatePaymentStatus('pay-dup', 'succeeded', 'evt-dup');
+      expect(second.success).toBe(true);
+      expect(second.duplicateEvent).toBe(true);
+      expect(second.transitioned).toBe(false);
+    });
+  });
+});

BackendAcademy/src/payments/payments.service.ts

@@ -132,14 +132,10 @@ export class PaymentsService {
   /** Base backoff for webhook retries (Issue #412). */
   private readonly webhookBaseBackoffMs: number;
   /** Cap for webhook retry backoff (Issue #412). */
-  private readonly defaultTimeoutMs: number;
-  private readonly webhookMaxRetries: number;
-  private readonly webhookBaseBackoffMs: number;
   private readonly webhookMaxBackoffMs: number;
 
   constructor(
     private readonly databaseService: DatabaseService,
-    private readonly configService?: ConfigService,
     @Optional()
     private readonly contractAdapter?: IContractAdapter,
     @Optional()
@@ -313,12 +309,16 @@ export class PaymentsService {
           this.logger.log(
             `Webhook ${webhook.id} delivered on attempt ${attempt}`,
           );
+          // Mark the durable outbox as delivered so its terminal state stays
+          // inspectable (the enqueue-before-deliver contract from #666).
+          await this.databaseService.completeWebhookDelivery(webhook.id, statusCode);
           return { success: true, attempts: attempt };
         }
         lastError = `HTTP ${statusCode}`;
       } catch (err) {
         lastError = err instanceof Error ? err.message : String(err);
       }
+    }
 
     // Drive the delivery to a terminal state through the durable outbox,
     // honouring the exponential-backoff schedule stored on the record.