From f9acea1d9a94845a092cea6332d5845f8dd3cfde Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Tue, 30 Jun 2026 22:51:54 +0200 Subject: [PATCH] fix(game): clear nudges on game completion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nudge badges lingered in the lobby on games that ended by turn-timeout, resignation or forfeit: those paths commit the finish directly, bypassing the move path's per-mover NudgeClearer, so the awaited seat's nudge was never marked read. A finished game's nudges are stale, so clear them all. Add a wired NudgeExpirer (social.ExpireNudges) called from the shared commit finish block — covering every completion path — and from the voidGame recovery path. It clears every seat's nudge bits for the game and leaves chat messages unread; unlike ClearNudges it records no publish-to-read latency, since a completion is an expiry, not a read. An integration test reproduces the timeout case (nudge cleared, chat kept). Docs: ARCHITECTURE §9.1, FUNCTIONAL (+_ru), backend/README. --- backend/README.md | 6 ++- backend/cmd/backend/main.go | 4 +- backend/internal/game/service.go | 44 +++++++++++++++-- backend/internal/inttest/chatread_test.go | 59 +++++++++++++++++++++++ backend/internal/social/chatread.go | 34 +++++++++++++ docs/ARCHITECTURE.md | 11 +++-- docs/FUNCTIONAL.md | 3 +- docs/FUNCTIONAL_ru.md | 3 +- 8 files changed, 152 insertions(+), 12 deletions(-) diff --git a/backend/README.md b/backend/README.md index 5a6230b..e3ba1a3 100644 --- a/backend/README.md +++ b/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**. diff --git a/backend/cmd/backend/main.go b/backend/cmd/backend/main.go index c8b4bfa..5db95db 100644 --- a/backend/cmd/backend/main.go +++ b/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) diff --git a/backend/internal/game/service.go b/backend/internal/game/service.go index 6a8dffb..3ed494a 100644 --- a/backend/internal/game/service.go +++ b/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. diff --git a/backend/internal/inttest/chatread_test.go b/backend/internal/inttest/chatread_test.go index 3b69423..47c3bc1 100644 --- a/backend/internal/inttest/chatread_test.go +++ b/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). diff --git a/backend/internal/social/chatread.go b/backend/internal/social/chatread.go index 5b1b495..966eee5 100644 --- a/backend/internal/social/chatread.go +++ b/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) { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ad04ea9..9c0fbdd 100644 --- a/docs/ARCHITECTURE.md +++ b/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 diff --git a/docs/FUNCTIONAL.md b/docs/FUNCTIONAL.md index 7393042..6d3d363 100644 --- a/docs/FUNCTIONAL.md +++ b/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 diff --git a/docs/FUNCTIONAL_ru.md b/docs/FUNCTIONAL_ru.md index 29225cd..36ed240 100644 --- a/docs/FUNCTIONAL_ru.md +++ b/docs/FUNCTIONAL_ru.md @@ -269,7 +269,8 @@ _Вход сейчас только через провайдера, поэто **лобби** и на **строке счёта** партии, **красную при непрочитанном сообщении и мягкую жёлтую, когда непрочитаны только nudge'и**. Открытие **истории ходов** считается прочтением чата, даже без захода в него: точка гаснет, а значок 💬 **дважды мигает**. Nudge также гаснет в момент, когда -его получатель **делает ход**. +его получатель **делает ход**, и **все nudge'и партии гаснут по её завершении** (после конца +партии бейдж неактуален) — но непрочитанные сообщения чата остаются непрочитанными. ### Профиль и настройки Редактирование отображаемого имени (буквы, разделённые одиночным пробелом / «.» /