From f968a62f6ef90249f2cb182c63aecd2499a4722a Mon Sep 17 00:00:00 2001 From: TheCoderBuilder-dev Date: Mon, 2 Mar 2026 00:32:39 +0300 Subject: [PATCH 1/6] fix: address code review feedback on protect-roles (#181) --- .../config-sections/ModerationSection.tsx | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/web/src/components/dashboard/config-sections/ModerationSection.tsx b/web/src/components/dashboard/config-sections/ModerationSection.tsx index 1b0ce7b05..366d0df38 100644 --- a/web/src/components/dashboard/config-sections/ModerationSection.tsx +++ b/web/src/components/dashboard/config-sections/ModerationSection.tsx @@ -1,7 +1,10 @@ 'use client'; +import { useEffect, useState } from 'react'; + import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/components/ui/card'; import { ChannelSelector } from '@/components/ui/channel-selector'; +import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; import { Switch } from '@/components/ui/switch'; import { useGuildSelection } from '@/hooks/use-guild-selection'; @@ -14,6 +17,7 @@ interface ModerationSectionProps { onFieldChange: (field: string, value: unknown) => void; onDmNotificationChange: (action: string, value: boolean) => void; onEscalationChange: (enabled: boolean) => void; + onProtectRolesChange: (field: string, value: unknown) => void; } /** @@ -35,8 +39,16 @@ export function ModerationSection({ onFieldChange, onDmNotificationChange, onEscalationChange, + onProtectRolesChange, }: ModerationSectionProps) { const guildId = useGuildSelection(); + const [roleIdsRaw, setRoleIdsRaw] = useState( + (draftConfig.moderation?.protectRoles?.roleIds ?? []).join(', '), + ); + + useEffect(() => { + setRoleIdsRaw((draftConfig.moderation?.protectRoles?.roleIds ?? []).join(', ')); + }, [draftConfig.moderation?.protectRoles?.roleIds]); if (!draftConfig.moderation) return null; @@ -124,6 +136,81 @@ export function ModerationSection({ aria-label="Toggle escalation" /> + + {/* Protect Roles sub-section */} +
+ Protect Roles from Moderation +
+ + onProtectRolesChange('enabled', v)} + disabled={saving} + aria-label="Toggle protect roles" + /> +
+
+ + onProtectRolesChange('includeAdmins', v)} + disabled={saving} + aria-label="Include admins" + /> +
+
+ + onProtectRolesChange('includeModerators', v)} + disabled={saving} + aria-label="Include moderators" + /> +
+
+ + onProtectRolesChange('includeServerOwner', v)} + disabled={saving} + aria-label="Include server owner" + /> +
+
+ + setRoleIdsRaw(e.target.value)} + onBlur={() => + onProtectRolesChange( + 'roleIds', + roleIdsRaw + .split(',') + .map((s) => s.trim()) + .filter(Boolean), + ) + } + disabled={saving} + placeholder="Role ID 1, Role ID 2" + /> +
+
); From a50d66f3b8e18b9b40643765463b7a891baaa787 Mon Sep 17 00:00:00 2001 From: TheCoderBuilder-dev Date: Mon, 2 Mar 2026 01:00:49 +0300 Subject: [PATCH 2/6] fix: deep-merge protectRoles defaults, remove dead code from ModerationSection --- .../config-sections/ModerationSection.tsx | 80 ------------------- 1 file changed, 80 deletions(-) diff --git a/web/src/components/dashboard/config-sections/ModerationSection.tsx b/web/src/components/dashboard/config-sections/ModerationSection.tsx index 366d0df38..d29b03694 100644 --- a/web/src/components/dashboard/config-sections/ModerationSection.tsx +++ b/web/src/components/dashboard/config-sections/ModerationSection.tsx @@ -1,7 +1,5 @@ 'use client'; -import { useEffect, useState } from 'react'; - import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/components/ui/card'; import { ChannelSelector } from '@/components/ui/channel-selector'; import { Input } from '@/components/ui/input'; @@ -17,7 +15,6 @@ interface ModerationSectionProps { onFieldChange: (field: string, value: unknown) => void; onDmNotificationChange: (action: string, value: boolean) => void; onEscalationChange: (enabled: boolean) => void; - onProtectRolesChange: (field: string, value: unknown) => void; } /** @@ -39,7 +36,6 @@ export function ModerationSection({ onFieldChange, onDmNotificationChange, onEscalationChange, - onProtectRolesChange, }: ModerationSectionProps) { const guildId = useGuildSelection(); const [roleIdsRaw, setRoleIdsRaw] = useState( @@ -49,7 +45,6 @@ export function ModerationSection({ useEffect(() => { setRoleIdsRaw((draftConfig.moderation?.protectRoles?.roleIds ?? []).join(', ')); }, [draftConfig.moderation?.protectRoles?.roleIds]); - if (!draftConfig.moderation) return null; const alertChannelId = draftConfig.moderation?.alertChannelId ?? ''; @@ -136,81 +131,6 @@ export function ModerationSection({ aria-label="Toggle escalation" /> - - {/* Protect Roles sub-section */} -
- Protect Roles from Moderation -
- - onProtectRolesChange('enabled', v)} - disabled={saving} - aria-label="Toggle protect roles" - /> -
-
- - onProtectRolesChange('includeAdmins', v)} - disabled={saving} - aria-label="Include admins" - /> -
-
- - onProtectRolesChange('includeModerators', v)} - disabled={saving} - aria-label="Include moderators" - /> -
-
- - onProtectRolesChange('includeServerOwner', v)} - disabled={saving} - aria-label="Include server owner" - /> -
-
- - setRoleIdsRaw(e.target.value)} - onBlur={() => - onProtectRolesChange( - 'roleIds', - roleIdsRaw - .split(',') - .map((s) => s.trim()) - .filter(Boolean), - ) - } - disabled={saving} - placeholder="Role ID 1, Role ID 2" - /> -
-
); From ce85feb5795e5ce8ae7160965ccfc3cd160b956b Mon Sep 17 00:00:00 2001 From: TheCoderBuilder-dev Date: Mon, 2 Mar 2026 07:39:59 +0300 Subject: [PATCH 3/6] feat: restore protectRoles UI to ModerationSection for future wiring --- .../config-sections/ModerationSection.tsx | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/web/src/components/dashboard/config-sections/ModerationSection.tsx b/web/src/components/dashboard/config-sections/ModerationSection.tsx index d29b03694..f6adb8bc6 100644 --- a/web/src/components/dashboard/config-sections/ModerationSection.tsx +++ b/web/src/components/dashboard/config-sections/ModerationSection.tsx @@ -1,5 +1,7 @@ 'use client'; +import { useEffect, useState } from 'react'; + import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/components/ui/card'; import { ChannelSelector } from '@/components/ui/channel-selector'; import { Input } from '@/components/ui/input'; @@ -15,6 +17,7 @@ interface ModerationSectionProps { onFieldChange: (field: string, value: unknown) => void; onDmNotificationChange: (action: string, value: boolean) => void; onEscalationChange: (enabled: boolean) => void; + onProtectRolesChange: (field: string, value: unknown) => void; } /** @@ -36,6 +39,7 @@ export function ModerationSection({ onFieldChange, onDmNotificationChange, onEscalationChange, + onProtectRolesChange, }: ModerationSectionProps) { const guildId = useGuildSelection(); const [roleIdsRaw, setRoleIdsRaw] = useState( @@ -131,6 +135,81 @@ export function ModerationSection({ aria-label="Toggle escalation" /> + + {/* Protect Roles sub-section */} +
+ Protect Roles from Moderation +
+ + onProtectRolesChange('enabled', v)} + disabled={saving} + aria-label="Toggle protect roles" + /> +
+
+ + onProtectRolesChange('includeAdmins', v)} + disabled={saving} + aria-label="Include admins" + /> +
+
+ + onProtectRolesChange('includeModerators', v)} + disabled={saving} + aria-label="Include moderators" + /> +
+
+ + onProtectRolesChange('includeServerOwner', v)} + disabled={saving} + aria-label="Include server owner" + /> +
+
+ + setRoleIdsRaw(e.target.value)} + onBlur={() => + onProtectRolesChange( + 'roleIds', + roleIdsRaw + .split(',') + .map((s) => s.trim()) + .filter(Boolean), + ) + } + disabled={saving} + placeholder="Role ID 1, Role ID 2" + /> +
+
); From c20ea737cb29ad7cc8bca346d04338dc46523324 Mon Sep 17 00:00:00 2001 From: TheCoderBuilder-dev Date: Mon, 2 Mar 2026 07:57:49 +0300 Subject: [PATCH 4/6] refactor: fetch config internally in isProtectedTarget for live config changes --- src/modules/moderation.js | 4 ++- src/modules/triage-respond.js | 2 +- src/utils/modAction.js | 2 +- tests/modules/moderation.test.js | 61 ++++++++++++++++---------------- 4 files changed, 36 insertions(+), 33 deletions(-) diff --git a/src/modules/moderation.js b/src/modules/moderation.js index 610c184b6..c90c7a9bc 100644 --- a/src/modules/moderation.js +++ b/src/modules/moderation.js @@ -450,7 +450,9 @@ export function stopTempbanScheduler() { * @param {Object} config - Bot configuration * @returns {boolean} True if the target should not be moderated */ -export function isProtectedTarget(target, guild, config) { +export function isProtectedTarget(target, guild) { + // Fetch config per-invocation so live config edits take effect immediately. + const config = getConfig(guild.id); /** * When the protectRoles block is missing from persisted configuration, * fall back to the intended defaults: protection enabled, include owner, diff --git a/src/modules/triage-respond.js b/src/modules/triage-respond.js index 96bb88a68..f1e286a8b 100644 --- a/src/modules/triage-respond.js +++ b/src/modules/triage-respond.js @@ -147,7 +147,7 @@ export async function sendModerationLog(client, classification, snapshot, channe seenUserIds.add(t.userId); try { const member = await guild.members.fetch(t.userId); - if (isProtectedTarget(member, guild, config)) { + if (isProtectedTarget(member, guild)) { warn('Triage skipped moderation log: target is a protected role', { userId: t.userId, channelId, diff --git a/src/utils/modAction.js b/src/utils/modAction.js index 27b567d6b..ecb6ee9ba 100644 --- a/src/utils/modAction.js +++ b/src/utils/modAction.js @@ -101,7 +101,7 @@ export async function executeModAction(interaction, opts) { // Protected-role check (skipped when skipProtection is true) if (!skipProtection && target) { - if (isProtectedTarget(target, interaction.guild, config)) { + if (isProtectedTarget(target, interaction.guild)) { warn('Moderation blocked: target is a protected role', { action, targetId: target.id, diff --git a/tests/modules/moderation.test.js b/tests/modules/moderation.test.js index d61314b64..e9d541340 100644 --- a/tests/modules/moderation.test.js +++ b/tests/modules/moderation.test.js @@ -46,6 +46,7 @@ vi.mock('../../src/utils/duration.js', () => ({ import { getPool } from '../../src/db.js'; import { error as loggerError, warn as loggerWarn } from '../../src/logger.js'; +import { getConfig } from '../../src/modules/config.js'; import { checkEscalation, checkHierarchy, @@ -597,29 +598,29 @@ describe('moderation module', () => { roles: { cache: { keys: () => roleIds } }, }); - const makeGuild = (ownerId) => ({ ownerId }); + const makeGuild = (ownerId) => ({ id: 'guild1', ownerId }); it('returns false when protectRoles is disabled', () => { const target = makeTarget('user1', ['admin-role']); const guild = makeGuild('owner1'); - const config = { + getConfig.mockReturnValueOnce({ moderation: { protectRoles: { enabled: false } }, permissions: { adminRoleId: 'admin-role' }, - }; - expect(isProtectedTarget(target, guild, config)).toBe(false); + }); + expect(isProtectedTarget(target, guild)).toBe(false); }); it('returns false when protectRoles config is absent', () => { const target = makeTarget('user1'); const guild = makeGuild('owner1'); - const config = { moderation: {} }; - expect(isProtectedTarget(target, guild, config)).toBe(false); + getConfig.mockReturnValueOnce({ moderation: {} }); + expect(isProtectedTarget(target, guild)).toBe(false); }); it('returns true for server owner when includeServerOwner is true', () => { const target = makeTarget('owner1'); const guild = makeGuild('owner1'); - const config = { + getConfig.mockReturnValueOnce({ moderation: { protectRoles: { enabled: true, @@ -629,14 +630,14 @@ describe('moderation module', () => { includeServerOwner: true, }, }, - }; - expect(isProtectedTarget(target, guild, config)).toBe(true); + }); + expect(isProtectedTarget(target, guild)).toBe(true); }); it('returns false for server owner when includeServerOwner is false', () => { const target = makeTarget('owner1'); const guild = makeGuild('owner1'); - const config = { + getConfig.mockReturnValueOnce({ moderation: { protectRoles: { enabled: true, @@ -646,14 +647,14 @@ describe('moderation module', () => { includeServerOwner: false, }, }, - }; - expect(isProtectedTarget(target, guild, config)).toBe(false); + }); + expect(isProtectedTarget(target, guild)).toBe(false); }); it('returns true for user with adminRoleId when includeAdmins is true', () => { const target = makeTarget('user1', ['admin-role']); const guild = makeGuild('owner1'); - const config = { + getConfig.mockReturnValueOnce({ moderation: { protectRoles: { enabled: true, @@ -664,14 +665,14 @@ describe('moderation module', () => { }, }, permissions: { adminRoleId: 'admin-role' }, - }; - expect(isProtectedTarget(target, guild, config)).toBe(true); + }); + expect(isProtectedTarget(target, guild)).toBe(true); }); it('returns false for admin role when includeAdmins is false', () => { const target = makeTarget('user1', ['admin-role']); const guild = makeGuild('owner1'); - const config = { + getConfig.mockReturnValueOnce({ moderation: { protectRoles: { enabled: true, @@ -682,14 +683,14 @@ describe('moderation module', () => { }, }, permissions: { adminRoleId: 'admin-role' }, - }; - expect(isProtectedTarget(target, guild, config)).toBe(false); + }); + expect(isProtectedTarget(target, guild)).toBe(false); }); it('returns true for user with moderatorRoleId when includeModerators is true', () => { const target = makeTarget('user1', ['mod-role']); const guild = makeGuild('owner1'); - const config = { + getConfig.mockReturnValueOnce({ moderation: { protectRoles: { enabled: true, @@ -700,14 +701,14 @@ describe('moderation module', () => { }, }, permissions: { moderatorRoleId: 'mod-role' }, - }; - expect(isProtectedTarget(target, guild, config)).toBe(true); + }); + expect(isProtectedTarget(target, guild)).toBe(true); }); it('returns true for user with a custom roleId in protectRoles.roleIds', () => { const target = makeTarget('user1', ['custom-role']); const guild = makeGuild('owner1'); - const config = { + getConfig.mockReturnValueOnce({ moderation: { protectRoles: { enabled: true, @@ -717,14 +718,14 @@ describe('moderation module', () => { includeServerOwner: false, }, }, - }; - expect(isProtectedTarget(target, guild, config)).toBe(true); + }); + expect(isProtectedTarget(target, guild)).toBe(true); }); it('returns false for regular user with no protected roles', () => { const target = makeTarget('user1', ['regular-role']); const guild = makeGuild('owner1'); - const config = { + getConfig.mockReturnValueOnce({ moderation: { protectRoles: { enabled: true, @@ -735,14 +736,14 @@ describe('moderation module', () => { }, }, permissions: { adminRoleId: 'admin-role', moderatorRoleId: 'mod-role' }, - }; - expect(isProtectedTarget(target, guild, config)).toBe(false); + }); + expect(isProtectedTarget(target, guild)).toBe(false); }); it('returns false when no protectedRoleIds resolve (no adminRoleId set) and user is non-owner', () => { const target = makeTarget('user1', ['some-role']); const guild = makeGuild('owner1'); - const config = { + getConfig.mockReturnValueOnce({ moderation: { protectRoles: { enabled: true, @@ -753,8 +754,8 @@ describe('moderation module', () => { }, }, permissions: {}, - }; - expect(isProtectedTarget(target, guild, config)).toBe(false); + }); + expect(isProtectedTarget(target, guild)).toBe(false); }); }); From 4b922de775fa2004320c0a5925f706bf290668dc Mon Sep 17 00:00:00 2001 From: TheCoderBuilder-dev Date: Mon, 2 Mar 2026 08:47:26 +0300 Subject: [PATCH 5/6] fix: stale JSDoc, skip fetch loop when protection disabled, validate roleIds items --- src/api/utils/configValidation.js | 2 +- src/modules/moderation.js | 1 - src/modules/triage-respond.js | 31 +++++++++++++++------------- tests/modules/triage-respond.test.js | 28 +++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 16 deletions(-) diff --git a/src/api/utils/configValidation.js b/src/api/utils/configValidation.js index bef5ceeae..eef226b04 100644 --- a/src/api/utils/configValidation.js +++ b/src/api/utils/configValidation.js @@ -115,7 +115,7 @@ export const CONFIG_SCHEMA = { type: 'object', properties: { enabled: { type: 'boolean' }, - roleIds: { type: 'array' }, + roleIds: { type: 'array', items: { type: 'string' } }, includeAdmins: { type: 'boolean' }, includeModerators: { type: 'boolean' }, includeServerOwner: { type: 'boolean' }, diff --git a/src/modules/moderation.js b/src/modules/moderation.js index c90c7a9bc..438c926f6 100644 --- a/src/modules/moderation.js +++ b/src/modules/moderation.js @@ -447,7 +447,6 @@ export function stopTempbanScheduler() { * configured under `moderation.protectRoles`. * @param {import('discord.js').GuildMember} target - Target member to check * @param {import('discord.js').Guild} guild - Discord guild - * @param {Object} config - Bot configuration * @returns {boolean} True if the target should not be moderated */ export function isProtectedTarget(target, guild) { diff --git a/src/modules/triage-respond.js b/src/modules/triage-respond.js index f1e286a8b..25cfae5fd 100644 --- a/src/modules/triage-respond.js +++ b/src/modules/triage-respond.js @@ -141,21 +141,24 @@ export async function sendModerationLog(client, classification, snapshot, channe // Skip moderation log if any flagged user is a protected role (admin/mod/owner) const guild = logChannel.guild; if (guild && targets.length > 0) { - const seenUserIds = new Set(); - for (const t of targets) { - if (seenUserIds.has(t.userId)) continue; - seenUserIds.add(t.userId); - try { - const member = await guild.members.fetch(t.userId); - if (isProtectedTarget(member, guild)) { - warn('Triage skipped moderation log: target is a protected role', { - userId: t.userId, - channelId, - }); - return; + // Skip the expensive member-fetch loop when protection is explicitly disabled. + if (config.moderation?.protectRoles?.enabled !== false) { + const seenUserIds = new Set(); + for (const t of targets) { + if (seenUserIds.has(t.userId)) continue; + seenUserIds.add(t.userId); + try { + const member = await guild.members.fetch(t.userId); + if (isProtectedTarget(member, guild)) { + warn('Triage skipped moderation log: target is a protected role', { + userId: t.userId, + channelId, + }); + return; + } + } catch { + // Member not in guild or fetch failed — proceed with logging } - } catch { - // Member not in guild or fetch failed — proceed with logging } } } diff --git a/tests/modules/triage-respond.test.js b/tests/modules/triage-respond.test.js index 81cc88ae3..dbf778585 100644 --- a/tests/modules/triage-respond.test.js +++ b/tests/modules/triage-respond.test.js @@ -347,6 +347,34 @@ describe('triage-respond', () => { expect(safeSend).toHaveBeenCalled(); }); + + it('should skip the protected-role fetch loop when protectRoles is explicitly disabled', async () => { + const mockFetch = vi.fn(); + const mockGuild = { members: { fetch: mockFetch } }; + const mockLogChannel = { id: 'log-channel', guild: mockGuild }; + const mockClient = { + channels: { fetch: vi.fn().mockResolvedValue(mockLogChannel) }, + }; + + const classification = { + recommendedAction: 'warn', + violatedRule: 'Rule 1', + reasoning: 'Rude message', + targetMessageIds: ['msg1'], + }; + const snapshot = [{ messageId: 'msg1', author: 'BadUser', userId: 'user1', content: 'msg' }]; + const config = { + triage: { moderationLogChannel: 'log-channel' }, + moderation: { protectRoles: { enabled: false } }, + }; + + await sendModerationLog(mockClient, classification, snapshot, 'channel1', config); + + // Member fetch should not be called since protection is disabled + expect(mockFetch).not.toHaveBeenCalled(); + expect(isProtectedTarget).not.toHaveBeenCalled(); + expect(safeSend).toHaveBeenCalled(); + }); }); describe('sendResponses', () => { From 7fc24c3bc0c3d313d60253045e119f4e96ffeaa2 Mon Sep 17 00:00:00 2001 From: TheCoderBuilder-dev Date: Mon, 2 Mar 2026 08:50:53 +0300 Subject: [PATCH 6/6] fix: commit roleIds onChange/onBlur before rebase --- web/src/components/dashboard/config-editor.tsx | 16 +++++++++++++--- .../config-sections/ModerationSection.tsx | 16 +++++++++++++--- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/web/src/components/dashboard/config-editor.tsx b/web/src/components/dashboard/config-editor.tsx index bf617ca04..64ab1ac45 100644 --- a/web/src/components/dashboard/config-editor.tsx +++ b/web/src/components/dashboard/config-editor.tsx @@ -1212,14 +1212,24 @@ export function ConfigEditor() { setProtectRoleIdsRaw(e.target.value)} - onBlur={() => + onChange={(e) => { + const raw = e.target.value; + setProtectRoleIdsRaw(raw); updateProtectRolesField( 'roleIds', - protectRoleIdsRaw + raw .split(',') .map((s) => s.trim()) .filter(Boolean), + ); + }} + onBlur={(e) => + setProtectRoleIdsRaw( + e.target.value + .split(',') + .map((s) => s.trim()) + .filter(Boolean) + .join(', '), ) } disabled={saving} diff --git a/web/src/components/dashboard/config-sections/ModerationSection.tsx b/web/src/components/dashboard/config-sections/ModerationSection.tsx index f6adb8bc6..d03e3f9e8 100644 --- a/web/src/components/dashboard/config-sections/ModerationSection.tsx +++ b/web/src/components/dashboard/config-sections/ModerationSection.tsx @@ -195,14 +195,24 @@ export function ModerationSection({ id="protect-role-ids" type="text" value={roleIdsRaw} - onChange={(e) => setRoleIdsRaw(e.target.value)} - onBlur={() => + onChange={(e) => { + const raw = e.target.value; + setRoleIdsRaw(raw); onProtectRolesChange( 'roleIds', - roleIdsRaw + raw .split(',') .map((s) => s.trim()) .filter(Boolean), + ); + }} + onBlur={(e) => + setRoleIdsRaw( + e.target.value + .split(',') + .map((s) => s.trim()) + .filter(Boolean) + .join(', '), ) } disabled={saving}