From 4ddada978e5a786f537d1a2e26b63bb7cc18e506 Mon Sep 17 00:00:00 2001 From: htilly Date: Tue, 27 Jan 2026 18:23:30 +0100 Subject: [PATCH 1/2] refactor: Consolidate modules into lib/, add handlers, improve tests - Move all root JS modules to lib/ folder (cleaner structure) - Rename spotify-async.js to lib/spotify.js - Extract add/addalbum/addplaylist/append handlers to lib/add-handlers.js - Add Spotify null guards in search functions - Replace .map() with .forEach() for side effects - Bump posthog-node to 5.24.2 - Add integration tests for addalbum and addplaylist with queue size verification - Fix integration test to detect threaded bot responses - Add rate-limit handling and reduce polling frequency in test runner - Add test retry with 10s backoff on failure --- .github/AI_PR_SETUP.md | 144 +++ .github/agent/generate-implementation.mjs | 184 ++++ .github/workflows/feature-request-enhance.yml | 136 ++- index.js | 921 +----------------- lib/add-handlers.js | 703 +++++++++++++ ai-handler.js => lib/ai-handler.js | 0 lib/command-handlers.js | 18 +- discord.js => lib/discord.js | 0 logger.js => lib/logger.js | 0 music-helper.js => lib/music-helper.js | 0 slack.js => lib/slack.js | 0 .../soundcraft-handler.js | 0 spotify-async.js => lib/spotify.js | 0 telemetry.js => lib/telemetry.js | 0 utils.js => lib/utils.js | 0 voting.js => lib/voting.js | 0 package-lock.json | 16 +- package.json | 2 +- test/add-handlers.test.mjs | 556 +++++++++++ test/ai-handler.test.mjs | 2 +- test/tools/integration-test-suite.mjs | 304 +++++- test/voting.test.mjs | 16 +- 22 files changed, 2076 insertions(+), 926 deletions(-) create mode 100644 .github/AI_PR_SETUP.md create mode 100644 .github/agent/generate-implementation.mjs create mode 100644 lib/add-handlers.js rename ai-handler.js => lib/ai-handler.js (100%) rename discord.js => lib/discord.js (100%) rename logger.js => lib/logger.js (100%) rename music-helper.js => lib/music-helper.js (100%) rename slack.js => lib/slack.js (100%) rename soundcraft-handler.js => lib/soundcraft-handler.js (100%) rename spotify-async.js => lib/spotify.js (100%) rename telemetry.js => lib/telemetry.js (100%) rename utils.js => lib/utils.js (100%) rename voting.js => lib/voting.js (100%) create mode 100644 test/add-handlers.test.mjs diff --git a/.github/AI_PR_SETUP.md b/.github/AI_PR_SETUP.md new file mode 100644 index 00000000..f428e252 --- /dev/null +++ b/.github/AI_PR_SETUP.md @@ -0,0 +1,144 @@ +# AI-Generated Pull Request Setup + +This guide describes how to configure automatic PR creation with AI-generated code suggestions for feature requests. + +## Overview + +When an issue is created with the `enhancement` label, the workflow will: + +1. **Enhance the issue description** - Convert to structured user story format +2. **Create Confluence page** (optional) - Document requirements +3. **Generate implementation with Claude AI** - Analyze codebase and create implementation plan +4. **Create automatic PR** - Commit implementation plan and create PR against `develop` branch + +## Configuration + +### 1. GitHub Secrets + +You need to configure the following secrets in your GitHub repository: + +**Settings → Secrets and variables → Actions → New repository secret** + +#### Required for AI-PR: +- `ANTHROPIC_API_KEY` - Your Anthropic API key for Claude + - Get from: https://console.anthropic.com/ + - Format: `sk-ant-api03-...` + +#### Optional (for enhanced features): +- `OPENAI_API_KEY` - For preprocessing with OpenAI +- `PREPROCESSING_MODEL` - Optional, default: `gpt-4o-mini` +- `CLAUDE_MODEL` - Optional, default: `claude-sonnet-4-5-20250929` + +#### Optional (for Confluence integration): +- `CONFLUENCE_URL` - Your Confluence URL (e.g. `https://your-domain.atlassian.net`) +- `CONFLUENCE_EMAIL` - Your Confluence account email +- `CONFLUENCE_API_TOKEN` - Confluence API token +- `CONFLUENCE_SPACE_KEY` - Space key where pages should be created (default: `AICODE`) +- `CONFLUENCE_PARENT_PAGE_ID` - Parent page ID (optional) + +### 2. GitHub CLI (gh) Access + +The workflow uses `gh` CLI to create PRs. This works automatically with `GITHUB_TOKEN` provided by GitHub Actions. + +### 3. Repository Permissions + +Verify that GitHub Actions has the correct permissions: + +**Settings → Actions → General → Workflow permissions** +- Select: "Read and write permissions" +- Enable: "Allow GitHub Actions to create and approve pull requests" + +## Usage + +### Create a Feature Request + +1. Go to **Issues → New Issue** +2. Write your feature request +3. Add the **`enhancement`** label +4. Create the issue + +### What Happens Automatically + +The workflow will: + +1. ✅ Enhance issue description with structured user story +2. 📄 Create a Confluence page (if configured) +3. 🤖 Use Claude AI to analyze the codebase +4. 📝 Generate an implementation plan +5. 🌿 Create a new branch: `feature/issue-{number}-implementation` +6. 📤 Commit the file `implementation-{number}.md` +7. 🔀 Create a PR against `develop` branch +8. 💬 Comment on original issue with PR link + +### PR Content + +The PR will contain: +- A markdown file with Claude's implementation plan +- Analysis of existing code +- Suggestions for which files need changes +- Concrete code examples and instructions + +### Next Steps After PR Creation + +1. **Review the implementation plan** in the PR +2. **Add actual code changes** based on the plan +3. **Test the implementation** +4. **Merge when ready** + +## Manual Trigger + +You can also trigger the workflow manually: + +1. Go to **Actions → Enhance Feature Requests** +2. Click **Run workflow** +3. Enter issue number +4. Click **Run workflow** + +## Troubleshooting + +### Workflow Doesn't Run + +- Verify that issue has the `enhancement` label +- Check that workflow file exists in main/master branch +- Review Actions log for error messages + +### PR Not Created + +- Verify that `ANTHROPIC_API_KEY` is configured +- Check that repository has correct permissions (see step 3 above) +- Verify that `gh` CLI works in workflow log + +### Low Quality Implementation + +- Try using `claude-opus-4.5` instead: set secret `CLAUDE_MODEL=claude-opus-4.5-20251101` +- Ensure issue description is clear and detailed +- Add more context in [generate-implementation.mjs](.github/agent/generate-implementation.mjs) + +## Cost + +- **Anthropic Claude API**: ~$0.50-2.00 per feature request (depending on model) +- **OpenAI** (optional): ~$0.01-0.10 per preprocessing +- **GitHub Actions**: Free for public repos, included in private repo plans + +## Customize the Implementation Generator + +Edit [.github/agent/generate-implementation.mjs](.github/agent/generate-implementation.mjs) to: + +- Add more project files for context +- Modify system prompt for better output +- Customize how implementation is presented + +## Branch Strategy + +**Important**: All PRs are automatically created against the `develop` branch. This is configured in the workflow at line 354: + +```yaml +--base develop \ +``` + +If you need to change the target branch, modify this line in [.github/workflows/feature-request-enhance.yml](.github/workflows/feature-request-enhance.yml). + +## Support + +- GitHub Issues: https://github.com/htilly/SlackONOS/issues +- Claude API Docs: https://docs.anthropic.com/ diff --git a/.github/agent/generate-implementation.mjs b/.github/agent/generate-implementation.mjs new file mode 100644 index 00000000..3da0a6c2 --- /dev/null +++ b/.github/agent/generate-implementation.mjs @@ -0,0 +1,184 @@ +import Anthropic from "@anthropic-ai/sdk"; +import { readFileSync, writeFileSync, existsSync } from "fs"; +import { resolve, dirname } from "path"; +import { fileURLToPath } from "url"; + +const __dirname = dirname(fileURLToPath(import.meta.url)); + +/** + * Generate Implementation - Use Claude to analyze codebase and generate implementation + * + * This script uses Claude to: + * 1. Analyze the existing codebase + * 2. Generate an implementation plan + * 3. Create code changes for the feature request + */ + +const enhancedTask = process.env.ENHANCED_TASK || process.env.TASK || ""; +const issueNumber = process.env.ISSUE_NUMBER || "unknown"; +const anthropicApiKey = process.env.ANTHROPIC_API_KEY; +const model = process.env.CLAUDE_MODEL || "claude-sonnet-4-5-20250929"; + +if (!enhancedTask) { + console.error("[IMPLEMENTATION] ENHANCED_TASK or TASK environment variable not set"); + process.exit(1); +} + +if (!anthropicApiKey) { + console.error("[IMPLEMENTATION] ANTHROPIC_API_KEY not set"); + process.exit(1); +} + +const anthropic = new Anthropic({ apiKey: anthropicApiKey }); + +/** + * Read relevant project files to give Claude context + */ +function getProjectContext() { + const repoRoot = resolve(__dirname, "../.."); + const files = []; + + // Read package.json for dependencies + try { + const packageJson = readFileSync(resolve(repoRoot, "package.json"), "utf8"); + files.push({ + path: "package.json", + content: packageJson + }); + } catch (e) { + console.warn("[IMPLEMENTATION] Could not read package.json"); + } + + // Read main index file + try { + const indexJs = readFileSync(resolve(repoRoot, "index.js"), "utf8"); + files.push({ + path: "index.js", + content: indexJs.substring(0, 5000) // First 5000 chars to avoid token limits + }); + } catch (e) { + console.warn("[IMPLEMENTATION] Could not read index.js"); + } + + // Read lib directory structure + try { + const libFiles = [ + "lib/slack.js", + "lib/discord.js", + "lib/voting.js", + "lib/command-handlers.js", + "lib/ai-handler.js", + "lib/spotify.js" + ]; + + for (const libFile of libFiles) { + const fullPath = resolve(repoRoot, libFile); + if (existsSync(fullPath)) { + const content = readFileSync(fullPath, "utf8"); + files.push({ + path: libFile, + content: content.substring(0, 3000) // First 3000 chars per file + }); + } + } + } catch (e) { + console.warn("[IMPLEMENTATION] Could not read lib files"); + } + + return files; +} + +/** + * Generate implementation using Claude + */ +async function generateImplementation() { + try { + console.log(`[IMPLEMENTATION] Generating implementation with ${model}...`); + console.log(`[IMPLEMENTATION] Feature request: ${enhancedTask}`); + + const projectFiles = getProjectContext(); + + // Build context from project files + let contextText = "Here are relevant files from the project:\n\n"; + for (const file of projectFiles) { + contextText += `--- ${file.path} ---\n${file.content}\n\n`; + } + + const systemPrompt = `You are an expert software developer working on a Slack/Discord bot for controlling Sonos speakers. + +The project is called SlackONOS and is a democratic bot where users can vote on songs to play. + +Your task is to analyze the feature request and generate a concrete implementation plan with code suggestions. + +Project context: +- Node.js application +- Uses Slack RTM API and Discord.js +- Controls Sonos speakers +- Has voting system for democratic music control +- Uses AI for natural language commands +- Supports Spotify integration + +Output format: +1. **Implementation Plan** - Brief overview of what needs to be changed +2. **Files to Modify/Create** - List specific files +3. **Code Changes** - Provide actual code snippets or full file contents + +Be specific and actionable. Focus on the actual code changes needed.`; + + const userPrompt = `Feature Request to Implement: + +${enhancedTask} + +${contextText} + +Please provide: +1. A brief implementation plan +2. List of files to modify or create +3. Actual code changes with clear instructions + +Make it actionable and ready to commit.`; + + const response = await anthropic.messages.create({ + model: model, + max_tokens: 4096, + messages: [ + { + role: "user", + content: [ + { + type: "text", + text: `${systemPrompt}\n\n${userPrompt}` + } + ] + } + ] + }); + + const implementation = response.content[0].text; + + console.log(`[IMPLEMENTATION] Generated implementation plan:`); + console.log(implementation); + + // Save implementation to file for the workflow to use + const outputPath = resolve(__dirname, `../../implementation-${issueNumber}.md`); + writeFileSync(outputPath, implementation, "utf8"); + console.log(`[IMPLEMENTATION] Saved to ${outputPath}`); + + // Output markers for workflow parsing + console.log(`\nIMPLEMENTATION_FILE:${outputPath}`); + console.log(`IMPLEMENTATION_START`); + console.log(implementation); + console.log(`IMPLEMENTATION_END`); + + return implementation; + + } catch (error) { + console.error(`[IMPLEMENTATION] Error generating implementation: ${error.message}`); + if (error.response) { + console.error(`[IMPLEMENTATION] API Error: ${JSON.stringify(error.response.data)}`); + } + process.exit(1); + } +} + +generateImplementation(); diff --git a/.github/workflows/feature-request-enhance.yml b/.github/workflows/feature-request-enhance.yml index bf7dc11b..56a7cd67 100644 --- a/.github/workflows/feature-request-enhance.yml +++ b/.github/workflows/feature-request-enhance.yml @@ -12,7 +12,8 @@ on: permissions: issues: write - contents: read + contents: write + pull-requests: write jobs: check-label: @@ -247,3 +248,136 @@ jobs: -H "Content-Type: application/json" \ -d @- \ "https://api.github.com/repos/${{ github.repository }}/issues/$ISSUE_NUMBER/comments" + + generate-implementation: + runs-on: ubuntu-latest + needs: [check-label, preprocess] + if: needs.preprocess.outcome == 'success' + permissions: + contents: write + pull-requests: write + issues: write + steps: + - name: Checkout develop + uses: actions/checkout@v4 + with: + ref: develop + fetch-depth: 0 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: 20 + cache: npm + + - name: Install agent dependencies + working-directory: .github/agent + run: npm install + + - name: Generate implementation with Claude + id: generate + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + CLAUDE_MODEL: ${{ secrets.CLAUDE_MODEL }} + ENHANCED_TASK: ${{ needs.preprocess.outputs.enhanced_task }} + TASK: ${{ needs.preprocess.outputs.issue_title }} + ISSUE_NUMBER: ${{ needs.check-label.outputs.issue_number }} + run: | + set -e + OUTPUT=$(node .github/agent/generate-implementation.mjs 2>&1) + echo "$OUTPUT" + + # Extract implementation file path + IMPL_FILE=$(echo "$OUTPUT" | grep "IMPLEMENTATION_FILE:" | sed 's/IMPLEMENTATION_FILE://' | tr -d '[:space:]') + echo "implementation_file=$IMPL_FILE" >> $GITHUB_OUTPUT + + - name: Create implementation branch and PR + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + ISSUE_NUMBER: ${{ needs.check-label.outputs.issue_number }} + IMPL_FILE: ${{ steps.generate.outputs.implementation_file }} + run: | + set -e + + # Configure git + git config user.name "github-actions[bot]" + git config user.email "github-actions[bot]@users.noreply.github.com" + + # Create feature branch + BRANCH_NAME="feature/issue-$ISSUE_NUMBER-implementation" + git checkout -b "$BRANCH_NAME" + + # Add the implementation file + if [ -f "$IMPL_FILE" ]; then + git add "$IMPL_FILE" + + # Create commit with heredoc to handle multiline + git commit -m "$(cat <<'EOF' + Add implementation plan for issue #$ISSUE_NUMBER + + This is an AI-generated implementation plan created by Claude. + Please review and adapt as needed. + + Related issue: #$ISSUE_NUMBER + EOF + )" + + # Push branch + git push -u origin "$BRANCH_NAME" + + # Create PR body using heredoc + PR_BODY=$(cat <<'EOF' + ## 🤖 AI-Generated Implementation Plan + + This PR contains an implementation plan generated by Claude AI for issue #$ISSUE_NUMBER. + + ### 📋 Implementation Plan + See [implementation-$ISSUE_NUMBER.md](./implementation-$ISSUE_NUMBER.md) for details. + + ### ⚠️ Important + This is an **AI-generated proposal**. Please: + - Review the implementation plan carefully + - Adapt and modify as needed + - Add actual code changes based on the plan + - Test thoroughly before merging + + Closes #$ISSUE_NUMBER + EOF + ) + + # Substitute environment variables in PR body + PR_BODY=$(echo "$PR_BODY" | sed "s/\$ISSUE_NUMBER/$ISSUE_NUMBER/g") + + # Create PR + gh pr create \ + --title "feat: Implementation plan for issue #$ISSUE_NUMBER" \ + --body "$PR_BODY" \ + --base develop \ + --head "$BRANCH_NAME" \ + --label "ai-generated,enhancement" + + echo "✅ Created PR for branch $BRANCH_NAME" + else + echo "❌ Implementation file not found: $IMPL_FILE" + exit 1 + fi + + - name: Comment on issue with PR link + if: success() + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + ISSUE_NUMBER: ${{ needs.check-label.outputs.issue_number }} + run: | + COMMENT="🚀 **Implementation PR Created!** + + An AI-generated implementation plan has been created. Check the pull request for details. + + Note: This is a starting point - please review and adapt the implementation as needed." + + jq -n --arg body "$COMMENT" '{body:$body}' \ + | curl -X POST \ + -H "Authorization: token $GITHUB_TOKEN" \ + -H "Accept: application/vnd.github+json" \ + -H "Content-Type: application/json" \ + -d @- \ + "https://api.github.com/repos/${{ github.repository }}/issues/$ISSUE_NUMBER/comments" diff --git a/index.js b/index.js index d28fbb00..1f99bfdb 100644 --- a/index.js +++ b/index.js @@ -35,17 +35,18 @@ const path = require('path'); const googleTTS = require('@sefinek/google-tts-api'); const config = require('nconf'); const winston = require('winston'); -const Spotify = require('./spotify-async'); -const utils = require('./utils'); +const Spotify = require('./lib/spotify'); +const utils = require('./lib/utils'); const process = require('process'); const parseString = require('xml2js').parseString; const http = require('http'); const https = require('https'); const selfsigned = require('selfsigned'); -const AIHandler = require('./ai-handler'); -const voting = require('./voting'); -const musicHelper = require('./music-helper'); +const AIHandler = require('./lib/ai-handler'); +const voting = require('./lib/voting'); +const musicHelper = require('./lib/music-helper'); const commandHandlers = require('./lib/command-handlers'); +const addHandlers = require('./lib/add-handlers'); const gongMessage = fs.readFileSync('templates/messages/gong.txt', 'utf8').split('\n').filter(Boolean); const voteMessage = fs.readFileSync('templates/messages/vote.txt', 'utf8').split('\n').filter(Boolean); const ttsMessage = fs.readFileSync('templates/messages/tts.txt', 'utf8').split('\n').filter(Boolean); @@ -100,8 +101,8 @@ const userActionsFile = path.join(__dirname, 'config/userActions.json'); const blacklistFile = path.join(__dirname, 'config/blacklist.json'); const trackBlacklistFile = path.join(__dirname, 'config/track-blacklist.json'); const aiUnparsedFile = path.join(__dirname, 'config/ai-unparsed.log'); -const WinstonWrapper = require('./logger'); -const Telemetry = require('./telemetry'); +const WinstonWrapper = require('./lib/logger'); +const Telemetry = require('./lib/telemetry'); // Helper to load user blacklist function loadBlacklist() { @@ -476,7 +477,7 @@ const spotify = Spotify({ }, logger); /* Initialize Soundcraft Handler */ -const SoundcraftHandler = require('./soundcraft-handler'); +const SoundcraftHandler = require('./lib/soundcraft-handler'); // Parse soundcraftChannels if it's a string (from config file) let soundcraftChannels = config.get('soundcraftChannels') || []; @@ -510,8 +511,8 @@ if (config.get('soundcraftEnabled')) { /* Initialize Music Helper with blacklist checker */ musicHelper.initialize(spotify, logger, isTrackBlacklisted); -const SlackSystem = require('./slack'); -const DiscordSystem = require('./discord'); +const SlackSystem = require('./lib/slack'); +const DiscordSystem = require('./lib/discord'); // Command router stub - will be properly defined after commandRegistry // This allows us to pass it to Slack/Discord initialization @@ -962,6 +963,20 @@ try { soundcraft: soundcraft, }); + // Initialize Add Handlers + addHandlers.initialize({ + logger: logger, + sonos: sonos, + spotify: spotify, + sendMessage: (msg, ch, opts) => _slackMessage(msg, ch, opts), + logUserAction: _logUserAction, + isTrackBlacklisted: isTrackBlacklisted, + musicHelper: musicHelper, + getConfig: () => config, + getAdminChannel: () => global.adminChannel, + getCurrentPlatform: () => currentPlatform, + }); + // Check that at least one platform is configured if (!hasSlack && !hasDiscord) { throw new Error('No platform configured! Provide either Slack tokens (slackAppToken + token) or Discord token (discordToken). Visit /setup to configure.'); @@ -2883,9 +2898,9 @@ function normalizeUser(userString) { const commandRegistry = new Map([ // Common commands - ['add', { fn: _add, admin: false }], - ['addalbum', { fn: _addalbum, admin: false }], - ['addplaylist', { fn: _addplaylist, admin: false }], + ['add', { fn: addHandlers.add, admin: false }], + ['addalbum', { fn: addHandlers.addalbum, admin: false }], + ['addplaylist', { fn: addHandlers.addplaylist, admin: false }], ['search', { fn: commandHandlers.search, admin: false }], ['searchalbum', { fn: (args, ch, u) => commandHandlers.searchalbum(args, ch), admin: false }], ['searchplaylist', { fn: commandHandlers.searchplaylist, admin: false }], @@ -2905,7 +2920,7 @@ const commandRegistry = new Map([ ['status', { fn: (args, ch, u) => _status(ch), admin: false }], ['help', { fn: (args, ch, u) => _help(args, ch, u), admin: false }], ['bestof', { fn: _bestof, admin: false }], - ['append', { fn: _append, admin: false }], + ['append', { fn: addHandlers.append, admin: false }], // Admin-only commands ['debug', { fn: (args, ch, u) => _debug(ch, u), admin: true }], @@ -4085,7 +4100,7 @@ async function _debug(channel, userName) { // Try to get client ID from Discord module let clientId = 'unknown'; try { - const discordModule = require('./discord'); + const discordModule = require('./lib/discord'); const discordClient = discordModule.getDiscordClient(); if (discordClient && discordClient.user) { clientId = discordClient.user.id; @@ -4186,805 +4201,9 @@ async function _telemetryStatus(channel) { } } -// This function needs to be a little smarter -async function _add(input, channel, userName) { - _logUserAction(userName, 'add'); - // Add a track to the queue - // If stopped: flush queue and start fresh - // If playing: just add to existing queue - if (!input || input.length < 2) { - _slackMessage('🎵 You gotta tell me what to add! Use `add ` 🎶', channel); - return; - } - const track = input.slice(1).join(' '); - logger.info('Track to add: ' + track); - - try { - const tracks = await spotify.searchTrackList(track, 3); // Reduced from 7 to 3 for faster validation - if (!tracks || tracks.length === 0) { - _slackMessage("🤷 Couldn't find anything matching that. Try different keywords or check the spelling! 🎵", channel); - return; - } - - // Sort tracks by relevance using the same logic as search command - const sortedTracks = _sortTracksByRelevance(tracks, track); - - // Pre-validate all candidates in parallel before attempting to queue - const candidates = sortedTracks - .filter(t => musicHelper.isValidSpotifyUri(t.uri)) - .map(t => ({ - name: t.name, - artist: t.artist, - uri: t.uri - })); - - if (candidates.length === 0) { - _slackMessage("🤷 Found tracks but they have invalid format. Try a different search! 🎵", channel); - return; - } - - // Check if first result is blacklisted (most common case) - const firstCandidate = candidates[0]; - if (isTrackBlacklisted(firstCandidate.name, firstCandidate.artist)) { - logger.info(`Track blocked by blacklist: ${firstCandidate.name} by ${firstCandidate.artist}`); - _slackMessage(`🚫 Sorry, *${firstCandidate.name}* by ${firstCandidate.artist} is on the blacklist and cannot be added.`, channel); - return; - } - - // Parallelize getting state and queue to save time - const [state, queue] = await Promise.all([ - sonos.getCurrentState(), - sonos.getQueue().catch(err => { - logger.warn('Could not get queue: ' + err.message); - return null; - }) - ]); - logger.info('Current state for add: ' + state); - - // Handle stopped state - flush queue - if (state === 'stopped') { - logger.info('Player stopped - ensuring queue is active source and flushing'); - try { - // Stop any active playback to force Sonos to use queue - try { - await sonos.stop(); - await new Promise(resolve => setTimeout(resolve, 300)); - } catch (stopErr) { - // Ignore stop errors (might already be stopped) - logger.debug('Stop command result (may already be stopped): ' + stopErr.message); - } - - // Flush queue to start fresh - await sonos.flush(); - await new Promise(resolve => setTimeout(resolve, 300)); - logger.info('Queue flushed and ready'); - } catch (flushErr) { - logger.warn('Could not flush queue: ' + flushErr.message); - } - } else if (queue && queue.items) { - // Check for duplicates if playing (using pre-fetched queue) - // Use findIndex directly instead of .some() then .findIndex() - avoids double scan - const duplicatePosition = queue.items.findIndex(item => - item.uri === firstCandidate.uri || - (item.title === firstCandidate.name && item.artist === firstCandidate.artist) - ); - - if (duplicatePosition >= 0) { - _slackMessage( - `*${firstCandidate.name}* by _${firstCandidate.artist}_ is already in the queue at position #${duplicatePosition}! :musical_note:\nWant it to play sooner? Use \`vote ${duplicatePosition}\` to move it up! :arrow_up:`, - channel - ); - return; - } - } - - // Try to queue the first valid candidate (most relevant result) - let result = null; - try { - logger.info(`Attempting to queue: ${firstCandidate.name} by ${firstCandidate.artist} (URI: ${firstCandidate.uri})`); - await sonos.queue(firstCandidate.uri); - logger.info('Successfully queued track: ' + firstCandidate.name); - result = firstCandidate; - } catch (e) { - const errorDetails = e.message || String(e); - const upnpErrorMatch = errorDetails.match(/errorCode[>](\d+)[<]/); - const errorCode = upnpErrorMatch ? upnpErrorMatch[1] : null; - - logger.warn(`Queue failed for "${firstCandidate.name}" by ${firstCandidate.artist}: ${errorDetails}${errorCode ? ` (error code: ${errorCode})` : ''}`); - - if (errorCode === '800') { - _slackMessage('🤷 Track not available in your region. Try searching for different songs! 🎵', channel); - - // Also notify admin channel about region configuration - if (global.adminChannel && channel !== global.adminChannel) { - const currentMarket = config.get('market') || 'US'; - const marketOptions = ['US', 'SE', 'GB', 'DE', 'FR', 'CA', 'AU', 'JP', 'NO', 'DK', 'FI']; - const marketOptionsList = marketOptions.map(m => m === currentMarket ? `*${m}* (current)` : m).join(', '); - - _slackMessage( - `⚠️ *Spotify Region Warning*\n` + - `Track "*${firstCandidate.name}*" by ${firstCandidate.artist} failed due to region availability.\n\n` + - `Please verify your Spotify region configuration.\n` + - `Current region: *${currentMarket}*\n` + - `Available options: ${marketOptionsList}\n` + - `Update via setup wizard or admin panel.`, - global.adminChannel - ); - } - } else { - _slackMessage('🤷 Couldn\'t add the track. It may not be available or there was an error. Try a different search! 🎵', channel); - } - return; - } - - // Respond immediately to user (don't wait for playback to start) - _slackMessage( - 'Added ' + '*' + result.name + '*' + ' by ' + result.artist + ' to the queue! :notes:', - channel, - { trackName: result.name, addReactions: currentPlatform === 'discord' } - ); - - // Handle playback state asynchronously (don't block user response) - if (state === 'stopped') { - // Start playback in background (don't await) - (async () => { - try { - // Ensure queue is the active source before starting playback - // Stop any active playback to force Sonos to use queue - try { - await sonos.stop(); - await new Promise(resolve => setTimeout(resolve, 500)); - } catch (stopErr) { - // Ignore stop errors (might already be stopped) - logger.debug('Stop before play (may already be stopped): ' + stopErr.message); - } - - // Verify queue has items before trying to play (prevents UPnP error 701) - let queueReady = false; - let retries = 0; - while (!queueReady && retries < 5) { - try { - const queue = await sonos.getQueue(); - if (queue && queue.items && queue.items.length > 0) { - queueReady = true; - logger.debug(`Queue verified: ${queue.items.length} items ready`); - } else { - logger.debug(`Queue not ready yet (attempt ${retries + 1}/5), waiting...`); - await new Promise(resolve => setTimeout(resolve, 300)); - retries++; - } - } catch (queueErr) { - logger.debug(`Queue check failed (attempt ${retries + 1}/5): ${queueErr.message}`); - await new Promise(resolve => setTimeout(resolve, 300)); - retries++; - } - } - - if (!queueReady) { - logger.warn('Queue not ready after 5 attempts, attempting playback anyway'); - } - - // Try to activate queue by seeking to position 1 (alternative to SetAVTransportURI) - // This should activate the queue as the transport source - try { - logger.debug('Attempting to seek to queue position 1 to activate queue'); - // Seek to track 1 in the queue to activate it - await sonos.avTransportService().Seek({ - InstanceID: 0, - Unit: 'TRACK_NR', - Target: '1' - }); - logger.debug('Successfully sought to track 1, queue should be active'); - // Wait for seek to complete - await new Promise(resolve => setTimeout(resolve, 500)); - } catch (seekErr) { - // If seek fails, try alternative: use next() to jump to first track - logger.debug('Seek failed, trying next() to activate queue: ' + seekErr.message); - try { - // Jump to first track in queue (this should activate the queue) - await sonos.next(); - logger.debug('Used next() to activate queue'); - await new Promise(resolve => setTimeout(resolve, 300)); - } catch (nextErr) { - logger.debug('next() also failed: ' + nextErr.message); - // Continue anyway - play() might still work - } - } - - // Wait a moment to ensure queue is ready - await new Promise(resolve => setTimeout(resolve, 500)); - - // Start playback from queue - await sonos.play(); - logger.info('Started playback from queue'); - } catch (playErr) { - logger.warn('Failed to start playback: ' + playErr.message); - } - })(); - } else if (state === 'paused') { - // Resume playback if paused - (async () => { - try { - await sonos.play(); - logger.info('Resumed playback'); - } catch (playErr) { - logger.warn('Failed to resume playback: ' + playErr.message); - } - })(); - } - } catch (err) { - logger.error('Error adding track: ' + err.message); - _slackMessage('🤷 Couldn\'t find that track or hit an error adding it. Try being more specific with the song name! 🎵', channel); - } -} - -async function _addalbum(input, channel, userName) { - _logUserAction(userName, 'addalbum'); - // Add an album to the queue, support Spotify URI or search - if (!input || input.length < 2) { - _slackMessage('💿 You gotta tell me which album to add! Try `addalbum ` 🎶', channel); - return; - } - const album = input.slice(1).join(' '); - logger.info('Album to add: ' + album); - - try { - // If it's a Spotify URI, use getAlbum directly - if (album.startsWith('spotify:album:') || album.includes('spotify.com/album/')) { - const result = await spotify.getAlbum(album); - await _queueAlbum(result, album, channel, userName); - return; - } - - // Otherwise search and sort by relevance - const albums = await spotify.searchAlbumList(album, 3); - if (!albums || albums.length === 0) { - _slackMessage("🤷 Couldn't find that album. Try including the artist name or checking the spelling! 🎶", channel); - return; - } - - // Sort by relevance and take first result - const sortedAlbums = _sortAlbumsByRelevance(albums, album); - const result = { ...sortedAlbums[0], uri: sortedAlbums[0].uri }; - logger.info(`Selected album: ${result.name} by ${result.artist}`); - - await _queueAlbum(result, album, channel, userName); - } catch (err) { - logger.error('Error adding album: ' + err.message); - _slackMessage('🔎 Couldn\'t find that album. Try a Spotify link, or use `searchalbum ` to pick one. 🎵', channel); - } -} - -// Helper function to queue an album (shared between URI and search flows) -async function _queueAlbum(result, albumSearchTerm, channel, userName) { - try { - // Check for blacklisted tracks in the album - const albumTracks = await spotify.getAlbumTracks(result.uri); - const blacklistedTracks = albumTracks.filter(track => - isTrackBlacklisted(track.name, track.artist) - ); - - // If ALL tracks are blacklisted, don't add anything - if (blacklistedTracks.length > 0 && blacklistedTracks.length === albumTracks.length) { - _slackMessage( - `🚫 Cannot add album *${result.name}* - all ${albumTracks.length} tracks are blacklisted!`, - channel - ); - return; - } - - let warningMessage = ''; - if (blacklistedTracks.length > 0) { - const bannedList = blacklistedTracks.map(t => `*${t.name}*`).join(', '); - warningMessage = `\n⚠️ Skipped ${blacklistedTracks.length} blacklisted track(s): ${bannedList}`; - logger.info(`Filtering out ${blacklistedTracks.length} blacklisted tracks from album ${result.name}`); - } - - // Get current player state - const state = await sonos.getCurrentState(); - logger.info('Current state for addalbum: ' + state); - - const isStopped = state === 'stopped'; - - // If stopped, ensure queue is active source and flush - if (isStopped) { - logger.info('Player stopped - ensuring queue is active and flushing'); - try { - // Stop any active playback to force Sonos to use queue - try { - await sonos.stop(); - await new Promise(resolve => setTimeout(resolve, 300)); - } catch (stopErr) { - // Ignore stop errors (might already be stopped) - logger.debug('Stop command result (may already be stopped): ' + stopErr.message); - } - - // Flush queue to start fresh - await sonos.flush(); - await new Promise(resolve => setTimeout(resolve, 300)); - logger.info('Queue flushed and ready'); - } catch (flushErr) { - logger.warn('Could not flush queue: ' + flushErr.message); - } - } - - // Respond to user immediately - const trackCountText = blacklistedTracks.length > 0 - ? `${albumTracks.length - blacklistedTracks.length} tracks from album` - : 'album'; - let text = `Added ${trackCountText} *${result.name}* by ${result.artist} to the queue! :notes:`; - text += warningMessage; - - if (result.coverUrl) { - _slackMessage(text, channel, { - trackName: result.name, // Track album name for reactions - blocks: [ - { - type: "section", - text: { - type: "mrkdwn", - text: text - }, - accessory: { - type: "image", - image_url: result.coverUrl, - alt_text: result.name + " cover" - } - } - ] - }); - } else { - _slackMessage(text, channel, { trackName: result.name }); // Track album name for reactions - } - - // Queue tracks in background (don't block user response) - (async () => { - try { - // If we have blacklisted tracks, add individually in parallel; otherwise use album URI - if (blacklistedTracks.length > 0) { - const allowedTracks = albumTracks.filter(track => - !isTrackBlacklisted(track.name, track.artist) - ); - - // Queue all tracks in parallel (much faster!) - const queuePromises = allowedTracks.map(track => - sonos.queue(track.uri).catch(err => { - logger.warn(`Could not queue track ${track.name}: ${err.message}`); - return null; - }) - ); - - await Promise.allSettled(queuePromises); - logger.info(`Added ${allowedTracks.length} tracks from album (filtered ${blacklistedTracks.length})`); - } else { - // No blacklisted tracks, add entire album via URI (more efficient) - await sonos.queue(result.uri); - logger.info('Added album: ' + result.name); - } - - // Start playback if needed - if (isStopped) { - await new Promise(resolve => setTimeout(resolve, 300)); // Reduced from 1000ms - await sonos.play(); - logger.info('Started playback after album add'); - } else if (state !== 'playing' && state !== 'transitioning') { - await sonos.play(); - logger.info('Player was not playing, started playback.'); - } - } catch (err) { - logger.error('Error in background album queueing: ' + err.message); - } - })(); - } catch (err) { - logger.error('Error adding album: ' + err.message); - _slackMessage('🔎 Couldn\'t find that album. Double-check the spelling or try including the artist name! 🎶', channel); - } -} - -// Note: _searchplaylist has been moved to lib/command-handlers.js - -async function _addplaylist(input, channel, userName) { - _logUserAction(userName, 'addplaylist'); - // Add a playlist to the queue, support Spotify URI or search - if (!input || input.length < 2) { - _slackMessage('📋 You need to tell me which playlist to add! Use `addplaylist ` 🎵', channel); - return; - } - const playlist = input.slice(1).join(' '); - logger.info('Playlist to add: ' + playlist); - - try { - let result; - try { - result = await spotify.getPlaylist(playlist); - } catch (e1) { - logger.warn('Direct playlist lookup failed, falling back to search: ' + e1.message); - const candidates = await spotify.searchPlaylistList(playlist, 5); - if (candidates && candidates.length > 0) { - // Sort by relevance and take first result - const sortedCandidates = _sortPlaylistsByRelevance(candidates, playlist); - result = sortedCandidates[0]; - logger.info(`Using playlist candidate: ${result.name} by ${result.owner}`); - } else { - throw new Error('Playlist not found'); - } - } - - // Check for blacklisted tracks in the playlist - const playlistTracks = await spotify.getPlaylistTracks(result.uri); - const blacklistedTracks = playlistTracks.filter(track => - isTrackBlacklisted(track.name, track.artist) - ); - - // If ALL tracks are blacklisted, don't add anything - if (blacklistedTracks.length > 0 && blacklistedTracks.length === playlistTracks.length) { - _slackMessage( - `🚫 Cannot add playlist *${result.name}* - all ${playlistTracks.length} tracks are blacklisted!`, - channel - ); - return; - } - - let warningMessage = ''; - if (blacklistedTracks.length > 0) { - const bannedList = blacklistedTracks.slice(0, 5).map(t => `*${t.name}*`).join(', '); - const moreText = blacklistedTracks.length > 5 ? ` and ${blacklistedTracks.length - 5} more` : ''; - warningMessage = `\n⚠️ Skipped ${blacklistedTracks.length} blacklisted track(s): ${bannedList}${moreText}`; - logger.info(`Filtering out ${blacklistedTracks.length} blacklisted tracks from playlist ${result.name}`); - } - - // Get current player state - const state = await sonos.getCurrentState(); - logger.info('Current state for addplaylist: ' + state); - - const isStopped = state === 'stopped'; - - // If stopped, ensure queue is active source and flush - if (isStopped) { - logger.info('Player stopped - ensuring queue is active and flushing'); - try { - // Stop any active playback to force Sonos to use queue - try { - await sonos.stop(); - await new Promise(resolve => setTimeout(resolve, 300)); - } catch (stopErr) { - // Ignore stop errors (might already be stopped) - logger.debug('Stop command result (may already be stopped): ' + stopErr.message); - } - - // Flush queue to start fresh - await sonos.flush(); - await new Promise(resolve => setTimeout(resolve, 300)); - logger.info('Queue flushed and ready'); - } catch (flushErr) { - logger.warn('Could not flush queue: ' + flushErr.message); - } - } - - // If we have blacklisted tracks, add individually; otherwise use playlist URI - if (blacklistedTracks.length > 0) { - const allowedTracks = playlistTracks.filter(track => - !isTrackBlacklisted(track.name, track.artist) - ); - - // Add allowed tracks individually - for (const track of allowedTracks) { - await sonos.queue(track.uri); - } - logger.info(`Added ${allowedTracks.length} tracks from playlist (filtered ${blacklistedTracks.length})`); - } else { - // No blacklisted tracks, add entire playlist via URI (more efficient) - await sonos.queue(result.uri); - logger.info('Added playlist: ' + result.name); - } - - // Start playback if needed - if (isStopped) { - // Ensure queue is the active source before starting playback - try { - // Stop any active playback to force Sonos to use queue - try { - await sonos.stop(); - await new Promise(resolve => setTimeout(resolve, 500)); - } catch (stopErr) { - // Ignore stop errors (might already be stopped) - logger.debug('Stop before play (may already be stopped): ' + stopErr.message); - } - - // Wait a moment to ensure queue is ready - await new Promise(resolve => setTimeout(resolve, 1000)); - - // Start playback from queue - await sonos.play(); - logger.info('Started playback from queue'); - } catch (playErr) { - logger.warn('Failed to start playback: ' + playErr.message); - } - } else if (state !== 'playing' && state !== 'transitioning') { - try { - await sonos.play(); - logger.info('Player was not playing, started playback.'); - } catch (playErr) { - logger.warn('Failed to auto-play: ' + playErr.message); - } - } - - // Respond to user immediately - const trackCountText = blacklistedTracks.length > 0 - ? `${playlistTracks.length - blacklistedTracks.length} tracks from playlist` - : 'playlist'; - let text = `Added ${trackCountText} *${result.name}* by ${result.owner} to the queue! :notes:`; - text += warningMessage; - - logger.info(`Sending playlist confirmation message: ${text}`); - _slackMessage(text, channel, { trackName: result.name }); // Track playlist name for reactions - - // Queue tracks in background (don't block user response) - (async () => { - try { - // If we have blacklisted tracks, add individually in parallel; otherwise use playlist URI - if (blacklistedTracks.length > 0) { - const allowedTracks = playlistTracks.filter(track => - !isTrackBlacklisted(track.name, track.artist) - ); - - // Queue all tracks in parallel (much faster for large playlists!) - const queuePromises = allowedTracks.map(track => - sonos.queue(track.uri).catch(err => { - logger.warn(`Could not queue track ${track.name}: ${err.message}`); - return null; - }) - ); - - await Promise.allSettled(queuePromises); - logger.info(`Added ${allowedTracks.length} tracks from playlist (filtered ${blacklistedTracks.length})`); - } else { - // No blacklisted tracks, add entire playlist via URI (more efficient) - await sonos.queue(result.uri); - logger.info('Added playlist: ' + result.name); - } - - // Start playback if needed - if (isStopped) { - await new Promise(resolve => setTimeout(resolve, 300)); // Reduced from 1500ms total - await sonos.play(); - logger.info('Started playback after playlist add'); - } else if (state !== 'playing' && state !== 'transitioning') { - await sonos.play(); - logger.info('Player was not playing, started playback.'); - } - } catch (err) { - logger.error('Error in background playlist queueing: ' + err.message); - } - })(); - } catch (err) { - logger.error('Error adding playlist: ' + err.message); - _slackMessage('🔎 Couldn\'t find that playlist. Try a Spotify link, or use `searchplaylist ` to pick one. 🎵', channel); - } -} - -/** - * Sort albums by relevance to search term - * Prioritizes exact matches of both artist and album name - * @param {Array} albums - Array of album objects from Spotify - * @param {string} searchTerm - The search term used - * @returns {Array} Sorted array of albums - */ -function _sortAlbumsByRelevance(albums, searchTerm) { - const termLower = searchTerm.toLowerCase(); - - // Try to detect "artist - album", "album - artist", "album by artist", or "artist by album" format - let separatorIndex = -1; - let separatorLength = 0; - - // Check for " - " separator - if (termLower.includes(' - ')) { - separatorIndex = termLower.indexOf(' - '); - separatorLength = 3; - } - // Check for " by " separator - else if (termLower.includes(' by ')) { - separatorIndex = termLower.indexOf(' by '); - separatorLength = 4; - } - - let artistWords = []; - let albumWords = []; - - if (separatorIndex > 0) { - const part1 = termLower.substring(0, separatorIndex).trim(); - const part2 = termLower.substring(separatorIndex + separatorLength).trim(); - - // For "by" separator: "album by artist" is most common - // For "-" separator: "artist - album" is most common - if (termLower.includes(' by ')) { - albumWords = part1.split(/\s+/).filter(w => w.length > 1); - artistWords = part2.split(/\s+/).filter(w => w.length > 2); - } else { - artistWords = part1.split(/\s+/).filter(w => w.length > 2); - albumWords = part2.split(/\s+/).filter(w => w.length > 1); - } - } else { - albumWords = termLower.split(/\s+/).filter(w => w.length > 1); - } - - return albums.sort((a, b) => { - const aName = a.name.toLowerCase(); - const aArtist = (a.artist || '').toLowerCase(); - const bName = b.name.toLowerCase(); - const bArtist = (b.artist || '').toLowerCase(); - - let aScore = 0; - let bScore = 0; - - if (artistWords.length > 0 && albumWords.length > 0) { - const aArtistMatch = artistWords.every(word => aArtist.includes(word)); - const bArtistMatch = artistWords.every(word => bArtist.includes(word)); - const aAlbumMatch = albumWords.every(word => aName.includes(word)); - const bAlbumMatch = albumWords.every(word => bName.includes(word)); - - if (aArtistMatch && aAlbumMatch) aScore += 10000; - if (bArtistMatch && bAlbumMatch) bScore += 10000; - if (aAlbumMatch) aScore += 5000; - if (bAlbumMatch) bScore += 5000; - if (aArtistMatch) aScore += 2000; - if (bArtistMatch) bScore += 2000; - } else { - const aAlbumMatches = albumWords.filter(w => aName.includes(w)).length; - const bAlbumMatches = albumWords.filter(w => bName.includes(w)).length; - aScore += aAlbumMatches * 1000; - bScore += bAlbumMatches * 1000; - - const aArtistMatches = albumWords.filter(w => w.length > 3 && aArtist.includes(w)).length; - const bArtistMatches = albumWords.filter(w => w.length > 3 && bArtist.includes(w)).length; - aScore += aArtistMatches * 500; - bScore += bArtistMatches * 500; - } - - if (aScore === bScore) { - return (b.popularity || 0) - (a.popularity || 0); - } - - return bScore - aScore; - }); -} - -/** - * Sort playlists by relevance to search term - * Prioritizes exact matches and follower count - * @param {Array} playlists - Array of playlist objects from Spotify - * @param {string} searchTerm - The search term used - * @returns {Array} Sorted array of playlists - */ -function _sortPlaylistsByRelevance(playlists, searchTerm) { - const termLower = searchTerm.toLowerCase(); - const searchWords = termLower.split(/\s+/).filter(w => w.length > 2); - - return playlists.sort((a, b) => { - const aName = a.name.toLowerCase(); - const bName = b.name.toLowerCase(); - - let aScore = 0; - let bScore = 0; - - // Exact match in playlist name - if (aName.includes(termLower)) aScore += 10000; - if (bName.includes(termLower)) bScore += 10000; - - // Word matches - const aMatches = searchWords.filter(w => aName.includes(w)).length; - const bMatches = searchWords.filter(w => bName.includes(w)).length; - aScore += aMatches * 1000; - bScore += bMatches * 1000; - - // Use followers as tie-breaker (popular playlists are usually better) - if (aScore === bScore) { - return (b.followers || 0) - (a.followers || 0); - } - - return bScore - aScore; - }); -} - -/** - * Sort tracks by relevance to search term - * Prioritizes exact matches of both artist and track name - * @param {Array} tracks - Array of track objects from Spotify - * @param {string} searchTerm - The search term used - * @returns {Array} Sorted array of tracks - */ -function _sortTracksByRelevance(tracks, searchTerm) { - const termLower = searchTerm.toLowerCase(); - - // Try to detect "artist - track", "track - artist", "track by artist", or "artist by track" format - let separatorIndex = -1; - let separatorLength = 0; - - // Check for " - " separator - if (termLower.includes(' - ')) { - separatorIndex = termLower.indexOf(' - '); - separatorLength = 3; - } - // Check for " by " separator - else if (termLower.includes(' by ')) { - separatorIndex = termLower.indexOf(' by '); - separatorLength = 4; - } - - let artistWords = []; - let trackWords = []; - - if (separatorIndex > 0) { - // Split on separator to separate artist and track - const part1 = termLower.substring(0, separatorIndex).trim(); - const part2 = termLower.substring(separatorIndex + separatorLength).trim(); - - // For "by" separator: "track by artist" is most common - // For "-" separator: "artist - track" is most common - if (termLower.includes(' by ')) { - // "Best of You by Foo Fighters" -> track by artist - trackWords = part1.split(/\s+/).filter(w => w.length > 1); - artistWords = part2.split(/\s+/).filter(w => w.length > 2); - } else { - // "Foo Fighters - Best of You" -> artist - track - artistWords = part1.split(/\s+/).filter(w => w.length > 2); - trackWords = part2.split(/\s+/).filter(w => w.length > 1); - } - } else { - // No clear separator, split all words - trackWords = termLower.split(/\s+/).filter(w => w.length > 1); - } - - return tracks.sort((a, b) => { - const aName = a.name.toLowerCase(); - const aArtist = (a.artists?.[0]?.name || a.artist || '').toLowerCase(); - const bName = b.name.toLowerCase(); - const bArtist = (b.artists?.[0]?.name || b.artist || '').toLowerCase(); - - let aScore = 0; - let bScore = 0; - - // HIGHEST PRIORITY: Both artist AND track match - if (artistWords.length > 0 && trackWords.length > 0) { - const aArtistMatch = artistWords.every(word => aArtist.includes(word)); - const bArtistMatch = artistWords.every(word => bArtist.includes(word)); - const aTrackMatch = trackWords.every(word => aName.includes(word)); - const bTrackMatch = trackWords.every(word => bName.includes(word)); - - if (aArtistMatch && aTrackMatch) aScore += 10000; - if (bArtistMatch && bTrackMatch) bScore += 10000; - - // High priority: Track name matches even if artist doesn't - if (aTrackMatch) aScore += 5000; - if (bTrackMatch) bScore += 5000; - - // Medium priority: Artist matches - if (aArtistMatch) aScore += 2000; - if (bArtistMatch) bScore += 2000; - } else { - // No " - " separator: check if words match track name or artist - const aTrackMatches = trackWords.filter(w => aName.includes(w)).length; - const bTrackMatches = trackWords.filter(w => bName.includes(w)).length; - aScore += aTrackMatches * 1000; - bScore += bTrackMatches * 1000; - - // Check artist matches (lower priority) - const aArtistMatches = trackWords.filter(w => w.length > 3 && aArtist.includes(w)).length; - const bArtistMatches = trackWords.filter(w => w.length > 3 && bArtist.includes(w)).length; - aScore += aArtistMatches * 500; - bScore += bArtistMatches * 500; - } - - // Use popularity as tie-breaker - if (aScore === bScore) { - return (b.popularity || 0) - (a.popularity || 0); - } - - return bScore - aScore; - }); -} - -// Note: Search commands (_search, _searchalbum) have been moved to lib/command-handlers.js +// Note: _add, _addalbum, _queueAlbum, _addplaylist have been moved to lib/add-handlers.js +// Note: _searchplaylist, _search, _searchalbum have been moved to lib/command-handlers.js +// Note: _sortAlbumsByRelevance, _sortPlaylistsByRelevance, _sortTracksByRelevance have been moved to lib/queue-utils.js function _currentTrackTitle(channel, cb) { sonos @@ -5434,7 +4653,7 @@ async function _sendDirectMessage(userName, text) { } try { - const discordModule = require('./discord'); + const discordModule = require('./lib/discord'); const discordClient = discordModule.getDiscordClient(); if (!discordClient) { @@ -5892,77 +5111,7 @@ async function _setconfig(input, channel, userName) { } } - -async function _append(input, channel, userName) { - _logUserAction(userName, 'append'); - - // Append a track to the queue (never flushes existing queue) - // Start playing if not already playing - if (!input || input.length < 2) { - _slackMessage('🎶 Tell me what song to append! Use `append ` 🎵', channel); - return; - } - - const track = input.slice(1).join(' '); - logger.info('Track to append: ' + track); - - try { - const result = await spotify.getTrack(track); - - // Check if track is already in queue - try { - const queue = await sonos.getQueue(); - if (queue && queue.items) { - let duplicatePosition = -1; - const isDuplicate = queue.items.some((item, index) => { - if (item.uri === result.uri || (item.title === result.name && item.artist === result.artist)) { - duplicatePosition = index; - return true; - } - return false; - }); - - if (isDuplicate) { - _slackMessage( - `*${result.name}* by _${result.artist}_ is already in the queue at position #${duplicatePosition}! :musical_note:\nWant it to play sooner? Use \`vote ${duplicatePosition}\` to move it up! :arrow_up:`, - channel - ); - return; - } - } - } catch (queueErr) { - // If we can't get the queue, just log and continue with adding - logger.warn('Could not check queue for duplicates: ' + queueErr.message); - } - - // Always add to queue (preserving existing tracks) - await sonos.queue(result.uri); - logger.info('Appended track: ' + result.name); - - let msg = '✅ Added *' + result.name + '* by _' + result.artist + '_ to the queue!'; - - // Check if we need to start playback - try { - const state = await sonos.getCurrentState(); - logger.info('Current state after append: ' + state); - - if (state !== 'playing' && state !== 'transitioning') { - // Wait a moment before starting playback - await new Promise(resolve => setTimeout(resolve, 1000)); - await sonos.play(); - logger.info('Started playback after append.'); - msg += ' Playback started! :notes:'; - } - } catch (stateErr) { - logger.warn('Could not check/start playback: ' + stateErr.message); - } - - _slackMessage(msg, channel); - } catch (err) { - logger.error('Error appending track: ' + err.message); - _slackMessage('🤷 Couldn\'t find that track or something went wrong. Try a different search! 🎶', channel); - } -} +// Note: _append has been moved to lib/add-handlers.js function _addToSpotifyPlaylist(input, channel) { // Admin check now handled in processInput (platform-aware) diff --git a/lib/add-handlers.js b/lib/add-handlers.js new file mode 100644 index 00000000..a72c26ac --- /dev/null +++ b/lib/add-handlers.js @@ -0,0 +1,703 @@ +/** + * Add Handlers Module + * Handles add, addalbum, addplaylist, and append commands + * + * Uses dependency injection for testability + * @module add-handlers + */ + +const queueUtils = require('./queue-utils'); + +// ========================================== +// DEPENDENCIES (injected via initialize) +// ========================================== + +let sonos = null; +let spotify = null; +let logger = null; +let sendMessage = async () => {}; +let logUserAction = async () => {}; +let isTrackBlacklisted = () => false; +let musicHelper = null; +let getConfig = () => ({}); +let getAdminChannel = () => null; +let getCurrentPlatform = () => 'slack'; + +/** + * Initialize the add handlers with dependencies + * @param {Object} deps - Dependencies + * @param {Object} deps.logger - Winston logger instance (required) + * @param {Object} deps.sonos - Sonos device instance (required) + * @param {Object} deps.spotify - Spotify API wrapper (required) + * @param {Function} deps.sendMessage - Message sending function (required) + * @param {Function} deps.logUserAction - User action logging function (optional) + * @param {Function} deps.isTrackBlacklisted - Blacklist check function (optional) + * @param {Object} deps.musicHelper - Music helper for URI validation (optional) + * @param {Function} deps.getConfig - Config getter function (optional) + * @param {Function} deps.getAdminChannel - Admin channel getter (optional) + * @param {Function} deps.getCurrentPlatform - Platform getter (optional) + */ +function initialize(deps) { + if (!deps.logger) { + throw new Error('Add handlers require a logger to be injected'); + } + if (!deps.sonos) { + throw new Error('Add handlers require sonos to be injected'); + } + if (!deps.spotify) { + throw new Error('Add handlers require spotify to be injected'); + } + if (!deps.sendMessage) { + throw new Error('Add handlers require sendMessage to be injected'); + } + + logger = deps.logger; + sonos = deps.sonos; + spotify = deps.spotify; + sendMessage = deps.sendMessage; + logUserAction = deps.logUserAction || (async () => {}); + isTrackBlacklisted = deps.isTrackBlacklisted || (() => false); + musicHelper = deps.musicHelper || { isValidSpotifyUri: () => true }; + getConfig = deps.getConfig || (() => ({})); + getAdminChannel = deps.getAdminChannel || (() => null); + getCurrentPlatform = deps.getCurrentPlatform || (() => 'slack'); + + logger.info('✅ Add handlers initialized'); +} + +// ========================================== +// ADD COMMANDS +// ========================================== + +/** + * Add a track to the queue + * If stopped: flush queue and start fresh + * If playing: just add to existing queue + */ +async function add(input, channel, userName) { + logUserAction(userName, 'add'); + + if (!input || input.length < 2) { + sendMessage('🎵 You gotta tell me what to add! Use `add ` 🎶', channel); + return; + } + const track = input.slice(1).join(' '); + logger.info('Track to add: ' + track); + + try { + const tracks = await spotify.searchTrackList(track, 3); + if (!tracks || tracks.length === 0) { + sendMessage("🤷 Couldn't find anything matching that. Try different keywords or check the spelling! 🎵", channel); + return; + } + + // Sort tracks by relevance using queue-utils + const sortedTracks = queueUtils.sortTracksByRelevance(tracks, track); + + // Pre-validate all candidates in parallel before attempting to queue + const candidates = sortedTracks + .filter(t => musicHelper.isValidSpotifyUri(t.uri)) + .map(t => ({ + name: t.name, + artist: t.artist, + uri: t.uri + })); + + if (candidates.length === 0) { + sendMessage("🤷 Found tracks but they have invalid format. Try a different search! 🎵", channel); + return; + } + + // Check if first result is blacklisted (most common case) + const firstCandidate = candidates[0]; + if (isTrackBlacklisted(firstCandidate.name, firstCandidate.artist)) { + logger.info(`Track blocked by blacklist: ${firstCandidate.name} by ${firstCandidate.artist}`); + sendMessage(`🚫 Sorry, *${firstCandidate.name}* by ${firstCandidate.artist} is on the blacklist and cannot be added.`, channel); + return; + } + + // Parallelize getting state and queue to save time + const [state, queue] = await Promise.all([ + sonos.getCurrentState(), + sonos.getQueue().catch(err => { + logger.warn('Could not get queue: ' + err.message); + return null; + }) + ]); + logger.info('Current state for add: ' + state); + + // Handle stopped state - flush queue + if (state === 'stopped') { + logger.info('Player stopped - ensuring queue is active source and flushing'); + try { + try { + await sonos.stop(); + await new Promise(resolve => setTimeout(resolve, 300)); + } catch (stopErr) { + logger.debug('Stop command result (may already be stopped): ' + stopErr.message); + } + + await sonos.flush(); + await new Promise(resolve => setTimeout(resolve, 300)); + logger.info('Queue flushed and ready'); + } catch (flushErr) { + logger.warn('Could not flush queue: ' + flushErr.message); + } + } else if (queue && queue.items) { + // Check for duplicates if playing (using pre-fetched queue) + const duplicatePosition = queue.items.findIndex(item => + item.uri === firstCandidate.uri || + (item.title === firstCandidate.name && item.artist === firstCandidate.artist) + ); + + if (duplicatePosition >= 0) { + sendMessage( + `*${firstCandidate.name}* by _${firstCandidate.artist}_ is already in the queue at position #${duplicatePosition}! :musical_note:\nWant it to play sooner? Use \`vote ${duplicatePosition}\` to move it up! :arrow_up:`, + channel + ); + return; + } + } + + // Try to queue the first valid candidate (most relevant result) + let result = null; + try { + logger.info(`Attempting to queue: ${firstCandidate.name} by ${firstCandidate.artist} (URI: ${firstCandidate.uri})`); + await sonos.queue(firstCandidate.uri); + logger.info('Successfully queued track: ' + firstCandidate.name); + result = firstCandidate; + } catch (e) { + const errorDetails = e.message || String(e); + const upnpErrorMatch = errorDetails.match(/errorCode[>](\d+)[<]/); + const errorCode = upnpErrorMatch ? upnpErrorMatch[1] : null; + + logger.warn(`Queue failed for "${firstCandidate.name}" by ${firstCandidate.artist}: ${errorDetails}${errorCode ? ` (error code: ${errorCode})` : ''}`); + + if (errorCode === '800') { + sendMessage('🤷 Track not available in your region. Try searching for different songs! 🎵', channel); + + // Also notify admin channel about region configuration + const adminChannel = getAdminChannel(); + if (adminChannel && channel !== adminChannel) { + const config = getConfig(); + const currentMarket = (config.get ? config.get('market') : config.market) || 'US'; + const marketOptions = ['US', 'SE', 'GB', 'DE', 'FR', 'CA', 'AU', 'JP', 'NO', 'DK', 'FI']; + const marketOptionsList = marketOptions.map(m => m === currentMarket ? `*${m}* (current)` : m).join(', '); + + sendMessage( + `⚠️ *Spotify Region Warning*\n` + + `Track "*${firstCandidate.name}*" by ${firstCandidate.artist} failed due to region availability.\n\n` + + `Please verify your Spotify region configuration.\n` + + `Current region: *${currentMarket}*\n` + + `Available options: ${marketOptionsList}\n` + + `Update via setup wizard or admin panel.`, + adminChannel + ); + } + } else { + sendMessage('🤷 Couldn\'t add the track. It may not be available or there was an error. Try a different search! 🎵', channel); + } + return; + } + + // Respond immediately to user (don't wait for playback to start) + const currentPlatform = getCurrentPlatform(); + sendMessage( + 'Added ' + '*' + result.name + '*' + ' by ' + result.artist + ' to the queue! :notes:', + channel, + { trackName: result.name, addReactions: currentPlatform === 'discord' } + ); + + // Handle playback state asynchronously (don't block user response) + if (state === 'stopped') { + (async () => { + try { + try { + await sonos.stop(); + await new Promise(resolve => setTimeout(resolve, 500)); + } catch (stopErr) { + logger.debug('Stop before play (may already be stopped): ' + stopErr.message); + } + + // Verify queue has items before trying to play + let queueReady = false; + let retries = 0; + while (!queueReady && retries < 5) { + try { + const q = await sonos.getQueue(); + if (q && q.items && q.items.length > 0) { + queueReady = true; + logger.debug(`Queue verified: ${q.items.length} items ready`); + } else { + logger.debug(`Queue not ready yet (attempt ${retries + 1}/5), waiting...`); + await new Promise(resolve => setTimeout(resolve, 300)); + retries++; + } + } catch (queueErr) { + logger.debug(`Queue check failed (attempt ${retries + 1}/5): ${queueErr.message}`); + await new Promise(resolve => setTimeout(resolve, 300)); + retries++; + } + } + + if (!queueReady) { + logger.warn('Queue not ready after 5 attempts, attempting playback anyway'); + } + + // Try to activate queue by seeking to position 1 + try { + logger.debug('Attempting to seek to queue position 1 to activate queue'); + await sonos.avTransportService().Seek({ + InstanceID: 0, + Unit: 'TRACK_NR', + Target: '1' + }); + logger.debug('Successfully sought to track 1, queue should be active'); + await new Promise(resolve => setTimeout(resolve, 500)); + } catch (seekErr) { + logger.debug('Seek failed, trying next() to activate queue: ' + seekErr.message); + try { + await sonos.next(); + logger.debug('Used next() to activate queue'); + await new Promise(resolve => setTimeout(resolve, 300)); + } catch (nextErr) { + logger.debug('next() also failed: ' + nextErr.message); + } + } + + await new Promise(resolve => setTimeout(resolve, 500)); + await sonos.play(); + logger.info('Started playback from queue'); + } catch (playErr) { + logger.warn('Failed to start playback: ' + playErr.message); + } + })(); + } else if (state === 'paused') { + (async () => { + try { + await sonos.play(); + logger.info('Resumed playback'); + } catch (playErr) { + logger.warn('Failed to resume playback: ' + playErr.message); + } + })(); + } + } catch (err) { + logger.error('Error adding track: ' + err.message); + sendMessage('🤷 Couldn\'t find that track or hit an error adding it. Try being more specific with the song name! 🎵', channel); + } +} + +/** + * Add an album to the queue + * Supports Spotify URI or search + */ +async function addalbum(input, channel, userName) { + logUserAction(userName, 'addalbum'); + + if (!input || input.length < 2) { + sendMessage('💿 You gotta tell me which album to add! Try `addalbum ` 🎶', channel); + return; + } + const album = input.slice(1).join(' '); + logger.info('Album to add: ' + album); + + try { + // If it's a Spotify URI, use getAlbum directly + if (album.startsWith('spotify:album:') || album.includes('spotify.com/album/')) { + const result = await spotify.getAlbum(album); + await queueAlbum(result, album, channel, userName); + return; + } + + // Otherwise search and sort by relevance + const albums = await spotify.searchAlbumList(album, 3); + if (!albums || albums.length === 0) { + sendMessage("🤷 Couldn't find that album. Try including the artist name or checking the spelling! 🎶", channel); + return; + } + + // Sort by relevance and take first result + const sortedAlbums = queueUtils.sortAlbumsByRelevance(albums, album); + const result = { ...sortedAlbums[0], uri: sortedAlbums[0].uri }; + logger.info(`Selected album: ${result.name} by ${result.artist}`); + + await queueAlbum(result, album, channel, userName); + } catch (err) { + logger.error('Error adding album: ' + err.message); + sendMessage('🔎 Couldn\'t find that album. Try a Spotify link, or use `searchalbum ` to pick one. 🎵', channel); + } +} + +/** + * Helper function to queue an album (shared between URI and search flows) + * @private + */ +async function queueAlbum(result, albumSearchTerm, channel, userName) { + try { + // Check for blacklisted tracks in the album + const albumTracks = await spotify.getAlbumTracks(result.uri); + const blacklistedTracks = albumTracks.filter(track => + isTrackBlacklisted(track.name, track.artist) + ); + + // If ALL tracks are blacklisted, don't add anything + if (blacklistedTracks.length > 0 && blacklistedTracks.length === albumTracks.length) { + sendMessage( + `🚫 Cannot add album *${result.name}* - all ${albumTracks.length} tracks are blacklisted!`, + channel + ); + return; + } + + let warningMessage = ''; + if (blacklistedTracks.length > 0) { + const bannedList = blacklistedTracks.map(t => `*${t.name}*`).join(', '); + warningMessage = `\n⚠️ Skipped ${blacklistedTracks.length} blacklisted track(s): ${bannedList}`; + logger.info(`Filtering out ${blacklistedTracks.length} blacklisted tracks from album ${result.name}`); + } + + // Get current player state + const state = await sonos.getCurrentState(); + logger.info('Current state for addalbum: ' + state); + + const isStopped = state === 'stopped'; + + // If stopped, ensure queue is active source and flush + if (isStopped) { + logger.info('Player stopped - ensuring queue is active and flushing'); + try { + try { + await sonos.stop(); + await new Promise(resolve => setTimeout(resolve, 300)); + } catch (stopErr) { + logger.debug('Stop command result (may already be stopped): ' + stopErr.message); + } + + await sonos.flush(); + await new Promise(resolve => setTimeout(resolve, 300)); + logger.info('Queue flushed and ready'); + } catch (flushErr) { + logger.warn('Could not flush queue: ' + flushErr.message); + } + } + + // Respond to user immediately + const trackCountText = blacklistedTracks.length > 0 + ? `${albumTracks.length - blacklistedTracks.length} tracks from album` + : 'album'; + let text = `Added ${trackCountText} *${result.name}* by ${result.artist} to the queue! :notes:`; + text += warningMessage; + + if (result.coverUrl) { + sendMessage(text, channel, { + trackName: result.name, + blocks: [ + { + type: "section", + text: { + type: "mrkdwn", + text: text + }, + accessory: { + type: "image", + image_url: result.coverUrl, + alt_text: result.name + " cover" + } + } + ] + }); + } else { + sendMessage(text, channel, { trackName: result.name }); + } + + // Queue tracks in background (don't block user response) + (async () => { + try { + if (blacklistedTracks.length > 0) { + const allowedTracks = albumTracks.filter(track => + !isTrackBlacklisted(track.name, track.artist) + ); + + const queuePromises = allowedTracks.map(track => + sonos.queue(track.uri).catch(err => { + logger.warn(`Could not queue track ${track.name}: ${err.message}`); + return null; + }) + ); + + await Promise.allSettled(queuePromises); + logger.info(`Added ${allowedTracks.length} tracks from album (filtered ${blacklistedTracks.length})`); + } else { + await sonos.queue(result.uri); + logger.info('Added album: ' + result.name); + } + + if (isStopped) { + await new Promise(resolve => setTimeout(resolve, 300)); + await sonos.play(); + logger.info('Started playback after album add'); + } else if (state !== 'playing' && state !== 'transitioning') { + await sonos.play(); + logger.info('Player was not playing, started playback.'); + } + } catch (err) { + logger.error('Error in background album queueing: ' + err.message); + } + })(); + } catch (err) { + logger.error('Error adding album: ' + err.message); + sendMessage('🔎 Couldn\'t find that album. Double-check the spelling or try including the artist name! 🎶', channel); + } +} + +/** + * Add a playlist to the queue + * Supports Spotify URI or search + */ +async function addplaylist(input, channel, userName) { + logUserAction(userName, 'addplaylist'); + + if (!input || input.length < 2) { + sendMessage('📋 You need to tell me which playlist to add! Use `addplaylist ` 🎵', channel); + return; + } + const playlist = input.slice(1).join(' '); + logger.info('Playlist to add: ' + playlist); + + try { + let result; + try { + result = await spotify.getPlaylist(playlist); + } catch (e1) { + logger.warn('Direct playlist lookup failed, falling back to search: ' + e1.message); + const candidates = await spotify.searchPlaylistList(playlist, 5); + if (candidates && candidates.length > 0) { + const sortedCandidates = queueUtils.sortPlaylistsByRelevance(candidates, playlist); + result = sortedCandidates[0]; + logger.info(`Using playlist candidate: ${result.name} by ${result.owner}`); + } else { + throw new Error('Playlist not found'); + } + } + + // Check for blacklisted tracks in the playlist + const playlistTracks = await spotify.getPlaylistTracks(result.uri); + const blacklistedTracks = playlistTracks.filter(track => + isTrackBlacklisted(track.name, track.artist) + ); + + // If ALL tracks are blacklisted, don't add anything + if (blacklistedTracks.length > 0 && blacklistedTracks.length === playlistTracks.length) { + sendMessage( + `🚫 Cannot add playlist *${result.name}* - all ${playlistTracks.length} tracks are blacklisted!`, + channel + ); + return; + } + + let warningMessage = ''; + if (blacklistedTracks.length > 0) { + const bannedList = blacklistedTracks.slice(0, 5).map(t => `*${t.name}*`).join(', '); + const moreText = blacklistedTracks.length > 5 ? ` and ${blacklistedTracks.length - 5} more` : ''; + warningMessage = `\n⚠️ Skipped ${blacklistedTracks.length} blacklisted track(s): ${bannedList}${moreText}`; + logger.info(`Filtering out ${blacklistedTracks.length} blacklisted tracks from playlist ${result.name}`); + } + + // Get current player state + const state = await sonos.getCurrentState(); + logger.info('Current state for addplaylist: ' + state); + + const isStopped = state === 'stopped'; + + // If stopped, ensure queue is active source and flush + if (isStopped) { + logger.info('Player stopped - ensuring queue is active and flushing'); + try { + try { + await sonos.stop(); + await new Promise(resolve => setTimeout(resolve, 300)); + } catch (stopErr) { + logger.debug('Stop command result (may already be stopped): ' + stopErr.message); + } + + await sonos.flush(); + await new Promise(resolve => setTimeout(resolve, 300)); + logger.info('Queue flushed and ready'); + } catch (flushErr) { + logger.warn('Could not flush queue: ' + flushErr.message); + } + } + + // If we have blacklisted tracks, add individually; otherwise use playlist URI + if (blacklistedTracks.length > 0) { + const allowedTracks = playlistTracks.filter(track => + !isTrackBlacklisted(track.name, track.artist) + ); + + for (const track of allowedTracks) { + await sonos.queue(track.uri); + } + logger.info(`Added ${allowedTracks.length} tracks from playlist (filtered ${blacklistedTracks.length})`); + } else { + await sonos.queue(result.uri); + logger.info('Added playlist: ' + result.name); + } + + // Start playback if needed + if (isStopped) { + try { + try { + await sonos.stop(); + await new Promise(resolve => setTimeout(resolve, 500)); + } catch (stopErr) { + logger.debug('Stop before play (may already be stopped): ' + stopErr.message); + } + + await new Promise(resolve => setTimeout(resolve, 1000)); + await sonos.play(); + logger.info('Started playback from queue'); + } catch (playErr) { + logger.warn('Failed to start playback: ' + playErr.message); + } + } else if (state !== 'playing' && state !== 'transitioning') { + try { + await sonos.play(); + logger.info('Player was not playing, started playback.'); + } catch (playErr) { + logger.warn('Failed to auto-play: ' + playErr.message); + } + } + + // Respond to user immediately + const trackCountText = blacklistedTracks.length > 0 + ? `${playlistTracks.length - blacklistedTracks.length} tracks from playlist` + : 'playlist'; + let text = `Added ${trackCountText} *${result.name}* by ${result.owner} to the queue! :notes:`; + text += warningMessage; + + logger.info(`Sending playlist confirmation message: ${text}`); + sendMessage(text, channel, { trackName: result.name }); + + // Queue tracks in background (don't block user response) + (async () => { + try { + if (blacklistedTracks.length > 0) { + const allowedTracks = playlistTracks.filter(track => + !isTrackBlacklisted(track.name, track.artist) + ); + + const queuePromises = allowedTracks.map(track => + sonos.queue(track.uri).catch(err => { + logger.warn(`Could not queue track ${track.name}: ${err.message}`); + return null; + }) + ); + + await Promise.allSettled(queuePromises); + logger.info(`Added ${allowedTracks.length} tracks from playlist (filtered ${blacklistedTracks.length})`); + } else { + await sonos.queue(result.uri); + logger.info('Added playlist: ' + result.name); + } + + if (isStopped) { + await new Promise(resolve => setTimeout(resolve, 300)); + await sonos.play(); + logger.info('Started playback after playlist add'); + } else if (state !== 'playing' && state !== 'transitioning') { + await sonos.play(); + logger.info('Player was not playing, started playback.'); + } + } catch (err) { + logger.error('Error in background playlist queueing: ' + err.message); + } + })(); + } catch (err) { + logger.error('Error adding playlist: ' + err.message); + sendMessage('🔎 Couldn\'t find that playlist. Try a Spotify link, or use `searchplaylist ` to pick one. 🎵', channel); + } +} + +/** + * Append a track to the queue (never flushes existing queue) + * Start playing if not already playing + */ +async function append(input, channel, userName) { + logUserAction(userName, 'append'); + + if (!input || input.length < 2) { + sendMessage('🎶 Tell me what song to append! Use `append ` 🎵', channel); + return; + } + + const track = input.slice(1).join(' '); + logger.info('Track to append: ' + track); + + try { + const result = await spotify.getTrack(track); + + // Check if track is already in queue + try { + const queue = await sonos.getQueue(); + if (queue && queue.items) { + let duplicatePosition = -1; + const isDuplicate = queue.items.some((item, index) => { + if (item.uri === result.uri || (item.title === result.name && item.artist === result.artist)) { + duplicatePosition = index; + return true; + } + return false; + }); + + if (isDuplicate) { + sendMessage( + `*${result.name}* by _${result.artist}_ is already in the queue at position #${duplicatePosition}! :musical_note:\nWant it to play sooner? Use \`vote ${duplicatePosition}\` to move it up! :arrow_up:`, + channel + ); + return; + } + } + } catch (queueErr) { + logger.warn('Could not check queue for duplicates: ' + queueErr.message); + } + + // Always add to queue (preserving existing tracks) + await sonos.queue(result.uri); + logger.info('Appended track: ' + result.name); + + let msg = '✅ Added *' + result.name + '* by _' + result.artist + '_ to the queue!'; + + // Check if we need to start playback + try { + const state = await sonos.getCurrentState(); + logger.info('Current state after append: ' + state); + + if (state !== 'playing' && state !== 'transitioning') { + await new Promise(resolve => setTimeout(resolve, 1000)); + await sonos.play(); + logger.info('Started playback after append.'); + msg += ' Playback started! :notes:'; + } + } catch (stateErr) { + logger.warn('Could not check/start playback: ' + stateErr.message); + } + + sendMessage(msg, channel); + } catch (err) { + logger.error('Error appending track: ' + err.message); + sendMessage('🤷 Couldn\'t find that track or something went wrong. Try a different search! 🎶', channel); + } +} + +// ========================================== +// MODULE EXPORTS +// ========================================== + +module.exports = { + initialize, + add, + addalbum, + addplaylist, + append +}; diff --git a/ai-handler.js b/lib/ai-handler.js similarity index 100% rename from ai-handler.js rename to lib/ai-handler.js diff --git a/lib/command-handlers.js b/lib/command-handlers.js index fe73f47e..352147d2 100644 --- a/lib/command-handlers.js +++ b/lib/command-handlers.js @@ -321,7 +321,7 @@ async function showQueue(channel) { const tracks = []; - result.items.map(function (item, i) { + result.items.forEach(function (item, i) { let trackTitle = item.title; let prefix = ''; @@ -576,6 +576,12 @@ function setVolume(input, channel, userName) { */ async function search(input, channel, userName) { logUserAction(userName, 'search'); + + if (!spotify) { + sendMessage('🎵 Spotify is not configured. Search is unavailable.', channel); + return; + } + const { searchLimit } = getConfig(); if (!input || input.length < 2) { @@ -612,6 +618,11 @@ async function search(input, channel, userName) { * Search for albums */ async function searchalbum(input, channel) { + if (!spotify) { + sendMessage('🎵 Spotify is not configured. Search is unavailable.', channel); + return; + } + const { searchLimit } = getConfig(); if (!input || input.length < 2) { @@ -649,6 +660,11 @@ async function searchalbum(input, channel) { async function searchplaylist(input, channel, userName) { logUserAction(userName, 'searchplaylist'); + if (!spotify) { + sendMessage('🎵 Spotify is not configured. Search is unavailable.', channel); + return; + } + if (!input || input.length < 2) { sendMessage('🔍 Tell me which playlist to search for! `searchplaylist ` 🎶', channel); return; diff --git a/discord.js b/lib/discord.js similarity index 100% rename from discord.js rename to lib/discord.js diff --git a/logger.js b/lib/logger.js similarity index 100% rename from logger.js rename to lib/logger.js diff --git a/music-helper.js b/lib/music-helper.js similarity index 100% rename from music-helper.js rename to lib/music-helper.js diff --git a/slack.js b/lib/slack.js similarity index 100% rename from slack.js rename to lib/slack.js diff --git a/soundcraft-handler.js b/lib/soundcraft-handler.js similarity index 100% rename from soundcraft-handler.js rename to lib/soundcraft-handler.js diff --git a/spotify-async.js b/lib/spotify.js similarity index 100% rename from spotify-async.js rename to lib/spotify.js diff --git a/telemetry.js b/lib/telemetry.js similarity index 100% rename from telemetry.js rename to lib/telemetry.js diff --git a/utils.js b/lib/utils.js similarity index 100% rename from utils.js rename to lib/utils.js diff --git a/voting.js b/lib/voting.js similarity index 100% rename from voting.js rename to lib/voting.js diff --git a/package-lock.json b/package-lock.json index fbd8caba..606fe325 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21,7 +21,7 @@ "mp3-duration": "^1.1.0", "nconf": "^0.13.0", "openai": "^6.16.0", - "posthog-node": "^5.24.1", + "posthog-node": "^5.24.2", "selfsigned": "^5.5.0", "sonos": "^1.14.2", "soundcraft-ui-connection": "^4.1.1", @@ -460,9 +460,9 @@ } }, "node_modules/@posthog/core": { - "version": "1.13.0", - "resolved": "https://registry.npmjs.org/@posthog/core/-/core-1.13.0.tgz", - "integrity": "sha512-knjncrk7qRmssFRbGzBl1Tunt21GRpe0Wv+uVelyL0Rh7PdQUsgguulzXFTps8hA6wPwTU4kq85qnbAJ3eH6Wg==", + "version": "1.14.0", + "resolved": "https://registry.npmjs.org/@posthog/core/-/core-1.14.0.tgz", + "integrity": "sha512-havjGYHwL8Gy6LXIR911h+M/sYlJLQbepxP/cc1M7Cp3v8F92bzpqkbuvUIUyb7/izkxfGwc9wMqKAo0QxMTrg==", "license": "MIT", "dependencies": { "cross-spawn": "^7.0.6" @@ -2455,12 +2455,12 @@ } }, "node_modules/posthog-node": { - "version": "5.24.1", - "resolved": "https://registry.npmjs.org/posthog-node/-/posthog-node-5.24.1.tgz", - "integrity": "sha512-1+wsosb5fjuor9zpp3h2uq0xKYY7rDz8gpw/10Scz8Ob/uVNrsHSwGy76D9rgt4cfyaEgpJwyYv+hPi2+YjWtw==", + "version": "5.24.2", + "resolved": "https://registry.npmjs.org/posthog-node/-/posthog-node-5.24.2.tgz", + "integrity": "sha512-cywIUYtSIC9BilgLlZd1R2xNk6omKL6tywG/SCPmUJKeG2jhjvJHSrHXYx4x3uQsUjn8aB9UVI8km+W326Zm8g==", "license": "MIT", "dependencies": { - "@posthog/core": "1.13.0" + "@posthog/core": "1.14.0" }, "engines": { "node": "^20.20.0 || >=22.22.0" diff --git a/package.json b/package.json index bb10fa3c..ed72f5f2 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "mp3-duration": "^1.1.0", "nconf": "^0.13.0", "openai": "^6.16.0", - "posthog-node": "^5.24.1", + "posthog-node": "^5.24.2", "selfsigned": "^5.5.0", "sonos": "^1.14.2", "soundcraft-ui-connection": "^4.1.1", diff --git a/test/add-handlers.test.mjs b/test/add-handlers.test.mjs new file mode 100644 index 00000000..55d7c813 --- /dev/null +++ b/test/add-handlers.test.mjs @@ -0,0 +1,556 @@ +import { expect } from 'chai'; +import sinon from 'sinon'; + +/** + * Add Handlers Tests + * Tests add, addalbum, addplaylist, and append commands with mocked dependencies + */ + +import { createRequire } from 'module'; +const require = createRequire(import.meta.url); + +describe('Add Handlers', function() { + let addHandlers; + let mockSonos; + let mockSpotify; + let mockLogger; + let mockMusicHelper; + let messages; + let userActions; + + beforeEach(function() { + // Clear module cache to get fresh module state + delete require.cache[require.resolve('../lib/add-handlers.js')]; + addHandlers = require('../lib/add-handlers.js'); + + messages = []; + userActions = []; + + // Create mock Sonos device + mockSonos = { + stop: sinon.stub().resolves(), + play: sinon.stub().resolves(), + flush: sinon.stub().resolves(), + queue: sinon.stub().resolves(), + getQueue: sinon.stub().resolves({ + items: [ + { title: 'Track 1', artist: 'Artist 1', uri: 'spotify:track:1' }, + { title: 'Track 2', artist: 'Artist 2', uri: 'spotify:track:2' } + ], + total: 2 + }), + getCurrentState: sinon.stub().resolves('playing'), + next: sinon.stub().resolves(), + avTransportService: sinon.stub().returns({ + Seek: sinon.stub().resolves() + }) + }; + + // Create mock Spotify + mockSpotify = { + searchTrackList: sinon.stub().resolves([ + { name: 'Test Track', artist: 'Test Artist', uri: 'spotify:track:abc123', popularity: 80 }, + { name: 'Test Track 2', artist: 'Test Artist', uri: 'spotify:track:def456', popularity: 60 } + ]), + searchAlbumList: sinon.stub().resolves([ + { name: 'Test Album', artist: 'Test Artist', uri: 'spotify:album:abc123', popularity: 75 } + ]), + searchPlaylistList: sinon.stub().resolves([ + { name: 'Test Playlist', owner: 'Test User', uri: 'spotify:playlist:abc123' } + ]), + getAlbum: sinon.stub().resolves({ + name: 'Test Album', + artist: 'Test Artist', + uri: 'spotify:album:abc123' + }), + getAlbumTracks: sinon.stub().resolves([ + { name: 'Track 1', artist: 'Test Artist', uri: 'spotify:track:1' }, + { name: 'Track 2', artist: 'Test Artist', uri: 'spotify:track:2' } + ]), + getPlaylist: sinon.stub().resolves({ + name: 'Test Playlist', + owner: 'Test User', + uri: 'spotify:playlist:abc123' + }), + getPlaylistTracks: sinon.stub().resolves([ + { name: 'Track 1', artist: 'Playlist Artist', uri: 'spotify:track:p1' }, + { name: 'Track 2', artist: 'Playlist Artist', uri: 'spotify:track:p2' } + ]), + getTrack: sinon.stub().resolves({ + name: 'Appended Track', + artist: 'Append Artist', + uri: 'spotify:track:append123' + }) + }; + + // Create mock logger + mockLogger = { + info: sinon.stub(), + error: sinon.stub(), + warn: sinon.stub(), + debug: sinon.stub() + }; + + // Create mock music helper + mockMusicHelper = { + isValidSpotifyUri: sinon.stub().returns(true) + }; + + // Initialize add handlers + addHandlers.initialize({ + logger: mockLogger, + sonos: mockSonos, + spotify: mockSpotify, + sendMessage: async (msg, ch, opts) => { + messages.push({ message: msg, channel: ch, options: opts }); + }, + logUserAction: async (userName, action) => { + userActions.push({ userName, action }); + }, + isTrackBlacklisted: () => false, + musicHelper: mockMusicHelper, + getConfig: () => ({ get: () => 'US' }), + getAdminChannel: () => null, + getCurrentPlatform: () => 'slack' + }); + }); + + afterEach(function() { + sinon.restore(); + }); + + // ========================================== + // INITIALIZATION TESTS + // ========================================== + + describe('Initialization', function() { + it('should throw error if logger is not provided', function() { + delete require.cache[require.resolve('../lib/add-handlers.js')]; + const freshHandlers = require('../lib/add-handlers.js'); + + expect(() => freshHandlers.initialize({ + sonos: mockSonos, + spotify: mockSpotify, + sendMessage: async () => {} + })).to.throw('Add handlers require a logger to be injected'); + }); + + it('should throw error if sonos is not provided', function() { + delete require.cache[require.resolve('../lib/add-handlers.js')]; + const freshHandlers = require('../lib/add-handlers.js'); + + expect(() => freshHandlers.initialize({ + logger: mockLogger, + spotify: mockSpotify, + sendMessage: async () => {} + })).to.throw('Add handlers require sonos to be injected'); + }); + + it('should throw error if spotify is not provided', function() { + delete require.cache[require.resolve('../lib/add-handlers.js')]; + const freshHandlers = require('../lib/add-handlers.js'); + + expect(() => freshHandlers.initialize({ + logger: mockLogger, + sonos: mockSonos, + sendMessage: async () => {} + })).to.throw('Add handlers require spotify to be injected'); + }); + + it('should throw error if sendMessage is not provided', function() { + delete require.cache[require.resolve('../lib/add-handlers.js')]; + const freshHandlers = require('../lib/add-handlers.js'); + + expect(() => freshHandlers.initialize({ + logger: mockLogger, + sonos: mockSonos, + spotify: mockSpotify + })).to.throw('Add handlers require sendMessage to be injected'); + }); + + it('should initialize successfully with all required dependencies', function() { + delete require.cache[require.resolve('../lib/add-handlers.js')]; + const freshHandlers = require('../lib/add-handlers.js'); + + expect(() => freshHandlers.initialize({ + logger: mockLogger, + sonos: mockSonos, + spotify: mockSpotify, + sendMessage: async () => {} + })).to.not.throw(); + }); + }); + + // ========================================== + // ADD COMMAND TESTS + // ========================================== + + describe('add', function() { + it('should send error message when no track specified', async function() { + await addHandlers.add(['add'], 'channel1', 'user1'); + + expect(messages).to.have.lengthOf(1); + expect(messages[0].message).to.include('gotta tell me what to add'); + }); + + it('should log user action', async function() { + await addHandlers.add(['add', 'test', 'track'], 'channel1', 'user1'); + + expect(userActions).to.have.lengthOf(1); + expect(userActions[0]).to.deep.equal({ userName: 'user1', action: 'add' }); + }); + + it('should search for tracks when valid input provided', async function() { + await addHandlers.add(['add', 'test', 'track'], 'channel1', 'user1'); + + expect(mockSpotify.searchTrackList.calledOnce).to.be.true; + expect(mockSpotify.searchTrackList.calledWith('test track', 3)).to.be.true; + }); + + it('should send error message when no tracks found', async function() { + mockSpotify.searchTrackList.resolves([]); + + await addHandlers.add(['add', 'nonexistent', 'track'], 'channel1', 'user1'); + + expect(messages).to.have.lengthOf(1); + expect(messages[0].message).to.include("Couldn't find anything"); + }); + + it('should queue track when found', async function() { + await addHandlers.add(['add', 'test', 'track'], 'channel1', 'user1'); + + // Wait for async operations + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(mockSonos.queue.called).to.be.true; + }); + + it('should send confirmation message when track added', async function() { + await addHandlers.add(['add', 'test', 'track'], 'channel1', 'user1'); + + // Wait for async operations + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(messages.some(m => m.message.includes('Added'))).to.be.true; + }); + + it('should detect duplicate tracks in queue', async function() { + // Set up queue with the track we'll try to add + mockSonos.getQueue.resolves({ + items: [ + { title: 'Test Track', artist: 'Test Artist', uri: 'spotify:track:abc123' } + ] + }); + + await addHandlers.add(['add', 'test', 'track'], 'channel1', 'user1'); + + expect(messages.some(m => m.message.includes('already in the queue'))).to.be.true; + }); + + it('should flush queue when player is stopped', async function() { + mockSonos.getCurrentState.resolves('stopped'); + + await addHandlers.add(['add', 'test', 'track'], 'channel1', 'user1'); + + // Wait for async operations + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(mockSonos.flush.called).to.be.true; + }); + + it('should block blacklisted tracks', async function() { + delete require.cache[require.resolve('../lib/add-handlers.js')]; + const freshHandlers = require('../lib/add-handlers.js'); + + freshHandlers.initialize({ + logger: mockLogger, + sonos: mockSonos, + spotify: mockSpotify, + sendMessage: async (msg, ch, opts) => { + messages.push({ message: msg, channel: ch, options: opts }); + }, + logUserAction: async () => {}, + isTrackBlacklisted: (name, artist) => name === 'Test Track', + musicHelper: mockMusicHelper, + getConfig: () => ({ get: () => 'US' }), + getAdminChannel: () => null, + getCurrentPlatform: () => 'slack' + }); + + await freshHandlers.add(['add', 'test', 'track'], 'channel1', 'user1'); + + expect(messages.some(m => m.message.includes('blacklist'))).to.be.true; + expect(mockSonos.queue.called).to.be.false; + }); + + it('should handle invalid URIs gracefully', async function() { + mockMusicHelper.isValidSpotifyUri.returns(false); + + await addHandlers.add(['add', 'test', 'track'], 'channel1', 'user1'); + + expect(messages.some(m => m.message.includes('invalid format'))).to.be.true; + }); + }); + + // ========================================== + // ADDALBUM COMMAND TESTS + // ========================================== + + describe('addalbum', function() { + it('should send error message when no album specified', async function() { + await addHandlers.addalbum(['addalbum'], 'channel1', 'user1'); + + expect(messages).to.have.lengthOf(1); + expect(messages[0].message).to.include('gotta tell me which album'); + }); + + it('should log user action', async function() { + await addHandlers.addalbum(['addalbum', 'test', 'album'], 'channel1', 'user1'); + + expect(userActions).to.have.lengthOf(1); + expect(userActions[0]).to.deep.equal({ userName: 'user1', action: 'addalbum' }); + }); + + it('should search for albums when valid input provided', async function() { + await addHandlers.addalbum(['addalbum', 'test', 'album'], 'channel1', 'user1'); + + expect(mockSpotify.searchAlbumList.calledOnce).to.be.true; + }); + + it('should send error message when no albums found', async function() { + mockSpotify.searchAlbumList.resolves([]); + + await addHandlers.addalbum(['addalbum', 'nonexistent'], 'channel1', 'user1'); + + expect(messages.some(m => m.message.includes("Couldn't find that album"))).to.be.true; + }); + + it('should handle Spotify URI directly', async function() { + await addHandlers.addalbum(['addalbum', 'spotify:album:abc123'], 'channel1', 'user1'); + + expect(mockSpotify.getAlbum.calledOnce).to.be.true; + expect(mockSpotify.searchAlbumList.called).to.be.false; + }); + + it('should handle Spotify URL directly', async function() { + await addHandlers.addalbum(['addalbum', 'https://open.spotify.com/album/abc123'], 'channel1', 'user1'); + + expect(mockSpotify.getAlbum.calledOnce).to.be.true; + }); + + it('should get album tracks for blacklist checking', async function() { + await addHandlers.addalbum(['addalbum', 'test', 'album'], 'channel1', 'user1'); + + // Wait for async operations + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(mockSpotify.getAlbumTracks.called).to.be.true; + }); + + it('should send confirmation message when album added', async function() { + await addHandlers.addalbum(['addalbum', 'test', 'album'], 'channel1', 'user1'); + + // Wait for async operations + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(messages.some(m => m.message.includes('Added') && m.message.includes('album'))).to.be.true; + }); + + it('should block completely blacklisted albums', async function() { + delete require.cache[require.resolve('../lib/add-handlers.js')]; + const freshHandlers = require('../lib/add-handlers.js'); + + freshHandlers.initialize({ + logger: mockLogger, + sonos: mockSonos, + spotify: mockSpotify, + sendMessage: async (msg, ch, opts) => { + messages.push({ message: msg, channel: ch, options: opts }); + }, + logUserAction: async () => {}, + isTrackBlacklisted: () => true, // All tracks blacklisted + musicHelper: mockMusicHelper, + getConfig: () => ({ get: () => 'US' }), + getAdminChannel: () => null, + getCurrentPlatform: () => 'slack' + }); + + await freshHandlers.addalbum(['addalbum', 'test', 'album'], 'channel1', 'user1'); + + expect(messages.some(m => m.message.includes('Cannot add album') && m.message.includes('blacklisted'))).to.be.true; + }); + }); + + // ========================================== + // ADDPLAYLIST COMMAND TESTS + // ========================================== + + describe('addplaylist', function() { + it('should send error message when no playlist specified', async function() { + await addHandlers.addplaylist(['addplaylist'], 'channel1', 'user1'); + + expect(messages).to.have.lengthOf(1); + expect(messages[0].message).to.include('need to tell me which playlist'); + }); + + it('should log user action', async function() { + await addHandlers.addplaylist(['addplaylist', 'test', 'playlist'], 'channel1', 'user1'); + + expect(userActions).to.have.lengthOf(1); + expect(userActions[0]).to.deep.equal({ userName: 'user1', action: 'addplaylist' }); + }); + + it('should try direct lookup first', async function() { + await addHandlers.addplaylist(['addplaylist', 'test', 'playlist'], 'channel1', 'user1'); + + expect(mockSpotify.getPlaylist.called).to.be.true; + }); + + it('should fall back to search when direct lookup fails', async function() { + mockSpotify.getPlaylist.rejects(new Error('Not found')); + + await addHandlers.addplaylist(['addplaylist', 'test', 'playlist'], 'channel1', 'user1'); + + expect(mockSpotify.searchPlaylistList.called).to.be.true; + }); + + it('should get playlist tracks for blacklist checking', async function() { + await addHandlers.addplaylist(['addplaylist', 'test', 'playlist'], 'channel1', 'user1'); + + // Wait for async operations + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(mockSpotify.getPlaylistTracks.called).to.be.true; + }); + + it('should send confirmation message when playlist added', async function() { + await addHandlers.addplaylist(['addplaylist', 'test', 'playlist'], 'channel1', 'user1'); + + // Wait for async operations + await new Promise(resolve => setTimeout(resolve, 100)); + + expect(messages.some(m => m.message.includes('Added') && m.message.includes('playlist'))).to.be.true; + }); + + it('should send error when search also fails', async function() { + mockSpotify.getPlaylist.rejects(new Error('Not found')); + mockSpotify.searchPlaylistList.resolves([]); + + await addHandlers.addplaylist(['addplaylist', 'nonexistent'], 'channel1', 'user1'); + + expect(messages.some(m => m.message.includes("Couldn't find that playlist"))).to.be.true; + }); + }); + + // ========================================== + // APPEND COMMAND TESTS + // ========================================== + + describe('append', function() { + it('should send error message when no track specified', async function() { + await addHandlers.append(['append'], 'channel1', 'user1'); + + expect(messages).to.have.lengthOf(1); + expect(messages[0].message).to.include('Tell me what song to append'); + }); + + it('should log user action', async function() { + await addHandlers.append(['append', 'test', 'track'], 'channel1', 'user1'); + + expect(userActions).to.have.lengthOf(1); + expect(userActions[0]).to.deep.equal({ userName: 'user1', action: 'append' }); + }); + + it('should get track from Spotify', async function() { + await addHandlers.append(['append', 'test', 'track'], 'channel1', 'user1'); + + expect(mockSpotify.getTrack.calledOnce).to.be.true; + expect(mockSpotify.getTrack.calledWith('test track')).to.be.true; + }); + + it('should queue track without flushing', async function() { + await addHandlers.append(['append', 'test', 'track'], 'channel1', 'user1'); + + // Wait for async operations + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(mockSonos.queue.called).to.be.true; + expect(mockSonos.flush.called).to.be.false; + }); + + it('should send confirmation message when track appended', async function() { + await addHandlers.append(['append', 'test', 'track'], 'channel1', 'user1'); + + // Wait for async operations + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(messages.some(m => m.message.includes('Added') && m.message.includes('Appended Track'))).to.be.true; + }); + + it('should detect duplicate tracks when appending', async function() { + mockSonos.getQueue.resolves({ + items: [ + { title: 'Appended Track', artist: 'Append Artist', uri: 'spotify:track:append123' } + ] + }); + + await addHandlers.append(['append', 'test', 'track'], 'channel1', 'user1'); + + expect(messages.some(m => m.message.includes('already in the queue'))).to.be.true; + expect(mockSonos.queue.called).to.be.false; + }); + + it('should start playback if not already playing', async function() { + this.timeout(3000); // Increase timeout for this test + mockSonos.getCurrentState.resolves('stopped'); + + await addHandlers.append(['append', 'test', 'track'], 'channel1', 'user1'); + + // Wait for async operations (including the 1000ms timeout in append) + await new Promise(resolve => setTimeout(resolve, 1200)); + + expect(mockSonos.play.called).to.be.true; + }); + + it('should send error when track not found', async function() { + mockSpotify.getTrack.rejects(new Error('Track not found')); + + await addHandlers.append(['append', 'nonexistent'], 'channel1', 'user1'); + + expect(messages.some(m => m.message.includes("Couldn't find that track"))).to.be.true; + }); + }); + + // ========================================== + // QUEUE BEHAVIOR TESTS + // ========================================== + + describe('Queue Behavior', function() { + it('add should flush queue when stopped, then start playback', async function() { + mockSonos.getCurrentState.resolves('stopped'); + + await addHandlers.add(['add', 'test', 'track'], 'channel1', 'user1'); + + // Wait for async operations + await new Promise(resolve => setTimeout(resolve, 100)); + + expect(mockSonos.flush.called).to.be.true; + }); + + it('add should not flush queue when playing', async function() { + mockSonos.getCurrentState.resolves('playing'); + + await addHandlers.add(['add', 'test', 'track'], 'channel1', 'user1'); + + expect(mockSonos.flush.called).to.be.false; + }); + + it('append should never flush queue regardless of state', async function() { + mockSonos.getCurrentState.resolves('stopped'); + + await addHandlers.append(['append', 'test', 'track'], 'channel1', 'user1'); + + expect(mockSonos.flush.called).to.be.false; + }); + }); +}); diff --git a/test/ai-handler.test.mjs b/test/ai-handler.test.mjs index 4672e509..4635cec5 100644 --- a/test/ai-handler.test.mjs +++ b/test/ai-handler.test.mjs @@ -1,5 +1,5 @@ import { expect } from 'chai'; -import { getSeasonalContext } from '../ai-handler.js'; +import { getSeasonalContext } from '../lib/ai-handler.js'; describe('AI Handler', function() { describe('#getSeasonalContext', function() { diff --git a/test/tools/integration-test-suite.mjs b/test/tools/integration-test-suite.mjs index 63d6289e..26e6610a 100644 --- a/test/tools/integration-test-suite.mjs +++ b/test/tools/integration-test-suite.mjs @@ -80,17 +80,32 @@ async function getChannelHistory(channelId, limit = 10) { channel: channelId, limit, }); - return result.messages || []; + return { messages: result.messages || [], retryAfter: null }; } catch (error) { + const retryAfter = error?.data?.retry_after || null; if (verbose) console.error('❌ Error getting history:', error.message); - return []; + return { messages: [], retryAfter }; + } +} + +async function getThreadReplies(channelId, threadTs, limit = 50) { + try { + const result = await slack.conversations.replies({ + channel: channelId, + ts: threadTs, + limit, + }); + return { messages: result.messages || [], retryAfter: null }; + } catch (error) { + const retryAfter = error?.data?.retry_after || null; + if (verbose) console.error('❌ Error getting thread replies:', error.message); + return { messages: [], retryAfter }; } } // Send a message and wait for response async function sendAndWaitForResponse(message, waitTime = 3, targetChannel = null) { const channel = targetChannel || channelId; - const timestampBefore = Date.now() / 1000; const sendTime = Date.now(); try { @@ -104,26 +119,38 @@ async function sendAndWaitForResponse(message, waitTime = 3, targetChannel = nul throw new Error('Failed to send message'); } + // Use the sent message's timestamp as reference (more reliable than local clock) + const sentMessageTs = parseFloat(result.ts); + // Poll for responses instead of just waiting // Check every 200ms for responses, but wait up to waitTime seconds total let firstResponseTime = null; - const pollInterval = 200; // Check every 200ms + const pollInterval = 1000; // Check every 1s to avoid rate limits const maxWaitTime = waitTime * 1000; // Convert to milliseconds const startTime = Date.now(); let allResponses = []; let seenMessageIds = new Set(); + // Track our own message so we don't pick it up + seenMessageIds.add(result.ts); while (Date.now() - startTime < maxWaitTime) { // Get messages after - const messagesAfter = await getChannelHistory(channel, 20); + const historyResult = await getChannelHistory(channel, 10); + if (historyResult.retryAfter) { + await new Promise(resolve => setTimeout(resolve, historyResult.retryAfter * 1000)); + continue; + } + const messagesAfter = historyResult.messages; - // Find new messages from OTHER bots (not TestBot itself) + // Find new messages that came AFTER our sent message const newBotResponses = messagesAfter.filter(msg => { - const isFromBot = msg.bot_id || (msg.user && msg.user !== botUserId); - const isNew = parseFloat(msg.ts) > timestampBefore; + const msgTs = parseFloat(msg.ts); + const isAfterOurMessage = msgTs > sentMessageTs; + const isNotOurMessage = msg.ts !== result.ts; const isNotTestBot = msg.user !== botUserId; const isNewMessage = !seenMessageIds.has(msg.ts); - return isFromBot && isNew && isNotTestBot && isNewMessage; + // Accept any message that's not from TestBot and came after our message + return isAfterOurMessage && isNotOurMessage && isNotTestBot && isNewMessage; }); // Add new responses @@ -137,6 +164,48 @@ async function sendAndWaitForResponse(message, waitTime = 3, targetChannel = nul } } + // Check for thread replies to OUR sent message (bot may reply in thread) + const ourThreadReplies = await getThreadReplies(channel, result.ts, 25); + if (ourThreadReplies.retryAfter) { + await new Promise(resolve => setTimeout(resolve, ourThreadReplies.retryAfter * 1000)); + } else { + for (const reply of ourThreadReplies.messages) { + const isNotOurMessage = reply.ts !== result.ts; + const isNotTestBot = reply.user !== botUserId; + const isNewMessage = !seenMessageIds.has(reply.ts); + if (isNotOurMessage && isNotTestBot && isNewMessage) { + seenMessageIds.add(reply.ts); + allResponses.push(reply); + if (firstResponseTime === null) { + firstResponseTime = Date.now(); + } + } + } + } + + // Also check thread replies on any bot responses that started threads + for (const resp of newBotResponses) { + if (!resp.thread_ts || resp.thread_ts === result.ts) continue; // Skip if it's our thread + const repliesResult = await getThreadReplies(channel, resp.thread_ts, 25); + if (repliesResult.retryAfter) { + await new Promise(resolve => setTimeout(resolve, repliesResult.retryAfter * 1000)); + continue; + } + for (const reply of repliesResult.messages) { + const isAfterOurMessage = parseFloat(reply.ts) > sentMessageTs; + const isNotOurMessage = reply.ts !== result.ts; + const isNotTestBot = reply.user !== botUserId; + const isNewMessage = !seenMessageIds.has(reply.ts); + if (isAfterOurMessage && isNotOurMessage && isNotTestBot && isNewMessage) { + seenMessageIds.add(reply.ts); + allResponses.push(reply); + if (firstResponseTime === null) { + firstResponseTime = Date.now(); + } + } + } + } + // If we got a response and we've waited at least 1 second, we can break early // But still wait a bit more to catch multiple responses if (firstResponseTime !== null && Date.now() - firstResponseTime > 1000) { @@ -177,7 +246,7 @@ async function sendAndWaitForResponse(message, waitTime = 3, targetChannel = nul // Test case class class TestCase { - constructor(name, command, validator, waitTime = 3, targetChannel = null) { + constructor(name, command, validator, waitTime = 7, targetChannel = null) { this.name = name; this.command = command; this.validator = validator; @@ -369,8 +438,8 @@ class TestCase { } } - async run() { - if (verbose) console.log(`\n🧪 Running: ${this.name}`); + async run(isRetry = false) { + if (verbose) console.log(`\n🧪 Running: ${this.name}${isRetry ? ' (RETRY)' : ''}`); if (verbose) console.log(` Command: "${this.command}"`); if (verbose && this.targetChannel) console.log(` Channel: ${this.targetChannel === adminChannelId ? 'Admin' : 'Standard'}`); if (verbose) console.log(` Expected: ${this.getExpectedDescription()}`); @@ -422,6 +491,15 @@ class TestCase { } // Validators +const queueSizeStore = {}; + +function extractQueueSize(responses) { + const allText = responses.map(r => r.text).join(' '); + const match = allText.match(/\b(\d+)\b/); + if (!match) return null; + return parseInt(match[1], 10); +} + const validators = { containsText: (text) => (responses) => { const allText = responses.map(r => r.text).join(' '); @@ -472,6 +550,24 @@ const validators = { return true; } return `Response should NOT contain "${text}"`; + }, + + recordQueueSize: (key) => (responses) => { + const size = extractQueueSize(responses); + if (size === null) return 'Could not parse queue size from response'; + queueSizeStore[key] = size; + return true; + }, + + queueSizeIncreaseFrom: (key, minIncrease = 1) => (responses) => { + const size = extractQueueSize(responses); + if (size === null) return 'Could not parse queue size from response'; + const baseline = queueSizeStore[key]; + if (baseline === undefined || baseline === null) { + return `No baseline queue size recorded for "${key}"`; + } + if (size >= baseline + minIncrease) return true; + return `Expected queue size to increase by ${minIncrease} from ${baseline}, got ${size}`; } }; @@ -700,6 +796,65 @@ const testSuiteArray = [ 7 ), + new TestCase( + 'Queue Size - Baseline Before Album', + 'size', + validators.and( + validators.responseCount(1, 2), + validators.recordQueueSize('beforeAlbum') + ), + 4 + ), + + new TestCase( + 'Add Album - Abbey Road', + 'addalbum abbey road', + validators.and( + validators.responseCount(1, 3), + validators.or( + validators.containsText('queue'), + validators.containsText('added'), + validators.containsText('Added') + ) + ), + 10 + ), + + new TestCase( + 'Queue Size - After Album', + 'size', + validators.and( + validators.responseCount(1, 2), + validators.queueSizeIncreaseFrom('beforeAlbum', 1), + validators.recordQueueSize('beforePlaylist') + ), + 4 + ), + + new TestCase( + 'Add Playlist - Rock Classics', + 'addplaylist rock classics', + validators.and( + validators.responseCount(1, 3), + validators.or( + validators.containsText('queue'), + validators.containsText('added'), + validators.containsText('Added') + ) + ), + 12 + ), + + new TestCase( + 'Queue Size - After Playlist', + 'size', + validators.and( + validators.responseCount(1, 2), + validators.queueSizeIncreaseFrom('beforePlaylist', 1) + ), + 4 + ), + new TestCase( 'Best Of Command', 'bestof led zeppelin 3', @@ -773,7 +928,7 @@ const testSuiteArray = [ validators.containsText('flushvote') ) ), - 3 + 5 ), new TestCase( @@ -1425,25 +1580,47 @@ async function runTestSuite() { for (const test of testSuite) { process.stdout.write(`${test.name}... `); - + const result = await test.run(); - + + let finalResult = result; + let retried = false; + + // If test failed, wait 10 seconds and retry once + if (!result) { + console.log(`❌ FAIL - Retrying in 10s...`); + await new Promise(resolve => setTimeout(resolve, 10000)); + + process.stdout.write(`${test.name} (RETRY)... `); + finalResult = await test.run(true); + retried = true; + } + // Log timing data timingLog.tests.push({ name: test.name, command: test.command, channel: test.targetChannel === adminChannelId ? 'admin' : 'standard', - passed: result, + passed: finalResult, timing: test.timing || { totalTime: null, firstResponseTime: null, responseCount: 0 }, responseCount: test.responses.length, - error: test.error || null + error: test.error || null, + retried: retried, + retrySucceeded: retried && finalResult }); - - if (result) { - console.log('✅ PASS'); + + if (finalResult) { + if (retried) { + console.log('✅ PASS (after retry)'); + } else { + console.log('✅ PASS'); + } passed++; } else { console.log(`❌ FAIL`); + if (retried) { + console.log(` Still failed after retry`); + } console.log(` Error: ${test.error}`); if (verbose) { console.log(` Expected: ${test.getExpectedDescription()}`); @@ -1499,11 +1676,18 @@ async function runTestSuite() { console.error(`\n⚠️ Failed to save timing log: ${err.message}`); } + // Calculate retry statistics + const retriedTests = timingLog.tests.filter(t => t.retried).length; + const retrySucceeded = timingLog.tests.filter(t => t.retrySucceeded).length; + console.log('\n' + '─'.repeat(60)); console.log('📊 Test Results:'); console.log(` ✅ Passed: ${passed}/${testSuite.length}`); console.log(` ❌ Failed: ${failed}/${testSuite.length}`); console.log(` 📈 Success Rate: ${Math.round((passed / testSuite.length) * 100)}%`); + if (retriedTests > 0) { + console.log(` 🔄 Retried: ${retriedTests} test(s) (${retrySucceeded} succeeded after retry)`); + } console.log(` ⏱️ Total Test Time: ${(totalTime / 1000).toFixed(2)}s (wall clock: ${(wallClockTime / 1000).toFixed(2)}s)`); // Compare with previous run if available @@ -1579,6 +1763,9 @@ async function runTestSuite() { console.log('─'.repeat(60)); + // Post results to admin channel + await postResultsToAdminChannel(passed, failed, testSuite.length, totalTime, wallClockTime, timingLog, previousTimingLog); + if (failed > 0) { console.log('\n⚠️ Some tests failed. Check that SlackONOS bot is running.'); process.exit(1); @@ -1588,6 +1775,83 @@ async function runTestSuite() { } } +/** + * Post test results summary to the admin Slack channel + */ +async function postResultsToAdminChannel(passed, failed, total, totalTime, wallClockTime, timingLog, previousTimingLog) { + try { + const successRate = Math.round((passed / total) * 100); + const statusEmoji = failed === 0 ? '✅' : '⚠️'; + const statusText = failed === 0 ? 'All tests passed!' : `${failed} test(s) failed`; + + // Calculate retry statistics + const retriedTests = timingLog.tests.filter(t => t.retried).length; + const retrySucceeded = timingLog.tests.filter(t => t.retrySucceeded).length; + + let retryInfo = ''; + if (retriedTests > 0) { + retryInfo = `\n*Retries:* ${retriedTests} test(s) retried (${retrySucceeded} succeeded)`; + } + + // Build failed tests list if any + let failedTestsList = ''; + if (failed > 0 && timingLog && timingLog.tests) { + const failedTests = timingLog.tests + .filter(t => !t.passed) + .map(t => { + const retryNote = t.retried ? ' (failed after retry)' : ''; + return `• ${t.name}${retryNote}`; + }) + .slice(0, 10); // Limit to 10 failed tests + + if (failedTests.length > 0) { + failedTestsList = `\n\n*Failed Tests:*\n${failedTests.join('\n')}`; + if (timingLog.tests.filter(t => !t.passed).length > 10) { + failedTestsList += `\n_...and ${timingLog.tests.filter(t => !t.passed).length - 10} more_`; + } + } + } + + // Build time comparison if previous run data exists + let timeComparison = ''; + if (previousTimingLog && previousTimingLog.tests) { + const previousTotalTime = previousTimingLog.tests.reduce((sum, test) => { + return sum + (test.timing?.totalTime || 0); + }, 0); + + if (previousTotalTime > 0) { + const timeDiff = totalTime - previousTotalTime; + const timeDiffPercent = ((timeDiff / previousTotalTime) * 100); + const timeDiffSymbol = timeDiff < -500 ? '⬇️' : timeDiff > 500 ? '⬆️' : '➡️'; + const timeDiffSign = timeDiff >= 0 ? '+' : ''; + + timeComparison = `\n*vs Previous:* ${timeDiffSymbol} ${timeDiffSign}${(timeDiff / 1000).toFixed(2)}s (${timeDiffSign}${timeDiffPercent.toFixed(1)}%)`; + } + } + + const message = `${statusEmoji} *Integration Test Results*\n\n` + + `*Status:* ${statusText}\n` + + `*Passed:* ${passed}/${total}\n` + + `*Failed:* ${failed}/${total}\n` + + `*Success Rate:* ${successRate}%\n` + + `*Duration:* ${(totalTime / 1000).toFixed(2)}s (wall clock: ${(wallClockTime / 1000).toFixed(2)}s)` + + retryInfo + + timeComparison + + failedTestsList; + + await slack.chat.postMessage({ + channel: adminChannelId, + text: message, + unfurl_links: false, + unfurl_media: false + }); + + console.log(`\n📨 Results posted to admin channel (${adminChannelId})`); + } catch (error) { + console.error(`\n⚠️ Failed to post results to admin channel: ${error.message}`); + } +} + // Run runTestSuite().catch(error => { console.error('💥 Fatal error:', error.message); diff --git a/test/voting.test.mjs b/test/voting.test.mjs index 6487e0b0..8a418645 100644 --- a/test/voting.test.mjs +++ b/test/voting.test.mjs @@ -635,8 +635,8 @@ describe('Voting Module (voting.js)', function() { beforeEach(function() { // Clear module cache to get fresh module state - delete require.cache[require.resolve('../voting.js')]; - voting = require('../voting.js'); + delete require.cache[require.resolve('../lib/voting.js')]; + voting = require('../lib/voting.js'); messages = []; userActions = []; @@ -684,15 +684,15 @@ describe('Voting Module (voting.js)', function() { describe('initialize', function() { it('should throw if logger not provided', function() { - delete require.cache[require.resolve('../voting.js')]; - const freshVoting = require('../voting.js'); + delete require.cache[require.resolve('../lib/voting.js')]; + const freshVoting = require('../lib/voting.js'); expect(() => freshVoting.initialize({})).to.throw('Voting module requires a logger'); }); it('should accept minimal dependencies', function() { - delete require.cache[require.resolve('../voting.js')]; - const freshVoting = require('../voting.js'); + delete require.cache[require.resolve('../lib/voting.js')]; + const freshVoting = require('../lib/voting.js'); expect(() => freshVoting.initialize({ logger: mockDeps.logger })).to.not.throw(); }); @@ -906,8 +906,8 @@ describe('Voting Module (voting.js)', function() { }); it('should show no immune tracks message when empty', async function() { - delete require.cache[require.resolve('../voting.js')]; - const freshVoting = require('../voting.js'); + delete require.cache[require.resolve('../lib/voting.js')]; + const freshVoting = require('../lib/voting.js'); freshVoting.initialize(mockDeps); await freshVoting.listImmune('C123'); From 005d6bd7bcd8476fa25903e0541c1de24a3a43bf Mon Sep 17 00:00:00 2001 From: htilly Date: Tue, 27 Jan 2026 18:42:03 +0100 Subject: [PATCH 2/2] fix: Address Copilot review suggestions - Pin Node.js to 20.20.0 (meets posthog-node engine requirement) - Fix Slack RTM -> Socket Mode in AI generator prompt - Remove duplicate playlist queueing (was queuing twice) - Fix heredoc variable expansion in workflow - Comment out unused utils import - Add githubToken to setconfig with sensitive masking - Improve featurerequest error message with setup instructions --- .github/agent/generate-implementation.mjs | 2 +- .github/workflows/feature-request-enhance.yml | 8 +++--- index.js | 25 ++++++++++++++++--- lib/add-handlers.js | 22 ++-------------- 4 files changed, 28 insertions(+), 29 deletions(-) diff --git a/.github/agent/generate-implementation.mjs b/.github/agent/generate-implementation.mjs index 3da0a6c2..b0c4b5f1 100644 --- a/.github/agent/generate-implementation.mjs +++ b/.github/agent/generate-implementation.mjs @@ -112,7 +112,7 @@ Your task is to analyze the feature request and generate a concrete implementati Project context: - Node.js application -- Uses Slack RTM API and Discord.js +- Uses Slack Socket Mode / Events API (via @slack/socket-mode) and Discord.js - Controls Sonos speakers - Has voting system for democratic music control - Uses AI for natural language commands diff --git a/.github/workflows/feature-request-enhance.yml b/.github/workflows/feature-request-enhance.yml index 56a7cd67..a3197ab9 100644 --- a/.github/workflows/feature-request-enhance.yml +++ b/.github/workflows/feature-request-enhance.yml @@ -88,7 +88,7 @@ jobs: - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: '20.20.0' cache: npm - name: Install dependencies @@ -157,7 +157,7 @@ jobs: - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: '20.20.0' cache: npm - name: Install dependencies @@ -267,7 +267,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v4 with: - node-version: 20 + node-version: '20.20.0' cache: npm - name: Install agent dependencies @@ -312,7 +312,7 @@ jobs: git add "$IMPL_FILE" # Create commit with heredoc to handle multiline - git commit -m "$(cat <<'EOF' + git commit -m "$(cat < 80 ? '…' : ''}\`\nNew: \`${newValue.slice(0, 80)}${newValue.length > 80 ? '…' : ''}\``, channel); + // Mask sensitive values (like tokens) + if (configDef.sensitive) { + const maskedValue = newValue.slice(0, 4) + '****' + newValue.slice(-4); + _slackMessage(`✅ Successfully updated \`${actualKey}\` and saved to config.\nNew: \`${maskedValue}\` (${newValue.length} chars)`, channel); + } else { + _slackMessage(`✅ Successfully updated \`${actualKey}\` and saved to config.\nOld: \`${oldValue.slice(0, 80)}${oldValue.length > 80 ? '…' : ''}\`\nNew: \`${newValue.slice(0, 80)}${newValue.length > 80 ? '…' : ''}\``, channel); + } }); } else if (configDef.type === 'boolean') { const lowerValue = value.toLowerCase(); diff --git a/lib/add-handlers.js b/lib/add-handlers.js index a72c26ac..6eaae714 100644 --- a/lib/add-handlers.js +++ b/lib/add-handlers.js @@ -579,28 +579,10 @@ async function addplaylist(input, channel, userName) { logger.info(`Sending playlist confirmation message: ${text}`); sendMessage(text, channel, { trackName: result.name }); - // Queue tracks in background (don't block user response) + // Note: Queueing is already done synchronously above (lines 532-545) + // This background task only handles playback if needed (async () => { try { - if (blacklistedTracks.length > 0) { - const allowedTracks = playlistTracks.filter(track => - !isTrackBlacklisted(track.name, track.artist) - ); - - const queuePromises = allowedTracks.map(track => - sonos.queue(track.uri).catch(err => { - logger.warn(`Could not queue track ${track.name}: ${err.message}`); - return null; - }) - ); - - await Promise.allSettled(queuePromises); - logger.info(`Added ${allowedTracks.length} tracks from playlist (filtered ${blacklistedTracks.length})`); - } else { - await sonos.queue(result.uri); - logger.info('Added playlist: ' + result.name); - } - if (isStopped) { await new Promise(resolve => setTimeout(resolve, 300)); await sonos.play();