fix: address PR review critical and high findings

- Remove duplicate _cap_log_content definition (dead code merge artifact)
  from manager.py; keep byte-count version that correctly handles multibyte UTF-8
- Fix storage.py unassigned handler reading wrong key (unassignedDevices → disks)
  — query already fetched `disks {}` but handler returned empty list every call
- Add null checks to all 8 Docker organizer object mutations; raise ToolError
  instead of silently returning success=True with organizer=None
- Raise ToolError in docker logs when server returns no log data
- Extract notification object from create response (was returning raw GraphQL
  wrapper dict instead of the notification itself)
- Raise ToolError in test_subscription_query on connection failure and unexpected
  exceptions (was returning error dicts, bypassing error handling)
- Remove stale "Bug N fix" inline comments from diagnostics.py
- Update docker.py module docstring to reflect 26 actions (was 15)
- Bump version 0.4.1 → 0.4.2

Co-authored-by: Claude <claude@anthropic.com>

pyproject.toml

@@ -10,7 +10,7 @@ build-backend = "hatchling.build"
 # ============================================================================
 [project]
 name = "unraid-mcp"
-version = "0.4.1"
+version = "0.4.2"
 description = "MCP Server for Unraid API - provides tools to interact with an Unraid server's GraphQL API"
 readme = "README.md"
 license = {file = "LICENSE"}

unraid_mcp/subscriptions/diagnostics.py

@@ -101,13 +101,11 @@ def register_diagnostic_tools(mcp: FastMCP) -> None:
         Returns:
             Dict containing test results and response data
         """
-        # Validate before any network I/O (Bug 1 fix)
         field_name = _validate_subscription_query(subscription_query)
 
         try:
             logger.info(f"[TEST_SUBSCRIPTION] Testing validated subscription field '{field_name}'")
 
-            # Build WebSocket URL — raises ValueError on invalid/missing scheme (Bug 4 fix)
             try:
                 ws_url = build_ws_url()
             except ValueError as e:
@@ -139,7 +137,7 @@ def register_diagnostic_tools(mcp: FastMCP) -> None:
                 init_response = json.loads(response)
 
                 if init_response.get("type") != "connection_ack":
-                    return {"error": f"Connection failed: {init_response}"}
+                    raise ToolError(f"Connection failed: {init_response}")
 
                 # Send subscription
                 await websocket.send(
@@ -168,7 +166,7 @@ def register_diagnostic_tools(mcp: FastMCP) -> None:
             raise
         except Exception as e:
             logger.error(f"[TEST_SUBSCRIPTION] Error: {e}", exc_info=True)
-            return {"error": str(e), "query_tested": subscription_query}
+            raise ToolError(f"Subscription test failed: {e!s}") from e
 
     @mcp.tool()
     async def diagnose_subscriptions() -> dict[str, Any]:

unraid_mcp/subscriptions/manager.py

@@ -29,56 +29,6 @@ _MAX_RESOURCE_DATA_LINES = 5_000
 _STABLE_CONNECTION_SECONDS = 30
 
 
-def _cap_log_content(data: dict[str, Any]) -> dict[str, Any]:
-    """Cap log content in subscription data to prevent unbounded memory growth.
-
-    Returns a new dict — does NOT mutate the input. If any nested 'content'
-    field (from log subscriptions) exceeds the byte limit, truncate it to the
-    most recent _MAX_RESOURCE_DATA_LINES lines.
-
-    The final content is guaranteed to be <= _MAX_RESOURCE_DATA_BYTES.
-    """
-    result: dict[str, Any] = {}
-    for key, value in data.items():
-        if isinstance(value, dict):
-            result[key] = _cap_log_content(value)
-        elif (
-            key == "content"
-            and isinstance(value, str)
-            and len(value) > _MAX_RESOURCE_DATA_BYTES  # fast pre-check on char count
-        ):
-            lines = value.splitlines()
-            original_line_count = len(lines)
-
-            # Keep most recent lines first.
-            if len(lines) > _MAX_RESOURCE_DATA_LINES:
-                lines = lines[-_MAX_RESOURCE_DATA_LINES:]
-
-            truncated = "\n".join(lines)
-            # Encode once and slice bytes instead of O(n²) line-trim loop
-            encoded = truncated.encode("utf-8", errors="replace")
-            if len(encoded) > _MAX_RESOURCE_DATA_BYTES:
-                truncated = encoded[-_MAX_RESOURCE_DATA_BYTES:].decode("utf-8", errors="ignore")
-                # Strip partial first line that may have been cut mid-character
-                nl_pos = truncated.find("\n")
-                if nl_pos != -1:
-                    truncated = truncated[nl_pos + 1 :]
-
-            logger.warning(
-                f"[RESOURCE] Capped log content from {original_line_count} to "
-                f"{len(lines)} lines ({len(value)} -> {len(truncated)} chars)"
-            )
-            result[key] = truncated
-        else:
-            result[key] = value
-    return result
-
-
-# Resource data size limits to prevent unbounded memory growth
-_MAX_RESOURCE_DATA_BYTES = 1_048_576  # 1 MB
-_MAX_RESOURCE_DATA_LINES = 5_000
-
-
 def _cap_log_content(data: dict[str, Any]) -> dict[str, Any]:
     """Cap log content in subscription data to prevent unbounded memory growth.
 

unraid_mcp/tools/docker.py

@@ -1,7 +1,7 @@
 """Docker container management.
 
-Provides the `unraid_docker` tool with 15 actions for container lifecycle,
+Provides the `unraid_docker` tool with 26 actions for container lifecycle,
-logs, networks, and update management.
+logs, networks, update management, and Docker organizer operations.
 """
 
 import re
@@ -395,6 +395,17 @@ def register_docker_tool(mcp: FastMCP) -> None:
           network_details - Details of a network (requires network_id)
           port_conflicts - Check for port conflicts
           check_updates - Check which containers have updates available
+          create_folder - Create Docker organizer folder (requires folder_name)
+          set_folder_children - Set children of a folder (requires children_ids)
+          delete_entries - Delete organizer entries (requires entry_ids, confirm=True)
+          move_to_folder - Move entries to a folder (requires source_entry_ids, destination_folder_id)
+          move_to_position - Move entries to position in folder (requires source_entry_ids, destination_folder_id, position)
+          rename_folder - Rename a folder (requires folder_id, new_folder_name)
+          create_folder_with_items - Create folder with items (requires folder_name)
+          update_view_prefs - Update organizer view preferences (requires view_prefs)
+          sync_templates - Sync Docker template paths
+          reset_template_mappings - Reset template mappings (confirm=True)
+          refresh_digests - Refresh container image digests
         """
         if action not in ALL_ACTIONS:
             raise ToolError(f"Invalid action '{action}'. Must be one of: {sorted(ALL_ACTIONS)}")
@@ -436,7 +447,10 @@ def register_docker_tool(mcp: FastMCP) -> None:
                 data = await make_graphql_request(
                     QUERIES["logs"], {"id": actual_id, "tail": tail_lines}
                 )
-                return {"logs": safe_get(data, "docker", "logs")}
+                logs = safe_get(data, "docker", "logs")
+                if logs is None:
+                    raise ToolError(f"No logs returned for container '{container_id}'")
+                return {"logs": logs}
 
             if action == "networks":
                 data = await make_graphql_request(QUERIES["networks"])
@@ -506,11 +520,10 @@ def register_docker_tool(mcp: FastMCP) -> None:
                 if children_ids is not None:
                     _vars["childrenIds"] = children_ids
                 data = await make_graphql_request(MUTATIONS["create_folder"], _vars)
-                return {
+                organizer = data.get("createDockerFolder")
-                    "success": True,
+                if organizer is None:
-                    "action": "create_folder",
+                    raise ToolError("create_folder failed: server returned no data")
-                    "organizer": data.get("createDockerFolder"),
+                return {"success": True, "action": "create_folder", "organizer": organizer}
-                }
 
             if action == "set_folder_children":
                 if children_ids is None:
@@ -519,11 +532,10 @@ def register_docker_tool(mcp: FastMCP) -> None:
                 if folder_id is not None:
                     _vars["folderId"] = folder_id
                 data = await make_graphql_request(MUTATIONS["set_folder_children"], _vars)
-                return {
+                organizer = data.get("setDockerFolderChildren")
-                    "success": True,
+                if organizer is None:
-                    "action": "set_folder_children",
+                    raise ToolError("set_folder_children failed: server returned no data")
-                    "organizer": data.get("setDockerFolderChildren"),
+                return {"success": True, "action": "set_folder_children", "organizer": organizer}
-                }
 
             if action == "delete_entries":
                 if not entry_ids:
@@ -531,11 +543,10 @@ def register_docker_tool(mcp: FastMCP) -> None:
                 data = await make_graphql_request(
                     MUTATIONS["delete_entries"], {"entryIds": entry_ids}
                 )
-                return {
+                organizer = data.get("deleteDockerEntries")
-                    "success": True,
+                if organizer is None:
-                    "action": "delete_entries",
+                    raise ToolError("delete_entries failed: server returned no data")
-                    "organizer": data.get("deleteDockerEntries"),
+                return {"success": True, "action": "delete_entries", "organizer": organizer}
-                }
 
             if action == "move_to_folder":
                 if not source_entry_ids:
@@ -549,11 +560,10 @@ def register_docker_tool(mcp: FastMCP) -> None:
                         "destinationFolderId": destination_folder_id,
                     },
                 )
-                return {
+                organizer = data.get("moveDockerEntriesToFolder")
-                    "success": True,
+                if organizer is None:
-                    "action": "move_to_folder",
+                    raise ToolError("move_to_folder failed: server returned no data")
-                    "organizer": data.get("moveDockerEntriesToFolder"),
+                return {"success": True, "action": "move_to_folder", "organizer": organizer}
-                }
 
             if action == "move_to_position":
                 if not source_entry_ids:
@@ -572,11 +582,10 @@ def register_docker_tool(mcp: FastMCP) -> None:
                         "position": position,
                     },
                 )
-                return {
+                organizer = data.get("moveDockerItemsToPosition")
-                    "success": True,
+                if organizer is None:
-                    "action": "move_to_position",
+                    raise ToolError("move_to_position failed: server returned no data")
-                    "organizer": data.get("moveDockerItemsToPosition"),
+                return {"success": True, "action": "move_to_position", "organizer": organizer}
-                }
 
             if action == "rename_folder":
                 if not folder_id:
@@ -586,11 +595,10 @@ def register_docker_tool(mcp: FastMCP) -> None:
                 data = await make_graphql_request(
                     MUTATIONS["rename_folder"], {"folderId": folder_id, "newName": new_folder_name}
                 )
-                return {
+                organizer = data.get("renameDockerFolder")
-                    "success": True,
+                if organizer is None:
-                    "action": "rename_folder",
+                    raise ToolError("rename_folder failed: server returned no data")
-                    "organizer": data.get("renameDockerFolder"),
+                return {"success": True, "action": "rename_folder", "organizer": organizer}
-                }
 
             if action == "create_folder_with_items":
                 if not folder_name:
@@ -603,10 +611,13 @@ def register_docker_tool(mcp: FastMCP) -> None:
                 if position is not None:
                     _vars["position"] = position
                 data = await make_graphql_request(MUTATIONS["create_folder_with_items"], _vars)
+                organizer = data.get("createDockerFolderWithItems")
+                if organizer is None:
+                    raise ToolError("create_folder_with_items failed: server returned no data")
                 return {
                     "success": True,
                     "action": "create_folder_with_items",
-                    "organizer": data.get("createDockerFolderWithItems"),
+                    "organizer": organizer,
                 }
 
             if action == "update_view_prefs":
@@ -615,19 +626,17 @@ def register_docker_tool(mcp: FastMCP) -> None:
                 data = await make_graphql_request(
                     MUTATIONS["update_view_prefs"], {"viewId": view_id, "prefs": view_prefs}
                 )
-                return {
+                organizer = data.get("updateDockerViewPreferences")
-                    "success": True,
+                if organizer is None:
-                    "action": "update_view_prefs",
+                    raise ToolError("update_view_prefs failed: server returned no data")
-                    "organizer": data.get("updateDockerViewPreferences"),
+                return {"success": True, "action": "update_view_prefs", "organizer": organizer}
-                }
 
             if action == "sync_templates":
                 data = await make_graphql_request(MUTATIONS["sync_templates"])
-                return {
+                result = data.get("syncDockerTemplatePaths")
-                    "success": True,
+                if result is None:
-                    "action": "sync_templates",
+                    raise ToolError("sync_templates failed: server returned no data")
-                    "result": data.get("syncDockerTemplatePaths"),
+                return {"success": True, "action": "sync_templates", "result": result}
-                }
 
             if action == "reset_template_mappings":
                 data = await make_graphql_request(MUTATIONS["reset_template_mappings"])

unraid_mcp/tools/notifications.py

@@ -261,7 +261,10 @@ def register_notifications_tool(mcp: FastMCP) -> None:
                     "importance": importance.upper(),
                 }
                 data = await make_graphql_request(MUTATIONS["create"], {"input": input_data})
-                return {"success": True, "data": data}
+                notification = data.get("createNotification")
+                if notification is None:
+                    raise ToolError("Notification creation failed: server returned no data")
+                return {"success": True, "notification": notification}
 
             if action in ("archive", "unread"):
                 if not notification_id:

unraid_mcp/tools/storage.py

@@ -204,7 +204,7 @@ def register_storage_tool(mcp: FastMCP) -> None:
                 return {"summary": summary, "details": raw}
 
             if action == "unassigned":
-                return {"devices": data.get("unassignedDevices", [])}
+                return {"devices": data.get("disks", [])}
 
             if action == "log_files":
                 return {"log_files": data.get("logFiles", [])}