Repository navigation
chore(lint): non-null assertion cleanup batch — bot music/spotify/handlers (#1378) - #1391
Conversation
…sic+spotify+handlers (#1378)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
@cubic-dev-ai please review — incremental non-null-assertion cleanup (38→18), bot-only, full suite green. |
|
Warning Review limit reached
More reviews will be available in 14 minutes and 48 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughSeven modules across music commands, event handlers, stream processing, Spotify API mapping, and queue utilities replace TypeScript non-null assertions ( ChangesNon-null assertion refactoring
Possibly related PRs
Suggested labels
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 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 |
@LucasSantana-Dev I have started the AI code review. It will take a few minutes to complete. |
|
Failed to generate code suggestions for PR |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 8 files
Auto-approved: Replaces non-null assertions with runtime checks across music commands, Spotify API, and handlers. No logic changes, reduces lint warnings, tests pass.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bot/src/functions/music/commands/stop.ts`:
- Line 25: Add an explicit await requireGuild(interaction) call before calling
requireDJRole in the stop command so the guildId is guaranteed like other music
commands (e.g., volume.ts, voteskip.ts); then keep the
assertDefined(interaction.guildId, ...) usage consistent by updating its error
message to reference "requireGuild" (e.g., "Guild ID required after requireGuild
check") or remove the misleading resolveGuildQueue mention—this ensures
requireGuild runs before requireDJRole and the assertDefined message accurately
reflects the guarantee.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 451206af-2897-438a-a4f4-e5ed5b6c52f2
📒 Files selected for processing (8)
packages/bot/src/functions/music/commands/play/handlers/playHandler.tspackages/bot/src/functions/music/commands/stop.tspackages/bot/src/functions/music/commands/volume.tspackages/bot/src/functions/music/commands/voteskip.tspackages/bot/src/handlers/message/autoModHandler.tspackages/bot/src/handlers/player/streamBridge.tspackages/bot/src/spotify/spotifyApi.tspackages/bot/src/utils/music/queueEditOps.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). (9)
- GitHub Check: Test — backend
- GitHub Check: Checks
- GitHub Check: Test — shared
- GitHub Check: Test — bot
- GitHub Check: Test — frontend
- GitHub Check: cubic · AI code reviewer
- GitHub Check: quality / SAST (CodeQL) (javascript-typescript)
- GitHub Check: Build — bot
- GitHub Check: Build — backend
🔇 Additional comments (8)
packages/bot/src/handlers/message/autoModHandler.ts (1)
4-4: LGTM!Also applies to: 116-116, 121-122
packages/bot/src/functions/music/commands/volume.ts (1)
82-82: LGTM!packages/bot/src/functions/music/commands/voteskip.ts (1)
39-39: LGTM!Also applies to: 115-115
packages/bot/src/handlers/player/streamBridge.ts (1)
72-72: LGTM!Also applies to: 76-84
packages/bot/src/spotify/spotifyApi.ts (2)
220-228: LGTM!
592-606: LGTM!packages/bot/src/utils/music/queueEditOps.ts (1)
178-178: LGTM!Also applies to: 206-206
packages/bot/src/functions/music/commands/play/handlers/playHandler.ts (1)
48-48: /play already validates voice channel beforeplayHandler
packages/bot/src/functions/music/commands/play/index.tscallsawait requireVoiceChannel(params.interaction)and returns early beforeawait executePlayHandler(params);requireVoiceChannelreplies withJoin a voice channel first.whenmember.voice.channelis missing, so theassertDefined(member.voice.channel, 'Voice channel guaranteed by requireVoiceChannel check')inplayHandler.tsis consistent with the command validation contract.
… truly guaranteed (#1378) Addresses CodeRabbit: stop had no requireGuild precondition, so assertDefined(guildId) could throw and its message was misleading. Now matches volume/voteskip pattern; adds guard-rejection test.
|
Addressed CodeRabbit's finding on |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
@LucasSantana-Dev The fix on A couple of things I noticed that are worth a quick look:
if (!(await requireGuild(interaction))) return
const { queue } = resolveGuildQueue(client, interaction.guildId ?? '')The The test covers the early-return path well. Mocking Overall this is a clean incremental improvement. The warning count drop (38 → 18) and green suite speak for themselves. Tip: get faster answers by chatting with cubic’s review copilot in the review UI |
|
Tip For best results, initiate chat on the files or code changes.
(ᵔᴥᵔ)🐇 ✏️ Learnings added
|
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Auto-approved: Replaces non-null assertions with runtime guards across isolated music commands, Spotify API, and handlers. All changes are mechanical; tests pass and lint count is reduced.
Re-trigger cubic
|
|
@coderabbitai review — the requireGuild precondition + corrected assertDefined message and the new rejection test have been pushed (head e4249ce). Please re-review; the prior thread is resolved/outdated. |
|
✅ Action performedFull review finished. |
Dismissing stale review: this CHANGES_REQUESTED is on the superseded commit de2fc5c. The requested fix (add requireGuild precondition before requireDJRole in stop.ts, plus the corrected assertDefined message + new rejection test) has been applied and pushed in e4249ce; the review thread is resolved+outdated and cubic-dev-ai re-approved the current head with all CI green. Requested a re-review (@coderabbitai review) ~20min ago with no new review delivered.
…ublisher (#1378) - skipto.ts: add requireGuild precondition before requireDJRole (was an unguarded interaction.guildId! — same latent crash-in-DM class as the stop.ts fix in #1391), then assertDefined. - musicButtonHandler.ts: interaction.guildId! -> interaction.guildId ?? '' (voice-channel check guarantees guild; matches resolveGuildQueue call convention, graceful no-queue path). - MusicControlService.ts:85: this.publisher! -> assertDefined (guaranteed by the isHealthy() guard at sendCommand entry). Completes the #1378 no-non-null-assertion sweep: 0 remaining in bot + shared.
…ublisher (#1378) - skipto.ts: add requireGuild precondition before requireDJRole (was an unguarded interaction.guildId! — same latent crash-in-DM class as the stop.ts fix in #1391), then assertDefined. - musicButtonHandler.ts: interaction.guildId! -> interaction.guildId ?? '' (voice-channel check guarantees guild; matches resolveGuildQueue call convention, graceful no-queue path). - MusicControlService.ts:85: this.publisher! -> assertDefined (guaranteed by the isHealthy() guard at sendCommand entry). Completes the #1378 no-non-null-assertion sweep: 0 remaining in bot + shared.
… services (#1378) (#1393) Part of #1378 (follow-up to #1391). Clears the remaining **29** `@typescript-eslint/no-non-null-assertion` sites across bot + shared → **0 repo-wide**. ## Changes Each `expr!` replaced with `assertDefined(expr, '<reason> — guaranteed by <guard>')` from `@lucky/shared/utils/guards` (submodule path; barrel untouched), where a preceding guard already guarantees non-nullness: - **Music commands:** album, artist, effects, leavecleanup, play/index, play/queryUtils, shuffle, skip, songinfo (behind `requireQueue` / `requireVoiceChannel` / `requireCurrentTrack` / explicit guards). - **Bot utils/services:** candidateFallback, voteSkipStore, feedbackService (Map `.get()` after `.has()`). - **Shared:** `AutoMessageService` (×6), `environment.ts` Infisical block (×4, behind the `isInfisicalConfigured()` early-return that verifies all 4 vars), `MusicControlService` publisher (behind `isHealthy()`). ### 2 latent-bug guard hardenings (same class CodeRabbit flagged on stop.ts in #1391) - **`skipto.ts`** — was an **unguarded** `interaction.guildId!` (would throw in a non-guild context). Added `requireGuild` precondition before `requireDJRole`, then `assertDefined` — matches the merged skip.ts/stop.ts pattern. - **`musicButtonHandler.ts`** — `interaction.guildId!` → `interaction.guildId ?? ''` (voice-channel check guarantees guild; matches the `resolveGuildQueue` call convention with its graceful no-queue path). ## Verification (local, post-rebase onto #1392) - eslint `no-non-null-assertion`: **0** across `packages/bot/src` + `packages/shared/src` - `build:shared`: clean - **shared 818/818**, **bot 2537 passed** (6 skipped) — no failures, no import.meta/barrel regressions Follow-up (separate PR): promote `no-unsafe-* warn → error` now that #1392 gives CI lint type info and these warnings are cleared. @cubic-dev-ai @coderabbitai <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Replaced all remaining non‑null assertions with `assertDefined` to satisfy `@typescript-eslint/no-non-null-assertion` and improve guard-based safety. Also tightened guild checks to prevent edge-case crashes; completes the cleanup tracked in #1378. - **Refactors** - Replaced `x!` with `assertDefined(x, 'reason')` from `@lucky/shared/utils/guards` where prior guards ensure non-null (queues, voice channels, tracks, Map/Set entries). - Touched music commands (`album`, `artist`, `play`, `effects`, `shuffle`, `skip`, `songinfo`, `leavecleanup`), utils/services (`candidateFallback`, `voteSkipStore`, `feedbackService`), and shared (`environment` Infisical block, `AutoMessageService`, `MusicControlService` publisher). - **Bug Fixes** - `skipto`: add `requireGuild` before `requireDJRole`; assert `interaction.guildId`. - `musicButtonHandler`: use `interaction.guildId ?? ''` when calling `resolveGuildQueue` for a graceful no-queue path. <sup>Written for commit f7dd0ea. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/LucasSantana-Dev/Lucky/pull/1393?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img](https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a){kind=link}
This branch was successfully deployed
1 participant
Incremental #1378 warning reduction (follow-up to #1389).
Change
Replaced
@typescript-eslint/no-non-null-assertionsites (foo!) withassertDefined(value, 'reason')(from@lucky/shared/utils/guards) or safe narrowing, across:stop.ts,volume.ts,voteskip.ts,play/handlers/playHandler.ts(music commands — after require* preconditions)spotify/spotifyApi.ts(filter-guaranteed id/name)utils/music/queueEditOps.ts,handlers/player/streamBridge.ts,handlers/message/autoModHandler.tsResults
no-non-null-assertion: 38 → 18 (20 removed)@lucky/shared/utils/guards(matches merged convention; avoids the ts-jest shared-barrel resolution quirk). No shared-package changes.Verification
npm run build:shared+npm run build --workspace=packages/bot— OKnpm run lint --workspace=packages/bot— 0 errorsIncremental — does not close #1378. Refs #1378.
Summary by cubic
Replaced non-null assertions with
assertDefinedand safe narrowing across music commands, Spotify API, and message/player handlers. Also fixes the stop command to callrequireGuildbeforerequireDJRoleto avoid misleading guard errors; cuts 20 lint warnings (38 → 18) toward #1378.Refactors
foo!withassertDefined(value, 'reason')where guards guarantee presence (voice channel,guildId, queue,stdout/stderr, Spotify IDs/names, track indices).@lucky/shared/utils/guardsacross music (playHandler,stop,volume,voteskip), Spotify (spotifyApi), handlers (autoModHandler,streamBridge), and utils (queueEditOps).Bug Fixes
stop, runrequireGuildbeforerequireDJRolesoguildIdis truly guaranteed; adds a test that returns early when the guild check fails.Written for commit e4249ce. Summary will update on new commits.
Summary by CodeRabbit