fix(multiplayer): terminate proposal offenders atomically

This commit is contained in:
Josh Creek
2026-09-03 00:09:07 +01:00
parent aa446cfbfe
commit f8af212e3f
6 changed files with 206 additions and 67 deletions
+110 -29
View File
@@ -612,12 +612,10 @@ func TestPostgreSQLProposalClaimAndResponseAreAtomic(t *testing.T) {
}
}
// TestPostgreSQLProposalDeclineRequeuesEveryParticipant protects the durable
// decline boundary: every ticket returns to QUEUED, while the decliner's
// separate penalty prevents an immediate replacement queue ticket. Without
// the requeue, tickets are invisible to the matcher and remain trapped in
// PROPOSED despite the proposal having closed.
func TestPostgreSQLProposalDeclineRequeuesEveryParticipant(t *testing.T) {
// TestPostgreSQLProposalDeclineCancelsOffenderAndRequeuesInnocent protects the
// durable decline boundary: the offender's ticket becomes terminal while
// every innocent ticket keeps its original queue precedence.
func TestPostgreSQLProposalDeclineCancelsOffenderAndRequeuesInnocent(t *testing.T) {
db := openIntegrationPostgres(t)
applyIntegrationMigrations(t, db)
@@ -659,8 +657,8 @@ func TestPostgreSQLProposalDeclineRequeuesEveryParticipant(t *testing.T) {
if err := db.QueryRow(`SELECT state, expires_at FROM queue_tickets WHERE ticket_id = 'decline-ticket-1'`).Scan(&stateB, &expiresB); err != nil {
t.Fatal(err)
}
if stateA != "QUEUED" {
t.Fatalf("decliner's own ticket state = %s, want QUEUED while cooldown is recorded separately", stateA)
if stateA != "CANCELLED" {
t.Fatalf("decliner's own ticket state = %s, want CANCELLED", stateA)
}
if stateB != "QUEUED" {
t.Fatalf("uninvolved participant's ticket state = %s, want QUEUED -- they must not be stranded by someone else's decline", stateB)
@@ -669,8 +667,8 @@ func TestPostgreSQLProposalDeclineRequeuesEveryParticipant(t *testing.T) {
t.Fatalf("requeued ticket expiry %v was not refreshed forward from %v", expiresB, now)
}
// The real, end-to-end regression: both players can be proposed a NEW
// match instead of ListQueuedCandidates silently never seeing them again.
// Only the innocent player can be selected again. A durable cooldown also
// rejects a new ticket from the decliner until the policy window ends.
candidates, err := ListQueuedCandidates(ctx, db, domain.Casual, now, 10)
if err != nil {
t.Fatalf("list queued candidates: %v", err)
@@ -679,17 +677,34 @@ func TestPostgreSQLProposalDeclineRequeuesEveryParticipant(t *testing.T) {
for _, candidate := range candidates {
found[candidate.PlayerID] = true
}
if !found["decline-player-a"] || !found["decline-player-b"] {
t.Fatalf("requeued players are not visible to the matcher: %+v", candidates)
if found["decline-player-a"] || !found["decline-player-b"] {
t.Fatalf("matcher did not isolate offender from innocent: %+v", candidates)
}
var cooldownEnd time.Time
if err := db.QueryRow(`SELECT ends_at FROM penalties WHERE player_id = 'decline-player-a' AND kind = 'PROPOSAL_DECLINED'`).Scan(&cooldownEnd); err != nil {
t.Fatal(err)
}
if want := now.Add(30 * time.Second); !cooldownEnd.Equal(want) {
t.Fatalf("decline cooldown end = %v, want %v", cooldownEnd, want)
}
// Recovering the closed proposal after its old deadline must not convert
// the innocent participant's PENDING response into a timeout penalty.
if _, err := GetProposal(ctx, db, "decline-player-b", proposal.ProposalID, now.Add(domain.ProposalWindow+time.Second)); err != nil {
t.Fatalf("recover declined proposal: %v", err)
}
var innocentTimeouts int
if err := db.QueryRow(`SELECT count(*) FROM penalties WHERE player_id = 'decline-player-b' AND kind = 'PROPOSAL_TIMEOUT'`).Scan(&innocentTimeouts); err != nil {
t.Fatal(err)
}
if innocentTimeouts != 0 {
t.Fatalf("innocent participant received %d timeout penalties after decline", innocentTimeouts)
}
}
// TestPostgreSQLProposalTimeoutRequeuesEveryParticipant protects the timeout
// sibling of the decline path: expiry must requeue every ticket and record a
// timeout cooldown for each participant who failed to respond. It uses
// GetProposal, the recovery/read path, to exercise a client returning after
// it missed the expiry event.
func TestPostgreSQLProposalTimeoutRequeuesEveryParticipant(t *testing.T) {
// TestPostgreSQLProposalTimeoutExpiresOffenderAndRequeuesAccepted protects the
// timeout sibling: accepted participants retain precedence, while no-shows
// receive a terminal ticket and cooldown.
func TestPostgreSQLProposalTimeoutExpiresOffenderAndRequeuesAccepted(t *testing.T) {
db := openIntegrationPostgres(t)
applyIntegrationMigrations(t, db)
@@ -712,9 +727,11 @@ func TestPostgreSQLProposalTimeoutRequeuesEveryParticipant(t *testing.T) {
if err := CreateProposal(ctx, db, proposal, map[string]string{"timeout-player-a": "timeout-ticket-0", "timeout-player-b": "timeout-ticket-1"}, now); err != nil {
t.Fatalf("create proposal: %v", err)
}
if _, err := RespondToProposal(ctx, db, "timeout-player-a", proposal.ProposalID, "timeout-accept-a-0001", true, 0, now.Add(time.Second)); err != nil {
t.Fatalf("accept proposal: %v", err)
}
// Nobody ever responds; recover the proposal well after its 10s window,
// exactly as a client reconnecting after missing the expiry event would.
// player-b never responds; recover well after the response window.
afterExpiry := now.Add(domain.ProposalWindow + time.Second)
recovered, err := GetProposal(ctx, db, "timeout-player-a", proposal.ProposalID, afterExpiry)
if err != nil {
@@ -725,18 +742,18 @@ func TestPostgreSQLProposalTimeoutRequeuesEveryParticipant(t *testing.T) {
}
var stateA, stateB string
var expiresB time.Time
if err := db.QueryRow(`SELECT state FROM queue_tickets WHERE ticket_id = 'timeout-ticket-0'`).Scan(&stateA); err != nil {
var expiresA time.Time
if err := db.QueryRow(`SELECT state, expires_at FROM queue_tickets WHERE ticket_id = 'timeout-ticket-0'`).Scan(&stateA, &expiresA); err != nil {
t.Fatal(err)
}
if err := db.QueryRow(`SELECT state, expires_at FROM queue_tickets WHERE ticket_id = 'timeout-ticket-1'`).Scan(&stateB, &expiresB); err != nil {
if err := db.QueryRow(`SELECT state FROM queue_tickets WHERE ticket_id = 'timeout-ticket-1'`).Scan(&stateB); err != nil {
t.Fatal(err)
}
if stateA != "QUEUED" || stateB != "QUEUED" {
t.Fatalf("timed-out participants left stranded: a=%s b=%s", stateA, stateB)
if stateA != "QUEUED" || stateB != "EXPIRED" {
t.Fatalf("timeout did not split accepted and offender tickets: a=%s b=%s", stateA, stateB)
}
if !expiresB.After(afterExpiry) {
t.Fatalf("requeued ticket expiry %v was not refreshed forward from %v", expiresB, afterExpiry)
if !expiresA.After(afterExpiry) {
t.Fatalf("requeued ticket expiry %v was not refreshed forward from %v", expiresA, afterExpiry)
}
candidates, err := ListQueuedCandidates(ctx, db, domain.Casual, afterExpiry, 10)
if err != nil {
@@ -746,8 +763,72 @@ func TestPostgreSQLProposalTimeoutRequeuesEveryParticipant(t *testing.T) {
for _, candidate := range candidates {
found[candidate.PlayerID] = true
}
if !found["timeout-player-a"] || !found["timeout-player-b"] {
t.Fatalf("requeued players are not visible to the matcher: %+v", candidates)
if !found["timeout-player-a"] || found["timeout-player-b"] {
t.Fatalf("matcher did not isolate timeout offender: %+v", candidates)
}
var cooldownEnd time.Time
if err := db.QueryRow(`SELECT ends_at FROM penalties WHERE player_id = 'timeout-player-b' AND kind = 'PROPOSAL_TIMEOUT'`).Scan(&cooldownEnd); err != nil {
t.Fatal(err)
}
if want := afterExpiry.Add(60 * time.Second); !cooldownEnd.Equal(want) {
t.Fatalf("timeout cooldown end = %v, want %v", cooldownEnd, want)
}
}
// A late response must report a closed proposal only after committing the
// expiry recovery. Returning that domain error from inside RunSerializable
// used to roll every recovery write back.
func TestPostgreSQLLateProposalResponseCommitsExpiryRecovery(t *testing.T) {
db := openIntegrationPostgres(t)
applyIntegrationMigrations(t, db)
now := time.Now().UTC().Truncate(time.Microsecond)
ctx := context.Background()
for _, player := range []string{"late-player-a", "late-player-b"} {
if _, err := db.ExecContext(ctx, `INSERT INTO identities (player_id, steam_id) VALUES ($1, $1)`, player); err != nil {
t.Fatal(err)
}
}
for i, player := range []string{"late-player-a", "late-player-b"} {
if _, err := db.ExecContext(ctx, `INSERT INTO queue_tickets (ticket_id, player_id, playlist, state, client_build, protocol_version, enqueued_at, expires_at) VALUES ($1, $2, 'casual', 'QUEUED', 'integration-build', 1, $3, $4)`, fmt.Sprintf("late-ticket-%d", i), player, now, now.Add(time.Minute)); err != nil {
t.Fatal(err)
}
}
proposal, err := domain.NewProposal("late-proposal", domain.Casual, []string{"late-player-a", "late-player-b"}, now)
if err != nil {
t.Fatal(err)
}
if err := CreateProposal(ctx, db, proposal, map[string]string{"late-player-a": "late-ticket-0", "late-player-b": "late-ticket-1"}, now); err != nil {
t.Fatalf("create proposal: %v", err)
}
late := now.Add(domain.ProposalWindow + time.Second)
_, err = RespondToProposal(ctx, db, "late-player-a", proposal.ProposalID, "late-response-a-0001", true, 0, late)
if !errors.Is(err, domain.ErrProposalClosed) {
t.Fatalf("late response error = %v, want ErrProposalClosed", err)
}
var proposalState, ticketA, ticketB string
if err := db.QueryRow(`SELECT state FROM proposals WHERE proposal_id = 'late-proposal'`).Scan(&proposalState); err != nil {
t.Fatal(err)
}
if err := db.QueryRow(`SELECT state FROM queue_tickets WHERE ticket_id = 'late-ticket-0'`).Scan(&ticketA); err != nil {
t.Fatal(err)
}
if err := db.QueryRow(`SELECT state FROM queue_tickets WHERE ticket_id = 'late-ticket-1'`).Scan(&ticketB); err != nil {
t.Fatal(err)
}
if proposalState != "EXPIRED" || ticketA != "EXPIRED" || ticketB != "EXPIRED" {
t.Fatalf("late recovery was not committed: proposal=%s tickets=%s,%s", proposalState, ticketA, ticketB)
}
var penalties, idempotencyRows int
if err := db.QueryRow(`SELECT count(*) FROM penalties WHERE kind = 'PROPOSAL_TIMEOUT' AND player_id IN ('late-player-a', 'late-player-b')`).Scan(&penalties); err != nil {
t.Fatal(err)
}
if err := db.QueryRow(`SELECT count(*) FROM idempotency_keys WHERE scope = $1 AND idempotency_key = 'late-response-a-0001'`, ProposalResponseIdempotencyScope).Scan(&idempotencyRows); err != nil {
t.Fatal(err)
}
if penalties != 2 || idempotencyRows != 0 {
t.Fatalf("late recovery side effects: penalties=%d idempotency_rows=%d", penalties, idempotencyRows)
}
}