diff --git a/server/store/postgres_integration_test.go b/server/store/postgres_integration_test.go index bc015da9..d5c13c42 100644 --- a/server/store/postgres_integration_test.go +++ b/server/store/postgres_integration_test.go @@ -495,6 +495,81 @@ func TestPostgreSQLProposalClaimAndResponseAreAtomic(t *testing.T) { } } +// TestPostgreSQLProposalDeclineRequeuesEveryParticipant is a real, severe +// bug this session found by reading the code, not by a failing test: no +// path anywhere transitioned a PROPOSED ticket back to QUEUED after a +// decline. A stranded ticket is invisible to the matcher (which only reads +// state='QUEUED'), still counts as the player's one active ticket (blocking +// a fresh queue_create), and is renewable forever by an ordinary heartbeat +// -- a player proposed a match with someone who declines had no way back +// into matchmaking without realising they had to manually cancel first. +func TestPostgreSQLProposalDeclineRequeuesEveryParticipant(t *testing.T) { + db := openIntegrationPostgres(t) + applyIntegrationMigrations(t, db) + + now := time.Now().UTC().Truncate(time.Microsecond) + ctx := context.Background() + for _, player := range []string{"decline-player-a", "decline-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{"decline-player-a", "decline-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("decline-ticket-%d", i), player, now, now.Add(time.Minute)); err != nil { + t.Fatal(err) + } + } + proposal, err := domain.NewProposal("decline-proposal", domain.Casual, []string{"decline-player-a", "decline-player-b"}, now) + if err != nil { + t.Fatal(err) + } + if err := CreateProposal(ctx, db, proposal, map[string]string{"decline-player-a": "decline-ticket-0", "decline-player-b": "decline-ticket-1"}, now); err != nil { + t.Fatalf("create proposal: %v", err) + } + + // player-a declines; player-b never responded at all -- the bug affects + // even a participant who was never asked to do anything wrong. + declined, err := RespondToProposal(ctx, db, "decline-player-a", proposal.ProposalID, "decline-response-a-0001", false, 0, now) + if err != nil { + t.Fatalf("decline: %v", err) + } + if declined.State != domain.Declined { + t.Fatalf("proposal did not close on decline: %+v", declined) + } + + var stateA, stateB string + var expiresB time.Time + if err := db.QueryRow(`SELECT state FROM queue_tickets WHERE ticket_id = 'decline-ticket-0'`).Scan(&stateA); err != nil { + t.Fatal(err) + } + 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 (no cooldown mechanism exists yet to justify leaving it stuck)", 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) + } + if !expiresB.After(now) { + 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. + candidates, err := ListQueuedCandidates(ctx, db, domain.Casual, now, 10) + if err != nil { + t.Fatalf("list queued candidates: %v", err) + } + found := map[string]bool{} + 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) + } +} + func TestPostgreSQLProposalCreationRollsBackPartialClaims(t *testing.T) { db := openIntegrationPostgres(t) applyIntegrationMigrations(t, db) diff --git a/server/store/proposal_recovery_sql.go b/server/store/proposal_recovery_sql.go index 22903caf..d7510e5e 100644 --- a/server/store/proposal_recovery_sql.go +++ b/server/store/proposal_recovery_sql.go @@ -69,6 +69,21 @@ const ProposalDeclineSQL = `UPDATE proposals SET state = 'DECLINED', revision = revision + 1 WHERE proposal_id = $1 AND state = 'OPEN'` +// ProposalDeclineRequeueSQL requeues every participant's ticket, including +// the decliner's own: nothing yet enforces the decline cooldown §8.17 +// documents as a separate, not-yet-built feature, so leaving any ticket +// behind at PROPOSED here isn't "cooldown behaviour", it's just a stranded +// ticket -- invisible to the matcher (which only ever reads state='QUEUED'), +// still counted as this player's one active ticket (blocking a fresh +// queue_create), and renewable forever by an ordinary heartbeat, so a player +// left in this state has no path back into matchmaking without realising +// they need to cancel and start over. Once §8.17's cooldown exists, it can +// exempt the decliner from this immediate requeue; today nothing does. +const ProposalDeclineRequeueSQL = `UPDATE queue_tickets q +SET state = 'QUEUED', expires_at = $2, revision = revision + 1 +FROM proposal_participants pp +WHERE pp.proposal_id = $1 AND q.ticket_id = pp.ticket_id AND q.player_id = pp.player_id AND q.state = 'PROPOSED'` + const ProposalRevisionBumpSQL = `UPDATE proposals SET revision = revision + 1 WHERE proposal_id = $1 AND state = 'OPEN'` @@ -217,6 +232,10 @@ func RespondToProposal(ctx context.Context, db *sql.DB, playerID, proposalID, id _, err = tx.ExecContext(ctx, ProposalAcceptSQL, proposalID) } else { _, err = tx.ExecContext(ctx, ProposalDeclineSQL, proposalID) + if err != nil { + return err + } + _, err = tx.ExecContext(ctx, ProposalDeclineRequeueSQL, proposalID, now.Add(domain.QueueExpiryWindow)) } if err != nil { return err diff --git a/server/store/proposal_recovery_sql_test.go b/server/store/proposal_recovery_sql_test.go index 13e2281a..d2566d32 100644 --- a/server/store/proposal_recovery_sql_test.go +++ b/server/store/proposal_recovery_sql_test.go @@ -16,6 +16,7 @@ func TestProposalRecoverySQLBindsParticipantAndExpiresAtReadBoundary(t *testing. ProposalParticipantLockSQL: {"proposal_id = $1", "player_id = $2", "FOR UPDATE"}, ProposalParticipantRespondSQL: {"response = 'PENDING'", "responded_at"}, ProposalRevisionBumpSQL: {"revision = revision + 1", "state = 'OPEN'"}, + ProposalDeclineRequeueSQL: {"SET state = 'QUEUED'", "state = 'PROPOSED'", "proposal_participants"}, } { for _, fragment := range fragments { if !contains(query, fragment) {