#6501 Prune connections map on game teardown

[boskodev790]

Summary

Fixes #6501: the in-memory connections map (crates/phase-server/src/main.rs) was inserted into on every host / join / reconnect but a game_code entry was never removed, unlike the parallel game_spectators map which is pruned at the same teardown points. This leaks one connections entry per finished game.

Root cause

connections: HashMap<String /*game_code*/, HashMap<PlayerId, Sender>> grows without bound: every completed or expired game leaves a permanent game_code -> {seat -> dead Sender} entry. MAX_GAMES bounds only concurrent live sessions, not this map, so a long-running server accumulates one dead entry per game played.

Fix

Mirror the existing game_spectators cleanup — connections.remove(game_code) in both background-reaper teardown loops:

• disconnect grace-expiry loop (already holds the connections lock; changed let conns → let mut conns and remove after the GameOver fan-out),
• lobby-expiry loop (added a connections prune block).

Lock order is preserved throughout (state -> connections -> spectators). No behavior change beyond releasing the dead entry.

Files changed

• crates/phase-server/src/main.rs — +14 / -1.

Verification

• cargo fmt --all — clean.
• cargo check -p phase-server — clean (compiles, borrow-checks). The change sits inside async websocket teardown paths that are not unit-testable in isolation; correctness follows from mirroring the adjacent, already-established game_spectators cleanup and preserving lock order.

Summary by CodeRabbit

• Bug Fixes
  • Improved background cleanup for completed and expired games.
  • Prevented stale connection data from accumulating after games are removed.
  • Enhanced resource management for both full-mode and lobby games.

[superagent-security]

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 899ed4cf-b34a-42b3-8187-b15ee73da629

📥 Commits

Reviewing files that changed from the base of the PR and between c754a08 and fc68538.

📒 Files selected for processing (1)

• crates/phase-server/src/main.rs

---

📝 Walkthrough

Walkthrough

Background maintenance now removes bg_connections entries when reconnect-expired games and broker-expired lobby games are cleaned up.

Changes

Connection Map Cleanup

Layer / File(s)	Summary
Reaper connection pruning crates/phase-server/src/main.rs	Full-mode and lobby expiry cleanup now removes corresponding entries from bg_connections; lobby cleanup uses an explicit lock-ordering scope.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: matthewevans

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: pruning the connections map on game teardown.
Linked Issues check	✅ Passed	The PR removes stale connections entries in both teardown paths and preserves the required lock order for #6501.
Out of Scope Changes check	✅ Passed	The changes are limited to the requested connections-map cleanup in the two teardown paths.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[matthewevans]

Verdict: blocked — the teardown cleanup has no discriminating regression test.

🔴 Blocker

crates/phase-server/src/main.rs:1106 and :1147 remove expired game codes from the connections map, but this PR adds no test that drives either expiry path and proves the entry is removed while another game’s connection remains. The author’s cited compile check cannot fail when either conns.remove(game_code) is reverted.

Suggested fix: add a focused server/lifecycle test that seeds two connection-map entries, exercises both relevant teardown routes, and asserts only the expired game’s entry is pruned.

✅ Clean

The cleanup follows the existing state → connections → spectators lock order and leaves the send-before-removal behavior intact.

Recommendation: add the discriminating teardown regression test, then return for approval.