feat(marketplace): seller transfer state machine matrix — v1.0.7 item B day 1
Some checks failed
Veza CI / Backend (Go) (push) Failing after 0s
Veza CI / Frontend (Web) (push) Failing after 0s
Veza CI / Rust (Stream Server) (push) Failing after 0s
Security Scan / Secret Scanning (gitleaks) (push) Failing after 0s
Veza CI / Notify on failure (push) Failing after 0s
Some checks failed
Veza CI / Backend (Go) (push) Failing after 0s
Veza CI / Frontend (Web) (push) Failing after 0s
Veza CI / Rust (Stream Server) (push) Failing after 0s
Security Scan / Secret Scanning (gitleaks) (push) Failing after 0s
Veza CI / Notify on failure (push) Failing after 0s
Day-1 foundation for item B (async Stripe Connect reversal worker).
No worker code, no runtime enforcement yet — just the authoritative
state machine that day 2's code will route through. Before writing
the worker we want a single place where the legal transitions are
defined and tested, so the worker's behavior can be argued against
the matrix rather than implicitly codified across call sites.
transfer_transitions.go:
* SellerTransferStatus constants (Pending, Completed, Failed,
ReversalPending [new], Reversed [new], PermanentlyFailed).
* AllowedTransferTransitions map: pending → {completed, failed};
completed → {reversal_pending}; failed → {completed,
permanently_failed}; reversal_pending → {reversed,
permanently_failed}; reversed and permanently_failed as dead ends.
* CanTransitionTransferStatus(from, to) — same-state always OK
(idempotent bumps of retry_count / next_retry_at); unknown from
fails conservatively (typos in call sites become visible).
transfer_transitions_test.go:
* TestTransferStateTransitions iterates the full 6×6 matrix (36
pairs) and asserts every pair against the expected outcome.
* TestTransferStateTransitions_TerminalStatesHaveNoOutgoing
double-locks Reversed + PermanentlyFailed as dead ends at the
map level (not just at the caller level).
* TestTransferStateTransitions_MatrixKeysAreAccountedFor keeps the
canonical status list in sync with the map; a new status added
to one but not the other fails the test.
* TestCanTransitionTransferStatus_UnknownFromIsConservative
documents the "unknown from → always false" policy so a future
reader sees the intent.
Migration 982 adds a partial composite index on (status,
next_retry_at) WHERE status='reversal_pending', sibling to the
existing idx_seller_transfers_retry (scoped to failed). Two parallel
partial indexes cost less than widening the existing one (which
would need a table-level lock) and keep the worker query planner-
friendly.
Day 2 routes processSellerTransfers, TransferRetryWorker,
reverseSellerAccounting, admin_transfer_handler through
CanTransitionTransferStatus at every Status mutation, and writes
StripeReversalWorker. Day 3 exercises the end-to-end flow
(refund → reversal_pending → worker → reversed) in a smoke probe.
Checkpoint: ping user at end of day 1 before day 2 per discipline
agreed upfront.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
40bc9100ba
commit
d4fbf2bf84
4 changed files with 267 additions and 0 deletions
|
|
@ -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
|
||||
}
|
||||
|
|
@ -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
|
||||
}
|
||||
|
|
@ -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';
|
||||
|
|
@ -0,0 +1 @@
|
|||
DROP INDEX IF EXISTS idx_seller_transfers_reversal_pending;
|
||||
Loading…
Reference in a new issue