Skip to content

Commit 005d6bd

Browse files
committed
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
1 parent 4ddada9 commit 005d6bd

File tree

4 files changed

+28
-29
lines changed

4 files changed

+28
-29
lines changed

.github/agent/generate-implementation.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ Your task is to analyze the feature request and generate a concrete implementati
112112
113113
Project context:
114114
- Node.js application
115-
- Uses Slack RTM API and Discord.js
115+
- Uses Slack Socket Mode / Events API (via @slack/socket-mode) and Discord.js
116116
- Controls Sonos speakers
117117
- Has voting system for democratic music control
118118
- Uses AI for natural language commands

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ jobs:
8888

8989
- uses: actions/setup-node@v4
9090
with:
91-
node-version: 20
91+
node-version: '20.20.0'
9292
cache: npm
9393

9494
- name: Install dependencies
@@ -157,7 +157,7 @@ jobs:
157157

158158
- uses: actions/setup-node@v4
159159
with:
160-
node-version: 20
160+
node-version: '20.20.0'
161161
cache: npm
162162

163163
- name: Install dependencies
@@ -267,7 +267,7 @@ jobs:
267267
- name: Setup Node.js
268268
uses: actions/setup-node@v4
269269
with:
270-
node-version: 20
270+
node-version: '20.20.0'
271271
cache: npm
272272

273273
- name: Install agent dependencies
@@ -312,7 +312,7 @@ jobs:
312312
git add "$IMPL_FILE"
313313
314314
# Create commit with heredoc to handle multiline
315-
git commit -m "$(cat <<'EOF'
315+
git commit -m "$(cat <<EOF
316316
Add implementation plan for issue #$ISSUE_NUMBER
317317
318318
This is an AI-generated implementation plan created by Claude.

index.js

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const googleTTS = require('@sefinek/google-tts-api');
3636
const config = require('nconf');
3737
const winston = require('winston');
3838
const Spotify = require('./lib/spotify');
39-
const utils = require('./lib/utils');
39+
// const utils = require('./lib/utils'); // Currently unused
4040
const process = require('process');
4141
const parseString = require('xml2js').parseString;
4242
const http = require('http');
@@ -4748,7 +4748,17 @@ async function _featurerequest(input, channel, userName) {
47484748
const githubToken = config.get('githubToken');
47494749
if (!githubToken) {
47504750
logger.warn('[FEATUREREQUEST] githubToken not configured');
4751-
_slackMessage('❌ Feature request functionality is not configured. Please set `githubToken` in the config to enable this feature.', channel);
4751+
_slackMessage(
4752+
'❌ *Feature request not configured*\n\n' +
4753+
'To enable this feature, you need a GitHub Personal Access Token:\n\n' +
4754+
'1. Go to: https://github.com/settings/tokens\n' +
4755+
'2. Click *"Generate new token (classic)"*\n' +
4756+
'3. Select scope: `repo` (or `public_repo` for public repos only)\n' +
4757+
'4. Set the token via admin command:\n' +
4758+
' `setconfig githubToken ghp_xxxxxxxxxxxx`\n\n' +
4759+
'📖 More info: https://github.com/htilly/SlackONOS#configuration',
4760+
channel
4761+
);
47524762
return;
47534763
}
47544764

@@ -4930,7 +4940,8 @@ async function _setconfig(input, channel, userName) {
49304940
soundcraftIp: { type: 'string', minLen: 0, maxLen: 50 },
49314941
crossfadeEnabled: { type: 'boolean' },
49324942
slackAlwaysThread: { type: 'boolean' },
4933-
logLevel: { type: 'string', minLen: 4, maxLen: 5, allowed: ['error', 'warn', 'info', 'debug'] }
4943+
logLevel: { type: 'string', minLen: 4, maxLen: 5, allowed: ['error', 'warn', 'info', 'debug'] },
4944+
githubToken: { type: 'string', minLen: 4, maxLen: 100, sensitive: true }
49344945
};
49354946

49364947
// Make config key case-insensitive
@@ -5056,7 +5067,13 @@ async function _setconfig(input, channel, userName) {
50565067
_slackMessage(`⚠️ Updated \`${actualKey}\` in memory, but failed to save to disk!`, channel);
50575068
return;
50585069
}
5059-
_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);
5070+
// Mask sensitive values (like tokens)
5071+
if (configDef.sensitive) {
5072+
const maskedValue = newValue.slice(0, 4) + '****' + newValue.slice(-4);
5073+
_slackMessage(`✅ Successfully updated \`${actualKey}\` and saved to config.\nNew: \`${maskedValue}\` (${newValue.length} chars)`, channel);
5074+
} else {
5075+
_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);
5076+
}
50605077
});
50615078
} else if (configDef.type === 'boolean') {
50625079
const lowerValue = value.toLowerCase();

lib/add-handlers.js

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -579,28 +579,10 @@ async function addplaylist(input, channel, userName) {
579579
logger.info(`Sending playlist confirmation message: ${text}`);
580580
sendMessage(text, channel, { trackName: result.name });
581581

582-
// Queue tracks in background (don't block user response)
582+
// Note: Queueing is already done synchronously above (lines 532-545)
583+
// This background task only handles playback if needed
583584
(async () => {
584585
try {
585-
if (blacklistedTracks.length > 0) {
586-
const allowedTracks = playlistTracks.filter(track =>
587-
!isTrackBlacklisted(track.name, track.artist)
588-
);
589-
590-
const queuePromises = allowedTracks.map(track =>
591-
sonos.queue(track.uri).catch(err => {
592-
logger.warn(`Could not queue track ${track.name}: ${err.message}`);
593-
return null;
594-
})
595-
);
596-
597-
await Promise.allSettled(queuePromises);
598-
logger.info(`Added ${allowedTracks.length} tracks from playlist (filtered ${blacklistedTracks.length})`);
599-
} else {
600-
await sonos.queue(result.uri);
601-
logger.info('Added playlist: ' + result.name);
602-
}
603-
604586
if (isStopped) {
605587
await new Promise(resolve => setTimeout(resolve, 300));
606588
await sonos.play();

0 commit comments

Comments
 (0)