Draft
Conversation
Contributor
WalkthroughAdds a Discord allowlist and role-requirement feature across UI, schemas, server enforcement, worker join flow, client model, translations, and tests; enforces checks on join/rejoin and config updates and sanitizes lobby_info for non-creators. Changes
Sequence Diagram(s)sequenceDiagram
participant Player
participant Worker
participant GameServer
participant ClientInstance as Client
Player->>Worker: Join request (token or anon)
Note over Worker: extract discordId and guildRoles from UserMe
alt GameConfig has allowedDiscordIds or requiredRoles
alt Player anonymous and allowlist exists
Worker-->>Player: Reject (discord_login_required)
else if discordId missing or not in allowlist
Worker-->>Player: Reject (kick_reason.discord_not_allowed)
else if missing required role
Worker-->>Player: Reject (kick_reason.discord_role_not_allowed)
else
Worker->>GameServer: Create client (include discordId, guildRoles)
GameServer->>ClientInstance: store discord info
GameServer-->>Player: Join accepted
end
else
Worker->>GameServer: Create client (no discord constraints)
GameServer-->>Player: Join accepted
end
sequenceDiagram
participant Creator
participant GameServer
participant ConnectedClient
Creator->>GameServer: Update gameConfig.allowedDiscordIds / requiredDiscordRoles
GameServer->>GameServer: Normalize & dedupe lists
GameServer->>GameServer: Build guild->roles map
loop for each connected client
alt client is creator OR matches allowlist/roles
Note over GameServer: keep connected
else
GameServer->>ConnectedClient: Kick (discord_not_allowed / discord_role_not_allowed)
end
end
GameServer-->>All: Broadcast sanitized lobby_info (hide discord lists for non-creators)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/Worker.ts (1)
366-435:⚠️ Potential issue | 🟡 MinorWorker discord allowlist check does not exempt the lobby creator.
The GameServer exempts the creator in
isDiscordAllowed()(line 143), but the Worker's fresh join path (lines 418–427) has no way to check if the user is the creator. ThecreatorPersistentIDis private in GameServer and not exposed via any public method.Normally this is not a problem: reconnecting clients go through
rejoinClient()(line 360), which callsisDiscordAllowed()and exempts the creator. However, ifrejoinClient()fails (e.g., game session was cleaned up), the creator hits the Worker's fresh join path and gets blocked if they are not in the allowlist they set.The inconsistency matters because:
- GameServer allows the creator to set any allowlist (even one excluding themselves) via
updateGameConfig().- If the creator disconnects and rejoins after their client is cleaned up, the Worker blocks them.
Consider adding a public method to GameServer to check if a persistentID is the creator, so the Worker can exempt them at line 418:
public isLobbyCreator(persistentID: string): boolean { return persistentID === this.creatorPersistentID; }Then in Worker, before the discord check at line 418, verify:
if (allowedDiscordIdSet !== null && !gameServer?.isLobbyCreator(persistentId)) { // ... perform discord check }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/Worker.ts` around lines 366 - 435, Worker rejects fresh-joining lobby creators because it checks allowedDiscordIdSet without exempting the creator like GameServer.isDiscordAllowed does; add a public GameServer.isLobbyCreator(persistentID: string): boolean that returns persistentID === this.creatorPersistentID, then update Worker in the fresh-join path (where it currently inspects allowedDiscordIdSet and discordId) to skip the Discord allowlist checks when gameServer?.isLobbyCreator(persistentId) is true (i.e., only enforce the allowlist if allowedDiscordIdSet !== null && !gameServer?.isLobbyCreator(persistentId)); keep existing log/close behavior for non-creators.
🧹 Nitpick comments (2)
tests/server/DiscordAllowlist.test.ts (1)
126-149: Test 2: direct mutation ofactiveClientscouples the test to internal state.
game.activeClients = [];directly overwrites the public array to simulate disconnection. This works today becauseenforceDiscordAllowlistiteratesallClients, notactiveClients. If that internal detail changes, this test will silently break. Consider using the WebSocket close flow instead (callws.close()and trigger thecloseevent on the mock).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/server/DiscordAllowlist.test.ts` around lines 126 - 149, The test mutates the GameServer.internal public array (game.activeClients = []) which couples the test to implementation; instead simulate a real disconnect by closing the client's socket so GameServer's normal removal path (e.g. the WebSocket close handler that updates activeClients and triggers enforceDiscordAllowlist) runs. Update the test to call the mock client's ws.close() (or trigger its 'close' event) after joinClient so the server's existing logic in enforceDiscordAllowlist and getClientIdForPersistentId is exercised, keeping createClient and GameServer behavior intact without directly setting activeClients.src/client/HostLobbyModal.ts (1)
751-777: Silent discard of invalid Discord IDs — no user feedback.If the user pastes a string that doesn't match the 17–20 digit pattern, it is silently ignored and the input clears. The user gets no indication that their input was rejected.
Consider adding a brief visual hint (e.g., shake the input, show a red border momentarily, or keep the invalid text in the input so the user can see it wasn't accepted).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/HostLobbyModal.ts` around lines 751 - 777, commitDiscordIdInput currently swallows invalid tokens and clears discordIdInput with no feedback; change it to collect invalid tokens and surface a UI error instead of silently clearing: in commitDiscordIdInput, build two lists (validIds and invalidTokens) by testing each token against DISCORD_ID_REGEX, add validIds to nextIds and only clear or update this.allowedDiscordIds/ call putGameConfig if there was a change, but if invalidTokens is non-empty set a component state flag (e.g., this.discordIdError or this.invalidDiscordTokens) and do not clear this.discordIdInput so the user can see/fix them; update the template/renderer to show a red border/brief shake or inline message when this.discordIdError/invalidDiscordTokens is set and clear that flag on next input or successful commit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/server/GameServer.ts`:
- Around line 151-160: Kicked clients are never unblocked because entries in
kickedPersistentIds persist after the allowlist is removed; modify
setAllowedDiscordIds/enforceDiscordAllowlist handling so that when the allowlist
is cleared (allowedDiscordIdsSet set to null or empty) you remove any
kickedPersistentIds that were added due solely to the Discord allowlist (or
reset kickedPersistentIds entirely), and/or track a allowlistVersion on kicks
and only consult kickedPersistentIds entries that match the current
allowlistVersion; update getClientIdForPersistentId accordingly to prevent stale
permanent bans. Ensure changes reference the existing symbols:
setAllowedDiscordIds, enforceDiscordAllowlist, kickedPersistentIds, and
getClientIdForPersistentId.
In `@tests/server/DiscordAllowlist.test.ts`:
- Around line 151-196: The test is a false positive because
startLobbyInfoBroadcast()/broadcastLobbyInfo() only run via an interval set when
the creator joins, but with vi.useFakeTimers() the interval callback never fires
after guest join; advance the fake timer after
expect(game.joinClient(guestClient)) so the interval executes and
guestWs.sentMessages contains a real "lobby_info" to inspect. Specifically, in
the test around joinClient and inspection of guestWs.sentMessages, call the
fake-timer advance (e.g., vi.advanceTimersByTime or equivalent) so
broadcastLobbyInfo is invoked, then re-query guestWs.sentMessages for a
"lobby_info" and assert allowedDiscordIds is absent; this touches GameServer
startLobbyInfoBroadcast, broadcastLobbyInfo, joinClient and the test's
guestWs.sentMessages usage.
---
Outside diff comments:
In `@src/server/Worker.ts`:
- Around line 366-435: Worker rejects fresh-joining lobby creators because it
checks allowedDiscordIdSet without exempting the creator like
GameServer.isDiscordAllowed does; add a public
GameServer.isLobbyCreator(persistentID: string): boolean that returns
persistentID === this.creatorPersistentID, then update Worker in the fresh-join
path (where it currently inspects allowedDiscordIdSet and discordId) to skip the
Discord allowlist checks when gameServer?.isLobbyCreator(persistentId) is true
(i.e., only enforce the allowlist if allowedDiscordIdSet !== null &&
!gameServer?.isLobbyCreator(persistentId)); keep existing log/close behavior for
non-creators.
---
Nitpick comments:
In `@src/client/HostLobbyModal.ts`:
- Around line 751-777: commitDiscordIdInput currently swallows invalid tokens
and clears discordIdInput with no feedback; change it to collect invalid tokens
and surface a UI error instead of silently clearing: in commitDiscordIdInput,
build two lists (validIds and invalidTokens) by testing each token against
DISCORD_ID_REGEX, add validIds to nextIds and only clear or update
this.allowedDiscordIds/ call putGameConfig if there was a change, but if
invalidTokens is non-empty set a component state flag (e.g., this.discordIdError
or this.invalidDiscordTokens) and do not clear this.discordIdInput so the user
can see/fix them; update the template/renderer to show a red border/brief shake
or inline message when this.discordIdError/invalidDiscordTokens is set and clear
that flag on next input or successful commit.
In `@tests/server/DiscordAllowlist.test.ts`:
- Around line 126-149: The test mutates the GameServer.internal public array
(game.activeClients = []) which couples the test to implementation; instead
simulate a real disconnect by closing the client's socket so GameServer's normal
removal path (e.g. the WebSocket close handler that updates activeClients and
triggers enforceDiscordAllowlist) runs. Update the test to call the mock
client's ws.close() (or trigger its 'close' event) after joinClient so the
server's existing logic in enforceDiscordAllowlist and
getClientIdForPersistentId is exercised, keeping createClient and GameServer
behavior intact without directly setting activeClients.
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/core/Schemas.ts (1)
243-247: Add.max()toallowedDiscordIdsandrequiredDiscordRolesarrays.Both arrays are included in
UpdateGameConfigIntentSchemaviaGameConfigSchema.partial(), so a lobby creator can send an arbitrarily large array over WebSocket (there is no WS frame size limit configured in thewsssetup). This could cause a memory spike onsetAllowedDiscordIds(Set construction) orenforceDiscordRestrictions(iteration).♻️ Proposed fix
- allowedDiscordIds: z.array(DiscordIDSchema).optional(), - requiredDiscordRoles: z.array(DiscordRoleRequirementSchema).optional(), + allowedDiscordIds: z.array(DiscordIDSchema).max(500).optional(), + requiredDiscordRoles: z.array(DiscordRoleRequirementSchema).max(50).optional(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/Schemas.ts` around lines 243 - 247, The arrays allowedDiscordIds and requiredDiscordRoles in GameConfigSchema can be arbitrarily large when partial-ed into UpdateGameConfigIntentSchema, risking memory/CPU spikes in setAllowedDiscordIds and enforceDiscordRestrictions; update the schema by adding a reasonable .max(n) to both DiscordIDSchema arrays (and similarly to requiredDiscordRoles' array) so the partialed UpdateGameConfigIntentSchema enforces an upper bound on input size and prevents huge payloads being accepted over the WebSocket.tests/server/DiscordAllowlist.test.ts (1)
106-154: Add a test for the lobby-creator bypass.None of the existing tests cover the case where the creator's own Discord ID is not in the allowlist —
getDiscordRejectionReasonreturnsundefinedfor the creator viaisLobbyCreatorPersistentId. This is a critical rule in the enforcement logic and should be explicitly tested to prevent regressions.Do you want me to draft this test case?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/server/DiscordAllowlist.test.ts` around lines 106 - 154, Add a test that verifies the lobby creator is exempt from Discord allowlist checks: create a GameServer with a private game and an allowlist that does NOT include the creator's Discord ID, then call joinClient for a client whose persistentId equals the creator's persistent id and assert it is allowed (e.g., joinClient returns "joined"); also assert that getDiscordRejectionReason (or the path that calls isLobbyCreatorPersistentId) returns undefined for the creator while still rejecting non-creator clients with blocked Discord IDs. Ensure you reference GameServer, joinClient, getDiscordRejectionReason and isLobbyCreatorPersistentId in the new test to cover the bypass rule.src/server/Worker.ts (1)
58-94:normalizeRequiredDiscordRolesis duplicated in three places — extract to a shared utility.The same normalization logic (dedup by
guildId:roleIdkey, merge legacy fields) exists in:
src/server/Worker.tslines 58–94 (standalone function)src/server/GameServer.tslines 154–179 (insidesetRequiredDiscordRoleRequirements)src/client/HostLobbyModal.tslines 914–952 (instance method)The server-side copies are especially brittle to diverge. Extracting to a shared utility (e.g.,
src/core/DiscordUtil.ts) removes the duplication and gives a single place to update if requirements change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/Worker.ts` around lines 58 - 94, Extract the duplicated normalizeRequiredDiscordRoles implementation into a single exported utility function (e.g., export function normalizeRequiredDiscordRoles(...) in a new src/core/DiscordUtil.ts) preserving the current signature and behavior (trim, skip empty, dedupe by `${guildId}:${roleId}`, and merge legacyGuildId/legacyRoleId), then replace the local definitions in Worker.ts (function normalizeRequiredDiscordRoles), GameServer.ts (inside setRequiredDiscordRoleRequirements), and HostLobbyModal.ts (the instance method) with imports that call the shared utility; ensure each callsite imports the utility and that types/signatures match so behavior remains identical.src/client/HostLobbyModal.ts (1)
954-980: Invalid Discord IDs are silently dropped — consider user feedback.In
commitDiscordIdInput, tokens that failDISCORD_ID_REGEX.test(id)are skipped without any visible indication. If a user pastes a username instead of an ID, the input field clears and nothing is added. A small validation message (e.g., highlighting the field red or showing a tooltip) would prevent confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client/HostLobbyModal.ts`:
- Around line 941-948: The legacy guild/role branch adds a normalized
requirement but forgets to record its key in the dedupe set; update the block
that checks normalizedLegacyGuildId and normalizedLegacyRoleId to call
uniqueRequirementKeys.add(key) (using the same key computed from
normalizedLegacyGuildId:normalizedLegacyRoleId) before pushing into
normalizedRequirements so the dedupe set (uniqueRequirementKeys) stays
consistent with normalizedRequirements and matches the behavior in Worker.ts and
GameServer.ts.
---
Duplicate comments:
In `@src/server/GameServer.ts`:
- Around line 117-141: setAllowedDiscordIds currently clears
allowedDiscordIdsSet but leaves kickedPersistentIds entries from prior Discord
kicks, so clients stay blocked; update setAllowedDiscordIds (and/or the related
setter for requiredDiscordRoles if present) to remove Discord-related entries
from kickedPersistentIds when the allowlist is removed and there are no
requiredDiscordRoles (i.e., when allowedDiscordIdsSet becomes null and
requiredDiscordRoles is empty): either clear kickedPersistentIds entirely for
Discord-originated kicks or remove only entries tracked as Discord kicks (you
can add a small sentinel/reason tracking or remove by checking entries that were
added by enforceDiscordRestrictions), and ensure getClientIdForPersistentId will
then allow rejoining once restrictions are lifted.
In `@tests/server/DiscordAllowlist.test.ts`:
- Around line 156-201: The test never advances fake timers, so the lobby info
interval callback (started by startLobbyInfoBroadcast which calls
broadcastLobbyInfo) never runs after the guest joins; update the test (after
expect(game.joinClient(guestClient)).toBe("joined")) to advance the fake timers
(e.g., run pending timers or advance by the broadcast interval) so
guestWs.sentMessages receives a lobby_info message and the allowedDiscordIds
sanitization is actually exercised.
---
Nitpick comments:
In `@src/core/Schemas.ts`:
- Around line 243-247: The arrays allowedDiscordIds and requiredDiscordRoles in
GameConfigSchema can be arbitrarily large when partial-ed into
UpdateGameConfigIntentSchema, risking memory/CPU spikes in setAllowedDiscordIds
and enforceDiscordRestrictions; update the schema by adding a reasonable .max(n)
to both DiscordIDSchema arrays (and similarly to requiredDiscordRoles' array) so
the partialed UpdateGameConfigIntentSchema enforces an upper bound on input size
and prevents huge payloads being accepted over the WebSocket.
In `@src/server/Worker.ts`:
- Around line 58-94: Extract the duplicated normalizeRequiredDiscordRoles
implementation into a single exported utility function (e.g., export function
normalizeRequiredDiscordRoles(...) in a new src/core/DiscordUtil.ts) preserving
the current signature and behavior (trim, skip empty, dedupe by
`${guildId}:${roleId}`, and merge legacyGuildId/legacyRoleId), then replace the
local definitions in Worker.ts (function normalizeRequiredDiscordRoles),
GameServer.ts (inside setRequiredDiscordRoleRequirements), and HostLobbyModal.ts
(the instance method) with imports that call the shared utility; ensure each
callsite imports the utility and that types/signatures match so behavior remains
identical.
In `@tests/server/DiscordAllowlist.test.ts`:
- Around line 106-154: Add a test that verifies the lobby creator is exempt from
Discord allowlist checks: create a GameServer with a private game and an
allowlist that does NOT include the creator's Discord ID, then call joinClient
for a client whose persistentId equals the creator's persistent id and assert it
is allowed (e.g., joinClient returns "joined"); also assert that
getDiscordRejectionReason (or the path that calls isLobbyCreatorPersistentId)
returns undefined for the creator while still rejecting non-creator clients with
blocked Discord IDs. Ensure you reference GameServer, joinClient,
getDiscordRejectionReason and isLobbyCreatorPersistentId in the new test to
cover the bypass rule.
Comment on lines
+941
to
+948
| if (normalizedLegacyGuildId && normalizedLegacyRoleId) { | ||
| const key = `${normalizedLegacyGuildId}:${normalizedLegacyRoleId}`; | ||
| if (!uniqueRequirementKeys.has(key)) { | ||
| normalizedRequirements.push({ | ||
| guildId: normalizedLegacyGuildId, | ||
| roleId: normalizedLegacyRoleId, | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Missing uniqueRequirementKeys.add(key) for the legacy pair.
When the legacy {guildId, roleId} pair is added to normalizedRequirements (line 944), the key is not added to uniqueRequirementKeys. The equivalent code in Worker.ts (line 85) and GameServer.ts (line 173) both call uniqueRequirementKeys.add(key) before pushing. While the missing add causes no bug today (the function returns immediately after), it is inconsistent and risks producing a duplicate if more processing is added later.
🐛 Proposed fix
if (!uniqueRequirementKeys.has(key)) {
+ uniqueRequirementKeys.add(key);
normalizedRequirements.push({
guildId: normalizedLegacyGuildId,
roleId: normalizedLegacyRoleId,
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (normalizedLegacyGuildId && normalizedLegacyRoleId) { | |
| const key = `${normalizedLegacyGuildId}:${normalizedLegacyRoleId}`; | |
| if (!uniqueRequirementKeys.has(key)) { | |
| normalizedRequirements.push({ | |
| guildId: normalizedLegacyGuildId, | |
| roleId: normalizedLegacyRoleId, | |
| }); | |
| } | |
| if (normalizedLegacyGuildId && normalizedLegacyRoleId) { | |
| const key = `${normalizedLegacyGuildId}:${normalizedLegacyRoleId}`; | |
| if (!uniqueRequirementKeys.has(key)) { | |
| uniqueRequirementKeys.add(key); | |
| normalizedRequirements.push({ | |
| guildId: normalizedLegacyGuildId, | |
| roleId: normalizedLegacyRoleId, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/HostLobbyModal.ts` around lines 941 - 948, The legacy guild/role
branch adds a normalized requirement but forgets to record its key in the dedupe
set; update the block that checks normalizedLegacyGuildId and
normalizedLegacyRoleId to call uniqueRequirementKeys.add(key) (using the same
key computed from normalizedLegacyGuildId:normalizedLegacyRoleId) before pushing
into normalizedRequirements so the dedupe set (uniqueRequirementKeys) stays
consistent with normalizedRequirements and matches the behavior in Worker.ts and
GameServer.ts.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/server/DiscordAllowlist.test.ts (2)
77-80: Addvi.useRealTimers()toafterEachto avoid timer leak.
vi.restoreAllMocks()andvi.clearAllTimers()do not restore native timer implementations.vi.useFakeTimers()wraps all timer calls untilvi.useRealTimers()is called — without it, fake timers persist into other test files sharing the same worker.🔧 Proposed fix
afterEach(() => { vi.restoreAllMocks(); vi.clearAllTimers(); + vi.useRealTimers(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/server/DiscordAllowlist.test.ts` around lines 77 - 80, The afterEach block in DiscordAllowlist.test.ts currently calls vi.restoreAllMocks() and vi.clearAllTimers() but doesn't restore native timers; add a call to vi.useRealTimers() inside the same afterEach so fake timers are reverted after each test (i.e., update the afterEach that contains vi.restoreAllMocks() and vi.clearAllTimers() to also call vi.useRealTimers()).
245-261: UsecreateClientinstead of duplicatingClientconstruction inline.This block repeats the exact same 12-argument
Clientconstruction pattern already encapsulated in thecreateClienthelper.♻️ Proposed refactor
- const ws = new MockWebSocket(); - const client = new Client( - "allowed-client" as any, - "allowed-pid", - null, - undefined, - undefined, - "127.0.0.1", - "allowed-client", - "allowed-client", - ws as unknown as WebSocket, - undefined, - ALLOWED_DISCORD_ID, - { - [REQUIRED_GUILD_ID]: [REQUIRED_ROLE_ID], - }, - ); + const { client } = createClient( + "allowed-client", + "allowed-pid", + ALLOWED_DISCORD_ID, + { [REQUIRED_GUILD_ID]: [REQUIRED_ROLE_ID] }, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/server/DiscordAllowlist.test.ts` around lines 245 - 261, Replace the duplicated 12-argument Client construction with the existing createClient test helper to avoid repetition: locate the inline new Client(...) instantiation in DiscordAllowlist.test.ts and call createClient(...) passing the same MockWebSocket instance (ws), the ALLOWED_DISCORD_ID, and the guild->roles mapping ({ [REQUIRED_GUILD_ID]: [REQUIRED_ROLE_ID] }) so the created client matches the original; ensure any additional expectations in the test use the returned client from createClient instead of the inline new Client.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/server/DiscordAllowlist.test.ts`:
- Around line 77-80: The afterEach block in DiscordAllowlist.test.ts currently
calls vi.restoreAllMocks() and vi.clearAllTimers() but doesn't restore native
timers; add a call to vi.useRealTimers() inside the same afterEach so fake
timers are reverted after each test (i.e., update the afterEach that contains
vi.restoreAllMocks() and vi.clearAllTimers() to also call vi.useRealTimers()).
- Around line 245-261: Replace the duplicated 12-argument Client construction
with the existing createClient test helper to avoid repetition: locate the
inline new Client(...) instantiation in DiscordAllowlist.test.ts and call
createClient(...) passing the same MockWebSocket instance (ws), the
ALLOWED_DISCORD_ID, and the guild->roles mapping ({ [REQUIRED_GUILD_ID]:
[REQUIRED_ROLE_ID] }) so the created client matches the original; ensure any
additional expectations in the test use the returned client from createClient
instead of the inline new Client.
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
1 participant
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.
If this PR fixes an issue, link it below. If not, delete these two lines.
Resolves #(issue number)
Description:
Describe the PR.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
DISCORD_USERNAME