Reject version-mismatched joins and prompt refresh on homepage after deploys#4672
Reject version-mismatched joins and prompt refresh on homepage after deploys#4672evanpelle wants to merge 1 commit into
Conversation
…loys The sim runs on each client and is only deterministic when every client in a game executes identical code. After a deploy, tabs opened before the LB flip still run the old bundle but connect (per-connection routing) to the new deployment, join fresh games alongside new-bundle clients, and desync. Two mechanisms close this: - Join gate: join/rejoin messages carry the client bundle's gitCommit; the worker rejects mismatches (missing = pre-feature = stale) with a typed version_mismatch error before token verification. The client answers the error with an update alert and reloads to pull the new bundle. - Homepage drain: the lobby feed's full snapshot advertises the server's commit. The homepage lobby socket (only active outside games) fires once on mismatch, shows the update alert, and reloads, so stale tabs migrate to the new version between games instead of at their next failed join. Dev is unaffected (both sides use GIT_COMMIT=DEV). An e2e test spawns the real server and drives the actual join handler and lobby feed over real WebSockets (npm run test:e2e; opt-in since it binds ports 3000/3001). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WalkthroughThis change adds commit-based version checks for game joins and public lobby updates. The server rejects mismatched clients, while the client displays an update alert and reloads. Schemas, transport messages, lobby snapshots, localization, and unit/e2e coverage are updated. ChangesDeploy version gate
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant Worker
participant WorkerLobbyService
participant PublicLobbySocket
Browser->>Worker: join with gitCommit
Worker->>Worker: compare client and server commits
Worker-->>Browser: version_mismatch and close 1002
WorkerLobbyService-->>PublicLobbySocket: full snapshot with gitCommit
PublicLobbySocket-->>Browser: update available alert
Browser->>Browser: reload page
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)package.jsonTraceback (most recent call last): resources/lang/en.jsonTraceback (most recent call last): 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/Schemas.ts`:
- Around line 253-256: Update the gitCommit schema fields in the identified
schema locations to remain optional while rejecting empty strings when provided,
preserving the existing 64-character maximum. Add tests covering rejection of an
empty gitCommit value and acceptance of omitted and non-empty values.
In `@src/server/Worker.ts`:
- Around line 454-472: The version-mismatch branch in the worker currently sends
the typed error but closes with a code that Transport maps to a generic
connection-refused alert. Update the handling around ServerEnv.gitCommit() and
the clientMsg version check so the client-only 1002 close reason is recognized
as the version-mismatch path, preventing connection_refused handling while
preserving ClientGameRunner’s version_mismatch behavior.
In `@tests/e2e/VersionCheck.e2e.test.ts`:
- Around line 98-123: The join helper currently only exercises type "join";
extend it with a rejoin variant that sends type "rejoin" using a generated
token. Add E2E coverage for mismatched and missing gitCommit values, asserting
the typed version_mismatch error and WebSocket close code 1002, while preserving
the existing join coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7ae9a67c-4a07-4e10-b264-e0f7d362d74b
📒 Files selected for processing (12)
package.jsonresources/lang/en.jsonsrc/client/ClientGameRunner.tssrc/client/GameModeSelector.tssrc/client/LobbySocket.tssrc/client/Transport.tssrc/core/Schemas.tssrc/server/Worker.tssrc/server/WorkerLobbyService.tstests/ClientVersionSchemas.test.tstests/e2e/VersionCheck.e2e.test.tstests/setup.ts
| // Build commit of the serving deployment. Clients on the homepage compare | ||
| // it to their own bundle's commit to detect that a new version deployed | ||
| // and prompt a refresh. Optional so clients tolerate older servers. | ||
| gitCommit: z.string().max(64).optional(), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
fd -a 'Schemas\.ts|Worker\.ts|ClientVersionSchemas\.test\.ts' . | sed 's#^\./##' | sort
echo "== Schemas outline =="
ast-grep outline src/core/Schemas.ts --view compact || true
echo "== Schemas relevant lines =="
sed -n '230,270p' src/core/Schemas.ts
echo "----"
sed -n '880,905p' src/core/Schemas.ts
echo "== Worker relevant lines =="
sed -n '430,475p' src/server/Worker.ts
echo "== tests outline/contents =="
wc -l tests/ClientVersionSchemas.test.ts
sed -n '1,220p' tests/ClientVersionSchemas.test.ts
echo "== string validator usage around version fields =="
rg -n "gitCommit|deployed|commit|min\\(|max\\(" src/core/Schemas.ts tests/ClientVersionSchemas.test.tsRepository: openfrontio/OpenFrontIO
Length of output: 10894
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Worker gitCommit implementation area =="
rg -n -C 4 "gitCommit\\(|version_mismatch|Version mismatch" src/server/Worker.ts
echo "== deterministic string validator probe =="
node - <<'JS'
const zod = require('zod');
const schema = zod.z.string().max(64).optional();
for (const value of [undefined, '', 'abc', 'a'.repeat(64), 'a'.repeat(65)]) {
const result = schema.safeParse(value);
console.log(JSON.stringify({ value, success: result.success, data: result.success ? result.data : undefined }));
}
JSRepository: openfrontio/OpenFrontIO
Length of output: 1493
Reject empty build identifiers.
z.string().max(64).optional() accepts "", and the worker compares clientMsg.gitCommit directly with ServerEnv.gitCommit(). If the server is misconfigured with an empty commit, a malformed client can bypass the version gate. Keep the field optional, but require a non-empty value when present, and add empty-string tests.
Proposed schema change
- gitCommit: z.string().max(64).optional(),
+ gitCommit: z.string().min(1).max(64).optional(),Also applies to: 886-890, 899-900
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/Schemas.ts` around lines 253 - 256, Update the gitCommit schema
fields in the identified schema locations to remain optional while rejecting
empty strings when provided, preserving the existing 64-character maximum. Add
tests covering rejection of an empty gitCommit value and acceptance of omitted
and non-empty values.
| // The sim is deterministic only when every client in a game runs | ||
| // identical code, so a client built from a different commit (e.g. a | ||
| // tab left open across a deploy) would desync the game. Reject it | ||
| // with a typed error the client answers by refreshing. A missing | ||
| // commit means a pre-feature bundle, which is stale by definition. | ||
| if (clientMsg.gitCommit !== ServerEnv.gitCommit()) { | ||
| log.info("rejecting version-mismatched client", { | ||
| gameID: clientMsg.gameID, | ||
| clientCommit: clientMsg.gitCommit, | ||
| }); | ||
| ws.send( | ||
| JSON.stringify({ | ||
| type: "error", | ||
| error: "version_mismatch", | ||
| } satisfies ServerErrorMessage), | ||
| ); | ||
| ws.close(1002, "Version mismatch"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tracked files matching Worker/ClientGameRunner/Transport =="
git ls-files | rg '(^|/)Worker\.ts$|ClientGameRunner\.ts$|Transport\.ts$|ServerErrorMessage' || true
echo
echo "== search version_mismatch and close handling =="
rg -n 'version_mismatch|onclose|on Close|close\(|1002|connection refused|refresh|reload|alert|error.*version' -S src || trueRepository: openfrontio/OpenFrontIO
Length of output: 28139
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Worker.ts relevant section =="
sed -n '430,485p' src/server/Worker.ts | nl -ba -v 430
echo
echo "== ClientGameRunner.ts version_mismatch and socket initialization section =="
sed -n '200,250p' src/client/ClientGameRunner.ts | nl -ba -v 200
echo
echo "== Transport.ts onclose section =="
sed -n '380,415p' src/client/Transport.ts | nl -ba -v 380
echo
echo "== deterministic ordering check from source snippets =="
python3 - <<'PY'
from pathlib import Path
import re
worker = Path("src/server/Worker.ts").read_text()
client = Path("src/client/ClientGameRunner.ts").read_text()
transport = Path("src/client/Transport.ts").read_text()
version_send = "JSON.stringify({\n type: \"error\",\n error: \"version_mismatch\""
if version_send in worker:
line = worker.splitlines().index(version_send.splitlines()[0]) + 1
print("Worker sends version_mismatch:", line)
error_handler = "message.error === \"version_mismatch\""
if error_handler in client:
line = client.splitlines().index(error_handler.splitlines()[0]) + 1
nearby = client.splitlines()[max(0,line+12-1):min(len(client.splitlines()),line+18)]
print("ClientGameRunner handles version_mismatch:", line)
print(" nearby:", nearby)
onclose = re.search(r"this\.socket\.onclose\s*=\s*\(\w+:\s*CloseEvent\)\s*=>\s*\{", transport)
if onclose:
print("Transport attaches socket onclose:", onclose.start())
onclose_line = transport[:onclose.start()].count("\n")+1
print("Transport onclose handler starts:", onclose_line)
# find next return or function after onclose assignment
PYRepository: openfrontio/OpenFrontIO
Length of output: 234
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Worker.ts relevant section =="
sed -n '430,485p' src/server/Worker.ts
echo
echo "== ClientGameRunner.ts version_mismatch handler =="
sed -n '200,255p' src/client/ClientGameRunner.ts
echo
echo "== Transport.ts onclose handler =="
sed -n '380,415p' src/client/Transport.ts
echo
echo "== Read-only source ordering probe =="
python3 - <<'PY'
from pathlib import Path
worker = Path("src/server/Worker.ts").read_text()
client = Path("src/client/ClientGameRunner.ts").read_text()
transport = Path("src/client/Transport.ts").read_text()
print({
"version_mismatch_text_in_worker": "error: \"version_mismatch\"" in worker,
"version_mismatch_handler_in_client": "message.error === \"version_mismatch\"" in client,
"socket_onclose_handler_in_transport": "this.socket.onclose" in transport and "event.code === 1002" in transport,
})
print("transport first 1000 bytes includes onclose setup:", "this.socket.onclose" in transport[:1000])
PYRepository: openfrontio/OpenFrontIO
Length of output: 5318
Handle version mismatches in the message path, not the 1002 close path.
The worker sends version_mismatch and still closes the socket with 1002; Transport.ts maps 1002 to connection_refused, while ClientGameRunner handles version_mismatch separately. Use a client-only 1002 reason for the typed error or branch so version mismatches do not also show the generic refused-connection alert.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/Worker.ts` around lines 454 - 472, The version-mismatch branch in
the worker currently sends the typed error but closes with a code that Transport
maps to a generic connection-refused alert. Update the handling around
ServerEnv.gitCommit() and the clientMsg version check so the client-only 1002
close reason is recognized as the version-mismatch path, preventing
connection_refused handling while preserving ClientGameRunner’s version_mismatch
behavior.
| function join(gitCommit: string | undefined): Promise<JoinResult> { | ||
| return new Promise((resolve, reject) => { | ||
| const ws = new WebSocket(WORKER_WS); | ||
| const messages: any[] = []; | ||
| const timeout = setTimeout(() => { | ||
| ws.terminate(); | ||
| reject( | ||
| new Error(`no join verdict in 15s: ${JSON.stringify(messages)}`), | ||
| ); | ||
| }, 15_000); | ||
| const finish = (closeCode?: number) => { | ||
| clearTimeout(timeout); | ||
| resolve({ messages, closeCode }); | ||
| }; | ||
| ws.on("open", () => { | ||
| ws.send( | ||
| JSON.stringify({ | ||
| type: "join", | ||
| gameID, | ||
| token: randomUUID(), // dev servers accept a bare persistent ID | ||
| username: "TestPlayer", | ||
| clanTag: null, | ||
| turnstileToken: null, | ||
| ...(gitCommit === undefined ? {} : { gitCommit }), | ||
| }), | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover rejoin in the E2E version gate.
This helper hardcodes type: "join", so the suite never verifies that a mismatched or missing commit on a rejoin reaches the same version_mismatch rejection path. Add a rejoin variant; because validation occurs before token verification, it can use a generated token and assert the typed error plus close code 1002.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn, type ChildProcess } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/e2e/VersionCheck.e2e.test.ts` around lines 98 - 123, The join helper
currently only exercises type "join"; extend it with a rejoin variant that sends
type "rejoin" using a generated token. Add E2E coverage for mismatched and
missing gitCommit values, asserting the typed version_mismatch error and
WebSocket close code 1002, while preserving the existing join coverage.
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
1 participant
Problem
Players desync after deploys. The sim runs on each client and is only deterministic when every client in a game executes identical code — but after the load balancer flips to a new deployment, tabs opened before the flip still run the old bundle while their new connections (joins always open a fresh WebSocket; the lobby-list socket reconnects on any blip) land on the new deployment. An old-bundle client joins a new game alongside new-bundle clients and desyncs it.
The bundle chain itself is correctly pinned (hashed worker chunk referenced by hashed main bundle) — the skew is across clients in the same game, not within one client.
Fix
Join gate —
join/rejoinmessages now carry the client bundle'sgitCommit. The worker compares it against its ownGIT_COMMITbefore token verification and rejects mismatches with a typedversion_mismatcherror; a missing commit (pre-feature bundle) counts as a mismatch. The client responds to that error with an update alert and reloads, pulling the fresh shell and bundle.Homepage drain — the lobby feed's
fullsnapshot advertises the server's commit. The homepage lobby socket compares it to the bundle's commit and, on mismatch, shows the update alert and reloads. This socket only runs on the homepage (Main.ts stops it when a game is joined), so the reload can never interrupt a game. Stale tabs migrate to the new version between games instead of at their next failed join.Dev is unaffected: both sides use
GIT_COMMIT=DEV, so the check passes. The schema field is a lenient bounded string (not the strict 40-hexGitCommitSchema) so an unusual commit value degrades to a rejection rather than a parse failure.Deploy note
The first deploy carrying this change hard-cuts tabs opened before it: their joins are rejected, but old bundles lack the reload handler, so they see the generic connection-error modal. From the second deploy on, stale tabs get the alert-and-refresh flow.
Testing
tests/ClientVersionSchemas.test.ts— schema round-trips for the new fieldtests/e2e/VersionCheck.e2e.test.ts(npm run test:e2e, opt-in — binds real ports 3000/3001) — spawns the real server (master + worker) and drives the actual join handler over real WebSockets: mismatched/missing commit →version_mismatch+ close 1002, matching commit →lobby_info,/lobbiesfull snapshot carries the server commit🤖 Generated with Claude Code