From 7e073dda0b05e6375c198d04c7059aed61fb7cd7 Mon Sep 17 00:00:00 2001 From: Evan Date: Sat, 18 Jul 2026 14:08:59 -0700 Subject: [PATCH] fix(server): keep matchmaking games out of lobby reports; salvage invalid entries (#4641) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem Prod masters log `Invalid IPC message from worker` for every ranked 1v1/2v2 match created: 1. The matchmaking poll loop creates the match game with the 1v1/2v2 playlist config (`gameType: Public`) but no `publicGameType` — correctly, since ranked games are invite-only and should never be advertised. 2. While that game sits in its ~7s lobby window, `GameManager.publicLobbies()` includes it (it only checks phase + `isPublic()`), and `WorkerLobbyService.sendMyLobbiesToMaster()` reports it with `publicGameType: gi.publicGameType!` — the non-null assertion hides the `undefined`, and IPC serialization drops the key. 3. The master's `WorkerMessageSchema` parse fails on the missing required field and **discards the worker's entire lobby report**. So for the lobby window of every ranked match, the master's view of that worker freezes: stale player counts and countdowns broadcast to all clients, `maybeScheduleLobby` repeatedly resetting the next lobby's countdown, and occasional duplicate scheduled lobbies (with no cleanup path for scheduled types). With matchmaking running continuously, some worker is blocked at any given moment on a busy server. ## Fix - **Worker** (`WorkerLobbyService`): filter games without a `publicGameType` out of the report. Matchmaking games are never advertised — which also guarantees they can't leak into the public lobby browser. - **Master** (`MasterLobbyService` + `IPCBridgeSchema`): validate lobby entries individually instead of failing the whole message. Malformed entries are logged and dropped; valid lobbies survive. A future bug of this class now degrades to one missing entry instead of a frozen worker view. The send side keeps compile-time safety via the existing per-entry `satisfies` annotations. ## Testing - New test: a matchmaking-style game (Public, `allowedPublicIds`, no `publicGameType`) is excluded from the worker's `lobbyList` report while a normal FFA lobby still goes through. - New test: a report containing a malformed entry keeps its valid lobbies in the next broadcast instead of being rejected wholesale. - Full `tests/server` suite passes (173 tests); `tsc --noEmit` and eslint clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 --- src/server/IPCBridgeSchema.ts | 8 +++- src/server/MasterLobbyService.ts | 21 ++++++++++- src/server/WorkerLobbyService.ts | 4 ++ tests/server/HostedLobbyListing.test.ts | 50 +++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 3 deletions(-) diff --git a/src/server/IPCBridgeSchema.ts b/src/server/IPCBridgeSchema.ts index 2de366122..9e31c600b 100644 --- a/src/server/IPCBridgeSchema.ts +++ b/src/server/IPCBridgeSchema.ts @@ -33,10 +33,14 @@ export const InternalPublicGamesSchema = z.object({ // --- Worker Messages --- -// Worker tells the master about its lobbies. +// Worker tells the master about its lobbies. Entries are deliberately not +// validated here: the master checks each against InternalGameInfoSchema and +// drops bad ones (MasterLobbyService.validLobbies), so a single malformed +// lobby can't invalidate the whole report and freeze the master's view of +// this worker's lobbies. const WorkerLobbyListSchema = z.object({ type: z.literal("lobbyList"), - lobbies: z.array(InternalGameInfoSchema), + lobbies: z.array(z.unknown()), }); const WorkerReadySchema = z.object({ diff --git a/src/server/MasterLobbyService.ts b/src/server/MasterLobbyService.ts index 035a537f5..17ccaace6 100644 --- a/src/server/MasterLobbyService.ts +++ b/src/server/MasterLobbyService.ts @@ -8,6 +8,7 @@ import { import { generateID } from "../core/Util"; import { InternalGameInfo, + InternalGameInfoSchema, MasterCreateGame, MasterLobbiesBroadcast, MasterUpdateGame, @@ -57,12 +58,30 @@ export class MasterLobbyService { this.handleWorkerReady(msg.workerId); break; case "lobbyList": - this.workerLobbies.set(workerId, msg.lobbies); + this.workerLobbies.set(workerId, this.validLobbies(msg.lobbies)); break; } }); } + // Lobby entries are validated individually so one malformed entry only + // drops itself. Rejecting the whole report would freeze this worker's + // lobbies in the master's view for as long as the bad entry exists — + // stale broadcasts to every client, countdown resets, and duplicate + // scheduling. + private validLobbies(lobbies: unknown[]): InternalGameInfo[] { + const valid: InternalGameInfo[] = []; + for (const lobby of lobbies) { + const result = InternalGameInfoSchema.safeParse(lobby); + if (result.success) { + valid.push(result.data); + } else { + this.log.error("Dropping invalid lobby in worker report:", lobby); + } + } + return valid; + } + removeWorker(workerId: number) { this.workers.delete(workerId); this.workerLobbies.delete(workerId); diff --git a/src/server/WorkerLobbyService.ts b/src/server/WorkerLobbyService.ts index 24694f9dd..01e34572d 100644 --- a/src/server/WorkerLobbyService.ts +++ b/src/server/WorkerLobbyService.ts @@ -131,9 +131,13 @@ export class WorkerLobbyService { } private sendMyLobbiesToMaster() { + // Matchmaking games have a Public gameType (so they appear in + // publicLobbies()) but no publicGameType: they are invite-only and must + // never be advertised, and the master rejects entries without one. const publicLobbies = this.gm .publicLobbies() .map((g) => g.gameInfo()) + .filter((gi) => gi.publicGameType !== undefined) .map((gi) => { return { gameID: gi.gameID, diff --git a/tests/server/HostedLobbyListing.test.ts b/tests/server/HostedLobbyListing.test.ts index 1f8ce75ae..804337440 100644 --- a/tests/server/HostedLobbyListing.test.ts +++ b/tests/server/HostedLobbyListing.test.ts @@ -522,6 +522,27 @@ describe("MasterLobbyService hosted lobbies", () => { ]); }); + it("keeps the valid lobbies when a report contains a malformed entry", () => { + const { service, workers } = createService(); + workers[0].emit("message", { + type: "lobbyList", + lobbies: [ + // Missing publicGameType, like a matchmaking game reported by + // mistake. It must be dropped without discarding the whole report. + { gameID: "bad", numClients: 0 }, + hostedLobby("ok", "creator-a"), + ], + }); + + (service as any).broadcastLobbies(); + + const broadcast = sentMessages(workers[0]).find( + (m) => m.type === "lobbiesBroadcast", + ); + const hosted = broadcast.publicGames.games.hosted; + expect(hosted.map((l: InternalGameInfo) => l.gameID)).toEqual(["ok"]); + }); + it("delists a dedup loser only after two consecutive losing broadcasts", () => { const { service, workers } = createService(); workers[0].emit("message", { @@ -695,6 +716,35 @@ describe("WorkerLobbyService hosted lobbies", () => { expect(reported.gameConfig.hostCheats).toBeUndefined(); }); + it("excludes matchmaking games (Public but no publicGameType) from the report", () => { + const ranked = new GameServer( + "ranked-g1", + mockLogger, + Date.now(), + { gameType: GameType.Public, allowedPublicIds: ["p1", "p2"] } as any, + undefined, + Date.now() + 7000, + undefined, // matchmaking games are created without a publicGameType + ); + const ffa = new GameServer( + "ffa-g1", + mockLogger, + Date.now(), + { gameType: GameType.Public } as any, + undefined, + undefined, + "ffa", + ); + gm.publicLobbies.mockReturnValue([ranked, ffa]); + + emitBroadcast({ ffa: [], team: [], special: [], hosted: [] }); + + const lobbyList = sendToMaster.mock.calls + .map((c: any[]) => c[0]) + .find((m: any) => m.type === "lobbyList"); + expect(lobbyList.lobbies.map((l: any) => l.gameID)).toEqual(["ffa-g1"]); + }); + it("strips creatorID from broadcasts and primed snapshots sent to clients", () => { const ws = connectClient(); emitBroadcast({