fix: address PR review threads - test assertions, ruff violations, format_kb consistency

Resolves review threads:
- PRRT_kwDOO6Hdxs5vNroH (Thread 36): tests now verify generic ToolError message
  instead of raw exception text (security: no sensitive data in user-facing errors)
- PRRT_kwDOO6Hdxs5vNuYg (Thread 14): format_kb KB branch now uses :.2f like all
  other branches (consistency fix)
- I001/F841/PERF401: fix ruff violations in http_layer, integration, safety tests

Changes:
- tests/test_array.py: match "Failed to execute array/parity_status" (not raw error)
- tests/test_keys.py: match "Failed to execute keys/list" (not raw error)
- tests/test_notifications.py: match "Failed to execute notifications/overview" (not raw error)
- tests/test_storage.py: update format_kb assertion to "512.00 KB" (:.2f format)
- tests/http_layer/test_request_construction.py: remove unused result var (F841)
  + fix import sort (I001)
- tests/safety/test_destructive_guards.py: use list.extend (PERF401) + fix import sort
- unraid_mcp/core/utils.py: format_kb returns f"{k:.2f} KB" for sub-MB values

Co-authored-by: @coderabbitai
Co-authored-by: @cubic-dev-ai
Co-authored-by: @copilot-pull-request-reviewer

tests/http_layer/test_request_construction.py

@@ -19,6 +19,7 @@ from tests.conftest import make_tool_fn
 from unraid_mcp.core.client import DEFAULT_TIMEOUT, DISK_TIMEOUT, make_graphql_request
 from unraid_mcp.core.exceptions import ToolError
 
+
 # ---------------------------------------------------------------------------
 # Shared fixtures
 # ---------------------------------------------------------------------------
@@ -582,7 +583,7 @@ class TestVMToolRequests:
             return_value=_graphql_response({"vm": {"stop": True}})
         )
         tool = self._get_tool()
-        result = await tool(action="stop", vm_id="vm-456")
+        await tool(action="stop", vm_id="vm-456")
         body = _extract_request_body(route.calls.last.request)
         assert "StopVM" in body["query"]
         assert body["variables"] == {"id": "vm-456"}

tests/safety/test_destructive_guards.py

@@ -10,6 +10,10 @@ from unittest.mock import AsyncMock, patch
 
 import pytest
 
+# Centralized import for make_tool_fn helper
+# conftest.py sits in tests/ and is importable without __init__.py
+from conftest import make_tool_fn
+
 from unraid_mcp.core.exceptions import ToolError
 
 # Import DESTRUCTIVE_ACTIONS sets from every tool module that defines one
@@ -24,10 +28,6 @@ from unraid_mcp.tools.rclone import MUTATIONS as RCLONE_MUTATIONS
 from unraid_mcp.tools.virtualization import DESTRUCTIVE_ACTIONS as VM_DESTRUCTIVE
 from unraid_mcp.tools.virtualization import MUTATIONS as VM_MUTATIONS
 
-# Centralized import for make_tool_fn helper
-# conftest.py sits in tests/ and is importable without __init__.py
-from conftest import make_tool_fn
-
 
 # ---------------------------------------------------------------------------
 # Known destructive actions registry (ground truth for this audit)
@@ -126,9 +126,11 @@ class TestDestructiveActionRegistries:
         missing: list[str] = []
         for tool_key, mutations in all_mutations.items():
             destructive = all_destructive[tool_key]
-            for action_name in mutations:
-                if ("delete" in action_name or "remove" in action_name) and action_name not in destructive:
-                    missing.append(f"{tool_key}/{action_name}")
+            missing.extend(
+                f"{tool_key}/{action_name}"
+                for action_name in mutations
+                if ("delete" in action_name or "remove" in action_name) and action_name not in destructive
+            )
         assert not missing, (
             f"Mutations with 'delete'/'remove' not in DESTRUCTIVE_ACTIONS: {missing}"
         )

tests/test_array.py

@@ -84,7 +84,7 @@ class TestArrayActions:
     async def test_generic_exception_wraps(self, _mock_graphql: AsyncMock) -> None:
         _mock_graphql.side_effect = RuntimeError("disk error")
         tool_fn = _make_tool()
-        with pytest.raises(ToolError, match="disk error"):
+        with pytest.raises(ToolError, match="Failed to execute array/parity_status"):
             await tool_fn(action="parity_status")
 
 

tests/test_keys.py

@@ -100,5 +100,5 @@ class TestKeysActions:
     async def test_generic_exception_wraps(self, _mock_graphql: AsyncMock) -> None:
         _mock_graphql.side_effect = RuntimeError("connection lost")
         tool_fn = _make_tool()
-        with pytest.raises(ToolError, match="connection lost"):
+        with pytest.raises(ToolError, match="Failed to execute keys/list"):
             await tool_fn(action="list")

tests/test_notifications.py

@@ -149,7 +149,7 @@ class TestNotificationsActions:
     async def test_generic_exception_wraps(self, _mock_graphql: AsyncMock) -> None:
         _mock_graphql.side_effect = RuntimeError("boom")
         tool_fn = _make_tool()
-        with pytest.raises(ToolError, match="boom"):
+        with pytest.raises(ToolError, match="Failed to execute notifications/overview"):
             await tool_fn(action="overview")
 
 

tests/test_storage.py

@@ -93,6 +93,14 @@ class TestStorageValidation:
         result = await tool_fn(action="logs", log_path="/var/log/syslog", tail_lines=10_000)
         assert result["content"] == "ok"
 
+    async def test_non_logs_action_ignores_tail_lines_validation(
+        self, _mock_graphql: AsyncMock
+    ) -> None:
+        _mock_graphql.return_value = {"shares": []}
+        tool_fn = _make_tool()
+        result = await tool_fn(action="shares", tail_lines=0)
+        assert result["shares"] == []
+
 
 class TestFormatKb:
     def test_none_returns_na(self) -> None:
@@ -102,7 +110,7 @@ class TestFormatKb:
         assert format_kb("not-a-number") == "N/A"
 
     def test_kilobytes_range(self) -> None:
-        assert format_kb(512) == "512 KB"
+        assert format_kb(512) == "512.00 KB"
 
     def test_megabytes_range(self) -> None:
         assert format_kb(2048) == "2.00 MB"

unraid_mcp/core/utils.py

@@ -13,6 +13,7 @@ def safe_get(data: dict[str, Any], *keys: str, default: Any = None) -> Any:
 
     Returns:
         The value at the end of the key chain, or default if unreachable.
+        Explicit ``None`` values at the final key also return ``default``.
     """
     current = data
     for key in keys:
@@ -65,4 +66,4 @@ def format_kb(k: Any) -> str:
         return f"{k / (1024 * 1024):.2f} GB"
     if k >= 1024:
         return f"{k / 1024:.2f} MB"
-    return f"{k} KB"
+    return f"{k:.2f} KB"