From 8ab10db0d6a887f44f9d00fdfa64c0d92ce3a438 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 28 Aug 2026 11:08:47 +0100 Subject: [PATCH 1/3] fix: repair payments.service.ts compile break and outbox durability The file did not compile on main: a missing closing brace on the deliverWebhookWithRetry for-loop, duplicate field declarations, and a duplicate constructor parameter all prevented the payment test suite from running at all. Also mark the durable webhook outbox as delivered on immediate successful delivery so its terminal state stays inspectable (#666). --- BackendAcademy/src/payments/payments.service.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/BackendAcademy/src/payments/payments.service.ts b/BackendAcademy/src/payments/payments.service.ts index ad9d66db4..e0cc0840e 100644 --- a/BackendAcademy/src/payments/payments.service.ts +++ b/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. From c6d0cfbdf42813b6c8667a142716b9d123ddacb4 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 28 Aug 2026 11:08:54 +0100 Subject: [PATCH 2/3] feat: centralize payment state machine and cover every status (BA-093) Expose the payment status transition rules as a single, centralized source of truth (ALLOWED_TRANSITIONS, isLegalPaymentTransition, isTerminalPaymentStatus, PAYMENT_STATUSES). Both the webhook ingress path and any internal status update route through updatePaymentStatus, which consults these rules, so illegal regressions and terminal-state changes are rejected everywhere. Add exhaustive transition tests covering every status plus duplicate-event safety. --- .../src/database/database.service.ts | 43 ++++++- .../payments/payment-state-machine.spec.ts | 106 ++++++++++++++++++ 2 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 BackendAcademy/src/payments/payment-state-machine.spec.ts diff --git a/BackendAcademy/src/database/database.service.ts b/BackendAcademy/src/database/database.service.ts index 6cf5a0ca9..797103661 100644 --- a/BackendAcademy/src/database/database.service.ts +++ b/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,8 +166,15 @@ 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 = { + static readonly ALLOWED_TRANSITIONS: Record = { pending: ['processing', 'succeeded', 'failed'], processing: ['succeeded', 'failed'], succeeded: ['refunded'], @@ -161,6 +182,26 @@ export class DatabaseService implements OnModuleInit, OnApplicationShutdown { 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(); diff --git a/BackendAcademy/src/payments/payment-state-machine.spec.ts b/BackendAcademy/src/payments/payment-state-machine.spec.ts new file mode 100644 index 000000000..b98d1baf4 --- /dev/null +++ b/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); + }); + }); +}); From 0462413989c161e717a7bd2e488e222c4de2734e Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 28 Aug 2026 11:18:19 +0100 Subject: [PATCH 3/3] test: make prompt-template reload spec active under #653 approval gating The 'keeps the last valid templates' spec loaded a template with no approval, but getActiveTemplate (added in #653) requires approval.status === 'approved', so the template was never active and the test failed on main. Adding an approval makes it active under current behaviour and turns the CI (src/ai) green. --- BackendAcademy/src/ai/prompt-template.service.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BackendAcademy/src/ai/prompt-template.service.spec.ts b/BackendAcademy/src/ai/prompt-template.service.spec.ts index c8b07cb22..dd85962e0 100644 --- a/BackendAcademy/src/ai/prompt-template.service.spec.ts +++ b/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);