Stop recycling game ids and lock the game row on delete

.claude/rules/backend/models.md

@@ -13,6 +13,8 @@ Two responsibilities: define fields, and define properties for convenient access
 
 **`Member` is a participant in a game, not necessarily a player.** `Member.kind` discriminates the roles; a game master holds a member row with no nation, no phase state, and no seat against the variant's nation count. Anything that means *player* — seat counting, nation assignment, phase states, draws, victory, abandonment — must go through `Member.objects.players()` / `game.members.players()`, and a new query over `game.members` has to decide which it means. Do not reintroduce a second source of truth for the role by re-deriving it from `Game.game_master`.
 
+**A human-readable primary key always carries a random suffix.** `Game.id` is a slug of the name plus a short uuid fragment, so no id is ever issued twice — including after the row it belonged to is deleted. Never hand out the bare slug when it happens to be free: a recycled id lets a request holding a stale one act on an unrelated row, and lets a delete strand the new row's children.
+
 **Do not leave unused domain in the schema for a future feature.** If fields are not used yet, remove them and re-add when the feature lands. Absent beats switched-off scaffolding.
 
 Query optimisation belongs on a custom QuerySet, never in a view:

.claude/rules/backend/views.md

@@ -39,6 +39,8 @@ Mutating a row that already exists is an `UpdateAPIView`, whatever the operation
 
 Resolving the object is the view's job, in `get_object()` — not the serializer's. Prefer DRF's default response over an overridden `create()` / `update()` / `destroy()`: an override to change the status code costs a view body and an `@extend_schema` annotation to keep the schema honest.
 
+A view that deletes a row re-fetches it under `select_for_update()` inside the transaction that deletes it — `resolve_game(request, game_id, lock=True)` for games — and re-runs `check_permissions` against the locked row. Django cascades deletes in Python: an unlocked delete collects children in one snapshot and removes them in another, so anything committed in between survives and the deferred foreign keys reject the whole transaction at COMMIT.
+
 Every view needs a docstring. drf-spectacular extracts it, and without one it picks up the mixin's — which is why unrelated endpoints in the committed schema are described as "Used by views that have a game parameter in the URL".
 
 **Review check:** using a DRF generic, not a raw `APIView`? permission classes declared rather than checked in the body? view is thin? mixins used for shared context? queryset uses a QuerySet method (`with_list_data()`, etc.)? generic matches the mutation, with no body override just to change the status code? docstring present?

service/game/models.py

@@ -5,7 +5,7 @@
 import uuid
 
 from django.conf import settings
-from django.db import IntegrityError, models, transaction
+from django.db import models, transaction
 from django.utils import timezone
 from django.db.models import (
     Count,
@@ -492,27 +492,14 @@ def save(self, *args, **kwargs):
             super().save(*args, **kwargs)
             return
 
-        base_id = self._generate_base_id()
-        self.id = self._generate_id(base_id)
+        self.id = self._suffixed_id(self._generate_base_id())
         kwargs.setdefault("force_insert", True)
-
-        try:
-            with transaction.atomic():
-                super().save(*args, **kwargs)
-        except IntegrityError:
-            self.id = self._suffixed_id(base_id)
-            super().save(*args, **kwargs)
+        super().save(*args, **kwargs)
 
     def _generate_base_id(self):
         base_id = re.sub(r"[^a-z0-9]+", "-", self.name.lower())
         return re.sub(r"^-+|-+$", "", base_id)
 
-    def _generate_id(self, base_id):
-        if Game.objects.filter(id=base_id).exists():
-            return self._suffixed_id(base_id)
-
-        return base_id
-
     def _suffixed_id(self, base_id):
         return f"{base_id}-{str(uuid.uuid4())[:8]}"
 

service/game/tests.py

@@ -1951,7 +1951,7 @@ def test_create_game_query_count_small_variant(self, authenticated_client, italy
 
         assert response.status_code == status.HTTP_201_CREATED
         query_count = len(connection.queries)
-        assert query_count == 49
+        assert query_count == 46
 
     @pytest.mark.django_db
     def test_create_game_query_count_large_variant(self, authenticated_client, classical_variant):
@@ -1970,7 +1970,7 @@ def test_create_game_query_count_large_variant(self, authenticated_client, class
 
         assert response.status_code == status.HTTP_201_CREATED
         query_count = len(connection.queries)
-        assert query_count == 49
+        assert query_count == 46
 
 
 class TestGamePrivateFiltering:
@@ -2360,6 +2360,28 @@ def test_create_sandbox_game_does_not_notify(
         assert Notification.objects.count() == 0
         assert NotificationDelivery.objects.count() == 0
 
+    @pytest.mark.django_db
+    def test_recreated_sandbox_game_does_not_reuse_deleted_id(
+        self, authenticated_client, classical_variant
+    ):
+        url = reverse(sandbox_create_viewname)
+        payload = {
+            "name": "My Sandbox Game",
+            "variant_id": classical_variant.id,
+        }
+
+        first = authenticated_client.post(url, payload, format="json")
+        assert first.status_code == status.HTTP_201_CREATED
+
+        delete_response = authenticated_client.delete(
+            reverse("game-delete", args=[first.data["id"]])
+        )
+        assert delete_response.status_code == status.HTTP_204_NO_CONTENT
+
+        second = authenticated_client.post(url, payload, format="json")
+        assert second.status_code == status.HTTP_201_CREATED
+        assert second.data["id"] != first.data["id"]
+
     @pytest.mark.django_db
     def test_create_sandbox_game_missing_name(self, authenticated_client, classical_variant):
         url = reverse(sandbox_create_viewname)
@@ -2423,7 +2445,7 @@ def test_create_sandbox_game_query_count_small_variant(
 
         assert response.status_code == status.HTTP_201_CREATED
         query_count = len(connection.queries)
-        assert query_count == 55
+        assert query_count == 52
 
     @pytest.mark.django_db
     def test_create_sandbox_game_query_count_large_variant(
@@ -2444,7 +2466,7 @@ def test_create_sandbox_game_query_count_large_variant(
 
         assert response.status_code == status.HTTP_201_CREATED
         query_count = len(connection.queries)
-        assert query_count == 55
+        assert query_count == 52
 
 
 class TestSandboxGameFiltering:
@@ -3005,27 +3027,18 @@ def test_clone_to_sandbox_twice_creates_two_games(
 class TestGameIdGeneration:
 
     @pytest.mark.django_db
-    def test_unique_name_keeps_slug_id(self, classical_variant):
+    def test_id_is_suffixed_slug(self, classical_variant):
         game = Game.objects.create(name="A Unique Name", variant=classical_variant)
-        assert game.id == "a-unique-name"
+
+        assert game.id != "a-unique-name"
+        assert game.id.startswith("a-unique-name-")
 
     @pytest.mark.django_db
-    def test_duplicate_name_is_suffixed(self, classical_variant):
+    def test_duplicate_name_gets_distinct_id(self, classical_variant):
         first = Game.objects.create(name="Shared Name", variant=classical_variant)
         second = Game.objects.create(name="Shared Name", variant=classical_variant)
 
-        assert first.id == "shared-name"
-        assert second.id.startswith("shared-name-")
-
-    @pytest.mark.django_db
-    def test_id_taken_after_availability_check_is_retried(self, classical_variant):
-        existing = Game.objects.create(name="Shared Name", variant=classical_variant)
-
-        with patch.object(Game, "_generate_id", return_value=existing.id):
-            game = Game.objects.create(name="Shared Name", variant=classical_variant)
-
-        assert game.id != existing.id
-        assert game.id.startswith("shared-name-")
+        assert first.id != second.id
         assert Game.objects.filter(name="Shared Name").count() == 2
 
 

service/game/tests/test_game_delete.py

@@ -63,6 +63,16 @@ def test_delete_sandbox_game_unauthenticated(
         response = unauthenticated_client.delete(url)
         assert response.status_code == status.HTTP_401_UNAUTHORIZED
 
+    @pytest.mark.django_db
+    def test_delete_sandbox_game_twice_returns_404(
+        self, authenticated_client, sandbox_game_factory
+    ):
+        game = sandbox_game_factory()
+        url = reverse(delete_viewname, args=[game.id])
+
+        assert authenticated_client.delete(url).status_code == status.HTTP_204_NO_CONTENT
+        assert authenticated_client.delete(url).status_code == status.HTTP_404_NOT_FOUND
+
     @pytest.mark.django_db
     def test_delete_nonexistent_game(self, authenticated_client):
         url = reverse(delete_viewname, args=["nonexistent-game"])

service/game/views.py

@@ -1,3 +1,4 @@
+from django.db import transaction
 from django.shortcuts import get_object_or_404
 from rest_framework import generics, permissions, status
 from rest_framework.response import Response
@@ -18,7 +19,7 @@
     GameUnpauseSerializer,
     GameExtendDeadlineSerializer,
 )
-from common.views import SelectedGameMixin
+from common.views import SelectedGameMixin, resolve_game
 from common.serializers import EmptySerializer
 from common.permissions import IsActiveGame, IsGamePlayer, IsGameManager, CanDeleteGame
 from common.pagination import StandardPageNumberPagination
@@ -146,14 +147,17 @@ def get_object(self):
         return self.get_game()
 
     def perform_destroy(self, instance):
-        is_game_master_delete = (
-            not instance.sandbox
-            and instance.game_master_id is not None
-            and instance.game_master_id == self.request.user.id
-        )
-        user_ids = list(instance.seated_member_user_ids() - {self.request.user.id})
-        game_name = instance.name
-        instance.delete()
+        with transaction.atomic():
+            game = resolve_game(self.request, self.kwargs.get("game_id"), lock=True)
+            self.check_permissions(self.request)
+            is_game_master_delete = (
+                not game.sandbox
+                and game.game_master_id is not None
+                and game.game_master_id == self.request.user.id
+            )
+            user_ids = list(game.seated_member_user_ids() - {self.request.user.id})
+            game_name = game.name
+            game.delete()
         if is_game_master_delete:
             emit("game_deleted", recipients=user_ids, game_name=game_name)