Skip to content

fix(bot): spotify-first, no duplicate play msg, soundcloud last - #557

Merged
LucasSantana-Dev merged 8 commits into
mainfrom
fix/play-provider-speed-duplicate
Apr 11, 2026
Merged

LucasSantana-Dev merged 8 commits into
mainfrom
fix/play-provider-speed-duplicate

Conversation

@LucasSantana-Dev

@LucasSantana-Dev LucasSantana-Dev commented Apr 11, 2026 •

Copy link
Copy Markdown
Owner

Changes

Provider priority: Spotify → YouTube → SoundCloud (was AUTO_SEARCH which resolved via SoundCloud)

Duplicate play message: Removed nowPlaying from the interaction reply — sendNowPlayingEmbed (playerStart) is now the single authoritative Now Playing display. Eliminates the race where playerStart fired before fetchReply() completed.

Fallback chain: Spotify → YouTube → SoundCloud (was AUTO which picked SoundCloud for YouTube-available tracks)

1879 tests, 0 failures.

Summary by CodeRabbit

  • Changes

    • /play now sends embeds only on initial reply (music control buttons removed).
    • Bot detects voice kicks and treats them as intentional stops, preventing automatic recovery and autoplay in those cases.
    • Intentional-stop state now persists longer before clearing.
  • Improvements

    • Search order adjusted to prefer Spotify with SoundCloud as a fallback when needed.
    • Queue replenishment better avoids duplicates using title-only matching.
    • Title cleaning now strips common hyphenated/version suffixes.

@vercel

vercel Bot commented Apr 11, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
lucky Ready Ready Preview, Comment Apr 11, 2026 4:57pm

Request Review

@github-actions github-actions Bot added the bot label Apr 11, 2026
@coderabbitai

coderabbitai Bot commented Apr 11, 2026 •

Copy link
Copy Markdown

Warning

Rate limit exceeded

@LucasSantana-Dev has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 43 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 10 minutes and 43 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 60d48acd-5c40-4cd3-94c2-78cb188f8992

📥 Commits

Reviewing files that changed from the base of the PR and between 22bc4dc and 50d0a6e.

📒 Files selected for processing (3)
  • packages/bot/src/functions/music/commands/play/index.spec.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/utils/music/queueManipulation.spec.ts
📝 Walkthrough

Walkthrough

Removed now-playing registration and control buttons from initial /play replies; changed default and fallback search providers; added title-only deduplication and expanded title-cleaning to strip hyphenated version suffixes; introduced intentional-stop tracking and voice-kick detection affecting watchdog recovery and lifecycle handlers.

Changes

Cohort / File(s) Summary
Play command implementation
packages/bot/src/functions/music/commands/play/index.ts
Removed registration of the interaction reply as the now-playing message for queuePosition === 0; non-playlist embeds now always use addedToQueue; initial /play reply no longer includes music-control buttons; YouTube fallback changed to QueryType.SOUNDCLOUD_SEARCH.
Play command tests
packages/bot/src/functions/music/commands/play/index.spec.ts
Removed Jest mock and test asserting registerNowPlayingMessage was invoked for queue position 0.
Search engine selection
packages/bot/src/functions/music/commands/play/queryUtils.ts
Default non-URL search provider changed from QueryType.AUTO_SEARCH to QueryType.SPOTIFY_SEARCH (fallbacks handled in play flow).
Queue deduplication logic
packages/bot/src/utils/music/queueManipulation.ts, packages/bot/src/utils/music/queueManipulation.spec.ts
Added normalized title-only keys to excluded set; isDuplicateCandidate now checks full title::author and title-only keys; refactored key accumulation and added tests for title-only deduplication.
Search query cleaning
packages/bot/src/utils/music/searchQueryCleaner.ts, .../searchQueryCleaner.spec.ts
cleanTitle() now strips hyphen/–/—-delimited trailing version suffixes (years, remaster, official audio/video, live, acoustic, demo, extended, radio/album/single version); tests added.
Queue embed
packages/bot/src/functions/music/commands/queue/queueEmbed.ts
addUpcomingTracks now displays 'No displayable tracks' when the track list display is falsy.
Player lifecycle & watchdog
packages/bot/src/handlers/player/lifecycleHandlers.ts, .../lifecycleHandlers.spec.ts
Added setupVoiceKickDetection(client) to mark intentional stops on voice disconnects; disconnect and connectionDestroyed handlers avoid watchdog recovery when marked intentional; emptyQueue marks intentional stop; tests updated.
Player handlers
packages/bot/src/handlers/player/index.ts, .../trackHandlers.ts, .../trackHandlers.spec.ts
Wired setupVoiceKickDetection into player setup; handlePlayerFinish/handlePlayerSkip early-return when guild is an intentional stop; minimal test mocks updated.
Watchdog service & tests
packages/bot/src/utils/music/watchdog.ts, packages/bot/tests/utils/music/watchdog.test.ts
markIntentionalStop delay changed from fixed 5_000ms to this.timeoutMs + 10_000; added isIntentionalStop(guildId): boolean; tests adjusted to new timing and query method.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Client as Client (Discord)
participant Voice as Voice State
participant VoiceKick as setupVoiceKickDetection
participant Watchdog as MusicWatchdogService
participant Player as Player Lifecycle Handlers

Client->>Voice: voiceStateUpdate (bot disconnected)
Voice->>VoiceKick: event handler
VoiceKick->>Watchdog: markIntentionalStop(guildId)
Watchdog-->>VoiceKick: ack
Note over Watchdog,Player: guild marked intentional stop

Player->>Watchdog: disconnect handler triggered
Watchdog->>Player: isIntentionalStop(guildId) => true
Player-->>Player: short-circuit recovery (no checkAndRecover)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • #542: Reintroduces registerNowPlayingMessage registration and related tests — directly contrasts the removal in this PR.
  • #538: Modifies /play search fallback and query provider choices — overlaps with the search-fallback and queryUtils changes here.
  • #512: Integrates/adjusts watchdog intentional-stop behavior and recovery flow — overlaps with watchdog and lifecycle handler changes.

Suggested labels

size/xl

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: Spotify-first search priority, removal of duplicate now-playing message, and SoundCloud as fallback.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/play-provider-speed-duplicate

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/bot/src/functions/music/commands/play/index.ts (1)

24-24: ⚠️ Potential issue | 🟡 Minor

Remove unused import: createMusicControlButtons.

This import on line 24 is no longer used after the components parameter was removed from the interactionReply call.

🧹 Proposed fix
-import { createMusicControlButtons } from '../../../../utils/music/buttonComponents'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/bot/src/functions/music/commands/play/index.ts` at line 24, Remove
the unused import createMusicControlButtons from the top of the file: locate the
import statement that references createMusicControlButtons and delete it so
there are no unused imports remaining (this relates to the import line that
previously provided music button components for the play command but is no
longer referenced after removing the components parameter from
interactionReply).
🧹 Nitpick comments (1)
packages/bot/src/functions/music/commands/play/index.ts (1)

233-238: Clarify: Position 0 tracks show "Added to Queue #0".

When queuePosition === 0, the embed will display "Added to Queue" with position "#0" instead of the previous "Now Playing" header. This is intentional per the PR to let sendNowPlayingEmbed be the single authoritative "Now Playing" display.

Consider whether "#0" is intuitive for users (typically queue positions start at 1). If position 0 means "playing now," you might want to omit the position field or show "Up Next" to avoid confusion.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/bot/src/functions/music/commands/play/index.ts` around lines 233 -
238, The embed currently shows "Added to Queue `#0`" when queuePosition === 0;
change the call to buildPlayResponseEmbed so zero positions don't show "#0": if
queuePosition === 0, call buildPlayResponseEmbed with kind: 'upNext' (or omit
queuePosition) and do not pass queuePosition, otherwise pass kind:
'addedToQueue' with queuePosition; update the conditional around the
buildPlayResponseEmbed invocation (the call site using track, requestedBy:
interaction.user, queuePosition) to handle the queuePosition === 0 case so the
embed shows "Up Next" or no position instead of "#0".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/bot/src/functions/music/commands/play/queryUtils.ts`:
- Around line 60-62: Comment incorrectly says the fallback after
QueryType.SPOTIFY_SEARCH is "YouTube then AUTO"; update the comment to reflect
the actual fallback used in play/index.ts (YouTube then SOUNDCLOUD_SEARCH).
Locate the comment adjacent to QueryType.SPOTIFY_SEARCH in queryUtils.ts and
change the text to explicitly state "tries YouTube then SOUNDCLOUD_SEARCH" so it
matches the implementation in play/index.ts.

---

Outside diff comments:
In `@packages/bot/src/functions/music/commands/play/index.ts`:
- Line 24: Remove the unused import createMusicControlButtons from the top of
the file: locate the import statement that references createMusicControlButtons
and delete it so there are no unused imports remaining (this relates to the
import line that previously provided music button components for the play
command but is no longer referenced after removing the components parameter from
interactionReply).

---

Nitpick comments:
In `@packages/bot/src/functions/music/commands/play/index.ts`:
- Around line 233-238: The embed currently shows "Added to Queue `#0`" when
queuePosition === 0; change the call to buildPlayResponseEmbed so zero positions
don't show "#0": if queuePosition === 0, call buildPlayResponseEmbed with kind:
'upNext' (or omit queuePosition) and do not pass queuePosition, otherwise pass
kind: 'addedToQueue' with queuePosition; update the conditional around the
buildPlayResponseEmbed invocation (the call site using track, requestedBy:
interaction.user, queuePosition) to handle the queuePosition === 0 case so the
embed shows "Up Next" or no position instead of "#0".
🪄 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: e73e1cd5-f856-45fe-b971-83ddce819a24

📥 Commits

Reviewing files that changed from the base of the PR and between e5aaf27 and d434019.

📒 Files selected for processing (3)
  • packages/bot/src/functions/music/commands/play/index.spec.ts
  • packages/bot/src/functions/music/commands/play/index.ts
  • packages/bot/src/functions/music/commands/play/queryUtils.ts
💤 Files with no reviewable changes (1)
  • packages/bot/src/functions/music/commands/play/index.spec.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: Quality Gates
  • GitHub Check: SonarCloud Scan
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{js,jsx,ts,tsx,vue,html}

📄 CodeRabbit inference engine (.cursor/rules/accessibility-openness.mdc)

Provide accessible UI components using semantic HTML and ARIA attributes where necessary

Files:

  • packages/bot/src/functions/music/commands/play/index.ts
  • packages/bot/src/functions/music/commands/play/queryUtils.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/dependency-injection.mdc)

**/*.{ts,tsx,js,jsx}: Prefer constructor injection for classes that require dependencies
Avoid global mutable singletons unless necessary
Use explicit interfaces for external dependencies to make testing easier

**/*.{ts,tsx,js,jsx}: Include required references in PRs/code for non-trivial logic: TypeScript (official docs), MDN (JavaScript reference), and official docs for any runtime/framework/libraries used (e.g., Node.js, React) as applicable.
Before assuming behavior of an API, include the doc link and a ≤25-word quote when the change relies on it.

**/*.{ts,tsx,js,jsx}: Prefer named exports for clear usage and easier refactors in TypeScript/JavaScript
Keep import order consistent: external first, then internal modules
Remove dead code and unused imports

**/*.{ts,tsx,js,jsx}: Use PascalCase naming convention for React/UI components
Use camelCase naming convention for variables and functions
Use UPPER_SNAKE_CASE naming convention for constants
Maintain consistent import grouping and ordering within the project, keeping third-party imports separate from local imports
For external data sources (HTTP, database), always validate and sanitize input using type guards or schema validators

Implement TypeScript typecheck and linter in CI quality checks

**/*.{ts,tsx,js,jsx}: Use TypeScript for enhanced type safety
Implement error handling and error logging
Avoid commenting code unless extremely necessary - code should explain itself with descriptive names
Leave NO todos, placeholders or missing pieces in the code
Variables and functions must use camelCase
Constants must use UPPER_SNAKE_CASE
Use arrow functions for methods and computed properties
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Maintain consistent import grouping/order: external imports first, then internal modules
Use named exports for clear usage and easier refactors
Always validate and sanitize external data (HTTP, DB) at the boundary using type guards ...

Files:

  • packages/bot/src/functions/music/commands/play/index.ts
  • packages/bot/src/functions/music/commands/play/queryUtils.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)

**/*.{js,jsx,ts,tsx}: Never throw strings. Throw Error (or typed subclasses) with descriptive messages
Include causal error as cause when available for better debugging
Define clear, stable error codes (e.g., ERR_AUTH_EXPIRED, ERR_NETWORK_TIMEOUT)
Provide optional metadata (e.g., details, retryable, status, correlationId) in error objects
Use domain error classes per area (e.g., AuthenticationError, ValidationError, NetworkError)
Log errors with structure (message, code, stack, cause, correlationId, user context where appropriate)
Mark retryable vs nonRetryable errors where helpful for operations
Set timeouts and handle aborts/cancellations; avoid dangling requests in API/network code
Implement backoff for transient failures; avoid infinite retries
Map HTTP status → domain errors; 4xx vs 5xx behave differently (e.g., retry for 5xx/network)

**/*.{js,jsx,ts,tsx}: Use functional components with hooks in React/React Native. Avoid class components.
Keep components focused on a single responsibility; extract complex logic into custom hooks.
Keep state local when possible. Use Context/Zustand/Redux only when necessary for state management.
If props or state traverse more than 3 levels, consider using context or a feature-scoped store instead of prop drilling.
Use performance optimization techniques: React.memo, useMemo, useCallback, Suspense (web), and virtualization for long lists; avoid unnecessary re-renders.
Web accessibility: use semantic HTML, labels, focus management, keyboard navigation, and aria-* attributes as needed.
React Native accessibility: use accessibility props (accessible, accessibilityLabel), proper roles and labels.
Identify and extract repetitive UI components proactively to components/ with clear props and minimal coupling.
Web styles: prefer co-located styles or design system tokens; avoid global style leakage.
React Native styles: prefer StyleSheet.create, design tokens, and theme providers; avoid in...

Files:

  • packages/bot/src/functions/music/commands/play/index.ts
  • packages/bot/src/functions/music/commands/play/queryUtils.ts
**/index.ts

📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)

Use index.ts only to re-export a small, intentional surface per module

Files:

  • packages/bot/src/functions/music/commands/play/index.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)

Introduce interfaces at module boundaries to enable testing and substitutions

**/*.{ts,tsx}: Avoid using any type in TypeScript. If unavoidable, use unknown with type guards and justify with a code comment
Prefer interface for defining public object shapes in TypeScript, use type for unions and utility types
Use TypeScript utility types such as Partial, Pick, Omit, Readonly, and Record when appropriate
Use I{Name} naming convention for interfaces in TypeScript
Use T{Name} naming convention for type aliases and utility types in TypeScript

**/*.{ts,tsx}: Prefer types over interfaces for most cases
Don't ever use any - type safety always
Avoid enums; use const objects instead
For complex types, create a separate file to declare them and import them
Avoid using any type; if unavoidable, use unknown with type guards and justify with code comment
Prefer interface for public API shapes; use type for unions and utility types
Use TypeScript utility types (Partial, Pick, Omit, Readonly, Record)

Files:

  • packages/bot/src/functions/music/commands/play/index.ts
  • packages/bot/src/functions/music/commands/play/queryUtils.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/documentation.mdc)

**/*.{js,ts,tsx,jsx}: Minimize comments in code; explain the 'why' when non-obvious, let code express the 'what' through clear naming
Document trade-offs briefly when deviating from ideal patterns

**/*.{js,ts,tsx,jsx}: Store secrets, ports, and hosts in environment variables (.env, .env.example) and never hardcode them
Avoid redundant or decorative AI comments; code should be self-explanatory and only commented when logic is non-obvious; prefer refactoring over lengthy comments

Files:

  • packages/bot/src/functions/music/commands/play/index.ts
  • packages/bot/src/functions/music/commands/play/queryUtils.ts
packages/bot/src/functions/*/commands/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)

packages/bot/src/functions/*/commands/**/*.{ts,tsx}: Command model must include data (slash builder), execute, and category properties exported from packages/bot/src/models/Command.ts
Use @discordjs/builders for building the data (SlashCommandBuilder) in command definitions
Command execute function must receive { interaction, client } parameters from CommandExecuteParams type
Use interactionReply and createUserFriendlyError utilities from @lucky/shared/general utils for command replies and error handling
Use existing validators from packages/bot/src/utils/command/ for voice channel, queue, and guild validations in commands

Files:

  • packages/bot/src/functions/music/commands/play/index.ts
  • packages/bot/src/functions/music/commands/play/queryUtils.ts
packages/bot/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)

packages/bot/**/*.{ts,tsx}: Use useMainPlayer() from discord-player to access the player instance; do not instantiate player directly
Do not duplicate queue or player state outside Discord Player; use shared services from @lucky/shared for persistent data like track history and session information
Use errorLog and debugLog from @lucky/shared/utils for logging throughout the bot package
Use embed and reply utilities from @lucky/shared for consistent message formatting and error sanitization across the bot
Use services from @lucky/shared (DatabaseService, Redis client) for database and cache access; do not instantiate Prisma or Redis directly in the bot package

Files:

  • packages/bot/src/functions/music/commands/play/index.ts
  • packages/bot/src/functions/music/commands/play/queryUtils.ts
packages/bot/**

📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)

The bot package depends on shared and contains Discord bot commands and player handlers using Discord.js and Discord Player

Files:

  • packages/bot/src/functions/music/commands/play/index.ts
  • packages/bot/src/functions/music/commands/play/queryUtils.ts
**/*.{js,mjs,ts,mts}

📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)

Use Node.js version ≥22 with ESM (ECMAScript modules) only; no CommonJS

Files:

  • packages/bot/src/functions/music/commands/play/index.ts
  • packages/bot/src/functions/music/commands/play/queryUtils.ts
packages/bot/src/functions/{general,music,download}/commands/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)

packages/bot/src/functions/{general,music,download}/commands/**/*.ts: Apply .cursor/rules/lucky-discord-bot.mdc rules for Discord bot commands and player implementation
Use .cursor/skills/discord-commands/SKILL.md for implementing slash commands

Files:

  • packages/bot/src/functions/music/commands/play/index.ts
  • packages/bot/src/functions/music/commands/play/queryUtils.ts
packages/bot/src/functions/music/commands/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)

Use .cursor/skills/music-queue-player/SKILL.md for play, queue, skip, volume commands and player lifecycle management

Files:

  • packages/bot/src/functions/music/commands/play/index.ts
  • packages/bot/src/functions/music/commands/play/queryUtils.ts
packages/bot/src/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)

Use @lucky/shared for database, Redis, logging, and embed utilities instead of implementing them locally

Files:

  • packages/bot/src/functions/music/commands/play/index.ts
  • packages/bot/src/functions/music/commands/play/queryUtils.ts
packages/bot/src/functions/music/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)

Use existing voice/queue/guild validators before manipulating player or queue state

Files:

  • packages/bot/src/functions/music/commands/play/index.ts
  • packages/bot/src/functions/music/commands/play/queryUtils.ts
🔇 Additional comments (2)
packages/bot/src/functions/music/commands/play/index.ts (2)

176-184: LGTM! Fallback chain correctly updated.

The fallback from YouTube to SoundCloud aligns with the PR objective of making the provider priority explicit (Spotify → YouTube → SoundCloud). The log message accurately reflects this change.


253-256: LGTM! Simplified initial response.

Removing the control buttons from the initial reply aligns with the PR objective of having sendNowPlayingEmbed (triggered by playerStart) as the single authoritative "Now Playing" display with controls. This prevents the race condition described in the PR.

Comment on lines +60 to +62
// Spotify first: best metadata (titles, artwork, artist).
// Fallback chain in play/index.ts tries YouTube then AUTO if Spotify throws.
return QueryType.SPOTIFY_SEARCH

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Comment references incorrect fallback engine.

Line 61 states the fallback chain "tries YouTube then AUTO," but the actual implementation in play/index.ts (lines 175-184) falls back to SOUNDCLOUD_SEARCH, not AUTO. This inconsistency could mislead future maintainers.

📝 Proposed fix
-            // Spotify first: best metadata (titles, artwork, artist).
-            // Fallback chain in play/index.ts tries YouTube then AUTO if Spotify throws.
+            // Spotify first: best metadata (titles, artwork, artist).
+            // Fallback chain in play/index.ts tries YouTube then SoundCloud if Spotify throws.
             return QueryType.SPOTIFY_SEARCH
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Spotify first: best metadata (titles, artwork, artist).
// Fallback chain in play/index.ts tries YouTube then AUTO if Spotify throws.
return QueryType.SPOTIFY_SEARCH
// Spotify first: best metadata (titles, artwork, artist).
// Fallback chain in play/index.ts tries YouTube then SoundCloud if Spotify throws.
return QueryType.SPOTIFY_SEARCH
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/bot/src/functions/music/commands/play/queryUtils.ts` around lines 60
- 62, Comment incorrectly says the fallback after QueryType.SPOTIFY_SEARCH is
"YouTube then AUTO"; update the comment to reflect the actual fallback used in
play/index.ts (YouTube then SOUNDCLOUD_SEARCH). Locate the comment adjacent to
QueryType.SPOTIFY_SEARCH in queryUtils.ts and change the text to explicitly
state "tries YouTube then SOUNDCLOUD_SEARCH" so it matches the implementation in
play/index.ts.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/bot/src/utils/music/queueManipulation.ts`:
- Around line 788-790: The batch dedup logic allows same-title/different-author
tracks because addSelectedTracks currently only inserts normalizeTrackKey(...)
into excludedKeys; modify addSelectedTracks to also add
normalizeTitleOnly(track.title) to excludedKeys whenever a track is
selected/added (use the same normalization helper normalizeTitleOnly and
normalizeTrackKey), so both exact track keys and title-only keys are marked
excluded within the same replenish batch and the check in the existing
conditional will correctly prevent inserting title-only duplicates.

In `@packages/bot/src/utils/music/searchQueryCleaner.ts`:
- Around line 88-91: The regexes in searchQueryCleaner (the list of suffix
patterns in packages/bot/src/utils/music/searchQueryCleaner.ts) miss em dash
characters and the "official lyric video" variant, and they can leave trailing
separators like "-" in cleaned titles; update the suffix patterns to include the
em dash character (use the class [-–—] wherever separators are matched), add a
pattern that matches "official lyric (video|audio)" and "lyric video"
variations, and ensure patterns allow optional trailing punctuation/whitespace
so titles don't end with leftover separators — update the array of regexes used
by the cleaner (the suffix match list) accordingly and add a final trim step to
remove any leftover separators/whitespace after applying the regexes.
🪄 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: 2b53567f-7462-4e60-9d79-13aa7a3f310a

📥 Commits

Reviewing files that changed from the base of the PR and between d434019 and a8ae289.

📒 Files selected for processing (2)
  • packages/bot/src/utils/music/queueManipulation.ts
  • packages/bot/src/utils/music/searchQueryCleaner.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: SonarCloud Scan
  • GitHub Check: Quality Gates
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx,vue,html}

📄 CodeRabbit inference engine (.cursor/rules/accessibility-openness.mdc)

Provide accessible UI components using semantic HTML and ARIA attributes where necessary

Files:

  • packages/bot/src/utils/music/queueManipulation.ts
  • packages/bot/src/utils/music/searchQueryCleaner.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/dependency-injection.mdc)

**/*.{ts,tsx,js,jsx}: Prefer constructor injection for classes that require dependencies
Avoid global mutable singletons unless necessary
Use explicit interfaces for external dependencies to make testing easier

**/*.{ts,tsx,js,jsx}: Include required references in PRs/code for non-trivial logic: TypeScript (official docs), MDN (JavaScript reference), and official docs for any runtime/framework/libraries used (e.g., Node.js, React) as applicable.
Before assuming behavior of an API, include the doc link and a ≤25-word quote when the change relies on it.

**/*.{ts,tsx,js,jsx}: Prefer named exports for clear usage and easier refactors in TypeScript/JavaScript
Keep import order consistent: external first, then internal modules
Remove dead code and unused imports

**/*.{ts,tsx,js,jsx}: Use PascalCase naming convention for React/UI components
Use camelCase naming convention for variables and functions
Use UPPER_SNAKE_CASE naming convention for constants
Maintain consistent import grouping and ordering within the project, keeping third-party imports separate from local imports
For external data sources (HTTP, database), always validate and sanitize input using type guards or schema validators

Implement TypeScript typecheck and linter in CI quality checks

**/*.{ts,tsx,js,jsx}: Use TypeScript for enhanced type safety
Implement error handling and error logging
Avoid commenting code unless extremely necessary - code should explain itself with descriptive names
Leave NO todos, placeholders or missing pieces in the code
Variables and functions must use camelCase
Constants must use UPPER_SNAKE_CASE
Use arrow functions for methods and computed properties
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Maintain consistent import grouping/order: external imports first, then internal modules
Use named exports for clear usage and easier refactors
Always validate and sanitize external data (HTTP, DB) at the boundary using type guards ...

Files:

  • packages/bot/src/utils/music/queueManipulation.ts
  • packages/bot/src/utils/music/searchQueryCleaner.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)

**/*.{js,jsx,ts,tsx}: Never throw strings. Throw Error (or typed subclasses) with descriptive messages
Include causal error as cause when available for better debugging
Define clear, stable error codes (e.g., ERR_AUTH_EXPIRED, ERR_NETWORK_TIMEOUT)
Provide optional metadata (e.g., details, retryable, status, correlationId) in error objects
Use domain error classes per area (e.g., AuthenticationError, ValidationError, NetworkError)
Log errors with structure (message, code, stack, cause, correlationId, user context where appropriate)
Mark retryable vs nonRetryable errors where helpful for operations
Set timeouts and handle aborts/cancellations; avoid dangling requests in API/network code
Implement backoff for transient failures; avoid infinite retries
Map HTTP status → domain errors; 4xx vs 5xx behave differently (e.g., retry for 5xx/network)

**/*.{js,jsx,ts,tsx}: Use functional components with hooks in React/React Native. Avoid class components.
Keep components focused on a single responsibility; extract complex logic into custom hooks.
Keep state local when possible. Use Context/Zustand/Redux only when necessary for state management.
If props or state traverse more than 3 levels, consider using context or a feature-scoped store instead of prop drilling.
Use performance optimization techniques: React.memo, useMemo, useCallback, Suspense (web), and virtualization for long lists; avoid unnecessary re-renders.
Web accessibility: use semantic HTML, labels, focus management, keyboard navigation, and aria-* attributes as needed.
React Native accessibility: use accessibility props (accessible, accessibilityLabel), proper roles and labels.
Identify and extract repetitive UI components proactively to components/ with clear props and minimal coupling.
Web styles: prefer co-located styles or design system tokens; avoid global style leakage.
React Native styles: prefer StyleSheet.create, design tokens, and theme providers; avoid in...

Files:

  • packages/bot/src/utils/music/queueManipulation.ts
  • packages/bot/src/utils/music/searchQueryCleaner.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)

Introduce interfaces at module boundaries to enable testing and substitutions

**/*.{ts,tsx}: Avoid using any type in TypeScript. If unavoidable, use unknown with type guards and justify with a code comment
Prefer interface for defining public object shapes in TypeScript, use type for unions and utility types
Use TypeScript utility types such as Partial, Pick, Omit, Readonly, and Record when appropriate
Use I{Name} naming convention for interfaces in TypeScript
Use T{Name} naming convention for type aliases and utility types in TypeScript

**/*.{ts,tsx}: Prefer types over interfaces for most cases
Don't ever use any - type safety always
Avoid enums; use const objects instead
For complex types, create a separate file to declare them and import them
Avoid using any type; if unavoidable, use unknown with type guards and justify with code comment
Prefer interface for public API shapes; use type for unions and utility types
Use TypeScript utility types (Partial, Pick, Omit, Readonly, Record)

Files:

  • packages/bot/src/utils/music/queueManipulation.ts
  • packages/bot/src/utils/music/searchQueryCleaner.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/documentation.mdc)

**/*.{js,ts,tsx,jsx}: Minimize comments in code; explain the 'why' when non-obvious, let code express the 'what' through clear naming
Document trade-offs briefly when deviating from ideal patterns

**/*.{js,ts,tsx,jsx}: Store secrets, ports, and hosts in environment variables (.env, .env.example) and never hardcode them
Avoid redundant or decorative AI comments; code should be self-explanatory and only commented when logic is non-obvious; prefer refactoring over lengthy comments

Files:

  • packages/bot/src/utils/music/queueManipulation.ts
  • packages/bot/src/utils/music/searchQueryCleaner.ts
packages/bot/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)

packages/bot/**/*.{ts,tsx}: Use useMainPlayer() from discord-player to access the player instance; do not instantiate player directly
Do not duplicate queue or player state outside Discord Player; use shared services from @lucky/shared for persistent data like track history and session information
Use errorLog and debugLog from @lucky/shared/utils for logging throughout the bot package
Use embed and reply utilities from @lucky/shared for consistent message formatting and error sanitization across the bot
Use services from @lucky/shared (DatabaseService, Redis client) for database and cache access; do not instantiate Prisma or Redis directly in the bot package

Files:

  • packages/bot/src/utils/music/queueManipulation.ts
  • packages/bot/src/utils/music/searchQueryCleaner.ts
packages/bot/**

📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)

The bot package depends on shared and contains Discord bot commands and player handlers using Discord.js and Discord Player

Files:

  • packages/bot/src/utils/music/queueManipulation.ts
  • packages/bot/src/utils/music/searchQueryCleaner.ts
**/*.{js,mjs,ts,mts}

📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)

Use Node.js version ≥22 with ESM (ECMAScript modules) only; no CommonJS

Files:

  • packages/bot/src/utils/music/queueManipulation.ts
  • packages/bot/src/utils/music/searchQueryCleaner.ts
packages/bot/src/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)

Use @lucky/shared for database, Redis, logging, and embed utilities instead of implementing them locally

Files:

  • packages/bot/src/utils/music/queueManipulation.ts
  • packages/bot/src/utils/music/searchQueryCleaner.ts
🔇 Additional comments (1)
packages/bot/src/utils/music/queueManipulation.ts (1)

332-343: Good normalization expansion for duplicate prevention.

Including both title::author and title-only normalized keys is a solid improvement and aligns with the dedup objective.

Also applies to: 767-769

Comment on lines +788 to +790
if (excludedKeys.has(normalizeTrackKey(track.title, track.author)))
return true
return excludedKeys.has(normalizeTitleOnly(track.title))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Title-only dedup is still bypassable within the same replenish batch.

Line 790 checks title-only duplicates against excludedKeys, but addSelectedTracks only adds normalizeTrackKey(...). In one batch, two same-title/different-author candidates can still both be inserted.

Proposed fix (apply in addSelectedTracks)
 function addSelectedTracks(
     queue: GuildQueue,
     selected: ScoredTrack[],
     excludedUrls: Set<string>,
     excludedKeys: Set<string>,
     requestedById?: string,
 ): void {
     for (const candidate of selected) {
+        const fullKey = normalizeTrackKey(
+            candidate.track.title,
+            candidate.track.author,
+        )
+        const titleOnlyKey = normalizeTitleOnly(candidate.track.title)
+        if (
+            (candidate.track.url && excludedUrls.has(candidate.track.url)) ||
+            excludedKeys.has(fullKey) ||
+            excludedKeys.has(titleOnlyKey)
+        ) {
+            continue
+        }
+
         markAsAutoplayTrack(candidate.track, candidate.reason, requestedById)
         queue.addTrack(candidate.track)
         excludedUrls.add(candidate.track.url)
-        excludedKeys.add(
-            normalizeTrackKey(candidate.track.title, candidate.track.author),
-        )
+        excludedKeys.add(fullKey)
+        excludedKeys.add(titleOnlyKey)
     }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/bot/src/utils/music/queueManipulation.ts` around lines 788 - 790,
The batch dedup logic allows same-title/different-author tracks because
addSelectedTracks currently only inserts normalizeTrackKey(...) into
excludedKeys; modify addSelectedTracks to also add
normalizeTitleOnly(track.title) to excludedKeys whenever a track is
selected/added (use the same normalization helper normalizeTitleOnly and
normalizeTrackKey), so both exact track keys and title-only keys are marked
excluded within the same replenish batch and the check in the existing
conditional will correctly prevent inserting title-only duplicates.

Comment thread packages/bot/src/utils/music/searchQueryCleaner.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (1)
packages/bot/src/handlers/player/lifecycleHandlers.ts (1)

62-71: ⚠️ Potential issue | 🟠 Major

Removing the direct recovery call isn't enough here.

connectionDestroyed no longer calls checkAndRecover(), but it also doesn't clear the armed watchdog or mark the guild as intentional. If that timer fires later, or a following disconnect runs before the stop is marked elsewhere, recovery can still run against a queue you just treated as terminal. Please clear or mark the guild in connectionDestroyed too.

Also applies to: 85-95

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/bot/src/handlers/player/lifecycleHandlers.ts` around lines 62 - 71,
The connectionDestroyed handler (the player.events.on('connectionDestroyed'
callback that calls voiceStatus.clearStatus, musicPresence.clearMusicPresence,
and musicSessionSnapshotService.saveSnapshot) must also neutralize any recovery
triggers: explicitly clear the armed watchdog/timer for the guild and mark the
guild/queue as an intentional terminal stop so recovery logic won't run later.
Update the handler to call the existing functions that clear the watchdog/timer
(e.g., clearArmedWatchdog or equivalent) and to set the guild/queue as
intentionally stopped (e.g., markGuildIntentional or setIntentionalStop)
immediately before returning; apply the same change to the disconnect handling
block around lines 85-95.
🧹 Nitpick comments (3)
packages/bot/src/functions/music/commands/queue/queueEmbed.ts (1)

67-67: Remove redundant fallback and keep message contract in one place.

On Line 67, trackList || 'No displayable tracks' is redundant because createTrackListDisplay already guarantees a non-empty string (packages/bot/src/functions/music/commands/queue/queueDisplay.ts, Line 89). Keeping both creates inconsistent copy paths.

Suggested simplification
-            value: trackList || 'No displayable tracks',
+            value: trackList,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/bot/src/functions/music/commands/queue/queueEmbed.ts` at line 67,
The value fallback is redundant—remove the inline '|| "No displayable tracks"'
in the embed field and rely on createTrackListDisplay to provide a non-empty
string; update the value assignment in queueEmbed.ts (the embed field using
trackList) to simply use trackList, and keep the "No displayable tracks" message
only inside createTrackListDisplay (referenced in queueDisplay.ts) so the
message contract remains centralized and consistent.
packages/bot/tests/utils/music/watchdog.test.ts (1)

91-100: Assert through isIntentionalStop() instead of the private Set.

These cases still reach into service['intentionalStops'], so they won't catch a regression in the public method you just added. Using service.isIntentionalStop(...) keeps the suite behavioral.

As per coding guidelines, "Test behavior, not implementation details".

Also applies to: 128-142

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/bot/tests/utils/music/watchdog.test.ts` around lines 91 - 100, The
test is asserting on the private Set service['intentionalStops'] instead of the
public API; change the assertions to call the public method
service.isIntentionalStop('guild-1') after using
service.markIntentionalStop('guild-1') (and after advancing timers) so the spec
tests behavior not implementation details; update both the block around the
markIntentionalStop assertion and the similar case later in the file to use
isIntentionalStop instead of accessing intentionalStops directly.
packages/bot/src/handlers/player/lifecycleHandlers.spec.ts (1)

37-38: setupVoiceKickDetection() still isn't exercised in this suite.

The new voice-kick path is the behavior that distinguishes manual kicks from normal disconnects, but these additions only cover player lifecycle events. A small voiceStateUpdate test for bot-kicked, other-user, and channel-move cases would pin that contract down.

As per coding guidelines, "Test behavior, not implementation details".

Also applies to: 102-167

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/bot/src/handlers/player/lifecycleHandlers.spec.ts` around lines 37 -
38, Add tests that exercise setupVoiceKickDetection by simulating
voiceStateUpdate events for the three distinguishing cases: bot-kicked (user was
disconnected by the bot), other-user-kick (disconnected by another user), and
channel-move (same guild but moved channel). In the existing
lifecycleHandlers.spec suite, invoke setupVoiceKickDetection and emit mock
voiceStateUpdate payloads (old and new states) to assert the handler marks
bot-kick vs normal disconnect vs move appropriately; reference the
setupVoiceKickDetection function and the voiceStateUpdate event in your test
cases and assert the observable behavior rather than internals.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/bot/src/utils/music/queueManipulation.spec.ts`:
- Around line 1392-1400: The test incorrectly calls replenishQueue with extra
parameters and uses an any cast and a weak assertion; change the call to
replenishQueue(queue) (remove targetQueueSize and guildId), eliminate the (queue
as any) cast by using the properly-typed test fixture or a minimal safe cast
(e.g., queue as unknown as Queue) and strengthen the assertion to verify no
tracks were added at all by asserting that queue.addTrack was not called
(expect(queue.addTrack).not.toHaveBeenCalled()) rather than checking for a
specific title.

In `@packages/bot/src/utils/music/watchdog.ts`:
- Around line 105-113: markIntentionalStop currently sets a guild in
intentionalStops for timeoutMs + 10_000 but nothing clears it when a new
playback/session starts, causing new sessions within that window to be treated
as "intentional" stops; to fix, remove the flag at the start of a new session by
calling this.intentionalStops.delete(guildId) from the code path that
creates/starts a player or establishes a new connection (e.g., inside the
player/connection initializer such as the function that joins voice or
startPlayback/createOrGetPlayer), and ensure markIntentionalStop still schedules
the timeout but the immediate delete on new session prevents bleed into
subsequent playbacks.

---

Outside diff comments:
In `@packages/bot/src/handlers/player/lifecycleHandlers.ts`:
- Around line 62-71: The connectionDestroyed handler (the
player.events.on('connectionDestroyed' callback that calls
voiceStatus.clearStatus, musicPresence.clearMusicPresence, and
musicSessionSnapshotService.saveSnapshot) must also neutralize any recovery
triggers: explicitly clear the armed watchdog/timer for the guild and mark the
guild/queue as an intentional terminal stop so recovery logic won't run later.
Update the handler to call the existing functions that clear the watchdog/timer
(e.g., clearArmedWatchdog or equivalent) and to set the guild/queue as
intentionally stopped (e.g., markGuildIntentional or setIntentionalStop)
immediately before returning; apply the same change to the disconnect handling
block around lines 85-95.

---

Nitpick comments:
In `@packages/bot/src/functions/music/commands/queue/queueEmbed.ts`:
- Line 67: The value fallback is redundant—remove the inline '|| "No displayable
tracks"' in the embed field and rely on createTrackListDisplay to provide a
non-empty string; update the value assignment in queueEmbed.ts (the embed field
using trackList) to simply use trackList, and keep the "No displayable tracks"
message only inside createTrackListDisplay (referenced in queueDisplay.ts) so
the message contract remains centralized and consistent.

In `@packages/bot/src/handlers/player/lifecycleHandlers.spec.ts`:
- Around line 37-38: Add tests that exercise setupVoiceKickDetection by
simulating voiceStateUpdate events for the three distinguishing cases:
bot-kicked (user was disconnected by the bot), other-user-kick (disconnected by
another user), and channel-move (same guild but moved channel). In the existing
lifecycleHandlers.spec suite, invoke setupVoiceKickDetection and emit mock
voiceStateUpdate payloads (old and new states) to assert the handler marks
bot-kick vs normal disconnect vs move appropriately; reference the
setupVoiceKickDetection function and the voiceStateUpdate event in your test
cases and assert the observable behavior rather than internals.

In `@packages/bot/tests/utils/music/watchdog.test.ts`:
- Around line 91-100: The test is asserting on the private Set
service['intentionalStops'] instead of the public API; change the assertions to
call the public method service.isIntentionalStop('guild-1') after using
service.markIntentionalStop('guild-1') (and after advancing timers) so the spec
tests behavior not implementation details; update both the block around the
markIntentionalStop assertion and the similar case later in the file to use
isIntentionalStop instead of accessing intentionalStops directly.
🪄 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: 18388f6d-f8a8-4956-8ae1-bc788cbc7749

📥 Commits

Reviewing files that changed from the base of the PR and between a8ae289 and 22bc4dc.

📒 Files selected for processing (11)
  • packages/bot/src/functions/music/commands/queue/queueEmbed.ts
  • packages/bot/src/handlers/player/index.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.ts
  • packages/bot/src/handlers/player/trackHandlers.spec.ts
  • packages/bot/src/handlers/player/trackHandlers.ts
  • packages/bot/src/utils/music/queueManipulation.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.ts
  • packages/bot/src/utils/music/watchdog.ts
  • packages/bot/tests/utils/music/watchdog.test.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/bot/src/handlers/player/trackHandlers.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/bot/src/utils/music/searchQueryCleaner.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: SonarCloud Scan
  • GitHub Check: Quality Gates
🧰 Additional context used
📓 Path-based instructions (21)
**/*.{js,jsx,ts,tsx,vue,html}

📄 CodeRabbit inference engine (.cursor/rules/accessibility-openness.mdc)

Provide accessible UI components using semantic HTML and ARIA attributes where necessary

Files:

  • packages/bot/src/functions/music/commands/queue/queueEmbed.ts
  • packages/bot/src/handlers/player/trackHandlers.ts
  • packages/bot/src/utils/music/queueManipulation.spec.ts
  • packages/bot/src/handlers/player/index.ts
  • packages/bot/tests/utils/music/watchdog.test.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.spec.ts
  • packages/bot/src/utils/music/watchdog.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/dependency-injection.mdc)

**/*.{ts,tsx,js,jsx}: Prefer constructor injection for classes that require dependencies
Avoid global mutable singletons unless necessary
Use explicit interfaces for external dependencies to make testing easier

**/*.{ts,tsx,js,jsx}: Include required references in PRs/code for non-trivial logic: TypeScript (official docs), MDN (JavaScript reference), and official docs for any runtime/framework/libraries used (e.g., Node.js, React) as applicable.
Before assuming behavior of an API, include the doc link and a ≤25-word quote when the change relies on it.

**/*.{ts,tsx,js,jsx}: Prefer named exports for clear usage and easier refactors in TypeScript/JavaScript
Keep import order consistent: external first, then internal modules
Remove dead code and unused imports

**/*.{ts,tsx,js,jsx}: Use PascalCase naming convention for React/UI components
Use camelCase naming convention for variables and functions
Use UPPER_SNAKE_CASE naming convention for constants
Maintain consistent import grouping and ordering within the project, keeping third-party imports separate from local imports
For external data sources (HTTP, database), always validate and sanitize input using type guards or schema validators

Implement TypeScript typecheck and linter in CI quality checks

**/*.{ts,tsx,js,jsx}: Use TypeScript for enhanced type safety
Implement error handling and error logging
Avoid commenting code unless extremely necessary - code should explain itself with descriptive names
Leave NO todos, placeholders or missing pieces in the code
Variables and functions must use camelCase
Constants must use UPPER_SNAKE_CASE
Use arrow functions for methods and computed properties
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Maintain consistent import grouping/order: external imports first, then internal modules
Use named exports for clear usage and easier refactors
Always validate and sanitize external data (HTTP, DB) at the boundary using type guards ...

Files:

  • packages/bot/src/functions/music/commands/queue/queueEmbed.ts
  • packages/bot/src/handlers/player/trackHandlers.ts
  • packages/bot/src/utils/music/queueManipulation.spec.ts
  • packages/bot/src/handlers/player/index.ts
  • packages/bot/tests/utils/music/watchdog.test.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.spec.ts
  • packages/bot/src/utils/music/watchdog.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)

**/*.{js,jsx,ts,tsx}: Never throw strings. Throw Error (or typed subclasses) with descriptive messages
Include causal error as cause when available for better debugging
Define clear, stable error codes (e.g., ERR_AUTH_EXPIRED, ERR_NETWORK_TIMEOUT)
Provide optional metadata (e.g., details, retryable, status, correlationId) in error objects
Use domain error classes per area (e.g., AuthenticationError, ValidationError, NetworkError)
Log errors with structure (message, code, stack, cause, correlationId, user context where appropriate)
Mark retryable vs nonRetryable errors where helpful for operations
Set timeouts and handle aborts/cancellations; avoid dangling requests in API/network code
Implement backoff for transient failures; avoid infinite retries
Map HTTP status → domain errors; 4xx vs 5xx behave differently (e.g., retry for 5xx/network)

**/*.{js,jsx,ts,tsx}: Use functional components with hooks in React/React Native. Avoid class components.
Keep components focused on a single responsibility; extract complex logic into custom hooks.
Keep state local when possible. Use Context/Zustand/Redux only when necessary for state management.
If props or state traverse more than 3 levels, consider using context or a feature-scoped store instead of prop drilling.
Use performance optimization techniques: React.memo, useMemo, useCallback, Suspense (web), and virtualization for long lists; avoid unnecessary re-renders.
Web accessibility: use semantic HTML, labels, focus management, keyboard navigation, and aria-* attributes as needed.
React Native accessibility: use accessibility props (accessible, accessibilityLabel), proper roles and labels.
Identify and extract repetitive UI components proactively to components/ with clear props and minimal coupling.
Web styles: prefer co-located styles or design system tokens; avoid global style leakage.
React Native styles: prefer StyleSheet.create, design tokens, and theme providers; avoid in...

Files:

  • packages/bot/src/functions/music/commands/queue/queueEmbed.ts
  • packages/bot/src/handlers/player/trackHandlers.ts
  • packages/bot/src/utils/music/queueManipulation.spec.ts
  • packages/bot/src/handlers/player/index.ts
  • packages/bot/tests/utils/music/watchdog.test.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.spec.ts
  • packages/bot/src/utils/music/watchdog.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)

Introduce interfaces at module boundaries to enable testing and substitutions

**/*.{ts,tsx}: Avoid using any type in TypeScript. If unavoidable, use unknown with type guards and justify with a code comment
Prefer interface for defining public object shapes in TypeScript, use type for unions and utility types
Use TypeScript utility types such as Partial, Pick, Omit, Readonly, and Record when appropriate
Use I{Name} naming convention for interfaces in TypeScript
Use T{Name} naming convention for type aliases and utility types in TypeScript

**/*.{ts,tsx}: Prefer types over interfaces for most cases
Don't ever use any - type safety always
Avoid enums; use const objects instead
For complex types, create a separate file to declare them and import them
Avoid using any type; if unavoidable, use unknown with type guards and justify with code comment
Prefer interface for public API shapes; use type for unions and utility types
Use TypeScript utility types (Partial, Pick, Omit, Readonly, Record)

Files:

  • packages/bot/src/functions/music/commands/queue/queueEmbed.ts
  • packages/bot/src/handlers/player/trackHandlers.ts
  • packages/bot/src/utils/music/queueManipulation.spec.ts
  • packages/bot/src/handlers/player/index.ts
  • packages/bot/tests/utils/music/watchdog.test.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.spec.ts
  • packages/bot/src/utils/music/watchdog.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/documentation.mdc)

**/*.{js,ts,tsx,jsx}: Minimize comments in code; explain the 'why' when non-obvious, let code express the 'what' through clear naming
Document trade-offs briefly when deviating from ideal patterns

**/*.{js,ts,tsx,jsx}: Store secrets, ports, and hosts in environment variables (.env, .env.example) and never hardcode them
Avoid redundant or decorative AI comments; code should be self-explanatory and only commented when logic is non-obvious; prefer refactoring over lengthy comments

Files:

  • packages/bot/src/functions/music/commands/queue/queueEmbed.ts
  • packages/bot/src/handlers/player/trackHandlers.ts
  • packages/bot/src/utils/music/queueManipulation.spec.ts
  • packages/bot/src/handlers/player/index.ts
  • packages/bot/tests/utils/music/watchdog.test.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.spec.ts
  • packages/bot/src/utils/music/watchdog.ts
packages/bot/src/functions/*/commands/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)

packages/bot/src/functions/*/commands/**/*.{ts,tsx}: Command model must include data (slash builder), execute, and category properties exported from packages/bot/src/models/Command.ts
Use @discordjs/builders for building the data (SlashCommandBuilder) in command definitions
Command execute function must receive { interaction, client } parameters from CommandExecuteParams type
Use interactionReply and createUserFriendlyError utilities from @lucky/shared/general utils for command replies and error handling
Use existing validators from packages/bot/src/utils/command/ for voice channel, queue, and guild validations in commands

Files:

  • packages/bot/src/functions/music/commands/queue/queueEmbed.ts
packages/bot/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)

packages/bot/**/*.{ts,tsx}: Use useMainPlayer() from discord-player to access the player instance; do not instantiate player directly
Do not duplicate queue or player state outside Discord Player; use shared services from @lucky/shared for persistent data like track history and session information
Use errorLog and debugLog from @lucky/shared/utils for logging throughout the bot package
Use embed and reply utilities from @lucky/shared for consistent message formatting and error sanitization across the bot
Use services from @lucky/shared (DatabaseService, Redis client) for database and cache access; do not instantiate Prisma or Redis directly in the bot package

Files:

  • packages/bot/src/functions/music/commands/queue/queueEmbed.ts
  • packages/bot/src/handlers/player/trackHandlers.ts
  • packages/bot/src/utils/music/queueManipulation.spec.ts
  • packages/bot/src/handlers/player/index.ts
  • packages/bot/tests/utils/music/watchdog.test.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.spec.ts
  • packages/bot/src/utils/music/watchdog.ts
packages/bot/**

📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)

The bot package depends on shared and contains Discord bot commands and player handlers using Discord.js and Discord Player

Files:

  • packages/bot/src/functions/music/commands/queue/queueEmbed.ts
  • packages/bot/src/handlers/player/trackHandlers.ts
  • packages/bot/src/utils/music/queueManipulation.spec.ts
  • packages/bot/src/handlers/player/index.ts
  • packages/bot/tests/utils/music/watchdog.test.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.spec.ts
  • packages/bot/src/utils/music/watchdog.ts
**/*.{js,mjs,ts,mts}

📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)

Use Node.js version ≥22 with ESM (ECMAScript modules) only; no CommonJS

Files:

  • packages/bot/src/functions/music/commands/queue/queueEmbed.ts
  • packages/bot/src/handlers/player/trackHandlers.ts
  • packages/bot/src/utils/music/queueManipulation.spec.ts
  • packages/bot/src/handlers/player/index.ts
  • packages/bot/tests/utils/music/watchdog.test.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.spec.ts
  • packages/bot/src/utils/music/watchdog.ts
packages/bot/src/functions/{general,music,download}/commands/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)

packages/bot/src/functions/{general,music,download}/commands/**/*.ts: Apply .cursor/rules/lucky-discord-bot.mdc rules for Discord bot commands and player implementation
Use .cursor/skills/discord-commands/SKILL.md for implementing slash commands

Files:

  • packages/bot/src/functions/music/commands/queue/queueEmbed.ts
packages/bot/src/functions/music/commands/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)

Use .cursor/skills/music-queue-player/SKILL.md for play, queue, skip, volume commands and player lifecycle management

Files:

  • packages/bot/src/functions/music/commands/queue/queueEmbed.ts
packages/bot/src/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)

Use @lucky/shared for database, Redis, logging, and embed utilities instead of implementing them locally

Files:

  • packages/bot/src/functions/music/commands/queue/queueEmbed.ts
  • packages/bot/src/handlers/player/trackHandlers.ts
  • packages/bot/src/utils/music/queueManipulation.spec.ts
  • packages/bot/src/handlers/player/index.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.spec.ts
  • packages/bot/src/utils/music/watchdog.ts
packages/bot/src/functions/music/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)

Use existing voice/queue/guild validators before manipulating player or queue state

Files:

  • packages/bot/src/functions/music/commands/queue/queueEmbed.ts
packages/bot/src/handlers/player/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)

Track handling, errors, and lifecycle must be managed through dedicated handlers in packages/bot/src/handlers/player/ (trackHandlers, errorHandlers, lifecycleHandlers)

Files:

  • packages/bot/src/handlers/player/trackHandlers.ts
  • packages/bot/src/handlers/player/index.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)

**/*.{test,spec}.{js,jsx,ts,tsx}: Test behavior, not implementation. Prefer Testing Library utilities for testing React/React Native components.
For React Native tests: mock native modules and test component interactions and accessibility labels.

Files:

  • packages/bot/src/utils/music/queueManipulation.spec.ts
  • packages/bot/tests/utils/music/watchdog.test.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.spec.ts
**/*.{test,spec}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)

**/*.{test,spec}.{ts,tsx,js,jsx}: Test behavior, not implementation details
Prefer unit tests for core logic; add integration tests at meaningful boundaries

Files:

  • packages/bot/src/utils/music/queueManipulation.spec.ts
  • packages/bot/tests/utils/music/watchdog.test.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.spec.ts
**/*.{test,spec}.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/testing-quality.mdc)

**/*.{test,spec}.{js,ts,jsx,tsx}: Use Jest + a React testing library for unit and component tests as applicable
Test behavior, not implementation details

Files:

  • packages/bot/src/utils/music/queueManipulation.spec.ts
  • packages/bot/tests/utils/music/watchdog.test.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.spec.ts
**/*.{spec,test}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/quality.mdc)

**/*.{spec,test}.{ts,tsx,js,jsx}: Use Jest for unit and integration tests
Test behavior, not implementation details
Run unit, integration tests, and coverage report in CI quality checks

Files:

  • packages/bot/src/utils/music/queueManipulation.spec.ts
  • packages/bot/tests/utils/music/watchdog.test.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.spec.ts
**/*.spec.ts

📄 CodeRabbit inference engine (.cursor/rules/quality.mdc)

Unit tests must use naming convention *.spec.ts

Files:

  • packages/bot/src/utils/music/queueManipulation.spec.ts
  • packages/bot/src/handlers/player/lifecycleHandlers.spec.ts
  • packages/bot/src/utils/music/searchQueryCleaner.spec.ts
**/index.ts

📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)

Use index.ts only to re-export a small, intentional surface per module

Files:

  • packages/bot/src/handlers/player/index.ts
{packages/*/tests/**/*.test.{js,ts},tests/**/*.test.{js,ts}}

📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)

Add or adjust unit and integration tests when changing behavior; follow existing patterns in packages/*/tests and root tests/ directories

Files:

  • packages/bot/tests/utils/music/watchdog.test.ts
🔇 Additional comments (1)
packages/bot/src/utils/music/searchQueryCleaner.spec.ts (1)

184-238: Good coverage for hyphenated suffix stripping behavior.

This new block cleanly validates both strip and no-strip outcomes across en-dash, hyphen, and em-dash separators, which matches the intended normalization behavior.

As per coding guidelines, “Test behavior, not implementation details” and “Use Jest for unit and integration tests”.

Comment on lines +1392 to +1400
await replenishQueue(queue as any, {
targetQueueSize: 1,
guildId: 'guild-1',
})

// The candidate should be deduplicated by title, so no track is added
expect((queue as any).addTrack).not.toHaveBeenCalledWith(
expect.objectContaining({ title: 'Bohemian Rhapsody' }),
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify remaining `as any` usage in TS tests and inspect this dedup assertion pattern.
rg -nP --type=ts '\bas any\b|not\.toHaveBeenCalledWith\('

Repository: LucasSantana-Dev/Lucky

Length of output: 50379


🏁 Script executed:

# Get the full context of the test section mentioned in the review
sed -n '1365,1403p' packages/bot/src/utils/music/queueManipulation.spec.ts | cat -n

Repository: LucasSantana-Dev/Lucky

Length of output: 1548


🏁 Script executed:

# Check imports and GuildQueue type in the test file
head -50 packages/bot/src/utils/music/queueManipulation.spec.ts | cat -n

Repository: LucasSantana-Dev/Lucky

Length of output: 1386


🏁 Script executed:

# Search for createQueueMock definition in the test file
rg -n 'createQueueMock|function createQueue|const createQueue' packages/bot/src/utils/music/queueManipulation.spec.ts

Repository: LucasSantana-Dev/Lucky

Length of output: 1118


🏁 Script executed:

# Find GuildQueue type definition
rg -n 'type GuildQueue|interface GuildQueue|class GuildQueue' packages/bot/src/utils/music/

Repository: LucasSantana-Dev/Lucky

Length of output: 218


🏁 Script executed:

# Check createQueueMock implementation and QueueMock type
sed -n '81,125p' packages/bot/src/utils/music/queueManipulation.spec.ts | cat -n

Repository: LucasSantana-Dev/Lucky

Length of output: 1796


🏁 Script executed:

# Check the actual GuildQueue import in the source file
sed -n '1,30p' packages/bot/src/utils/music/queueManipulation.ts | cat -n

Repository: LucasSantana-Dev/Lucky

Length of output: 1338


🏁 Script executed:

# Check if GuildQueue is properly imported from discord-player type definitions
rg -A5 'import.*GuildQueue' packages/bot/src/utils/music/queueManipulation.ts

Repository: LucasSantana-Dev/Lucky

Length of output: 48


🏁 Script executed:

# Find the QueueMock type definition
rg -n 'type QueueMock|interface QueueMock' packages/bot/src/utils/music/queueManipulation.spec.ts -A10

Repository: LucasSantana-Dev/Lucky

Length of output: 450


🏁 Script executed:

# Let me check the entire context around the test section to understand queue mock setup and assertions pattern
sed -n '1365,1403p' packages/bot/src/utils/music/queueManipulation.spec.ts

Repository: LucasSantana-Dev/Lucky

Length of output: 1275


🏁 Script executed:

# Check the replenishQueue function signature to see what type it expects
rg -n 'function replenishQueue|export.*replenishQueue' packages/bot/src/utils/music/queueManipulation.ts -A3

Repository: LucasSantana-Dev/Lucky

Length of output: 226


🏁 Script executed:

# Check other assertions in the test file to see the pattern for addTrack mock
rg -n 'addTrack.*toHaveBeenCalled' packages/bot/src/utils/music/queueManipulation.spec.ts | head -20

Repository: LucasSantana-Dev/Lucky

Length of output: 1254


🏁 Script executed:

# Check if the queue can be passed without the `as any` cast
sed -n '1379,1395p' packages/bot/src/utils/music/queueManipulation.spec.ts | cat -n

Repository: LucasSantana-Dev/Lucky

Length of output: 657


🏁 Script executed:

# Check what type createQueueMock returns and how it's used elsewhere
rg -n 'const queue = createQueueMock' packages/bot/src/utils/music/queueManipulation.spec.ts -A2 | head -30

Repository: LucasSantana-Dev/Lucky

Length of output: 963


🏁 Script executed:

# Check if queue can be used directly without `as any` cast in the test file
# by looking at what the function signature expects vs what createQueueMock returns
sed -n '73,110p' packages/bot/src/utils/music/queueManipulation.spec.ts | cat -n

Repository: LucasSantana-Dev/Lucky

Length of output: 1412


🏁 Script executed:

# Check how other tests use replenishQueue and if they also use `as any`
rg -B2 'await replenishQueue' packages/bot/src/utils/music/queueManipulation.spec.ts | head -40

Repository: LucasSantana-Dev/Lucky

Length of output: 833


🏁 Script executed:

# Check the signature of replenishQueue more carefully
sed -n '190,210p' packages/bot/src/utils/music/queueManipulation.ts | cat -n

Repository: LucasSantana-Dev/Lucky

Length of output: 989


🏁 Script executed:

# Verify the actual assertion change is correct by looking at the test intent
# Check if other tests use `.not.toHaveBeenCalled()` when they expect NO calls
rg -n 'not\.toHaveBeenCalled\(\)' packages/bot/src/utils/music/queueManipulation.spec.ts

Repository: LucasSantana-Dev/Lucky

Length of output: 477


🏁 Script executed:

# Look at similar deduplication or no-call scenarios to see assertion pattern
sed -n '1320,1365p' packages/bot/src/utils/music/queueManipulation.spec.ts | cat -n

Repository: LucasSantana-Dev/Lucky

Length of output: 1886


🏁 Script executed:

# Check the entire test context to understand the scenario better
sed -n '1365,1410p' packages/bot/src/utils/music/queueManipulation.spec.ts

Repository: LucasSantana-Dev/Lucky

Length of output: 1578


🏁 Script executed:

# Check if replenishQueue has different signatures or overloads
rg -n 'export.*function replenishQueue|function replenishQueue' packages/bot/src/utils/music/queueManipulation.ts -A5

Repository: LucasSantana-Dev/Lucky

Length of output: 322


🏁 Script executed:

# Check the exact parameters the new test is passing
sed -n '1388,1403p' packages/bot/src/utils/music/queueManipulation.spec.ts

Repository: LucasSantana-Dev/Lucky

Length of output: 468


🏁 Script executed:

# Check if there are any function overloads for replenishQueue
rg -C5 'replenishQueue' packages/bot/src/utils/music/queueManipulation.ts | head -50

Repository: LucasSantana-Dev/Lucky

Length of output: 862


🏁 Script executed:

# Check how the test is actually written - the parameters seem wrong
git -C packages/bot log --oneline -5 -- src/utils/music/queueManipulation.spec.ts 2>/dev/null || echo "Git not available"

Repository: LucasSantana-Dev/Lucky

Length of output: 135


🏁 Script executed:

# Check the full replenishQueue implementation to see if it uses targetQueueSize parameter anywhere
sed -n '190,250p' packages/bot/src/utils/music/queueManipulation.ts | cat -n

Repository: LucasSantana-Dev/Lucky

Length of output: 2568


🏁 Script executed:

# Verify the actual function signature - check if there's an overload or if parameters are extra
cat packages/bot/src/utils/music/queueManipulation.ts | grep -A20 'export async function replenishQueue'

Repository: LucasSantana-Dev/Lucky

Length of output: 842


🏁 Script executed:

# Check the actual error or issue by looking at the test file more carefully
# The test is passing extra parameters - verify if this is intentional or a bug
grep -n 'replenishQueue' packages/bot/src/utils/music/queueManipulation.spec.ts | grep -v '//'

Repository: LucasSantana-Dev/Lucky

Length of output: 1564


🏁 Script executed:

# Verify the exact line 1392 in context to confirm the parameters issue
sed -n '1389,1403p' packages/bot/src/utils/music/queueManipulation.spec.ts | cat -n

Repository: LucasSantana-Dev/Lucky

Length of output: 553


🏁 Script executed:

# Check the full function signature one more time
rg -A30 'export async function replenishQueue' packages/bot/src/utils/music/queueManipulation.ts | head -35

Repository: LucasSantana-Dev/Lucky

Length of output: 1247


🏁 Script executed:

# Verify there are no function overloads by checking the entire file for replenishQueue
rg -n 'replenishQueue' packages/bot/src/utils/music/queueManipulation.ts

Repository: LucasSantana-Dev/Lucky

Length of output: 223


🏁 Script executed:

# Check if this test was recently added and if the extra parameters are a bug
git log -p packages/bot/src/utils/music/queueManipulation.spec.ts 2>/dev/null | grep -A15 'title-only deduplication' | head -30 || echo "Git history not available"

Repository: LucasSantana-Dev/Lucky

Length of output: 48


🏁 Script executed:

# Check if the extra parameters are being silently ignored or if they would cause a type error
# Let's verify the TypeScript compilation would catch this
cat packages/bot/src/utils/music/queueManipulation.ts | grep -A5 'export async function replenishQueue'

Repository: LucasSantana-Dev/Lucky

Length of output: 298


🏁 Script executed:

# Verify the exact signature once more and check for possible overloads we might have missed
ast-grep --pattern 'function replenishQueue' packages/bot/src/utils/music/queueManipulation.ts

Repository: LucasSantana-Dev/Lucky

Length of output: 9069


🏁 Script executed:

# Check the tsconfig to understand strictness
cat packages/bot/tsconfig.json 2>/dev/null | head -30 || echo "tsconfig not found"

Repository: LucasSantana-Dev/Lucky

Length of output: 841


🏁 Script executed:

# Final verification: check if the extra parameters are intentional or a mistake
# by looking at how replenishQueue uses its parameters and if there are any other versions
grep -B5 -A15 'const missingTracks = AUTOPLAY_BUFFER_SIZE' packages/bot/src/utils/music/queueManipulation.ts

Repository: LucasSantana-Dev/Lucky

Length of output: 845


Remove extra parameters from replenishQueue call; simplify the type cast and improve assertion strength.

Line 1392 incorrectly passes targetQueueSize and guildId parameters—replenishQueue accepts only a queue parameter. All other tests use the correct single-parameter signature. Additionally, the assertion at Lines 1398-1400 is too weak; change it to verify no tracks are added at all rather than checking for a specific title.

Proposed patch
-        await replenishQueue(queue as any, {
-            targetQueueSize: 1,
-            guildId: 'guild-1',
-        })
+        await replenishQueue(queue as unknown as GuildQueue)

         // The candidate should be deduplicated by title, so no track is added
-        expect((queue as any).addTrack).not.toHaveBeenCalledWith(
-            expect.objectContaining({ title: 'Bohemian Rhapsody' }),
-        )
+        expect(queue.addTrack).not.toHaveBeenCalled()

Per coding guidelines: avoid any type casts in TypeScript tests and ensure assertions test behavior robustly rather than allowing false positives.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await replenishQueue(queue as any, {
targetQueueSize: 1,
guildId: 'guild-1',
})
// The candidate should be deduplicated by title, so no track is added
expect((queue as any).addTrack).not.toHaveBeenCalledWith(
expect.objectContaining({ title: 'Bohemian Rhapsody' }),
)
await replenishQueue(queue as unknown as GuildQueue)
// The candidate should be deduplicated by title, so no track is added
expect(queue.addTrack).not.toHaveBeenCalled()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/bot/src/utils/music/queueManipulation.spec.ts` around lines 1392 -
1400, The test incorrectly calls replenishQueue with extra parameters and uses
an any cast and a weak assertion; change the call to replenishQueue(queue)
(remove targetQueueSize and guildId), eliminate the (queue as any) cast by using
the properly-typed test fixture or a minimal safe cast (e.g., queue as unknown
as Queue) and strengthen the assertion to verify no tracks were added at all by
asserting that queue.addTrack was not called
(expect(queue.addTrack).not.toHaveBeenCalled()) rather than checking for a
specific title.

Comment on lines 105 to +113
markIntentionalStop(guildId: string): void {
this.intentionalStops.add(guildId)
this.clear(guildId)
setTimeout(() => this.intentionalStops.delete(guildId), 5_000)
// Window must outlive the watchdog timeout so the flag is still set
// when any already-scheduled checkAndRecover fires.
setTimeout(
() => this.intentionalStops.delete(guildId),
this.timeoutMs + 10_000,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

The longer intentional-stop TTL can bleed into the next session.

markIntentionalStop() now keeps a guild flagged for timeoutMs + 10_000, but nothing in the new flow clears that flag when playback starts again. A stop followed by a new play inside that window will make finish/skip/disconnect treat the new queue as intentionally stopped and skip autoplay/snapshot/recovery. Please scope this to a session, or clear it on the next connection/player start.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/bot/src/utils/music/watchdog.ts` around lines 105 - 113,
markIntentionalStop currently sets a guild in intentionalStops for timeoutMs +
10_000 but nothing clears it when a new playback/session starts, causing new
sessions within that window to be treated as "intentional" stops; to fix, remove
the flag at the start of a new session by calling
this.intentionalStops.delete(guildId) from the code path that creates/starts a
player or establishes a new connection (e.g., inside the player/connection
initializer such as the function that joins voice or
startPlayback/createOrGetPlayer), and ensure markIntentionalStop still schedules
the timeout but the immediate delete on new session prevents bleed into
subsequent playbacks.

@sonarqubecloud

Copy link
Copy Markdown

@LucasSantana-Dev
LucasSantana-Dev merged commit 33f7be8 into main Apr 11, 2026
12 checks passed
@LucasSantana-Dev
LucasSantana-Dev deleted the fix/play-provider-speed-duplicate branch April 11, 2026 17:04
LucasSantana-Dev added a commit that referenced this pull request May 13, 2026
* fix(bot): spotify-first, no duplicate play msg, soundcloud last

* fix(bot): title-only dedup and hyphenated version suffix strip

* fix(bot): anchor hyphenated suffix regex to end-of-string

* fix(bot): /stop no longer reconnects via watchdog after queue delete

* fix(bot): detect manual voice kick and empty queue as intentional stop

* fix(bot): use string search for version suffix strip, guard queue embed

* test: add coverage for voice handlers and title deduplication

* test: add voice kick, soundcloud fallback, title dedup coverage

This branch was successfully deployed

1 active deployment
Preview — 50d0a6e6 Deployed Apr 11, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant