From fd9d9c7433f4efb58208d9117c63ceeaf6086a79 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 05:43:51 +0000 Subject: [PATCH 1/9] Fix bugs surfaced by full-codebase review Backend, highest-impact first: - fanout: deliver broadcasts concurrently with a per-send timeout so one backpressured client can't wedge the single subscriber task and stall every game on the instance (head-of-line blocking). - telemetry: run each rollup handler in its own savepoint so one SQL error no longer aborts the whole batch (raw events included); drain the queue on shutdown instead of dropping it; weight avg_dt by dt-sample count; a tie no longer steals the live leader. - ws: move connection setup inside the try/finally so a failed welcome send can't leak the per-IP counter / session / pinger; drop a voluntary leaver immediately even while paused (no roster ghost); cancel pending grace-drops on game teardown. - config/security: refuse to boot on the default JWT_SECRET in a prod build; stop gating HSTS on the CSP master switch. - places: validate place_id at resolve() before it reaches a Google URL path or Redis key. - gamestore: make rate_allow's INCR+EXPIRE atomic (no TTL-less lockout on a crash); keep per-roll drand audit fields out of the broadcast snapshot. - auth: return 503 (not a bare 500) when Postgres is absent. - routes/widget: constant-time bearer + widget-key compares; order the widget matchup names deterministically. - drand: fail closed when BLS verification raises. - discord: cap the 429 retry-after backoff. Frontend: - sheet: only treat a click on the dialog itself as a backdrop tap, so the touch guard's synthesized (0,0) click can't slam the sheet shut mid-use. - overlays/router: make the resume pause-overlay close cancellable so a quick re-pause doesn't strand a non-host on a live board. - net/game-render: close the menu and stop the pause tick on a fatal teardown; re-sync the paused roll button after an in-game error. - profile: assign the recent-games click handler instead of stacking one per render. - game-detail: guard verification with a generation token so overlapping runs don't write into a newer view. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_012xoZvcd19p4WWYnPkLZkfn --- .dockerignore | 4 ++ ops/prometheus.prod.yml | 12 +++- scripts/reattribute_user.sql | 7 ++- server/assets.py | 12 ++-- server/auth.py | 12 ++++ server/broadcast.py | 34 ++++++++-- server/config.py | 18 +++++- server/discord.py | 14 ++++- server/drand.py | 10 ++- server/fanout.py | 31 +++++++--- server/gamestore.py | 34 +++++++--- server/places.py | 9 +++ server/routes.py | 7 ++- server/security.py | 7 ++- server/telemetry/writer.py | 72 +++++++++++++++++----- server/widget.py | 6 +- server/ws.py | 38 +++++++----- static/js/components/game-detail-screen.js | 13 ++++ static/js/components/profile-screen.js | 7 ++- static/js/game-render.js | 6 +- static/js/net.js | 14 ++++- static/js/overlays.js | 24 ++++++++ static/js/router.js | 4 +- static/js/sheet.js | 7 +++ 24 files changed, 328 insertions(+), 74 deletions(-) diff --git a/.dockerignore b/.dockerignore index 0d639359..38152156 100644 --- a/.dockerignore +++ b/.dockerignore @@ -9,6 +9,10 @@ __pycache__/ venv/ .playwright-mcp/ .DS_Store +# Deploy-time secrets (e.g. ops/secrets/metrics_token) are mounted into the +# prometheus/web containers at runtime — never bake them into an image layer, +# where `docker history`/layer extraction would recover them. +ops/secrets/ # Frontend build: node_modules is reinstalled (npm ci) and dist is rebuilt # inside the `assets` stage; never ship the host copies into the build context. node_modules/ diff --git a/ops/prometheus.prod.yml b/ops/prometheus.prod.yml index 7ac479a9..2124395e 100644 --- a/ops/prometheus.prod.yml +++ b/ops/prometheus.prod.yml @@ -4,8 +4,16 @@ global: scrape_configs: - job_name: tensies - static_configs: - - targets: ['web:8000'] + # DNS service discovery, not a single static target: under `--scale web=N` + # the `web` service name resolves to N container IPs. A static_configs target + # opens one connection per scrape and would hit a single rotating replica, + # so process-local counters (rolls, WS frames) appear to jump and 2/3 of + # replicas stay invisible. dns_sd_configs enumerates every A record and + # scrapes each replica as its own target. + dns_sd_configs: + - names: ['web'] + type: A + port: 8000 metrics_path: /metrics # /metrics is bearer-gated in prod (audit M2). Prometheus reads the same # token from a mounted file (write it with: diff --git a/scripts/reattribute_user.sql b/scripts/reattribute_user.sql index e062ce14..ab52b999 100644 --- a/scripts/reattribute_user.sql +++ b/scripts/reattribute_user.sql @@ -28,10 +28,15 @@ SELECT id::text AS user_id, username FROM users WHERE id::text = trim(both '''' from :'target_id'); +-- Stash the requested id in a real GUC so the abort message can echo it back. +-- `:'target_id'` is a psql client variable, not a server setting, so +-- current_setting('target_id') returned NULL and printed a blank id. +SELECT set_config('tensies.target_id', trim(both '''' from :'target_id'), false); + DO $$ BEGIN IF NOT EXISTS (SELECT 1 FROM _target) THEN - RAISE EXCEPTION 'No user found with id %', current_setting('target_id', true); + RAISE EXCEPTION 'No user found with id %', current_setting('tensies.target_id', true); END IF; END $$; diff --git a/server/assets.py b/server/assets.py index f9175aa4..2cbd9c33 100644 --- a/server/assets.py +++ b/server/assets.py @@ -129,16 +129,20 @@ class DevAssets: def __init__(self, app_url: str = "") -> None: self._app_url = app_url - self._sig: tuple[int, int] | None = None + self._sig: tuple[int, int, int] | None = None self._tmpl: Template | None = None self._defaults: dict[str, str] = {} self._js: dict[str, str] = {} - def _signature(self) -> tuple[int, int]: + def _signature(self) -> tuple[int, int, int]: css, js, legacy = _collect_assets() paths = [*css, *js, *legacy, STATIC_DIR / "index.html"] - newest = max((p.stat().st_mtime_ns for p in paths if p.exists()), default=0) - return newest, len(paths) + mtimes = [p.stat().st_mtime_ns for p in paths if p.exists()] + # (newest, count, sum-of-mtimes): a `git checkout` that swaps one module + # for another keeps the count and can restore an OLDER mtime than the + # current newest — max+count alone would miss it and keep serving the + # stale module graph. The mtime sum moves whenever any file's mtime does. + return (max(mtimes, default=0), len(mtimes), sum(mtimes)) def _refresh_if_stale(self) -> None: sig = self._signature() diff --git a/server/auth.py b/server/auth.py index 6750951e..1771797b 100644 --- a/server/auth.py +++ b/server/auth.py @@ -43,6 +43,14 @@ router = APIRouter(prefix="/auth", tags=["auth"]) +def _require_db() -> None: + """Return a clean 503 when Postgres is absent (the no-DB dev wrapper, or a + failed db.init()). Without this, db.pool() asserts and the endpoint 500s with + a bare AssertionError instead of a meaningful 'accounts unavailable'.""" + if not db.available(): + raise HTTPException(503, "Accounts are temporarily unavailable") + + async def _rate_limit(request: Request) -> None: """Per-IP limiter for the auth endpoints. They are unauthenticated and do Redis + Postgres work per call, and registration/login options responses @@ -146,6 +154,7 @@ class LoginVerifyRequest(BaseModel): @router.post("/register/options") async def register_options(body: RegisterOptionsRequest, request: Request): + _require_db() await _rate_limit(request) username = _validate_username(body.username) @@ -208,6 +217,7 @@ async def register_options(body: RegisterOptionsRequest, request: Request): @router.post("/register/verify") async def register_verify(body: RegisterVerifyRequest, request: Request): + _require_db() await _rate_limit(request) username = _validate_username(body.username) challenge = await _pop_challenge(body.nonce) @@ -327,6 +337,7 @@ async def _insert_user(lp: str | None) -> None: @router.post("/login/options") async def login_options(body: LoginOptionsRequest, request: Request): + _require_db() await _rate_limit(request) username = _validate_username(body.username) @@ -380,6 +391,7 @@ async def login_options(body: LoginOptionsRequest, request: Request): @router.post("/login/verify") async def login_verify(body: LoginVerifyRequest, request: Request): + _require_db() await _rate_limit(request) username = body.username.strip() challenge = await _pop_challenge(body.nonce) diff --git a/server/broadcast.py b/server/broadcast.py index 4f4d41a3..1ddc18dc 100644 --- a/server/broadcast.py +++ b/server/broadcast.py @@ -74,6 +74,13 @@ async def delayed_broadcast(code: str, pid: str, is_win: bool, winner_name: str return ev = asyncio.Event() + # If a previous roll's broadcast is still waiting on this pid's ack, releasing + # it now (rather than orphaning it) lets it fan out the latest snapshot at + # once instead of stalling the full ROLL_ACK_TIMEOUT — its own `finally` + # identity-check keeps it from popping our new event. + prev = state.ack_events.get(pid) + if prev is not None: + prev.set() state.ack_events[pid] = ev try: await asyncio.wait_for(ev.wait(), timeout=ROLL_ACK_TIMEOUT) @@ -146,6 +153,7 @@ async def end_if_paused_over(code: str) -> None: "msg": "Game ended — it was paused too long."}) state.connections.pop(code, None) state.pause_tasks.pop(code, None) + cancel_drop_tasks(code) async def _emit_checkout(code: str, pid: str | None, info: dict, *, @@ -177,6 +185,16 @@ async def checkout_game(code: str, pid: str | None, *, reason: str, await _emit_checkout(code, pid, info, reason=reason, session_id=session_id) +def cancel_drop_tasks(code: str) -> None: + """Cancel any still-pending grace-drop tasks for a game that's being torn + down. They'd otherwise sleep out their grace, then no-op against the deleted + game (self-healing, but they hold a reference to the dead code until then).""" + for key in [k for k in state.drop_tasks if k[0] == code]: + t = state.drop_tasks.pop(key, None) + if t: + t.cancel() + + async def drop_player(code: str, pid: str) -> None: """Remove a disconnected player after the grace period (local-task path).""" await asyncio.sleep(DISCONNECT_GRACE) @@ -200,11 +218,16 @@ async def do_drop( if player is None or not player.get("disconnected"): return - if snap.get("paused"): - # While paused, nobody is dropped — the host may have stepped away. But - # a paused game must not be held hostage by an absent host: if the host - # is the one who's gone, hand the host role (and the resume control) to - # a still-connected player so the game stays recoverable. + if snap.get("paused") and reason != "leave": + # While paused, nobody is dropped *on disconnect* — the host may have + # stepped away and will reconnect. A voluntary leave (reason="leave") is + # different: the player deliberately quit and their client is gone, so it + # falls through to the real drop below and the roster updates for everyone + # immediately (otherwise the leaver lingers as a ghost until resume or the + # 1-hour pause cap). But a paused game must not be held hostage by an + # absent host: if the host is the one who's gone, hand the host role (and + # the resume control) to a still-connected player so the game stays + # recoverable. if snap["host"] == pid: new_host = next((q for q, pl in snap["players"].items() if q != pid and not pl.get("disconnected")), None) @@ -245,6 +268,7 @@ async def do_drop( if checkin: await _emit_checkout(code, pid, checkin, reason="game_ended") state.connections.pop(code, None) + cancel_drop_tasks(code) return if res["new_host"]: diff --git a/server/config.py b/server/config.py index 54553092..ac7d101b 100644 --- a/server/config.py +++ b/server/config.py @@ -55,6 +55,10 @@ def _list(name: str) -> list[str]: # client path enforces a matching 700ms pacing floor (animations.js). MIN_ROLL_INTERVAL = _float("MIN_ROLL_INTERVAL", 0.75) ROLL_ACK_TIMEOUT = 2.0 # wait for the roller's reveal ack before broadcasting +# Upper bound on a single WS frame delivery during fan-out. A backpressured +# client (a phone that stopped reading its socket) must not wedge the shared +# fanout subscriber task; past this it is treated as dead and its socket dropped. +BROADCAST_SEND_TIMEOUT = _float("BROADCAST_SEND_TIMEOUT", 5.0) DISCONNECT_GRACE = 60.0 # seconds a dropped player's slot is held for reconnect ROUND_WIN_DELAY = 3.0 # winner overlay hold before advancing the round @@ -275,7 +279,19 @@ def _list(name: str) -> list[str]: for o in os.environ.get("WEBAUTHN_ORIGIN", APP_URL or "http://localhost:8888").split(",") if o.strip() ] -JWT_SECRET = os.environ.get("JWT_SECRET", "dev-secret-change-in-prod") +_JWT_SECRET_DEFAULT = "dev-secret-change-in-prod" +JWT_SECRET = os.environ.get("JWT_SECRET", _JWT_SECRET_DEFAULT) +# Fail closed: the JWT signing secret gates account auth (a forged HS256 token +# rebinds a WebSocket to any account UUID). In prod — signalled by FRONTEND_DIST, +# which only the built image sets — refuse to boot on the well-known default so a +# forgotten JWT_SECRET can't ship a deploy where anyone can impersonate any +# player. Dev (no FRONTEND_DIST) keeps the convenient default. +if FRONTEND_DIST and JWT_SECRET == _JWT_SECRET_DEFAULT: + raise RuntimeError( + "JWT_SECRET is unset (using the insecure default) in a production build " + "(FRONTEND_DIST is set). Set JWT_SECRET to a strong random value — e.g. " + "`openssl rand -hex 32` — before deploying." + ) JWT_EXPIRY_DAYS = _int("JWT_EXPIRY_DAYS", 30) # ─── Founding member ("Founding Roller") ────────────────────────────── diff --git a/server/discord.py b/server/discord.py index 288b09ef..e65b9ebb 100644 --- a/server/discord.py +++ b/server/discord.py @@ -387,14 +387,22 @@ async def _request(method: str, path: str, body: dict | list) -> httpx.Response return None +# Cap the 429 backoff so a hostile or buggy `retry_after` (e.g. a huge value in +# the body) can't park the notifier task asleep for minutes/hours. +_MAX_RETRY_AFTER = 30.0 + + def _retry_after(r: httpx.Response) -> float: try: - return float(r.json().get("retry_after", 1.0)) + val = float(r.json().get("retry_after", 1.0)) except Exception: try: - return float(r.headers.get("Retry-After", "1")) + val = float(r.headers.get("Retry-After", "1")) except (TypeError, ValueError): - return 1.0 + val = 1.0 + if val != val or val < 0: # NaN or negative + val = 1.0 + return min(val, _MAX_RETRY_AFTER) # ─── Slash commands ───────────────────────────────────────────────────────── diff --git a/server/drand.py b/server/drand.py index 39632d50..9d27a55c 100644 --- a/server/drand.py +++ b/server/drand.py @@ -60,8 +60,14 @@ def _verify_bls(sig_bytes: bytes, round_num: int) -> bool: h = G1Element.from_message(msg, _DST) return sig.pair(G2Element.generator()) == h.pair(pk) except Exception: - log.exception("BLS verification error — skipping") - return True + # Fail CLOSED: a malformed signature / pairing error means we could not + # verify the beacon, so reject it. (It used to return True here, treating + # an unverifiable beacon as valid — a party controlling the drand HTTP + # response could satisfy the SHA-256 layer and slip a forged beacon past + # this one.) The caller then keeps the last good beacon / falls back to + # local RNG rather than trusting an unverified value. + log.exception("BLS verification error — rejecting beacon") + return False # ── Lifecycle (matches reaper/fanout start/stop pattern) ─────────────── diff --git a/server/fanout.py b/server/fanout.py index 39d9a83f..941fa534 100644 --- a/server/fanout.py +++ b/server/fanout.py @@ -11,7 +11,7 @@ import redis.asyncio as aioredis -from server.config import REDIS_URL, log +from server.config import BROADCAST_SEND_TIMEOUT, REDIS_URL, log from server.state import connections CHANNEL_PREFIX = "bcast:" @@ -80,13 +80,26 @@ async def _deliver(code: str, message: dict, exclude: str | None) -> None: local = connections.get(code) if not local: return - dead = [] - for pid, ws in list(local.items()): - if pid == exclude: - continue + targets = [(pid, ws) for pid, ws in list(local.items()) if pid != exclude] + if not targets: + return + + # Deliver to every local socket CONCURRENTLY, each bounded by + # BROADCAST_SEND_TIMEOUT. Sequential unbounded awaits here meant a single + # backpressured client (a phone that stopped reading its socket) blocked the + # one shared fanout subscriber task indefinitely — stalling broadcasts for + # EVERY game on the instance (head-of-line blocking). Concurrency + a + # per-send timeout bounds the worst case to the timeout and reaps the stuck + # socket instead of wedging the loop. Ordering is preserved: _run awaits this + # to completion before delivering the next message for any game. + async def _one(pid: str, ws) -> str | None: try: - await send(ws, message) + await asyncio.wait_for(send(ws, message), BROADCAST_SEND_TIMEOUT) except Exception: - dead.append(pid) - for pid in dead: - local.pop(pid, None) + return pid # dead, errored, or too slow — drop the local socket + return None + + results = await asyncio.gather(*(_one(pid, ws) for pid, ws in targets)) + for pid in results: + if pid is not None: + local.pop(pid, None) diff --git a/server/gamestore.py b/server/gamestore.py index 7946e0e2..3bcdfa0a 100644 --- a/server/gamestore.py +++ b/server/gamestore.py @@ -37,7 +37,7 @@ _r: aioredis.Redis | None = None # Lua scripts, registered on init(). -_create = _join = _finish = _drop = _restamp = _end_paused = None +_create = _join = _finish = _drop = _restamp = _end_paused = _rate = None def client() -> aioredis.Redis: @@ -154,14 +154,27 @@ async def close() -> None: """ +_RATE_LUA = """ +local n = redis.call('INCR', KEYS[1]) +-- Guarantee a TTL atomically so a crash between INCR and EXPIRE can't leave a +-- TTL-less key that throttles an identity forever. TTL < 0 means no expiry +-- (-1) — set it; also self-heals any pre-existing leaked key. +if redis.call('TTL', KEYS[1]) < 0 then + redis.call('EXPIRE', KEYS[1], tonumber(ARGV[1])) +end +return n +""" + + def _register_scripts() -> None: - global _create, _join, _finish, _drop, _restamp, _end_paused + global _create, _join, _finish, _drop, _restamp, _end_paused, _rate _create = _r.register_script(_CREATE_LUA) _join = _r.register_script(_JOIN_LUA) _finish = _r.register_script(_FINISH_LUA) _drop = _r.register_script(_DROP_LUA) _restamp = _r.register_script(_RESTAMP_LUA) _end_paused = _r.register_script(_END_PAUSED_LUA) + _rate = _r.register_script(_RATE_LUA) # ─── Code generation (audit L1: secrets, not random) ─────────────────────── @@ -304,6 +317,12 @@ async def snapshot(code: str) -> dict | None: if k.startswith("p:"): _, pid, field = k.split(":", 2) players.setdefault(pid, {})[field] = _coerce_player(field, v) + elif k.startswith("drand:"): + # Per-roll provable-fairness audit fields — one accumulates per roll. + # They are read directly via get_drand_round() (HGET), never through + # this snapshot, so keep them out of the rebuilt game dict: otherwise + # every broadcast state frame would carry O(total rolls) of them. + continue elif k != "order": game[k] = _coerce_game(k, v) game["players"] = players @@ -633,12 +652,13 @@ async def discovery_card(code: str) -> dict | None: # ─── Abuse limits (audit H1) — enforced in Redis so they hold across instances ─ async def rate_allow(scope: str, ident: str, limit: int, window: float) -> bool: - """Sliding-ish fixed-window limiter. True if under `limit` per `window`.""" + """Sliding-ish fixed-window limiter. True if under `limit` per `window`. + + INCR + EXPIRE run in one atomic Lua script so a crash can't split them and + strand a TTL-less key that would throttle the identity forever.""" key = f"rl:{scope}:{ident}" - n = await _r.incr(key) - if n == 1: - await _r.expire(key, int(window) or 1) - return n <= limit + n = await _rate(keys=[key], args=[int(window) or 1]) + return int(n) <= limit async def conn_incr(ip: str) -> int: diff --git a/server/places.py b/server/places.py index a89af3b7..07165324 100644 --- a/server/places.py +++ b/server/places.py @@ -206,11 +206,20 @@ async def search_text(query: str, lat: float, lon: float) -> list[dict]: return results +# A Google place_id is opaque but always `[A-Za-z0-9_-]`. Validate here, at the +# single choke point, before it is interpolated into a Google URL path or a +# Redis cache key — a client-supplied `/`, `?`, `#`, or `..` would otherwise +# reach unintended API paths/params and seed arbitrary cache keys. +_PLACE_ID_RE = re.compile(r"^[\w-]{1,256}$") + + async def resolve(place_id: str) -> dict | None: """Authoritative {place_id, name, lat, lon} for a place — from the cache, or a Google Place Details call. None if unknown. Deriving coords server-side (never trusting client-sent coords) is what stops a game being dropped at an arbitrary spot.""" + if not _PLACE_ID_RE.match(place_id): + return None cached = await _cache_get(place_id) if cached is not None: return cached diff --git a/server/routes.py b/server/routes.py index 34c0f03f..fe269692 100644 --- a/server/routes.py +++ b/server/routes.py @@ -1,4 +1,5 @@ import asyncio +import hmac import math import re from pathlib import Path @@ -78,7 +79,11 @@ def _bearer_guard(expected: str | None): async def _dep(authorization: str | None = Header(default=None)) -> None: if expected is None: return - if authorization != f"Bearer {expected}": + # Constant-time compare: a plain `!=` short-circuits on the first + # differing byte (and leaks length), letting a timing attack recover the + # token byte-by-byte. + if not (authorization + and hmac.compare_digest(authorization, f"Bearer {expected}")): raise HTTPException(status_code=401, detail="unauthorized") return _dep diff --git a/server/security.py b/server/security.py index a8298356..9b3e6d71 100644 --- a/server/security.py +++ b/server/security.py @@ -90,7 +90,12 @@ class SecurityHeadersMiddleware: def __init__(self, app) -> None: self.app = app self.csp = build_csp() if SECURITY_HEADERS else None - self.hsts = build_hsts() if SECURITY_HEADERS else None + # HSTS is governed solely by HSTS_ENABLED (build_hsts returns None when + # off). It is deliberately NOT gated by SECURITY_HEADERS: that switch is + # documented as the CSP master switch, so an operator disabling CSP (e.g. + # to debug a policy violation) must not silently lose HSTS on an HTTPS + # deploy and reopen the SSL-strip/downgrade window. + self.hsts = build_hsts() async def __call__(self, scope, receive, send) -> None: if scope["type"] != "http": diff --git a/server/telemetry/writer.py b/server/telemetry/writer.py index bad5d40a..a525f96c 100644 --- a/server/telemetry/writer.py +++ b/server/telemetry/writer.py @@ -18,25 +18,53 @@ BATCH_MAX = 500 BATCH_INTERVAL_S = 0.25 _task: asyncio.Task | None = None +_q: asyncio.Queue | None = None +_stopping = False async def start() -> None: - global _task - q = bus.subscribe(maxsize=20_000) - _task = asyncio.create_task(_run(q), name="telemetry.writer") + global _task, _q, _stopping + _stopping = False + _q = bus.subscribe(maxsize=20_000) + _task = asyncio.create_task(_run(_q), name="telemetry.writer") async def stop() -> None: + # Graceful stop: signal the loop instead of cancelling, so it finishes its + # current flush and drains what it's holding rather than dropping up to a + # full batch mid-await. Then flush anything still queued, so a shutdown or + # rolling redeploy doesn't silently lose the last batches of telemetry. + global _stopping + _stopping = True if _task is not None: - _task.cancel() try: - await _task - except (asyncio.CancelledError, Exception): - pass + await asyncio.wait_for(_task, timeout=5.0) + except (TimeoutError, asyncio.CancelledError): + _task.cancel() + except Exception: + log.exception("telemetry writer task ended with error") + if _q is not None: + try: + leftover = _drain_nowait(_q) + while leftover: + await _flush(leftover) + leftover = _drain_nowait(_q) + except Exception: + log.exception("final telemetry drain failed") + + +def _drain_nowait(q: asyncio.Queue) -> list[dict]: + batch: list[dict] = [] + while len(batch) < BATCH_MAX: + try: + batch.append(q.get_nowait()) + except asyncio.QueueEmpty: + break + return batch async def _run(q: asyncio.Queue) -> None: - while True: + while not _stopping: try: metrics.telemetry_queue_depth.labels(subscriber="writer").set(q.qsize()) batch = await _drain(q) @@ -87,11 +115,20 @@ async def _flush(batch: list[dict]) -> None: ) for ev in batch: handler = _HANDLERS.get(ev["type"]) - if handler is not None: - try: + if handler is None: + continue + # Each rollup runs in its OWN savepoint (a nested asyncpg + # transaction). Without it, a single handler's SQL error aborts the + # whole outer transaction — every *later* handler then fails with + # InFailedSQLTransactionError and the COMMIT turns into a ROLLBACK, + # silently discarding the entire batch INCLUDING the raw events + # insert above. The savepoint rolls back only the failed handler so + # the events log and every good rollup still commit. + try: + async with con.transaction(): await handler(con, ev) - except Exception: - log.exception("rollup handler failed: %s", ev["type"]) + except Exception: + log.exception("rollup handler failed: %s", ev["type"]) def _event_row(ev: dict) -> tuple: @@ -422,7 +459,12 @@ async def _h_roll(con, ev): avg_dt_between_rolls_ms = CASE WHEN $6 IS NULL THEN round_player.avg_dt_between_rolls_ms WHEN round_player.avg_dt_between_rolls_ms IS NULL THEN $6 - ELSE ((round_player.avg_dt_between_rolls_ms * round_player.rolls) + $6) / (round_player.rolls + 1) + -- Weight by the number of dt SAMPLES, not total rolls. The + -- first roll of a round has dt=NULL (no prior roll), so after + -- N rolls only N-1 dt samples exist; `rolls` over-weights the + -- running mean toward earlier samples. old_sample_count = + -- round_player.rolls - 1, new count = round_player.rolls. + ELSE ((round_player.avg_dt_between_rolls_ms * (round_player.rolls - 1)) + $6) / round_player.rolls END, fastest_dt_ms = CASE WHEN $6 IS NULL THEN round_player.fastest_dt_ms @@ -443,8 +485,8 @@ async def _h_roll(con, ev): SET rolls_this_round = rolls_this_round + 1, total_rolls = total_rolls + 1, last_roll_ts = to_timestamp($2 / 1000.0), - leader_user_id = CASE WHEN $3 >= leader_matched THEN $4 ELSE leader_user_id END, - leader_name = CASE WHEN $3 >= leader_matched THEN $5 ELSE leader_name END, + leader_user_id = CASE WHEN $3 > leader_matched THEN $4 ELSE leader_user_id END, + leader_name = CASE WHEN $3 > leader_matched THEN $5 ELSE leader_name END, leader_matched = GREATEST(leader_matched, $3), updated_ts = now() WHERE game_code = $1 diff --git a/server/widget.py b/server/widget.py index 3d494547..6a50ddfb 100644 --- a/server/widget.py +++ b/server/widget.py @@ -154,7 +154,7 @@ async def _db_stats() -> tuple[str, list[dict], bool]: ORDER BY e.game_code, e.user_id, e.ts DESC ) SELECT r.game_code, r.ended_ts, - (SELECT json_agg(name) FROM names n + (SELECT json_agg(name ORDER BY name) FROM names n WHERE n.game_code = r.game_code) AS players FROM recent r ORDER BY r.ended_ts DESC @@ -173,7 +173,9 @@ async def _db_stats() -> tuple[str, list[dict], bool]: async def widget_page(key: str = "") -> HTMLResponse: if WIDGET_TOKEN is None: raise HTTPException(status_code=503, detail="widget disabled") - if not secrets.compare_digest(key, WIDGET_TOKEN): + # Compare as bytes: secrets.compare_digest raises TypeError (→ 500, not 401) + # on a non-ASCII str, which a non-ASCII WIDGET_TOKEN + key would trigger. + if not secrets.compare_digest(key.encode("utf-8"), WIDGET_TOKEN.encode("utf-8")): raise HTTPException(status_code=401, detail="unauthorized") active, redis_ok = await _game_stats() diff --git a/server/ws.py b/server/ws.py index 6c62cbc3..f2a7803d 100644 --- a/server/ws.py +++ b/server/ws.py @@ -10,6 +10,7 @@ from .broadcast import ( advance_round, broadcast, + cancel_drop_tasks, checkout_game, delayed_broadcast, do_drop, @@ -542,6 +543,7 @@ async def handle_end_game(session: Session, msg: dict) -> None: t = state.pause_tasks.pop(code, None) if t: t.cancel() + cancel_drop_tasks(code) async def handle_leave(session: Session, msg: dict) -> None: @@ -621,24 +623,30 @@ async def websocket_endpoint(ws: WebSocket) -> None: return session = Session(ws, str(uuid.uuid4())) - sessions[id(ws)] = session - - metrics.ws_connections_active.inc() - metrics.ws_connects_total.inc() - emit("connection_opened", - session_id=session.session_id, peer=session.peer, - user_agent=session.user_agent) + disconnect_reason = "client" + # Everything past conn_incr runs inside the try so the finally always fires. + # The initial `welcome` send (and pinger setup) used to sit ABOVE the try; + # if that send raised — a client that vanished right after the handshake, + # routine on flaky mobile links — the finally never ran, so conn_decr / + # sessions.pop / pinger.stop were all skipped and the per-IP counter leaked + # (its TTL refreshed on every reconnect), eventually locking the IP and its + # NAT peers out for up to an hour. + try: + sessions[id(ws)] = session + metrics.ws_connections_active.inc() + metrics.ws_connects_total.inc() + emit("connection_opened", + session_id=session.session_id, peer=session.peer, + user_agent=session.user_agent) - # Pinger: routes through send() so its frames respect the per-session - # send_lock and get counted in the outbound metrics. - session.pinger = Pinger(session.session_id, lambda m: send(ws, m)) - session.pinger.start() + # Pinger: routes through send() so its frames respect the per-session + # send_lock and get counted in the outbound metrics. + session.pinger = Pinger(session.session_id, lambda m: send(ws, m)) + session.pinger.start() - await send(ws, {"type": "welcome", "player_id": session.pid}) - log.info("connect pid=%s session=%s", session.pid[:8], session.session_id[:8]) + await send(ws, {"type": "welcome", "player_id": session.pid}) + log.info("connect pid=%s session=%s", session.pid[:8], session.session_id[:8]) - disconnect_reason = "client" - try: while True: text = await ws.receive_text() # Reject oversized frames before parsing (audit L2). diff --git a/static/js/components/game-detail-screen.js b/static/js/components/game-detail-screen.js index 8e9f62e0..659611b1 100644 --- a/static/js/components/game-detail-screen.js +++ b/static/js/components/game-detail-screen.js @@ -12,6 +12,10 @@ import { state } from '../state.js'; * Light DOM: the host element *is* `#game-detail.screen`. */ export class GameDetailScreen extends HTMLElement { + /** Bumped each time verification (re)starts so a superseded run bails at its + * next await instead of writing into the newly-rendered detail view. */ + #verifyGen = 0; + connectedCallback() { if (this.dataset.rendered) return; this.dataset.rendered = 'true'; @@ -151,6 +155,11 @@ export class GameDetailScreen extends HTMLElement { const statusEl = document.getElementById('gd-trust-status'); const boxEl = document.getElementById('gd-trust-box'); if (!statusEl || !boxEl) return; + // Generation guard: quickly switching between two /games/ views starts + // overlapping verification loops. Capture our generation; a newer run bumps + // it, so we bail at the next await instead of writing stale results (e.g. + // toggling the other view's scanner class) into the freshly-rendered view. + const gen = ++this.#verifyGen; // Phase 1: scanning animation const phases = [ @@ -162,24 +171,28 @@ export class GameDetailScreen extends HTMLElement { for (const msg of phases) { statusEl.innerHTML = msg; await new Promise((r) => setTimeout(r, 600)); + if (gen !== this.#verifyGen) return; } // Phase 2: actual verification statusEl.innerHTML = 'Verifying rolls…'; try { const res = await fetch(`/api/game/${encodeURIComponent(code)}/verify`); + if (gen !== this.#verifyGen) return; if (!res.ok) { statusEl.innerHTML = 'Verification unavailable'; boxEl.classList.add('gd-trust-done'); return; } const v = await res.json(); + if (gen !== this.#verifyGen) return; // Phase 3: reveal per-player results with stagger const scannerEl = document.getElementById('gd-trust-scanner'); if (scannerEl) scannerEl.classList.add('gd-trust-scan-done'); await new Promise((r) => setTimeout(r, 400)); + if (gen !== this.#verifyGen) return; // Build results const allPassed = v.failed === 0 && v.total > 0; diff --git a/static/js/components/profile-screen.js b/static/js/components/profile-screen.js index e4ddd553..1284b927 100644 --- a/static/js/components/profile-screen.js +++ b/static/js/components/profile-screen.js @@ -262,10 +262,13 @@ export class ProfileScreen extends HTMLElement { }).join('')} `; recentEl.hidden = false; - recentEl.addEventListener('click', (e) => { + // Assign (not addEventListener): #profile-recent persists across profile + // navigations, so addEventListener stacked a new handler every render. + // onclick replaces, keeping exactly one. + recentEl.onclick = (e) => { const row = /** @type {HTMLElement | null} */ (/** @type {HTMLElement} */ (e.target).closest('.recent-game[data-game-code]')); if (row?.dataset.gameCode) showGameDetail(row.dataset.gameCode); - }); + }; } // Trigger shimmer animation once diff --git a/static/js/game-render.js b/static/js/game-render.js index 29dc504e..6f34989e 100644 --- a/static/js/game-render.js +++ b/static/js/game-render.js @@ -193,7 +193,11 @@ function fmtRemaining(ms) { /** @type {ReturnType | null} */ let pauseTick = null; -function stopPauseTick() { +/** Stop the host's 1 Hz pause countdown. Exported so the net layer can clear it + * on game teardown — game_ended / a fatal pause-cap error tear the game down + * without another renderMenu() call, so the interval would otherwise keep + * firing forever against the hidden #pause-remaining element. */ +export function stopPauseTick() { if (pauseTick) { clearInterval(pauseTick); pauseTick = null; diff --git a/static/js/net.js b/static/js/net.js index df9a87d9..9017d83b 100644 --- a/static/js/net.js +++ b/static/js/net.js @@ -1,7 +1,7 @@ // @ts-check import { myDiceKey } from './dice.js'; import { byId } from './dom.js'; -import { renderMyArea, renderPlayersBar } from './game-render.js'; +import { renderMyArea, renderPlayersBar, stopPauseTick, syncPaused } from './game-render.js'; import { showWinner } from './overlays.js'; import { landing, showFor, showGameDetail, showLanding } from './router.js'; import { getAuthToken, isSignedIn, getAuthUser } from './auth.js'; @@ -341,6 +341,7 @@ function handleMessage(msg) { } case 'game_ended': { resetRollState(); + stopPauseTick(); // The game screen is a persistent shell element, so its in-game menu // (open when you tapped "End Game") would otherwise stay open and show // up on the next game's board. Reset it as the game tears down. @@ -368,6 +369,12 @@ function handleMessage(msg) { */ function handleError(msg) { if (msg.fatal) { + // The in-game menu is a persistent shell element. A fatal frame (the pause + // cap) can arrive with it open — the host pauses with the menu open by + // design — so close it here too, or it leaks into the next game's board. + // Mirrors the game_ended path above. + /** @type {import('./components/game-screen.js').GameScreen} */ (byId('game')).closeMenu(); + stopPauseTick(); clearSession(); state.currentState = null; state.reconnecting = false; @@ -406,5 +413,10 @@ function handleError(msg) { resetRollState(); renderMyArea(state.currentState); renderPlayersBar(state.currentState); + // renderMyArea rebuilds the roll button in its default (enabled) state, so + // re-apply the paused flag — otherwise an error that lands while paused + // leaves the button reading "Roll" (harmless, roll() guards on paused, but + // visually wrong). + syncPaused(state.currentState); } } diff --git a/static/js/overlays.js b/static/js/overlays.js index 0abba9bf..8a35b8eb 100644 --- a/static/js/overlays.js +++ b/static/js/overlays.js @@ -24,11 +24,20 @@ export const RESUME_CLOSE_DELAY_MS = 600; // ── Pause overlay (non-host) ── +/** @type {ReturnType | undefined} Pending resume-close. */ +let pauseCloseTimer; + /** * Open the pause wait dialog with the given message. * @param {string} text */ export function showPaused(text) { + // Cancel any in-flight resume-close: if the host resumed and then re-paused + // within RESUME_CLOSE_DELAY_MS, the stale timer would otherwise fire and + // close the dialog we just re-opened, stranding a non-host on a live-but- + // paused board with no wait screen. + clearTimeout(pauseCloseTimer); + pauseCloseTimer = undefined; const msg = document.getElementById('pause-overlay-msg'); if (msg) msg.textContent = text; if (pauseOverlay && !pauseOverlay.open) pauseOverlay.showModal(); @@ -36,9 +45,24 @@ export function showPaused(text) { /** Close the pause wait dialog if it's open. */ export function hidePaused() { + clearTimeout(pauseCloseTimer); + pauseCloseTimer = undefined; if (pauseOverlay?.open) pauseOverlay.close(); } +/** + * Close the pause dialog after a delay (so the resume toggle's slide-off is + * visible), cancellably — a re-pause via showPaused() aborts the pending close. + * @param {number} delayMs + */ +export function hidePausedSoon(delayMs) { + clearTimeout(pauseCloseTimer); + pauseCloseTimer = setTimeout(() => { + pauseCloseTimer = undefined; + hidePaused(); + }, delayMs); +} + /** * "Waiting for to resume the game" * @param {GameSnapshot} snap diff --git a/static/js/router.js b/static/js/router.js index 3c044703..77e6d0c2 100644 --- a/static/js/router.js +++ b/static/js/router.js @@ -1,7 +1,7 @@ // @ts-check import { byId } from './dom.js'; import { - RESUME_CLOSE_DELAY_MS, hidePaused, hideWinner, pausedText, showPaused, waitingText, + RESUME_CLOSE_DELAY_MS, hidePaused, hidePausedSoon, hideWinner, pausedText, showPaused, waitingText, } from './overlays.js'; import { saveGameCode, hasSession } from './session.js'; import { state } from './state.js'; @@ -360,7 +360,7 @@ export function showFor(snap) { else reveal(); // Just resumed: drop the pause overlay after the toggle's slide-off. const pauseDialog = /** @type {HTMLDialogElement | null} */ (document.getElementById('pause-overlay')); - if (pauseDialog?.open) setTimeout(hidePaused, RESUME_CLOSE_DELAY_MS); + if (pauseDialog?.open) hidePausedSoon(RESUME_CLOSE_DELAY_MS); else hidePaused(); }); } diff --git a/static/js/sheet.js b/static/js/sheet.js index d5ae4db9..0c87eb14 100644 --- a/static/js/sheet.js +++ b/static/js/sheet.js @@ -35,7 +35,14 @@ export class SheetController { this.close(); }); // Backdrop tap (a modal dialog reports it as a click on the dialog itself). + // Gate on event.target === dialog FIRST: a click on any child (a button, an + // input) targets that child, not the dialog. Without this, the touch guard's + // synthesized second-tap click — dispatched at (0,0) — bubbles up from a + // button inside the sheet and, because (0,0) is geometrically outside this + // bottom-anchored dialog, was misread as a backdrop tap and slammed the + // sheet shut mid-interaction (e.g. tearing down the mic on the listen sheet). dialog.addEventListener('click', (event) => { + if (event.target !== dialog) return; const r = dialog.getBoundingClientRect(); const outside = event.clientX < r.left || event.clientX > r.right || event.clientY < r.top || event.clientY > r.bottom; From dfa6cc0947c2406afff885f03f56d6766a12621b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 05:51:34 +0000 Subject: [PATCH 2/9] session-start hook: trust the egress-proxy CA at runtime too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generated compose override fed the sandbox proxy CA into the image build as a BuildKit secret, but the running web container egresses through the same intercepting proxy — so the app's own httpx clients (drand, discord, places) hit "certificate verify failed" and drand silently fell back to local RNG. Extend the override to also mount the host CA bundle into the web container and point SSL_CERT_FILE / REQUESTS_CA_BUNDLE at it, so trust_env-aware clients verify TLS through the proxy at runtime. Verified: drand loads the quicknet chain (bls=on), fetches beacons, and a roll re-derives + verifies via /api/game/{code}/verify (total:1 verified:1 failed:0). Override stays git-ignored and remote-only. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_012xoZvcd19p4WWYnPkLZkfn --- .claude/hooks/session-start.sh | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/.claude/hooks/session-start.sh b/.claude/hooks/session-start.sh index c66cb759..9e9f7f50 100755 --- a/.claude/hooks/session-start.sh +++ b/.claude/hooks/session-start.sh @@ -90,28 +90,40 @@ else add "⚠️ Docker Hub login: skipped — no creds (pulls may hit the anon rate limit)" fi -# ── 3. Proxy-CA build secret (local-only compose override) ──────────────────── -# The sandbox MITMs TLS; only the host trusts the egress CA, so build containers -# can't verify pypi/npm. Hand the full host CA bundle to the build as a secret. +# ── 3. Proxy-CA build secret + runtime trust (local-only compose override) ──── +# The sandbox MITMs TLS; only the host trusts the egress CA. Build containers +# can't verify pypi/npm (handled via the BuildKit `proxy_ca` secret), and — just +# as important — the *running* web container egresses through the same proxy, so +# the app's own httpx clients (drand, discord, places) can't verify api.drand.sh +# et al. either. Mount the full host CA bundle into the container at runtime and +# point the standard CA env vars at it so trust_env-aware clients pick it up. CA_BUNDLE="${SSL_CERT_FILE:-/etc/ssl/certs/ca-certificates.crt}" OVERRIDE="$CLAUDE_PROJECT_DIR/docker-compose.override.yml" +CA_IN_CONTAINER="/etc/ssl/ccr-ca-bundle.crt" if [ -r "$CA_BUNDLE" ]; then cat > "$OVERRIDE" < Date: Sat, 18 Jul 2026 06:05:21 +0000 Subject: [PATCH 3/9] Authenticate the legacy-pid stat claim with the private reconnect token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Registration transferred an anonymous player's stats onto the new account from a client-supplied `legacy_pid` with no proof of ownership. Anonymous pids are visible to every co-player via state_msg, so a co-player could harvest a pid and claim its stats before the real owner registered. Require the pid's private reconnect token (which never leaves the owner's client — only the pid is broadcast) as proof: - create/join record the anon pid's token hash in a TTL'd Redis key (claim:{pid}, CLAIM_TTL=7d, self-expiring so no cleanup job); skipped for already-authenticated sessions. - registration sends the token as claim_token; register_verify transfers stats only when gamestore.verify_claim matches, and clears the claim on success (single use). Registration still succeeds without a valid token — just without the transfer, matching the existing graceful-degradation behavior. Verified: the correct token transfers; wrong/empty/pid-as-token all refuse; the claim is single-use. create/join/roll smoke unaffected. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_012xoZvcd19p4WWYnPkLZkfn --- server/auth.py | 16 +++++++++++++++- server/config.py | 8 ++++++++ server/gamestore.py | 31 +++++++++++++++++++++++++++++++ server/ws.py | 9 +++++++++ static/js/auth.js | 5 ++++- 5 files changed, 67 insertions(+), 2 deletions(-) diff --git a/server/auth.py b/server/auth.py index 1771797b..0241d982 100644 --- a/server/auth.py +++ b/server/auth.py @@ -140,6 +140,10 @@ class RegisterVerifyRequest(BaseModel): username: str credential: dict legacy_pid: str | None = None + # Private reconnect token for legacy_pid. The pid leaks to co-players via + # state_msg, so it can't authorise a stat transfer on its own; the token + # (never broadcast) proves the registrant actually owns that anon identity. + claim_token: str | None = None class LoginOptionsRequest(BaseModel): username: str @@ -254,7 +258,12 @@ async def register_verify(body: RegisterVerifyRequest, request: Request): "SELECT 1 FROM users WHERE id::text = $1 OR legacy_pid = $1", legacy_pid, ) - if taken: + # Prove ownership: the caller must present the pid's private + # reconnect token, not just the (co-player-visible) pid. Without + # a valid token the account is still created — just without the + # stat transfer. + owns = await gamestore.verify_claim(legacy_pid, body.claim_token or "") + if taken or not owns: legacy_pid = None async def _insert_user(lp: str | None) -> None: @@ -310,6 +319,11 @@ async def _insert_user(lp: str | None) -> None: legacy_pid, ) + # The claim is single-use: once the stats have moved, drop the token so the + # same pid can't be re-claimed. (Outside the DB transaction — a Redis op.) + if legacy_pid: + await gamestore.clear_claim(legacy_pid) + # Fetch any transferred stats for the onboarding screen stats = None async with db.pool().acquire() as con: diff --git a/server/config.py b/server/config.py index ac7d101b..4c263a1a 100644 --- a/server/config.py +++ b/server/config.py @@ -294,6 +294,14 @@ def _list(name: str) -> list[str]: ) JWT_EXPIRY_DAYS = _int("JWT_EXPIRY_DAYS", 30) +# How long an anonymous player's reconnect-token hash is kept (Redis, keyed by +# pid) so a later account registration can prove ownership of that pid's stats. +# The pid leaks to co-players via state_msg; the token never does — so requiring +# it at claim time stops a co-player from harvesting a pid and stealing its +# stats. Generous window so "register a while after playing" still transfers; +# self-expiring, so no cleanup job is needed. +CLAIM_TTL = _int("CLAIM_TTL", 7 * 24 * 3600) # 7 days + # ─── Founding member ("Founding Roller") ────────────────────────────── # Accounts created strictly before this instant earn the Founding Roller # designation on their profile. Fixed UTC cutoff: through end of Jul 19, 2026. diff --git a/server/gamestore.py b/server/gamestore.py index 3bcdfa0a..82afdd3c 100644 --- a/server/gamestore.py +++ b/server/gamestore.py @@ -24,6 +24,7 @@ import redis.asyncio as aioredis from server.config import ( + CLAIM_TTL, GAME_TTL, MAX_GAMES, MAX_PLAYERS_PER_GAME, @@ -661,6 +662,36 @@ async def rate_allow(scope: str, ident: str, limit: int, window: float) -> bool: return int(n) <= limit +# ─── Stat-claim ownership tokens ─────────────────────────────────────────────── +# An anonymous player's pid leaks to co-players via state_msg, so the pid alone +# can't authorise transferring that pid's player_stats onto a new account (a +# co-player could harvest it and claim someone else's stats). The private +# reconnect token never leaves the owner's client, so we keep its hash keyed by +# pid — outliving the ephemeral game — and require the token at registration. + +def _claim_key(pid: str) -> str: + return f"claim:{pid}" + + +async def record_claim(pid: str, token_hash: str) -> None: + """Remember an anonymous pid's reconnect-token hash so a later registration + can prove ownership of its stats. Self-expiring (CLAIM_TTL).""" + await _r.set(_claim_key(pid), token_hash, ex=CLAIM_TTL) + + +async def verify_claim(pid: str, token: str) -> bool: + """True if `token` matches the stored claim hash for `pid`.""" + if not pid or not token: + return False + from .game import verify_token # local import avoids an import cycle + return verify_token(await _r.get(_claim_key(pid)), token) + + +async def clear_claim(pid: str) -> None: + """Drop a claim once its stats have been transferred (single use).""" + await _r.delete(_claim_key(pid)) + + async def conn_incr(ip: str) -> int: key = f"conn:{ip}" n = await _r.incr(key) diff --git a/server/ws.py b/server/ws.py index f2a7803d..f1e04c64 100644 --- a/server/ws.py +++ b/server/ws.py @@ -157,6 +157,11 @@ async def handle_create(session: Session, msg: dict) -> None: connections[code] = {session.pid: session.ws} session.code = code session.games_joined += 1 + # Remember this anon pid's token hash so a later account registration can + # prove ownership of its stats (see gamestore.record_claim). Authenticated + # sessions are already an account — nothing to claim. + if session.user_id is None: + await gamestore.record_claim(session.pid, token_hash) _ensure_session_started(session) log.info("create game=%s host=%s", code, name) emit("game_created", game_code=code, user_id=session.pid, name=name, @@ -199,6 +204,10 @@ async def handle_join(session: Session, msg: dict) -> None: session.code = join_code connections.setdefault(join_code, {})[session.pid] = session.ws session.games_joined += 1 + # Anon pid → token-hash claim, so registration can later prove ownership of + # its stats (mirrors handle_create). + if session.user_id is None: + await gamestore.record_claim(session.pid, token_hash) _ensure_session_started(session) log.info("join game=%s player=%s players=%d", join_code, name, res) emit("player_joined", game_code=join_code, user_id=session.pid, name=name, diff --git a/static/js/auth.js b/static/js/auth.js index c1d66baf..0ee13a0e 100644 --- a/static/js/auth.js +++ b/static/js/auth.js @@ -105,7 +105,7 @@ export function isWebAuthnAvailable() { */ export async function registerPasskey(username) { // 1. Get options from server - const legacyPid = readSession().playerId; + const { playerId: legacyPid, token: claimToken } = readSession(); const optRes = await fetch('/auth/register/options', { method: 'POST', headers: { 'Content-Type': 'application/json' }, @@ -162,6 +162,9 @@ export async function registerPasskey(username) { user_id, }, legacy_pid: legacyPid, + // Private reconnect token proving we own legacyPid's stats (the pid alone + // is visible to co-players, so the server won't transfer stats on it). + claim_token: claimToken, }), }); if (!verifyRes.ok) { From b9b84d50451764ad508c78ea2aeff340ca03ef5e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 08:45:44 +0000 Subject: [PATCH 4/9] Actually drop a voluntary leaver while paused (Lua guard) + cancel pause watchdog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier leave-while-paused fix only lifted the paused guard in do_drop's Python; the atomic _DROP_LUA has its OWN "never drop while paused" guard, so a voluntary leave still no-oped and the leaver lingered as a ghost — the fix never actually worked. Verified live: before, the server snapshot still showed the guest present after leaving a paused game. - _DROP_LUA / gamestore.drop_player take allow_paused; a voluntary leave (reason="leave") sets it so the slot is removed even while paused, while disconnect/reaper drops stay held (players are never dropped on a disconnect during a pause). - do_drop's "deleted" branch now cancels the pause_timeout watchdog too — that branch became reachable for a paused game once a leave can delete it (last player out), and would otherwise orphan the 1h watchdog task. Verified live: guest leaving a paused game is removed (players 2->1, host gets the broadcast); solo host leaving a paused game deletes it cleanly; smoke, claim gate, and drand verification all still pass with no errors. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_012xoZvcd19p4WWYnPkLZkfn --- server/broadcast.py | 11 ++++++++++- server/gamestore.py | 15 +++++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/server/broadcast.py b/server/broadcast.py index 1ddc18dc..b61586ff 100644 --- a/server/broadcast.py +++ b/server/broadcast.py @@ -244,7 +244,10 @@ async def do_drop( name = player["name"] # Snapshot any check-in before the drop, which may delete the whole hash. checkin = await gamestore.get_place(code) - res = await gamestore.drop_player(code, pid, grace_ms) + # A voluntary leave removes the slot even while paused; the Lua otherwise + # holds every player during a pause (disconnect/reaper drops stay held). + res = await gamestore.drop_player(code, pid, grace_ms, + allow_paused=(reason == "leave")) if res["action"] == "noop": return local = state.connections.get(code) @@ -269,6 +272,12 @@ async def do_drop( await _emit_checkout(code, pid, checkin, reason="game_ended") state.connections.pop(code, None) cancel_drop_tasks(code) + # A voluntary leave can now delete a *paused* game (last player out), so + # cancel its pause watchdog here too — this branch was unreachable while + # paused before that change. + pt = state.pause_tasks.pop(code, None) + if pt: + pt.cancel() return if res["new_host"]: diff --git a/server/gamestore.py b/server/gamestore.py index 82afdd3c..d0c68efa 100644 --- a/server/gamestore.py +++ b/server/gamestore.py @@ -120,14 +120,16 @@ async def close() -> None: _DROP_LUA = """ -- KEYS[1]=game key KEYS[2]=index KEYS[3]=geo index --- ARGV: code, pid, grace_ms, now_ms, ttl +-- ARGV: code, pid, grace_ms, now_ms, ttl, allow_paused -- Removes a disconnected player past the grace window. Idempotent: a second -- caller (local task vs reaper) finds the player gone and no-ops. -- Returns {0}=noop, {1,new_host}=removed (new_host '' if unchanged), -- {2}=removed and game deleted (was last player). local key, idx, geo, code, pid = KEYS[1], KEYS[2], KEYS[3], ARGV[1], ARGV[2] if redis.call('EXISTS', key) == 0 then return {0} end -if redis.call('HGET', key, 'paused') == '1' then return {0} end -- never drop while paused +-- Never drop on a *disconnect* while paused (players are held). A voluntary +-- leave (allow_paused='1') is explicit intent and removes the slot even paused. +if redis.call('HGET', key, 'paused') == '1' and ARGV[6] ~= '1' then return {0} end local p = 'p:' .. pid .. ':' if redis.call('HGET', key, p .. 'disconnected') ~= '1' then return {0} end local dat = tonumber(redis.call('HGET', key, p .. 'disconnected_at_ms') or '0') @@ -488,13 +490,18 @@ async def transfer_host(code: str, new_host: str) -> None: await _r.hset(_gkey(code), "host", new_host) -async def drop_player(code: str, pid: str, grace_ms: int) -> dict: +async def drop_player(code: str, pid: str, grace_ms: int, + allow_paused: bool = False) -> dict: """Remove a disconnected player past grace (atomic, idempotent). + `allow_paused` overrides the never-drop-while-paused guard for a voluntary + leave (explicit intent), while disconnect/reaper drops stay held when paused. + Returns {"action": "noop"|"removed"|"deleted", "new_host": str|None}. """ res = await _drop(keys=[_gkey(code), INDEX, GEO_INDEX], - args=[code, pid, grace_ms, now_ms(), GAME_TTL]) + args=[code, pid, grace_ms, now_ms(), GAME_TTL, + "1" if allow_paused else "0"]) status = int(res[0]) if status == 2: return {"action": "deleted", "new_host": None} From e11062a5b1c4af7eff2fe2b7ca080f659e69d8f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 17:32:12 +0000 Subject: [PATCH 5/9] test-game: update stale target-cycle + deep-link-join assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preflight self-update while running the suite: - The target sequence changed from a straight 1→6→1 cycle (next_target) to a ping-pong triangle wave (target_for_round, commit 78a03df). Step 14 and the headless ws_integration_test asserted the old cycle (initial target 6, advance 6→5); update to the wave (initial target 1, round 2 → target 2). - Deep-link `/` now opens the join sheet on the landing screen (openJoinOnLanding) rather than a separate #join screen; update Step 4 to check #join-sheet[open] + prefilled #code-input. Verified against the live server: ws_integration_test now 25/25. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_012xoZvcd19p4WWYnPkLZkfn --- .claude/skills/test-game/SKILL.md | 10 +++++----- .../skills/test-game/scripts/ws_integration_test.py | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.claude/skills/test-game/SKILL.md b/.claude/skills/test-game/SKILL.md index aced719d..b01b91a1 100644 --- a/.claude/skills/test-game/SKILL.md +++ b/.claude/skills/test-game/SKILL.md @@ -200,10 +200,10 @@ In **instance #2** (`mcp__playwright-guest__*`), navigate to: http://localhost:8888/ ``` -Verify: -- The join screen (`#join`) is active -- `#code-input` is pre-filled with `GAME_CODE` (deep-link works — `/` is the primary format; the legacy `?join=` still works as a fallback) -- The URL has been cleaned to `/` (no code remains in the address bar) +Verify (the deep-link now opens the **join sheet on the landing screen** — `openJoinOnLanding` in `static/js/router.js` — not a separate `#join` screen): +- `#landing` is the active screen and the `#join-sheet` `` is **open** (`document.getElementById('join-sheet').open === true`) +- `#code-input` (inside the sheet) is pre-filled with `GAME_CODE` (deep-link works — `/` is the primary format; the legacy `?join=` still works as a fallback) +- The URL is canonicalised to `/` (the legacy `?join=` form is rewritten to the path form) Type `Beta` into `#join-name-input`, then submit the join form (`#join-form button[type="submit"]`). @@ -492,7 +492,7 @@ After the winner overlay appears, wait (up to 4 seconds, less if Step 13 already Verify on both tabs: - Winner overlay is closed — `document.getElementById('winner-overlay').open === false` - `round_num` has incremented by 1 -- `target` has changed and matches the cycle **1→2→3→4→5→6→1** (one step up, wrapping 6→1). This is the live check of `next_target` (`t % 6 + 1` in `server/game.py`); confirm the new `target` is exactly the successor in that cycle. +- `target` has changed and follows the **triangle wave** over the 1-based round number: `1→2→3→4→5→6→5→4→3→2→1→2→…` (climbs to 6, then descends to 1, then climbs again). This is the live check of `target_for_round(round_num)` in `server/game.py` (`m=(round_num-1)%10; m+1 if m<=5 else 11-m`) — the old `next_target` (`t % 6 + 1`, straight 6→1 wrap) was replaced by the ping-pong in commit 78a03df. Confirm the new `target` equals `target_for_round(new round_num)`. - All dice are unlocked (new fresh dice dealt) - `has_rolled` is `false` for all players - Roll button is enabled on Tab 1 diff --git a/.claude/skills/test-game/scripts/ws_integration_test.py b/.claude/skills/test-game/scripts/ws_integration_test.py index 174d1875..dac5fca0 100644 --- a/.claude/skills/test-game/scripts/ws_integration_test.py +++ b/.claude/skills/test-game/scripts/ws_integration_test.py @@ -66,8 +66,8 @@ async def main(): check("create: private reconnect_token frame sent", bool(host_token), f"len={len(host_token) if host_token else 0}") check("create: token_hash NOT leaked in state", "token_hash" not in json.dumps(state)) - check("create: not started, target=6, round 1", - state["started"] is False and state["target"] == 6 and state["round_num"] == 1) + check("create: not started, target=1, round 1", + state["started"] is False and state["target"] == 1 and state["round_num"] == 1) check("create: host has HOST role", state["host"] == host_pid) # ── invalid code rejection ─────────────────────────────────────── @@ -144,7 +144,7 @@ async def main(): adv = await recv_until(host, "state", timeout=6.0) check("round advance: round_num -> 2", adv["round_num"] == 2, f"round={adv['round_num']}") - check("round advance: target cycled 6->5", adv["target"] == 5, f"target={adv['target']}") + check("round advance: triangle wave 1->2 (round 2)", adv["target"] == 2, f"target={adv['target']}") check("round advance: dice re-dealt, has_rolled reset", adv["players"][host_pid]["has_rolled"] is False) From da726da68763b2c5aca446d1129e68eff6bb552a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 17:39:34 +0000 Subject: [PATCH 6/9] test-game run log 2026-07-18 (adapted harness) Full-suite pass on claude/code-review-ogi5bx. MCP Playwright browsers weren't connected this session, so the suite ran adapted: headless ws_integration_test (25/25), loadtest (66 rolls/s, no game leak), node-Playwright two-context UI + CDP WebAuthn (8/8), and the prod bundle smoketest (4/4). No game bugs; all branch fixes live-exercised with no regressions. 5 regression-specific steps (11/13/16/20/21/22) are PARTIAL/NOT-RUN pending the MCP bespoke probes. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_012xoZvcd19p4WWYnPkLZkfn --- docs/test-runs/game/2026-07-18T17-38-57.md | 86 ++++++++++++++++++++++ docs/test-runs/game/README.md | 1 + 2 files changed, 87 insertions(+) create mode 100644 docs/test-runs/game/2026-07-18T17-38-57.md diff --git a/docs/test-runs/game/2026-07-18T17-38-57.md b/docs/test-runs/game/2026-07-18T17-38-57.md new file mode 100644 index 00000000..0785ce41 --- /dev/null +++ b/docs/test-runs/game/2026-07-18T17-38-57.md @@ -0,0 +1,86 @@ +# Test run — 2026-07-18 (adapted harness) + +## Methodology note +The Playwright **MCP** browser instances this skill is built around were **not +connected this session** (the session-start hook reported the browser install +failed; `mcp__playwright__*` tools don't resolve). The suite was run **adapted**: +- the skill's own headless protocol harness `ws_integration_test.py` (real WS + protocol, reconnect + token-auth matrix), +- `loadtest.py` for concurrency + a Redis `games:index` leak check, +- **node-driven Playwright** against the installed Chromium (two isolated + browser contexts = isolated per-player localStorage; CDP virtual authenticator + for WebAuthn), mobile viewport 390×844, +- `curl` for the prod bundle-structure checks. +Screenshots were not captured (no MCP screenshot tool); DOM/state assertions +were used instead. + +## Results +| # | Result | Description | +|---|--------|-------------| +| 01 | ✅ PASS | Server health | +| 02 | ✅ PASS | Landing screen (random-name placeholder, no errors) | +| 03 | ✅ PASS | Create game + lobby + inline QR + solo hint | +| 04 | ✅ PASS | Deep-link join **sheet** + two-way lobby sync | +| 05 | ✅ PASS | Invalid game code rejection (protocol) | +| 06 | ✅ PASS | Game start + initial render (both clients) | +| 07 | ✅ PASS | Join-after-start rejection (protocol) | +| 08 | ✅ PASS | Roll + reveal, roll_count advances | +| 09 | ✅ PASS | Multiple rolls, no roll-ack hang | +| 10 | ✅ PASS | Rate limit ("Slow down" on 2nd rapid raw roll) | +| 11 | 📝 PARTIAL | Broadcast timing: delayed_broadcast verified (broadcast follows ack); exact 800–2000ms window not measured | +| 12 | ✅ PASS | Roll to win + winner overlay (Winner banner, round) | +| 13 | 📝 NOT RUN | Sticky-overlay spacebar-hammer regression (needs the bespoke keydown loop) | +| 14 | ✅ PASS | Round transition + **triangle-wave** target (round2→target2) | +| 15 | ✅ PASS | Host pause toggle (host-only), non-host overlay, roll disabled ("Paused") | +| 16 | 📝 PARTIAL | Pause basics verified; full "everyone steps away → host returns to open menu" choreography not driven | +| 17 | ✅ PASS | Resume returns both clients to play | +| 18 | ✅ PASS | Player disconnect mid-game (host sees held/disconnected) | +| 19 | ✅ PASS | Player reconnect (reload) + **token auth matrix** (missing/wrong/cross-player rejected, correct accepted) | +| 20 | 📝 PARTIAL | Reconnect mechanism covered by matrix; host-specific disconnect+reconnect not separately driven | +| 21 | 📝 PARTIAL | Roll machine settles (rolling/awaitingAck clear); no screenshots (no MCP tool) | +| 22 | 📝 PARTIAL | 3 prod rounds showed correct Winner/Loser per round, no flash seen; dedicated staggered-cadence flash test not run | +| 23 | ✅ PASS | Console clean (0 errors, dev + prod, both clients) | +| 24 | ✅ PASS | CDP virtual authenticator + signin screen | +| 25 | ✅ PASS | Sign-up (register passkey) → onboarding, JWT saved, user in Postgres | +| 26 | ✅ PASS | Signed-in landing (name hidden, @username pill) | +| 27 | ✅ PASS | Signed-in gameplay (username as player name) | +| 28 | ✅ PASS | Sign-out returns anon state | +| 29 | ✅ PASS | Sign-in existing account restores session | +| 30 | ✅ PASS | Switch to prod build (esbuild + nginx) | +| 31 | ✅ PASS | Bundle structure (one hashed app JS, no modules/modulepreload, gzip, immutable) + widget manifest fingerprinting | +| 32 | ✅ PASS | Resource count = 13 (bundled, no individual modules) | +| 33 | ✅ PASS | 3-round game on prod bundle, correct overlays, no errors | +| 34 | ✅ PASS | Restore dev stack | +| — | ✅ PASS | Load test: 12 games, 66 rolls/s, 0 errors; games:index drains to baseline (no leak); per-IP conn cap correctly bounds 200-game run | + +**26 PASS, 5 PARTIAL, 1 NOT RUN, 0 FAIL** (of 34 + load test) + +## Findings +- No game bugs found. All core flows (create/join/start/roll/win/advance/pause/ + resume/disconnect/reconnect/auth/prod-bundle) pass. +- **Two stale test assets fixed during preflight** (committed e11062a): the + target sequence is now a triangle wave (`target_for_round`), not the old + `next_target` 1→6→1 cycle — Step 14 + `ws_integration_test.py` asserted the old + cycle; and deep-link `/` now opens the join **sheet** on landing, not a + separate `#join` screen — Step 4 updated. After fixes: ws_integration_test 25/25. +- `ws_integration_test.py` had one transient miss (round advance not seen within + 6s) on 1 of ~5 runs — a win→advance timing brush against the 6s wait, not a + bug (3 consecutive clean 25/25 runs after). + +## Notes +- MCP browser instances unavailable → adapted harness (see Methodology). This + changes the *mechanism* (node-Playwright + headless), not the coverage of the + core flows; the regression-specific probes marked PARTIAL/NOT RUN are the ones + whose bespoke instrumentation wasn't ported. +- Leak check done via Redis `games:index` (the loadtest's own gauge read returns + None because `/metrics` is bearer-gated and the loadtest sends no token). +- All backend fixes from branch `claude/code-review-ogi5bx` were live-exercised + by this run (fan-out, telemetry, leave-while-paused, claim gate, drand, widget + pipeline) with no regressions. + +## Watch next run +- If MCP browsers are available, run the full bespoke probes for Steps 11, 13, + 16, 20, 21, 22 (broadcast-ms window, spacebar sticky overlay, pause + abandonment, host reconnect, animation screenshots, multi-round flash cadence). +- The 6s round-advance wait in ws_integration_test.py is occasionally tight — + consider bumping to 8s to remove the rare transient. diff --git a/docs/test-runs/game/README.md b/docs/test-runs/game/README.md index f5b08c79..a56bf4b3 100644 --- a/docs/test-runs/game/README.md +++ b/docs/test-runs/game/README.md @@ -7,6 +7,7 @@ loop across two isolated Playwright instances. | Date | Scope | Result | Passed | Total | Highlight | |------|-------|--------|--------|-------|-----------| +| [2026-07-18T17:38:57](2026-07-18T17-38-57.md) | Game | ✅ PASS | 26+LT | 34 | `claude/code-review-ogi5bx` post-fix pass. **MCP browsers unavailable → adapted harness** (headless `ws_integration_test` 25/25, `loadtest` 66 rolls/s + no game leak, node-Playwright 2-context UI + CDP WebAuthn 8/8, prod bundle 4/4). No game bugs. Fixed 2 stale test assets (triangle-wave target + deep-link join sheet). 5 regression-specific steps PARTIAL/NOT-RUN (need MCP bespoke probes). All branch fixes live-exercised, no regressions | | [2026-07-08T07:24:00](2026-07-08T07-24-00.md) | Game | ✅ PASS | 34 | 34 | `claude/multiplayer-game-discovery-gps-ttcma6` post dead-code-sweep pass. **Inline QR stamp verified** (`.qr` is a `data:` URL, no fetch). Skill self-updated for the lobby-stamp + unified-auth (`#auth-submit-btn`) reworks + others-only roster + `lower(username)` SQL. Overlay consistency 2988–3041ms across 7 rounds, no flash. Env gotchas only: `WEBAUTHN_RP_ID=localhost` override, `.env.prod` inline secrets, dev↔prod `pg_data` password mismatch needing `down -v` both ways | | [2026-07-05T22:29:15](2026-07-05T22-29-15.md) | Game | ✅ PASS | 21 | 21 | `fix/fable5-review-fixes` pre-merge pass (targeted to changed surfaces + prod bundle). **Restamp fix proven live**: pause→resume→drop at 61s not ~15s. Roll floor 0.75s doesn't reject honest play (min gap 1086ms). Prod asset refactor intact (CSP still stamped). Per-fix live checks all green (reaper CAS/SREM, legacy_pid guard, auth 429, tsc 0, dev no-restart hash) | | [2026-07-03T20:49:21](2026-07-03T20-49-21.md) | Game | ✅ PASS | 34 | 34 | `feature/video-intro` pre-merge pass; **real WebAuthn** (RP_ID override, not degraded); overlay consistency 2997–3089ms across 6 rounds, no flash; prod bundle validated incl. **fingerprinted `/static/video`** (the CI frontend fix); only env-setup friction (`.env.prod` dummy secrets + `down -v`) | From 04dfc2b557158051648bd41bd673002c2df1c56e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 17:49:45 +0000 Subject: [PATCH 7/9] session-start hook: install the correct Playwright MCP browser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hook installed `chrome-for-testing`, but the .mcp.json servers launch with `--browser chromium`, which resolves to Playwright's own pinned Chromium build (revision 1232 for the current @playwright/mcp), not Google's chrome-for-testing. So `--browser chromium` had no executable, every MCP server failed to launch a browser, and none of the mcp__playwright__* tools registered — which is why the test-game skill couldn't run as designed this session. - Install `chromium` (+ its headless-shell) instead of chrome-for-testing. - Clear a stale/corrupt @playwright/mcp npx cache first (a broken 1.62.0-alpha with a missing playwright-core also breaks launch) so @latest re-resolves. - Pin PLAYWRIGHT_BROWSERS_PATH so the build lands where the servers look. Verified: with chromium-1232 installed and the cache cleared, a standalone `@playwright/mcp@latest --browser chromium` server initializes and launches a browser with no error. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_012xoZvcd19p4WWYnPkLZkfn --- .claude/hooks/session-start.sh | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/.claude/hooks/session-start.sh b/.claude/hooks/session-start.sh index 9e9f7f50..f79fa746 100755 --- a/.claude/hooks/session-start.sh +++ b/.claude/hooks/session-start.sh @@ -131,18 +131,27 @@ fi # ── 4. Playwright MCP browser ───────────────────────────────────────────────── # The Playwright MCP servers (.mcp.json) launch with `--browser chromium`, which -# resolves to a chrome-for-testing build that is NOT baked into the image — so a -# fresh container can't drive a browser until it's fetched. Ensure it's present. +# resolves to Playwright's OWN pinned Chromium build (revision 1232 for the +# current @playwright/mcp), NOT Google's chrome-for-testing. Installing +# chrome-for-testing (the old value here) left `--browser chromium` with no +# executable, so every MCP server failed to launch a browser and none of the +# mcp__playwright__* tools registered. Install the correct build — both the full +# browser and the headless-shell the servers use. # Idempotent: install-browser is a fast no-op when the build is already there. # NB: route ALL its output to the log — this hook's STDOUT is reserved for the # additionalContext JSON below, and download progress bars would corrupt it. if command -v npx >/dev/null 2>&1; then - log "ensuring Playwright MCP browser (chrome-for-testing) is installed…" - if npx --yes @playwright/mcp@latest install-browser chrome-for-testing >>"$LOGFILE" 2>&1; then + log "ensuring Playwright MCP browser (chromium) is installed…" + # A stale/corrupt @playwright/mcp npx cache (seen as a broken 1.62.0-alpha with + # a missing playwright-core) also breaks server launch — clear it first so + # @latest re-resolves cleanly. + rm -rf /root/.npm/_npx 2>/dev/null || true + if PLAYWRIGHT_BROWSERS_PATH="${PLAYWRIGHT_BROWSERS_PATH:-/opt/pw-browsers}" \ + npx --yes @playwright/mcp@latest install-browser chromium >>"$LOGFILE" 2>&1; then log "Playwright MCP browser ready" - add "✅ Playwright browser: chrome-for-testing ready" + add "✅ Playwright browser: chromium ready" else - log "WARNING: failed to install chrome-for-testing for Playwright MCP" + log "WARNING: failed to install chromium for Playwright MCP" add "‼️ Playwright browser: install FAILED (browser-driving may not work)" fi else From 1817b3c78fe572aa3530142848a6185faaef5019 Mon Sep 17 00:00:00 2001 From: Michael Simmons Date: Sun, 19 Jul 2026 11:11:16 -0500 Subject: [PATCH 8/9] Give anonymous players one durable pid so sign-up collects every game MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously every WS connect minted a fresh pid and the client wiped its saved pid on game-end/leave, so an anonymous player fragmented across many pids. Sign-up only moved the player_stats aggregate for a single pid, and the profile's recent-games list (keyed on round_player.user_id) never transferred at all — so a player who played then signed up saw none of their games on the new account. Keep one durable anonymous identity and transfer the whole history: - Client preserves tensies_pid/tensies_token across games (clearGame() now clears only tensies_code); presents them on create/join; welcome no longer clobbers a saved identity; reconnect_token syncs the authoritative pid the server echoes back. - ws._adopt_identity re-adopts the presented pid on create/join only when its private reconnect token passes gamestore.verify_claim — the pid leaks to co-players via state_msg, so the token is the credential (same threat model as the existing claim fix). Anonymous sessions only. - register_verify re-attributes the claimed pid's full history — player_stats, round_player, sessions, events, rounds.winner_user_id (the recent-games list reads round_player.user_id) — matching scripts/reattribute_user.sql's table set, still gated behind the token. Verified: ruff + tsc clean; the transfer SQL moves every table row against the live schema (none orphaned); two-game browser run at 390x844 shows the pid surviving a leave and being re-adopted server-side on the next create; verify_claim refuses adoption without the correct token. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 8 +++---- server/auth.py | 32 ++++++++++++++++++++++---- server/ws.py | 21 +++++++++++++++++ static/js/net.js | 54 ++++++++++++++++++++++++++++++++++---------- static/js/session.js | 14 +++++++++++- static/js/types.js | 1 + 6 files changed, 108 insertions(+), 22 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 4327ee79..3c97ee66 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -94,7 +94,7 @@ sessions, ack_events, drop_tasks, pause_tasks # live asyncio objects, owned by t Games are destroyed when the last player disconnects. Distinct players write distinct hash fields (atomic `HSET`/`HINCRBY`, no contention), so simultaneous rolling stays parallel; the one contended write — crowning the round winner — is an atomic Lua compare-and-set (`try_finish_round`). A periodic **reaper** (`server/reaper.py`) is the cross-instance backstop for grace-drops / pause-caps whose owning instance died, and publishes the global active-games gauge (aggregate it with `max()` across instances, not `sum()`). -**Accounts / auth.** Passkey (WebAuthn) sign-up/sign-in lives in `server/auth.py` — a `/auth/*` router (register/login `options`+`verify`, `/auth/me`) backed by a Postgres `users`/`webauthn_credentials` schema via `server/db.py`. Sessions are JWTs (HS256); the client authenticates its WebSocket with the `auth` action, which rebinds `session.pid` to the account UUID. `main.py` calls `db.init()` before serving because auth needs Postgres even when telemetry is off — gameplay itself still degrades gracefully when the DB is absent (`db.available()`). Public read APIs hang off `server/routes.py`: `/api/profile/{username}`, `/api/game/{code}`, `/api/game/{code}/verify`, `/api/verify/{code}/{pid}/{roll_count}`, plus SPA shells for `/@{username}`, `/games/{code}`, `/signin`, and `/welcome`. +**Accounts / auth.** Passkey (WebAuthn) sign-up/sign-in lives in `server/auth.py` — a `/auth/*` router (register/login `options`+`verify`, `/auth/me`) backed by a Postgres `users`/`webauthn_credentials` schema via `server/db.py`. Sessions are JWTs (HS256); the client authenticates its WebSocket with the `auth` action, which rebinds `session.pid` to the account UUID. **Anonymous players keep one durable pid across games:** the client preserves `tensies_pid`/`tensies_token` when a game ends (only `tensies_code` is cleared) and presents them on the next `create`/`join`, where `ws._adopt_identity` re-adopts the pid if the private token verifies (`gamestore.verify_claim` — the pid alone leaks via `state_msg`, so the token is the credential). Because every game then shares one pid, sign-up transfers the player's **whole history** onto the account: `register_verify` re-attributes `player_stats`, `round_player`, `sessions`, `events`, and `rounds.winner_user_id` for that pid (the recent-games list reads `round_player.user_id`, so those rows must move, not just the aggregate) — the same table set as `scripts/reattribute_user.sql`, still gated behind the private-token claim. `main.py` calls `db.init()` before serving because auth needs Postgres even when telemetry is off — gameplay itself still degrades gracefully when the DB is absent (`db.available()`). Public read APIs hang off `server/routes.py`: `/api/profile/{username}`, `/api/game/{code}`, `/api/game/{code}/verify`, `/api/verify/{code}/{pid}/{roll_count}`, plus SPA shells for `/@{username}`, `/games/{code}`, `/signin`, and `/welcome`. Key env vars (`server/config.py`): `REDIS_URL`, `TELEMETRY_ENABLED`, `ALLOWED_ORIGINS` (WS origin allowlist), `METRICS_TOKEN`/`STATS_TOKEN` (bearer-gate `/metrics`+`/stats`), `WIDGET_TOKEN` (`?key=` gate for the `/api/widget` home-screen card; unset → 503), `MAX_GAMES`, `MAX_PLAYERS_PER_GAME`, `MAX_CONNECTIONS_PER_IP`, `CREATE_RATE_*`/`JOIN_RATE_*`, `MAX_WS_MESSAGE_BYTES`. Accounts add `JWT_SECRET`, `JWT_EXPIRY_DAYS`, `WEBAUTHN_RP_ID`, `WEBAUTHN_RP_NAME`, `WEBAUTHN_ORIGIN`; provably-fair rolling adds `ENABLE_DRAND_ROLLING`, `DRAND_BASE_URL`, `DRAND_CHAIN_HASH`, `DRAND_POLL_INTERVAL` (`server/drand.py`); the optional Discord notifier adds `DISCORD_ENABLED`, `DISCORD_BOT_TOKEN`, `DISCORD_CHANNEL_ID`, `DISCORD_PUBLIC_KEY`, `DISCORD_APPLICATION_ID`, `DISCORD_GUILD_ID` (`server/discord.py`); the nearby-games radar + place check-in add `DISCOVERY_ENABLED` (+ `DISCOVERY_RADIUS_M`/`DISCOVERY_DISTANCE_BUCKET_M`/`DISCOVERY_MAX_RESULTS`, `NEARBY_RATE_*`/`CHECKIN_RATE_*`) and, for the Google-backed picker, `PLACES_ENABLED`, `GOOGLE_MAPS_API_KEY`, `GOOGLE_APPLICATION_CREDENTIALS`, `GOOGLE_CLOUD_PROJECT` (+ `PLACES_RADIUS_M`/`PLACES_*_CACHE_TTL`, `PLACES_RATE_*`) — a game is discoverable only while checked in to a place (`server/places.py`, `server/routes.py`; see `.env.prod.example`); `APP_URL` sets the absolute og:image origin. Behind a trusted proxy, set `TRUST_PROXY_HEADERS`/`TRUSTED_PROXY_HOPS` so the per-IP caps read the real client from `X-Forwarded-For`. Security response headers are governed by `SECURITY_HEADERS` (CSP, on by default), `CSP_OVERRIDE`/`CSP_EXTRA_SCRIPT_SRC`/`CSP_EXTRA_CONNECT_SRC`/`CSP_EXTRA_IMG_SRC`, and the `HSTS_*` group (off in dev, on for HTTPS deploys) — see `server/security.py`. Asset serving is split on `FRONTEND_DIST` (see Cache-busting). @@ -265,8 +265,8 @@ maxDiffPixels:0; behaviour is unchanged except documented fixes.) | action | description | |--------|-------------| | `auth` | authenticate the session with a passkey JWT; payload: `token` (rebinds `session.pid` to the account UUID) | -| `create` | create new game; payload: `name` | -| `join` | join existing game; payload: `name`, `code` | +| `create` | create new game; payload: `name` (+ optional `player_id`, `token` — the client's durable anonymous identity, re-adopted via `verify_claim` so all its games share one pid) | +| `join` | join existing game; payload: `name`, `code` (+ optional `player_id`, `token`, same anon-identity continuity as `create`) | | `reconnect` | rejoin a held slot after a drop; payload: `player_id`, `game_code`, `token` (the private reconnect token) | | `start` | host starts the game (host only) | | `pause` | host-only toggle that freezes/unfreezes rolling for everyone | @@ -281,7 +281,7 @@ maxDiffPixels:0; behaviour is unchanged except documented fixes.) |------|-------------| | `welcome` | connection established; contains `player_id` | | `auth_ok` | auth succeeded; carries `username`, `user_id`, `player_id` | -| `reconnect_token` | private token (sent after create/join) the client stores to rejoin a held slot | +| `reconnect_token` | private token (sent after create/join) the client stores to rejoin a held slot; also carries the authoritative `player_id` the server bound the game to (it may have re-adopted the client's durable anon pid), which the client syncs to | | `state` | full game state snapshot | | `round_won` | state snapshot with `winner_name`; triggers overlay | | `game_ended` | host ended the game; carries `ended_by`, `round_num`, and a `players` map of `name`/`wins` | diff --git a/server/auth.py b/server/auth.py index 0241d982..e1121400 100644 --- a/server/auth.py +++ b/server/auth.py @@ -309,14 +309,36 @@ async def _insert_user(lp: str | None) -> None: transports or None, ) - # Data transfer: link old anonymous stats to the new account. - # legacy_pid is None here if the pid was already assigned (guard - # above) or lost the claim race — the row must not move twice. + # Data transfer: re-attribute the anonymous identity's whole history + # to the new account. legacy_pid is None here if the pid was already + # assigned (guard above) or lost the claim race — nothing must move + # twice. Because the client now keeps one durable pid across games + # (see ws._adopt_identity), this single pid pulls every game the + # player played onto the account — the profile's recent-games list + # reads round_player.user_id, so those rows must move too, not just + # the player_stats aggregate. Same table set as + # scripts/reattribute_user.sql; all user_id columns are plain TEXT + # (no FK), and a brand-new account has no rows to collide with. if legacy_pid: await con.execute( "UPDATE player_stats SET user_id = $1 WHERE user_id = $2", - user_id_str, - legacy_pid, + user_id_str, legacy_pid, + ) + await con.execute( + "UPDATE round_player SET user_id = $1 WHERE user_id = $2", + user_id_str, legacy_pid, + ) + await con.execute( + "UPDATE sessions SET user_id = $1 WHERE user_id = $2", + user_id_str, legacy_pid, + ) + await con.execute( + "UPDATE events SET user_id = $1 WHERE user_id = $2", + user_id_str, legacy_pid, + ) + await con.execute( + "UPDATE rounds SET winner_user_id = $1 WHERE winner_user_id = $2", + user_id_str, legacy_pid, ) # The claim is single-use: once the stats have moved, drop the token so the diff --git a/server/ws.py b/server/ws.py index f1e04c64..f5a4ded0 100644 --- a/server/ws.py +++ b/server/ws.py @@ -139,7 +139,25 @@ def _invite_qr(session: Session, code: str) -> str: return qr.qr_data_url(f"{base}/{code}") +async def _adopt_identity(session: Session, msg: dict) -> None: + """Anonymous identity continuity: if the client presents its durable pid + + that pid's private token, re-adopt the pid so every game the player plays + collects under one identity (and a later sign-up claims them all at once). + + The pid leaks to co-players via state_msg, so it can't authorise adoption on + its own — the token (never broadcast) proves ownership, exactly as at claim + time. Only for anonymous sessions; a signed-in session already IS an account. + """ + if session.user_id is not None: + return + prev_pid = (msg.get("player_id") or "").strip() + token = msg.get("token", "") + if prev_pid and prev_pid != session.pid and await gamestore.verify_claim(prev_pid, token): + session.pid = prev_pid + + async def handle_create(session: Session, msg: dict) -> None: + await _adopt_identity(session, msg) # Signed-in users use their account username as the player name. raw_name = session.username or msg.get("name") or "Player" name = sanitize_name(raw_name) or "Player" @@ -171,6 +189,7 @@ async def handle_create(session: Session, msg: dict) -> None: if session.photo: await gamestore.set_player_photo(code, session.pid, session.photo) await send(session.ws, {"type": "reconnect_token", "token": token, + "player_id": session.pid, "qr": _invite_qr(session, code)}) snap = await gamestore.snapshot(code) if snap: @@ -178,6 +197,7 @@ async def handle_create(session: Session, msg: dict) -> None: async def handle_join(session: Session, msg: dict) -> None: + await _adopt_identity(session, msg) join_code = (msg.get("code") or "").upper().strip() raw_name = session.username or msg.get("name") or "Player" name = sanitize_name(raw_name) or "Player" @@ -215,6 +235,7 @@ async def handle_join(session: Session, msg: dict) -> None: if session.photo: await gamestore.set_player_photo(join_code, session.pid, session.photo) await send(session.ws, {"type": "reconnect_token", "token": token, + "player_id": session.pid, "qr": _invite_qr(session, join_code)}) snap = await gamestore.snapshot(join_code) if snap: diff --git a/static/js/net.js b/static/js/net.js index 9017d83b..28052387 100644 --- a/static/js/net.js +++ b/static/js/net.js @@ -6,7 +6,7 @@ import { showWinner } from './overlays.js'; import { landing, showFor, showGameDetail, showLanding } from './router.js'; import { getAuthToken, isSignedIn, getAuthUser } from './auth.js'; import { - savePlayerId, saveReconnectToken, readSession, hasSession, clearSession, + savePlayerId, saveReconnectToken, readSession, hasSession, clearGame, } from './session.js'; import { state, resetRollState } from './state.js'; import { showScreen, showLoading, leaveLoading } from './transitions.js'; @@ -44,7 +44,7 @@ function send(action, extra = {}) { /** The saved session is unusable — forget it and land on landing with the reason. */ function expireSession() { state.reconnecting = false; - clearSession(); + clearGame(); state.currentState = null; leaveLoading(() => { showScreen('landing'); @@ -149,12 +149,24 @@ function currentName() { return input.value.trim() || state.randomNamePlaceholder; } +/** + * Anonymous identity-continuity payload for create/join: our durable pid + its + * private token, so the server re-adopts the same pid (see `verify_claim`) and + * every game we play collects under one identity. Empty for a signed-in session + * (the account UUID is the identity) or a brand-new client with nothing saved. + */ +function identityClaim() { + if (isSignedIn()) return {}; + const { playerId, token } = readSession(); + return playerId && token ? { player_id: playerId, token } : {}; +} + /** Create a new game as `currentName()`. */ export function createGame() { const name = currentName(); state.pendingOrigin = 'landing'; showLoading('Creating game…'); - connectWs(() => send('create', { name })); + connectWs(() => send('create', { name, ...identityClaim() })); } /** @@ -167,7 +179,7 @@ export function joinWithCode(code, origin = 'join') { const name = currentName(); state.pendingOrigin = origin; showLoading('Joining game…'); - connectWs(() => send('join', { name, code })); + connectWs(() => send('join', { name, code, ...identityClaim() })); } /** Join the game whose code is in the join form. */ @@ -208,13 +220,14 @@ export function startGame() { * Leave the lobby without playing and return to landing. We send an explicit * `leave` frame *before* closing so the server drops us with no grace hold and * the roster updates for everyone immediately (a plain close would leave us in - * the list for the full reconnect grace). Order matters: clearSession() runs + * the list for the full reconnect grace). Order matters: clearGame() runs * before the close so handleWsClose() doesn't read it as a dropped connection - * and reconnect us straight back in. + * and reconnect us straight back in. (The durable pid/token survive — only the + * game pointer is forgotten.) */ export function leaveGame() { send('leave'); // ask the server to drop us now, while the socket is still open - clearSession(); + clearGame(); state.reconnecting = false; state.currentState = null; state.gameCode = null; @@ -257,10 +270,20 @@ function handleMessage(msg) { case 'ping': send('pong', { t: msg.t }); return; - case 'welcome': - state.myId = msg.player_id; - savePlayerId(msg.player_id); + case 'welcome': { + // Keep our durable anonymous identity if we hold one — we present it on + // create/join for the server to re-adopt (so all our games share one + // pid). Only a brand-new client takes the server-assigned pid. The + // authoritative pid is confirmed back on `reconnect_token`. + const saved = readSession(); + if (saved.playerId && saved.token) { + state.myId = saved.playerId; + } else { + state.myId = msg.player_id; + savePlayerId(msg.player_id); + } return; + } case 'auth_ok': state.authUsername = msg.username; state.authUserId = msg.user_id; @@ -271,6 +294,13 @@ function handleMessage(msg) { } return; case 'reconnect_token': + // The server echoes the authoritative pid it bound this game to — it may + // have re-adopted our durable pid, or (token expired) kept a fresh one. + // Sync to it so client and server never disagree on who "me" is. + if (msg.player_id) { + state.myId = msg.player_id; + savePlayerId(msg.player_id); + } saveReconnectToken(msg.token); if (msg.qr) state.qr = msg.qr; // inline invite QR — cache for the stamp return; @@ -347,7 +377,7 @@ function handleMessage(msg) { // up on the next game's board. Reset it as the game tears down. /** @type {import('./components/game-screen.js').GameScreen} */ (byId('game')).closeMenu(); const code = state.gameCode; - clearSession(); + clearGame(); state.currentState = null; // Brief delay so the telemetry writer can flush to Postgres before // the game-detail screen fetches the API. @@ -375,7 +405,7 @@ function handleError(msg) { // Mirrors the game_ended path above. /** @type {import('./components/game-screen.js').GameScreen} */ (byId('game')).closeMenu(); stopPauseTick(); - clearSession(); + clearGame(); state.currentState = null; state.reconnecting = false; leaveLoading(() => { diff --git a/static/js/session.js b/static/js/session.js index cceb501d..1e72da34 100644 --- a/static/js/session.js +++ b/static/js/session.js @@ -44,7 +44,19 @@ export function hasSession() { return Boolean(playerId && gameCode); } -/** Forget the saved session (game ended, or reconnect window expired). */ +/** + * Forget the current game but keep the durable anonymous identity + * (`pid` + `token`). Called when a game ends / we leave / a reconnect window + * lapses: there's no live slot to resume, but the pid must survive so the + * player's *next* game shares one identity — and a later sign-up can collect + * every game they played under the new account. Re-adoption of the pid on the + * next create/join is authenticated by the token (see server `verify_claim`). + */ +export function clearGame() { + localStorage.removeItem(GAME_CODE_KEY); +} + +/** Forget everything, including the durable identity (e.g. sign-out). */ export function clearSession() { localStorage.removeItem(PLAYER_ID_KEY); localStorage.removeItem(GAME_CODE_KEY); diff --git a/static/js/types.js b/static/js/types.js index 4517e835..b3b7368a 100644 --- a/static/js/types.js +++ b/static/js/types.js @@ -87,6 +87,7 @@ * @typedef {object} ReconnectTokenMessage * @property {'reconnect_token'} type * @property {string} token + * @property {string} [player_id] Authoritative pid the server bound this game to (it may have re-adopted the client's durable anonymous pid). * @property {string} [qr] Inline invite QR (data URL) sent alongside the token after create/join. */ From 49418f9d1679c9e323cec117232b37b2820bb36e Mon Sep 17 00:00:00 2001 From: Michael Simmons Date: Sun, 19 Jul 2026 12:20:00 -0500 Subject: [PATCH 9/9] Stop the game top bar flashing/fading when the menu toggles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frosted top bar put backdrop-filter on the .game-topbar element, making it a stacking context that trapped the title row (logo + user pill) below the z-index:50 menu overlay. The design freed the title row by toggling that backdrop-filter off on menu-open — and that instant toggle, racing the overlay's 0.28s fade, is what flashed the live board through the header on open and dimmed the logo/pill on close. Move the frosted surface onto a ::before (behind the content, z-index:-1) so the topbar is never a stacking context. The title row's z-index:100 now sits above the overlay permanently — no toggle, no flip, so nothing in the header flashes or fades in either direction. Drops the whole menu-open backdrop-filter rule. Verified at 390x844: frosted bar intact at rest; logo/close crisp on the dark overlay when open; scoreboard still drops under the menu; hamburger stays tappable; the topbar never becomes a stacking context during the close (stackingFlips=0). Rest-state pixels unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- static/css/shell.css | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/static/css/shell.css b/static/css/shell.css index 8ac9acf8..239afc29 100644 --- a/static/css/shell.css +++ b/static/css/shell.css @@ -12,8 +12,21 @@ } /* Game screen only (not the pre-game app-header): one frosted surface behind - the whole top bar — the title row and players bar sit on it transparently. */ - .game-topbar:not(.app-header) { + the whole top bar — the title row and players bar sit on it transparently. + The surface lives on a ::before (painted BEHIND the content via z-index:-1) + rather than on the topbar element itself, so the topbar never becomes a + stacking context. That keeps the title row's z-index:100 permanently above + the z-index:50 menu overlay — the hamburger stays tappable AND the overlay + never paints over the logo/user pill, so nothing in the header flashes or + fades when the menu toggles. Only the players bar (z-auto) drops under the + overlay. (Putting the blur on the topbar itself made it a stacking context + that trapped the title row under the overlay; toggling it off on menu-open + to free the title row is what caused the flash/fade — no toggle now.) */ + .game-topbar:not(.app-header)::before { + content: ''; + position: absolute; + inset: 0; + z-index: -1; background: var(--color-panel); backdrop-filter: blur(8px); -webkit-backdrop-filter: blur(8px); @@ -74,24 +87,14 @@ -webkit-backdrop-filter: blur(8px); } - /* Game screen only: the frosted surface lives on .game-topbar, so the title - row sits on it transparently (no double blur). */ + /* Game screen only: the frosted surface lives on the topbar's ::before, so the + title row sits on it transparently (no double blur). */ .game-topbar:not(.app-header) .topbar-title-row { background: transparent; backdrop-filter: none; -webkit-backdrop-filter: none; } - /* Menu open: drop the parent's backdrop-filter so it stops being a stacking - context — the title row's z-index:100 re-escapes above the z-index:50 menu - (hamburger stays tappable). The header stays transparent, so the dark menu - overlay shows straight through it; the players bar drops under the menu. */ - .game-screen:has(.game-menu.open) .game-topbar:not(.app-header) { - background: transparent; - backdrop-filter: none; - -webkit-backdrop-filter: none; - } - .game-title { display: flex; align-items: center;