From d36d256dafcfa3f692f84742702dd946a3ae6d79 Mon Sep 17 00:00:00 2001 From: DEV-bot Date: Tue, 17 Mar 2026 05:41:13 +0000 Subject: [PATCH] fix(gameplay): make scoreboard host exits idempotent --- ...10-HOST-TRANSITION-IDEMPOTENCY-ARTIFACT.md | 33 +++ lobby/tests.py | 42 ++++ lobby/views.py | 206 ++++++++++-------- 3 files changed, 191 insertions(+), 90 deletions(-) create mode 100644 docs/ISSUE-310-HOST-TRANSITION-IDEMPOTENCY-ARTIFACT.md diff --git a/docs/ISSUE-310-HOST-TRANSITION-IDEMPOTENCY-ARTIFACT.md b/docs/ISSUE-310-HOST-TRANSITION-IDEMPOTENCY-ARTIFACT.md new file mode 100644 index 0000000..8a06681 --- /dev/null +++ b/docs/ISSUE-310-HOST-TRANSITION-IDEMPOTENCY-ARTIFACT.md @@ -0,0 +1,33 @@ +# Issue #310 — Host transition idempotency and error catalog + +## Scope + +This artifact hardens the two host-owned scoreboard exits in the canonical gameplay flow: + +- `POST /lobby/sessions/{code}/rounds/next` +- `POST /lobby/sessions/{code}/finish` + +The goal is retry-safe host behavior when the scoreboard transition already succeeded server-side but the client retries because of a duplicate click, timeout, or lost response. + +## Transition contract + +| Endpoint | First valid transition | Idempotent replay state | Replay result | Broadcast behavior | Still-invalid states | +|---|---|---|---|---|---| +| `POST /lobby/sessions/{code}/rounds/next` | `scoreboard -> lie` | `lie` with persisted current-round bootstrap (`RoundConfig` + `RoundQuestion`) | `200 OK` with the same canonical next-round payload shape | `phase.lie_started` fires only on the first transition | `lobby`, `guess`, `reveal`, `finished` → `next_round_invalid_phase` | +| `POST /lobby/sessions/{code}/finish` | `scoreboard -> finished` | `finished` | `200 OK` with the same final leaderboard payload shape | `phase.game_over` fires only on the first transition | `lobby`, `lie`, `guess`, `reveal` → `finish_game_invalid_phase` | + +## Error catalog notes + +No new backend error codes were introduced for this slice. + +The contract change is behavioral: + +- `next_round_invalid_phase` now means the session is in a phase where the scoreboard → next-round transition has **not** already been completed, or the expected bootstrap artifact for the already-started round is missing. +- `finish_game_invalid_phase` now means the session is in a phase where the scoreboard → finish transition has **not** already been completed. +- Successful replays are returned as normal `200 OK` canonical responses instead of phase errors. + +## Acceptance evidence + +- Repeated `rounds/next` calls after a successful scoreboard exit return the same canonical lie/bootstrap payload without incrementing the round twice. +- Repeated `finish` calls after a successful scoreboard exit return the same finished leaderboard payload without rebroadcasting game-over. +- Wrong-phase calls outside those replay states still return the existing shared error codes. diff --git a/lobby/tests.py b/lobby/tests.py index dead0e8..5dac6e8 100644 --- a/lobby/tests.py +++ b/lobby/tests.py @@ -1216,6 +1216,25 @@ class RevealRoundFlowTests(TestCase): self.session.refresh_from_db() self.assertEqual(self.session.status, GameSession.Status.FINISHED) + @patch("lobby.views.sync_broadcast_phase_event") + def test_finish_game_is_idempotent_after_transition_to_finished(self, mock_sync_broadcast_phase_event): + self.client.login(username="host_reveal", password="secret123") + self.client.get(reverse("lobby:reveal_scoreboard", kwargs={"code": self.session.code})) + + first_response = self.client.post(reverse("lobby:finish_game", kwargs={"code": self.session.code})) + second_response = self.client.post(reverse("lobby:finish_game", kwargs={"code": self.session.code})) + + self.assertEqual(first_response.status_code, 200) + self.assertEqual(second_response.status_code, 200) + self.assertEqual(first_response.json(), second_response.json()) + self.assertEqual(second_response.json()["session"]["status"], GameSession.Status.FINISHED) + + self.session.refresh_from_db() + self.assertEqual(self.session.status, GameSession.Status.FINISHED) + self.assertEqual(mock_sync_broadcast_phase_event.call_count, 2) + self.assertEqual(mock_sync_broadcast_phase_event.call_args_list[0].args[1], "phase.scoreboard") + self.assertEqual(mock_sync_broadcast_phase_event.call_args_list[1].args[1], "phase.game_over") + def test_finish_game_requires_host(self): self.client.login(username="other_reveal", password="secret123") @@ -1284,6 +1303,29 @@ class RevealRoundFlowTests(TestCase): self.assertEqual(mock_sync_broadcast_phase_event.call_args.args[0], self.session.code) self.assertEqual(mock_sync_broadcast_phase_event.call_args.args[1], "phase.lie_started") + @patch("lobby.views.sync_broadcast_phase_event") + def test_start_next_round_is_idempotent_after_transition_to_lie(self, mock_sync_broadcast_phase_event): + self.client.login(username="host_reveal", password="secret123") + self.client.get(reverse("lobby:reveal_scoreboard", kwargs={"code": self.session.code})) + mock_sync_broadcast_phase_event.reset_mock() + + first_response = self.client.post(reverse("lobby:start_next_round", kwargs={"code": self.session.code})) + second_response = self.client.post(reverse("lobby:start_next_round", kwargs={"code": self.session.code})) + + self.assertEqual(first_response.status_code, 200) + self.assertEqual(second_response.status_code, 200) + self.assertEqual(first_response.json(), second_response.json()) + self.assertEqual(second_response.json()["session"]["status"], GameSession.Status.LIE) + self.assertEqual(second_response.json()["session"]["current_round"], 2) + + self.session.refresh_from_db() + self.assertEqual(self.session.status, GameSession.Status.LIE) + self.assertEqual(self.session.current_round, 2) + self.assertEqual(RoundConfig.objects.filter(session=self.session, number=2).count(), 1) + self.assertEqual(RoundQuestion.objects.filter(session=self.session, round_number=2).count(), 1) + mock_sync_broadcast_phase_event.assert_called_once() + self.assertEqual(mock_sync_broadcast_phase_event.call_args.args[1], "phase.lie_started") + def test_start_next_round_requires_host(self): self.session.status = GameSession.Status.SCOREBOARD self.session.save(update_fields=["status"]) diff --git a/lobby/views.py b/lobby/views.py index 414b8a0..23afb39 100644 --- a/lobby/views.py +++ b/lobby/views.py @@ -65,6 +65,57 @@ def _create_unique_session_code() -> str: raise RuntimeError("Could not generate unique session code") +def _build_start_next_round_response( + session: GameSession, + round_config: RoundConfig, + round_question: RoundQuestion, +) -> JsonResponse: + lie_started_payload = _build_lie_started_payload(session, round_config, round_question) + return JsonResponse( + { + "session": { + "code": session.code, + "status": session.status, + "current_round": session.current_round, + }, + "round": { + "number": round_config.number, + "category": { + "slug": round_config.category.slug, + "name": round_config.category.name, + }, + }, + "round_question": { + "id": round_question.id, + "prompt": round_question.question.prompt, + "round_number": round_question.round_number, + "shown_at": round_question.shown_at.isoformat(), + "lie_deadline_at": lie_started_payload["lie_deadline_at"], + }, + "config": { + "lie_seconds": round_config.lie_seconds, + }, + } + ) + + + +def _build_finish_game_response(session: GameSession) -> JsonResponse: + leaderboard = _build_leaderboard(session) + winner = leaderboard[0] if leaderboard else None + return JsonResponse( + { + "session": { + "code": session.code, + "status": GameSession.Status.FINISHED, + "current_round": session.current_round, + }, + "winner": winner, + "leaderboard": leaderboard, + } + ) + + def _maybe_promote_reveal_to_scoreboard(session: GameSession) -> GameSession: if session.status != GameSession.Status.REVEAL: return session @@ -917,72 +968,61 @@ def start_next_round(request: HttpRequest, code: str) -> JsonResponse: if session.host_id != request.user.id: return api_error(request, code="host_only_start_next_round", status=403) + should_broadcast = False with transaction.atomic(): - locked_session = GameSession.objects.select_for_update().get(pk=session.pk) - if locked_session.status != GameSession.Status.SCOREBOARD: + locked_session = GameSession.objects.select_for_update().select_related("host").get(pk=session.pk) + next_round_config = None + round_question = None + + if locked_session.status == GameSession.Status.SCOREBOARD: + previous_round_config = RoundConfig.objects.filter( + session=locked_session, + number=locked_session.current_round, + ).select_related("category").first() + if previous_round_config is None: + return api_error(request, code="round_config_missing", status=400) + + next_round_number = locked_session.current_round + 1 + next_round_config = RoundConfig( + session=locked_session, + number=next_round_number, + category=previous_round_config.category, + lie_seconds=previous_round_config.lie_seconds, + guess_seconds=previous_round_config.guess_seconds, + points_correct=previous_round_config.points_correct, + points_bluff=previous_round_config.points_bluff, + ) + locked_session.current_round = next_round_number + + try: + round_question = _select_round_question(locked_session, next_round_config) + except ValueError as exc: + return api_error(request, code=str(exc), status=400) + + next_round_config.save() + locked_session.status = GameSession.Status.LIE + locked_session.save(update_fields=["current_round", "status"]) + should_broadcast = True + elif locked_session.status == GameSession.Status.LIE: + next_round_config = RoundConfig.objects.filter( + session=locked_session, + number=locked_session.current_round, + ).select_related("category").first() + round_question = _get_current_round_question(locked_session) + if next_round_config is None or round_question is None: + return api_error(request, code="next_round_invalid_phase", status=400) + else: return api_error(request, code="next_round_invalid_phase", status=400) - previous_round_config = RoundConfig.objects.filter( - session=locked_session, - number=locked_session.current_round, - ).select_related("category").first() - if previous_round_config is None: - return api_error(request, code="round_config_missing", status=400) - - next_round_number = locked_session.current_round + 1 - next_round_config = RoundConfig( - session=locked_session, - number=next_round_number, - category=previous_round_config.category, - lie_seconds=previous_round_config.lie_seconds, - guess_seconds=previous_round_config.guess_seconds, - points_correct=previous_round_config.points_correct, - points_bluff=previous_round_config.points_bluff, + if should_broadcast: + lie_started_payload = _build_lie_started_payload(locked_session, next_round_config, round_question) + sync_broadcast_phase_event( + locked_session.code, + "phase.lie_started", + lie_started_payload, ) - locked_session.current_round = next_round_number - try: - round_question = _select_round_question(locked_session, next_round_config) - except ValueError as exc: - return api_error(request, code=str(exc), status=400) - - next_round_config.save() - locked_session.status = GameSession.Status.LIE - locked_session.save(update_fields=["current_round", "status"]) - - lie_started_payload = _build_lie_started_payload(locked_session, next_round_config, round_question) - sync_broadcast_phase_event( - locked_session.code, - "phase.lie_started", - lie_started_payload, - ) - - return JsonResponse( - { - "session": { - "code": locked_session.code, - "status": locked_session.status, - "current_round": locked_session.current_round, - }, - "round": { - "number": next_round_config.number, - "category": { - "slug": next_round_config.category.slug, - "name": next_round_config.category.name, - }, - }, - "round_question": { - "id": round_question.id, - "prompt": round_question.question.prompt, - "round_number": round_question.round_number, - "shown_at": round_question.shown_at.isoformat(), - "lie_deadline_at": lie_started_payload["lie_deadline_at"], - }, - "config": { - "lie_seconds": next_round_config.lie_seconds, - }, - } - ) + return _build_start_next_round_response(locked_session, next_round_config, round_question) @require_POST @login_required @@ -997,40 +1037,26 @@ def finish_game(request: HttpRequest, code: str) -> JsonResponse: if session.host_id != request.user.id: return api_error(request, code="host_only_finish_game", status=403) + should_broadcast = False with transaction.atomic(): locked_session = GameSession.objects.select_for_update().get(pk=session.pk) - if locked_session.status != GameSession.Status.SCOREBOARD: + if locked_session.status == GameSession.Status.SCOREBOARD: + locked_session.status = GameSession.Status.FINISHED + locked_session.save(update_fields=["status"]) + should_broadcast = True + elif locked_session.status != GameSession.Status.FINISHED: return api_error(request, code="finish_game_invalid_phase", status=400) + if should_broadcast: + leaderboard = _build_leaderboard(locked_session) + winner = leaderboard[0] if leaderboard else None + sync_broadcast_phase_event( + locked_session.code, + "phase.game_over", + {"winner": winner, "leaderboard": list(leaderboard)}, + ) - locked_session.status = GameSession.Status.FINISHED - locked_session.save(update_fields=["status"]) - - leaderboard = list( - Player.objects.filter(session=session) - .order_by("-score", "nickname") - .values("id", "nickname", "score") - ) - - winner = leaderboard[0] if leaderboard else None - - sync_broadcast_phase_event( - session.code, - "phase.game_over", - {"winner": winner, "leaderboard": list(leaderboard)}, - ) - - return JsonResponse( - { - "session": { - "code": session.code, - "status": GameSession.Status.FINISHED, - "current_round": session.current_round, - }, - "winner": winner, - "leaderboard": leaderboard, - } - ) + return _build_finish_game_response(locked_session) @require_POST