Summary
generate_game_code (crates/server-core/src/session.rs:2722) returns 6 characters from a 36-symbol alphabet with no collision check, and create_game_n_players inserts the result into SessionManager::sessions with HashMap::insert. On a collision that silently destroys a live game session — the previous entry is dropped with no error, no log, and no notification to the players in it.
The lack of a check is deliberate and documented, but the comment that documents it (session.rs:60-68) justifies it for a different property. It argues the code is a sound InteractionSessionId because "uniqueness only has to hold within a state". That reasoning is sound for interaction-id namespacing, and does not extend to the registry key, where uniqueness has to hold across the whole process.
Why this is being filed now
Surfaced while reviewing #9172. That PR hardens the lobby expiry sweep so a replacement registration under a recycled code is not deleted and delisted by a stale expiry observation. Tracing reachability for that fix showed the only way a replacement takes an expired code in Full mode is a generate_game_code collision — and that under the very same precondition, the session registry already loses a live game outright, which is strictly worse than a mis-delisted lobby row.
#9172 deliberately did not harden this; it is a different seam, and hardening a symptom while the generator stays unguarded is the wrong order.
Scope
create_game / create_game_n_players should not insert under a code the registry already holds. Regenerate, or fail loudly.- Worth auditing the other consumers of the code as a key: the lobby registry (
LobbyManager::register_game likewise overwrites), the token index, and the persistence tables.
session.rs:60-68's comment should be narrowed to the claim it actually supports (interaction-id namespacing), so it stops reading as a blanket justification for the registry key.
Residual from #9172 that this would close
handle_expired_lobby_games retires a session by code before the broker is consulted, so under a collision it can destroy a replacement's session while #9172's fix keeps that session's lobby listing — a listing advertising a dead game. Gating that on identity would need identity threaded through SessionManager; guarding the generator removes the precondition instead.
Probability note
36^6 ≈ 2.18e9, so a collision is rare per creation but is a birthday problem against the set of live codes, and the failure mode is silent rather than degraded. The cost of a guard is one registry lookup on game creation.
Summary
generate_game_code(crates/server-core/src/session.rs:2722) returns 6 characters from a 36-symbol alphabet with no collision check, andcreate_game_n_playersinserts the result intoSessionManager::sessionswithHashMap::insert. On a collision that silently destroys a live game session — the previous entry is dropped with no error, no log, and no notification to the players in it.The lack of a check is deliberate and documented, but the comment that documents it (
session.rs:60-68) justifies it for a different property. It argues the code is a soundInteractionSessionIdbecause "uniqueness only has to hold within a state". That reasoning is sound for interaction-id namespacing, and does not extend to the registry key, where uniqueness has to hold across the whole process.Why this is being filed now
Surfaced while reviewing #9172. That PR hardens the lobby expiry sweep so a replacement registration under a recycled code is not deleted and delisted by a stale expiry observation. Tracing reachability for that fix showed the only way a replacement takes an expired code in Full mode is a
generate_game_codecollision — and that under the very same precondition, the session registry already loses a live game outright, which is strictly worse than a mis-delisted lobby row.#9172 deliberately did not harden this; it is a different seam, and hardening a symptom while the generator stays unguarded is the wrong order.
Scope
create_game/create_game_n_playersshould not insert under a code the registry already holds. Regenerate, or fail loudly.LobbyManager::register_gamelikewise overwrites), the token index, and the persistence tables.session.rs:60-68's comment should be narrowed to the claim it actually supports (interaction-id namespacing), so it stops reading as a blanket justification for the registry key.Residual from #9172 that this would close
handle_expired_lobby_gamesretires a session by code before the broker is consulted, so under a collision it can destroy a replacement's session while #9172's fix keeps that session's lobby listing — a listing advertising a dead game. Gating that on identity would need identity threaded throughSessionManager; guarding the generator removes the precondition instead.Probability note
36^6 ≈ 2.18e9, so a collision is rare per creation but is a birthday problem against the set of live codes, and the failure mode is silent rather than degraded. The cost of a guard is one registry lookup on game creation.