From 14da286e11435b6eb4eb70e460e29b19cd20a33f Mon Sep 17 00:00:00 2001
From: Josh Creek <8179928+jcreek@users.noreply.github.com>
Date: Sat, 5 Sep 2026 19:30:41 +0100
Subject: [PATCH] fix(store): return 409 for idempotency key reuse, not 422

Both idempotency paths returned a bare fmt.Errorf, and writeDomainError
maps anything it does not recognise to its 422 "invalid_request"
default. So reusing a key with a different payload answered 422 where
openapi.json declares 409 and state-transitions.json requires
"reject_conflict_without_state_change".

That is the difference between "your request was malformed" and "that
key is taken". A client acting on 422 would rewrite a request that was
never wrong, and the 409 branch of every generated client was
unreachable.

Wrap domain.ErrConflict on both the create and mutate paths, and add an
integration test covering identical replay and conflicting reuse.

Pre-existing: both bare errors are unchanged from 089c127c, which is why
verify-allocated-compose failed in CI before this branch's work as well.
Found only after adding the diagnostics in 432e5a11 and fc2f5c86 --
until then the assertion aborted silently and three CI runs reported
nothing but "make: *** Error 1".
---
 server/store/postgres_integration_test.go | 40 +++++++++++++++++++++++
 server/store/queue_sql.go                 |  9 +++--
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/server/store/postgres_integration_test.go b/server/store/postgres_integration_test.go
index f45dc4f2..cfd6dd85 100644
--- a/server/store/postgres_integration_test.go
+++ b/server/store/postgres_integration_test.go
@@ -2562,3 +2562,43 @@ func TestPostgreSQLSessionRevocationIsImmediateOnAnotherReplica(t *testing.T) {
 		t.Fatalf("revoking one session invalidated another: %v", err)
 	}
 }
+
+// Reusing an idempotency key with a different payload must be a conflict.
+// Both idempotency paths returned a bare error, which writeDomainError maps
+// to its 422 default, so the API answered 422 where openapi.json and
+// state-transitions.json ("same_key_different_payload":
+// "reject_conflict_without_state_change") both require 409. It is the
+// difference between "your request was malformed" and "that key is taken",
+// and a client acting on the former would rewrite a correct request.
+func TestPostgreSQLIdempotencyKeyReuseIsAConflict(t *testing.T) {
+	db := openIntegrationPostgres(t)
+	applyIntegrationMigrations(t, db)
+
+	ctx := context.Background()
+	now := time.Now().UTC().Truncate(time.Microsecond)
+	if _, err := db.ExecContext(ctx, `INSERT INTO identities (player_id, steam_id) VALUES ('idem-player', 'idem-steam')`); err != nil {
+		t.Fatal(err)
+	}
+	spec := domain.QueueSpec{Playlist: domain.Casual, ClientBuild: "build-1", ProtocolVersion: 1}
+	if _, err := CreateQueueTicket(ctx, db, "idem-ticket-one", "idem-player", "idem-key-00000001", spec, now); err != nil {
+		t.Fatalf("first create: %v", err)
+	}
+
+	// Same key, same payload: replays the original result.
+	replay, err := CreateQueueTicket(ctx, db, "idem-ticket-one", "idem-player", "idem-key-00000001", spec, now.Add(time.Second))
+	if err != nil {
+		t.Fatalf("identical replay must succeed: %v", err)
+	}
+	if replay.TicketID != "idem-ticket-one" {
+		t.Fatalf("replay returned %q", replay.TicketID)
+	}
+
+	// Same key, different payload: conflict, not a validation error.
+	_, err = CreateQueueTicket(ctx, db, "idem-ticket-two", "idem-player", "idem-key-00000001", spec, now.Add(time.Second))
+	if err == nil {
+		t.Fatal("reusing a key with a different ticket was accepted")
+	}
+	if !errors.Is(err, domain.ErrConflict) {
+		t.Fatalf("err = %v; must wrap domain.ErrConflict so the API answers 409, not its 422 default", err)
+	}
+}
diff --git a/server/store/queue_sql.go b/server/store/queue_sql.go
index 2e97937d..57293e91 100644
--- a/server/store/queue_sql.go
+++ b/server/store/queue_sql.go
@@ -178,7 +178,12 @@ func CreateQueueTicket(ctx context.Context, db *sql.DB, ticketID, playerID, idem
 				return err
 			}
 			if !bytes.Equal(priorDigest, digest[:]) {
-				return fmt.Errorf("queue create idempotency conflict")
+				// Must wrap ErrConflict: writeDomainError maps unrecognised
+				// errors to 422, but the contract
+				// (state-transitions.json "same_key_different_payload") and
+				// openapi.json both require 409 for reusing a key with a
+				// different payload.
+				return fmt.Errorf("%w: queue create idempotency conflict", domain.ErrConflict)
 			}
 			var prior queueTicketRecord
 			if err := json.Unmarshal(priorResult, &prior); err != nil {
@@ -321,7 +326,7 @@ func mutateQueueTicket(ctx context.Context, db *sql.DB, playerID, ticketID, idem
 				return err
 			}
 			if !bytes.Equal(priorDigest, digest[:]) {
-				return fmt.Errorf("queue mutation idempotency conflict")
+				return fmt.Errorf("%w: queue mutation idempotency conflict", domain.ErrConflict)
 			}
 			var prior queueTicketRecord
 			if err := json.Unmarshal(priorResult, &prior); err != nil {