Merge pull request 'fix(game): clear nudges on game completion' (#148) from feature/nudge-clear-on-completion into development

backend/README.md

@@ -43,8 +43,10 @@ per-user blocks, and per-game chat with nudges folded in as a message kind; chat
 messages are length-capped, content-filtered (no links/emails/phone numbers,
 including obfuscated forms) and stored with the sender's IP. Each message carries an
 `unread_seats` read bitmask (a set bit per recipient seat still to read it); `MarkRead`
-clears a reader's bit when they open the move history or chat, and a wired `NudgeClearer`
-clears a nudge when its recipient moves — both record the publish-to-read latency.
+clears a reader's bit when they open the move history or chat, a wired `NudgeClearer`
+clears a nudge when its recipient moves, and a wired `NudgeExpirer` clears **all** of a game's
+nudges when it finishes (any completion path) — the first two record the publish-to-read latency,
+the completion expiry does not (it is not a read); chat messages stay unread on completion.
 A friend request (or block) aimed at a **disguised pooled robot** is recorded per game+seat
 in `robot_friend_requests` / `robot_blocks`, never against the shared robot account; a
 background reaper drops a robot friend request once its game has been finished for **7 days**.

backend/cmd/backend/main.go

@@ -180,8 +180,10 @@ func run(ctx context.Context, cfg config.Config, logger *zap.Logger) error {
 	socialSvc := social.NewService(social.NewStore(db), accounts, games)
 	socialSvc.SetNotifier(hub)
 	socialSvc.SetMetrics(tel.MeterProvider().Meter("scrabble/backend/social"))
-	// A nudge the recipient answered by moving is marked read on the move path.
+	// A nudge the recipient answered by moving is marked read on the move path; every nudge in a
+	// game is marked read when the game finishes (a stale badge), on any completion path.
 	games.SetNudgeClearer(socialSvc.ClearNudges)
+	games.SetNudgeExpirer(socialSvc.ExpireNudges)
 	// Reap per-game disguised-robot friend requests once their game is long finished
 	// (the robot ignores them; the row only pins the in-game "request sent" state).
 	robotReqReaper := social.NewRobotFriendRequestReaper(socialSvc, logger)

backend/internal/game/service.go

@@ -54,8 +54,14 @@ type Service struct {
 	// have committed a move (a nudge answered by moving stops counting as unread). It is
 	// best-effort and kept as a func so the game package never imports the social package.
 	clearNudges func(ctx context.Context, gameID, accountID uuid.UUID) error
-	metrics     *gameMetrics
-	log         *zap.Logger
+	// expireNudges, when set, marks every pending nudge in a game read once the game
+	// finishes (the nudge badge is stale on a completed game). Unlike clearNudges it is
+	// keyed by game alone — it clears all seats' nudges, not one mover's — and runs on
+	// every completion path through commit. Best-effort; a func so the game package never
+	// imports the social package.
+	expireNudges func(ctx context.Context, gameID uuid.UUID) error
+	metrics      *gameMetrics
+	log          *zap.Logger
 }
 
 // NewService constructs a Service. store and accounts wrap the same pool;
@@ -107,6 +113,15 @@ func (svc *Service) SetNudgeClearer(fn func(ctx context.Context, gameID, account
 	svc.clearNudges = fn
 }
 
+// SetNudgeExpirer installs the hook that marks every pending nudge in a game read once the
+// game finishes, on any completion path (a closing move, a resignation, a turn-timeout or a
+// forfeit). It must be called during startup wiring; the default (nil) leaves a finished
+// game's nudges to expire only when a recipient opens the move history or chat. The social
+// package wires its ExpireNudges here. Chat messages are deliberately left unread.
+func (svc *Service) SetNudgeExpirer(fn func(ctx context.Context, gameID uuid.UUID) error) {
+	svc.expireNudges = fn
+}
+
 // SetFirstMoveEntropy overrides the entropy source for the first-move draw
 // (docs/ARCHITECTURE.md §6). It must be called during wiring or test setup before any
 // game is created; the production default is crypto/rand and is never overridden.
@@ -699,6 +714,16 @@ func (svc *Service) commit(ctx context.Context, gameID uuid.UUID, g *engine.Game
 	}
 	if c.finished {
 		svc.cache.remove(gameID)
+		// A finished game's nudges are stale, so clear them all here — every completion path
+		// funnels through commit (a closing move, a resignation, a forfeit or a turn-timeout),
+		// and only the move path also clears the mover's nudge on its own. Best-effort like
+		// clearNudges: the finish has committed, so a cleanup failure is logged, not surfaced.
+		// ExpireNudges leaves chat messages unread.
+		if svc.expireNudges != nil {
+			if err := svc.expireNudges(ctx, gameID); err != nil {
+				svc.log.Warn("expire nudges on game finish", zap.Error(err))
+			}
+		}
 	}
 	post, err := svc.store.GetGame(ctx, gameID)
 	if err != nil {
@@ -1440,13 +1465,24 @@ func (svc *Service) voidGame(ctx context.Context, pre Game, g *engine.Game) erro
 	if err != nil {
 		return err
 	}
-	return svc.store.VoidGame(ctx, voidCommit{
+	if err := svc.store.VoidGame(ctx, voidCommit{
 		gameID:    pre.ID,
 		endReason: g.Reason().String(),
 		scores:    scores,
 		now:       svc.clock(),
 		stats:     buildStats(g, statSeats),
-	})
+	}); err != nil {
+		return err
+	}
+	// A voided game is finished (as a draw) but bypasses commit, so clear its now-stale nudges
+	// here too. Best-effort, like the commit path: the void has persisted, so a cleanup failure
+	// is logged, not surfaced.
+	if svc.expireNudges != nil {
+		if err := svc.expireNudges(ctx, pre.ID); err != nil {
+			svc.log.Warn("expire nudges on voided game", zap.Error(err))
+		}
+	}
+	return nil
 }
 
 // replayMove re-applies one journalled move to g through the decoded engine API.

backend/internal/inttest/chatread_test.go

@@ -138,6 +138,65 @@ func TestNudgeClearedByMove(t *testing.T) {
 	}
 }
 
+// TestGameCompletionExpiresNudgesKeepsChat checks that finishing a game by turn-timeout marks every
+// pending nudge in it read — the lobby's nudge badge is stale once the game is over — while leaving
+// real chat messages unread. It reproduces the reported bug: the timeout path commits the finish
+// directly, bypassing the move path's per-mover nudge clear, so without a completion-driven expiry
+// the awaited seat's nudge lingered as a badge on the finished game.
+func TestGameCompletionExpiresNudgesKeepsChat(t *testing.T) {
+	ctx := context.Background()
+	gameSvc := newGameService()
+	socialSvc := newSocialService()
+	gameSvc.SetNudgeExpirer(socialSvc.ExpireNudges)
+
+	seats := []uuid.UUID{provisionAccount(t), provisionAccount(t)}
+	g, err := gameSvc.Create(ctx, game.CreateParams{
+		Variant: engine.VariantEnglish, Seats: seats, TurnTimeout: time.Hour, Seed: openingSeed(t),
+	})
+	if err != nil {
+		t.Fatalf("create: %v", err)
+	}
+
+	// Seat 1 nudges the to-move seat 0 (the awaited player), and seat 0 posts a real chat message,
+	// which seat 1 then holds unread. So before the timeout each side has exactly one unread entry:
+	// seat 0 a nudge, seat 1 a message — letting HasUnread isolate each kind.
+	if _, err := socialSvc.Nudge(ctx, g.ID, seats[1]); err != nil {
+		t.Fatalf("nudge: %v", err)
+	}
+	if _, err := socialSvc.PostMessage(ctx, g.ID, seats[0], "good luck", ""); err != nil {
+		t.Fatalf("post message: %v", err)
+	}
+	if unread, _ := socialSvc.HasUnread(ctx, g.ID, seats[0]); !unread {
+		t.Fatal("seat 0 should hold the nudge unread before the timeout")
+	}
+	if unread, _ := socialSvc.HasUnread(ctx, g.ID, seats[1]); !unread {
+		t.Fatal("seat 1 should hold the message unread before the timeout")
+	}
+
+	// Age the turn past its deadline and time seat 0 out; an empty away window keeps this
+	// deterministic regardless of the wall clock. The sweep finishes the game through the direct
+	// commit path, never the move path.
+	backdate(t, g.ID, time.Now().UTC().Add(-2*time.Hour))
+	setAway(t, seats[0], "UTC", "00:00", "00:00")
+	if n, err := gameSvc.SweepTimeouts(ctx, time.Now().UTC()); err != nil || n < 1 {
+		t.Fatalf("sweep swept %d (err %v), want >= 1", n, err)
+	}
+	if status, reason := gameStatus(t, gameSvc, g.ID); status != game.StatusFinished || reason != "timeout" {
+		t.Fatalf("game not timed out: status %q reason %q", status, reason)
+	}
+
+	// The nudge is stale on a finished game and must be cleared; the chat message must survive.
+	if unread, _ := socialSvc.HasUnread(ctx, g.ID, seats[0]); unread {
+		t.Error("the nudge should be expired once the game has finished")
+	}
+	if unread, _ := socialSvc.HasUnread(ctx, g.ID, seats[1]); !unread {
+		t.Error("a real chat message must stay unread after the game finishes")
+	}
+	if msg, _ := socialSvc.HasUnreadMessage(ctx, g.ID, seats[1]); !msg {
+		t.Error("the chat message must remain flagged as an unread message after completion")
+	}
+}
+
 // TestChatToRobotIsBornRead checks a text message to a disguised robot opponent (a pooled
 // robot substituted into an ordinary, non-AI game) is born read: the robot never opens the
 // chat, so the message must not linger unread (skewing the count and the read metric).

backend/internal/social/chatread.go

@@ -55,6 +55,25 @@ func (svc *Service) ClearNudges(ctx context.Context, gameID, accountID uuid.UUID
 	return nil
 }
 
+// ExpireNudges marks every pending nudge in the game read, for when the game finishes: a nudge
+// badge is stale once the game is over, so completion clears them all for every seat. Chat
+// messages are left untouched (they stay unread). It satisfies game.NudgeExpirer and is wired
+// into the completion path; failures are the caller's to log (the game has already finished).
+// Unlike ClearNudges it records no publish-to-read latency — a completion is an expiry, not the
+// recipient reading the nudge — so the latency metric is not skewed by games that end hours later.
+func (svc *Service) ExpireNudges(ctx context.Context, gameID uuid.UUID) error {
+	ctx, span := svc.tracer.Start(ctx, "social.ExpireNudges",
+		trace.WithAttributes(attribute.String("game.id", gameID.String())))
+	defer span.End()
+	n, err := svc.store.expireNudges(ctx, gameID)
+	if err != nil {
+		span.RecordError(err)
+		return err
+	}
+	span.SetAttributes(attribute.Int64("expired.count", n))
+	return nil
+}
+
 // UnreadGames returns the set of games in which viewerID has at least one unread chat
 // entry, for seeding the lobby's per-card unread badge in a single query.
 func (svc *Service) UnreadGames(ctx context.Context, viewerID uuid.UUID) (map[uuid.UUID]bool, error) {
@@ -140,6 +159,21 @@ RETURNING m.created_at`
 	return out, rows.Err()
 }
 
+// expireNudges marks every still-unread nudge in the game read for all seats at once, used when
+// the game finishes. It returns the number of nudge entries it cleared. Only 'nudge' rows are
+// touched, so chat messages keep their unread bits; it returns no post times because a completion
+// is an expiry, not a player reading, and must not feed the publish-to-read latency metric.
+func (s *Store) expireNudges(ctx context.Context, gameID uuid.UUID) (int64, error) {
+	const q = `UPDATE backend.chat_messages
+SET unread_seats = 0
+WHERE game_id = $1 AND kind = 'nudge' AND unread_seats <> 0`
+	res, err := s.db.ExecContext(ctx, q, gameID)
+	if err != nil {
+		return 0, fmt.Errorf("social: expire nudges: %w", err)
+	}
+	return res.RowsAffected()
+}
+
 // unreadGames returns the games where viewerID has an unread entry, resolving their
 // seat per game through the game_players join.
 func (s *Store) unreadGames(ctx context.Context, viewerID uuid.UUID) (map[uuid.UUID]bool, error) {

docs/ARCHITECTURE.md

@@ -646,7 +646,11 @@ in either direction (the enqueue excludes the caller's `BlockedWith` set);
   robot instead clears when the robot answers by moving, as for a human. A seat's bit clears when that player **opens the move history or the chat**
   (`POST /games/:id/chat/read`, which the client sends only when it holds unread, so a
   history open is not a constant backend call), and a **nudge additionally clears when its
-  recipient answers by moving** (the move path calls a wired `NudgeClearer`). The mask is
+  recipient answers by moving** (the move path calls a wired `NudgeClearer`); and **every nudge in
+  a game is marked read when that game finishes** — on any completion path (a closing move, a
+  resignation, a forfeit or a turn-timeout, all funnelling through the shared `commit`), since the
+  nudge badge is stale once the game is over (a wired `NudgeExpirer`; chat messages stay unread and
+  this expiry records no read latency). The mask is
   inverted so "anything unread" is a plain `unread_seats <> 0`, which the per-viewer
   `unread_chat` game-view flag (seeding the lobby and in-game unread **dot**), the admin
   unread filter and the unread gauge all use. A second per-viewer flag, **`unread_messages`**
@@ -656,8 +660,9 @@ in either direction (the enqueue excludes the caller's `BlockedWith` set);
   REST-seed-then-event-bump lifecycle (the live-event game-view leaves them false; a nudge
   event raises only `unread_chat`, a message event raises both). The lobby additionally
   **floats games with any unread entry to the top** of the your-turn and opponent-turn
-  sections (the finished section keeps its activity order). On each clear the publish-to-read
-  latency is recorded; the read time itself is not retained.
+  sections (the finished section keeps its activity order). On each player-driven clear the
+  publish-to-read latency is recorded — the completion expiry records none (it is not a read);
+  the read time itself is not retained.
 - **Profile**: `preferred_language` (en/ru; tracks the interface language — §4), display name, email
   (confirm-code binding, see §4), **timezone**, the daily **away window**, the
   **variant preferences** (`variant_preferences`, the matchable-variant set that gates New

docs/FUNCTIONAL.md

@@ -260,7 +260,8 @@ entry — a message **or a nudge** from an opponent — raises a small **dot** n
 in the **lobby** and on the game's **score bar**, **red when an unread message is waiting and a
 softer amber when only nudges are**. Opening the **move history** counts as reading
 the chat, even without entering it: the dot clears and the 💬 icon **fade-blinks twice**. A nudge
-also clears the moment its recipient **takes their move**.
+also clears the moment its recipient **takes their move**, and **all of a game's nudges clear once
+the game finishes** (the badge is stale once the game is over) — but unread chat messages stay unread.
 
 ### Profile & settings
 Edit the display name (letters joined by a single space / "." / "_" separator, with an

docs/FUNCTIONAL_ru.md

@@ -269,7 +269,8 @@ _Вход сейчас только через провайдера, поэто
 **лобби** и на **строке счёта** партии, **красную при непрочитанном сообщении и мягкую жёлтую,
 когда непрочитаны только nudge'и**. Открытие **истории ходов** считается прочтением чата, даже
 без захода в него: точка гаснет, а значок 💬 **дважды мигает**. Nudge также гаснет в момент, когда
-его получатель **делает ход**.
+его получатель **делает ход**, и **все nudge'и партии гаснут по её завершении** (после конца
+партии бейдж неактуален) — но непрочитанные сообщения чата остаются непрочитанными.
 
 ### Профиль и настройки
 Редактирование отображаемого имени (буквы, разделённые одиночным пробелом / «.» /