feat: add starboard, permissions, memory, and rate limit config sections to web dashboard#100
Conversation
…ons to web dashboard
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR expands the bot's configuration system by introducing new configuration sections (starboard, permissions, memory) and nested features (rate limiting and link filtering under moderation), updating the type definitions, allowlist validation, and dashboard editor UI accordingly. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Claude finished @BillChirico's task in 2m 35s —— View job Review of PR #100 — 2 warnings, 2 nitpicks
Most issues from previous review rounds have been fixed in the latest commits (parseNumberInput min mismatch, Remaining issues:
See inline comments for details and fix instructions. |
|
| Filename | Overview |
|---|---|
| web/src/types/config.ts | Added comprehensive type definitions for starboard, permissions, memory, rate limit, and link filter configs. Types are well-structured and match backend expectations, but some fields not exposed in UI. |
| web/src/components/dashboard/config-editor.tsx | Added UI sections for starboard, permissions, memory, rate limit, and link filter. Uses parseNumberInput helper consistently, but missing step={1} on integer inputs and some permission fields not exposed. |
| src/api/utils/configAllowlist.js | Added starboard, permissions, and memory to SAFE_CONFIG_KEYS. Correctly moved them from READABLE_CONFIG_KEYS. However, starboard backend implementation doesn't exist yet. |
| tests/api/utils/configAllowlist.test.js | Added test assertions for new config keys in both SAFE_CONFIG_KEYS and READABLE_CONFIG_KEYS. Tests are complete and correct. |
Last reviewed commit: 6a78c97
There was a problem hiding this comment.
Review Summary — 2 warnings, 3 nitpicks
🟡 Warnings:
PermissionsConfigtype mismatch (config.ts:119-123):adminRoleIdandmoderatorRoleIdtyped asstringbut backendconfig.jsondefaults them tonull. Should bestring | null.- Number inputs accept invalid values (
config-editor.tsx, all new number inputs):onChangehandlers don't enforce theminconstraint — negative/zero values pass through. Addif (num < 1) return;afterisFinitechecks. - Missing tests (
config-editor.tsx): No tests added for new sections. AGENTS.md requires tests for all new code.
🔵 Nitpicks:
4. Stale JSDoc (config-editor.tsx:34-36): isGuildConfig comment lists 5 sections but code checks 8.
5. Incomplete PermissionsConfig type (config.ts:118-123): Missing usePermissions and allowedCommands fields from backend.
…nstraints, update JSDoc
There was a problem hiding this comment.
Review Summary — 2 warnings, 1 nitpick (+ 1 carried-forward warning)
🟡 Warnings:
allowedCommandstype mismatch (config.ts:124):Record<string, string[]>should beRecord<string, string>. Backendconfig.jsonandpermissions.jsuse single-string values, not arrays.- Number inputs missing min enforcement (
config-editor.tsx:697-776,1187-1192): Rate limit + memory inputs still use the old inline pattern that doesn't enforcemin. TheparseNumberInputhelper was added but only applied to starboard threshold. - Missing tests (
config-editor.tsx): No test file exists. AGENTS.md requires tests for all new code.
🔵 Nitpick:
4. statusReactions not in backend (config.ts:160): Not referenced anywhere in src/.
AI fix prompt (copy-paste into an AI agent)
Fix the following issues in the feat/config-editor-sections branch of VolvoxLLC/volvox-bot:
-
web/src/types/config.tsline 124: ChangeallowedCommands: Record<string, string[]>toallowedCommands: Record<string, string>. The backendconfig.jsonmaps command names to single permission level strings ("everyone", "admin", "moderator"), not arrays. -
web/src/components/dashboard/config-editor.tsxlines 697-776 and 1187-1192: Replace all inline number parsing in rate limit onChange handlers and the memory maxContextMemories onChange handler with the existingparseNumberInputhelper. Pattern to use:
onChange={(e) => {
const num = parseNumberInput(e.target.value, 1);
if (num !== undefined) updateRateLimitField("fieldName", num);
}}Apply this to: maxMessages, windowSeconds, muteAfterTriggers, muteWindowSeconds, muteDurationSeconds (all with min=1), and memory maxContextMemories (min=1).
- Create test file
web/tests/components/dashboard/config-editor.test.tsx(or.test.ts) with tests covering:isGuildConfigrecognizing new sections (starboard, permissions, memory)parseNumberInputenforcing min/max constraints- New updater callbacks producing correct state
- Rendering new sections with default config data
… use parseNumberInput everywhere
There was a problem hiding this comment.
Review Summary — 3 warnings, 1 nitpick
Most previously reported issues from the first two reviews have been addressed in commit 7b5beaa. A few items remain:
🟡 Warnings:
parseNumberInputmin param mismatch (config-editor.tsx, 6 locations): All rate limit number inputs and memorymaxContextMemoriespassmin=0toparseNumberInputbut havemin={1}in the HTML attribute. This allows0to be saved, which is functionally invalid for these fields (0 messages, 0 seconds, 0 memories). Fix: changeparseNumberInput(e.target.value, 0)toparseNumberInput(e.target.value, 1)on lines 698, 712, 728, 742, 756, and 1155.- Missing config-editor tests: No test file exists for
config-editor.tsx. AGENTS.md states: "Any new code must include tests — PRs that drop coverage below 80% will fail CI." At minimum,isGuildConfigandparseNumberInputshould be unit-tested. - Missing test assertions for new allowlist keys (
tests/api/utils/configAllowlist.test.js):starboard,permissions, andmemorywere added toSAFE_CONFIG_KEYSbut the test doesn't verify them.starboardis also missing fromREADABLE_CONFIG_KEYSassertions.
🔵 Nitpick:
4. statusReactions not in backend (config.ts:160): Field added to TriageConfig and UI, but src/modules/triage.js doesn't reference it. Consider adding a comment noting this is a planned feature.
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 (2)
web/src/components/dashboard/config-editor.tsx (1)
240-245:⚠️ Potential issue | 🟠 Major401 save path can still fall through into normal completion UX.
On unauthorized responses,
sendSectionredirects but returns normally. The outer flow can still continue and show success/partial-save messaging before navigation completes.Suggested fix
const failedSections: string[] = []; + let redirectedToLogin = false; async function sendSection(sectionPatches: Array<{ path: string; value: unknown }>) { for (const patch of sectionPatches) { const res = await fetch( `/api/guilds/${encodeURIComponent(guildId)}/config`, { @@ if (res.status === 401) { // Abort all other in-flight requests before redirecting + redirectedToLogin = true; saveAbortController.abort(); window.location.href = "/login"; - return; + throw new Error("UNAUTHORIZED_REDIRECT"); } @@ try { const results = await Promise.allSettled( @@ ); + + if (redirectedToLogin) return; const hasFailures = results.some((r) => r.status === "rejected"); @@ } catch (err) { + if (redirectedToLogin) return; const msg = (err as Error).message || "Failed to save config"; toast.error("Failed to save config", { description: msg }); } finally {Also applies to: 256-299
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/dashboard/config-editor.tsx` around lines 240 - 245, The 401 handling in sendSection (where saveAbortController.abort() and window.location.href = "/login" are called) returns normally and allows outer success/partial-save UX to run; after redirect you must stop further processing by throwing or returning a rejected promise. Update the 401 branches in sendSection (and the similar blocks later) to abort and then immediately throw an Error (or return Promise.reject(new Error('Unauthorized'))) so callers don't proceed to success/partial-save logic; use the same pattern in all 401 handling spots to ensure the outer flow is terminated.web/src/types/config.ts (1)
107-177:⚠️ Potential issue | 🟠 MajorDocumentation missing for new config sections in README.md
The PR adds three new configuration sections (
starboard,permissions,memory) to the type definitions, but documentation is incomplete inREADME.md:
permissions— documented ✓starboard— not documented (missing all fields:enabled,channelId,threshold,emoji,selfStarAllowed,ignoredChannels)memory— not documented (missing all fields:enabled,maxContextMemories,autoExtract)Add config tables for the missing sections following the existing pattern in the Configuration section of
README.md.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/types/config.ts` around lines 107 - 177, Add missing documentation for the StarboardConfig and MemoryConfig types to README.md by adding two new configuration tables in the Configuration section that follow the existing pattern used for other sections (e.g., PermissionsConfig): create a "Starboard" table documenting each field from StarboardConfig (enabled, channelId, threshold, emoji, selfStarAllowed, ignoredChannels) with type, description, and default/example values; create a "Memory" table documenting each field from MemoryConfig (enabled, maxContextMemories, autoExtract) likewise; ensure these sections mention where they appear in the UI/API (matching how other sections like "permissions" are described) and include the corresponding ConfigSection entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/components/dashboard/config-editor.tsx`:
- Around line 1093-1094: Normalize empty-string role IDs to null before updating
the config: when handling onChange for the inputs that call
updatePermissionsField("adminRoleId", ...) and
updatePermissionsField("moderatorRoleId", ...), convert e.target.value === '' to
null (or alternatively implement this coercion inside updatePermissionsField
itself) so the config stores null for cleared values instead of ''. Keep
disabled={saving} as-is.
- Around line 698-699: The positive-only inputs call parseNumberInput(..., 0)
which allows 0 despite the UI min={1}; update those calls to use
parseNumberInput(..., 1) so zero cannot be parsed/persisted, e.g. change the
parseNumberInput call paired with updateRateLimitField("maxMessages", ...) and
the other similar handlers to pass 1 as the minimum; apply this change for every
positive-only field referenced in the comment so parseNumberInput enforces a
minimum of 1.
---
Outside diff comments:
In `@web/src/components/dashboard/config-editor.tsx`:
- Around line 240-245: The 401 handling in sendSection (where
saveAbortController.abort() and window.location.href = "/login" are called)
returns normally and allows outer success/partial-save UX to run; after redirect
you must stop further processing by throwing or returning a rejected promise.
Update the 401 branches in sendSection (and the similar blocks later) to abort
and then immediately throw an Error (or return Promise.reject(new
Error('Unauthorized'))) so callers don't proceed to success/partial-save logic;
use the same pattern in all 401 handling spots to ensure the outer flow is
terminated.
In `@web/src/types/config.ts`:
- Around line 107-177: Add missing documentation for the StarboardConfig and
MemoryConfig types to README.md by adding two new configuration tables in the
Configuration section that follow the existing pattern used for other sections
(e.g., PermissionsConfig): create a "Starboard" table documenting each field
from StarboardConfig (enabled, channelId, threshold, emoji, selfStarAllowed,
ignoredChannels) with type, description, and default/example values; create a
"Memory" table documenting each field from MemoryConfig (enabled,
maxContextMemories, autoExtract) likewise; ensure these sections mention where
they appear in the UI/API (matching how other sections like "permissions" are
described) and include the corresponding ConfigSection entries.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/api/utils/configAllowlist.jsweb/src/components/dashboard/config-editor.tsxweb/src/types/config.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Greptile Review
- GitHub Check: claude-review
🧰 Additional context used
📓 Path-based instructions (2)
**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
**/*.js: Use ESM modules only — useimport/export, neverrequire()
Usenode:protocol for Node.js builtins (e.g.import { readFileSync } from 'node:fs')
Always use semicolons
Use single quotes for strings
Use 2-space indentation
No TypeScript — use plain JavaScript with JSDoc comments for documentation
Files:
src/api/utils/configAllowlist.js
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.js: Always use Winston for logging — import{ info, warn, error }from../logger.js
NEVER useconsole.log,console.warn,console.error, or anyconsole.*method in src/ files
Pass structured metadata to Winston logging calls (e.g.info('Message processed', { userId, channelId }))
Use custom error classes fromsrc/utils/errors.jsfor error handling
Always log errors with context before re-throwing
UsegetConfig(guildId?)fromsrc/modules/config.jsto read config
UsesetConfigValue(path, value, guildId?)fromsrc/modules/config.jsto update config at runtime
UsesplitMessage()utility for messages exceeding Discord's 2000-character limit
UsesafeSend()wrapper for outgoing Discord messages to sanitize mentions and enforce allowedMentions
Files:
src/api/utils/configAllowlist.js
🧠 Learnings (8)
📚 Learning: 2026-02-25T02:39:33.506Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T02:39:33.506Z
Learning: Update `config.json` documentation in README.md when adding new config sections or keys
Applied to files:
web/src/types/config.ts
📚 Learning: 2026-02-25T02:39:33.506Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T02:39:33.506Z
Learning: Applies to **/*.js : No TypeScript — use plain JavaScript with JSDoc comments for documentation
Applied to files:
web/src/components/dashboard/config-editor.tsx
📚 Learning: 2026-02-25T02:39:33.506Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T02:39:33.506Z
Learning: Applies to tests/**/*.js : Test files must achieve at least 80% code coverage on statements, branches, functions, and lines
Applied to files:
web/src/components/dashboard/config-editor.tsx
📚 Learning: 2026-02-25T02:39:33.506Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T02:39:33.506Z
Learning: Any new code must include tests before being committed
Applied to files:
web/src/components/dashboard/config-editor.tsx
📚 Learning: 2026-02-25T02:39:33.506Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T02:39:33.506Z
Learning: Applies to src/**/*.js : Use `setConfigValue(path, value, guildId?)` from `src/modules/config.js` to update config at runtime
Applied to files:
web/src/components/dashboard/config-editor.tsx
📚 Learning: 2026-02-25T02:39:33.506Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T02:39:33.506Z
Learning: Applies to src/commands/*mod*.js : Send DM notifications to moderation targets *before* executing kicks/bans (once kicked/banned, users cannot receive DMs from the bot)
Applied to files:
web/src/components/dashboard/config-editor.tsx
📚 Learning: 2026-02-25T02:39:33.506Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T02:39:33.506Z
Learning: Applies to src/modules/{ai,spam,moderation}.js : Per-request modules (AI, spam, moderation) should call `getConfig(interaction.guildId)` on every invocation for automatic hot-reload support
Applied to files:
web/src/components/dashboard/config-editor.tsx
📚 Learning: 2026-02-25T02:39:33.506Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T02:39:33.506Z
Learning: Applies to src/**/*.js : Use `getConfig(guildId?)` from `src/modules/config.js` to read config
Applied to files:
web/src/components/dashboard/config-editor.tsx
🔇 Additional comments (3)
src/api/utils/configAllowlist.js (2)
7-16: Allowlist expansion is correct and aligned with the new config model.Adding
starboard,permissions, andmemorytoSAFE_CONFIG_KEYSis the right change to unblock API read/write for the new sections.
18-18: Readable keys derivation is a good anti-drift change.Building
READABLE_CONFIG_KEYSfromSAFE_CONFIG_KEYSand appending'logging'keeps allowlists consistent over time.web/src/types/config.ts (1)
79-177: Type surface expansion is coherent with the new sections.The added interfaces and union updates are internally consistent and map cleanly to the new dashboard sections/sub-sections.
There was a problem hiding this comment.
3 warnings, 2 nitpicks — see inline comments.
🟡 Warnings:
- Missing
config.jsonstarboard defaults + README docs (config.ts:115): Nostarboardblock inconfig.json— first save creates it from UI-hardcoded fallbacks. README missing config tables forstarboardandmemory. - Missing
modRolesinPermissionsConfig(config.ts:125): Backend readsconfig.permissions?.modRolesbut type omits it. - Missing config-editor tests (
config-editor.tsx:51): No test file exists forparseNumberInputorisGuildConfig. AGENTS.md requires tests for all new code.
🔵 Nitpicks:
4. statusReactions has no backend consumer (config.ts:160): Toggle persists but bot won't act on it.
5. Integer fields accept decimals (config-editor.tsx:694): Count-based inputs missing step={1}.
…json, starboard/modRoles README docs
There was a problem hiding this comment.
Review Summary — 2 warnings, 2 nitpicks
Most issues from previous review rounds (parseNumberInput min mismatch, allowedCommands type, nullable role IDs, 401 throw, config.json starboard defaults, modRoles field, allowlist test coverage, README starboard/permissions docs) have been fixed in the latest commits. Well done on addressing feedback iteratively.
🟡 Warnings:
- Missing config-editor tests (
config-editor.tsx:38): No test file exists forparseNumberInputorisGuildConfig. AGENTS.md requires tests for all new code. - Missing memory config docs in README (
config.ts:132): README has config tables forstarboardandpermissionsbut notmemory. AGENTS.md requires documenting new config sections.
🔵 Nitpicks:
3. statusReactions not consumed by backend (config.ts:161): Toggle persists but src/modules/triage.js doesn't reference it.
4. Integer fields accept decimals (config-editor.tsx:694): Count-based inputs missing step={1} HTML attribute.
AI fix prompt (copy-paste into an AI agent)
Fix the following issues on the feat/config-editor-sections branch of VolvoxLLC/volvox-bot:
-
Create
web/tests/components/dashboard/config-editor.test.ts(or.test.tsx) with unit tests covering:isGuildConfigrecognizing new sections (starboard, permissions, memory), rejecting arrays/nulls/non-objectsparseNumberInputenforcing min/max constraints (empty string → undefined, non-finite → undefined, below min → clamped to min, above max → clamped to max, valid → number)
Note:isGuildConfigandparseNumberInputare module-private. Export them for direct testing or test indirectly.
-
README.md: Add a memory config reference table after the Permissions section, following the existing format:### Memory (`memory`) | Key | Type | Description | |-----|------|-------------| | `enabled` | boolean | Enable AI context memory | | `maxContextMemories` | number | Maximum memories injected into AI context (default: 5) | | `autoExtract` | boolean | Automatically extract and store memories from conversations |
-
web/src/types/config.tsline 161: Add/** @todo implement in triage module */comment abovestatusReactions. -
web/src/components/dashboard/config-editor.tsx: Addstep={1}to all count-based number inputs: maxMessages (line 694), muteAfterTriggers (line 723), starboard threshold (line 1018), maxContextMemories (line 1168).
Summary
Adds missing config sections to the web dashboard config editor that already exist in the bot backend.
New top-level sections
New moderation sub-sections
Additions to existing sections
Bonus fix
DeepPartialspread inference TS errors in allsetDraftConfigupdater callbacks by addingas GuildConfigcastsFiles changed
web/src/types/config.ts— new interfaces (StarboardConfig,RateLimitConfig,LinkFilterConfig,PermissionsConfig,MemoryConfig), updatedModerationConfig,TriageConfig,BotConfig, andConfigSectionweb/src/components/dashboard/config-editor.tsx— new UI sections, updater callbacks,isGuildConfigupdatedChecks
tsc --noEmit: 0 errors in modified files (fixed pre-existing issues)biome check: 0 errorsvitest run: 74 test files, 1538 tests passed