Fix bugs surfaced by full-codebase review (+ ops/test hardening)#77
Open
radiantnode wants to merge 9 commits into
Open
Fix bugs surfaced by full-codebase review (+ ops/test hardening)#77radiantnode wants to merge 9 commits into
radiantnode wants to merge 9 commits into
Conversation
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xoZvcd19p4WWYnPkLZkfn
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012xoZvcd19p4WWYnPkLZkfn
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012xoZvcd19p4WWYnPkLZkfn
…use watchdog 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xoZvcd19p4WWYnPkLZkfn
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 `/<CODE>` 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xoZvcd19p4WWYnPkLZkfn
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xoZvcd19p4WWYnPkLZkfn
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xoZvcd19p4WWYnPkLZkfn
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Full-codebase review of
tensies, followed by fixing everything it surfaced and verifying each fix by exercising the running app. 7 commits, 30 files (+545/−102).Backend fixes (highest-impact first)
fanout.py): broadcasts now deliver concurrently with a per-socket timeout, so one backpressured client can't wedge the single subscriber task and stall every game on the instance. Verified: with a fake stuck socket,_deliverreturns bounded (~5s) instead of hanging forever; the healthy peer is still served and the stuck socket is reaped.telemetry/writer.py): each rollup handler runs in its own savepoint, so one SQL error no longer aborts the whole ~500-event batch (raw event log included); plus a graceful drain on shutdown. Verified: an injected failing handler no longer rolls back sibling events. Also fixes theavg_dtweighting and a leader-tie edge.ws.py): connection setup moved inside thetry/finally, so a failedwelcomesend can't leak the per-IP counter / session / pinger. Verified: conn counter balances across open/close cycles.broadcast.py+gamestore.pyLua): a voluntary leave now actually removes the slot even while paused — the_DROPLua had its own paused guard that made the first attempt a no-op. Verified live: leaver drops 2→1 with the roster broadcast; solo-leave deletes the game and cancels the pause watchdog.JWT_SECRETfail-closed in prod builds; HSTS decoupled from the CSP master switch (config.py,security.py).place_idvalidation atplaces.resolve()before it reaches a Google URL / Redis key.rate_allowINCR+EXPIRE made atomic (Lua) — no TTL-less permanent lockout on a crash.Frontend fixes
sheet.js: only a click on the dialog itself counts as a backdrop tap, so the touch guard's synthesized(0,0)click can't slam the sheet shut mid-use. Verified in-browser.overlays.js/router.js: cancellable pause-overlay close so a quick re-pause doesn't strand a non-host.net.js/game-render.js: close the menu + stop the pause tick on fatal teardown; re-sync the paused roll button after an in-game error.profile-screen.js: assign the recent-games click handler instead of stacking one per render.game-detail-screen.js: generation guard so overlapping verification runs don't write into a newer view.Security follow-up
legacy_pidstat claim: registration used to transfer an anonymous player's stats from a client-supplied pid with no ownership proof (pids leak to co-players viastate_msg). Now the private reconnect-token hash is kept in a TTL'd Redis key at create/join and required at registration. Verified: correct token transfers; wrong/empty/pid-as-token all refused; single-use.Ops / infra
.dockerignore: stop bakingops/secrets/(the prodMETRICS_TOKEN) into the image.prometheus.prod.yml: DNS-SD so scaledwebreplicas are all scraped.session-starthook: trust the egress-proxy CA at runtime too (so drand/discord/places work), and install the correct Playwright MCP browser (chromium, notchrome-for-testing).Verification
25/25, plus targeted live tests for every fix above.tscclean; full pixel suite run A/B (HEAD vs base) → zero regressions from these changes (pre-existing baseline drift is unrelated); in-browser drives of the key flows.docs/test-runs/game/.🤖 Generated with Claude Code
https://claude.ai/code/session_012xoZvcd19p4WWYnPkLZkfn
Generated by Claude Code