CLAUDE: Refactor voice cleanup service to use @tasks.loop pattern

Problem:
- Voice cleanup service used manual while loop instead of @tasks.loop
- Did not wait for bot readiness before starting
- Startup verification could miss stale entries
- Manual channel deletions did not unpublish associated scorecards

Changes:
- Refactored VoiceChannelCleanupService to use @tasks.loop(minutes=1)
- Added @before_loop decorator with await bot.wait_until_ready()
- Updated bot.py to use setup_voice_cleanup() pattern
- Fixed scorecard unpublishing for manually deleted channels
- Fixed scorecard unpublishing for wrong channel type scenarios
- Updated cleanup interval from 60 seconds to 1 minute (same behavior)
- Changed cleanup reason message from "15+ minutes" to "5+ minutes" (matches actual threshold)

Benefits:
- Safe startup: cleanup waits for bot to be fully ready
- Reliable stale cleanup: startup verification guaranteed to run
- Complete cleanup: scorecards unpublished in all scenarios
- Consistent pattern: follows same pattern as other background tasks
- Better error handling: integrated with discord.py task lifecycle

Testing:
- All 19 voice command tests passing
- Updated test fixtures to handle new task pattern
- Fixed test assertions for new cleanup reason message

Documentation:
- Updated commands/voice/CLAUDE.md with new architecture
- Documented all four cleanup scenarios for scorecards
- Added task lifecycle information
- Updated configuration section

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Cal Corum 2025-10-24 00:05:35 -05:00
parent 950f3ed640
commit 32cb5da632
4 changed files with 171 additions and 103 deletions

10
bot.py
View File

@ -183,12 +183,8 @@ class SBABot(commands.Bot):
self.logger.info("✅ Transaction freeze/thaw task started")
# Initialize voice channel cleanup service
from commands.voice.cleanup_service import VoiceChannelCleanupService
self.voice_cleanup_service = VoiceChannelCleanupService()
# Start voice channel monitoring (includes startup verification)
import asyncio
asyncio.create_task(self.voice_cleanup_service.start_monitoring(self))
from commands.voice.cleanup_service import setup_voice_cleanup
self.voice_cleanup_service = setup_voice_cleanup(self)
self.logger.info("✅ Voice channel cleanup service started")
# Initialize live scorebug tracker
@ -345,7 +341,7 @@ class SBABot(commands.Bot):
if hasattr(self, 'voice_cleanup_service'):
try:
self.voice_cleanup_service.stop_monitoring()
self.voice_cleanup_service.cog_unload()
self.logger.info("Voice channel cleanup service stopped")
except Exception as e:
self.logger.error(f"Error stopping voice cleanup service: {e}")

View File

@ -60,12 +60,18 @@ This directory contains Discord slash commands for creating and managing voice c
- **Role Integration**: Finds Discord roles matching team full names (`team.lname`)
### Automatic Cleanup System
- **Monitoring Interval**: Configurable (default: 60 seconds)
- **Empty Threshold**: Configurable (default: 5 minutes empty before deletion)
- **Monitoring Interval**: 1 minute (using `@tasks.loop` pattern)
- **Empty Threshold**: 5 minutes empty before deletion
- **Restart Resilience**: JSON file persistence survives bot restarts
- **Startup Verification**: Validates tracked channels still exist on bot startup
- **Safe Startup**: Uses `@before_loop` to wait for bot readiness before starting
- **Startup Verification**: Validates tracked channels still exist and cleans stale entries on bot startup
- **Manual Deletion Handling**: Detects manually deleted channels and cleans up tracking
- **Graceful Error Handling**: Continues operation even if individual operations fail
- **Scorecard Cleanup**: Automatically unpublishes scorecards when associated voice channels are deleted
- **Scorecard Cleanup**: Automatically unpublishes scorecards in all cleanup scenarios:
- Normal cleanup (channel empty for 5+ minutes)
- Manual deletion (user deletes channel)
- Startup verification (stale entries on bot restart)
- Wrong channel type (corrupted tracking data)
## Architecture
@ -114,9 +120,11 @@ overwrites = {
### Cleanup Service Integration
```python
# Bot initialization (bot.py)
from commands.voice.cleanup_service import VoiceChannelCleanupService
self.voice_cleanup_service = VoiceChannelCleanupService()
asyncio.create_task(self.voice_cleanup_service.start_monitoring(self))
from commands.voice.cleanup_service import setup_voice_cleanup
self.voice_cleanup_service = setup_voice_cleanup(self)
# The service uses @tasks.loop pattern with @before_loop
# It automatically waits for bot readiness before starting
# Channel tracking
if hasattr(self.bot, 'voice_cleanup_service'):
@ -124,24 +132,44 @@ if hasattr(self.bot, 'voice_cleanup_service'):
cleanup_service.tracker.add_channel(channel, channel_type, interaction.user.id)
```
### Scorecard Cleanup Integration
When a voice channel is cleaned up (deleted after being empty for the configured threshold), the cleanup service automatically unpublishes any scorecard associated with that voice channel's text channel. This prevents the live scorebug tracker from continuing to update scores for games that no longer have active voice channels.
**Task Lifecycle**:
- **Initialization**: `VoiceChannelCleanupService(bot)` creates instance
- **Startup Wait**: `@before_loop` ensures bot is ready before first cycle
- **Verification**: First cycle runs `verify_tracked_channels()` to clean stale entries
- **Monitoring**: Runs every 1 minute checking all tracked channels
- **Shutdown**: `cog_unload()` cancels the cleanup loop gracefully
**Cleanup Flow**:
1. Voice channel becomes empty and exceeds empty threshold
2. Cleanup service deletes the voice channel
3. Service checks if voice channel has associated `text_channel_id`
4. If found, unpublishes scorecard from that text channel
5. Live scorebug tracker stops updating that scorecard
### Scorecard Cleanup Integration
The cleanup service automatically unpublishes any scorecard associated with a voice channel when that channel is removed from tracking. This prevents the live scorebug tracker from continuing to update scores for games that no longer have active voice channels.
**Cleanup Scenarios**:
1. **Normal Cleanup** (channel empty for 5+ minutes):
- Cleanup service deletes the voice channel
- Unpublishes associated scorecard
- Logs: `"📋 Unpublished scorecard from text channel [id] (voice channel cleanup)"`
2. **Manual Deletion** (user deletes channel):
- Next cleanup cycle detects missing channel
- Removes from tracking and unpublishes scorecard
- Logs: `"📋 Unpublished scorecard from text channel [id] (manually deleted voice channel)"`
3. **Startup Verification** (stale entries on bot restart):
- Bot startup detects channels that no longer exist
- Cleans up tracking and unpublishes scorecards
- Logs: `"📋 Unpublished scorecard from text channel [id] (stale voice channel)"`
4. **Wrong Channel Type** (corrupted tracking data):
- Tracked channel exists but is not a voice channel
- Removes from tracking and unpublishes scorecard
- Logs: `"📋 Unpublished scorecard from text channel [id] (wrong channel type)"`
**Integration Points**:
- `cleanup_service.py` imports `ScorecardTracker` from `commands.gameplay.scorecard_tracker`
- Scorecard unpublishing happens in three scenarios:
- Normal cleanup (channel deleted after being empty)
- Stale channel cleanup (channel already deleted externally)
- Startup verification (channel no longer exists when bot starts)
- All cleanup paths check for `text_channel_id` and unpublish if found
- Recovery time: Maximum 1 minute delay for manual deletions
**Logging**:
**Example Logging**:
```
✅ Cleaned up empty voice channel: Gameplay Phoenix (ID: 123456789)
📋 Unpublished scorecard from text channel 987654321 (voice channel cleanup)
@ -171,9 +199,10 @@ When a voice channel is cleaned up (deleted after being empty for the configured
## Configuration
### Cleanup Service Settings
- **`cleanup_interval`**: How often to check channels (default: 60 seconds)
- **`empty_threshold`**: Minutes empty before deletion (default: 5 minutes)
- **Monitoring Loop**: `@tasks.loop(minutes=1)` - runs every 1 minute
- **`empty_threshold`**: 5 minutes empty before deletion
- **`data_file`**: JSON persistence file path (default: "data/voice_channels.json")
- **Task Pattern**: Uses discord.py `@tasks.loop` with `@before_loop` for safe startup
### Channel Categories
- Channels are created in the "Voice Channels" category if it exists

View File

@ -3,14 +3,14 @@ Voice Channel Cleanup Service
Provides automatic cleanup of empty voice channels with restart resilience.
"""
import asyncio
import logging
import discord
from discord.ext import commands
from discord.ext import commands, tasks
from .tracker import VoiceChannelTracker
from commands.gameplay.scorecard_tracker import ScorecardTracker
from utils.logging import get_contextual_logger
logger = logging.getLogger(f'{__name__}.VoiceChannelCleanupService')
@ -27,52 +27,49 @@ class VoiceChannelCleanupService:
- Automatic scorecard unpublishing when voice channel is cleaned up
"""
def __init__(self, data_file: str = "data/voice_channels.json"):
def __init__(self, bot: commands.Bot, data_file: str = "data/voice_channels.json"):
"""
Initialize the cleanup service.
Args:
bot: Discord bot instance
data_file: Path to the JSON data file for persistence
"""
self.bot = bot
self.logger = get_contextual_logger(f'{__name__}.VoiceChannelCleanupService')
self.tracker = VoiceChannelTracker(data_file)
self.scorecard_tracker = ScorecardTracker()
self.cleanup_interval = 60 # 5 minutes check interval
self.empty_threshold = 5 # Delete after 15 minutes empty
self._running = False
self.empty_threshold = 5 # Delete after 5 minutes empty
async def start_monitoring(self, bot: commands.Bot) -> None:
# Start the cleanup task - @before_loop will wait for bot readiness
self.cleanup_loop.start()
self.logger.info("Voice channel cleanup service initialized")
def cog_unload(self):
"""Stop the task when service is unloaded."""
self.cleanup_loop.cancel()
self.logger.info("Voice channel cleanup service stopped")
@tasks.loop(minutes=1)
async def cleanup_loop(self):
"""
Start the cleanup monitoring loop.
Main cleanup loop - runs every minute.
Args:
bot: Discord bot instance
Checks all tracked channels and cleans up empty ones.
"""
if self._running:
logger.warning("Cleanup service is already running")
return
try:
await self.cleanup_cycle(self.bot)
except Exception as e:
self.logger.error(f"Cleanup cycle error: {e}", exc_info=True)
self._running = True
logger.info("Starting voice channel cleanup service")
@cleanup_loop.before_loop
async def before_cleanup_loop(self):
"""Wait for bot to be ready before starting - REQUIRED FOR SAFE STARTUP."""
await self.bot.wait_until_ready()
self.logger.info("Bot is ready, voice cleanup service starting")
# On startup, verify tracked channels still exist and clean up stale entries
await self.verify_tracked_channels(bot)
# Start the monitoring loop
while self._running:
try:
await self.cleanup_cycle(bot)
await asyncio.sleep(self.cleanup_interval)
except Exception as e:
logger.error(f"Cleanup cycle error: {e}", exc_info=True)
# Use shorter retry interval on errors
await asyncio.sleep(60)
logger.info("Voice channel cleanup service stopped")
def stop_monitoring(self) -> None:
"""Stop the cleanup monitoring loop."""
self._running = False
logger.info("Stopping voice channel cleanup service")
await self.verify_tracked_channels(self.bot)
async def verify_tracked_channels(self, bot: commands.Bot) -> None:
"""
@ -81,7 +78,7 @@ class VoiceChannelCleanupService:
Args:
bot: Discord bot instance
"""
logger.info("Verifying tracked voice channels on startup")
self.logger.info("Verifying tracked voice channels on startup")
valid_channel_ids = []
channels_to_remove = []
@ -93,13 +90,13 @@ class VoiceChannelCleanupService:
guild = bot.get_guild(guild_id)
if not guild:
logger.warning(f"Guild {guild_id} not found, removing channel {channel_data['name']}")
self.logger.warning(f"Guild {guild_id} not found, removing channel {channel_data['name']}")
channels_to_remove.append(channel_id)
continue
channel = guild.get_channel(channel_id)
if not channel:
logger.warning(f"Channel {channel_data['name']} (ID: {channel_id}) no longer exists")
self.logger.warning(f"Channel {channel_data['name']} (ID: {channel_id}) no longer exists")
channels_to_remove.append(channel_id)
continue
@ -107,7 +104,7 @@ class VoiceChannelCleanupService:
valid_channel_ids.append(channel_id)
except (ValueError, TypeError, KeyError) as e:
logger.warning(f"Invalid channel data: {e}, removing entry")
self.logger.warning(f"Invalid channel data: {e}, removing entry")
if "channel_id" in channel_data:
try:
channels_to_remove.append(int(channel_data["channel_id"]))
@ -126,18 +123,18 @@ class VoiceChannelCleanupService:
text_channel_id_int = int(channel_data["text_channel_id"])
was_unpublished = self.scorecard_tracker.unpublish_scorecard(text_channel_id_int)
if was_unpublished:
logger.info(f"📋 Unpublished scorecard from text channel {text_channel_id_int} (stale voice channel)")
self.logger.info(f"📋 Unpublished scorecard from text channel {text_channel_id_int} (stale voice channel)")
except (ValueError, TypeError) as e:
logger.warning(f"Invalid text_channel_id in stale voice channel data: {e}")
self.logger.warning(f"Invalid text_channel_id in stale voice channel data: {e}")
# Also clean up any additional stale entries
stale_removed = self.tracker.cleanup_stale_entries(valid_channel_ids)
total_removed = len(channels_to_remove) + stale_removed
if total_removed > 0:
logger.info(f"Cleaned up {total_removed} stale channel tracking entries")
self.logger.info(f"Cleaned up {total_removed} stale channel tracking entries")
logger.info(f"Verified {len(valid_channel_ids)} valid tracked channels")
self.logger.info(f"Verified {len(valid_channel_ids)} valid tracked channels")
async def cleanup_cycle(self, bot: commands.Bot) -> None:
"""
@ -146,7 +143,7 @@ class VoiceChannelCleanupService:
Args:
bot: Discord bot instance
"""
logger.debug("Starting cleanup cycle")
self.logger.debug("Starting cleanup cycle")
# Update status of all tracked channels
await self.update_all_channel_statuses(bot)
@ -155,7 +152,7 @@ class VoiceChannelCleanupService:
channels_for_cleanup = self.tracker.get_channels_for_cleanup(self.empty_threshold)
if channels_for_cleanup:
logger.info(f"Found {len(channels_for_cleanup)} channels ready for cleanup")
self.logger.info(f"Found {len(channels_for_cleanup)} channels ready for cleanup")
# Delete empty channels
for channel_data in channels_for_cleanup:
@ -185,30 +182,54 @@ class VoiceChannelCleanupService:
guild = bot.get_guild(guild_id)
if not guild:
logger.debug(f"Guild {guild_id} not found for channel {channel_data['name']}")
self.logger.debug(f"Guild {guild_id} not found for channel {channel_data['name']}")
return
channel = guild.get_channel(channel_id)
if not channel:
logger.debug(f"Channel {channel_data['name']} no longer exists, will be cleaned up")
self.logger.debug(f"Channel {channel_data['name']} no longer exists, removing from tracking")
self.tracker.remove_channel(channel_id)
# Unpublish associated scorecard if it exists
text_channel_id = channel_data.get("text_channel_id")
if text_channel_id:
try:
text_channel_id_int = int(text_channel_id)
was_unpublished = self.scorecard_tracker.unpublish_scorecard(text_channel_id_int)
if was_unpublished:
self.logger.info(f"📋 Unpublished scorecard from text channel {text_channel_id_int} (manually deleted voice channel)")
except (ValueError, TypeError) as e:
self.logger.warning(f"Invalid text_channel_id in manually deleted voice channel data: {e}")
return
# Ensure it's a voice channel before checking members
if not isinstance(channel, discord.VoiceChannel):
logger.warning(f"Channel {channel_data['name']} is not a voice channel, removing from tracking")
self.logger.warning(f"Channel {channel_data['name']} is not a voice channel, removing from tracking")
self.tracker.remove_channel(channel_id)
# Unpublish associated scorecard if it exists
text_channel_id = channel_data.get("text_channel_id")
if text_channel_id:
try:
text_channel_id_int = int(text_channel_id)
was_unpublished = self.scorecard_tracker.unpublish_scorecard(text_channel_id_int)
if was_unpublished:
self.logger.info(f"📋 Unpublished scorecard from text channel {text_channel_id_int} (wrong channel type)")
except (ValueError, TypeError) as e:
self.logger.warning(f"Invalid text_channel_id in wrong channel type data: {e}")
return
# Check if channel is empty
is_empty = len(channel.members) == 0
self.tracker.update_channel_status(channel_id, is_empty)
logger.debug(f"Channel {channel_data['name']}: {'empty' if is_empty else 'occupied'} "
self.logger.debug(f"Channel {channel_data['name']}: {'empty' if is_empty else 'occupied'} "
f"({len(channel.members)} members)")
except Exception as e:
logger.error(f"Error checking channel status for {channel_data.get('name', 'unknown')}: {e}")
self.logger.error(f"Error checking channel status for {channel_data.get('name', 'unknown')}: {e}")
async def cleanup_channel(self, bot: commands.Bot, channel_data: dict) -> None:
"""
@ -225,33 +246,33 @@ class VoiceChannelCleanupService:
guild = bot.get_guild(guild_id)
if not guild:
logger.info(f"Guild {guild_id} not found, removing tracking for {channel_name}")
self.logger.info(f"Guild {guild_id} not found, removing tracking for {channel_name}")
self.tracker.remove_channel(channel_id)
return
channel = guild.get_channel(channel_id)
if not channel:
logger.info(f"Channel {channel_name} already deleted, removing from tracking")
self.logger.info(f"Channel {channel_name} already deleted, removing from tracking")
self.tracker.remove_channel(channel_id)
return
# Ensure it's a voice channel before checking members
if not isinstance(channel, discord.VoiceChannel):
logger.warning(f"Channel {channel_name} is not a voice channel, removing from tracking")
self.logger.warning(f"Channel {channel_name} is not a voice channel, removing from tracking")
self.tracker.remove_channel(channel_id)
return
# Final check: make sure channel is still empty before deleting
if len(channel.members) > 0:
logger.info(f"Channel {channel_name} is no longer empty, skipping cleanup")
self.logger.info(f"Channel {channel_name} is no longer empty, skipping cleanup")
self.tracker.update_channel_status(channel_id, False)
return
# Delete the channel
await channel.delete(reason="Automatic cleanup - empty for 15+ minutes")
await channel.delete(reason="Automatic cleanup - empty for 5+ minutes")
self.tracker.remove_channel(channel_id)
logger.info(f"✅ Cleaned up empty voice channel: {channel_name} (ID: {channel_id})")
self.logger.info(f"✅ Cleaned up empty voice channel: {channel_name} (ID: {channel_id})")
# Unpublish associated scorecard if it exists
text_channel_id = channel_data.get("text_channel_id")
@ -260,15 +281,15 @@ class VoiceChannelCleanupService:
text_channel_id_int = int(text_channel_id)
was_unpublished = self.scorecard_tracker.unpublish_scorecard(text_channel_id_int)
if was_unpublished:
logger.info(f"📋 Unpublished scorecard from text channel {text_channel_id_int} (voice channel cleanup)")
self.logger.info(f"📋 Unpublished scorecard from text channel {text_channel_id_int} (voice channel cleanup)")
else:
logger.debug(f"No scorecard found for text channel {text_channel_id_int}")
self.logger.debug(f"No scorecard found for text channel {text_channel_id_int}")
except (ValueError, TypeError) as e:
logger.warning(f"Invalid text_channel_id in voice channel data: {e}")
self.logger.warning(f"Invalid text_channel_id in voice channel data: {e}")
except discord.NotFound:
# Channel was already deleted
logger.info(f"Channel {channel_data.get('name', 'unknown')} was already deleted")
self.logger.info(f"Channel {channel_data.get('name', 'unknown')} was already deleted")
self.tracker.remove_channel(int(channel_data["channel_id"]))
# Still try to unpublish associated scorecard
@ -278,13 +299,13 @@ class VoiceChannelCleanupService:
text_channel_id_int = int(text_channel_id)
was_unpublished = self.scorecard_tracker.unpublish_scorecard(text_channel_id_int)
if was_unpublished:
logger.info(f"📋 Unpublished scorecard from text channel {text_channel_id_int} (stale voice channel cleanup)")
self.logger.info(f"📋 Unpublished scorecard from text channel {text_channel_id_int} (stale voice channel cleanup)")
except (ValueError, TypeError) as e:
logger.warning(f"Invalid text_channel_id in voice channel data: {e}")
self.logger.warning(f"Invalid text_channel_id in voice channel data: {e}")
except discord.Forbidden:
logger.error(f"Missing permissions to delete channel {channel_data.get('name', 'unknown')}")
self.logger.error(f"Missing permissions to delete channel {channel_data.get('name', 'unknown')}")
except Exception as e:
logger.error(f"Error cleaning up channel {channel_data.get('name', 'unknown')}: {e}")
self.logger.error(f"Error cleaning up channel {channel_data.get('name', 'unknown')}: {e}")
def get_tracker(self) -> VoiceChannelTracker:
"""
@ -306,9 +327,21 @@ class VoiceChannelCleanupService:
empty_channels = [ch for ch in all_channels if ch.get("empty_since")]
return {
"running": self._running,
"running": self.cleanup_loop.is_running(),
"total_tracked": len(all_channels),
"empty_channels": len(empty_channels),
"cleanup_interval": self.cleanup_interval,
"empty_threshold": self.empty_threshold
}
def setup_voice_cleanup(bot: commands.Bot) -> VoiceChannelCleanupService:
"""
Setup function to initialize the voice channel cleanup service.
Args:
bot: Discord bot instance
Returns:
VoiceChannelCleanupService instance
"""
return VoiceChannelCleanupService(bot)

View File

@ -17,6 +17,7 @@ from discord.ext import commands
from commands.voice.channels import VoiceChannelCommands
from commands.voice.cleanup_service import VoiceChannelCleanupService
from commands.voice.tracker import VoiceChannelTracker
from commands.gameplay.scorecard_tracker import ScorecardTracker
from models.game import Game
from models.team import Team
@ -180,19 +181,28 @@ class TestVoiceChannelTracker:
class TestVoiceChannelCleanupService:
"""Test voice channel cleanup service functionality."""
@pytest.fixture
def cleanup_service(self):
"""Create a cleanup service instance."""
with tempfile.TemporaryDirectory() as temp_dir:
data_file = Path(temp_dir) / "test_channels.json"
return VoiceChannelCleanupService(str(data_file))
@pytest.fixture
def mock_bot(self):
"""Create a mock bot instance."""
bot = AsyncMock(spec=commands.Bot)
return bot
@pytest.fixture
def cleanup_service(self, mock_bot):
"""Create a cleanup service instance."""
from utils.logging import get_contextual_logger
with tempfile.TemporaryDirectory() as temp_dir:
data_file = Path(temp_dir) / "test_channels.json"
service = VoiceChannelCleanupService.__new__(VoiceChannelCleanupService)
service.bot = mock_bot
service.logger = get_contextual_logger('test.VoiceChannelCleanupService')
service.tracker = VoiceChannelTracker(str(data_file))
service.scorecard_tracker = ScorecardTracker()
service.empty_threshold = 5
# Don't start the loop (no event loop in tests)
return service
@pytest.mark.asyncio
async def test_verify_tracked_channels(self, cleanup_service, mock_bot):
"""Test verification of tracked channels on startup."""
@ -287,7 +297,7 @@ class TestVoiceChannelCleanupService:
await cleanup_service.cleanup_channel(mock_bot, channel_data)
# Should have deleted the channel
mock_channel.delete.assert_called_once_with(reason="Automatic cleanup - empty for 15+ minutes")
mock_channel.delete.assert_called_once_with(reason="Automatic cleanup - empty for 5+ minutes")
# Should have removed from tracking
assert "123" not in cleanup_service.tracker._data["voice_channels"]
@ -335,7 +345,7 @@ class TestVoiceChannelCleanupService:
await cleanup_service.cleanup_channel(mock_bot, channel_data)
# Should have deleted the channel
mock_channel.delete.assert_called_once_with(reason="Automatic cleanup - empty for 15+ minutes")
mock_channel.delete.assert_called_once_with(reason="Automatic cleanup - empty for 5+ minutes")
# Should have removed from voice channel tracking
assert "123" not in cleanup_service.tracker._data["voice_channels"]