Discord(et al.) embedded URLs #2740
Conversation
|
Warning Rate limit exceeded@ryanbarlow97 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 17 minutes and 22 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplace hash-based join/navigation with canonical /game/{id} paths and a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Browser as Browser (user / crawler)
participant Server as Master
participant Worker as Worker (lobby API)
Browser->>Server: GET /game/:gameId
Server->>Server: check User-Agent & per-IP limiter
Server->>Worker: fetchLobbyInfo(gameId) (abortable, ~1500ms)
alt lobby info returned
Worker-->>Server: lobby data
else timeout or error
Worker-->>Server: no lobby data
end
Server->>Worker: fetchPublicGameInfo(gameId) (abortable, ~1500ms)
alt public info returned
Worker-->>Server: public metadata
else timeout or error
Worker-->>Server: no public metadata
end
alt Bot detected
Server->>Server: buildPreview(meta) & renderPreview()
Server-->>Browser: pre-rendered OG HTML + cache headers
else Human visitor
Server-->>Browser: serve SPA index.html (no-cache)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 0
🧹 Nitpick comments (2)
src/client/JoinPrivateLobbyModal.ts (1)
139-161: Nice refactor using URL API for robust parsing.The structured URL parsing handles multiple URL shapes gracefully. The try-catch ensures malformed URLs don't crash the flow.
One small observation: the regex
/join\/([A-Za-z0-9]{8})/at line 153 would also match longer strings like/join/ABCD1234extra. Consider anchoring it with$or word boundary if you want stricter matching:- const match = url.pathname.match(/join\/([A-Za-z0-9]{8})/); + const match = url.pathname.match(/\/join\/([A-Za-z0-9]{8})$/);That said, since callers likely validate with
ID.safeParse()anyway, this is a minor nitpick.src/server/Master.ts (1)
173-179: Consider escaping URLs in HTML attributes.The
meta.redirectUrlandjoinIdare inserted into thecontentattribute and script without escaping. WhileredirectUrlis built fromorigin(which comes from request headers), a malformed host header could potentially break the HTML or introduce injection.Since
joinIdis already validated byID.safeParse(), it's safe. ForredirectUrl, you might want to validate or escape the origin:+const escapeAttr = (value: string): string => + value.replace(/"/g, """).replace(/</g, "<").replace(/>/g, ">"); const refreshTag = botRequest ? "" - : `<meta http-equiv="refresh" content="0; url=${meta.redirectUrl}#join=${joinId}">`; + : `<meta http-equiv="refresh" content="0; url=${escapeAttr(meta.redirectUrl)}#join=${joinId}">`;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/client/AccountModal.tssrc/client/HostLobbyModal.tssrc/client/JoinPrivateLobbyModal.tssrc/client/Main.tssrc/server/Master.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/client/AccountModal.tssrc/client/JoinPrivateLobbyModal.tssrc/server/Master.tssrc/client/Main.tssrc/client/HostLobbyModal.ts
📚 Learning: 2025-10-21T20:06:04.823Z
Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Applied to files:
src/client/HostLobbyModal.ts
🧬 Code graph analysis (2)
src/server/Master.ts (2)
src/core/Schemas.ts (2)
GameInfo(130-136)ID(208-211)src/core/configuration/DefaultConfig.ts (1)
workerPort(207-209)
src/client/Main.ts (1)
src/core/Schemas.ts (1)
ID(208-211)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy to openfront.dev
🔇 Additional comments (11)
src/client/HostLobbyModal.ts (1)
831-834: LGTM!The clipboard URL now uses the path-based format
/join/{lobbyId}, which is consistent with the PR's goal of transitioning to path-based routing. SincelobbyIdis an 8-character alphanumeric string (per theIDschema), no URL encoding is needed for the path segment.src/server/Master.ts (5)
52-57: Verify proxy trust configuration aligns with deployment.
requestOriginreadsx-forwarded-protodirectly from headers. Whileapp.set("trust proxy", 3)is configured at line 282, ensure your deployment actually has exactly 3 trusted proxy hops. Misconfiguration could allow clients to spoof the protocol/host.
126-127: Redirect URL uses query param format.The
redirectUrlis built as${origin}/?join=${gameID}, using query param style. The rest of the PR moves to path-based/join/{id}format. Is this intentional for backwards compatibility with the client'shandleUrlwhich checks query params first?If intentional, a brief comment explaining the choice would help future readers.
59-78: Good use of AbortController for fetch timeout.The 1.5 second timeout prevents the preview page from hanging if the worker is unresponsive. Clean pattern with proper cleanup via
clearTimeout.
220-245: Preview serves 404 status when lobby not found, but still returns HTML.Returning a 404 status with a preview page is unconventional. Bots might not index the page, but users still see a functional redirect. If that's the intent (graceful degradation), consider adding a comment. Otherwise, you might want to return a different response for non-existent lobbies.
247-252: Route handler looks good.The
/join/:gameIdroute delegates toserveJoinPreviewand properly catches errors with a 500 response. Clean error logging.src/client/AccountModal.ts (1)
145-152: LGTM!The navigation now uses path-based routing with
encodeURIComponentfor safety. The hash fragment is kept for the client-side join handler. DispatchinghashchangeensuresMain.tsprocesses the navigation.src/client/Main.ts (4)
612-634: Clean extraction logic with proper validation.The function checks multiple URL sources (query, path, hash) in a sensible priority order. Using
ID.safeParsefrom zod ensures the extracted value matches the expected 8-character alphanumeric format. Good defensive programming.
636-643: Good URL normalization helper.
updateJoinUrlForShareensures the URL is in the canonical format before sharing. UsingreplaceStateavoids polluting browser history with duplicate entries.
479-485: LGTM on the refactored join handling.The flow now extracts the join code, updates the URL to canonical format, opens the modal, and returns early. Clear and readable.
603-607: URL format now matches the canonical path-based pattern.The
pushStatecall uses/join/${gameID}#join=${gameID}, aligning with the rest of the PR. The hash fragment provides backwards compatibility for client-side routing.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/server/Master.ts (1)
284-289: Add rate limiting to prevent abuse.This route performs file system access but is registered before the rate limit middleware (lines 320-325), making it vulnerable to abuse. Move this route after the rate limiting setup.
🔎 Proposed fix
Move the route registration to after the rate limit middleware:
app.use(express.json()); - -app.get("/join/:gameId", (req, res) => { - serveJoinPreview(req, res, req.params.gameId).catch((error) => { - log.error("failed to render join preview", { error }); - res.status(500).send("Unable to render lobby preview"); - }); -}); - app.use( express.static(path.join(__dirname, "../../static"), { maxAge: "1y", // Set max-age to 1 year for all static assetsThen add after line 325:
max: 20, // 20 requests per IP per second }), ); + +app.get("/join/:gameId", (req, res) => { + serveJoinPreview(req, res, req.params.gameId).catch((error) => { + log.error("failed to render join preview", { error }); + res.status(500).send("Unable to render lobby preview"); + }); +});
🧹 Nitpick comments (1)
src/client/Main.ts (1)
608-623: Consider avoiding hardcoded length in the regex.The regex
/^\/join\/([A-Za-z0-9]{8})/hardcodes the 8-character length. If theIDschema's length changes in the future, this regex could become inconsistent. SinceID.safeParse()validates afterward, this provides a safety net, but the regex could become misleading.🔎 Option: Make the regex more flexible
private extractJoinCodeFromUrl(): string | null { const searchParams = new URLSearchParams(window.location.search); const joinFromQuery = searchParams.get("join"); if (joinFromQuery && ID.safeParse(joinFromQuery).success) { return joinFromQuery; } const pathMatch = window.location.pathname.match( - /^\/join\/([A-Za-z0-9]{8})/, + /^\/join\/([A-Za-z0-9]+)/, ); if (pathMatch && ID.safeParse(pathMatch[1]).success) { return pathMatch[1]; } return null; }This makes the regex more resilient to schema changes while still relying on
ID.safeParse()for validation.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/client/AccountModal.tssrc/client/Main.tssrc/server/Master.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/client/AccountModal.tssrc/server/Master.tssrc/client/Main.ts
🧬 Code graph analysis (2)
src/server/Master.ts (2)
src/core/Schemas.ts (2)
GameInfo(130-136)ID(208-211)src/core/configuration/DefaultConfig.ts (1)
workerPort(207-209)
src/client/Main.ts (1)
src/core/Schemas.ts (1)
ID(208-211)
🪛 GitHub Check: 🔍 ESLint
src/server/Master.ts
[failure] 191-191:
Prefer using nullish coalescing operator (??) instead of a logical or (||), as it is a safer operator.
🪛 GitHub Check: CodeQL
src/server/Master.ts
[failure] 284-289: Missing rate limiting
This route handler performs a file system access, but is not rate-limited.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy to openfront.dev
🔇 Additional comments (11)
src/client/Main.ts (3)
479-485: LGTM! Clean URL extraction and early return.The flow correctly extracts the lobby ID, normalizes the URL for sharing, opens the modal, and returns early to prevent further processing. The logic is clear and handles both query parameter and path-based joins.
625-632: LGTM! Proper URL normalization without history pollution.The method correctly uses
replaceStateto normalize the URL to/join/{lobbyId}format without adding unnecessary entries to the browser history. The guard condition prevents redundant operations.
603-603: The server is already configured correctly for path-based URLs. The/join/:gameIdroute is defined atsrc/server/Master.tsline 284, and a catch-all SPA fallback route at line 532 servesindex.htmlfor all unmatched paths. This means direct navigation and page refresh to/join/*paths will work as expected.src/client/AccountModal.ts (1)
147-148: Clean URL construction looks good.The URL is properly constructed using
encodeURIComponentfor the path segment, creating a clean/join/{gameId}format that aligns with the PR's path-based URL standardization.src/server/Master.ts (7)
39-45: LGTM!The HTML escaping correctly handles the five key characters needed to prevent XSS in HTML contexts.
47-50: LGTM!Case-insensitive substring matching with proper null handling is the right approach for bot detection.
52-57: LGTM!The origin resolution correctly handles x-forwarded-proto with comma-separated values and provides sensible fallbacks for reverse proxy scenarios.
80-97: LGTM!The type definition appropriately uses optional fields for an external API response where the shape may vary.
117-123: LGTM!The type definition clearly captures the preview metadata structure.
199-249: LGTM!The HTML rendering correctly escapes user content, conditionally redirects humans vs bots, and follows Open Graph/Twitter Card specifications.
251-282: LGTM!The function correctly validates input, fetches data in parallel, and branches appropriately for bot vs human requests with proper cache headers.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/client/AccountModal.tssrc/client/Main.tssrc/server/Master.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/client/AccountModal.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/server/Master.tssrc/client/Main.ts
🧬 Code graph analysis (2)
src/server/Master.ts (4)
src/core/configuration/ConfigLoader.ts (1)
getServerConfigFromServer(47-50)src/core/Schemas.ts (2)
GameInfo(130-136)ID(208-211)src/core/configuration/DefaultConfig.ts (1)
workerPort(207-209)tests/util/TestServerConfig.ts (1)
workerPort(67-69)
src/client/Main.ts (1)
src/core/Schemas.ts (1)
ID(208-211)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy to openfront.dev
🔇 Additional comments (19)
src/client/Main.ts (5)
379-379: LGTM!The custom event listener allows programmatic navigation to trigger the URL handler, consistent with the existing
popstateandhashchangelisteners.
480-486: LGTM!The join code extraction and URL normalization flow is clean, with an early return that correctly prevents further processing after handling the join link.
604-604: LGTM!The path-based URL is consistent with the new server route and the overall join link strategy.
609-624: LGTM!The extraction logic correctly prioritizes query parameters over path patterns and uses schema validation for both sources.
626-633: LGTM!The URL normalization correctly uses
replaceStateto avoid polluting history and includes a conditional check to prevent unnecessary updates.src/server/Master.ts (14)
47-58: LGTM!The bot list appropriately covers social media and messaging platform crawlers that consume Open Graph tags for link previews.
60-66: LGTM!The HTML escape utility correctly handles the five essential characters and processes
&first to prevent double-escaping.
68-71: LGTM!The bot detection logic is simple and effective, with case-insensitive matching and a safe default for missing user-agents.
73-78: LGTM!The origin extraction correctly handles proxy scenarios with
x-forwarded-protoand provides sensible fallbacks for both protocol and host.
80-100: LGTM!The lobby info fetch correctly uses
try/finallyto prevent timeout resource leaks and includes appropriate error handling.
102-119: LGTM!The type definition appropriately reflects the external API shape with optional fields for defensive parsing.
121-138: LGTM!The public game info fetch correctly uses
try/finallyto prevent timeout resource leaks and includes appropriate error handling.
140-146: LGTM!The preview metadata type is straightforward with clear, descriptive field names.
148-221: LGTM!The preview builder correctly prioritizes live lobby data over archived public data, uses nullish coalescing consistently, and handles all game states (finished, active, unknown).
223-273: LGTM!The HTML renderer correctly differentiates bot vs. human behavior, includes comprehensive Open Graph and Twitter Card tags, and uses HTML escaping to prevent XSS.
275-306: LGTM!The preview handler correctly validates IDs, fetches data in parallel, and differentiates between bot (static preview) and human (SPA) responses with appropriate cache headers.
308-313: LGTM!The route handler correctly applies rate limiting middleware and includes appropriate error handling with logging.
347-347: Verify the rate limit increase is intentional and appropriate.The global rate limit has been increased to 1000 requests per IP per second. Ensure this aligns with your system's capacity and DDoS protection strategy, as this is a significant threshold.
356-438: LGTM!The
startMasterfunction is now part of the public API. The implementation correctly handles worker lifecycle, scheduling coordination, and server startup.
…perfectly happy to parse the meta
…ts by checking for clientID, Always render meta tags Removed the unused escapeJsString function
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/server/GamePreviewBuilder.ts (3)
36-44: Consider guarding against invalid duration values.If
secondsis negative or NaN (from corrupted data), the output would be confusing. A simple guard at the start would make this more robust.🔎 Optional guard
function formatDuration(seconds: number): string { + if (!Number.isFinite(seconds) || seconds < 0) return "Unknown"; const mins = Math.floor(seconds / 60); const secs = seconds % 60;
230-262: Description format is coupled to rendering logic.The string prefixes like
"Game Options:"and"Disabled Units:"are parsed inrenderPreviewfor structured HTML output. This implicit contract works but is fragile—if the prefix changes here, rendering silently degrades to plain<p>tags.Consider returning structured data from
buildPreviewand generating both plain text and HTML from it. This would make the contract explicit.For now, this works fine since both functions live in the same file.
407-418: Unused.ctaCSS class.The
.ctaclass is defined in the stylesheet but never applied in the HTML template. This appears to be leftover from removed "Join now!" functionality (noted in past review comments).🔎 Remove unused CSS
} - .cta { - margin-top: 1.5rem; - padding: 0.875rem 1.5rem; - background: linear-gradient(135deg, #3b82f6, #2563eb); - color: white; - text-align: center; - border-radius: 8px; - font-weight: 600; - font-size: 0.95rem; - box-shadow: 0 4px 12px rgba(59, 130, 246, 0.3); - letter-spacing: 0.025em; - } .lobby-code {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/server/GamePreviewBuilder.ts
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: ryanbarlow97
Repo: openfrontio/OpenFrontIO PR: 2740
File: src/client/HostLobbyModal.ts:821-821
Timestamp: 2026-01-02T18:11:06.832Z
Learning: In src/client/HostLobbyModal.ts, the `?s=xxxxx` URL suffix in lobby URLs is purely for cache-busting embed previews on platforms like Discord, WhatsApp, and x.com. The suffix value is ignored by the join logic (any value works), so regenerating it on config changes via `updateUrlWithSuffix()` doesn't break existing shared URLs - it only forces platforms to re-fetch updated preview metadata.
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/server/GamePreviewBuilder.ts
📚 Learning: 2026-01-02T18:11:06.832Z
Learnt from: ryanbarlow97
Repo: openfrontio/OpenFrontIO PR: 2740
File: src/client/HostLobbyModal.ts:821-821
Timestamp: 2026-01-02T18:11:06.832Z
Learning: In src/client/HostLobbyModal.ts, the `?s=xxxxx` URL suffix in lobby URLs is purely for cache-busting embed previews on platforms like Discord, WhatsApp, and x.com. The suffix value is ignored by the join logic (any value works), so regenerating it on config changes via `updateUrlWithSuffix()` doesn't break existing shared URLs - it only forces platforms to re-fetch updated preview metadata.
Applied to files:
src/server/GamePreviewBuilder.ts
📚 Learning: 2025-10-08T17:14:49.369Z
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.
Applied to files:
src/server/GamePreviewBuilder.ts
📚 Learning: 2025-12-13T14:58:29.645Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2607
File: src/core/execution/PlayerExecution.ts:271-295
Timestamp: 2025-12-13T14:58:29.645Z
Learning: In src/core/execution/PlayerExecution.ts surroundedBySamePlayer(), the `as Player` cast on `mg.playerBySmallID(scan.enemyId)` is intentional. Since scan.enemyId comes from ownerID() on an owned tile and playerBySmallID() only returns Player or undefined, the cast expresses a known invariant. The maintainers prefer loud failures (runtime errors) over silent masking (early returns with guards) for corrupted game state scenarios at trusted call sites.
Applied to files:
src/server/GamePreviewBuilder.ts
📚 Learning: 2025-10-27T09:47:26.395Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.
Applied to files:
src/server/GamePreviewBuilder.ts
📚 Learning: 2025-10-20T20:15:28.858Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.
Applied to files:
src/server/GamePreviewBuilder.ts
📚 Learning: 2025-10-26T15:37:07.732Z
Learnt from: GlacialDrift
Repo: openfrontio/OpenFrontIO PR: 2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.732Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.
Applied to files:
src/server/GamePreviewBuilder.ts
📚 Learning: 2025-08-12T00:31:50.144Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 1752
File: src/core/game/Game.ts:750-752
Timestamp: 2025-08-12T00:31:50.144Z
Learning: In the OpenFrontIO codebase, changes to the PlayerInteraction interface (like adding canDonateGold and canDonateTroops flags) do not require corresponding updates to src/core/Schemas.ts or server serialization code.
Applied to files:
src/server/GamePreviewBuilder.ts
📚 Learning: 2025-10-20T11:02:16.969Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:57-57
Timestamp: 2025-10-20T11:02:16.969Z
Learning: In src/core/execution/FakeHumanExecution.ts, the correct MIRV victory denial thresholds are VICTORY_DENIAL_TEAM_THRESHOLD = 0.8 (80% for team games) and VICTORY_DENIAL_INDIVIDUAL_THRESHOLD = 0.65 (65% for individual players), not 0.85 and 0.7 as might be mentioned in some documentation.
Applied to files:
src/server/GamePreviewBuilder.ts
📚 Learning: 2025-10-21T20:06:04.823Z
Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Applied to files:
src/server/GamePreviewBuilder.ts
📚 Learning: 2025-05-21T04:10:33.435Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:34-38
Timestamp: 2025-05-21T04:10:33.435Z
Learning: In the codebase, PlayerStats is defined as a type inferred from a Zod schema that is marked as optional, which means PlayerStats already includes undefined as a possible type (PlayerStats | undefined).
Applied to files:
src/server/GamePreviewBuilder.ts
📚 Learning: 2025-10-08T17:14:49.369Z
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: In `ClientGameRunner.ts`, the `myPlayer` field is always set when `shouldPreventWindowClose()` is called, so the null check in that method is sufficient without needing to fetch it again from `gameView.playerByClientID()`.
Applied to files:
src/server/GamePreviewBuilder.ts
📚 Learning: 2025-05-21T04:10:33.435Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:34-38
Timestamp: 2025-05-21T04:10:33.435Z
Learning: In the codebase, PlayerStats is defined as `z.infer<typeof PlayerStatsSchema>` where PlayerStatsSchema has `.optional()` applied at the object level, making PlayerStats a union type that already includes undefined (PlayerStats | undefined).
Applied to files:
src/server/GamePreviewBuilder.ts
📚 Learning: 2025-05-19T06:00:38.007Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:125-134
Timestamp: 2025-05-19T06:00:38.007Z
Learning: In StatsImpl.ts, unused parameters in boat/stats-related methods are intentionally kept for future use and shouldn't be removed.
Applied to files:
src/server/GamePreviewBuilder.ts
📚 Learning: 2025-06-16T03:03:59.778Z
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1192
File: src/client/graphics/layers/RadialMenuElements.ts:312-314
Timestamp: 2025-06-16T03:03:59.778Z
Learning: In RadialMenuElements.ts, when fixing undefined params errors in subMenu functions, use explicit checks like `if (params === undefined || params.selected === null)` rather than optional chaining, as it makes the intent clearer and matches the specific error scenarios where params can be undefined during spawn phase operations.
Applied to files:
src/server/GamePreviewBuilder.ts
📚 Learning: 2025-05-21T04:10:59.706Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:44-53
Timestamp: 2025-05-21T04:10:59.706Z
Learning: The PlayerStats type from ArchiveSchemas already includes undefined in its definition, making explicit union types with undefined (PlayerStats | undefined) redundant.
Applied to files:
src/server/GamePreviewBuilder.ts
🧬 Code graph analysis (1)
src/server/GamePreviewBuilder.ts (1)
src/core/Schemas.ts (1)
GameInfo(134-140)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy to openfront.dev
🔇 Additional comments (9)
src/server/GamePreviewBuilder.ts (9)
1-9: Clean type definition with defensive typing.Using
unknownforstatsforces proper type guards before use, which is good practice. The localPlayerInfotype keeps this module decoupled from internal player schemas.
10-34: Types are clean and defensive.All fields are optional, which correctly handles partial data from external sources. Good use of composition with nested config object.
46-68: Timestamp handling is solid.The
1e12threshold is a reasonable heuristic for detecting seconds vs milliseconds. Using UTC consistently avoids timezone confusion for users in different regions.
70-98: Winner parsing handles known formats well.Good defensive approach: returns
undefinedfor unknown formats rather than displaying confusing output. The fallback toclientIdwhen username is missing keeps the display functional.
100-111: Player counting logic is correct.The function correctly excludes NPCs (no clientID) and inactive players (no stats). The strict equality check addresses the earlier ESLint concern.
113-120: HTML escaping covers necessary characters.The five characters escaped (
&,<,>,",') are sufficient for preventing XSS in HTML attributes and content contexts.
122-191: URL and metadata construction is well-organized.The logic clearly branches for finished/private/public states. The map thumbnail URL correctly handles special characters with
encodeURIComponentafter sanitizing to alphanumeric only.
274-310: Rendering logic is clean with proper escaping.All dynamic values are escaped before insertion. The structured parsing for "Game Options:" and "Disabled Units:" sections creates nice badge displays for private lobbies.
456-466: HTML structure is accessible and well-formed.Good use of semantic HTML with
<main>androle="main". The lobby code display provides clear context for users who land on this preview page.
Description:
Changes URL embeds within other platforms, e.g. Discord, WhatsApp & X.
Updates game URLs to
/game/<code>instead of/#join=<code>(required for embedded URLs). An added benefit of this is that you would be able to change a url fromopenfront.io/game/RQDUy8nP?replaytoapi.openfront.io/game/RQDUy8nP?replay(add api. In front) and be in the right place for the API data.Updates URLs when joining/leaving private lobbies
Appends a random string to the end of the URL when inside a private lobby and options change - this is to force discord to update the embedded details.
Updates URL in different game states to ?lobby / ?live and ?replay. These do nothing other than being used as a cache-busting solution.
Lobby Info
Discord:
WhatsApp:
x.com:
Game Win Details
Discord:
WhatsApp:
x.com
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
w.o.n