diff --git a/veza-backend-api/internal/core/marketplace/transfer_transitions.go b/veza-backend-api/internal/core/marketplace/transfer_transitions.go new file mode 100644 index 000000000..8f8b57b52 --- /dev/null +++ b/veza-backend-api/internal/core/marketplace/transfer_transitions.go @@ -0,0 +1,95 @@ +package marketplace + +// SellerTransfer.Status values. The field is a VARCHAR at the DB level +// (no ENUM), but these constants are the authoritative list. Any new +// status value must be added here, to AllowedTransferTransitions, and +// to TestTransferStateTransitions — all three are designed as a single +// choke-point so an extension is either complete or fails loudly. +// +// Introduced alongside v1.0.7 item B (async Stripe Connect reversal +// worker), which adds ReversalPending + Reversed. Earlier values +// (Pending, Completed, Failed, PermanentlyFailed) existed as string +// literals before and are codified here without behavior change. +const ( + TransferStatusPending = "pending" + TransferStatusCompleted = "completed" + TransferStatusFailed = "failed" + TransferStatusReversalPending = "reversal_pending" + TransferStatusReversed = "reversed" + TransferStatusPermanentlyFailed = "permanently_failed" +) + +// AllowedTransferTransitions is the state machine for +// SellerTransfer.Status. Read as: from the key, the value lists +// every legal destination. Transitions not listed are forbidden. +// +// Same-state "transitions" (e.g. reversal_pending → reversal_pending +// to bump updated_at / retry_count / next_retry_at on a transient +// retry) are always allowed and not listed here — see +// CanTransitionTransferStatus. +// +// Terminal states — Reversed and PermanentlyFailed — have empty +// value lists. An admin manual override can write any status via +// raw SQL with documented intent (audit log entry), but that path +// deliberately sits outside this matrix so a code review of any +// state-mutating call sees the override as a conscious escape hatch +// rather than a legitimate transition. +// +// The state machine covers three flows: +// +// 1. Forward settlement: +// pending → completed (Stripe transfer succeeded) +// pending → failed (Stripe transfer errored) +// failed → completed (retry worker recovered) +// failed → permanently_failed (retry worker exhausted) +// +// 2. Reversal on refund (item B): +// completed → reversal_pending (refund triggered reversal) +// reversal_pending → reversed (Stripe reversal succeeded) +// reversal_pending → permanently_failed (reversal exhausted retries) +// +// 3. No valid transitions from terminal states; Reversed and +// PermanentlyFailed are end-of-life for the row. +var AllowedTransferTransitions = map[string][]string{ + TransferStatusPending: { + TransferStatusCompleted, + TransferStatusFailed, + }, + TransferStatusCompleted: { + TransferStatusReversalPending, + }, + TransferStatusFailed: { + TransferStatusCompleted, + TransferStatusPermanentlyFailed, + }, + TransferStatusReversalPending: { + TransferStatusReversed, + TransferStatusPermanentlyFailed, + }, + TransferStatusReversed: {}, + TransferStatusPermanentlyFailed: {}, +} + +// CanTransitionTransferStatus reports whether the state machine allows +// a transition from `from` to `to`. Same-state is always allowed, so +// callers that want to bump an auxiliary column (updated_at, +// retry_count, next_retry_at) without changing Status never get +// rejected. Unknown `from` values are treated conservatively: no +// transition is allowed from a status the matrix doesn't know about — +// this surfaces typos and stale values rather than silently permitting +// them. +func CanTransitionTransferStatus(from, to string) bool { + if from == to { + return true + } + allowed, known := AllowedTransferTransitions[from] + if !known { + return false + } + for _, candidate := range allowed { + if candidate == to { + return true + } + } + return false +} diff --git a/veza-backend-api/internal/core/marketplace/transfer_transitions_test.go b/veza-backend-api/internal/core/marketplace/transfer_transitions_test.go new file mode 100644 index 000000000..348bfe155 --- /dev/null +++ b/veza-backend-api/internal/core/marketplace/transfer_transitions_test.go @@ -0,0 +1,135 @@ +package marketplace + +import ( + "fmt" + "sort" + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestTransferStateTransitions is the regression-grade check on the +// seller_transfers state machine. It iterates the full (from × to) +// product and asserts every pair against the expected outcome. If +// someone adds a new status constant without also adding to the +// AllowedTransferTransitions map or to this test's `expected` matrix, +// one of two things happens: the new status is treated as a dead end +// (no outgoing transitions) and this test silently passes, OR the new +// status isn't iterated at all. To close that gap, the test lists +// `allStatuses` explicitly and fails if AllowedTransferTransitions +// contains a key not in `allStatuses` (see TestTransferStateTransitions_ +// MatrixKeysAreAccountedFor below). +func TestTransferStateTransitions(t *testing.T) { + allStatuses := []string{ + TransferStatusPending, + TransferStatusCompleted, + TransferStatusFailed, + TransferStatusReversalPending, + TransferStatusReversed, + TransferStatusPermanentlyFailed, + } + + // expected[from][to] = true if transition is allowed. + // Any pair not in `expected` defaults to false. Same-state pairs + // are always true (handled below) and need not be listed. + expected := map[string]map[string]bool{ + TransferStatusPending: { + TransferStatusCompleted: true, + TransferStatusFailed: true, + }, + TransferStatusCompleted: { + TransferStatusReversalPending: true, + }, + TransferStatusFailed: { + TransferStatusCompleted: true, + TransferStatusPermanentlyFailed: true, + }, + TransferStatusReversalPending: { + TransferStatusReversed: true, + TransferStatusPermanentlyFailed: true, + }, + TransferStatusReversed: {}, + TransferStatusPermanentlyFailed: {}, + } + + for _, from := range allStatuses { + for _, to := range allStatuses { + t.Run(fmt.Sprintf("%s_to_%s", from, to), func(t *testing.T) { + got := CanTransitionTransferStatus(from, to) + var want bool + if from == to { + want = true // same-state is always allowed (retry bumps) + } else { + want = expected[from][to] + } + assert.Equal(t, want, got, "transition %s → %s: expected allowed=%v, got allowed=%v", from, to, want, got) + }) + } + } +} + +// TestTransferStateTransitions_TerminalStatesHaveNoOutgoing asserts +// that Reversed and PermanentlyFailed are dead ends in the matrix +// definition itself — not just that callers happen to not transition +// out of them. +func TestTransferStateTransitions_TerminalStatesHaveNoOutgoing(t *testing.T) { + terminals := []string{TransferStatusReversed, TransferStatusPermanentlyFailed} + for _, terminal := range terminals { + t.Run(terminal, func(t *testing.T) { + outgoing, ok := AllowedTransferTransitions[terminal] + assert.True(t, ok, "terminal status %q must be a key in AllowedTransferTransitions even if empty", terminal) + assert.Empty(t, outgoing, "terminal status %q must have no outgoing transitions", terminal) + }) + } +} + +// TestTransferStateTransitions_MatrixKeysAreAccountedFor ensures the +// AllowedTransferTransitions map and the test's canonical statuses +// list stay in sync. If someone adds TransferStatusXxx and an entry +// in the map but forgets to extend allStatuses above, this test +// fails — surfacing the omission rather than letting the new status +// sit unchecked. +func TestTransferStateTransitions_MatrixKeysAreAccountedFor(t *testing.T) { + allStatuses := []string{ + TransferStatusPending, + TransferStatusCompleted, + TransferStatusFailed, + TransferStatusReversalPending, + TransferStatusReversed, + TransferStatusPermanentlyFailed, + } + known := make(map[string]bool, len(allStatuses)) + for _, s := range allStatuses { + known[s] = true + } + + var orphans []string + for key := range AllowedTransferTransitions { + if !known[key] { + orphans = append(orphans, key) + } + } + sort.Strings(orphans) + assert.Empty(t, orphans, "AllowedTransferTransitions contains keys not in the canonical allStatuses list: %v", orphans) + + var missing []string + for _, s := range allStatuses { + if _, ok := AllowedTransferTransitions[s]; !ok { + missing = append(missing, s) + } + } + sort.Strings(missing) + assert.Empty(t, missing, "canonical allStatuses contains values missing from AllowedTransferTransitions: %v", missing) +} + +// TestCanTransitionTransferStatus_UnknownFromIsConservative asserts +// the policy documented in the function doc: an unknown `from` value +// never passes the guard. This makes typos in call sites loud (a +// mutation using "completed_" instead of "completed" will fail the +// guard and surface in tests / logs, rather than silently permitting +// an unintended transition). +func TestCanTransitionTransferStatus_UnknownFromIsConservative(t *testing.T) { + assert.False(t, CanTransitionTransferStatus("bogus_status", TransferStatusCompleted)) + assert.False(t, CanTransitionTransferStatus("", TransferStatusCompleted)) + assert.False(t, CanTransitionTransferStatus("Completed", TransferStatusReversalPending)) // case-sensitive +} diff --git a/veza-backend-api/migrations/982_seller_transfers_reversal_pending_index.sql b/veza-backend-api/migrations/982_seller_transfers_reversal_pending_index.sql new file mode 100644 index 000000000..9387b461b --- /dev/null +++ b/veza-backend-api/migrations/982_seller_transfers_reversal_pending_index.sql @@ -0,0 +1,36 @@ +-- v1.0.7 item B (day 1): partial composite index for the reversal +-- worker's hot path. +-- +-- The worker's select per tick is: +-- SELECT * FROM seller_transfers +-- WHERE status = 'reversal_pending' +-- AND (next_retry_at IS NULL OR next_retry_at <= NOW()) +-- ORDER BY next_retry_at NULLS FIRST +-- LIMIT 20; +-- +-- Migration 116 already provides idx_seller_transfers_retry on +-- `(status, next_retry_at) WHERE status='failed' AND next_retry_at +-- IS NOT NULL` for TransferRetryWorker. That index cannot serve the +-- reversal worker's query because (a) the partial WHERE clause +-- excludes reversal_pending rows, and (b) it excludes rows with +-- next_retry_at NULL — which are exactly the freshly-inserted +-- reversal_pending rows the worker should pick up on first pass. +-- +-- This migration adds a sibling partial index scoped to +-- reversal_pending, including NULL next_retry_at so the first-pass +-- rows land in the index. +-- +-- Why not widen migration 116's existing index instead: widening a +-- partial index requires dropping and recreating it, which would +-- require a table-level lock. Two parallel partial indexes is the +-- cheaper DDL. +-- +-- No state machine enforcement in this migration — the authoritative +-- transition matrix lives in +-- `internal/core/marketplace/transfer_transitions.go` and is exercised +-- by TestTransferStateTransitions. Day 2 routes call sites through +-- the matrix; day 3 exercises the end-to-end flow in a smoke probe. + +CREATE INDEX IF NOT EXISTS idx_seller_transfers_reversal_pending + ON seller_transfers(status, next_retry_at) + WHERE status = 'reversal_pending'; diff --git a/veza-backend-api/migrations/rollback/982_seller_transfers_reversal_pending_index_down.sql b/veza-backend-api/migrations/rollback/982_seller_transfers_reversal_pending_index_down.sql new file mode 100644 index 000000000..efe1e75d2 --- /dev/null +++ b/veza-backend-api/migrations/rollback/982_seller_transfers_reversal_pending_index_down.sql @@ -0,0 +1 @@ +DROP INDEX IF EXISTS idx_seller_transfers_reversal_pending;