From dc52496c2e9f36cfce42f9085733dcb14f39c1b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 26 May 2026 06:56:37 +0000 Subject: [PATCH 1/2] config: demote magic-constant keys to module-local constants (closes #442) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three config keys existed because the original code reached for getNumber() instead of declaring a constant — not because operators ever tuned them. Each surfaced in the Settings UI, demanded label/ description/type metadata, and added noise to the operator surface for zero practical benefit. Demoted (config key → module-local const): quotes.cleanup_interval → quote-channel-manager.ts CLEANUP_INTERVAL_MINUTES = 5 notices.cleanup_interval → notices-channel-manager.ts CLEANUP_INTERVAL_MINUTES = 5 voicechannels.ownership.grace_period_seconds → voice-channel-manager.ts OWNERSHIP_TRANSFER_GRACE_SECONDS = 30 Each module-local const carries a short comment explaining why the value is here and not in the schema, citing #442 so the demotion is self-documenting. Kept: quotes.max_length. Operators genuinely have preferences about quote length — some communities want short pithy quotes, some allow longer. Not a magic constant. Schema: removed the three keys from ConfigSchema, defaultConfig, and settingsMetadata. ConfigService.cleanupUnknownSettings() already runs on startup and deletes orphan DB rows for any key not in defaultConfig, so existing deployments shed the rows on next start with no extra migration code. All 744 tests pass. --- src/services/config-schema.ts | 27 ------------------------- src/services/notices-channel-manager.ts | 16 +++++++-------- src/services/quote-channel-manager.ts | 16 +++++++-------- src/services/voice-channel-manager.ts | 16 +++++++-------- 4 files changed, 24 insertions(+), 51 deletions(-) diff --git a/src/services/config-schema.ts b/src/services/config-schema.ts index 64dfcd49..b4000d9f 100644 --- a/src/services/config-schema.ts +++ b/src/services/config-schema.ts @@ -7,7 +7,6 @@ export interface ConfigSchema { "voicechannels.channel.prefix": string; "voicechannels.channel.suffix": string; "voicechannels.controlpanel.enabled": boolean; - "voicechannels.ownership.grace_period_seconds": number; "voicechannels.presets.enabled": boolean; "voicechannels.presets.max_per_user": number; @@ -37,7 +36,6 @@ export interface ConfigSchema { "quotes.delete_roles": string; // Comma-separated role IDs "quotes.max_length": number; // Maximum quote length "quotes.cooldown": number; // Cooldown in seconds between quote additions - "quotes.cleanup_interval": number; // Cleanup interval in minutes (default: 5) "quotes.header_enabled": boolean; // Enable header post in quote channel "quotes.header_message_id": string; // Message ID of the header post "quotes.header_pin_enabled": boolean; // Pin the header post @@ -69,7 +67,6 @@ export interface ConfigSchema { // Notices System "notices.enabled": boolean; "notices.channel_id": string; // Channel ID for notice messages - "notices.cleanup_interval": number; // Cleanup interval in minutes "notices.header_enabled": boolean; // Enable header post in notices channel "notices.header_message_id": string; // Message ID of the header post "notices.header_pin_enabled": boolean; // Pin the header post @@ -96,7 +93,6 @@ export const defaultConfig: ConfigSchema = { "voicechannels.channel.prefix": "🎮", "voicechannels.channel.suffix": "", "voicechannels.controlpanel.enabled": true, - "voicechannels.ownership.grace_period_seconds": 30, "voicechannels.presets.enabled": false, "voicechannels.presets.max_per_user": 3, @@ -126,7 +122,6 @@ export const defaultConfig: ConfigSchema = { "quotes.delete_roles": "", // Empty means only admins can delete "quotes.max_length": 1000, "quotes.cooldown": 60, - "quotes.cleanup_interval": 5, // Clean up unauthorized messages every 5 minutes "quotes.header_enabled": true, // Enable informational header post "quotes.header_message_id": "", // Stores header message ID "quotes.header_pin_enabled": true, // Pin header for easy access @@ -157,7 +152,6 @@ export const defaultConfig: ConfigSchema = { // Notices System defaults "notices.enabled": false, "notices.channel_id": "", - "notices.cleanup_interval": 5, // Clean up unauthorized messages every 5 minutes "notices.header_enabled": true, // Enable informational header post "notices.header_message_id": "", // Stores header message ID "notices.header_pin_enabled": true, // Pin header for easy access @@ -347,13 +341,6 @@ export const settingsMetadata: Record = { category: "voicechannels", type: "boolean", }, - "voicechannels.ownership.grace_period_seconds": { - label: "Ownership transfer grace period (seconds)", - description: - "Seconds to wait before transferring ownership when the channel owner leaves.", - category: "voicechannels", - type: "number", - }, "voicechannels.presets.enabled": { label: "Per-user channel presets enabled", description: @@ -493,13 +480,6 @@ export const settingsMetadata: Record = { category: "quotes", type: "number", }, - "quotes.cleanup_interval": { - label: "Channel cleanup interval (minutes)", - description: - "Interval in minutes between sweeps for unauthorised messages in the quote channel.", - category: "quotes", - type: "number", - }, "quotes.header_enabled": { label: "Pinned header post enabled", description: @@ -617,13 +597,6 @@ export const settingsMetadata: Record = { category: "notices", type: "channel", }, - "notices.cleanup_interval": { - label: "Channel cleanup interval (minutes)", - description: - "Interval in minutes between sweeps for unauthorised messages in the notices channel.", - category: "notices", - type: "number", - }, "notices.header_enabled": { label: "Pinned header post enabled", description: diff --git a/src/services/notices-channel-manager.ts b/src/services/notices-channel-manager.ts index 18ec6c1d..6d543c7e 100644 --- a/src/services/notices-channel-manager.ts +++ b/src/services/notices-channel-manager.ts @@ -5,6 +5,12 @@ import logger from "../utils/logger.js"; import Notice, { INotice } from "../models/notice.js"; import { NOTICE_CATEGORIES } from "../content/notice-categories.js"; +// Internal sweep interval for purging unauthorised messages from the +// notices channel. Demoted from `notices.cleanup_interval` config key in +// #442 — operators never tuned it; the cadence is an implementation +// detail of the channel-cleanup loop. +const CLEANUP_INTERVAL_MINUTES = 5; + export class NoticesChannelManager { private static instance: NoticesChannelManager; private client: Client; @@ -394,14 +400,8 @@ export class NoticesChannelManager { } private async startCleanupJob(): Promise { - // Get cleanup interval from config (in minutes) - const intervalMinutes = await this.configService.getNumber( - "notices.cleanup_interval", - 5, - ); - // Convert to cron expression (*/N * * * * means every N minutes) - const cronExpression = `*/${intervalMinutes} * * * *`; + const cronExpression = `*/${CLEANUP_INTERVAL_MINUTES} * * * *`; this.cleanupJob = new CronJob( cronExpression, @@ -414,7 +414,7 @@ export class NoticesChannelManager { ); logger.info( - `Started notices channel cleanup job (every ${intervalMinutes} minutes)`, + `Started notices channel cleanup job (every ${CLEANUP_INTERVAL_MINUTES} minutes)`, ); } diff --git a/src/services/quote-channel-manager.ts b/src/services/quote-channel-manager.ts index 0111451f..45b94932 100644 --- a/src/services/quote-channel-manager.ts +++ b/src/services/quote-channel-manager.ts @@ -12,6 +12,12 @@ import { ConfigService } from "./config-service.js"; import logger from "../utils/logger.js"; import { quoteService } from "./quote-service.js"; +// Internal sweep interval for purging unauthorised messages from the +// quote channel. Demoted from `quotes.cleanup_interval` config key in +// #442 — operators never tuned it; the cadence is an implementation +// detail of the channel-cleanup loop. +const CLEANUP_INTERVAL_MINUTES = 5; + /** * Normalize a Discord user ID from various formats to a clean numeric ID * Handles: <@123>, <@!123>, @username, or plain 123 @@ -375,14 +381,8 @@ export class QuoteChannelManager { logger.debug("Stopped existing cleanup job before starting new one"); } - // Get cleanup interval from config (in minutes) - const intervalMinutes = await this.configService.getNumber( - "quotes.cleanup_interval", - 5, - ); - // Convert to cron expression (*/N * * * * means every N minutes) - const cronExpression = `*/${intervalMinutes} * * * *`; + const cronExpression = `*/${CLEANUP_INTERVAL_MINUTES} * * * *`; this.cleanupJob = new CronJob( cronExpression, @@ -395,7 +395,7 @@ export class QuoteChannelManager { ); logger.info( - `Started quote channel cleanup job (every ${intervalMinutes} minutes)`, + `Started quote channel cleanup job (every ${CLEANUP_INTERVAL_MINUTES} minutes)`, ); } diff --git a/src/services/voice-channel-manager.ts b/src/services/voice-channel-manager.ts index 54a9f08d..8592ea3c 100644 --- a/src/services/voice-channel-manager.ts +++ b/src/services/voice-channel-manager.ts @@ -27,6 +27,12 @@ const configService = ConfigService.getInstance(); // leaving stale entries behind (issue #404, follow-up to #339). const UNKNOWN_CHANNEL_ERROR_CODE = 10003; +// Seconds to wait before transferring ownership when the channel owner +// leaves. Demoted from `voicechannels.ownership.grace_period_seconds` +// config key in #442 — operators never tuned it; it's an internal pacing +// constant for the ownership-handoff state machine. +const OWNERSHIP_TRANSFER_GRACE_SECONDS = 30; + function isUnknownChannelError(error: unknown): boolean { return ( error instanceof DiscordAPIError && @@ -432,12 +438,6 @@ export class VoiceChannelManager { return; } - // Get grace period from config - const gracePeriodSeconds = await this.configService.getNumber( - "voicechannels.ownership.grace_period_seconds", - 30, - ); - // Check if there's already a pending ownership transfer for this channel if (this.ownershipTransferTimers.has(channel.id)) { logger.info( @@ -447,7 +447,7 @@ export class VoiceChannelManager { } logger.info( - `Owner ${currentOwnerId} left channel ${channel.name}. Scheduling ownership transfer in ${gracePeriodSeconds} seconds...`, + `Owner ${currentOwnerId} left channel ${channel.name}. Scheduling ownership transfer in ${OWNERSHIP_TRANSFER_GRACE_SECONDS} seconds...`, ); // Schedule ownership transfer after grace period @@ -516,7 +516,7 @@ export class VoiceChannelManager { logger.error("Error during scheduled ownership transfer:", error); this.ownershipTransferTimers.delete(channel.id); } - }, gracePeriodSeconds * 1000); + }, OWNERSHIP_TRANSFER_GRACE_SECONDS * 1000); // Store the timer and original owner info this.ownershipTransferTimers.set(channel.id, { From 64c286f01b98476439cf8825e57fb1b79ac0022a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 26 May 2026 08:11:14 +0000 Subject: [PATCH 2/2] docs: drop demoted keys from SETTINGS.md (review on #467) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review caught that SETTINGS.md still documented the three keys this PR demoted. Anyone consulting the doc would be told to configure keys that no longer exist in defaultConfig — cleanupUnknownSettings() silently deletes them on startup, and the YAML import path now rejects them as "unknown key". Removed from SETTINGS.md: - The quotes settings table row and follow-up note for quotes.cleanup_interval. - The notices settings table row for notices.cleanup_interval. - The voicechannels.ownership.grace_period_seconds table row plus the entire "Ownership grace period" subsection with its "Recommended values" guidance. - The four Quick-reference entries for the same three keys. The constants now live in the relevant service modules; tuning them is a code change, not a config change. --- SETTINGS.md | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/SETTINGS.md b/SETTINGS.md index e173c3ca..281529d5 100644 --- a/SETTINGS.md +++ b/SETTINGS.md @@ -178,7 +178,6 @@ Configure the quote management system. | `quotes.enabled` | `false` | Enable/disable the quote system | | `quotes.channel_id` | `""` | Channel ID for quote messages (empty = use command channel) | | `quotes.cooldown` | `60` | Seconds between quote additions (per user) | -| `quotes.cleanup_interval` | `5` | Minutes between cleanup of unauthorized quote messages | | `quotes.max_length` | `1000` | Maximum character length for quotes | | `quotes.add_roles` | `""` | Role IDs allowed to add quotes (comma-separated, empty = all) | | `quotes.delete_roles` | `""` | Role IDs allowed to delete quotes (comma-separated, empty = admins only) | @@ -190,8 +189,6 @@ Configure the quote management system. - `quotes.channel_id` — If set, all quotes are posted to this channel. If empty, quotes post in the channel where the command was used. -- `quotes.cleanup_interval` — Controls how often unauthorized messages - in the quote channel are removed. - `quotes.header_enabled` — Displays a pinned informational header. Auto-recreated if deleted. - `quotes.header_pin_enabled` — Controls whether the header is pinned. @@ -210,7 +207,6 @@ the channel itself. | --- | --- | --- | | `notices.enabled` | `false` | Enable/disable the notices system | | `notices.channel_id` | `""` | Channel ID for notice messages | -| `notices.cleanup_interval` | `5` | Minutes between cleanup of unauthorized messages | | `notices.header_enabled` | `true` | Show informational header post in notices channel | | `notices.header_pin_enabled` | `true` | Pin the header post for easy access | | `notices.header_message_id` | `""` | Stores header message ID (managed automatically) | @@ -312,19 +308,6 @@ Dynamic voice channel creation and management. | `voicechannels.channel.prefix` | `"🎮"` | Prefix for user-created channels | | `voicechannels.channel.suffix` | `""` | Suffix for user-created channels | | `voicechannels.controlpanel.enabled` | `true` | Show interactive control panel in channel text chat | -| `voicechannels.ownership.grace_period_seconds` | `30` | Grace period before transferring ownership when owner disconnects | - -### Ownership grace period - -When a channel owner disconnects (e.g., network issue, client restart), -the bot waits for the grace period before transferring ownership. If -the owner rejoins within this time, they retain ownership. - -**Recommended values:** - -- `30` (default) — Good balance for most servers -- `60` — Servers with frequent connection issues -- `10` — For faster ownership transfers ### Manual cleanup @@ -747,7 +730,7 @@ The Web UI form controls map to the underlying schema types: This is a selected subset — the authoritative list of every key (with defaults and metadata) lives in `src/services/config-schema.ts`. Notable keys that may be missing from this summary include the `help.*` toggle, -`voicechannels.presets.*`, `quotes.cleanup_interval`, the +`voicechannels.presets.*`, the `*.header_message_id` storage slots managed by the bot, and the `leaderboard_roles.*` family (covered in [its own section](#-leaderboard-role-rewards) above). @@ -778,7 +761,6 @@ above). - `quotes.channel_id` (string, default: "") - `quotes.cooldown` (number, default: 60) -- `quotes.cleanup_interval` (number, default: 5) - `quotes.max_length` (number, default: 1000) - `quotes.add_roles` (string, default: "") - `quotes.delete_roles` (string, default: "") @@ -789,7 +771,6 @@ above). - `notices.enabled` (bool, default: false) - `notices.channel_id` (string, default: "") -- `notices.cleanup_interval` (number, default: 5) - `notices.header_enabled` (bool, default: true) - `notices.header_pin_enabled` (bool, default: true) @@ -808,7 +789,6 @@ above). - `voicechannels.channel.prefix` (string, default: "🎮") - `voicechannels.channel.suffix` (string, default: "") - `voicechannels.controlpanel.enabled` (bool, default: true) -- `voicechannels.ownership.grace_period_seconds` (number, default: 30) #### Voice Tracking