diff --git a/bot/activities.yaml b/bot/activities.yaml index c07939e..a6d9ca0 100644 --- a/bot/activities.yaml +++ b/bot/activities.yaml @@ -505,10 +505,6 @@ normal: name: Gintama weight: 1 state: Comedy Anime - test: - - type: playing - name: G - weight: 2 evil: aggressive: - type: listening diff --git a/bot/bot.py b/bot/bot.py index 7e1a8cb..bc2b37b 100644 --- a/bot/bot.py +++ b/bot/bot.py @@ -138,8 +138,11 @@ async def on_ready(): # Set initial Discord presence based on current mood try: - from utils.activities import update_bot_presence - if globals.EVIL_MODE: + from utils.activities import update_bot_presence, is_manual_override_active + # On reconnect, don't overwrite an active manual override + if is_manual_override_active(): + logger.info("Manual override active on ready, preserving it") + elif globals.EVIL_MODE: await update_bot_presence(globals.EVIL_DM_MOOD, is_evil=True, force=True) else: await update_bot_presence(globals.DM_MOOD, is_evil=False, force=True) diff --git a/bot/routes/activities.py b/bot/routes/activities.py index 1db36c8..fa5e3d2 100644 --- a/bot/routes/activities.py +++ b/bot/routes/activities.py @@ -41,7 +41,11 @@ async def set_mood_activities(section: str, mood: str, request: Request): if section not in ("normal", "evil"): return JSONResponse(status_code=400, content={"error": "Section must be 'normal' or 'evil'"}) - data = await request.json() + try: + data = await request.json() + except Exception: + return JSONResponse(status_code=400, content={"error": "Invalid JSON body"}) + activities = data.get("activities") if activities is None: @@ -97,12 +101,24 @@ async def set_current_activity(request: Request): Body: {"type": "listening"|"playing"|"watching"|"competing"|"streaming", "name": "...", "state": "..." (optional), "url": "..." (required for streaming)} """ - data = await request.json() + try: + data = await request.json() + except Exception: + return JSONResponse(status_code=400, content={"error": "Invalid JSON body"}) + activity_type = data.get("type", "").lower().strip() name = data.get("name", "").strip() state = data.get("state") or None url = data.get("url") or None + # Pre-validate before passing to activity module + if not activity_type: + return JSONResponse(status_code=400, content={"error": "'type' is required"}) + if not name: + return JSONResponse(status_code=400, content={"error": "'name' is required"}) + if len(name) > 128: + return JSONResponse(status_code=400, content={"error": f"'name' exceeds 128 characters ({len(name)})"}) + try: from utils.activities import set_activity_manual await set_activity_manual(activity_type, name, state=state, url=url) diff --git a/bot/static/index.html b/bot/static/index.html index 4d2776a..d4da1ef 100644 --- a/bot/static/index.html +++ b/bot/static/index.html @@ -7137,7 +7137,7 @@ async function activitySetFromEntry(btnElement) { const raw = btnElement.getAttribute('data-entry'); if (!raw) return; let entry; - try { entry = JSON.parse(decodeURIComponent(raw)); } catch { return; } + try { entry = JSON.parse(decodeURIComponent(raw)); } catch (e) { showNotification('Failed to parse activity data', 'error'); return; } const type = entry.type; const name = entry.name; const state = entry.state || null; diff --git a/bot/utils/activities.py b/bot/utils/activities.py index f932a56..acb8d32 100644 --- a/bot/utils/activities.py +++ b/bot/utils/activities.py @@ -12,6 +12,8 @@ Supports 5 activity types: listening, playing, watching, competing, streaming. import os import random +import tempfile +import threading import time import yaml import discord @@ -22,6 +24,9 @@ logger = get_logger('activity') ACTIVITIES_FILE = os.path.join(os.path.dirname(os.path.dirname(__file__)), "activities.yaml") +# Discord activity name character limit +DISCORD_ACTIVITY_NAME_MAX = 128 + # All valid activity types VALID_ACTIVITY_TYPES = {"listening", "playing", "watching", "competing", "streaming"} @@ -56,6 +61,9 @@ ACTIVITY_PROBABILITY = { "manic": 0.85, } +# ── Thread lock for all shared mutable state ── +_state_lock = threading.Lock() + # ── Manual override state ── _manual_override = False _manual_override_until = 0.0 # Unix timestamp; 0 = no override @@ -74,44 +82,64 @@ _cache_mtime = 0.0 # ══════════════════════════════════════════════════════════════════════════════ def _load_activities(force=False): - """Load activities.yaml with file-mtime-based caching.""" + """Load activities.yaml with file-mtime-based caching. Returns a deep copy.""" global _activities_cache, _cache_mtime - try: - mtime = os.path.getmtime(ACTIVITIES_FILE) - except OSError: - logger.warning(f"Activities file not found: {ACTIVITIES_FILE}") - return {"normal": {}, "evil": {}} + with _state_lock: + try: + mtime = os.path.getmtime(ACTIVITIES_FILE) + except OSError: + logger.warning(f"Activities file not found: {ACTIVITIES_FILE}") + return {"normal": {}, "evil": {}} - if not force and _activities_cache is not None and mtime == _cache_mtime: - return _activities_cache + if not force and _activities_cache is not None and mtime == _cache_mtime: + # Return a deep copy so callers cannot mutate the cache + import copy + return copy.deepcopy(_activities_cache) - try: - with open(ACTIVITIES_FILE, "r", encoding="utf-8") as f: - data = yaml.safe_load(f) or {} - _activities_cache = data - _cache_mtime = mtime - logger.debug(f"Loaded activities from {ACTIVITIES_FILE}") - return data - except Exception as e: - logger.error(f"Failed to load activities file: {e}") - return _activities_cache or {"normal": {}, "evil": {}} + try: + with open(ACTIVITIES_FILE, "r", encoding="utf-8") as f: + data = yaml.safe_load(f) or {} + _activities_cache = data + _cache_mtime = mtime + logger.debug(f"Loaded activities from {ACTIVITIES_FILE}") + import copy + return copy.deepcopy(data) + except Exception as e: + logger.error(f"Failed to load activities file: {e}") + if _activities_cache is not None: + import copy + return copy.deepcopy(_activities_cache) + return {"normal": {}, "evil": {}} def save_activities(data: dict): - """Write the full activities dict back to YAML.""" + """Write the full activities dict back to YAML using atomic write (temp + rename).""" global _activities_cache, _cache_mtime - try: - with open(ACTIVITIES_FILE, "w", encoding="utf-8") as f: - yaml.dump(data, f, default_flow_style=False, allow_unicode=True, sort_keys=False) + with _state_lock: + try: + # Atomic write: write to temp file in same directory, then rename + dir_name = os.path.dirname(ACTIVITIES_FILE) + fd, tmp_path = tempfile.mkstemp(dir=dir_name, suffix=".yaml.tmp") + try: + with os.fdopen(fd, "w", encoding="utf-8") as f: + yaml.dump(data, f, default_flow_style=False, allow_unicode=True, sort_keys=False) + os.replace(tmp_path, ACTIVITIES_FILE) + except BaseException: + # Clean up temp file on failure + try: + os.unlink(tmp_path) + except OSError: + pass + raise - _activities_cache = data - _cache_mtime = os.path.getmtime(ACTIVITIES_FILE) - logger.info(f"Saved activities to {ACTIVITIES_FILE}") - except Exception as e: - logger.error(f"Failed to save activities file: {e}") - raise + _activities_cache = data + _cache_mtime = os.path.getmtime(ACTIVITIES_FILE) + logger.info(f"Saved activities to {ACTIVITIES_FILE}") + except Exception as e: + logger.error(f"Failed to save activities file: {e}") + raise # ══════════════════════════════════════════════════════════════════════════════ @@ -119,7 +147,7 @@ def save_activities(data: dict): # ══════════════════════════════════════════════════════════════════════════════ def get_all_activities() -> dict: - """Return the full activities dict (normal + evil sections).""" + """Return the full activities dict (normal + evil sections). Returns a deep copy.""" return _load_activities() @@ -151,12 +179,16 @@ def set_activities_for_mood(mood_name: str, is_evil: bool, activities: list): ) if not entry.get("name") or not isinstance(entry["name"], str): raise ValueError(f"Entry {i} must have a non-empty string 'name'") + if len(entry["name"]) > DISCORD_ACTIVITY_NAME_MAX: + raise ValueError(f"Entry {i} name exceeds {DISCORD_ACTIVITY_NAME_MAX} characters") if not isinstance(entry.get("weight", 0), int) or entry.get("weight", 0) < 1: raise ValueError(f"Entry {i} weight must be a positive integer") if "state" in entry and entry["state"] is not None and not isinstance(entry["state"], str): raise ValueError(f"Entry {i} 'state' must be a string if provided") if "url" in entry and entry["url"] is not None and not isinstance(entry["url"], str): raise ValueError(f"Entry {i} 'url' must be a string if provided") + if entry.get("type") == "streaming" and not entry.get("url"): + raise ValueError(f"Entry {i} is streaming type but has no url") section = "evil" if is_evil else "normal" data = _load_activities() @@ -173,18 +205,43 @@ def set_activities_for_mood(mood_name: str, is_evil: bool, activities: list): def pick_activity_for_mood(mood_name: str, is_evil: bool = False): """Pick a weighted-random activity for a mood. + Validates entries and skips malformed ones with a warning. + Returns: dict: {"type": ..., "name": ..., "state": ..., "url": ...} state and url may be None. - Returns None if mood has no entries. + Returns None if mood has no valid entries. """ activities = get_activities_for_mood(mood_name, is_evil) if not activities: return None - weights = [entry.get("weight", 1) for entry in activities] - chosen = random.choices(activities, weights=weights, k=1)[0] + # Validate entries, skipping malformed ones + valid = [] + weights = [] + for i, entry in enumerate(activities): + if not isinstance(entry, dict): + logger.warning(f"Skipping non-dict entry {i} in {'evil/' if is_evil else ''}{mood_name}") + continue + if "type" not in entry or "name" not in entry: + logger.warning(f"Skipping entry {i} missing 'type' or 'name' in {'evil/' if is_evil else ''}{mood_name}: {entry}") + continue + if entry["type"] not in VALID_ACTIVITY_TYPES: + logger.warning(f"Skipping entry {i} with unrecognized type '{entry['type']}' in {'evil/' if is_evil else ''}{mood_name}") + continue + w = entry.get("weight", 1) + if not isinstance(w, int) or w < 1: + logger.warning(f"Skipping entry {i} with invalid weight {w} in {'evil/' if is_evil else ''}{mood_name}") + continue + valid.append(entry) + weights.append(w) + + if not valid: + logger.warning(f"No valid entries for {'evil/' if is_evil else ''}{mood_name}") + return None + + chosen = random.choices(valid, weights=weights, k=1)[0] return { "type": chosen["type"], "name": chosen["name"], @@ -208,31 +265,35 @@ def should_have_activity(mood_name: str) -> bool: # ══════════════════════════════════════════════════════════════════════════════ def is_manual_override_active() -> bool: - """Check if a manual override is in effect (hasn't expired).""" - global _manual_override - if not _manual_override: - return False - if _manual_override_until > 0 and time.time() > _manual_override_until: - _manual_override = False - logger.info("Manual override expired, returning to automatic mode") - return False - return True + """Check if a manual override is in effect (hasn't expired). Thread-safe.""" + with _state_lock: + global _manual_override + if not _manual_override: + return False + if _manual_override_until > 0 and time.time() > _manual_override_until: + _manual_override = False + logger.info("Manual override expired, returning to automatic mode") + return False + return True def set_manual_override(duration: int = MANUAL_OVERRIDE_DURATION): - """Activate manual override for the given duration (seconds).""" - global _manual_override, _manual_override_until - _manual_override = True - _manual_override_until = time.time() + duration - logger.info(f"Manual override activated for {duration}s") + """Activate manual override for the given duration (seconds). Thread-safe.""" + with _state_lock: + global _manual_override, _manual_override_until + _manual_override = True + expiry = time.time() + duration + _manual_override_until = expiry + logger.info(f"Manual override activated for {duration}s (expires at {time.strftime('%H:%M:%S', time.localtime(expiry))})") def clear_manual_override(): - """Deactivate manual override immediately.""" - global _manual_override, _manual_override_until - _manual_override = False - _manual_override_until = 0.0 - logger.info("Manual override cleared") + """Deactivate manual override immediately. Thread-safe.""" + with _state_lock: + global _manual_override, _manual_override_until + _manual_override = False + _manual_override_until = 0.0 + logger.info("Manual override cleared") # ══════════════════════════════════════════════════════════════════════════════ @@ -240,14 +301,16 @@ def clear_manual_override(): # ══════════════════════════════════════════════════════════════════════════════ def get_current_activity(): - """Return the current activity dict or None if idle.""" - return _current_activity + """Return the current activity dict or None if idle. Thread-safe.""" + with _state_lock: + return _current_activity def _set_current_activity(activity_dict): - """Update the tracked current activity.""" + """Update the tracked current activity. Thread-safe.""" global _current_activity - _current_activity = activity_dict + with _state_lock: + _current_activity = activity_dict # ══════════════════════════════════════════════════════════════════════════════ @@ -255,10 +318,13 @@ def _set_current_activity(activity_dict): # ══════════════════════════════════════════════════════════════════════════════ def _build_activity(payload: dict): - """Build a discord.Activity (or discord.Streaming) from a payload dict.""" + """Build a discord.Activity (or discord.Streaming) from a payload dict. + + Logs a warning if the activity type is unrecognized (falls back to playing). + """ atype = payload["type"] name = payload["name"] - state = payload.get("state") + state = payload.get("state") or None # normalize empty string to None url = payload.get("url") if atype == "streaming" and url: @@ -271,8 +337,12 @@ def _build_activity(payload: dict): "competing": discord.ActivityType.competing, "streaming": discord.ActivityType.streaming, # fallback without url } + resolved_type = type_map.get(atype) + if resolved_type is None: + logger.warning(f"Unrecognized activity type '{atype}', falling back to 'playing'") + resolved_type = discord.ActivityType.playing return discord.Activity( - type=type_map.get(atype, discord.ActivityType.playing), + type=resolved_type, name=name, state=state, ) @@ -351,20 +421,22 @@ async def update_bot_presence(mood_name: str, is_evil: bool = False, force: bool logger.info(f"Set presence: {label} (mood={'evil/' if is_evil else ''}{mood_name})") except Exception as e: - logger.error(f"Failed to update bot presence: {e}") + logger.error(f"Failed to update bot presence: {e}", exc_info=True) async def set_activity_manual(activity_type: str, name: str, state: str = None, url: str = None): """Manually set the bot's activity (bypasses mood system). Raises: - ValueError: if activity_type is invalid or streaming lacks url + ValueError: if activity_type is invalid, name too long, or streaming lacks url RuntimeError: if bot is not ready """ if activity_type not in VALID_ACTIVITY_TYPES: raise ValueError(f"Invalid type '{activity_type}', must be one of: {', '.join(sorted(VALID_ACTIVITY_TYPES))}") if not name or not isinstance(name, str): raise ValueError("name must be a non-empty string") + if len(name) > DISCORD_ACTIVITY_NAME_MAX: + raise ValueError(f"name exceeds {DISCORD_ACTIVITY_NAME_MAX} characters") if activity_type == "streaming" and not url: raise ValueError("streaming type requires a url") @@ -401,13 +473,20 @@ async def clear_activity_manual(): async def release_manual_override(): - """Release manual override and immediately recalculate presence from current mood.""" + """Release manual override and immediately recalculate presence from current mood. + + Uses force=True so the bot always gets an activity instead of potentially + going idle right away (which would be confusing UX after clicking "Return to Auto"). + """ clear_manual_override() - if globals.EVIL_MODE: - mood = globals.EVIL_DM_MOOD - is_evil = True - else: - mood = globals.DM_MOOD - is_evil = False - await update_bot_presence(mood, is_evil=is_evil, force=False) - logger.info(f"Released manual override, recalculated for mood={'evil/' if is_evil else ''}{mood}") + try: + if globals.EVIL_MODE: + mood = globals.EVIL_DM_MOOD + is_evil = True + else: + mood = globals.DM_MOOD + is_evil = False + await update_bot_presence(mood, is_evil=is_evil, force=True) + logger.info(f"Released manual override, set presence for mood={'evil/' if is_evil else ''}{mood}") + except Exception as e: + logger.error(f"Failed to recalculate presence after releasing override: {e}")