phase: 05_anonymous_lobbies_and_room_lifecycle

All green. Final report:

**Phase 05 final verification pass — complete, 100% green.**

**Verified (all 5 tasks already in `complete/`):**
- `RoomSupervisor` import, `POST /api/rooms` → 5-char Crockford code (`VBE8Y`), `GET /api/rooms/{code}` → seat list, join/session/404 flows — all live over HTTP
- `docs/ROOMS.md` present with FSM diagram, crash-isolation and "What is lost on restart" sections
- Phase tests all pass: lifecycle 4-seat `WON`→`LOBBY` (7 tests), crash isolation, GC/idle via `FakeClock` (no sleeps), FSM, tick cadence, supervisor, sessions

**Results:**
- `uv run pytest` → **1506 passed, 0 failed**; coverage **98.89%** (>90% floor; `room.py` 99%, `rooms.py` 97%, `sessions.py` 99% — all ≥95% per-module gate)
- `uv run ruff check .` → clean; `uv run pyright` → 0 errors; `uv run undercooked-levels validate` → content OK; server boots, `/healthz` 200, client at `/` 200
- e2e US-01 suite re-run after fixes: 10 passed

**Documentation — inconsistencies found and fixed:**
- `README.md`: status now lists phases 01–05 complete, next pending `06_realtime_websocket_protocol`; dropped "(phase 05, in progress)"; 3 stale "Not implemented until phase 05." references corrected
- `app/static/js/app.js` + `tests/e2e/test_us01_landing_surface.py`: placeholder string updated in lockstep to "Not implemented until phase 07." (phase 07 task 02 wires the buttons), comments updated to reflect phase-05 completion
- `AGENTS.md`: reviewed in full — §6 already documents all phase-05 modules accurately; no changes needed

**Decisions:** no code behavior changes beyond the user-visible placeholder string; no live-infrastructure touch (only a local loopback server, stopped).

**Next pending phase:** `06_realtime_websocket_protocol`.
This commit is contained in:
2026-10-08 05:17:21 -04:00
parent 79b7f1e4e0
commit f51ba919af
35 changed files with 8391 additions and 32 deletions
+57
View File
@@ -0,0 +1,57 @@
"""Unit: `Clock`, `SystemClock` and `FakeClock` — the injectable time contract.
`FakeClock` is the test clock every time-dependent server object is driven
with (phase 05: "no test sleeps — all time via `FakeClock`"); these tests
lock its behaviour so that contract is honest.
"""
from __future__ import annotations
import pytest
from undercooked.server.clock import Clock, FakeClock, SystemClock
pytestmark = pytest.mark.unit
def test_system_clock_reads_a_monotonic_clock() -> None:
clock = SystemClock()
first = clock.now()
second = clock.now()
assert second >= first
def test_system_clock_accepts_a_substitute_time_function() -> None:
values = iter([1.0, 2.5, 7.25])
clock = SystemClock(lambda: next(values))
assert clock.now() == 1.0
assert clock.now() == 2.5
assert clock.now() == 7.25
def test_fake_clock_starts_at_the_given_instant() -> None:
assert FakeClock().now() == 0.0
assert FakeClock(start=12.5).now() == 12.5
def test_fake_clock_advances_and_reports_the_new_instant() -> None:
clock = FakeClock(start=10.0)
assert clock.advance(5.0) == 15.0
assert clock.now() == 15.0
assert clock.advance(0.5) == 15.5
assert clock.advance(0) == 15.5
def test_fake_clock_never_goes_backwards() -> None:
clock = FakeClock()
with pytest.raises(ValueError, match="cannot go backwards"):
clock.advance(-1.0)
assert clock.now() == 0.0
def test_both_clocks_satisfy_the_protocol() -> None:
"""The `Clock` protocol is a structural contract: both implementations
must be usable anywhere a `Clock` is demanded."""
clocks: list[Clock] = [SystemClock(), FakeClock()]
for clock in clocks:
assert isinstance(clock.now(), float)
+194
View File
@@ -0,0 +1,194 @@
"""Unit: the Crockford code alphabet, minting and input normalisation (A15).
`random.Random` seeds are deliberate: the minting contract is *determinism
given a seed*, so the suite re-plays the same draws. The production path
(`rng=None`) is exercised for its shape, not its bytes.
"""
from __future__ import annotations
import random
import pytest
from undercooked.server.codes import (
CODE_LEN,
CROCKFORD,
MAX_MINT_RETRIES,
CodeSpaceFull,
new_code,
normalize_code,
)
pytestmark = pytest.mark.unit
EXPECTED_ALPHABET = "0123456789ABCDEFGHJKMNPQRSTVWXYZ"
# ------------------------------------------------------------------------ alphabet
def test_the_alphabet_is_crockford_base32_excluding_ilo_u() -> None:
assert len(CROCKFORD) == 32
assert set(CROCKFORD) == set(EXPECTED_ALPHABET)
assert len(set(CROCKFORD)) == 32 # no duplicate symbols
for excluded in "ILOU":
assert excluded not in CROCKFORD, f"{excluded} must be excluded"
for digit in "0123456789":
assert digit in CROCKFORD
def test_codes_are_five_characters_long() -> None:
assert CODE_LEN == 5
# ------------------------------------------------------------------------ minting
def test_twenty_thousand_minted_codes_are_unique_and_in_the_alphabet() -> None:
taken: set[str] = set()
rng = random.Random(0x5EED_0401) # noqa: S311 - seeded, deliberate test draws
for _ in range(20_000):
code = new_code(taken, rng)
taken.add(code)
assert len(taken) == 20_000
for code in taken:
assert len(code) == CODE_LEN
assert set(code) <= set(CROCKFORD)
sample = min(taken)
assert "I" not in sample and "L" not in sample and "O" not in sample and "U" not in sample
def test_new_code_retries_a_collision_until_the_draw_is_free() -> None:
rng = random.Random(7) # noqa: S311 - seeded, deliberate test draws
first = new_code(set(), rng)
second = new_code({first}, rng)
assert second != first
assert second not in {first}
assert len(second) == CODE_LEN
def test_a_deterministic_seed_mints_a_deterministic_first_code() -> None:
"""The same seed must mint the same first code — the A4/A5 determinism
contract, applied to the minting draws."""
a = new_code(set(), random.Random(42)) # noqa: S311 - seeded, deliberate
b = new_code(set(), random.Random(42)) # noqa: S311 - seeded, deliberate
assert a == b
assert len(a) == CODE_LEN
assert set(a) <= set(CROCKFORD)
class _AlwaysTaken:
def __contains__(self, _code: str) -> bool:
return True
class _CountingRandom:
"""A `random.Random` lookalike that counts draws and always collides."""
def __init__(self) -> None:
self.choice_calls = 0
def choice(self, _sequence: str) -> str:
self.choice_calls += 1
return "A"
def test_a_full_space_raises_code_space_full_after_exactly_100_draws() -> None:
rng = _CountingRandom()
with pytest.raises(CodeSpaceFull):
new_code(_AlwaysTaken(), rng) # type: ignore[arg-type] - duck-typed lookalike
assert rng.choice_calls == MAX_MINT_RETRIES * CODE_LEN
def test_code_space_full_names_the_retries_in_its_message() -> None:
with pytest.raises(CodeSpaceFull, match="100 draws all collided"):
new_code(_AlwaysTaken())
def test_production_minting_without_an_rng_yields_a_legal_code() -> None:
code = new_code(set()) # secrets.choice path
assert len(code) == CODE_LEN
assert set(code) <= set(CROCKFORD)
# ------------------------------------------------------------------------ normalisation
@pytest.mark.parametrize(
("raw", "expected"),
[
("ABCDE", "ABCDE"),
("abcde", "ABCDE"),
("aBcDe", "ABCDE"),
("a b c d e", "ABCDE"),
("A B-C D-E", "ABCDE"),
(" AB\tCD E ", "ABCDE"),
("12345", "12345"),
("99999", "99999"),
("VWXYZ", "VWXYZ"),
],
)
def test_normalize_accepts_legal_codes_with_case_and_separators(
raw: str, expected: str
) -> None:
assert normalize_code(raw) == expected
@pytest.mark.parametrize(
("raw", "expected"),
[
("i2345", "12345"),
("I2345", "12345"),
("l2345", "12345"),
("L2345", "12345"),
("o2345", "02345"),
("O2345", "02345"),
("u2345", "V2345"),
("U2345", "V2345"),
],
)
def test_normalize_maps_every_excluded_confusable_to_its_intended_symbol(
raw: str, expected: str
) -> None:
assert normalize_code(raw) == expected
@pytest.mark.parametrize(
"raw",
[
"",
"A",
"AB",
"ABC", # too short
"ABCDEF",
"ABCDEFG", # too long
"AB12CD\n", # six characters after the newline is stripped
"ABC?E", # the question mark is not an alphabet character
"AB.CD", # the dot is not a separator: it stays, and it is invalid
"AB-CD", # the dash is stripped, leaving four characters
"I", # maps to `1`, which is still one character, not five
"U", # maps to `V`, still one character
"O", # maps to `0`, still one character
"L", # maps to `1`, still one character
"?????",
],
)
def test_normalize_rejects_wrong_lengths_and_invalid_characters(raw: str) -> None:
assert normalize_code(raw) is None
@pytest.mark.parametrize("raw", ["ABCDE\n", "ABCDE ", "ABC DE"])
def test_normalize_strips_whitespace_without_making_the_code_illegal(raw: str) -> None:
"""Whitespace is stripped, not fatal: `ABCDE\n` and `ABCDE ` are still
five-character codes, and `ABC DE` normalises to `ABCDE`."""
assert normalize_code(raw) == "ABCDE"
def test_normalize_is_a_pure_format_check_never_a_registry_query() -> None:
"""The US-02 criterion-4 contract: the result depends on the input
alone, so a caller cannot tell "malformed" from "no such room"."""
for _ in range(5):
assert normalize_code("i2345") == "12345"
assert normalize_code("nope!") is None
assert normalize_code("ABCDE") == normalize_code("ABCDE")
+1
View File
@@ -210,6 +210,7 @@ def test_the_docs_on_disk_are_the_shipped_documents() -> None:
"docs/DESIGN.md",
"docs/DEPLOYMENT.md",
"docs/LEVELS.md",
"docs/ROOMS.md",
"docs/SIMULATION.md",
"docs/TESTING.md",
}
+103
View File
@@ -0,0 +1,103 @@
"""Unit: the display-name boundary (phase 05, task 03; US-02 criterion 6).
`sanitize_name` is the one place a player-typed name becomes a stored name
(`server/seats.py`): strip Unicode control and format characters (zero-width
marks, RTL overrides, soft hyphen) and `<>"'`, collapse runs of whitespace,
trim, cap at 16; an empty result falls back to `Cook #{seat_index + 1}`.
Names are non-unique by design and are rendered by the client as text
(`textContent`, never HTML) — this table pins the shape of that boundary:
the output is always `len <= 16`, never contains `<`, `>` or `"`, and an
empty name names the seat.
"""
from __future__ import annotations
import pytest
from undercooked.server.rooms import sanitize_name as rooms_sanitize
from undercooked.server.seats import NAME_MAX_LEN, sanitize_name
pytestmark = pytest.mark.unit
def test_the_table_of_raw_to_stored_names() -> None:
"""The boundary, case by case: fallbacks, control/format stripping,
whitespace collapsing, the 16-char cap, and the non-ASCII keeps."""
cases: list[tuple[str | None, int, str]] = [
# The fallback: no name (or an empty one) names the seat.
(None, 0, "Cook #1"),
(None, 2, "Cook #3"),
("", 0, "Cook #1"),
(" \t\n ", 1, "Cook #2"),
# Plain names pass (whitespace runs collapse to one space).
("Ada", 0, "Ada"),
("Tom Jerry", 0, "Tom Jerry"),
(" Hello World ", 0, "Hello World"),
("Ünïcödé ☂", 0, "Ünïcödé ☂"),
# Control characters (Cc) are stripped, not collapsed: the newline
# between two letters disappears.
("A\nB\x00C\x07D", 0, "ABCD"),
("\tMi la \t", 0, "Mi la"),
# Markup (the angle/quote pair) is stripped — the `textContent`
# backstop.
("O'Brien", 0, "OBrien"),
('"quoted"', 0, "quoted"),
("<script>alert(1)</script>", 0, "scriptalert(1)/s"),
# The cap: 16 chars, trimmed after the cut.
("A" * 40, 0, "A" * 16),
("x" * 1000, 0, "x" * 16),
("a" * 15 + " b", 0, "a" * 15),
("a b " * 20, 0, "a b a b a b a b"),
("a" * 15 + "b", 0, "a" * 15 + "b"),
# Unicode format characters (Cf) are stripped: zero-width, RTL
# override, soft hyphen — a zero-width-only name is a fallback.
("\u202bHELLO", 0, "HELLO"),
("\u200b\u200b\u200b", 0, "Cook #1"),
("\u200bA\u200bB", 0, "AB"),
("Mi\u00adna", 0, "Mina"),
# Emoji and accents are kept (So / Ll), and the cap counts them.
("🍳🍅🧅🐟", 0, "🍳🍅🧅🐟"),
("🍳" * 20, 0, "🍳" * 16),
("Café ☕ ", 0, "Café ☕"),
]
for raw, seat_index, expected in cases:
assert sanitize_name(raw, seat_index) == expected, (raw, seat_index)
assert NAME_MAX_LEN == 16
def test_the_output_is_bounded_and_markup_free() -> None:
"""The shape contract, property-style, over the nasty inputs: the
output is always `len <= 16` and never carries `<`, `>` or `"` —
whatever the input tried to do."""
nasty = [
"<script>alert(1)</script>",
"<<<>>>>>>>",
'"',
"A" * 1000,
(" " * 500) + "x" * 500,
"\u200b" * 100,
"\u202b\u202c\u202d\u202e\u2066\u2067" + "Name" * 100,
"Mi\u00adna\u00ad\u00ad" * 10,
"<'\"\\>\x00\x01\x1f\x7f",
"é" * 500,
]
for raw in nasty:
out = sanitize_name(raw, 3)
assert len(out) <= 16, (raw[:40], out)
assert not {"<", ">", '"'} & set(out), (raw[:40], out)
def test_the_fallback_names_the_seat() -> None:
"""An empty (or control-only) name is `Cook #{seat_index + 1}` — the
client can always address the cook."""
assert sanitize_name(None, 0) == "Cook #1"
assert sanitize_name(None, 7) == "Cook #8"
assert sanitize_name(" \x00\t ", 4) == "Cook #5"
assert sanitize_name("\u200b\u202b", 9) == "Cook #10"
def test_rooms_reexports_the_same_boundary() -> None:
"""`server/rooms.py` keeps its task-01 export: the supervisor's
boundary and the seats' boundary are the one function (imported, not
duplicated)."""
assert rooms_sanitize is sanitize_name
@@ -0,0 +1,60 @@
"""Unit (grep-style): no wall-clock calls anywhere in `server/`.
The phase-05 contract (task 01, work item 1): every time-dependent server
object receives a `Clock`; the *only* file in `src/undercooked/server/`
allowed to import `time` is `clock.py`, and even there the clock function
is held as a reference — `time.monotonic()` / `time.time()` are never
called in server code. This test greps for the literals (and for stray
`time` imports) so a future edit cannot reintroduce a bare wall-clock call
without failing the suite.
"""
from __future__ import annotations
import re
from pathlib import Path
import pytest
pytestmark = pytest.mark.unit
REPO_ROOT = Path(__file__).resolve().parents[2]
SERVER_DIR = REPO_ROOT / "src" / "undercooked" / "server"
#: The wall-clock calls the contract forbids in server code.
FORBIDDEN_CALLS = ("time.monotonic()", "time.time()")
#: `time` may be imported by exactly one module.
TIME_IMPORT = re.compile(r"^\s*(?:import time\b|from time import\b)", re.MULTILINE)
def test_no_wall_clock_call_lives_in_server_code() -> None:
offenders = [
f"{path.name}: {literal}"
for path in sorted(SERVER_DIR.rglob("*.py"))
for literal in FORBIDDEN_CALLS
if literal in path.read_text(encoding="utf-8")
]
assert offenders == [], "wall-clock calls leaked into server/:\n " + "\n ".join(
offenders
)
def test_only_the_clock_module_imports_time() -> None:
offenders = [
path.name
for path in sorted(SERVER_DIR.rglob("*.py"))
if path.name != "clock.py" and TIME_IMPORT.search(path.read_text(encoding="utf-8"))
]
assert offenders == [], (
f"modules that import `time` outside clock.py: {', '.join(offenders)}"
)
def test_the_guard_is_not_vacuous() -> None:
"""A guard that never fires is no guard: prove both checks detect a
planted violation in this very file's patterns."""
assert FORBIDDEN_CALLS[0] in "x = time.monotonic()\n"
assert TIME_IMPORT.search("import time\n") is not None
assert SERVER_DIR.is_dir()
assert (SERVER_DIR / "clock.py").is_file()
+45
View File
@@ -0,0 +1,45 @@
"""Unit: session token minting and `SessionRecord` (A16).
The token is the player's only identity: 24 random bytes, urlsafe
base64 — the same entropy the browser side generates with
`crypto.getRandomValues`. These tests lock the shape that
`SessionStore` (task 04) builds on.
"""
from __future__ import annotations
import string
import pytest
from undercooked.server.clock import FakeClock
from undercooked.server.sessions import mint_token, new_session
pytestmark = pytest.mark.unit
_URLSAFE = set(string.ascii_letters + string.digits + "-_")
def test_mint_token_is_24_random_bytes_of_urlsafe_base64() -> None:
token = mint_token()
assert len(token) == 32 # 24 bytes -> 32 base64url characters
assert set(token) <= _URLSAFE
assert "+" not in token and "/" not in token and "=" not in token
def test_minted_tokens_are_distinct() -> None:
tokens = {mint_token() for _ in range(100)}
assert len(tokens) == 100
def test_new_session_mints_a_token_and_stamps_the_injected_clock() -> None:
clock = FakeClock(start=42.0)
session = new_session("Ada", clock)
assert len(session.token) == 32
assert session.display_name == "Ada"
assert session.created_at == 42.0
anonymous = new_session(None, clock)
assert anonymous.display_name is None
assert anonymous.token != session.token
assert anonymous.created_at == 42.0
+144
View File
@@ -0,0 +1,144 @@
"""Unit: the session's privacy surface (phase 05, task 04, US-07).
The store is anonymous by design (A16): the identity is a 24-byte
random token and *nothing else*. These tests lock the surface so a PII
field can never join without a test failing first:
* **the `SessionRecord` field set** — exactly the task's nine fields
(identity, name, progression, counters, stamps),
* **a `repr()` scan** — a representative record (and a representative
store) never carries `ip`, `user_agent`, `authorization` or `cookie`
(case-insensitive),
* **`SessionStore` has no attribute holding raw headers** — neither the
record's fields nor the store's attributes name a `header` of any
kind, and no dict-valued store attribute holds a header-like key.
"""
from __future__ import annotations
import dataclasses
import pytest
from undercooked.server.clock import FakeClock
from undercooked.server.sessions import BestRun, SessionRecord, SessionStore
pytestmark = pytest.mark.unit
#: The only fields a session record may hold (task 04's list).
ALLOWED_RECORD_FIELDS = frozenset(
{
"token",
"display_name",
"created_at",
"unlocked_level",
"best",
"rounds_played",
"dishes_served",
"last_seen",
"endless_best",
}
)
#: The strings a session's repr must never contain (the PII markers).
FORBIDDEN_REPR_SUBSTRINGS = ("ip", "user_agent", "authorization", "cookie")
#: A request-header key that must never appear in the store's own data.
HEADER_LIKE_KEYS = frozenset(
{"host", "user-agent", "authorization", "cookie", "x-forwarded-for"}
)
def _benign_record() -> SessionRecord:
"""A representative record with fixed, benign values (a random
base64url token could incidentally contain a marker; the test needs
a *guaranteed* shape, so the token is fixed too)."""
return SessionRecord(
token="t" * 32,
display_name="Ada",
created_at=1.0,
unlocked_level=2,
best={"01_kitchen": BestRun(score=100, stars=3, completed_at=2.0)},
rounds_played=4,
dishes_served=9,
last_seen=3.0,
endless_best=None,
)
def _benign_store() -> SessionStore:
"""A representative store: a `FakeClock`, one campaign entry, and one
record inside (so a walk of its data covers the record's shape)."""
store = SessionStore(
max_entries=10, clock=FakeClock(start=1.0), campaign={"01_kitchen": 1}
)
record = _benign_record()
store.put(record)
return store
def test_the_record_field_set_is_exactly_the_task_list() -> None:
fields = {field.name for field in dataclasses.fields(SessionRecord)}
assert fields == ALLOWED_RECORD_FIELDS
def test_the_record_repr_carrys_no_pii_markers() -> None:
text = repr(_benign_record()).lower()
for marker in FORBIDDEN_REPR_SUBSTRINGS:
assert marker not in text, f"the record's repr carries {marker!r}"
def test_the_store_repr_carrys_no_pii_markers() -> None:
text = repr(_benign_store()).lower()
for marker in FORBIDDEN_REPR_SUBSTRINGS:
assert marker not in text, f"the store's repr carries {marker!r}"
def test_the_record_and_the_best_run_are_slots_dataclasses() -> None:
"""Slots (not a `__dict__` bag) is the shape that keeps the field
set auditable: a new field is a named, visible change."""
assert hasattr(SessionRecord, "__slots__")
assert hasattr(BestRun, "__slots__")
def test_neither_record_nor_store_names_a_header_attribute() -> None:
record_fields = {field.name for field in dataclasses.fields(SessionRecord)}
assert not any("header" in name.lower() for name in record_fields)
store = _benign_store()
assert not any("header" in name.lower() for name in dir(store))
assert not any("header" in name.lower() for name in vars(store))
def test_the_store_holds_no_request_header_mapping() -> None:
"""Every dict-valued attribute of the store is either `token ->
record` or `level id -> order`: neither may hold a header-like key."""
store = _benign_store()
for name, value in vars(store).items():
if isinstance(value, dict):
assert not (
set(value) & HEADER_LIKE_KEYS
), f"store attribute {name!r} holds header-like keys"
record = _benign_record()
for field in dataclasses.fields(record):
assert field.name.lower() not in HEADER_LIKE_KEYS, f"record field {field.name!r}"
value = getattr(record, field.name)
if isinstance(value, dict):
assert not (
set(value) & HEADER_LIKE_KEYS
), f"record field {field.name!r} holds header-like keys"
def test_the_store_never_minted_with_headers_keeps_none() -> None:
"""A store built the way the supervisor builds it (a clock and the
campaign mapping only) exposes exactly its own five attributes — no
header bag can hide in a constructor that does not take one."""
store = SessionStore(
clock=FakeClock(), campaign={"01_kitchen": 1}
)
assert set(vars(store)) == {
"_max_entries",
"_clock",
"_campaign",
"_records",
"_evictions",
}