fix: address 17 remaining PR review comments

Resolves review threads:
- PRRT_kwDOO6Hdxs50mcYz: oidc/validate_session now documents required `token`
- PRRT_kwDOO6Hdxs50mcY8: setting/update corrected to require `settings_input`
- PRRT_kwDOO6Hdxs50mcZE: rclone/create_remote corrected to `provider_type`+`config_data`
- PRRT_kwDOO6Hdxs50mcZL: disk/logs corrected to `log_path`+`tail_lines`
- PRRT_kwDOO6Hdxs50mcZe: parity_progress added to event-driven subscriptions list
- PRRT_kwDOO6Hdxs50mcZh: log_tail README example now includes required `path`
- PRRT_kwDOO6Hdxs50mcaR: parity_start quick-reference now includes required `correct=False`
- PRRT_kwDOO6Hdxs50mcaq: array_state documented as "may show" not "will always show"
- PRRT_kwDOO6Hdxs50mnR8: key/create roles is optional; add_role/remove_role use `roles` (plural)
- PRRT_kwDOO6Hdxs50mnRd: endpoints.md heading moved before blockquote (MD041)
- PRRT_kwDOO6Hdxs50mnTB: test_resources.py uses _get_resource() helper instead of raw internals
- PRRT_kwDOO6Hdxs50mYkZ: N/A — _build_google_auth removed in prior refactor commit
- PRRT_kwDOO6Hdxs50mnQf: N/A — plugin.json already at 1.1.2, matches pyproject.toml
- PRRT_kwDOO6Hdxs50mnQ7: N/A — blank line already present in CLAUDE.md
- PRRT_kwDOO6Hdxs50mnRD: N/A — fastmcp.http.json removed in prior refactor commit
- PRRT_kwDOO6Hdxs50mnRH: N/A — blank line already present in README.md
- PRRT_kwDOO6Hdxs50mnSW: N/A — test_auth_builder.py removed in prior refactor commit
This commit is contained in:
Jacob Magar
2026-03-24 22:50:40 -04:00
parent e548f6e6c9
commit 183db70d97
6 changed files with 31 additions and 37 deletions

View File

@@ -17,6 +17,16 @@ def _make_resources():
return test_mcp
def _get_resource(mcp: FastMCP, uri: str):
"""Look up a registered resource by URI.
Accesses FastMCP provider internals. If this breaks after a FastMCP upgrade,
check whether a public resource-lookup API has been added upstream.
"""
key = f"resource:{uri}@"
return mcp.providers[0]._components[key]
@pytest.fixture
def _mock_ensure_started():
with patch(
@@ -36,9 +46,7 @@ class TestLiveResourcesUseManagerCache:
with patch("unraid_mcp.subscriptions.resources.subscription_manager") as mock_mgr:
mock_mgr.get_resource_data = AsyncMock(return_value=cached)
mcp = _make_resources()
# Accessing FastMCP internals intentionally for unit test isolation.
# This may break on FastMCP upgrades — consider a make_resource_fn() helper if it does.
resource = mcp.providers[0]._components[f"resource:unraid://live/{action}@"]
resource = _get_resource(mcp, f"unraid://live/{action}")
result = await resource.fn()
assert json.loads(result) == cached
@@ -51,9 +59,7 @@ class TestLiveResourcesUseManagerCache:
mock_mgr.get_resource_data = AsyncMock(return_value=None)
mock_mgr.last_error = {}
mcp = _make_resources()
# Accessing FastMCP internals intentionally for unit test isolation.
# This may break on FastMCP upgrades — consider a make_resource_fn() helper if it does.
resource = mcp.providers[0]._components[f"resource:unraid://live/{action}@"]
resource = _get_resource(mcp, f"unraid://live/{action}")
result = await resource.fn()
parsed = json.loads(result)
assert parsed["status"] == "connecting"
@@ -67,9 +73,7 @@ class TestLiveResourcesUseManagerCache:
mock_mgr.connection_states = {action: "auth_failed"}
mock_mgr.auto_start_enabled = True
mcp = _make_resources()
# Accessing FastMCP internals intentionally for unit test isolation.
# This may break on FastMCP upgrades — consider a make_resource_fn() helper if it does.
resource = mcp.providers[0]._components[f"resource:unraid://live/{action}@"]
resource = _get_resource(mcp, f"unraid://live/{action}")
result = await resource.fn()
parsed = json.loads(result)
assert parsed["status"] == "error"
@@ -103,10 +107,7 @@ class TestLogsStreamResource:
with patch("unraid_mcp.subscriptions.resources.subscription_manager") as mock_mgr:
mock_mgr.get_resource_data = AsyncMock(return_value=None)
mcp = _make_resources()
local_provider = mcp.providers[0]
# Accessing FastMCP internals intentionally for unit test isolation.
# This may break on FastMCP upgrades — consider a make_resource_fn() helper if it does.
resource = local_provider._components["resource:unraid://logs/stream@"]
resource = _get_resource(mcp, "unraid://logs/stream")
result = await resource.fn()
parsed = json.loads(result)
assert "status" in parsed
@@ -117,10 +118,7 @@ class TestLogsStreamResource:
with patch("unraid_mcp.subscriptions.resources.subscription_manager") as mock_mgr:
mock_mgr.get_resource_data = AsyncMock(return_value={})
mcp = _make_resources()
local_provider = mcp.providers[0]
# Accessing FastMCP internals intentionally for unit test isolation.
# This may break on FastMCP upgrades — consider a make_resource_fn() helper if it does.
resource = local_provider._components["resource:unraid://logs/stream@"]
resource = _get_resource(mcp, "unraid://logs/stream")
result = await resource.fn()
assert json.loads(result) == {}
@@ -143,9 +141,7 @@ class TestAutoStartDisabledFallback:
mock_mgr.last_error = {}
mock_mgr.auto_start_enabled = False
mcp = _make_resources()
# Accessing FastMCP internals intentionally for unit test isolation.
# This may break on FastMCP upgrades — consider a make_resource_fn() helper if it does.
resource = mcp.providers[0]._components[f"resource:unraid://live/{action}@"]
resource = _get_resource(mcp, f"unraid://live/{action}")
result = await resource.fn()
assert json.loads(result) == fallback_data
@@ -164,8 +160,6 @@ class TestAutoStartDisabledFallback:
mock_mgr.last_error = {}
mock_mgr.auto_start_enabled = False
mcp = _make_resources()
# Accessing FastMCP internals intentionally for unit test isolation.
# This may break on FastMCP upgrades — consider a make_resource_fn() helper if it does.
resource = mcp.providers[0]._components[f"resource:unraid://live/{action}@"]
resource = _get_resource(mcp, f"unraid://live/{action}")
result = await resource.fn()
assert json.loads(result)["status"] == "connecting"