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 5 minutes and 0 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 path-based /game/{id} URLs across client; emit Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Browser as Browser (user or crawler)
participant Server as Master Server
participant Worker as Worker / Data API
Browser->>Server: GET /game/:gameId
Server->>Server: inspect Origin & User-Agent
Server->>Server: apply per-IP preview limiter
Server->>Worker: fetchLobbyInfo(gameId) (abortable, ~1500ms)
Worker-->>Server: lobby data or timeout/error
Server->>Worker: fetchPublicGameInfo(gameId) (abortable, ~1500ms)
Worker-->>Server: public metadata or timeout/error
alt Bot detected
Server->>Server: buildPreview(meta) (GamePreviewBuilder.buildPreview)
Server->>Browser: return pre-rendered OG HTML + ETag/Cache-Control (short cache)
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.
ryanbarlow97
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/core/Schemas.ts (1)
213-218: Well-structured refactor that centralizes game ID validation.The changes correctly extract the validation pattern into a reusable constant and provide a convenient helper function. Removing the redundant
.length(8)constraint is correct since the regex{8}already enforces exactly 8 characters.The regex pattern is solid: anchors ensure full string matching, and the alphanumeric character class with fixed length is appropriate for game IDs.
Optional: Consider adding JSDoc comments for clarity.
While the naming is clear, JSDoc comments would help future developers understand the constraints:
/** * Regular expression for valid game IDs. * Matches exactly 8 alphanumeric characters (A-Z, a-z, 0-9). */ export const GAME_ID_REGEX = /^[A-Za-z0-9]{8}$/; /** * Validates whether a string is a valid game ID. * @param value - The string to validate * @returns true if the value matches the game ID pattern */ export const isValidGameID = (value: string): boolean => GAME_ID_REGEX.test(value);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/Schemas.tssrc/server/GamePreviewBuilder.ts
🧰 Additional context used
🧠 Learnings (13)
📓 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.
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.
📚 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/core/Schemas.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/core/Schemas.ts
📚 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: 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: 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-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
🧬 Code graph analysis (1)
src/server/GamePreviewBuilder.ts (2)
src/core/Schemas.ts (1)
GameInfo(134-140)src/core/configuration/DefaultConfig.ts (2)
playerTeams(295-297)bots(307-309)
⏰ 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 (1)
src/server/GamePreviewBuilder.ts (1)
1-420: Excellent implementation - all previous concerns addressed!The code is clean, secure, and well-structured. All issues from previous reviews have been properly fixed:
- XSS protection: Both
escapeHtmlandescapeJsStringare correctly applied throughout- Type safety: Array guards added where needed (line 202)
- Correct fallbacks:
GameMode.FFAused instead of mixinggameTypewithgameMode- Dead code removed: Cleaned up unreachable branches
The implementation follows good TypeScript practices:
- Pure, composable functions instead of classes
- Well-defined type unions for data structures
- Defensive programming with sensible fallbacks (
??operators)- Clear separation of concerns (build metadata → render HTML)
- No unnecessary dependencies
Based on retrieved learnings, the URL suffix pattern for cache-busting Discord/WhatsApp previews is correctly implemented.
Description:
Changes URL embeds within other platforms, e.g. Discord.
Lobby Info
Discord:
WhatsApp:
x.com:
Game Win Details
Discord:
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
w.o.n