From 89cf90ce78b3419b56cb1fdc925ad70e38006050 Mon Sep 17 00:00:00 2001 From: Bill Chirico Date: Sat, 7 Mar 2026 18:30:39 -0500 Subject: [PATCH 1/9] feat: migrate adminRoleId/moderatorRoleId to arrays with backward compat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - config.json: adminRoleId/moderatorRoleId → adminRoleIds/moderatorRoleIds (arrays) - web/src/types/config.ts: PermissionsConfig updated to string[] fields - src/utils/permissions.js: isAdmin/isModerator check arrays; falls back to singular field for old configs - src/utils/modExempt.js: isExempt checks adminRoleIds/moderatorRoleIds arrays with backward compat for singular field - src/modules/moderation.js: protect-roles spreads full arrays instead of wrapping a single ID - web/src/components/dashboard/config-editor.tsx: RoleSelector for admin/ moderator switched to true multi-select (no maxSelections, no array wrapping) - tests/utils/permissions.test.js: updated to use array fields; added multi- role and backward-compat tests - tests/utils/modExempt.test.js: updated to use array fields; added multi- role and backward-compat tests --- config.json | 4 +- src/modules/moderation.js | 16 ++-- src/utils/modExempt.js | 23 +++-- src/utils/permissions.js | 31 +++++-- tests/utils/modExempt.test.js | 40 ++++++-- tests/utils/permissions.test.js | 91 ++++++++++++++++--- .../components/dashboard/config-editor.tsx | 38 +++----- web/src/types/config.ts | 4 +- 8 files changed, 175 insertions(+), 72 deletions(-) diff --git a/config.json b/config.json index 2d10a13e8..058431ef4 100644 --- a/config.json +++ b/config.json @@ -152,8 +152,8 @@ }, "permissions": { "enabled": true, - "adminRoleId": null, - "moderatorRoleId": null, + "adminRoleIds": [], + "moderatorRoleIds": [], "botOwners": [], "usePermissions": true, "allowedCommands": { diff --git a/src/modules/moderation.js b/src/modules/moderation.js index cb25f3676..138da3b53 100644 --- a/src/modules/moderation.js +++ b/src/modules/moderation.js @@ -585,13 +585,17 @@ export function isProtectedTarget(target, guild) { return true; } + // Resolve admin/moderator role ID arrays with backward compat for old singular fields + const adminRoleIds = + config.permissions?.adminRoleIds ?? + (config.permissions?.adminRoleId ? [config.permissions.adminRoleId] : []); + const moderatorRoleIds = + config.permissions?.moderatorRoleIds ?? + (config.permissions?.moderatorRoleId ? [config.permissions.moderatorRoleId] : []); + const protectedRoleIds = [ - ...(protectRoles.includeAdmins && config.permissions?.adminRoleId - ? [config.permissions.adminRoleId] - : []), - ...(protectRoles.includeModerators && config.permissions?.moderatorRoleId - ? [config.permissions.moderatorRoleId] - : []), + ...(protectRoles.includeAdmins ? adminRoleIds : []), + ...(protectRoles.includeModerators ? moderatorRoleIds : []), ...(Array.isArray(protectRoles.roleIds) ? protectRoles.roleIds : []), ].filter(Boolean); diff --git a/src/utils/modExempt.js b/src/utils/modExempt.js index ddfc382cd..354b70155 100644 --- a/src/utils/modExempt.js +++ b/src/utils/modExempt.js @@ -12,10 +12,13 @@ import { PermissionFlagsBits } from 'discord.js'; * * Exempt if the member: * - has the ADMINISTRATOR Discord permission, OR - * - holds the role at `config.permissions.adminRoleId` (singular ID), OR - * - holds the role at `config.permissions.moderatorRoleId` (singular ID), OR + * - holds any role in `config.permissions.adminRoleIds` (array), OR + * - holds any role in `config.permissions.moderatorRoleIds` (array), OR * - holds any role ID or name listed in `config.permissions.modRoles` (array) * + * Backward compat: also checks singular `adminRoleId` / `moderatorRoleId` fields + * so old configs continue to work without migration. + * * @param {import('discord.js').Message} message * @param {Object} config - Merged guild config * @returns {boolean} @@ -27,11 +30,17 @@ export function isExempt(message, config) { // ADMINISTRATOR permission bypasses everything if (member.permissions.has(PermissionFlagsBits.Administrator)) return true; - // Singular role IDs — the actual config schema (permissions.adminRoleId / moderatorRoleId) - const adminRoleId = config.permissions?.adminRoleId; - const moderatorRoleId = config.permissions?.moderatorRoleId; - if (adminRoleId && member.roles.cache.has(adminRoleId)) return true; - if (moderatorRoleId && member.roles.cache.has(moderatorRoleId)) return true; + // Array role IDs — new schema (permissions.adminRoleIds / moderatorRoleIds) + // Backward compat: fall back to singular field if array field is absent + const adminRoleIds = + config.permissions?.adminRoleIds ?? + (config.permissions?.adminRoleId ? [config.permissions.adminRoleId] : []); + if (adminRoleIds.some((id) => member.roles.cache.has(id))) return true; + + const moderatorRoleIds = + config.permissions?.moderatorRoleIds ?? + (config.permissions?.moderatorRoleId ? [config.permissions.moderatorRoleId] : []); + if (moderatorRoleIds.some((id) => member.roles.cache.has(id))) return true; // Legacy / test-facing array of role IDs or names (permissions.modRoles) const modRoles = config.permissions?.modRoles ?? []; diff --git a/src/utils/permissions.js b/src/utils/permissions.js index f1fe4afdf..5a83c8006 100644 --- a/src/utils/permissions.js +++ b/src/utils/permissions.js @@ -62,9 +62,13 @@ export function isAdmin(member, config) { return true; } - // Check if member has the configured admin role - if (config.permissions?.adminRoleId) { - return member.roles.cache.has(config.permissions.adminRoleId); + // Check if member has any of the configured admin roles + // Backward compat: support old singular adminRoleId field + const adminRoleIds = + config.permissions?.adminRoleIds ?? + (config.permissions?.adminRoleId ? [config.permissions.adminRoleId] : []); + if (adminRoleIds.length > 0) { + return adminRoleIds.some((id) => member.roles.cache.has(id)); } return false; @@ -158,15 +162,22 @@ export function isModerator(member, config) { return true; } - // Check bot admin role from config - if (config.permissions?.adminRoleId) { - if (member.roles.cache.has(config.permissions.adminRoleId)) { - return true; - } + // Check bot admin roles from config + // Backward compat: support old singular adminRoleId field + const adminRoleIds = + config.permissions?.adminRoleIds ?? + (config.permissions?.adminRoleId ? [config.permissions.adminRoleId] : []); + if (adminRoleIds.some((id) => member.roles.cache.has(id))) { + return true; } - if (config.permissions?.moderatorRoleId) { - return member.roles.cache.has(config.permissions.moderatorRoleId); + // Check bot moderator roles from config + // Backward compat: support old singular moderatorRoleId field + const moderatorRoleIds = + config.permissions?.moderatorRoleIds ?? + (config.permissions?.moderatorRoleId ? [config.permissions.moderatorRoleId] : []); + if (moderatorRoleIds.some((id) => member.roles.cache.has(id))) { + return true; } return false; diff --git a/tests/utils/modExempt.test.js b/tests/utils/modExempt.test.js index 4cbecff92..25876b60b 100644 --- a/tests/utils/modExempt.test.js +++ b/tests/utils/modExempt.test.js @@ -48,30 +48,54 @@ describe('isExempt', () => { expect(isExempt(msg, {})).toBe(false); }); - it('should return true when member has adminRoleId', () => { + it('should return true when member has a role in adminRoleIds array', () => { const msg = makeMessage({ isAdmin: false, roleIds: ['admin-role-id'] }); - const config = { permissions: { adminRoleId: 'admin-role-id' } }; + const config = { permissions: { adminRoleIds: ['admin-role-id'] } }; expect(isExempt(msg, config)).toBe(true); }); - it('should return false when adminRoleId is set but member does not have it', () => { + it('should return true when member has any of multiple adminRoleIds', () => { + const msg = makeMessage({ isAdmin: false, roleIds: ['admin-role-2'] }); + const config = { permissions: { adminRoleIds: ['admin-role-1', 'admin-role-2'] } }; + expect(isExempt(msg, config)).toBe(true); + }); + + it('should return false when adminRoleIds is set but member does not have any', () => { const msg = makeMessage({ isAdmin: false, roleIds: ['other-role'] }); - const config = { permissions: { adminRoleId: 'admin-role-id' } }; + const config = { permissions: { adminRoleIds: ['admin-role-id'] } }; expect(isExempt(msg, config)).toBe(false); }); - it('should return true when member has moderatorRoleId', () => { + it('should return true when member has a role in moderatorRoleIds array', () => { const msg = makeMessage({ isAdmin: false, roleIds: ['mod-role-id'] }); - const config = { permissions: { moderatorRoleId: 'mod-role-id' } }; + const config = { permissions: { moderatorRoleIds: ['mod-role-id'] } }; expect(isExempt(msg, config)).toBe(true); }); - it('should return false when moderatorRoleId is set but member does not have it', () => { + it('should return true when member has any of multiple moderatorRoleIds', () => { + const msg = makeMessage({ isAdmin: false, roleIds: ['mod-role-2'] }); + const config = { permissions: { moderatorRoleIds: ['mod-role-1', 'mod-role-2'] } }; + expect(isExempt(msg, config)).toBe(true); + }); + + it('should return false when moderatorRoleIds is set but member does not have any', () => { const msg = makeMessage({ isAdmin: false, roleIds: [] }); - const config = { permissions: { moderatorRoleId: 'mod-role-id' } }; + const config = { permissions: { moderatorRoleIds: ['mod-role-id'] } }; expect(isExempt(msg, config)).toBe(false); }); + it('should support backward compat: singular adminRoleId still grants exemption', () => { + const msg = makeMessage({ isAdmin: false, roleIds: ['admin-role-id'] }); + const config = { permissions: { adminRoleId: 'admin-role-id' } }; + expect(isExempt(msg, config)).toBe(true); + }); + + it('should support backward compat: singular moderatorRoleId still grants exemption', () => { + const msg = makeMessage({ isAdmin: false, roleIds: ['mod-role-id'] }); + const config = { permissions: { moderatorRoleId: 'mod-role-id' } }; + expect(isExempt(msg, config)).toBe(true); + }); + it('should return true when member has a role ID in modRoles array', () => { const msg = makeMessage({ isAdmin: false, roleIds: ['custom-mod'] }); const config = { permissions: { modRoles: ['custom-mod'] } }; diff --git a/tests/utils/permissions.test.js b/tests/utils/permissions.test.js index 683d0b7aa..ed24f8ce4 100644 --- a/tests/utils/permissions.test.js +++ b/tests/utils/permissions.test.js @@ -86,32 +86,55 @@ describe('isAdmin', () => { expect(isAdmin(member, {})).toBe(true); }); - it('should return true for members with admin role', () => { + it('should return true for members with admin role (adminRoleIds array)', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, roles: { cache: { has: vi.fn().mockReturnValue(true) } }, }; - const config = { permissions: { adminRoleId: '123456' } }; + const config = { permissions: { adminRoleIds: ['123456'] } }; expect(isAdmin(member, config)).toBe(true); expect(member.roles.cache.has).toHaveBeenCalledWith('123456'); }); + it('should return true for members with any of multiple admin roles', () => { + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { + cache: { + has: vi.fn().mockImplementation((id) => id === '999999'), + }, + }, + }; + const config = { permissions: { adminRoleIds: ['123456', '999999'] } }; + expect(isAdmin(member, config)).toBe(true); + }); + it('should return false for regular members', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, roles: { cache: { has: vi.fn().mockReturnValue(false) } }, }; - const config = { permissions: { adminRoleId: '123456' } }; + const config = { permissions: { adminRoleIds: ['123456'] } }; expect(isAdmin(member, config)).toBe(false); }); - it('should return false when no adminRoleId configured and not Admin', () => { + it('should return false when no adminRoleIds configured and not Admin', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, roles: { cache: { has: vi.fn() } }, }; expect(isAdmin(member, {})).toBe(false); }); + + it('should support backward compat: singular adminRoleId still works', () => { + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { cache: { has: vi.fn().mockReturnValue(true) } }, + }; + const config = { permissions: { adminRoleId: '123456' } }; + expect(isAdmin(member, config)).toBe(true); + expect(member.roles.cache.has).toHaveBeenCalledWith('123456'); + }); }); describe('hasPermission', () => { @@ -320,12 +343,12 @@ describe('isGuildAdmin', () => { expect(isGuildAdmin(member, {})).toBe(true); }); - it('should return true for members with admin role', () => { + it('should return true for members with admin role (adminRoleIds array)', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, roles: { cache: { has: vi.fn().mockReturnValue(true) } }, }; - const config = { permissions: { adminRoleId: '123456' } }; + const config = { permissions: { adminRoleIds: ['123456'] } }; expect(isGuildAdmin(member, config)).toBe(true); expect(member.roles.cache.has).toHaveBeenCalledWith('123456'); }); @@ -382,26 +405,52 @@ describe('isModerator', () => { expect(isModerator(member, {})).toBe(true); }); - it('should return true for members with admin role', () => { + it('should return true for members with admin role (adminRoleIds array)', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, roles: { cache: { has: vi.fn().mockReturnValue(true) } }, }; - const config = { permissions: { adminRoleId: '123456' } }; + const config = { permissions: { adminRoleIds: ['123456'] } }; expect(isModerator(member, config)).toBe(true); expect(member.roles.cache.has).toHaveBeenCalledWith('123456'); }); - it('should return true for members with moderator role', () => { + it('should return true for members with any of multiple admin roles', () => { + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { + cache: { + has: vi.fn().mockImplementation((id) => id === '999999'), + }, + }, + }; + const config = { permissions: { adminRoleIds: ['123456', '999999'] } }; + expect(isModerator(member, config)).toBe(true); + }); + + it('should return true for members with moderator role (moderatorRoleIds array)', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, roles: { cache: { has: vi.fn().mockReturnValue(true) } }, }; - const config = { permissions: { moderatorRoleId: '654321' } }; + const config = { permissions: { moderatorRoleIds: ['654321'] } }; expect(isModerator(member, config)).toBe(true); expect(member.roles.cache.has).toHaveBeenCalledWith('654321'); }); + it('should return true for members with any of multiple moderator roles', () => { + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { + cache: { + has: vi.fn().mockImplementation((id) => id === '888888'), + }, + }, + }; + const config = { permissions: { moderatorRoleIds: ['654321', '888888'] } }; + expect(isModerator(member, config)).toBe(true); + }); + it('should return true for moderator role when admin and moderator roles are both configured', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, @@ -412,13 +461,33 @@ describe('isModerator', () => { }, }; const config = { - permissions: { adminRoleId: '123456', moderatorRoleId: '654321' }, + permissions: { adminRoleIds: ['123456'], moderatorRoleIds: ['654321'] }, }; expect(isModerator(member, config)).toBe(true); expect(member.roles.cache.has).toHaveBeenCalledWith('123456'); expect(member.roles.cache.has).toHaveBeenCalledWith('654321'); }); + it('should support backward compat: singular adminRoleId still works', () => { + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { cache: { has: vi.fn().mockReturnValue(true) } }, + }; + const config = { permissions: { adminRoleId: '123456' } }; + expect(isModerator(member, config)).toBe(true); + expect(member.roles.cache.has).toHaveBeenCalledWith('123456'); + }); + + it('should support backward compat: singular moderatorRoleId still works', () => { + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { cache: { has: vi.fn().mockReturnValue(true) } }, + }; + const config = { permissions: { moderatorRoleId: '654321' } }; + expect(isModerator(member, config)).toBe(true); + expect(member.roles.cache.has).toHaveBeenCalledWith('654321'); + }); + it('should return false for regular members', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, diff --git a/web/src/components/dashboard/config-editor.tsx b/web/src/components/dashboard/config-editor.tsx index 51efec570..fd962040e 100644 --- a/web/src/components/dashboard/config-editor.tsx +++ b/web/src/components/dashboard/config-editor.tsx @@ -1994,40 +1994,26 @@ export function ConfigEditor() { disabled={saving} basicContent={
-
diff --git a/web/src/types/config.ts b/web/src/types/config.ts index 64a337f2c..27870e632 100644 --- a/web/src/types/config.ts +++ b/web/src/types/config.ts @@ -188,8 +188,8 @@ export interface StarboardConfig { /** Permissions configuration. */ export interface PermissionsConfig { enabled: boolean; - adminRoleId: string | null; - moderatorRoleId: string | null; + adminRoleIds: string[]; + moderatorRoleIds: string[]; modRoles: string[]; botOwners: string[]; usePermissions: boolean; From 6bf104a4e1bbc502dab0549681f859cac3922b99 Mon Sep 17 00:00:00 2001 From: Bill Chirico Date: Sat, 7 Mar 2026 18:30:56 -0500 Subject: [PATCH 2/9] feat(permissions): support multiple admin and moderator roles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - adminRoleId/moderatorRoleId (singular) → adminRoleIds/moderatorRoleIds (arrays) - isAdmin()/isModerator() check member.roles against any role in array - Backward compat: falls back to singular field for old configs - modExempt.js and moderation.js updated to use arrays - Dashboard RoleSelectors now true multi-select - Tests updated with multi-role coverage --- TASK.md | 77 +++++++++++++++++++ .../dashboard/performance-dashboard.tsx | 8 +- 2 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 TASK.md diff --git a/TASK.md b/TASK.md new file mode 100644 index 000000000..6bc563c43 --- /dev/null +++ b/TASK.md @@ -0,0 +1,77 @@ +# TASK: Multi-role permissions (admin + moderator) + +Branch: `feat/multi-role-permissions` +Repo: VolvoxLLC/volvox-bot +Work in: `/home/bill/worktrees/volvox-bot-multi-roles` + +## Goal +Allow configuring multiple Discord roles as admin and multiple roles as moderator, instead of a single role each. + +## Schema change +- `config.permissions.adminRoleId: string | null` → `config.permissions.adminRoleIds: string[]` +- `config.permissions.moderatorRoleId: string | null` → `config.permissions.moderatorRoleIds: string[]` + +## Files to change + +### 1. `src/config.json` (or wherever defaults live) +- Find the permissions defaults +- Change `adminRoleId: null` → `adminRoleIds: []` +- Change `moderatorRoleId: null` → `moderatorRoleIds: []` + +### 2. `web/src/types/config.ts` +- Line ~191: Change `adminRoleId: string | null` → `adminRoleIds: string[]` +- Line ~192: Change `moderatorRoleId: string | null` → `moderatorRoleIds: string[]` + +### 3. `src/utils/permissions.js` +- Line 66: `config.permissions?.adminRoleId` check → check if member has ANY role in `config.permissions?.adminRoleIds ?? []` + ```js + const adminRoleIds = config.permissions?.adminRoleIds ?? []; + if (adminRoleIds.length > 0) { + return adminRoleIds.some(id => member.roles.cache.has(id)); + } + ``` +- Line 162-169: Same pattern for both admin and moderator checks +- Keep backward compat: if `adminRoleId` (singular) exists in config, treat it as `[adminRoleId]` so old configs still work + +### 4. `src/utils/modExempt.js` +- Update to check array: `(config.permissions?.adminRoleIds ?? []).some(id => member.roles.cache.has(id))` +- Same for moderatorRoleIds +- Keep backward compat for old singular field + +### 5. `src/modules/moderation.js` +- Lines 589-594: Spread the arrays for protect roles: + ```js + ...(protectRoles.includeAdmins ? (config.permissions?.adminRoleIds ?? []) : []), + ...(protectRoles.includeModerators ? (config.permissions?.moderatorRoleIds ?? []) : []), + ``` + +### 6. `web/src/components/dashboard/config-editor.tsx` +- The RoleSelector for admin is currently single-select (wraps value in array, takes `selected[0]`). Change to true multi-select: + - `selected={draftConfig.permissions?.adminRoleIds ?? []}` (no wrapping) + - `onChange={(selected) => updatePermissionsField('adminRoleIds', selected)}` + - Remove `maxSelections={1}` if present +- Same for moderator: + - `selected={draftConfig.permissions?.moderatorRoleIds ?? []}` + - `onChange={(selected) => updatePermissionsField('moderatorRoleIds', selected)}` + +### 7. Check for any other references to old singular fields +```bash +grep -rn "adminRoleId\b\|moderatorRoleId\b" src/ web/src/ --include="*.js" --include="*.ts" --include="*.tsx" +``` +Fix any remaining references. + +### 8. Update tests +- `tests/utils/permissions.test.js` — update mocks and assertions to use arrays +- `tests/utils/modExempt.test.js` — same + +## Backward compat pattern +In permissions.js and modExempt.js, support old configs that have the singular field: +```js +const adminRoleIds = config.permissions?.adminRoleIds + ?? (config.permissions?.adminRoleId ? [config.permissions.adminRoleId] : []); +``` + +## Rules +- Conventional commits +- Run `pnpm format && pnpm lint && pnpm test` and `pnpm --prefix web lint && pnpm --prefix web typecheck` +- Do NOT push diff --git a/web/src/components/dashboard/performance-dashboard.tsx b/web/src/components/dashboard/performance-dashboard.tsx index 15edfe1c8..8028267c3 100644 --- a/web/src/components/dashboard/performance-dashboard.tsx +++ b/web/src/components/dashboard/performance-dashboard.tsx @@ -360,9 +360,7 @@ export function PerformanceDashboard() { - (v != null ? [`${v} MB`] : [''])} - /> + (v != null ? [`${v} MB`] : [''])} /> - (v != null ? [`${v}%`] : [''])} - /> + (v != null ? [`${v}%`] : [''])} /> Date: Sat, 7 Mar 2026 23:37:18 +0000 Subject: [PATCH 3/9] =?UTF-8?q?=F0=9F=93=9D=20Add=20docstrings=20to=20`fea?= =?UTF-8?q?t/multi-role-permissions`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docstrings generation was requested by @BillChirico. The following files were modified: * `src/modules/moderation.js` * `src/utils/permissions.js` * `web/src/components/dashboard/config-editor.tsx` * `web/src/components/dashboard/performance-dashboard.tsx` These files were kept as they were: * `src/utils/modExempt.js` These files were ignored: * `tests/utils/modExempt.test.js` * `tests/utils/permissions.test.js` These file types are not supported: * `TASK.md` * `config.json` --- src/modules/moderation.js | 11 +++++------ src/utils/permissions.js | 17 +++++++++-------- web/src/components/dashboard/config-editor.tsx | 9 +++++---- .../dashboard/performance-dashboard.tsx | 9 +++++++++ 4 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/modules/moderation.js b/src/modules/moderation.js index 138da3b53..f3fd872a2 100644 --- a/src/modules/moderation.js +++ b/src/modules/moderation.js @@ -550,12 +550,11 @@ export function stopTempbanScheduler() { } /** - * Check if a target member is protected from moderation actions. - * Protected members include the server owner, admins, moderators, and any custom role IDs - * configured under `moderation.protectRoles`. - * @param {import('discord.js').GuildMember} target - Target member to check - * @param {import('discord.js').Guild} guild - Discord guild - * @returns {boolean} True if the target should not be moderated + * Determine whether a guild member is protected from moderation actions. + * Protection is driven by the guild's live moderation.protectRoles settings (server owner, admin/moderator roles, and explicit role IDs). + * @param {import('discord.js').GuildMember} target - Member to evaluate. + * @param {import('discord.js').Guild} guild - Guild containing the member. + * @returns {boolean} `true` if the member is protected from moderation actions, `false` otherwise. */ export function isProtectedTarget(target, guild) { // Fetch config per-invocation so live config edits take effect immediately. diff --git a/src/utils/permissions.js b/src/utils/permissions.js index 5a83c8006..f255a7af9 100644 --- a/src/utils/permissions.js +++ b/src/utils/permissions.js @@ -43,11 +43,11 @@ export function isBotOwner(member, config) { } /** - * Check if a member is an admin + * Determine whether a guild member has administrative privileges. * - * @param {GuildMember} member - Discord guild member - * @param {Object} config - Bot configuration - * @returns {boolean} True if member is admin + * @param {GuildMember} member - The guild member to check. + * @param {Object} config - Bot configuration containing permission role IDs. + * @returns {boolean} `true` if the member is an admin, `false` otherwise. */ export function isAdmin(member, config) { if (!member) return false; @@ -138,11 +138,12 @@ export function isGuildAdmin(member, config) { } /** - * Check if a member is a moderator (has MANAGE_GUILD permission or bot admin role) + * Determine whether a guild member is considered a moderator. * - * @param {GuildMember} member - Discord guild member - * @param {Object} config - Bot configuration - * @returns {boolean} True if member is a moderator + * Considers bot owners, members with the Administrator or Manage Guild permission, and members with any configured admin or moderator role IDs (supports legacy singular role ID fields). + * @param {GuildMember} member - Discord guild member to check. + * @param {Object} config - Bot configuration containing permission role settings (e.g., permissions.adminRoleIds, permissions.moderatorRoleIds or legacy adminRoleId/moderatorRoleId). + * @returns {boolean} `true` if the member is a moderator, `false` otherwise. */ export function isModerator(member, config) { if (!member) return false; diff --git a/web/src/components/dashboard/config-editor.tsx b/web/src/components/dashboard/config-editor.tsx index fd962040e..df9afa5cf 100644 --- a/web/src/components/dashboard/config-editor.tsx +++ b/web/src/components/dashboard/config-editor.tsx @@ -130,11 +130,12 @@ function isGuildConfig(data: unknown): data is GuildConfig { } /** - * Edit a guild's bot configuration through a multi-section UI. + * Edit a guild's bot configuration via a categorized editor with per-section controls. * - * Loads the authoritative config for the selected guild, maintains a mutable draft for user edits, - * computes and applies per-section patches to persist changes, and provides controls to save, - * discard, and validate edits (including an unsaved-changes warning and keyboard shortcut). + * Loads the authoritative config for the selected guild, keeps an editable draft, validates edits, + * and persists changes as batched, top-level section PATCHes. Provides discard and undo flows, + * an unsaved-changes warning, keyboard shortcuts (Ctrl/Cmd+S to open the diff preview, '/' to focus + * search), and a diff modal to review or revert per-section changes before saving. * * @returns The editor UI as JSX when a guild is selected and a draft config exists; `null` otherwise. */ diff --git a/web/src/components/dashboard/performance-dashboard.tsx b/web/src/components/dashboard/performance-dashboard.tsx index 8028267c3..d405c64c2 100644 --- a/web/src/components/dashboard/performance-dashboard.tsx +++ b/web/src/components/dashboard/performance-dashboard.tsx @@ -119,6 +119,15 @@ function StatCard({ title, value, subtitle, icon: Icon, alert, loading }: StatCa const AUTO_REFRESH_MS = 30_000; +/** + * Render the performance dashboard UI showing memory, CPU, and response-time metrics, + * including time-series charts, a response-time histogram, recent samples, and editable alert thresholds. + * + * The dashboard auto-refreshes in the background at a fixed interval, allows manual refresh, + * seeds the threshold editor from fetched data, and provides saving of threshold changes. + * + * @returns The dashboard UI as a JSX element. + */ export function PerformanceDashboard() { const [data, setData] = useState(null); const [loading, setLoading] = useState(false); From 10cccf4d49b73cdf9f6691dfd60fe56bdf3e3671 Mon Sep 17 00:00:00 2001 From: Bill Chirico Date: Sat, 7 Mar 2026 18:38:15 -0500 Subject: [PATCH 4/9] fix: sort imports in dbMaintenance.js (biome CI) --- src/utils/dbMaintenance.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/dbMaintenance.js b/src/utils/dbMaintenance.js index 77a5da7f0..51f5d2e4f 100644 --- a/src/utils/dbMaintenance.js +++ b/src/utils/dbMaintenance.js @@ -9,8 +9,8 @@ */ import { info, error as logError, warn } from '../logger.js'; -import { getConfig } from '../modules/config.js'; import { purgeOldAuditLogs } from '../modules/auditLogger.js'; +import { getConfig } from '../modules/config.js'; /** Track optional tables we've already warned about to avoid hourly log spam */ const warnedMissingOptionalTables = new Set(); From f341b2ea6a76503b9daeac2892827cb0c3d03356 Mon Sep 17 00:00:00 2001 From: Bill Chirico Date: Sat, 7 Mar 2026 18:52:06 -0500 Subject: [PATCH 5/9] fix(permissions): correct backward compat for merged legacy config shape The ?? fallback broke when defaults inject adminRoleIds:[] alongside a legacy guild config with adminRoleId set. Switch to mergeRoleIds() which always combines both fields regardless of array emptiness. - Extract mergeRoleIds() helper (exported) in permissions.js - Use mergeRoleIds() in permissions.js, modExempt.js, moderation.js - Dashboard hydrates legacy adminRoleId/moderatorRoleId into selectors and clears them on first multi-select interaction - Add deprecated singular fields to TS type for compat - Add merged-config test cases to permissions and modExempt tests --- src/modules/moderation.js | 18 +++++--- src/utils/modExempt.js | 18 +++++--- src/utils/permissions.js | 43 +++++++++++++------ tests/utils/modExempt.test.js | 12 ++++++ tests/utils/permissions.test.js | 20 +++++++++ .../components/dashboard/config-editor.tsx | 36 ++++++++++++++-- web/src/types/config.ts | 4 ++ 7 files changed, 121 insertions(+), 30 deletions(-) diff --git a/src/modules/moderation.js b/src/modules/moderation.js index f3fd872a2..141dcf04e 100644 --- a/src/modules/moderation.js +++ b/src/modules/moderation.js @@ -9,6 +9,7 @@ import { getPool } from '../db.js'; import { info, error as logError, warn as logWarn } from '../logger.js'; import { fetchChannelCached } from '../utils/discordCache.js'; import { parseDuration } from '../utils/duration.js'; +import { mergeRoleIds } from '../utils/permissions.js'; import { safeSend } from '../utils/safeSend.js'; import { getConfig } from './config.js'; import { fireEvent } from './webhookNotifier.js'; @@ -584,13 +585,16 @@ export function isProtectedTarget(target, guild) { return true; } - // Resolve admin/moderator role ID arrays with backward compat for old singular fields - const adminRoleIds = - config.permissions?.adminRoleIds ?? - (config.permissions?.adminRoleId ? [config.permissions.adminRoleId] : []); - const moderatorRoleIds = - config.permissions?.moderatorRoleIds ?? - (config.permissions?.moderatorRoleId ? [config.permissions.moderatorRoleId] : []); + // Resolve admin/moderator role ID arrays — mergeRoleIds handles the case where + // defaults inject adminRoleIds:[] alongside a legacy adminRoleId guild override + const adminRoleIds = mergeRoleIds( + config.permissions?.adminRoleIds, + config.permissions?.adminRoleId, + ); + const moderatorRoleIds = mergeRoleIds( + config.permissions?.moderatorRoleIds, + config.permissions?.moderatorRoleId, + ); const protectedRoleIds = [ ...(protectRoles.includeAdmins ? adminRoleIds : []), diff --git a/src/utils/modExempt.js b/src/utils/modExempt.js index 354b70155..27b562ea2 100644 --- a/src/utils/modExempt.js +++ b/src/utils/modExempt.js @@ -5,6 +5,7 @@ */ import { PermissionFlagsBits } from 'discord.js'; +import { mergeRoleIds } from './permissions.js'; /** * Check whether a message author has mod/admin permissions and should be @@ -31,15 +32,18 @@ export function isExempt(message, config) { if (member.permissions.has(PermissionFlagsBits.Administrator)) return true; // Array role IDs — new schema (permissions.adminRoleIds / moderatorRoleIds) - // Backward compat: fall back to singular field if array field is absent - const adminRoleIds = - config.permissions?.adminRoleIds ?? - (config.permissions?.adminRoleId ? [config.permissions.adminRoleId] : []); + // Use mergeRoleIds to handle configs that have both the new empty-array default + // AND the old singular field set from a legacy guild override. + const adminRoleIds = mergeRoleIds( + config.permissions?.adminRoleIds, + config.permissions?.adminRoleId, + ); if (adminRoleIds.some((id) => member.roles.cache.has(id))) return true; - const moderatorRoleIds = - config.permissions?.moderatorRoleIds ?? - (config.permissions?.moderatorRoleId ? [config.permissions.moderatorRoleId] : []); + const moderatorRoleIds = mergeRoleIds( + config.permissions?.moderatorRoleIds, + config.permissions?.moderatorRoleId, + ); if (moderatorRoleIds.some((id) => member.roles.cache.has(id))) return true; // Legacy / test-facing array of role IDs or names (permissions.modRoles) diff --git a/src/utils/permissions.js b/src/utils/permissions.js index f255a7af9..77773a656 100644 --- a/src/utils/permissions.js +++ b/src/utils/permissions.js @@ -6,6 +6,24 @@ import { PermissionFlagsBits } from 'discord.js'; +/** + * Merge the new plural role IDs array with the legacy singular field. + * + * After defaults are merged, old guild configs will have BOTH `roleIds: []` + * (from defaults) AND `roleId: 'abc'` (from their stored override). Using `??` + * alone misses this case because the empty array is truthy. We always combine + * both so no configured role is ever silently dropped. + * + * @param {string[]} [roleIds=[]] - New plural field (may be empty from defaults) + * @param {string|null} [roleId=null] - Legacy singular field + * @returns {string[]} Deduplicated merged list + */ +export function mergeRoleIds(roleIds, roleId) { + const merged = [...(roleIds ?? [])]; + if (roleId && !merged.includes(roleId)) merged.push(roleId); + return merged; +} + /** * Retrieve the configured bot owner user IDs. * @@ -63,10 +81,11 @@ export function isAdmin(member, config) { } // Check if member has any of the configured admin roles - // Backward compat: support old singular adminRoleId field - const adminRoleIds = - config.permissions?.adminRoleIds ?? - (config.permissions?.adminRoleId ? [config.permissions.adminRoleId] : []); + // mergeRoleIds handles the case where defaults inject adminRoleIds:[] alongside a legacy adminRoleId value + const adminRoleIds = mergeRoleIds( + config.permissions?.adminRoleIds, + config.permissions?.adminRoleId, + ); if (adminRoleIds.length > 0) { return adminRoleIds.some((id) => member.roles.cache.has(id)); } @@ -164,19 +183,19 @@ export function isModerator(member, config) { } // Check bot admin roles from config - // Backward compat: support old singular adminRoleId field - const adminRoleIds = - config.permissions?.adminRoleIds ?? - (config.permissions?.adminRoleId ? [config.permissions.adminRoleId] : []); + const adminRoleIds = mergeRoleIds( + config.permissions?.adminRoleIds, + config.permissions?.adminRoleId, + ); if (adminRoleIds.some((id) => member.roles.cache.has(id))) { return true; } // Check bot moderator roles from config - // Backward compat: support old singular moderatorRoleId field - const moderatorRoleIds = - config.permissions?.moderatorRoleIds ?? - (config.permissions?.moderatorRoleId ? [config.permissions.moderatorRoleId] : []); + const moderatorRoleIds = mergeRoleIds( + config.permissions?.moderatorRoleIds, + config.permissions?.moderatorRoleId, + ); if (moderatorRoleIds.some((id) => member.roles.cache.has(id))) { return true; } diff --git a/tests/utils/modExempt.test.js b/tests/utils/modExempt.test.js index 25876b60b..0494468c1 100644 --- a/tests/utils/modExempt.test.js +++ b/tests/utils/modExempt.test.js @@ -96,6 +96,18 @@ describe('isExempt', () => { expect(isExempt(msg, config)).toBe(true); }); + it('should grant exemption via legacy adminRoleId even when adminRoleIds:[] default is present (merged config)', () => { + const msg = makeMessage({ isAdmin: false, roleIds: ['legacy-admin'] }); + const config = { permissions: { adminRoleIds: [], adminRoleId: 'legacy-admin' } }; + expect(isExempt(msg, config)).toBe(true); + }); + + it('should grant exemption via legacy moderatorRoleId even when moderatorRoleIds:[] default is present (merged config)', () => { + const msg = makeMessage({ isAdmin: false, roleIds: ['legacy-mod'] }); + const config = { permissions: { moderatorRoleIds: [], moderatorRoleId: 'legacy-mod' } }; + expect(isExempt(msg, config)).toBe(true); + }); + it('should return true when member has a role ID in modRoles array', () => { const msg = makeMessage({ isAdmin: false, roleIds: ['custom-mod'] }); const config = { permissions: { modRoles: ['custom-mod'] } }; diff --git a/tests/utils/permissions.test.js b/tests/utils/permissions.test.js index ed24f8ce4..14a2ba30b 100644 --- a/tests/utils/permissions.test.js +++ b/tests/utils/permissions.test.js @@ -135,6 +135,17 @@ describe('isAdmin', () => { expect(isAdmin(member, config)).toBe(true); expect(member.roles.cache.has).toHaveBeenCalledWith('123456'); }); + + it('should find legacy adminRoleId even when adminRoleIds:[] default is present (merged config)', () => { + // This is the real breaking case: defaults merge in adminRoleIds:[] before guild overrides + // apply, so the config has BOTH fields. ?? alone would miss the legacy value. + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { cache: { has: (id) => id === 'legacy-role-789' } }, + }; + const config = { permissions: { adminRoleIds: [], adminRoleId: 'legacy-role-789' } }; + expect(isAdmin(member, config)).toBe(true); + }); }); describe('hasPermission', () => { @@ -488,6 +499,15 @@ describe('isModerator', () => { expect(member.roles.cache.has).toHaveBeenCalledWith('654321'); }); + it('should find legacy moderatorRoleId even when moderatorRoleIds:[] default is present (merged config)', () => { + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { cache: { has: (id) => id === 'legacy-mod-999' } }, + }; + const config = { permissions: { moderatorRoleIds: [], moderatorRoleId: 'legacy-mod-999' } }; + expect(isModerator(member, config)).toBe(true); + }); + it('should return false for regular members', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, diff --git a/web/src/components/dashboard/config-editor.tsx b/web/src/components/dashboard/config-editor.tsx index df9afa5cf..938d08aa3 100644 --- a/web/src/components/dashboard/config-editor.tsx +++ b/web/src/components/dashboard/config-editor.tsx @@ -2000,8 +2000,22 @@ export function ConfigEditor() { updatePermissionsField('adminRoleIds', selected)} + selected={[ + ...(draftConfig.permissions?.adminRoleIds ?? []), + ...(draftConfig.permissions?.adminRoleId && + !(draftConfig.permissions?.adminRoleIds ?? []).includes( + draftConfig.permissions.adminRoleId, + ) + ? [draftConfig.permissions.adminRoleId] + : []), + ]} + onChange={(selected) => { + updatePermissionsField('adminRoleIds', selected); + // Clear legacy field once user has interacted with the multi-selector + if (draftConfig.permissions?.adminRoleId) { + updatePermissionsField('adminRoleId', null); + } + }} placeholder="Select admin roles" disabled={saving} /> @@ -2011,8 +2025,22 @@ export function ConfigEditor() { updatePermissionsField('moderatorRoleIds', selected)} + selected={[ + ...(draftConfig.permissions?.moderatorRoleIds ?? []), + ...(draftConfig.permissions?.moderatorRoleId && + !(draftConfig.permissions?.moderatorRoleIds ?? []).includes( + draftConfig.permissions.moderatorRoleId, + ) + ? [draftConfig.permissions.moderatorRoleId] + : []), + ]} + onChange={(selected) => { + updatePermissionsField('moderatorRoleIds', selected); + // Clear legacy field once user has interacted with the multi-selector + if (draftConfig.permissions?.moderatorRoleId) { + updatePermissionsField('moderatorRoleId', null); + } + }} placeholder="Select moderator roles" disabled={saving} /> diff --git a/web/src/types/config.ts b/web/src/types/config.ts index 27870e632..84b646979 100644 --- a/web/src/types/config.ts +++ b/web/src/types/config.ts @@ -190,6 +190,10 @@ export interface PermissionsConfig { enabled: boolean; adminRoleIds: string[]; moderatorRoleIds: string[]; + /** @deprecated Use adminRoleIds. Kept for backward compat with legacy guild configs. */ + adminRoleId?: string | null; + /** @deprecated Use moderatorRoleIds. Kept for backward compat with legacy guild configs. */ + moderatorRoleId?: string | null; modRoles: string[]; botOwners: string[]; usePermissions: boolean; From f41423cd12d246591097b215df6d5e34d586376d Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Sat, 7 Mar 2026 23:54:17 +0000 Subject: [PATCH 6/9] fix(api): add permissions section to CONFIG_SCHEMA for role array validation adminRoleIds and moderatorRoleIds PATCH requests previously bypassed type validation because CONFIG_SCHEMA had no permissions entry. An unknown top-level section short-circuits validateSingleValue, allowing malformed data (e.g. a string instead of an array) to be persisted. Co-authored-by: Bill Chirico --- TASK.md | 77 ------------------------------- src/api/utils/configValidation.js | 12 +++++ 2 files changed, 12 insertions(+), 77 deletions(-) delete mode 100644 TASK.md diff --git a/TASK.md b/TASK.md deleted file mode 100644 index 6bc563c43..000000000 --- a/TASK.md +++ /dev/null @@ -1,77 +0,0 @@ -# TASK: Multi-role permissions (admin + moderator) - -Branch: `feat/multi-role-permissions` -Repo: VolvoxLLC/volvox-bot -Work in: `/home/bill/worktrees/volvox-bot-multi-roles` - -## Goal -Allow configuring multiple Discord roles as admin and multiple roles as moderator, instead of a single role each. - -## Schema change -- `config.permissions.adminRoleId: string | null` → `config.permissions.adminRoleIds: string[]` -- `config.permissions.moderatorRoleId: string | null` → `config.permissions.moderatorRoleIds: string[]` - -## Files to change - -### 1. `src/config.json` (or wherever defaults live) -- Find the permissions defaults -- Change `adminRoleId: null` → `adminRoleIds: []` -- Change `moderatorRoleId: null` → `moderatorRoleIds: []` - -### 2. `web/src/types/config.ts` -- Line ~191: Change `adminRoleId: string | null` → `adminRoleIds: string[]` -- Line ~192: Change `moderatorRoleId: string | null` → `moderatorRoleIds: string[]` - -### 3. `src/utils/permissions.js` -- Line 66: `config.permissions?.adminRoleId` check → check if member has ANY role in `config.permissions?.adminRoleIds ?? []` - ```js - const adminRoleIds = config.permissions?.adminRoleIds ?? []; - if (adminRoleIds.length > 0) { - return adminRoleIds.some(id => member.roles.cache.has(id)); - } - ``` -- Line 162-169: Same pattern for both admin and moderator checks -- Keep backward compat: if `adminRoleId` (singular) exists in config, treat it as `[adminRoleId]` so old configs still work - -### 4. `src/utils/modExempt.js` -- Update to check array: `(config.permissions?.adminRoleIds ?? []).some(id => member.roles.cache.has(id))` -- Same for moderatorRoleIds -- Keep backward compat for old singular field - -### 5. `src/modules/moderation.js` -- Lines 589-594: Spread the arrays for protect roles: - ```js - ...(protectRoles.includeAdmins ? (config.permissions?.adminRoleIds ?? []) : []), - ...(protectRoles.includeModerators ? (config.permissions?.moderatorRoleIds ?? []) : []), - ``` - -### 6. `web/src/components/dashboard/config-editor.tsx` -- The RoleSelector for admin is currently single-select (wraps value in array, takes `selected[0]`). Change to true multi-select: - - `selected={draftConfig.permissions?.adminRoleIds ?? []}` (no wrapping) - - `onChange={(selected) => updatePermissionsField('adminRoleIds', selected)}` - - Remove `maxSelections={1}` if present -- Same for moderator: - - `selected={draftConfig.permissions?.moderatorRoleIds ?? []}` - - `onChange={(selected) => updatePermissionsField('moderatorRoleIds', selected)}` - -### 7. Check for any other references to old singular fields -```bash -grep -rn "adminRoleId\b\|moderatorRoleId\b" src/ web/src/ --include="*.js" --include="*.ts" --include="*.tsx" -``` -Fix any remaining references. - -### 8. Update tests -- `tests/utils/permissions.test.js` — update mocks and assertions to use arrays -- `tests/utils/modExempt.test.js` — same - -## Backward compat pattern -In permissions.js and modExempt.js, support old configs that have the singular field: -```js -const adminRoleIds = config.permissions?.adminRoleIds - ?? (config.permissions?.adminRoleId ? [config.permissions.adminRoleId] : []); -``` - -## Rules -- Conventional commits -- Run `pnpm format && pnpm lint && pnpm test` and `pnpm --prefix web lint && pnpm --prefix web typecheck` -- Do NOT push diff --git a/src/api/utils/configValidation.js b/src/api/utils/configValidation.js index 5c98f4163..8f0b5ac37 100644 --- a/src/api/utils/configValidation.js +++ b/src/api/utils/configValidation.js @@ -199,6 +199,18 @@ export const CONFIG_SCHEMA = { logChannel: { type: 'string', nullable: true }, }, }, + permissions: { + type: 'object', + properties: { + enabled: { type: 'boolean' }, + usePermissions: { type: 'boolean' }, + adminRoleIds: { type: 'array', items: { type: 'string' } }, + moderatorRoleIds: { type: 'array', items: { type: 'string' } }, + modRoles: { type: 'array', items: { type: 'string' } }, + botOwners: { type: 'array', items: { type: 'string' } }, + allowedCommands: { type: 'object' }, + }, + }, }; /** From 03d0ae1b0cf9e12cff7d9af7f36680bff3cd22cb Mon Sep 17 00:00:00 2001 From: Bill Chirico Date: Sat, 7 Mar 2026 19:41:03 -0500 Subject: [PATCH 7/9] fix(permissions): add permissions schema to configValidation; harden mergeRoleIds - Add permissions block to CONFIG_SCHEMA (adminRoleIds, moderatorRoleIds, legacy singular fields, botOwners, modRoles) so PATCH validation works for existing guilds using the old singular fields - mergeRoleIds now defensively handles non-array roleIds (string or malformed persisted config) using a Set to deduplicate --- src/api/utils/configValidation.js | 3 +++ src/utils/permissions.js | 17 ++++++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/api/utils/configValidation.js b/src/api/utils/configValidation.js index 8f0b5ac37..cfc3ec2ec 100644 --- a/src/api/utils/configValidation.js +++ b/src/api/utils/configValidation.js @@ -206,6 +206,9 @@ export const CONFIG_SCHEMA = { usePermissions: { type: 'boolean' }, adminRoleIds: { type: 'array', items: { type: 'string' } }, moderatorRoleIds: { type: 'array', items: { type: 'string' } }, + // Legacy singular fields — kept for backward compat during migration + adminRoleId: { type: 'string', nullable: true }, + moderatorRoleId: { type: 'string', nullable: true }, modRoles: { type: 'array', items: { type: 'string' } }, botOwners: { type: 'array', items: { type: 'string' } }, allowedCommands: { type: 'object' }, diff --git a/src/utils/permissions.js b/src/utils/permissions.js index 77773a656..0bdea0d9a 100644 --- a/src/utils/permissions.js +++ b/src/utils/permissions.js @@ -19,9 +19,20 @@ import { PermissionFlagsBits } from 'discord.js'; * @returns {string[]} Deduplicated merged list */ export function mergeRoleIds(roleIds, roleId) { - const merged = [...(roleIds ?? [])]; - if (roleId && !merged.includes(roleId)) merged.push(roleId); - return merged; + // Normalize roleIds defensively — persisted config may contain a string instead of an array + let base; + if (Array.isArray(roleIds)) { + base = roleIds; + } else if (typeof roleIds === 'string' && roleIds.length > 0) { + base = [roleIds]; + } else { + base = []; + } + const merged = new Set(base); + if (typeof roleId === 'string' && roleId.length > 0) { + merged.add(roleId); + } + return [...merged]; } /** From 473ee55ff8ccce5f6ed28b33be4c97bfa586dd16 Mon Sep 17 00:00:00 2001 From: Bill Chirico Date: Sat, 7 Mar 2026 21:15:04 -0500 Subject: [PATCH 8/9] test(permissions): add dedicated mergeRoleIds describe block with 13 cases Covers: array merge, dedup, empty array + singular, null/undefined fallbacks, malformed string normalization, empty string guards, and the production merged-config failure scenario (defaults inject [] + legacy guild override). --- tests/utils/permissions.test.js | 57 +++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/utils/permissions.test.js b/tests/utils/permissions.test.js index 14a2ba30b..9d61a9950 100644 --- a/tests/utils/permissions.test.js +++ b/tests/utils/permissions.test.js @@ -16,6 +16,7 @@ import { isBotOwner, isGuildAdmin, isModerator, + mergeRoleIds, } from '../../src/utils/permissions.js'; const BOT_OWNER_ID = '191633014441115648'; @@ -559,3 +560,59 @@ describe('isBotOwner', () => { expect(isBotOwner(member, config)).toBe(false); }); }); + +describe('mergeRoleIds', () => { + it('merges a non-empty array with a singular id', () => { + expect(mergeRoleIds(['a', 'b'], 'c')).toEqual(['a', 'b', 'c']); + }); + + it('deduplicates when singular id is already in array', () => { + expect(mergeRoleIds(['a', 'b'], 'a')).toEqual(['a', 'b']); + }); + + it('handles empty array + singular id', () => { + expect(mergeRoleIds([], 'abc')).toEqual(['abc']); + }); + + it('handles array only (no singular id)', () => { + expect(mergeRoleIds(['x', 'y'], null)).toEqual(['x', 'y']); + }); + + it('handles null array + singular id (legacy-only config)', () => { + expect(mergeRoleIds(null, 'legacy-id')).toEqual(['legacy-id']); + }); + + it('handles undefined array + singular id (defaults not merged yet)', () => { + expect(mergeRoleIds(undefined, 'legacy-id')).toEqual(['legacy-id']); + }); + + it('handles both null — returns empty array', () => { + expect(mergeRoleIds(null, null)).toEqual([]); + }); + + it('normalizes a string roleIds to single-element array (malformed config)', () => { + expect(mergeRoleIds('malformed-string-id', null)).toEqual(['malformed-string-id']); + }); + + it('string roleIds + singular id deduplicates if same', () => { + expect(mergeRoleIds('role-123', 'role-123')).toEqual(['role-123']); + }); + + it('string roleIds + different singular id merges both', () => { + expect(mergeRoleIds('role-abc', 'role-xyz')).toEqual(['role-abc', 'role-xyz']); + }); + + it('empty string roleId is ignored', () => { + expect(mergeRoleIds(['a'], '')).toEqual(['a']); + }); + + it('empty string roleIds falls back to empty array', () => { + expect(mergeRoleIds('', 'abc')).toEqual(['abc']); + }); + + it('real merged-config case: defaults inject [] alongside legacy guild override', () => { + // This is the production failure scenario: defaults merge adminRoleIds:[] before + // guild overrides apply, so config has BOTH fields. + expect(mergeRoleIds([], 'legacy-guild-role')).toEqual(['legacy-guild-role']); + }); +}); From 4ae0333843845bb100051d4ab1e98a88753156a9 Mon Sep 17 00:00:00 2001 From: Bill Chirico Date: Sat, 7 Mar 2026 23:40:02 -0500 Subject: [PATCH 9/9] fix(permissions): address PR #261 review comments - tests: add isModerator merged-config test for legacy adminRoleId (admin-first check) - configValidation: allowedCommands uses openProperties:true to allow freeform command keys without triggering unknown-key validation errors - config-editor: updatePermissionsField auto-nulls legacy singular fields when plural arrays are written, so computePatches stays clean regardless of which field the user edits --- src/api/utils/configValidation.js | 6 ++++-- tests/utils/permissions.test.js | 12 ++++++++++++ web/src/components/dashboard/config-editor.tsx | 15 ++++++--------- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/api/utils/configValidation.js b/src/api/utils/configValidation.js index cfc3ec2ec..5d8f1b565 100644 --- a/src/api/utils/configValidation.js +++ b/src/api/utils/configValidation.js @@ -211,7 +211,8 @@ export const CONFIG_SCHEMA = { moderatorRoleId: { type: 'string', nullable: true }, modRoles: { type: 'array', items: { type: 'string' } }, botOwners: { type: 'array', items: { type: 'string' } }, - allowedCommands: { type: 'object' }, + // allowedCommands is a freeform map of command → permission level — no fixed property list + allowedCommands: { type: 'object', openProperties: true }, }, }, }; @@ -289,9 +290,10 @@ export function validateValue(value, schema, path) { for (const [key, val] of Object.entries(value)) { if (Object.hasOwn(schema.properties, key)) { errors.push(...validateValue(val, schema.properties[key], `${path}.${key}`)); - } else { + } else if (!schema.openProperties) { errors.push(`${path}.${key}: unknown config key`); } + // openProperties: true — freeform map, unknown keys are allowed } } break; diff --git a/tests/utils/permissions.test.js b/tests/utils/permissions.test.js index 9d61a9950..4a1898a6a 100644 --- a/tests/utils/permissions.test.js +++ b/tests/utils/permissions.test.js @@ -509,6 +509,18 @@ describe('isModerator', () => { expect(isModerator(member, config)).toBe(true); }); + it('should grant moderator via legacy adminRoleId even when adminRoleIds:[] default is present', () => { + // isModerator() checks admin roles first — legacy adminRoleId must be found + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { cache: { has: (id) => id === 'legacy-admin-123' } }, + }; + const config = { + permissions: { adminRoleIds: [], adminRoleId: 'legacy-admin-123', moderatorRoleIds: [] }, + }; + expect(isModerator(member, config)).toBe(true); + }); + it('should return false for regular members', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, diff --git a/web/src/components/dashboard/config-editor.tsx b/web/src/components/dashboard/config-editor.tsx index 938d08aa3..55c97a35d 100644 --- a/web/src/components/dashboard/config-editor.tsx +++ b/web/src/components/dashboard/config-editor.tsx @@ -872,7 +872,12 @@ export function ConfigEditor() { (field: string, value: unknown) => { updateDraftConfig((prev) => { if (!prev) return prev; - return { ...prev, permissions: { ...prev.permissions, [field]: value } } as GuildConfig; + const updated = { ...prev.permissions, [field]: value }; + // Auto-clear legacy singular fields whenever the plural arrays are written, + // so computePatches never includes both the old and new forms simultaneously. + if (field === 'adminRoleIds') updated.adminRoleId = null; + if (field === 'moderatorRoleIds') updated.moderatorRoleId = null; + return { ...prev, permissions: updated } as GuildConfig; }); }, [updateDraftConfig], @@ -2011,10 +2016,6 @@ export function ConfigEditor() { ]} onChange={(selected) => { updatePermissionsField('adminRoleIds', selected); - // Clear legacy field once user has interacted with the multi-selector - if (draftConfig.permissions?.adminRoleId) { - updatePermissionsField('adminRoleId', null); - } }} placeholder="Select admin roles" disabled={saving} @@ -2036,10 +2037,6 @@ export function ConfigEditor() { ]} onChange={(selected) => { updatePermissionsField('moderatorRoleIds', selected); - // Clear legacy field once user has interacted with the multi-selector - if (draftConfig.permissions?.moderatorRoleId) { - updatePermissionsField('moderatorRoleId', null); - } }} placeholder="Select moderator roles" disabled={saving}