v2.3.0: Consolidate modules, add handlers, improve tests by htilly · Pull Request #229 · htilly/SlackONOS

htilly · 2026-01-27T17:28:36Z

Summary

Consolidate modules: Move all root JS files to lib/ folder for cleaner project structure
Extract add handlers: New lib/add-handlers.js with add/addalbum/addplaylist/append commands
Security fixes: Add Spotify null guards in search functions (Copilot suggestion)
Code quality: Replace .map() with .forEach() for side effects
Dependency update: Bump posthog-node to 5.24.2

Test improvements

Add integration tests for addalbum and addplaylist with queue size verification
Fix test runner to detect threaded bot responses
Add rate-limit handling to avoid Slack API throttling
Add automatic retry with 10s backoff on test failure

Changes

22 files changed, +2076 -926 lines
10 modules moved to lib/
42 new unit tests for add-handlers
5 new integration tests

Test plan

All 553 unit tests pass (npm test)
Integration tests pass (npm run test:integration)
Manual verification of bot commands

Note

Introduces AI-assisted PR creation and refactors command handling.

CI/Automation: New feature-request-enhance.yml workflow adds preprocessing, optional Confluence page, Claude-powered implementation generation, and auto-PR creation against develop; pins Node to 20.20.0 and adjusts permissions. Adds .github/AI_PR_SETUP.md and agent/generate-implementation.mjs.
Core refactor: Moves many root imports to lib/*; extracts add, addalbum, addplaylist, append into new lib/add-handlers.js and wires them into index.js command registry; cleans up legacy implementations and updates references to lib/discord, lib/slack, etc.
Config/UX: Adds githubToken config (masked on save) and improves featurerequest guidance.
Tests: Adds unit tests for add-handlers; updates paths for moved modules; enhances Slack integration tests (thread replies, rate-limit handling, queue-size tracking, retry-on-fail).
Deps: Bumps posthog-node to 5.24.2 (and @posthog/core), minor forEach/cosmetic fixes.

^{Written by Cursor Bugbot for commit 005d6bd. This will update automatically on new commits. Configure here.}

- 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

Copilot

Pull request overview

This PR restructures the project by moving core modules into lib/, extracting the “add*” command logic into a dedicated module, and expanding/strengthening both unit and integration test coverage around queueing behavior.

Changes:

Move root JS modules into lib/ and update imports throughout the app and tests.
Extract add, addalbum, addplaylist, and append into lib/add-handlers.js and add a comprehensive unit test suite for it.
Enhance the Slack-based integration test runner with threaded-response detection, rate-limit handling, queue-size assertions, and retry-on-failure behavior.

Reviewed changes

Copilot reviewed 11 out of 22 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
test/voting.test.mjs	Updates module import path to the new `lib/` location.
test/tools/integration-test-suite.mjs	Improves integration test robustness (threads, rate limits, retries, queue-size assertions, admin posting).
test/ai-handler.test.mjs	Updates AI handler import path to the new `lib/` location.
test/add-handlers.test.mjs	Adds unit tests covering the extracted add/addalbum/addplaylist/append behavior.
package.json	Bumps `posthog-node` dependency.
package-lock.json	Locks updated `posthog-node` and transitive dependencies; updates Node engine constraints.
lib/voting.js	New `lib/`-scoped voting module (gong/vote/flushvote, etc.).
lib/utils.js	New `lib/`-scoped utilities module.
lib/telemetry.js	Adds/relocates telemetry module backed by PostHog.
lib/spotify.js	Adds/relocates Spotify wrapper used by the app.
lib/soundcraft-handler.js	Adds/relocates Soundcraft integration handler.
lib/slack.js	Adds/relocates Slack Socket Mode integration and reaction handling.
lib/music-helper.js	Adds/relocates music helper logic (search/boosters/theme mixing/queueing).
lib/logger.js	Adds/relocates Winston wrapper required by SocketModeClient.
lib/discord.js	Adds/relocates Discord integration and reaction handling.
lib/command-handlers.js	Refactors queue display iteration and adds Spotify-null guards for search handlers.
lib/ai-handler.js	Adds/relocates OpenAI-based natural language parsing + context tracking.
lib/add-handlers.js	New extracted add/addalbum/addplaylist/append implementation.
index.js	Repoints imports to `lib/` and wires extracted add-handlers into the command registry.
.github/workflows/feature-request-enhance.yml	Extends automation to generate implementation plans + create PRs from issues.
.github/agent/generate-implementation.mjs	Adds a Claude-based implementation-plan generator script.
.github/AI_PR_SETUP.md	Documents how to configure and use the AI PR automation.

Copilot · 2026-01-27T17:36:23Z

.github/workflows/feature-request-enhance.yml

+      - name: Setup Node.js
+        uses: actions/setup-node@v4
+        with:
+          node-version: 20
+          cache: npm


actions/setup-node is configured with node-version: 20, but posthog-node declares an engine requirement of ^20.20.0 || >=22.22.0 (per package-lock). Pin the workflow to 20.20.0 (or bump to a supported 22.x/24.x) to avoid CI/runtime incompatibilities as the default "20" changes over time.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-01-27T17:36:23Z

.github/agent/generate-implementation.mjs

+
+Project context:
+- Node.js application
+- Uses Slack RTM API and Discord.js


The system prompt says the project uses the Slack RTM API, but the codebase uses Socket Mode (@slack/socket-mode). This will mislead the generator and can produce incorrect implementation guidance; update the prompt to reflect Socket Mode / Events API usage.

Suggested change

- Uses Slack RTM API and Discord.js

- Uses Slack Socket Mode / Events API (via @slack/socket-mode) and Discord.js

Copilot · 2026-01-27T17:36:24Z

lib/add-handlers.js

+    // 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);
+    }
+


addplaylist queues the playlist twice: once synchronously (either sonos.queue(result.uri) or iterating allowedTracks) and then again in the background IIFE below. This will duplicate tracks in the Sonos queue. Consider choosing one approach (either do all queueing in the background and respond immediately, or keep the synchronous queueing and remove the background queueing block).

Suggested change

// 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);

}

// Queueing is handled asynchronously in the background worker/IIFE below.

// Avoid queueing here to prevent adding the playlist twice.

// (blacklistedTracks and playlistTracks remain available to that code.)

// NOTE: Previously this block synchronously queued the playlist via

// `sonos.queue(result.uri)` or by iterating `allowedTracks`. That logic

// has been removed to eliminate duplicate queuing; the background

// processing now owns all queuing responsibilities.

// Intentionally no direct calls to `sonos.queue` here.

Copilot · 2026-01-27T17:36:24Z

.github/workflows/feature-request-enhance.yml

+            # 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
+          )"


The git commit -m heredoc uses a single-quoted delimiter (<<'EOF'), so $ISSUE_NUMBER will not expand and the commit message will literally contain #$ISSUE_NUMBER. Use an unquoted heredoc (or build the message string another way) so variables expand correctly.

Copilot · 2026-01-27T17:36:24Z

index.js

-const Spotify = require('./spotify-async');
-const utils = require('./utils');
+const Spotify = require('./lib/spotify');
+const utils = require('./lib/utils');


Unused variable utils.

Suggested change

const utils = require('./lib/utils');

.github/workflows/feature-request-enhance.yml

Copilot · 2026-01-27T17:39:33Z

@htilly I've opened a new pull request, #230, to work on those changes. Once the pull request is ready, I'll request review from you.

- 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

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

cursor · 2026-01-27T17:51:37Z

lib/add-handlers.js

+      } catch (err) {
+        logger.error('Error in background playlist queueing: ' + err.message);
+      }
+    })();


Duplicate playback calls in addplaylist function

Low Severity

The addplaylist function contains redundant playback logic. Playback is started synchronously at lines 548-570, and then the same playback logic is repeated in a background async task at lines 584-597. Both sections check isStopped and state conditions and call sonos.play(), causing the Sonos speaker to receive duplicate play commands. The comment at line 582 acknowledges that queueing is already done but doesn't account for the fact that playback was also already started above.

Additional Locations (1)

lib/add-handlers.js#L547-L570

Copilot AI review requested due to automatic review settings January 27, 2026 17:28

Copilot started reviewing on behalf of htilly January 27, 2026 17:29 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

cursor bot reviewed Jan 27, 2026

View reviewed changes

.github/workflows/feature-request-enhance.yml Show resolved Hide resolved

Copilot AI mentioned this pull request Jan 27, 2026

[WIP] WIP address feedback on module consolidation and test improvements #230

Closed

3 tasks

cursor bot reviewed Jan 27, 2026

View reviewed changes

htilly merged commit 207c1c3 into master Jan 27, 2026
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

v2.3.0: Consolidate modules, add handlers, improve tests#229

v2.3.0: Consolidate modules, add handlers, improve tests#229
htilly merged 2 commits intomasterfrom
develop

htilly commented Jan 27, 2026 •

edited by cursor bot

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

htilly Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Uh oh!

Copilot AI commented Jan 27, 2026

Uh oh!

cursor bot left a comment

Uh oh!

cursor bot Jan 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	- Uses Slack RTM API and Discord.js
	- Uses Slack Socket Mode / Events API (via @slack/socket-mode) and Discord.js

Conversation

htilly commented Jan 27, 2026 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test improvements

Changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

htilly Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI commented Jan 27, 2026

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Jan 27, 2026

Choose a reason for hiding this comment

Duplicate playback calls in addplaylist function

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

htilly commented Jan 27, 2026 •

edited by cursor bot

Loading