Fix all activity system edge cases

Critical fixes:
- Add threading.Lock for all shared mutable state (override, cache, current activity)
- Atomic YAML writes (temp file + os.replace) to prevent corruption on crash
- Deep-copy cache on reads to prevent callers from mutating shared state

High-severity fixes:
- Validate entries in pick_activity_for_mood() — skip/log malformed instead of KeyError
- Log warning on unrecognized activity type fallback
- Normalize empty-string state to None (avoid 'None' display)
- release_manual_override() now uses force=True so bot always shows activity
- Add try/except in release_manual_override() to handle failures gracefully

Medium fixes:
- Remove dead 'test' mood from activities.yaml
- Validate name length (128 char Discord limit) in CRUD and manual set
- Validate streaming entries have URL in CRUD path
- Add JSON parse error handling in API routes
- on_ready preserves active manual override instead of overwriting
- Log override expiry timestamp (HH:MM:SS) for easier debugging
- exc_info=True on presence update errors for full stack traces

Low fixes:
- JS activitySetFromEntry() shows notification on parse error

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

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
+        from utils.activities import update_bot_presence, is_manual_override_active
-        if globals.EVIL_MODE:
+        # 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)

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)

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;

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:
+    with _state_lock:
-        mtime = os.path.getmtime(ACTIVITIES_FILE)
+        try:
-    except OSError:
+            mtime = os.path.getmtime(ACTIVITIES_FILE)
-        logger.warning(f"Activities file not found: {ACTIVITIES_FILE}")
+        except OSError:
-        return {"normal": {}, "evil": {}}
+            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:
+        if not force and _activities_cache is not None and mtime == _cache_mtime:
-        return _activities_cache
+            # Return a deep copy so callers cannot mutate the cache
+            import copy
+            return copy.deepcopy(_activities_cache)
 
-    try:
+        try:
-        with open(ACTIVITIES_FILE, "r", encoding="utf-8") as f:
+            with open(ACTIVITIES_FILE, "r", encoding="utf-8") as f:
-            data = yaml.safe_load(f) or {}
+                data = yaml.safe_load(f) or {}
-        _activities_cache = data
+            _activities_cache = data
-        _cache_mtime = mtime
+            _cache_mtime = mtime
-        logger.debug(f"Loaded activities from {ACTIVITIES_FILE}")
+            logger.debug(f"Loaded activities from {ACTIVITIES_FILE}")
-        return data
+            import copy
-    except Exception as e:
+            return copy.deepcopy(data)
-        logger.error(f"Failed to load activities file: {e}")
+        except Exception as e:
-        return _activities_cache or {"normal": {}, "evil": {}}
+            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 _state_lock:
-        with open(ACTIVITIES_FILE, "w", encoding="utf-8") as f:
+        try:
-            yaml.dump(data, f, default_flow_style=False, allow_unicode=True, sort_keys=False)
+            # 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
+            _activities_cache = data
-        _cache_mtime = os.path.getmtime(ACTIVITIES_FILE)
+            _cache_mtime = os.path.getmtime(ACTIVITIES_FILE)
-        logger.info(f"Saved activities to {ACTIVITIES_FILE}")
+            logger.info(f"Saved activities to {ACTIVITIES_FILE}")
-    except Exception as e:
+        except Exception as e:
-        logger.error(f"Failed to save activities file: {e}")
+            logger.error(f"Failed to save activities file: {e}")
-        raise
+            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]
+    # Validate entries, skipping malformed ones
-    chosen = random.choices(activities, weights=weights, k=1)[0]
+    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)."""
+    """Check if a manual override is in effect (hasn't expired). Thread-safe."""
-    global _manual_override
+    with _state_lock:
-    if not _manual_override:
+        global _manual_override
-        return False
+        if not _manual_override:
-    if _manual_override_until > 0 and time.time() > _manual_override_until:
+            return False
-        _manual_override = False
+        if _manual_override_until > 0 and time.time() > _manual_override_until:
-        logger.info("Manual override expired, returning to automatic mode")
+            _manual_override = False
-        return False
+            logger.info("Manual override expired, returning to automatic mode")
-    return True
+            return False
+        return True
 
 
 def set_manual_override(duration: int = MANUAL_OVERRIDE_DURATION):
-    """Activate manual override for the given duration (seconds)."""
+    """Activate manual override for the given duration (seconds). Thread-safe."""
-    global _manual_override, _manual_override_until
+    with _state_lock:
-    _manual_override = True
+        global _manual_override, _manual_override_until
-    _manual_override_until = time.time() + duration
+        _manual_override = True
-    logger.info(f"Manual override activated for {duration}s")
+        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."""
+    """Deactivate manual override immediately. Thread-safe."""
-    global _manual_override, _manual_override_until
+    with _state_lock:
-    _manual_override = False
+        global _manual_override, _manual_override_until
-    _manual_override_until = 0.0
+        _manual_override = False
-    logger.info("Manual override cleared")
+        _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 the current activity dict or None if idle. Thread-safe."""
-    return _current_activity
+    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:
+    try:
-        mood = globals.EVIL_DM_MOOD
+        if globals.EVIL_MODE:
-        is_evil = True
+            mood = globals.EVIL_DM_MOOD
-    else:
+            is_evil = True
-        mood = globals.DM_MOOD
+        else:
-        is_evil = False
+            mood = globals.DM_MOOD
-    await update_bot_presence(mood, is_evil=is_evil, force=False)
+            is_evil = False
-    logger.info(f"Released manual override, recalculated for mood={'evil/' if is_evil else ''}{mood}")
+        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}")