feat: enhanced onboarding MVP (rules accept, role menu, intro prompts)#149
feat: enhanced onboarding MVP (rules accept, role menu, intro prompts)#149BillChirico merged 12 commits intomainfrom
Conversation
|
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 (7)
📝 WalkthroughWalkthroughImplements enhanced server onboarding with support for welcome DM sequences, role selection menus, rules agreement flows, and returning member detection. Extends configuration schema, adds onboarding handlers, introduces a /welcome setup command, and expands the web dashboard with configuration UI for the new onboarding features. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 4m 39s —— View job Review Complete — 6 issues found
Most issues from prior reviews have been addressed across the 12 fix commits. The core implementation is solid — proper Remaining issues:
See the PR review for full details and a copy-paste AI fix prompt. |
There was a problem hiding this comment.
Pull request overview
Adds a configurable onboarding MVP to the Discord bot, extending the existing welcome system with rules acceptance, self-assignable roles, intro prompting, DM onboarding steps, and returning-member messaging.
Changes:
- Extend
welcomeconfig schema/types and dashboard editor to support onboarding fields (rules/role/intro channels, role menu, DM steps). - Add onboarding module +
/welcome setupcommand and persistent interaction handlers (rules button + role select menu). - Add/adjust tests to cover onboarding defaults/handlers and config validation paths.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/types/config.ts | Adds typed onboarding config structures under welcome. |
| web/src/components/dashboard/config-editor.tsx | Adds dashboard controls + parsing/serialization for role menu options and DM steps. |
| src/modules/welcomeOnboarding.js | Implements onboarding normalization, rules accept handler, and role menu handler. |
| src/modules/welcome.js | Adds returning-member branch in welcome message flow. |
| src/modules/events.js | Registers new InteractionCreate handlers for onboarding components. |
| src/commands/welcome.js | Adds /welcome setup admin helper command to post onboarding panels. |
| src/api/utils/configValidation.js | Extends writable config validation schema for new onboarding keys. |
| tests/modules/welcomeOnboarding.test.js | Adds unit tests for onboarding defaults, role menu limit, rules acceptance, and role updates. |
| tests/api/utils/configValidation.test.js | Adds validation coverage for new welcome onboarding fields. |
| tests/api/utils/redisClient.coverage.test.js | Formatting/import ordering adjustments. |
| tests/api/routes/guilds.coverage.test.js | Formatting adjustments in test fixtures/token creation. |
| config.json | Adds welcome onboarding defaults + minor string/formatting updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/utils/configValidation.js`:
- Around line 52-64: Update the JSON schema for roleMenu and dmSequence so their
arrays validate item shapes instead of accepting any values: replace the current
roleMenu.options: { type: 'array' } with an items schema that enforces each
option object’s expected properties (e.g., label:string, value:string, ... and
required fields) and replace dmSequence.steps: { type: 'array' } with an items
schema that enforces each step object shape (e.g., type:string, content:object,
order:number or other required fields). Ensure you reference and modify
roleMenu.options and dmSequence.steps in configValidation.js and add required
constraints where applicable so malformed payloads fail validation.
In `@src/commands/welcome.js`:
- Around line 52-66: The role menu panel currently reads the raw setting via
guildConfig?.welcome?.channelId while other panels use the normalized onboarding
config (e.g., onboarding.rulesChannel); update the code to use a single
normalized property or add the normalized welcome channel to the normalized
config. Specifically, either read the welcome channel from the normalized object
(e.g., onboarding.welcomeChannel or onboarding.welcome?.channelId) instead of
guildConfig, or modify the normalization function that produces onboarding (the
code that provides onboarding.rulesChannel) to populate a welcome channel ID so
buildRoleMenuMessage/roleMenuMsg logic can consistently use
onboarding.welcomeChannel; adjust references in the role menu block
(roleMenuMsg, buildRoleMenuMessage, and any place using
guildConfig?.welcome?.channelId) to the new normalized field.
- Around line 12-17: This command is missing the required adminOnly export for
moderator-only commands; add an export (export const adminOnly = true) alongside
the existing exports (the SlashCommandBuilder `data` export and the `execute()`
function) in src/commands/welcome.js so auto-discovery recognizes it as a
mod-only command—leave the existing in-function permission checks in `execute()`
intact.
In `@src/modules/events.js`:
- Around line 534-537: The onboarding button handler is running even when the
welcome module is disabled; before processing the RULES_ACCEPT_BUTTON_ID path
(the block under interaction.isButton() && interaction.customId ===
RULES_ACCEPT_BUTTON_ID) fetch the guild config via getConfig(guildId) and guard
the flow with a check that guildConfig.welcome && guildConfig.welcome.enabled
(or equivalent optional chaining guildConfig.welcome?.enabled) so the
role-assignment/onboarding logic only runs when the welcome module is enabled.
In `@src/modules/welcomeOnboarding.js`:
- Line 148: Wrap the call to member.roles.add(role, 'Accepted server rules') in
a try-catch inside the same handler (where interaction is available); on success
continue as before, on failure catch the error, log it (using your logger or
console.error) including the error object and role/member identifiers, and send
a follow-up or reply to the interaction (e.g., interaction.reply or
interaction.followUp) with a friendly error message so the user gets feedback
and the promise rejection is handled. Ensure you reference member.roles.add and
the interaction reply method in the same scope so the catch can notify the user.
- Around line 163-171: In the DM send loop that iterates
welcome.dmSequence.steps and calls interaction.user.send(step), replace the
empty catch with a catch that captures the error (catch (err)) and logs a
concise diagnostic (including interaction.user.id and/or interaction.user.tag
and the step index or content) before breaking; use the existing logger instance
(e.g., processLogger or logger) if available, or console.warn as a fallback, so
failures to deliver DMs are observable while keeping the break behavior.
In `@tests/api/utils/configValidation.test.js`:
- Around line 70-75: Update the test in tests/api/utils/configValidation.test.js
that uses validateSingleValue to also assert valid handling of the new key
'welcome.introChannel' (e.g., expect validateSingleValue('welcome.introChannel',
'123') toEqual []), and add negative cases for malformed onboarding payloads:
call validateSingleValue with invalid nested values such as
'welcome.roleMenu.enabled' set to a non-boolean, 'welcome.dmSequence.steps' set
to a non-array or containing non-string items, and 'welcome.verifiedRole' set to
null/invalid ID, asserting those return non-empty error arrays; target the
existing test block and the validateSingleValue function to ensure the test
covers both new field acceptance and rejection paths for malformed nested
values.
In `@tests/modules/welcomeOnboarding.test.js`:
- Around line 28-99: Add unit tests to reach the missing coverage: write tests
for isReturningMember() (returning vs new member behaviors) and
buildRulesAgreementMessage() (message structure and required fields); extend
handleRulesAcceptButton tests to cover verifiedRole unset, role not found in
guild.roles.cache or fetch failing, role present but editable=false, and the DM
sequence path where dmSequence.enabled=true and steps execute; add
handleRoleMenuSelection tests for roleMenu disabled/empty and the case where
selected roles require no changes; and expand normalizeWelcomeOnboardingConfig
tests to validate option filtering (invalid entries dropped), max-option
enforcement, and description handling. Target the existing test file structure
and use the helpers/mocks already present (e.g., interaction, member, guild,
safeSend/safeReply) to assert expected calls and error handling for each named
function: isReturningMember, buildRulesAgreementMessage,
handleRulesAcceptButton, handleRoleMenuSelection,
normalizeWelcomeOnboardingConfig.
In `@web/src/components/dashboard/config-editor.tsx`:
- Around line 40-56: The role-menu textarea parsing currently normalizes and
filters input on every onChange (via parseRoleMenuOptions), which drops
in-progress lines; change the component to keep a raw textarea state (e.g.,
roleMenuRaw) that is updated directly on onChange without aggressive
trimming/filtering, and only call parseRoleMenuOptions (the existing function)
on onBlur (or on explicit save) to produce the parsed options/state used
elsewhere (e.g., roleMenuOptions); ensure the textarea value is bound to
roleMenuRaw and that parseRoleMenuOptions still performs final
trimming/filtering when invoked so partial edits are preserved while typing.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
config.jsonsrc/api/utils/configValidation.jssrc/commands/welcome.jssrc/modules/events.jssrc/modules/welcome.jssrc/modules/welcomeOnboarding.jstests/api/routes/guilds.coverage.test.jstests/api/utils/configValidation.test.jstests/api/utils/redisClient.coverage.test.jstests/modules/welcomeOnboarding.test.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: Agent
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (7)
**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
**/*.js: Use ESM modules only — import/export, never require()
Always use node: protocol for Node.js builtin imports (e.g., import { readFileSync } from 'node:fs')
Files:
src/commands/welcome.jssrc/modules/events.jssrc/api/utils/configValidation.jstests/api/routes/guilds.coverage.test.jstests/api/utils/configValidation.test.jstests/api/utils/redisClient.coverage.test.jssrc/modules/welcomeOnboarding.jstests/modules/welcomeOnboarding.test.jssrc/modules/welcome.js
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,tsx}: Always use semicolons
Use single quotes — enforced by Biome
Use 2-space indentation — enforced by Biome
Files:
src/commands/welcome.jsweb/src/types/config.tssrc/modules/events.jssrc/api/utils/configValidation.jstests/api/routes/guilds.coverage.test.jstests/api/utils/configValidation.test.jstests/api/utils/redisClient.coverage.test.jssrc/modules/welcomeOnboarding.jstests/modules/welcomeOnboarding.test.jsweb/src/components/dashboard/config-editor.tsxsrc/modules/welcome.js
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.js: NEVER use console.log, console.warn, console.error, or any console.* method in src/ files — always use Winston logger instead: import { info, warn, error } from '../logger.js'
Pass structured metadata to Winston logger: info('Message processed', { userId, channelId })
Use custom error classes from src/utils/errors.js and always log errors with context before re-throwing
Use getConfig(guildId?) from src/modules/config.js to read config; use setConfigValue(path, value, guildId?) to update at runtime
Use safeSend() utility for outgoing Discord messages to sanitize mentions and enforce allowedMentions
Use splitMessage() utility for messages exceeding Discord's 2000-character limit
Add tests for all new code with mandatory 80% coverage threshold on statements, branches, functions, and lines; run pnpm test before every commit
Files:
src/commands/welcome.jssrc/modules/events.jssrc/api/utils/configValidation.jssrc/modules/welcomeOnboarding.jssrc/modules/welcome.js
src/commands/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/commands/*.js: Use parseDuration() from src/utils/duration.js for duration-based commands (timeout, tempban, slowmode); enforce Discord duration caps (timeouts max 28 days, slowmode max 6 hours)
Create slash commands by exporting data (SlashCommandBuilder) and execute() function from src/commands/*.js; export adminOnly = true for mod-only commands; commands are auto-discovered on startup
Files:
src/commands/welcome.js
web/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use next/image Image component with appropriate layout and sizing props in Next.js components
Files:
web/src/types/config.tsweb/src/components/dashboard/config-editor.tsx
web/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use Zustand store (zustand) for state management in React components; implement fetch-on-demand pattern in stores
Files:
web/src/types/config.tsweb/src/components/dashboard/config-editor.tsx
src/modules/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/modules/*.js: Register event handlers in src/modules/events.js by importing handler functions and calling client.on() with config parameter
Check config.yourModule.enabled before processing in module event handlers
Prefer per-request getConfig() pattern in new modules over reactive onConfigChange() wiring; only add onConfigChange() listeners for stateful resources that cannot re-read config on each use
Files:
src/modules/events.jssrc/modules/welcomeOnboarding.jssrc/modules/welcome.js
🧠 Learnings (6)
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/commands/*.js : Create slash commands by exporting data (SlashCommandBuilder) and execute() function from src/commands/*.js; export adminOnly = true for mod-only commands; commands are auto-discovered on startup
Applied to files:
src/commands/welcome.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/commands/*mod*.js : Moderation commands must follow the shared pattern: deferReply({ ephemeral: true }), validate inputs, sendDmNotification(), execute Discord action, createCase(), sendModLogEmbed(), checkEscalation()
Applied to files:
src/commands/welcome.jsconfig.jsonsrc/modules/welcomeOnboarding.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/commands/*.js : Use parseDuration() from src/utils/duration.js for duration-based commands (timeout, tempban, slowmode); enforce Discord duration caps (timeouts max 28 days, slowmode max 6 hours)
Applied to files:
src/commands/welcome.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/modules/*.js : Register event handlers in src/modules/events.js by importing handler functions and calling client.on() with config parameter
Applied to files:
src/modules/events.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/**/*.js : Add tests for all new code with mandatory 80% coverage threshold on statements, branches, functions, and lines; run pnpm test before every commit
Applied to files:
tests/modules/welcomeOnboarding.test.js
📚 Learning: 2026-02-27T16:24:15.054Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-27T16:24:15.054Z
Learning: Applies to src/**/*.js : Use getConfig(guildId?) from src/modules/config.js to read config; use setConfigValue(path, value, guildId?) to update at runtime
Applied to files:
web/src/components/dashboard/config-editor.tsx
🧬 Code graph analysis (8)
src/commands/welcome.js (5)
src/modules/config.js (1)
getConfig(282-313)src/utils/permissions.js (1)
isModerator(143-173)src/utils/safeSend.js (2)
safeEditReply(178-185)safeSend(116-123)src/modules/welcomeOnboarding.js (4)
onboarding(79-79)normalizeWelcomeOnboardingConfig(28-58)buildRulesAgreementMessage(64-76)buildRoleMenuMessage(78-103)src/logger.js (1)
info(230-232)
src/modules/events.js (1)
src/modules/welcomeOnboarding.js (6)
RULES_ACCEPT_BUTTON_ID(11-11)RULES_ACCEPT_BUTTON_ID(11-11)handleRulesAcceptButton(109-183)ROLE_MENU_SELECT_ID(12-12)ROLE_MENU_SELECT_ID(12-12)handleRoleMenuSelection(185-233)
tests/api/routes/guilds.coverage.test.js (1)
web/src/lib/auth.ts (1)
jwt(144-178)
tests/api/utils/configValidation.test.js (1)
src/api/utils/configValidation.js (1)
validateSingleValue(216-233)
src/modules/welcomeOnboarding.js (3)
src/commands/welcome.js (1)
onboarding(33-33)src/utils/safeSend.js (2)
safeReply(138-145)safeSend(116-123)src/logger.js (1)
info(230-232)
tests/modules/welcomeOnboarding.test.js (2)
src/modules/welcomeOnboarding.js (9)
normalizeWelcomeOnboardingConfig(28-58)buildRoleMenuMessage(78-103)role(121-121)role(205-205)member(120-120)member(196-196)introChannel(151-153)handleRulesAcceptButton(109-183)handleRoleMenuSelection(185-233)src/utils/safeSend.js (2)
safeSend(116-123)safeReply(138-145)
web/src/components/dashboard/config-editor.tsx (3)
web/src/lib/config-utils.ts (1)
GuildConfig(4-4)web/src/components/ui/card.tsx (1)
CardContent(55-55)web/src/components/dashboard/toggle-switch.tsx (1)
ToggleSwitch(14-32)
src/modules/welcome.js (1)
src/modules/welcomeOnboarding.js (3)
isReturningMember(60-62)member(120-120)member(196-196)
🔇 Additional comments (16)
tests/api/routes/guilds.coverage.test.js (2)
73-84: LGTM!The Map initialization using
[key, value]tuple format is the correct approach for initializing a Map with entries. The mock member object structure appropriately mirrors the Discord.js member cache.
177-179: LGTM!Explicitly specifying
algorithm: 'HS256'is a security best practice that prevents algorithm confusion attacks. The multi-line formatting improves readability.tests/api/utils/redisClient.coverage.test.js (1)
31-31: Looks good — mock import timing and assertion remain correct.This keeps the logger import aligned with the mocked module initialization and preserves test behavior.
Also applies to: 62-65
src/modules/welcome.js (1)
141-155: Returning-member welcome branch is implemented cleanly.The conditional keeps existing welcome behavior for new members while introducing a clear re-join path.
config.json (1)
58-69: Welcome onboarding defaults are well-structured.The new keys are initialized safely and keep onboarding features opt-in by default.
web/src/types/config.ts (1)
28-33: Type additions for welcome onboarding are solid.The new interfaces and
WelcomeConfigfields are consistent and improve config contract clarity.Also applies to: 35-45, 53-57
src/modules/events.js (1)
521-529:⚠️ Potential issue | 🟡 MinorMove the orphaned challenge-button JSDoc so it documents the right function.
The challenge-handler doc block now precedes
registerWelcomeOnboardingHandlers, which makes the API docs misleading.⛔ Skipped due to learnings
Learnt from: CR Repo: VolvoxLLC/volvox-bot PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-02-27T16:24:15.054Z Learning: Applies to src/modules/*.js : Register event handlers in src/modules/events.js by importing handler functions and calling client.on() with config parametertests/modules/welcomeOnboarding.test.js (2)
41-51: LGTM!The test correctly validates that
buildRoleMenuMessageenforces the 25-option cap by creating 30 options and verifying only 25 are included in the output.
101-149: LGTM!The test correctly validates the differential role update logic - removing deselected roles and adding newly selected ones.
src/commands/welcome.js (1)
19-31: LGTM!Good pattern: defers reply ephemerally, then checks both Administrator permission and moderator role before proceeding. This aligns with the expected command pattern.
src/modules/welcomeOnboarding.js (6)
28-58: LGTM!Solid defensive normalization: filters invalid options, trims strings, caps at 25 options, and provides safe defaults for all fields.
60-62: LGTM!Defensive implementation with proper optional chaining for the member flags check.
64-76: LGTM!Clean component construction for the rules agreement button.
78-103: LGTM!Proper handling of Discord select menu constraints: returns null when disabled/empty, caps descriptions at 100 characters, and allows flexible selection range.
105-107: LGTM!Efficient cache-first lookup with graceful fallback to API fetch.
185-233: LGTM!Clean implementation of differential role updates with proper deduplication and editable-role filtering. The sequential role operations are acceptable given the capped option count.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary — 14 issues found
🔴 Critical (2)
- Tests broken after defer fix (
tests/modules/welcomeOnboarding.test.js): Thevi.mockforsafeSend.jsdoesn't includesafeEditReply, and interaction mocks don't includedeferReply. After commitda41799addeddeferReplycalls, all handler tests throwTypeErrorimmediately. - Missing
welcome.enabledgate (src/modules/events.js:529-534):registerWelcomeOnboardingHandlersprocesses interactions even when the welcome module is disabled, violating the project convention "Checkconfig.yourModule.enabledbefore processing."
🟡 Warning (8)
- Error reply unreachable after defer (
src/modules/events.js:546,570): Catch blocks check!interaction.deferred, but since handlers calldeferReplyfirst, this is alwaysfalse— error replies are silently dropped. - Missing
adminOnlyexport (src/commands/welcome.js:17): Per AGENTS.md, mod-only commands must exportadminOnly = truefor auto-discovery. - Unhandled
roles.add()failure (src/modules/welcomeOnboarding.js:145): If role assignment throws, error propagates to the unreachable catch in events.js — user gets no feedback. - Silent DM catch (
src/modules/welcomeOnboarding.js:164-165): Empty catch block provides zero observability for DM failures. - Role menu textarea drops input (
web/src/components/dashboard/config-editor.tsx:786-789):parseRoleMenuOptionsruns on every keystroke, dropping partial entries while typing. - DM sequence textarea drops input (
web/src/components/dashboard/config-editor.tsx:812-819): Same issue —.trim().filter(Boolean)on every keystroke drops empty lines. - Missing
welcomeinpermissions.allowedCommands(config.json): The/welcomecommand isn't listed in the permissions map. - Orphaned JSDoc (
src/modules/events.js:515-520): Challenge-button JSDoc now precedesregisterWelcomeOnboardingHandlers.
🔵 Nitpick (4)
- Missing JSDoc on
isReturningMember(src/modules/welcomeOnboarding.js:60) - Inconsistent config access (
src/commands/welcome.js:52-56): Rules panel uses normalized config, role menu panel uses raw config. - Missing
introChanneltest (tests/api/utils/configValidation.test.js:70-75) - Missing negative validation tests (
tests/api/utils/configValidation.test.js): No rejection tests for malformed onboarding payloads.
AI agent fix prompt (copy-paste ready)
Fix all issues listed below. Verify each finding against the current code before fixing.
1. In tests/modules/welcomeOnboarding.test.js:
- Add `safeEditReply: vi.fn(async () => {})` to the vi.mock factory for safeSend.js (lines 9-18)
- Import `safeEditReply` alongside `safeReply` and `safeSend` on line 26
- Add `deferReply: vi.fn(async () => {})` to both interaction mocks (lines 68-83 and 113-127)
- Change assertion on lines 95-98 from `safeReply` to `safeEditReply` and remove `ephemeral: true` from the expected object (since ephemeral is now set in deferReply, not editReply)
2. In src/modules/events.js line 534:
- Add `if (!guildConfig.welcome?.enabled) return;` after the getConfig call
3. In src/modules/events.js lines 546 and 570:
- Change `if (!interaction.replied && !interaction.deferred)` to `if (!interaction.replied)`
- Use `const reply = interaction.deferred ? safeEditReply : safeReply;` and call that
- Only include `ephemeral: true` when not deferred
4. In src/modules/events.js lines 515-520:
- Delete the orphaned JSDoc block for challenge buttons (it's duplicated at line 585)
5. In src/commands/welcome.js after line 17:
- Add `export const adminOnly = true;`
6. In src/modules/welcomeOnboarding.js line 145:
- Wrap `member.roles.add(role, 'Accepted server rules')` in try-catch
- On catch: import and call `error()` from logger.js with context, then `await safeEditReply(interaction, { content: '...' })` and return
7. In src/modules/welcomeOnboarding.js lines 164-165:
- Change `} catch {` to `} catch (err) {`
- Add `info('DM sequence interrupted', { userId: interaction.user.id, guildId: interaction.guildId, error: err.message });` before break
8. In src/modules/welcomeOnboarding.js line 60:
- Add JSDoc comment for isReturningMember
9. In web/src/components/dashboard/config-editor.tsx lines 786-789:
- Add a `roleMenuRaw` state variable and sync it from draftConfig via useEffect
- Change textarea value to roleMenuRaw, onChange to setRoleMenuRaw, add onBlur to parse
10. In web/src/components/dashboard/config-editor.tsx lines 812-819:
- Same pattern: add `dmStepsRaw` state, bind to textarea, parse on blur
11. In config.json permissions.allowedCommands (around line 167):
- Add `"welcome": "admin"`
12. In tests/api/utils/configValidation.test.js lines 70-75:
- Add `expect(validateSingleValue('welcome.introChannel', '456')).toEqual([]);`
- Add new test: `it('should reject invalid onboarding field types', ...)` with checks for roleMenu.enabled as string and dmSequence.steps as non-array
|
| Filename | Overview |
|---|---|
| src/modules/welcomeOnboarding.js | New onboarding module with role operations missing error handling and logging for failed role fetches |
| src/modules/events.js | Registers onboarding handlers with proper error boundaries (previous issues noted in threads) |
| src/commands/welcome.js | New setup command missing bot permission validation before posting to channels |
| src/modules/welcome.js | Clean integration of returning member detection into existing welcome flow |
| src/api/utils/configValidation.js | Comprehensive validation for new onboarding config fields with array item type checking |
| web/src/components/dashboard/config-editor.tsx | New onboarding UI controls with textarea parsing issues (noted in previous threads) |
Sequence Diagram
sequenceDiagram
participant User
participant Discord
participant events.js
participant welcomeOnboarding.js
participant Guild as Discord Guild
User->>Discord: Click "Accept Rules" button
Discord->>events.js: InteractionCreate (RULES_ACCEPT_BUTTON_ID)
events.js->>welcomeOnboarding.js: handleRulesAcceptButton(interaction, config)
welcomeOnboarding.js->>welcomeOnboarding.js: deferReply (ephemeral)
welcomeOnboarding.js->>welcomeOnboarding.js: normalizeWelcomeOnboardingConfig()
welcomeOnboarding.js->>Guild: fetch verifiedRole
welcomeOnboarding.js->>Guild: member.roles.add(verifiedRole)
welcomeOnboarding.js->>Guild: post intro prompt to introChannel
alt DM sequence enabled
welcomeOnboarding.js->>User: send DM steps (breaks on failure)
end
welcomeOnboarding.js->>Discord: editReply "Rules accepted!"
User->>Discord: Select roles from menu
Discord->>events.js: InteractionCreate (ROLE_MENU_SELECT_ID)
events.js->>welcomeOnboarding.js: handleRoleMenuSelection(interaction, config)
welcomeOnboarding.js->>welcomeOnboarding.js: deferReply (ephemeral)
welcomeOnboarding.js->>Guild: fetch configured roles
welcomeOnboarding.js->>welcomeOnboarding.js: compute addable/removable
welcomeOnboarding.js->>Guild: member.roles.remove(removable)
welcomeOnboarding.js->>Guild: member.roles.add(addable)
welcomeOnboarding.js->>Discord: editReply "Updated roles"
Last reviewed commit: 4624c4a
…error handling, validation - Add `adminOnly = true` export to welcome command - Gate onboarding interaction handlers on `welcome.enabled` - Fix error reply path after deferReply (use safeEditReply instead of unreachable safeReply) - Wrap member.roles.add() in try-catch with user-facing error feedback - Add Winston logging to silent DM catch block - Add JSDoc to isReturningMember function - Validate item shapes in roleMenu.options and dmSequence.steps - Fix broken tests: add safeEditReply mock, deferReply/deferred/replied to interaction mocks, update assertions to match safeEditReply calls - Add tests for introChannel, malformed roleMenu items, non-array steps Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…xtareas Parse on blur instead of onChange to prevent partial input from being stripped while the user is still typing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Review Summary — 5 issues remaining
Most issues from prior reviews have been addressed across commits da41799, 0188bf6, 9452ba9, and b90d216. The onboarding implementation is solid — proper normalization, deferred replies, error handling with user feedback, and raw textarea state for the dashboard. A few items remain:
🟡 Warning (2)
- Missing
welcomeinpermissions.allowedCommands(config.json:167) — WithusePermissions: true, the/welcomecommand isn't listed in the permissions map. Add"welcome": "admin". - Silent failure when role menu channel is missing (
src/commands/welcome.js:66-68) — When role menu is enabled butwelcome.channelIdis not set, the setup command produces no feedback. Theelse if (!roleMenuMsg)branch doesn't cover this case.
🔵 Nitpick (3)
- Orphaned JSDoc block (
src/modules/events.js:521) — Challenge button docs disconnected fromregisterChallengeButtonHandlerat line 594. - Missing JSDoc on
buildRulesAgreementMessage(src/modules/welcomeOnboarding.js:70) - Missing JSDoc on
buildRoleMenuMessage(src/modules/welcomeOnboarding.js:84)
AI agent fix prompt (copy-paste ready)
Fix the following issues. Verify each finding against the current code before fixing.
1. In config.json, add "welcome": "admin" to permissions.allowedCommands (around line 167,
after the "showcase": "everyone" entry).
2. In src/commands/welcome.js lines 66-68, add an else clause after the `else if (!roleMenuMsg)`
block to handle the case where roleMenuMsg is truthy but channelId is missing:
} else {
resultLines.push('⚠️ `welcome.channelId` is not configured; role menu not posted.');
}
3. In src/modules/events.js, move the orphaned JSDoc block at lines 515-520 (the challenge
button handler docs) to directly above registerChallengeButtonHandler at line 594.
4. In src/modules/welcomeOnboarding.js line 70, add JSDoc to buildRulesAgreementMessage:
/**
* Build the rules agreement message with an "Accept Rules" button component.
* @returns {{ content: string, components: ActionRowBuilder[] }} Message payload.
*/
5. In src/modules/welcomeOnboarding.js line 84, add JSDoc to buildRoleMenuMessage:
/**
* Build a self-assignable role selection menu from the welcome config.
* Returns null if the role menu is disabled or has no valid options.
* @param {object} welcomeConfig - The raw config.welcome object.
* @returns {{ content: string, components: ActionRowBuilder[] } | null}
*/
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| try { | ||
| await member.roles.add(role, 'Accepted server rules'); | ||
| } catch (roleErr) { | ||
| await safeEditReply(interaction, { | ||
| content: '❌ Failed to assign the verified role. Please try again or contact an admin.', | ||
| }); | ||
| throw roleErr; | ||
| } |
There was a problem hiding this comment.
Throwing after sending specific error message causes outer handler to overwrite it with generic message. Since the error handler in events.js (lines 547-560) will catch this and call safeEditReply again with "Failed to verify. Please ping an admin.", users won't see the helpful "Failed to assign the verified role" message. Remove the throw roleErr; and just return instead to preserve the specific error message.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/modules/welcomeOnboarding.js
Line: 151-158
Comment:
Throwing after sending specific error message causes outer handler to overwrite it with generic message. Since the error handler in `events.js` (lines 547-560) will catch this and call `safeEditReply` again with "Failed to verify. Please ping an admin.", users won't see the helpful "Failed to assign the verified role" message. Remove the `throw roleErr;` and just `return` instead to preserve the specific error message.
How can I resolve this? If you propose a fix, please make it concise.| <textarea | ||
| value={roleMenuRaw} | ||
| onChange={(e) => setRoleMenuRaw(e.target.value)} | ||
| onBlur={() => { | ||
| const parsed = parseRoleMenuOptions(roleMenuRaw); | ||
| updateWelcomeRoleMenu('options', parsed); | ||
| setRoleMenuRaw(stringifyRoleMenuOptions(parsed)); | ||
| }} | ||
| rows={5} | ||
| disabled={saving} | ||
| className={inputClasses} | ||
| placeholder={ | ||
| 'Format: Label|RoleID|Description (optional)\nOne option per line (max 25).' | ||
| } | ||
| /> |
There was a problem hiding this comment.
Role menu changes won't save if user clicks Save button without blurring the textarea first. The onBlur handler (line 796-799) parses roleMenuRaw and updates draftConfig, but if the user types changes and immediately clicks Save, onBlur won't fire yet. Consider either: (1) calling blur on all inputs before saving, or (2) parsing raw values in saveChanges before computing patches.
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/components/dashboard/config-editor.tsx
Line: 793-807
Comment:
Role menu changes won't save if user clicks Save button without blurring the textarea first. The `onBlur` handler (line 796-799) parses `roleMenuRaw` and updates `draftConfig`, but if the user types changes and immediately clicks Save, `onBlur` won't fire yet. Consider either: (1) calling blur on all inputs before saving, or (2) parsing raw values in `saveChanges` before computing patches.
How can I resolve this? If you propose a fix, please make it concise.| <textarea | ||
| value={dmStepsRaw} | ||
| onChange={(e) => setDmStepsRaw(e.target.value)} | ||
| onBlur={() => { | ||
| const parsed = dmStepsRaw | ||
| .split('\n') | ||
| .map((line) => line.trim()) | ||
| .filter(Boolean); | ||
| updateWelcomeDmSequence('steps', parsed); | ||
| setDmStepsRaw(parsed.join('\n')); | ||
| }} | ||
| rows={4} | ||
| disabled={saving} | ||
| className={inputClasses} | ||
| placeholder="One DM step per line" | ||
| /> | ||
| </fieldset> |
There was a problem hiding this comment.
Same issue as role menu - DM sequence changes won't save if user clicks Save without blurring the textarea. The onBlur handler (line 824-830) parses dmStepsRaw but won't fire if Save is clicked immediately after typing.
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/components/dashboard/config-editor.tsx
Line: 821-837
Comment:
Same issue as role menu - DM sequence changes won't save if user clicks Save without blurring the textarea. The `onBlur` handler (line 824-830) parses `dmStepsRaw` but won't fire if Save is clicked immediately after typing.
How can I resolve this? If you propose a fix, please make it concise.…acks
After handleRulesAcceptButton / handleRoleMenuSelection call deferReply(),
interaction.deferred=true but interaction.replied=false. The old else branch
using safeReply is dead code and would fail on a deferred interaction.
Change the error-fallback blocks to:
if (!interaction.replied) { await safeEditReply(...) }
This avoids double-replies when the handler already sent a specific error
message via safeEditReply before rethrowing, and correctly uses editReply
for all deferred interactions.
Fixes PRRT_kwDORICdSM5xV0QK, PRRT_kwDORICdSM5xV0QW
Both the rules-accept and role-menu interaction mocks were missing editReply: vi.fn(), which is called internally by safeEditReply. Add it to both fixtures for completeness. Fixes PRRT_kwDORICdSM5xV0RQ, PRRT_kwDORICdSM5xV0Rn, PRRT_kwDORICdSM5xV0R6
…missing Previously, when buildRoleMenuMessage() returned a valid message but welcome.channelId was falsy, neither branch of the if/else-if executed, causing silent partial output with no feedback to the operator. Add an explicit else-if(roleMenuMsg) branch that emits a warning so the admin knows the role menu panel was not posted and why. Fixes PRRT_kwDORICdSM5xV2Ts, PRRT_kwDORICdSM5xV2Yg
…imit Double-reply (PRRT_kwDORICdSM5xV2Yc): In handleRulesAcceptButton, when member.roles.add() fails, the handler was sending a specific error via safeEditReply then rethrowing. The outer InteractionCreate catch in events.js would then also send a generic error, overwriting the specific message. Fix: log the error and return instead of rethrowing. Discord select menu label limit (PRRT_kwDORICdSM5xV2Yf): Discord limits select menu option labels to 100 characters. Truncate opt.label with .slice(0, 100) when building select options. (description already had the 100-char slice; label was missing it.) Fixes PRRT_kwDORICdSM5xV2Yc, PRRT_kwDORICdSM5xV2Yf
…sage typeof null === 'object' in JavaScript, so when an array item is null the error message incorrectly reported 'object'. Add a null check before typeof so the message correctly says 'null'. Before: expected object, got object After: expected object, got null Fixes PRRT_kwDORICdSM5xV2Yh
| <textarea | ||
| value={roleMenuRaw} | ||
| onChange={(e) => setRoleMenuRaw(e.target.value)} | ||
| onBlur={() => { | ||
| const parsed = parseRoleMenuOptions(roleMenuRaw); | ||
| updateWelcomeRoleMenu('options', parsed); | ||
| setRoleMenuRaw(stringifyRoleMenuOptions(parsed)); | ||
| }} | ||
| rows={5} | ||
| disabled={saving} | ||
| className={inputClasses} | ||
| placeholder={ | ||
| 'Format: Label|RoleID|Description (optional)\nOne option per line (max 25).' | ||
| } | ||
| /> |
There was a problem hiding this comment.
🟡 Warning: Ctrl/Cmd+S keyboard shortcut bypasses textarea onBlur, losing unsaved edits.
Both the role menu and DM sequence textareas parse their content into draftConfig only on blur. But the existing Ctrl/Cmd+S keyboard shortcut (line 359) calls saveChanges() directly — if the user is focused in either textarea and presses Ctrl+S, onBlur never fires and the raw text isn't flushed into draftConfig. The changes are silently lost.
Fix: trigger a blur before saving in the keyboard handler:
function onKeyDown(e: KeyboardEvent) {
if ((e.metaKey || e.ctrlKey) && e.key === 's') {
e.preventDefault();
if (document.activeElement instanceof HTMLElement) {
document.activeElement.blur();
}
if (hasChanges && !saving && !hasValidationErrors) {
saveChanges();
}
}
}Or alternatively, flush the raw values at the start of saveChanges().
There was a problem hiding this comment.
Review Summary — 6 issues remaining
Good work iterating on the review feedback across 12 commits. The core onboarding implementation is solid: proper deferReply usage, defensive normalization, error handling with user feedback, welcome.enabled gate, and raw textarea state for the dashboard. Most issues from prior reviews have been addressed.
🟡 Warning (2)
-
config.json— Missingwelcomeinpermissions.allowedCommands(not in diff, so no inline comment)
WithusePermissions: true, the/welcomecommand isn't listed. Per AGENTS.md: "Add permission inconfig.jsonunderpermissions.allowedCommands." Add"welcome": "admin"after the"showcase"entry. -
web/src/components/dashboard/config-editor.tsx:793-807— Ctrl/Cmd+S bypasses textareaonBlur, losing unsaved edits
The keyboard shortcut callssaveChanges()without triggering blur on the focused textarea. If the user is mid-edit in the role menu or DM sequence textarea and presses Ctrl+S, changes are silently lost.
🔵 Nitpick (4)
-
src/modules/events.js:521— Orphaned JSDoc block
Challenge-button JSDoc separated fromregisterChallengeButtonHandlerby the new onboarding handler. -
src/modules/welcomeOnboarding.js:70— Missing JSDoc onbuildRulesAgreementMessage -
src/modules/welcomeOnboarding.js:84— Missing JSDoc onbuildRoleMenuMessage -
AGENTS.md— Missing Key Files entries (not in diff, so no inline comment)
Per project conventions: "Added a new command → update Key Files table" and "Added a new module → update Key Files table." Missing entries forsrc/modules/welcomeOnboarding.jsandsrc/commands/welcome.js.
AI agent fix prompt (copy-paste ready)
Fix all issues listed below. Verify each finding against the current code before fixing.
1. In config.json, add "welcome": "admin" to permissions.allowedCommands
(around line 167, after the "showcase": "everyone" entry).
2. In web/src/components/dashboard/config-editor.tsx, update the Ctrl/Cmd+S
keyboard handler (around line 358) to blur the active element before saving:
function onKeyDown(e: KeyboardEvent) {
if ((e.metaKey || e.ctrlKey) && e.key === 's') {
e.preventDefault();
if (document.activeElement instanceof HTMLElement) {
document.activeElement.blur();
}
if (hasChanges && !saving && !hasValidationErrors) {
saveChanges();
}
}
}
3. In src/modules/events.js, move the orphaned JSDoc block at lines 515-520
(challenge button handler docs) to directly above registerChallengeButtonHandler
at line 584. Delete the empty line and stale JSDoc from lines 515-520.
4. In src/modules/welcomeOnboarding.js line 70, add JSDoc to buildRulesAgreementMessage:
/**
* Build the rules agreement message with an "Accept Rules" button component.
* @returns {{ content: string, components: ActionRowBuilder[] }} Message payload.
*/
5. In src/modules/welcomeOnboarding.js line 84, add JSDoc to buildRoleMenuMessage:
/**
* Build a self-assignable role selection menu from the welcome config.
* Returns null if the role menu is disabled or has no valid options.
* @param {object} welcomeConfig - The raw config.welcome object.
* @returns {{ content: string, components: ActionRowBuilder[] } | null}
*/
6. In AGENTS.md Key Files table, add entries for:
| src/modules/welcomeOnboarding.js | Onboarding flows: rules acceptance, role menu, DM sequence, returning-member detection |
| src/commands/welcome.js | /welcome setup command — posts onboarding panels to configured channels |
| if (removable.length > 0) { | ||
| await member.roles.remove( | ||
| removable.map((r) => r.id), | ||
| 'Updated self-assignable onboarding roles', | ||
| ); | ||
| } | ||
| if (addable.length > 0) { | ||
| await member.roles.add( | ||
| addable.map((r) => r.id), | ||
| 'Updated self-assignable onboarding roles', | ||
| ); | ||
| } |
There was a problem hiding this comment.
Missing error handling for role operations. If member.roles.remove() succeeds but member.roles.add() fails, member is left in an inconsistent state (some roles removed but none added).
Wrap each operation in try-catch:
try {
if (removable.length > 0) {
await member.roles.remove(
removable.map((r) => r.id),
'Updated self-assignable onboarding roles',
);
}
if (addable.length > 0) {
await member.roles.add(
addable.map((r) => r.id),
'Updated self-assignable onboarding roles',
);
}
} catch (roleErr) {
info('Role update failed in role menu handler', {
guildId: interaction.guildId,
userId: interaction.user.id,
error: roleErr?.message,
});
await safeEditReply(interaction, {
content: '❌ Failed to update roles. Please try again or contact an admin.',
});
return;
}Prompt To Fix With AI
This is a comment left during a code review.
Path: src/modules/welcomeOnboarding.js
Line: 233-244
Comment:
Missing error handling for role operations. If `member.roles.remove()` succeeds but `member.roles.add()` fails, member is left in an inconsistent state (some roles removed but none added).
Wrap each operation in try-catch:
```javascript
try {
if (removable.length > 0) {
await member.roles.remove(
removable.map((r) => r.id),
'Updated self-assignable onboarding roles',
);
}
if (addable.length > 0) {
await member.roles.add(
addable.map((r) => r.id),
'Updated self-assignable onboarding roles',
);
}
} catch (roleErr) {
info('Role update failed in role menu handler', {
guildId: interaction.guildId,
userId: interaction.user.id,
error: roleErr?.message,
});
await safeEditReply(interaction, {
content: '❌ Failed to update roles. Please try again or contact an admin.',
});
return;
}
```
How can I resolve this? If you propose a fix, please make it concise.
Summary
Ships an onboarding MVP with configurable rules agreement, role selection, intro prompting, and returning-member handling.
What changed
config.welcomewith onboarding fields:rulesChannel,verifiedRole,introChannelroleMenu.enabled,roleMenu.optionsdmSequence.enabled,dmSequence.stepssrc/modules/welcomeOnboarding.js/welcome setupcommand to post onboarding panelsevents.jsfor:Verification
vitestpasses (backend + module tests)biomeclean for changed filesCloses #138