fix: flash_backup validation, smoke test assertions, docker/notification test coverage

- storage.py: validate initiateFlashBackup response before returning success=True
- test-tools.sh: remove set -e/inherit_errexit; add success assertion to smoke tests
- test_destructive_guards.py: add confirm-guard tests for new docker destructive actions
- test_docker.py: assert mutation variables in organizer tests; add items branch test
- test_query_validation.py: add 5 missing notification mutation schema test methods
- test_notifications.py: use lowercase importance to test uppercasing logic

Resolves review threads PRRT_kwDOO6Hdxs50FgPb PRRT_kwDOO6Hdxs50FgO4 PRRT_kwDOO6Hdxs50FgO8 PRRT_kwDOO6Hdxs50FgPI PRRT_kwDOO6Hdxs50FgPL PRRT_kwDOO6Hdxs50FgPm PRRT_kwDOO6Hdxs50E2iK PRRT_kwDOO6Hdxs50E2im

scripts/test-tools.sh

@@ -20,8 +20,7 @@
 #   2 — prerequisite check failed (mcporter, uv, server startup)
 # =============================================================================
 
-set -Eeuo pipefail
-shopt -s inherit_errexit
+set -uo pipefail
 
 # ---------------------------------------------------------------------------
 # Constants
@@ -178,6 +177,29 @@ smoke_test_server() {
     return 2
   fi
 
+  # Assert the response contains a valid tool response field, not a bare JSON error.
+  # unraid_health action=check always returns {"status": ...} on success.
+  local key_check
+  key_check="$(
+    printf '%s' "${output}" | python3 -c "
+import sys, json
+try:
+    d = json.load(sys.stdin)
+    if 'status' in d or 'success' in d or 'error' in d:
+        print('ok')
+    else:
+        print('missing: no status/success/error key in response')
+except Exception as e:
+    print('parse_error: ' + str(e))
+" 2>/dev/null
+  )" || key_check="parse_error"
+
+  if [[ "${key_check}" != "ok" ]]; then
+    log_error "Smoke test: unexpected response shape — ${key_check}"
+    printf '%s\n' "${output}" >&2
+    return 2
+  fi
+
   log_info "Server started successfully (health response received)."
   return 0
 }

tests/safety/test_destructive_guards.py

@@ -148,6 +148,8 @@ _DESTRUCTIVE_TEST_CASES: list[tuple[str, str, dict]] = [
     # Docker
     ("docker", "remove", {"container_id": "abc123"}),
     ("docker", "update_all", {}),
+    ("docker", "delete_entries", {"entry_ids": ["e1"]}),
+    ("docker", "reset_template_mappings", {}),
     # VM
     ("vm", "force_stop", {"vm_id": "test-vm-uuid"}),
     ("vm", "reset", {"vm_id": "test-vm-uuid"}),
@@ -290,6 +292,28 @@ class TestConfirmAllowsExecution:
         assert result["success"] is True
         assert result["action"] == "update_all"
 
+    async def test_docker_delete_entries_with_confirm(
+        self, _mock_docker_graphql: AsyncMock
+    ) -> None:
+        organizer_response = {
+            "version": 1.0,
+            "views": [{"id": "default", "name": "Default", "rootId": "root", "flatEntries": []}],
+        }
+        _mock_docker_graphql.return_value = {"deleteDockerEntries": organizer_response}
+        tool_fn = make_tool_fn("unraid_mcp.tools.docker", "register_docker_tool", "unraid_docker")
+        result = await tool_fn(action="delete_entries", entry_ids=["e1"], confirm=True)
+        assert result["success"] is True
+        assert result["action"] == "delete_entries"
+
+    async def test_docker_reset_template_mappings_with_confirm(
+        self, _mock_docker_graphql: AsyncMock
+    ) -> None:
+        _mock_docker_graphql.return_value = {"resetDockerTemplateMappings": True}
+        tool_fn = make_tool_fn("unraid_mcp.tools.docker", "register_docker_tool", "unraid_docker")
+        result = await tool_fn(action="reset_template_mappings", confirm=True)
+        assert result["success"] is True
+        assert result["action"] == "reset_template_mappings"
+
     async def test_docker_remove_with_confirm(self, _mock_docker_graphql: AsyncMock) -> None:
         cid = "a" * 64 + ":local"
         _mock_docker_graphql.side_effect = [

tests/schema/test_query_validation.py

@@ -537,6 +537,36 @@ class TestNotificationMutations:
         errors = _validate_operation(schema, MUTATIONS["archive_all"])
         assert not errors, f"archive_all mutation validation failed: {errors}"
 
+    def test_archive_many_mutation(self, schema: GraphQLSchema) -> None:
+        from unraid_mcp.tools.notifications import MUTATIONS
+
+        errors = _validate_operation(schema, MUTATIONS["archive_many"])
+        assert not errors, f"archive_many mutation validation failed: {errors}"
+
+    def test_create_unique_mutation(self, schema: GraphQLSchema) -> None:
+        from unraid_mcp.tools.notifications import MUTATIONS
+
+        errors = _validate_operation(schema, MUTATIONS["create_unique"])
+        assert not errors, f"create_unique mutation validation failed: {errors}"
+
+    def test_unarchive_many_mutation(self, schema: GraphQLSchema) -> None:
+        from unraid_mcp.tools.notifications import MUTATIONS
+
+        errors = _validate_operation(schema, MUTATIONS["unarchive_many"])
+        assert not errors, f"unarchive_many mutation validation failed: {errors}"
+
+    def test_unarchive_all_mutation(self, schema: GraphQLSchema) -> None:
+        from unraid_mcp.tools.notifications import MUTATIONS
+
+        errors = _validate_operation(schema, MUTATIONS["unarchive_all"])
+        assert not errors, f"unarchive_all mutation validation failed: {errors}"
+
+    def test_recalculate_mutation(self, schema: GraphQLSchema) -> None:
+        from unraid_mcp.tools.notifications import MUTATIONS
+
+        errors = _validate_operation(schema, MUTATIONS["recalculate"])
+        assert not errors, f"recalculate mutation validation failed: {errors}"
+
     def test_all_notification_mutations_covered(self, schema: GraphQLSchema) -> None:
         from unraid_mcp.tools.notifications import MUTATIONS
 

tests/test_docker.py

@@ -70,7 +70,9 @@ class TestDockerValidation:
             await tool_fn(action="remove", container_id="abc123")
 
     @pytest.mark.parametrize("action", ["start", "stop", "details", "logs", "pause", "unpause"])
-    async def test_container_actions_require_id(self, _mock_graphql: AsyncMock, action: str) -> None:
+    async def test_container_actions_require_id(
+        self, _mock_graphql: AsyncMock, action: str
+    ) -> None:
         tool_fn = _make_tool()
         with pytest.raises(ToolError, match="container_id"):
             await tool_fn(action=action)
@@ -102,13 +104,7 @@ class TestDockerActions:
         # First call resolves ID, second performs start
         cid = "a" * 64 + ":local"
         _mock_graphql.side_effect = [
-            {
-                "docker": {
-                    "containers": [
-                        {"id": cid, "names": ["plex"]}
-                    ]
-                }
-            },
+            {"docker": {"containers": [{"id": cid, "names": ["plex"]}]}},
             {
                 "docker": {
                     "start": {
@@ -239,8 +235,14 @@ class TestDockerActions:
         _mock_graphql.return_value = {
             "docker": {
                 "containers": [
-                    {"id": "abcdef1234560000000000000000000000000000000000000000000000000000:local", "names": ["plex"]},
-                    {"id": "abcdef1234561111111111111111111111111111111111111111111111111111:local", "names": ["sonarr"]},
+                    {
+                        "id": "abcdef1234560000000000000000000000000000000000000000000000000000:local",
+                        "names": ["plex"],
+                    },
+                    {
+                        "id": "abcdef1234561111111111111111111111111111111111111111111111111111:local",
+                        "names": ["sonarr"],
+                    },
                 ]
             }
         }
@@ -344,7 +346,10 @@ class TestDockerNetworkErrors:
             await tool_fn(action="list")
 
 
-_ORGANIZER_RESPONSE = {"version": 1.0, "views": [{"id": "default", "name": "Default", "rootId": "root", "flatEntries": []}]}
+_ORGANIZER_RESPONSE = {
+    "version": 1.0,
+    "views": [{"id": "default", "name": "Default", "rootId": "root", "flatEntries": []}],
+}
 
 
 class TestDockerOrganizerMutations:
@@ -352,6 +357,8 @@ class TestDockerOrganizerMutations:
         _mock_graphql.return_value = {"createDockerFolder": _ORGANIZER_RESPONSE}
         result = await _make_tool()(action="create_folder", folder_name="Media")
         assert result["success"] is True
+        call_vars = _mock_graphql.call_args[0][1]
+        assert call_vars["name"] == "Media"
 
     async def test_create_folder_requires_name(self, _mock_graphql: AsyncMock) -> None:
         with pytest.raises(ToolError, match="folder_name"):
@@ -361,6 +368,8 @@ class TestDockerOrganizerMutations:
         _mock_graphql.return_value = {"setDockerFolderChildren": _ORGANIZER_RESPONSE}
         result = await _make_tool()(action="set_folder_children", children_ids=["c1"])
         assert result["success"] is True
+        call_vars = _mock_graphql.call_args[0][1]
+        assert call_vars["childrenIds"] == ["c1"]
 
     async def test_set_folder_children_requires_children(self, _mock_graphql: AsyncMock) -> None:
         with pytest.raises(ToolError, match="children_ids"):
@@ -376,13 +385,20 @@ class TestDockerOrganizerMutations:
 
     async def test_delete_entries_success(self, _mock_graphql: AsyncMock) -> None:
         _mock_graphql.return_value = {"deleteDockerEntries": _ORGANIZER_RESPONSE}
-        result = await _make_tool()(action="delete_entries", entry_ids=["e1"], confirm=True)
+        result = await _make_tool()(action="delete_entries", entry_ids=["e1", "e2"], confirm=True)
         assert result["success"] is True
+        call_vars = _mock_graphql.call_args[0][1]
+        assert call_vars["entryIds"] == ["e1", "e2"]
 
     async def test_move_to_folder_success(self, _mock_graphql: AsyncMock) -> None:
         _mock_graphql.return_value = {"moveDockerEntriesToFolder": _ORGANIZER_RESPONSE}
-        result = await _make_tool()(action="move_to_folder", source_entry_ids=["e1"], destination_folder_id="f1")
+        result = await _make_tool()(
+            action="move_to_folder", source_entry_ids=["e1"], destination_folder_id="f1"
+        )
         assert result["success"] is True
+        call_vars = _mock_graphql.call_args[0][1]
+        assert call_vars["sourceEntryIds"] == ["e1"]
+        assert call_vars["destinationFolderId"] == "f1"
 
     async def test_move_to_folder_requires_source_ids(self, _mock_graphql: AsyncMock) -> None:
         with pytest.raises(ToolError, match="source_entry_ids"):
@@ -394,17 +410,31 @@ class TestDockerOrganizerMutations:
 
     async def test_move_to_position_success(self, _mock_graphql: AsyncMock) -> None:
         _mock_graphql.return_value = {"moveDockerItemsToPosition": _ORGANIZER_RESPONSE}
-        result = await _make_tool()(action="move_to_position", source_entry_ids=["e1"], destination_folder_id="f1", position=2.0)
+        result = await _make_tool()(
+            action="move_to_position",
+            source_entry_ids=["e1"],
+            destination_folder_id="f1",
+            position=2.0,
+        )
         assert result["success"] is True
+        call_vars = _mock_graphql.call_args[0][1]
+        assert call_vars["sourceEntryIds"] == ["e1"]
+        assert call_vars["destinationFolderId"] == "f1"
+        assert call_vars["position"] == 2.0
 
     async def test_move_to_position_requires_position(self, _mock_graphql: AsyncMock) -> None:
         with pytest.raises(ToolError, match="position"):
-            await _make_tool()(action="move_to_position", source_entry_ids=["e1"], destination_folder_id="f1")
+            await _make_tool()(
+                action="move_to_position", source_entry_ids=["e1"], destination_folder_id="f1"
+            )
 
     async def test_rename_folder_success(self, _mock_graphql: AsyncMock) -> None:
         _mock_graphql.return_value = {"renameDockerFolder": _ORGANIZER_RESPONSE}
         result = await _make_tool()(action="rename_folder", folder_id="f1", new_folder_name="New")
         assert result["success"] is True
+        call_vars = _mock_graphql.call_args[0][1]
+        assert call_vars["folderId"] == "f1"
+        assert call_vars["newName"] == "New"
 
     async def test_rename_folder_requires_folder_id(self, _mock_graphql: AsyncMock) -> None:
         with pytest.raises(ToolError, match="folder_id"):
@@ -418,6 +448,22 @@ class TestDockerOrganizerMutations:
         _mock_graphql.return_value = {"createDockerFolderWithItems": _ORGANIZER_RESPONSE}
         result = await _make_tool()(action="create_folder_with_items", folder_name="New")
         assert result["success"] is True
+        call_vars = _mock_graphql.call_args[0][1]
+        assert call_vars["name"] == "New"
+        assert "sourceEntryIds" not in call_vars  # not forwarded when not provided
+
+    async def test_create_folder_with_items_with_source_ids(self, _mock_graphql: AsyncMock) -> None:
+        """Passing source_entry_ids must forward sourceEntryIds to the mutation."""
+        _mock_graphql.return_value = {"createDockerFolderWithItems": _ORGANIZER_RESPONSE}
+        result = await _make_tool()(
+            action="create_folder_with_items",
+            folder_name="Media",
+            source_entry_ids=["c1", "c2"],
+        )
+        assert result["success"] is True
+        call_vars = _mock_graphql.call_args[0][1]
+        assert call_vars["name"] == "Media"
+        assert call_vars["sourceEntryIds"] == ["c1", "c2"]
 
     async def test_create_folder_with_items_requires_name(self, _mock_graphql: AsyncMock) -> None:
         with pytest.raises(ToolError, match="folder_name"):
@@ -427,13 +473,17 @@ class TestDockerOrganizerMutations:
         _mock_graphql.return_value = {"updateDockerViewPreferences": _ORGANIZER_RESPONSE}
         result = await _make_tool()(action="update_view_prefs", view_prefs={"sort": "name"})
         assert result["success"] is True
+        call_vars = _mock_graphql.call_args[0][1]
+        assert call_vars["prefs"] == {"sort": "name"}
 
     async def test_update_view_prefs_requires_prefs(self, _mock_graphql: AsyncMock) -> None:
         with pytest.raises(ToolError, match="view_prefs"):
             await _make_tool()(action="update_view_prefs")
 
     async def test_sync_templates_success(self, _mock_graphql: AsyncMock) -> None:
-        _mock_graphql.return_value = {"syncDockerTemplatePaths": {"scanned": 5, "matched": 4, "skipped": 1, "errors": []}}
+        _mock_graphql.return_value = {
+            "syncDockerTemplatePaths": {"scanned": 5, "matched": 4, "skipped": 1, "errors": []}
+        }
         result = await _make_tool()(action="sync_templates")
         assert result["success"] is True
 

tests/test_notifications.py

@@ -327,11 +327,12 @@ class TestNewNotificationMutations:
         assert result["success"] is True
 
     async def test_unarchive_all_with_importance(self, _mock_graphql: AsyncMock) -> None:
+        """Lowercase importance input must be uppercased before being sent to GraphQL."""
         _mock_graphql.return_value = {
             "unarchiveAll": {"unread": {"total": 1}, "archive": {"total": 0}}
         }
         tool_fn = _make_tool()
-        await tool_fn(action="unarchive_all", importance="WARNING")
+        await tool_fn(action="unarchive_all", importance="warning")
         call_args = _mock_graphql.call_args
         assert call_args[0][1] == {"importance": "WARNING"}
 

unraid_mcp/tools/storage.py

@@ -157,10 +157,13 @@ def register_storage_tool(mcp: FastMCP) -> None:
             with tool_error_handler("storage", action, logger):
                 logger.info("Executing unraid_storage action=flash_backup")
                 data = await make_graphql_request(MUTATIONS["flash_backup"], {"input": input_data})
+                backup = data.get("initiateFlashBackup")
+                if not backup:
+                    raise ToolError("Failed to start flash backup: no confirmation from server")
                 return {
                     "success": True,
                     "action": "flash_backup",
-                    "data": data.get("initiateFlashBackup"),
+                    "data": backup,
                 }
 
         query = QUERIES[action]