fix(tools): remove 10 dead actions referencing mutations absent from live API

settings.py: drop update_temperature, update_time, update_api,
connect_sign_in, connect_sign_out, setup_remote_access,
enable_dynamic_remote_access, update_ssh — all 8 reference mutations
confirmed absent from Unraid API v4.29.2. Keep update + configure_ups.

info.py: drop update_server (updateServerIdentity not in Mutation type)
and update_ssh (duplicate of removed settings action). MUTATIONS is now
empty; DESTRUCTIVE_ACTIONS is now an empty set.

notifications.py: drop create_unique (notifyIfUnique not in Mutation type).

Tests: remove corresponding test classes, add parametrized regression
tests asserting removed actions are not in each tool's Literal type,
update KNOWN_DESTRUCTIVE and _DESTRUCTIVE_TEST_CASES in safety audit,
update schema coverage assertions. 858 tests passing, 0 failures.
This commit is contained in:
Jacob Magar
2026-03-15 23:21:25 -04:00
parent c37d4b1c5a
commit 4b43c47091
8 changed files with 59 additions and 661 deletions

View File

@@ -91,9 +91,6 @@ KNOWN_DESTRUCTIVE: dict[str, dict[str, set[str] | str]] = {
"tool_name": "unraid_settings",
"actions": {
"configure_ups",
"setup_remote_access",
"enable_dynamic_remote_access",
"update_ssh",
},
"runtime_set": SETTINGS_DESTRUCTIVE,
},
@@ -222,7 +219,6 @@ _DESTRUCTIVE_TEST_CASES: list[tuple[str, str, dict]] = [
),
# Settings
("settings", "configure_ups", {"ups_config": {"mode": "slave"}}),
("settings", "update_ssh", {"ssh_enabled": True, "ssh_port": 22}),
# Plugins
("plugins", "remove", {"names": ["my-plugin"]}),
]
@@ -461,16 +457,6 @@ class TestConfirmAllowsExecution:
)
assert result["success"] is True
async def test_settings_update_ssh_with_confirm(
self, _mock_settings_graphql: AsyncMock
) -> None:
_mock_settings_graphql.return_value = {"updateSshSettings": {"useSsh": True, "portssh": 22}}
tool_fn = make_tool_fn(
"unraid_mcp.tools.settings", "register_settings_tool", "unraid_settings"
)
result = await tool_fn(action="update_ssh", confirm=True, ssh_enabled=True, ssh_port=22)
assert result["success"] is True
async def test_array_remove_disk_with_confirm(self, _mock_array_graphql: AsyncMock) -> None:
_mock_array_graphql.return_value = {"array": {"removeDiskFromArray": {"state": "STOPPED"}}}
tool_fn = make_tool_fn("unraid_mcp.tools.array", "register_array_tool", "unraid_array")

View File

@@ -575,7 +575,6 @@ class TestNotificationMutations:
expected = {
"create",
"create_unique",
"archive",
"unread",
"delete",
@@ -739,14 +738,6 @@ class TestSettingsMutations:
expected = {
"update",
"configure_ups",
"update_temperature",
"update_time",
"update_api",
"connect_sign_in",
"connect_sign_out",
"setup_remote_access",
"enable_dynamic_remote_access",
"update_ssh",
}
assert set(MUTATIONS.keys()) == expected

View File

@@ -1,6 +1,7 @@
"""Tests for unraid_info tool."""
from collections.abc import Generator
from typing import get_args
from unittest.mock import AsyncMock, patch
import pytest
@@ -8,6 +9,7 @@ from conftest import make_tool_fn
from unraid_mcp.core.exceptions import ToolError
from unraid_mcp.tools.info import (
INFO_ACTIONS,
_analyze_disk_health,
_process_array_status,
_process_system_info,
@@ -303,62 +305,13 @@ class TestInfoNetworkErrors:
await tool_fn(action="network")
class TestInfoMutations:
async def test_update_server_requires_name(self, _mock_graphql: AsyncMock) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="server_name"):
await tool_fn(action="update_server")
# ---------------------------------------------------------------------------
# Regression: removed actions must not appear in INFO_ACTIONS
# ---------------------------------------------------------------------------
async def test_update_server_success(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {
"updateServerIdentity": {
"id": "s:1",
"name": "tootie",
"comment": None,
"status": "online",
}
}
tool_fn = _make_tool()
result = await tool_fn(action="update_server", server_name="tootie")
assert result["success"] is True
assert result["data"]["name"] == "tootie"
async def test_update_server_passes_optional_fields(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {
"updateServerIdentity": {"id": "s:1", "name": "x", "comment": None, "status": "online"}
}
tool_fn = _make_tool()
await tool_fn(action="update_server", server_name="x", sys_model="custom")
assert _mock_graphql.call_args[0][1]["sysModel"] == "custom"
async def test_update_ssh_requires_confirm(self, _mock_graphql: AsyncMock) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="confirm=True"):
await tool_fn(action="update_ssh", ssh_enabled=True, ssh_port=22)
async def test_update_ssh_requires_enabled(self, _mock_graphql: AsyncMock) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="ssh_enabled"):
await tool_fn(action="update_ssh", confirm=True, ssh_port=22)
async def test_update_ssh_requires_port(self, _mock_graphql: AsyncMock) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="ssh_port"):
await tool_fn(action="update_ssh", confirm=True, ssh_enabled=True)
async def test_update_ssh_success(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {
"updateSshSettings": {"id": "s:1", "useSsh": True, "portssh": 22}
}
tool_fn = _make_tool()
result = await tool_fn(action="update_ssh", confirm=True, ssh_enabled=True, ssh_port=22)
assert result["success"] is True
assert result["data"]["useSsh"] is True
async def test_update_ssh_passes_correct_input(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {
"updateSshSettings": {"id": "s:1", "useSsh": False, "portssh": 2222}
}
tool_fn = _make_tool()
await tool_fn(action="update_ssh", confirm=True, ssh_enabled=False, ssh_port=2222)
assert _mock_graphql.call_args[0][1] == {"input": {"enabled": False, "port": 2222}}
@pytest.mark.parametrize("action", ["update_server", "update_ssh"])
def test_removed_info_actions_are_gone(action: str) -> None:
assert action not in get_args(INFO_ACTIONS), (
f"{action} references a non-existent mutation and must not be in INFO_ACTIONS"
)

View File

@@ -17,6 +17,12 @@ def test_warnings_action_removed() -> None:
)
def test_create_unique_action_removed() -> None:
assert "create_unique" not in get_args(NOTIFICATION_ACTIONS), (
"create_unique references notifyIfUnique which is not in live API"
)
@pytest.fixture
def _mock_graphql() -> Generator[AsyncMock, None, None]:
with patch(
@@ -265,40 +271,6 @@ class TestNewNotificationMutations:
with pytest.raises(ToolError, match="notification_ids"):
await tool_fn(action="archive_many")
async def test_create_unique_success(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {
"notifyIfUnique": {"id": "n:1", "title": "Test", "importance": "INFO"}
}
tool_fn = _make_tool()
result = await tool_fn(
action="create_unique",
title="Test",
subject="Subj",
description="Desc",
importance="info",
)
assert result["success"] is True
async def test_create_unique_returns_none_when_duplicate(
self, _mock_graphql: AsyncMock
) -> None:
_mock_graphql.return_value = {"notifyIfUnique": None}
tool_fn = _make_tool()
result = await tool_fn(
action="create_unique",
title="T",
subject="S",
description="D",
importance="info",
)
assert result["success"] is True
assert result["duplicate"] is True
async def test_create_unique_requires_fields(self, _mock_graphql: AsyncMock) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="requires title"):
await tool_fn(action="create_unique")
async def test_unarchive_many_success(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {
"unarchiveNotifications": {

View File

@@ -1,15 +1,14 @@
"""Tests for the unraid_settings tool."""
from __future__ import annotations
from collections.abc import Generator
from typing import get_args
from unittest.mock import AsyncMock, patch
import pytest
from fastmcp import FastMCP
from unraid_mcp.core.exceptions import ToolError
from unraid_mcp.tools.settings import register_settings_tool
from unraid_mcp.tools.settings import SETTINGS_ACTIONS, register_settings_tool
@pytest.fixture
@@ -27,6 +26,35 @@ def _make_tool() -> AsyncMock:
return tool.fn # type: ignore[union-attr]
# ---------------------------------------------------------------------------
# Regression: removed actions must not appear in SETTINGS_ACTIONS
# ---------------------------------------------------------------------------
@pytest.mark.parametrize(
"action",
[
"update_temperature",
"update_time",
"update_api",
"connect_sign_in",
"connect_sign_out",
"setup_remote_access",
"enable_dynamic_remote_access",
"update_ssh",
],
)
def test_removed_settings_actions_are_gone(action: str) -> None:
assert action not in get_args(SETTINGS_ACTIONS), (
f"{action} references a non-existent mutation and must not be in SETTINGS_ACTIONS"
)
# ---------------------------------------------------------------------------
# Validation
# ---------------------------------------------------------------------------
class TestSettingsValidation:
"""Tests for action validation and destructive guard."""
@@ -42,26 +70,10 @@ class TestSettingsValidation:
with pytest.raises(ToolError, match="confirm=True"):
await tool_fn(action="configure_ups", ups_config={"mode": "slave"})
async def test_destructive_setup_remote_access_requires_confirm(
self, _mock_graphql: AsyncMock
) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="confirm=True"):
await tool_fn(action="setup_remote_access", access_type="STATIC")
async def test_destructive_enable_dynamic_remote_access_requires_confirm(
self, _mock_graphql: AsyncMock
) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="confirm=True"):
await tool_fn(
action="enable_dynamic_remote_access", access_url_type="WAN", dynamic_enabled=True
)
async def test_destructive_update_ssh_requires_confirm(self, _mock_graphql: AsyncMock) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="confirm=True"):
await tool_fn(action="update_ssh", ssh_enabled=True, ssh_port=22)
# ---------------------------------------------------------------------------
# update
# ---------------------------------------------------------------------------
class TestSettingsUpdate:
@@ -81,54 +93,10 @@ class TestSettingsUpdate:
assert result["success"] is True
assert result["action"] == "update"
async def test_update_temperature_requires_config(self, _mock_graphql: AsyncMock) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="temperature_config is required"):
await tool_fn(action="update_temperature")
async def test_update_temperature_success(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {"updateTemperatureConfig": True}
tool_fn = _make_tool()
result = await tool_fn(action="update_temperature", temperature_config={"unit": "C"})
assert result["success"] is True
assert result["action"] == "update_temperature"
class TestSystemTime:
"""Tests for update_time action."""
async def test_update_time_requires_at_least_one_field(self, _mock_graphql: AsyncMock) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="update_time requires"):
await tool_fn(action="update_time")
async def test_update_time_with_timezone(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {
"updateSystemTime": {
"currentTime": "2026-03-13T00:00:00Z",
"timeZone": "America/New_York",
"useNtp": True,
"ntpServers": [],
}
}
tool_fn = _make_tool()
result = await tool_fn(action="update_time", time_zone="America/New_York")
assert result["success"] is True
assert result["action"] == "update_time"
async def test_update_time_with_ntp(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {
"updateSystemTime": {"useNtp": True, "ntpServers": ["0.pool.ntp.org"]}
}
tool_fn = _make_tool()
result = await tool_fn(action="update_time", use_ntp=True, ntp_servers=["0.pool.ntp.org"])
assert result["success"] is True
async def test_update_time_manual(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {"updateSystemTime": {"currentTime": "2026-03-13T12:00:00Z"}}
tool_fn = _make_tool()
result = await tool_fn(action="update_time", manual_datetime="2026-03-13T12:00:00Z")
assert result["success"] is True
# ---------------------------------------------------------------------------
# configure_ups
# ---------------------------------------------------------------------------
class TestUpsConfig:
@@ -147,157 +115,3 @@ class TestUpsConfig:
)
assert result["success"] is True
assert result["action"] == "configure_ups"
class TestApiSettings:
"""Tests for update_api action."""
async def test_update_api_requires_at_least_one_field(self, _mock_graphql: AsyncMock) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="update_api requires"):
await tool_fn(action="update_api")
async def test_update_api_with_port(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {
"updateApiSettings": {"accessType": "STATIC", "forwardType": "NONE", "port": 8080}
}
tool_fn = _make_tool()
result = await tool_fn(action="update_api", port=8080)
assert result["success"] is True
assert result["action"] == "update_api"
async def test_update_api_with_access_type(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {"updateApiSettings": {"accessType": "STATIC"}}
tool_fn = _make_tool()
result = await tool_fn(action="update_api", access_type="STATIC")
assert result["success"] is True
class TestConnectActions:
"""Tests for connect_sign_in and connect_sign_out actions."""
async def test_connect_sign_in_requires_api_key(self, _mock_graphql: AsyncMock) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="api_key is required"):
await tool_fn(action="connect_sign_in")
async def test_connect_sign_in_success(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {"connectSignIn": True}
tool_fn = _make_tool()
result = await tool_fn(action="connect_sign_in", api_key="test-api-key-abc123")
assert result["success"] is True
assert result["action"] == "connect_sign_in"
async def test_connect_sign_in_with_user_info(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {"connectSignIn": True}
tool_fn = _make_tool()
result = await tool_fn(
action="connect_sign_in",
api_key="test-api-key",
username="testuser",
email="test@example.com",
avatar="https://example.com/avatar.png",
)
assert result["success"] is True
async def test_connect_sign_out_success(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {"connectSignOut": True}
tool_fn = _make_tool()
result = await tool_fn(action="connect_sign_out")
assert result["success"] is True
assert result["action"] == "connect_sign_out"
class TestRemoteAccess:
"""Tests for setup_remote_access and enable_dynamic_remote_access actions."""
async def test_setup_remote_access_requires_access_type(self, _mock_graphql: AsyncMock) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="access_type is required"):
await tool_fn(action="setup_remote_access", confirm=True)
async def test_setup_remote_access_success(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {"setupRemoteAccess": True}
tool_fn = _make_tool()
result = await tool_fn(action="setup_remote_access", confirm=True, access_type="STATIC")
assert result["success"] is True
assert result["action"] == "setup_remote_access"
async def test_setup_remote_access_with_port(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {"setupRemoteAccess": True}
tool_fn = _make_tool()
result = await tool_fn(
action="setup_remote_access",
confirm=True,
access_type="STATIC",
forward_type="UPNP",
port=9999,
)
assert result["success"] is True
async def test_enable_dynamic_requires_url_type(self, _mock_graphql: AsyncMock) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="access_url_type is required"):
await tool_fn(action="enable_dynamic_remote_access", confirm=True, dynamic_enabled=True)
async def test_enable_dynamic_requires_dynamic_enabled(self, _mock_graphql: AsyncMock) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="dynamic_enabled is required"):
await tool_fn(
action="enable_dynamic_remote_access", confirm=True, access_url_type="WAN"
)
async def test_enable_dynamic_success(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {"enableDynamicRemoteAccess": True}
tool_fn = _make_tool()
result = await tool_fn(
action="enable_dynamic_remote_access",
confirm=True,
access_url_type="WAN",
dynamic_enabled=True,
)
assert result["success"] is True
assert result["action"] == "enable_dynamic_remote_access"
async def test_enable_dynamic_with_optional_fields(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {"enableDynamicRemoteAccess": True}
tool_fn = _make_tool()
result = await tool_fn(
action="enable_dynamic_remote_access",
confirm=True,
access_url_type="WAN",
dynamic_enabled=False,
access_url_name="myserver",
access_url_ipv4="1.2.3.4",
access_url_ipv6="::1",
)
assert result["success"] is True
class TestSshSettings:
"""Tests for update_ssh action."""
async def test_update_ssh_requires_ssh_enabled(self, _mock_graphql: AsyncMock) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="ssh_enabled is required"):
await tool_fn(action="update_ssh", confirm=True, ssh_port=22)
async def test_update_ssh_requires_ssh_port(self, _mock_graphql: AsyncMock) -> None:
tool_fn = _make_tool()
with pytest.raises(ToolError, match="ssh_port is required"):
await tool_fn(action="update_ssh", confirm=True, ssh_enabled=True)
async def test_update_ssh_success(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {"updateSshSettings": {"useSsh": True, "portssh": 22}}
tool_fn = _make_tool()
result = await tool_fn(action="update_ssh", confirm=True, ssh_enabled=True, ssh_port=22)
assert result["success"] is True
assert result["action"] == "update_ssh"
async def test_update_ssh_disable(self, _mock_graphql: AsyncMock) -> None:
_mock_graphql.return_value = {"updateSshSettings": {"useSsh": False, "portssh": 22}}
tool_fn = _make_tool()
result = await tool_fn(action="update_ssh", confirm=True, ssh_enabled=False, ssh_port=22)
assert result["success"] is True
call_vars = _mock_graphql.call_args[0][1]
assert call_vars["input"] == {"enabled": False, "port": 22}