Repository navigation
fix(bot): harden /play stream bridge + upgrade now-playing embed - #501
Conversation
The SoundCloud bridge in the YouTubei extractor was the single point of
failure for every `/play` request. When a track was not on SoundCloud
(kpop, niche, indie) or the resolved YouTube title was too noisy to match
("GOLDEN ... [Download] by Best Songs"), playdl.search returned no
viable candidates, the bridge threw, discord-player emitted
NoResultError("Could not extract stream for this track") — but the
"Now Playing" embed had already been sent, so users saw the message and
heard silence.
Two-layer fix:
1. Query hygiene + fallback chain in playerFactory
- New shared searchQueryCleaner utility (cleanTitle, cleanAuthor,
cleanSearchQuery, isSpamChannel) with an expanded NOISE_PATTERNS
list that now strips `[Download]`, `(Official)`, `(Music Video)`,
`(HD)`, `(4K)`, `(Remastered YYYY)`, `(Extended Mix)`, pipe
separators, empty bracket pairs, "VEVO" and "- Topic" channel
suffixes, etc.
- queueManipulation imports the shared cleaner instead of keeping its
own NOISE_PATTERNS (one source of truth for all music search flows).
- playerFactory.createResilientStream replaces streamViaSoundCloud as
the YoutubeiExtractor createStream hook and runs a 3-stage fallback:
a) SoundCloud search with cleaned `${title} ${author}`
b) SoundCloud search with cleaned title only
c) Direct playdl.stream(track.url) against the source URL
Spam-channel uploads (Best Songs, NCS, etc.) skip stage (a)+(b)
entirely and go straight to (c) since they would never have matched
the real track on SoundCloud.
- findMatchingSoundCloudResult and parseDurationString are now
exported for direct testing.
2. Upgraded Now Playing embed (user-facing polish)
- New nowPlayingEmbed helper with three kinds: nowPlaying,
addedToQueue, playlistQueued.
- Detects source platform (Spotify / YouTube / SoundCloud / Apple
Music / Vimeo) from track.source or URL sniffing, applies the
platform's brand color, and labels it in a dedicated field.
- Shows track thumbnail, duration, source label, queue position (for
addedToQueue), requester tag + avatar in the footer, clickable
track URL on the title.
- play/index.ts now picks the right embed kind based on whether the
request landed in an empty queue (plays now) or an existing queue
(added at position N).
Tests
- 21 new tests for searchQueryCleaner covering noise patterns, feat/ft
prefixes, pipe collapse, empty-field tolerance, and the GOLDEN/HUNTR/X
regression case.
- 15 new tests for nowPlayingEmbed covering source detection, kind-
based headers, field composition, playlistQueued layout, empty-title
fallback, and Spotify brand color.
- play/index.spec.ts mock interaction now carries `tag` and
`displayAvatarURL` so the new embed helper can render from the test.
- Full bot suite: 1541 passing (+36), tsc clean, lint clean.
Fixes LUCKY-2J (NoResultError regression in Command.execute on /play).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
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 12 minutes and 47 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughRefactored music handling utilities by extracting search query cleaning into a dedicated module, introducing a new embed builder for play command responses with support for multiple response kinds, and replacing playerFactory's single SoundCloud bridge with a multi-stage fallback stream resolution system featuring cleaned-query SoundCloud attempts followed by direct YouTube fallback. Changes
Sequence Diagram(s)sequenceDiagram
participant Player
participant Factory as playerFactory
participant SC as SoundCloud API
participant YT as YouTube
Player->>Factory: createResilientStream(track)
Factory->>Factory: cleanTitle & cleanAuthor
Factory->>Factory: detectSpamChannel
rect rgba(255, 200, 100, 0.5)
note over Factory,SC: Stage 1: Cleaned title+author query
Factory->>SC: streamViaSoundCloud(cleaned query)
SC-->>Factory: stream (success) OR error
end
alt Success
Factory-->>Player: Readable stream
else Failed
rect rgba(255, 150, 150, 0.5)
note over Factory,SC: Stage 2: Cleaned title-only query
Factory->>SC: streamViaSoundCloud(title only)
SC-->>Factory: stream (success) OR error
end
alt Success
Factory-->>Player: Readable stream
else Failed or Spam Author
rect rgba(100, 150, 255, 0.5)
note over Factory,YT: Stage 3: Direct YouTube fallback
Factory->>YT: playdl.stream(track.url)
YT-->>Factory: stream OR error
end
alt Success
Factory-->>Player: Readable stream
else Failed
Factory-->>Player: Error (bridge exhausted)
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Three changes landing together to address the Sonar quality gate blockers on PR #501 plus a small user-facing fix surfaced during review: 1. Attach music control buttons to /play responses The button infrastructure (createMusicControlButtons, musicButtonHandler, interactionHandler routing) has always been fully wired, but /play never attached the row to its reply. Users only ever saw the buttons on /queue show. Passing components: [createMusicControlButtons(queue)] in the interactionReply for /play gives the user pause/skip/prev/ shuffle/loop directly from the "Now Playing" embed. 2. Add playerFactory.bridge.spec.ts to cover createResilientStream The new resilient stream fallback chain was untested — new-code coverage sat at 69.9% and the Sonar quality gate requires 80%. The new spec mocks play-dl, discord-player, and @discord-player/extractor at the module level and exercises every branch of createResilientStream plus the real exports of streamViaSoundCloud, findMatchingSoundCloudResult, and parseDurationString. 21 new tests, all green. 3. Bound the whitespace quantifiers in searchQueryCleaner regexes Sonar typescript:S5852 flagged 5 patterns using unbounded \s* as potentially ReDoS-prone. The patterns weren't actually exploitable (simple character classes on trusted internal metadata) but the rule is conservative. Swapped every \s* for \s{0,3} — still matches any realistic whitespace padding, bounded enough to satisfy the rule. Full bot suite: 1562 passing (+21 bridge), tsc clean, lint clean.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/bot/src/utils/music/nowPlayingEmbed.ts (1)
5-8: Prefer a lightweight shared export for this embed token/helper.Hardcoding the Lucky music color here creates another source of truth for bot embeds. A small dependency-light export from
@lucky/sharedwould keep formatting aligned without pulling the heavy barrel this comment is working around.As per coding guidelines: "Use embed and reply utilities from
@lucky/sharedfor consistent message formatting and error sanitization across the bot."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/utils/music/nowPlayingEmbed.ts` around lines 5 - 8, Replace the hardcoded LUCKY_MUSIC_COLOR constant with a lightweight shared export: add a small, dependency-light export (e.g., EMBED_COLORS.MUSIC or MUSIC_EMBED_COLOR) in the shared utilities package and import that symbol into nowPlayingEmbed.ts; update any references to use the imported symbol (replace LUCKY_MUSIC_COLOR) so embeds use the single source of truth while avoiding the heavy barrel import.
🤖 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/index.ts`:
- Around line 188-191: The current calculation of queuePosition uses the queue
length (queue.tracks.toArray()?.length) which can be wrong after
moveUserTrackToPriority() or blendAutoplayTracks(); instead find the actual
final index of the enqueued track inside queue.tracks (e.g., call
queue.tracks.toArray() and locate the track by its id or reference) and set
queuePosition = index + 1; update the code around queuePosition,
hadQueueBeforePlay and queue.tracks to compute position from the track's final
slot rather than the queue length.
In `@packages/bot/src/handlers/player/playerFactory.ts`:
- Around line 249-259: The current predicate in the results.find uses
queryNorm.includes(resultNorm) or vice versa, which lets short result names
match longer queries; update the predicate inside the playerFactory search (the
function using normalizeForMatch and variables
queryNorm/resultNorm/trackSec/durationInSec) to require that all non-empty
tokens of the cleaned query are present in the candidate string instead of
allowing resultNorm to be a substring of queryNorm; implement this by splitting
queryNorm into tokens, filtering empty tokens, and checking tokens.every(t =>
resultNorm.includes(t)) OR alternatively build a combined candidate string of
normalized result name plus normalized author and require tokens to be present
there; keep the existing duration tolerance check (Math.abs(result.durationInSec
- trackSec) <= 15) unchanged.
In `@packages/bot/src/utils/music/nowPlayingEmbed.ts`:
- Around line 132-136: The playlist branch currently sets
embed.setURL(track.url) which links the playlist title to the individual track;
update the logic in the kind === 'playlistQueued' branch (where playlist and
embed are used) to only set the embed URL from playlist.url if it exists (e.g.,
if (playlist.url) embed.setURL(playlist.url)); do not use track.url here and
omit setting any URL when no playlist.url is provided.
---
Nitpick comments:
In `@packages/bot/src/utils/music/nowPlayingEmbed.ts`:
- Around line 5-8: Replace the hardcoded LUCKY_MUSIC_COLOR constant with a
lightweight shared export: add a small, dependency-light export (e.g.,
EMBED_COLORS.MUSIC or MUSIC_EMBED_COLOR) in the shared utilities package and
import that symbol into nowPlayingEmbed.ts; update any references to use the
imported symbol (replace LUCKY_MUSIC_COLOR) so embeds use the single source of
truth while avoiding the heavy barrel import.
🪄 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: 240a37c5-cc92-4256-b8fe-daf35247b739
📒 Files selected for processing (8)
packages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.tspackages/bot/src/handlers/player/playerFactory.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/utils/music/searchQueryCleaner.spec.tspackages/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: Quality Gates
- GitHub Check: SonarCloud Scan
🧰 Additional context used
📓 Path-based instructions (20)
**/*.{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.tspackages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.tspackages/bot/src/utils/music/searchQueryCleaner.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.tspackages/bot/src/handlers/player/playerFactory.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
**/*.{ts,tsx,js,jsx}: Use Prettier with no semicolons, single quotes, 4-space indent, 80 character width
Files must not exceed 250 lines and this is enforcedImplement 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...
Files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.tspackages/bot/src/utils/music/searchQueryCleaner.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.tspackages/bot/src/handlers/player/playerFactory.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{js,jsx,ts,tsx}: Never throw strings. ThrowError(or typed subclasses) with descriptive messages
Include causal error ascausewhen 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)
MarkretryablevsnonRetryableerrors 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, andaria-*attributes as needed.
React Native accessibility: use accessibility props (accessible,accessibilityLabel), proper roles and labels.
Identify and extract repetitive UI components proactively tocomponents/with clear props and minimal coupling.
Web styles: prefer co-located styles or design system tokens; avoid global style leakage.
React Native styles: preferStyleSheet.create, design tokens, and theme providers; avoid in...
Files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.tspackages/bot/src/utils/music/searchQueryCleaner.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.tspackages/bot/src/handlers/player/playerFactory.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
Introduce interfaces at module boundaries to enable testing and substitutions
**/*.{ts,tsx}: Avoid usinganytype in TypeScript. If unavoidable, useunknownwith type guards and justify with a code comment
Preferinterfacefor defining public object shapes in TypeScript, usetypefor unions and utility types
Use TypeScript utility types such asPartial,Pick,Omit,Readonly, andRecordwhen appropriate
UseI{Name}naming convention for interfaces in TypeScript
UseT{Name}naming convention for type aliases and utility types in TypeScript
**/*.{ts,tsx}: Functions must be less than 50 lines with cyclomatic complexity less than 10
Do not useanytypes - ESLint enforces this at error level
**/*.{ts,tsx}: Prefer types over interfaces for most cases
Don't ever useany- type safety always
Avoid enums; use const objects instead
For complex types, create a separate file to declare them and import them
Avoid usinganytype; if unavoidable, useunknownwith type guards and justify with code comment
Preferinterfacefor public API shapes; usetypefor unions and utility types
Use TypeScript utility types (Partial, Pick, Omit, Readonly, Record)
Files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.tspackages/bot/src/utils/music/searchQueryCleaner.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.tspackages/bot/src/handlers/player/playerFactory.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 commentsNever hardcode secrets, IPs, or ports; use
.envanddocs/for required configuration variables
Files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.tspackages/bot/src/utils/music/searchQueryCleaner.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.tspackages/bot/src/handlers/player/playerFactory.ts
packages/bot/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)
packages/bot/**/*.{ts,tsx}: UseuseMainPlayer()fromdiscord-playerto access the player instance; do not instantiate player directly
Do not duplicate queue or player state outside Discord Player; use shared services from@lucky/sharedfor persistent data like track history and session information
UseerrorLoganddebugLogfrom@lucky/shared/utilsfor logging throughout the bot package
Use embed and reply utilities from@lucky/sharedfor 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.tspackages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.tspackages/bot/src/utils/music/searchQueryCleaner.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.tspackages/bot/src/handlers/player/playerFactory.ts
packages/bot/**
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
The
botpackage depends onsharedand contains Discord bot commands and player handlers using Discord.js and Discord Player
Files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.tspackages/bot/src/utils/music/searchQueryCleaner.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.tspackages/bot/src/handlers/player/playerFactory.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.tspackages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.tspackages/bot/src/utils/music/searchQueryCleaner.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.tspackages/bot/src/handlers/player/playerFactory.ts
packages/bot/src/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)
Use
@lucky/sharedfor database, Redis, logging, and embed utilities instead of implementing them locally
Files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.tspackages/bot/src/utils/music/searchQueryCleaner.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.tspackages/bot/src/handlers/player/playerFactory.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/functions/music/commands/play/index.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/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/functions/music/commands/play/index.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/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/functions/music/commands/play/index.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.spec.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 includedata(slash builder),execute, andcategoryproperties exported frompackages/bot/src/models/Command.ts
Use@discordjs/buildersfor building thedata(SlashCommandBuilder) in command definitions
Commandexecutefunction must receive{ interaction, client }parameters fromCommandExecuteParamstype
UseinteractionReplyandcreateUserFriendlyErrorutilities from@lucky/shared/generalutils for command replies and error handling
Use existing validators frompackages/bot/src/utils/command/for voice channel, queue, and guild validations in commands
Files:
packages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.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/functions/music/commands/play/index.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/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/functions/music/commands/play/index.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.spec.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.mdcrules for Discord bot commands and player implementation
Use.cursor/skills/discord-commands/SKILL.mdfor implementing slash commands
Files:
packages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.ts
packages/bot/src/functions/music/commands/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)
Use
.cursor/skills/music-queue-player/SKILL.mdfor play, queue, skip, volume commands and player lifecycle management
Files:
packages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.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.spec.tspackages/bot/src/functions/music/commands/play/index.ts
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
Use
index.tsonly to re-export a small, intentional surface per module
Files:
packages/bot/src/functions/music/commands/play/index.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/playerFactory.ts
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/**/*.{ts,tsx} : Use embed and reply utilities from `lucky/shared` for consistent message formatting and error sanitization across the bot
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-15T21:57:49.951Z
Learning: When working on music queue/player features (play/queue/skip/volume, player lifecycle), use the `music-queue-player` skill
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-discord.mdc:0-0
Timestamp: 2026-03-09T20:21:52.065Z
Learning: Applies to packages/bot/src/functions/music/commands/**/*.ts : Use `.cursor/skills/music-queue-player/SKILL.md` for play, queue, skip, volume commands and player lifecycle management
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-discord.mdc:0-0
Timestamp: 2026-03-09T20:21:52.065Z
Learning: Applies to packages/bot/src/functions/music/**/*.ts : Use existing voice/queue/guild validators before manipulating player or queue state
📚 Learning: 2026-03-09T20:21:52.065Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-discord.mdc:0-0
Timestamp: 2026-03-09T20:21:52.065Z
Learning: Applies to packages/bot/src/functions/music/commands/**/*.ts : Use `.cursor/skills/music-queue-player/SKILL.md` for play, queue, skip, volume commands and player lifecycle management
Applied to files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.tspackages/bot/src/utils/music/searchQueryCleaner.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.tspackages/bot/src/handlers/player/playerFactory.ts
📚 Learning: 2026-03-09T20:21:52.065Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-discord.mdc:0-0
Timestamp: 2026-03-09T20:21:52.065Z
Learning: Applies to packages/bot/src/functions/music/**/*.ts : Use existing voice/queue/guild validators before manipulating player or queue state
Applied to files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.tspackages/bot/src/utils/music/searchQueryCleaner.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.tspackages/bot/src/handlers/player/playerFactory.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/**/*.{ts,tsx} : 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
Applied to files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/nowPlayingEmbed.tspackages/bot/src/handlers/player/playerFactory.ts
📚 Learning: 2026-03-15T21:57:49.951Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-15T21:57:49.951Z
Learning: When working on music queue/player features (play/queue/skip/volume, player lifecycle), use the `music-queue-player` skill
Applied to files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/functions/music/commands/play/index.ts
📚 Learning: 2026-03-09T20:21:52.065Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-discord.mdc:0-0
Timestamp: 2026-03-09T20:21:52.065Z
Learning: Applies to packages/bot/src/functions/{general,music,download}/commands/**/*.ts : Apply `.cursor/rules/lucky-discord-bot.mdc` rules for Discord bot commands and player implementation
Applied to files:
packages/bot/src/utils/music/queueManipulation.tspackages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/searchQueryCleaner.tspackages/bot/src/utils/music/nowPlayingEmbed.ts
📚 Learning: 2026-03-09T20:21:08.612Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-project.mdc:0-0
Timestamp: 2026-03-09T20:21:08.612Z
Learning: Applies to {packages/*/tests/**/*.test.{js,ts},tests/**/*.test.{js,ts}} : Add or adjust unit and integration tests when changing behavior; follow existing patterns in `packages/*/tests` and root `tests/` directories
Applied to files:
packages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.spec.ts
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to **/*.{spec,test}.{ts,tsx,js,jsx} : Test behavior, not implementation details
Applied to files:
packages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.spec.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/src/functions/*/commands/**/*.{ts,tsx} : Command `execute` function must receive `{ interaction, client }` parameters from `CommandExecuteParams` type
Applied to files:
packages/bot/src/functions/music/commands/play/index.spec.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/**/*.{ts,tsx} : Use `useMainPlayer()` from `discord-player` to access the player instance; do not instantiate player directly
Applied to files:
packages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/nowPlayingEmbed.tspackages/bot/src/handlers/player/playerFactory.ts
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to tests/**/*.test.{ts,tsx,js,jsx} : Add integration tests where appropriate
Applied to files:
packages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.spec.ts
📚 Learning: 2026-03-09T20:21:38.098Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-backend.mdc:0-0
Timestamp: 2026-03-09T20:21:38.098Z
Learning: Applies to packages/backend/tests/**/*.ts : Follow existing patterns for unit and integration tests in `packages/backend/tests/`
Applied to files:
packages/bot/src/functions/music/commands/play/index.spec.tspackages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.spec.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/src/functions/*/commands/**/*.{ts,tsx} : Use `interactionReply` and `createUserFriendlyError` utilities from `lucky/shared/general` utils for command replies and error handling
Applied to files:
packages/bot/src/functions/music/commands/play/index.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/**/*.{ts,tsx} : Use `errorLog` and `debugLog` from `lucky/shared/utils` for logging throughout the bot package
Applied to files:
packages/bot/src/functions/music/commands/play/index.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/**/*.{ts,tsx} : Use embed and reply utilities from `lucky/shared` for consistent message formatting and error sanitization across the bot
Applied to files:
packages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/nowPlayingEmbed.ts
📚 Learning: 2026-03-09T20:21:52.065Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/subagent-discord.mdc:0-0
Timestamp: 2026-03-09T20:21:52.065Z
Learning: Applies to packages/bot/src/**/*.ts : Use `lucky/shared` for database, Redis, logging, and embed utilities instead of implementing them locally
Applied to files:
packages/bot/src/functions/music/commands/play/index.ts
📚 Learning: 2026-03-09T20:21:15.595Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-shared.mdc:0-0
Timestamp: 2026-03-09T20:21:15.595Z
Learning: Applies to packages/shared/**/*.ts : Organize the Lucky Shared Package with the following directory structure: Config in `packages/shared/src/config/` (environment, constants, feature toggles, YouTube config); Services in `packages/shared/src/services/` (DatabaseService, Redis client/operations, FeatureToggleService, ReactionRoles, RoleManagement); Types in `packages/shared/src/types/` (errors, commands, common, discord, music); Utils in `packages/shared/src/utils/` (error handling, retry, embeds, log, monitoring, composables, prismaClient)
Applied to files:
packages/bot/src/functions/music/commands/play/index.ts
📚 Learning: 2026-03-09T20:21:08.612Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-project.mdc:0-0
Timestamp: 2026-03-09T20:21:08.612Z
Learning: Applies to packages/bot/** : The `bot` package depends on `shared` and contains Discord bot commands and player handlers using Discord.js and Discord Player
Applied to files:
packages/bot/src/functions/music/commands/play/index.tspackages/bot/src/utils/music/nowPlayingEmbed.tspackages/bot/src/handlers/player/playerFactory.ts
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to **/*.{spec,test}.{ts,tsx,js,jsx} : Use Jest for unit and integration tests
Applied to files:
packages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.spec.ts
📚 Learning: 2026-03-09T20:20:56.356Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-frontend.mdc:0-0
Timestamp: 2026-03-09T20:20:56.356Z
Learning: Applies to packages/frontend/tests/**/*.{ts,tsx,js} : Write tests in `packages/frontend/tests/` using existing test patterns (e.g., Playwright for e2e if configured)
Applied to files:
packages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.spec.ts
📚 Learning: 2026-03-09T20:20:38.694Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-backend-api.mdc:0-0
Timestamp: 2026-03-09T20:20:38.694Z
Learning: Applies to packages/backend/tests/**/*.{ts,tsx} : Organize tests in `packages/backend/tests/` with unit tests under `unit/` and integration tests under `integration/`, following existing patterns with fixtures and setup
Applied to files:
packages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.spec.ts
📚 Learning: 2026-03-09T20:21:31.459Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/quality.mdc:0-0
Timestamp: 2026-03-09T20:21:31.459Z
Learning: Applies to **/*.{spec,test}.{ts,tsx,js,jsx} : Run unit, integration tests, and coverage report in CI quality checks
Applied to files:
packages/bot/src/utils/music/nowPlayingEmbed.spec.tspackages/bot/src/utils/music/searchQueryCleaner.spec.ts
📚 Learning: 2026-03-09T20:20:47.877Z
Learnt from: CR
Repo: LucasSantana-Dev/Lucky PR: 0
File: .cursor/rules/lucky-discord-bot.mdc:0-0
Timestamp: 2026-03-09T20:20:47.877Z
Learning: Applies to packages/bot/src/handlers/player/**/*.{ts,tsx} : Track handling, errors, and lifecycle must be managed through dedicated handlers in `packages/bot/src/handlers/player/` (trackHandlers, errorHandlers, lifecycleHandlers)
Applied to files:
packages/bot/src/handlers/player/playerFactory.ts
Three correctness fixes raised in the CodeRabbit review of the play stream bridge work: 1. queuePosition computed from the track's real index in the queue instead of the post-mutation queue length. moveUserTrackToPriority and blendAutoplayTracks can rearrange the queue between the search result resolving and the reply being sent, so the raw length is not the user-facing position of the added track. Now locates the track in queue.tracks.toArray() by identity/id and falls back to the length only if the track cannot be found. 2. findMatchingSoundCloudResult now requires every non-empty token of the cleaned query to appear in the candidate name instead of a symmetric substring check. The old predicate allowed a short remix title to satisfy a long query just because the short name was a substring of the query — that's not a real match. Tokenizing on whitespace and requiring tokens.every(t => resultNorm.includes(t)) keeps the tolerance for word reordering but blocks the short-string hijack. 3. buildPlayResponseEmbed playlist branch no longer links the playlist title to a single track URL. The playlist context now carries an optional url field and the embed sets it only when the playlist itself has a URL, leaving the title unlinked otherwise rather than misleading users into clicking through to an arbitrary individual track. play/index.ts forwards result.searchResult.playlist.url into the context accordingly. Full bot suite: 1562 passing, tsc clean, lint clean.
|
CodeRabbit feedback addressed in 463c318:
Also bundled in from the previous push (5b05e93):
Full bot suite: 1562 passing, tsc + lint clean. |
|
* fix(bot): harden /play stream bridge and upgrade now-playing embed
The SoundCloud bridge in the YouTubei extractor was the single point of
failure for every `/play` request. When a track was not on SoundCloud
(kpop, niche, indie) or the resolved YouTube title was too noisy to match
("GOLDEN ... [Download] by Best Songs"), playdl.search returned no
viable candidates, the bridge threw, discord-player emitted
NoResultError("Could not extract stream for this track") — but the
"Now Playing" embed had already been sent, so users saw the message and
heard silence.
Two-layer fix:
1. Query hygiene + fallback chain in playerFactory
- New shared searchQueryCleaner utility (cleanTitle, cleanAuthor,
cleanSearchQuery, isSpamChannel) with an expanded NOISE_PATTERNS
list that now strips `[Download]`, `(Official)`, `(Music Video)`,
`(HD)`, `(4K)`, `(Remastered YYYY)`, `(Extended Mix)`, pipe
separators, empty bracket pairs, "VEVO" and "- Topic" channel
suffixes, etc.
- queueManipulation imports the shared cleaner instead of keeping its
own NOISE_PATTERNS (one source of truth for all music search flows).
- playerFactory.createResilientStream replaces streamViaSoundCloud as
the YoutubeiExtractor createStream hook and runs a 3-stage fallback:
a) SoundCloud search with cleaned `${title} ${author}`
b) SoundCloud search with cleaned title only
c) Direct playdl.stream(track.url) against the source URL
Spam-channel uploads (Best Songs, NCS, etc.) skip stage (a)+(b)
entirely and go straight to (c) since they would never have matched
the real track on SoundCloud.
- findMatchingSoundCloudResult and parseDurationString are now
exported for direct testing.
2. Upgraded Now Playing embed (user-facing polish)
- New nowPlayingEmbed helper with three kinds: nowPlaying,
addedToQueue, playlistQueued.
- Detects source platform (Spotify / YouTube / SoundCloud / Apple
Music / Vimeo) from track.source or URL sniffing, applies the
platform's brand color, and labels it in a dedicated field.
- Shows track thumbnail, duration, source label, queue position (for
addedToQueue), requester tag + avatar in the footer, clickable
track URL on the title.
- play/index.ts now picks the right embed kind based on whether the
request landed in an empty queue (plays now) or an existing queue
(added at position N).
Tests
- 21 new tests for searchQueryCleaner covering noise patterns, feat/ft
prefixes, pipe collapse, empty-field tolerance, and the GOLDEN/HUNTR/X
regression case.
- 15 new tests for nowPlayingEmbed covering source detection, kind-
based headers, field composition, playlistQueued layout, empty-title
fallback, and Spotify brand color.
- play/index.spec.ts mock interaction now carries `tag` and
`displayAvatarURL` so the new embed helper can render from the test.
- Full bot suite: 1541 passing (+36), tsc clean, lint clean.
Fixes LUCKY-2J (NoResultError regression in Command.execute on /play).
* fix(bot): attach music controls + bridge coverage + regex bounds
Three changes landing together to address the Sonar quality gate blockers
on PR #501 plus a small user-facing fix surfaced during review:
1. Attach music control buttons to /play responses
The button infrastructure (createMusicControlButtons, musicButtonHandler,
interactionHandler routing) has always been fully wired, but /play never
attached the row to its reply. Users only ever saw the buttons on
/queue show. Passing components: [createMusicControlButtons(queue)]
in the interactionReply for /play gives the user pause/skip/prev/
shuffle/loop directly from the "Now Playing" embed.
2. Add playerFactory.bridge.spec.ts to cover createResilientStream
The new resilient stream fallback chain was untested — new-code coverage
sat at 69.9% and the Sonar quality gate requires 80%. The new spec
mocks play-dl, discord-player, and @discord-player/extractor at the
module level and exercises every branch of createResilientStream plus
the real exports of streamViaSoundCloud, findMatchingSoundCloudResult,
and parseDurationString. 21 new tests, all green.
3. Bound the whitespace quantifiers in searchQueryCleaner regexes
Sonar typescript:S5852 flagged 5 patterns using unbounded \s* as
potentially ReDoS-prone. The patterns weren't actually exploitable
(simple character classes on trusted internal metadata) but the rule
is conservative. Swapped every \s* for \s{0,3} — still matches any
realistic whitespace padding, bounded enough to satisfy the rule.
Full bot suite: 1562 passing (+21 bridge), tsc clean, lint clean.
* fix(bot): address coderabbit review on pr #501
Three correctness fixes raised in the CodeRabbit review of the play
stream bridge work:
1. queuePosition computed from the track's real index in the queue
instead of the post-mutation queue length. moveUserTrackToPriority
and blendAutoplayTracks can rearrange the queue between the search
result resolving and the reply being sent, so the raw length is not
the user-facing position of the added track. Now locates the track
in queue.tracks.toArray() by identity/id and falls back to the
length only if the track cannot be found.
2. findMatchingSoundCloudResult now requires every non-empty token of
the cleaned query to appear in the candidate name instead of a
symmetric substring check. The old predicate allowed a short remix
title to satisfy a long query just because the short name was a
substring of the query — that's not a real match. Tokenizing on
whitespace and requiring tokens.every(t => resultNorm.includes(t))
keeps the tolerance for word reordering but blocks the
short-string hijack.
3. buildPlayResponseEmbed playlist branch no longer links the playlist
title to a single track URL. The playlist context now carries an
optional url field and the embed sets it only when the playlist
itself has a URL, leaving the title unlinked otherwise rather than
misleading users into clicking through to an arbitrary individual
track. play/index.ts forwards result.searchResult.playlist.url into
the context accordingly.
Full bot suite: 1562 passing, tsc clean, lint clean.
* fix(bot): harden /play stream bridge and upgrade now-playing embed
The SoundCloud bridge in the YouTubei extractor was the single point of
failure for every `/play` request. When a track was not on SoundCloud
(kpop, niche, indie) or the resolved YouTube title was too noisy to match
("GOLDEN ... [Download] by Best Songs"), playdl.search returned no
viable candidates, the bridge threw, discord-player emitted
NoResultError("Could not extract stream for this track") — but the
"Now Playing" embed had already been sent, so users saw the message and
heard silence.
Two-layer fix:
1. Query hygiene + fallback chain in playerFactory
- New shared searchQueryCleaner utility (cleanTitle, cleanAuthor,
cleanSearchQuery, isSpamChannel) with an expanded NOISE_PATTERNS
list that now strips `[Download]`, `(Official)`, `(Music Video)`,
`(HD)`, `(4K)`, `(Remastered YYYY)`, `(Extended Mix)`, pipe
separators, empty bracket pairs, "VEVO" and "- Topic" channel
suffixes, etc.
- queueManipulation imports the shared cleaner instead of keeping its
own NOISE_PATTERNS (one source of truth for all music search flows).
- playerFactory.createResilientStream replaces streamViaSoundCloud as
the YoutubeiExtractor createStream hook and runs a 3-stage fallback:
a) SoundCloud search with cleaned `${title} ${author}`
b) SoundCloud search with cleaned title only
c) Direct playdl.stream(track.url) against the source URL
Spam-channel uploads (Best Songs, NCS, etc.) skip stage (a)+(b)
entirely and go straight to (c) since they would never have matched
the real track on SoundCloud.
- findMatchingSoundCloudResult and parseDurationString are now
exported for direct testing.
2. Upgraded Now Playing embed (user-facing polish)
- New nowPlayingEmbed helper with three kinds: nowPlaying,
addedToQueue, playlistQueued.
- Detects source platform (Spotify / YouTube / SoundCloud / Apple
Music / Vimeo) from track.source or URL sniffing, applies the
platform's brand color, and labels it in a dedicated field.
- Shows track thumbnail, duration, source label, queue position (for
addedToQueue), requester tag + avatar in the footer, clickable
track URL on the title.
- play/index.ts now picks the right embed kind based on whether the
request landed in an empty queue (plays now) or an existing queue
(added at position N).
Tests
- 21 new tests for searchQueryCleaner covering noise patterns, feat/ft
prefixes, pipe collapse, empty-field tolerance, and the GOLDEN/HUNTR/X
regression case.
- 15 new tests for nowPlayingEmbed covering source detection, kind-
based headers, field composition, playlistQueued layout, empty-title
fallback, and Spotify brand color.
- play/index.spec.ts mock interaction now carries `tag` and
`displayAvatarURL` so the new embed helper can render from the test.
- Full bot suite: 1541 passing (+36), tsc clean, lint clean.
Fixes LUCKY-2J (NoResultError regression in Command.execute on /play).
* fix(bot): attach music controls + bridge coverage + regex bounds
Three changes landing together to address the Sonar quality gate blockers
on PR #501 plus a small user-facing fix surfaced during review:
1. Attach music control buttons to /play responses
The button infrastructure (createMusicControlButtons, musicButtonHandler,
interactionHandler routing) has always been fully wired, but /play never
attached the row to its reply. Users only ever saw the buttons on
/queue show. Passing components: [createMusicControlButtons(queue)]
in the interactionReply for /play gives the user pause/skip/prev/
shuffle/loop directly from the "Now Playing" embed.
2. Add playerFactory.bridge.spec.ts to cover createResilientStream
The new resilient stream fallback chain was untested — new-code coverage
sat at 69.9% and the Sonar quality gate requires 80%. The new spec
mocks play-dl, discord-player, and @discord-player/extractor at the
module level and exercises every branch of createResilientStream plus
the real exports of streamViaSoundCloud, findMatchingSoundCloudResult,
and parseDurationString. 21 new tests, all green.
3. Bound the whitespace quantifiers in searchQueryCleaner regexes
Sonar typescript:S5852 flagged 5 patterns using unbounded \s* as
potentially ReDoS-prone. The patterns weren't actually exploitable
(simple character classes on trusted internal metadata) but the rule
is conservative. Swapped every \s* for \s{0,3} — still matches any
realistic whitespace padding, bounded enough to satisfy the rule.
Full bot suite: 1562 passing (+21 bridge), tsc clean, lint clean.
* fix(bot): address coderabbit review on pr #501
Three correctness fixes raised in the CodeRabbit review of the play
stream bridge work:
1. queuePosition computed from the track's real index in the queue
instead of the post-mutation queue length. moveUserTrackToPriority
and blendAutoplayTracks can rearrange the queue between the search
result resolving and the reply being sent, so the raw length is not
the user-facing position of the added track. Now locates the track
in queue.tracks.toArray() by identity/id and falls back to the
length only if the track cannot be found.
2. findMatchingSoundCloudResult now requires every non-empty token of
the cleaned query to appear in the candidate name instead of a
symmetric substring check. The old predicate allowed a short remix
title to satisfy a long query just because the short name was a
substring of the query — that's not a real match. Tokenizing on
whitespace and requiring tokens.every(t => resultNorm.includes(t))
keeps the tolerance for word reordering but blocks the
short-string hijack.
3. buildPlayResponseEmbed playlist branch no longer links the playlist
title to a single track URL. The playlist context now carries an
optional url field and the embed sets it only when the playlist
itself has a URL, leaving the title unlinked otherwise rather than
misleading users into clicking through to an arbitrary individual
track. play/index.ts forwards result.searchResult.playlist.url into
the context accordingly.
Full bot suite: 1562 passing, tsc clean, lint clean.
This branch was successfully deployed
1 participant
Summary
Production fix for LUCKY-2J: `/play` was silently broken for any track the SoundCloud bridge couldn't match (kpop, niche, indie, or spam-uploader YouTube results like "GOLDEN ... [Download] by Best Songs"). The embed said "Now Playing" but audio never started, and Sentry only captured a single `NoResultError` per failure (noise suppression after the 'regressed' flag made this hard to spot).
Root cause: `playerFactory.streamViaSoundCloud` was the single point of failure — every extracted stream went through a SoundCloud lookup that fuzzy-matched on `\`${title} ${author}\``. If that noisy query returned nothing, the bridge threw and there was no fallback, so `discord-player` emitted `NoResultError("Could not extract stream for this track")` while the "Now Playing" embed had already been sent.
Changes
1. Bridge hardening (playerFactory)
2. Shared search query cleaner
3. Upgraded Now Playing embed
Addresses the "too simple and not good looking" feedback. New `buildPlayResponseEmbed` helper:
Test plan
Fixes LUCKY-2J.
Summary by CodeRabbit
New Features
Bug Fixes