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

README.md

@@ -252,7 +252,7 @@ The server exposes two classes of MCP resources backed by persistent WebSocket c
 
 > **Note**: Resources return cached data from persistent WebSocket subscriptions. A `{"status": "connecting"}` placeholder is returned while the subscription initializes — retry in a moment.
 >
-> **`log_tail` and `notification_feed`** are accessible as tool subactions (`unraid(action="live", subaction="log_tail")`) but are not registered as MCP resources — they use transient one-shot subscriptions and require parameters.
+> **`log_tail`** is accessible as a tool subaction (`unraid(action="live", subaction="log_tail", path="/var/log/syslog")`) and requires a `path`; **`notification_feed`** is also available as a tool subaction but uses a transient one-shot subscription and accepts optional parameters. Neither is registered as an MCP resource.
 
 > **Security note**: The `disk/logs` and `live/log_tail` subactions allow reading files under `/var/log/` and `/boot/logs/` on the Unraid server. Authenticated MCP clients can stream any log file within these directories.
 

skills/unraid/SKILL.md

@@ -93,7 +93,7 @@ unraid(action="live",    subaction="cpu")
 | `disks` | All physical disks with health and temperatures |
 | `disk_details` | Detailed info for a specific disk (requires `disk_id`) |
 | `log_files` | List available log files |
-| `logs` | Read log content (requires `path`; optional `lines`) |
+| `logs` | Read log content (requires `log_path`; optional `tail_lines`) |
 | `flash_backup` | ⚠️ Trigger a flash backup (requires `confirm=True`) |
 
 ### `docker` — Containers
@@ -143,11 +143,11 @@ unraid(action="live",    subaction="cpu")
 |-----------|-------------|
 | `list` | All API keys |
 | `get` | Single key details (requires `key_id`) |
-| `create` | Create a new key (requires `name`, `roles`) |
+| `create` | Create a new key (requires `name`; optional `roles`, `permissions`) |
 | `update` | Update a key (requires `key_id`) |
 | `delete` | ⚠️ Delete a key (requires `key_id`, `confirm=True`) |
-| `add_role` | Add a role to a key (requires `key_id`, `role`) |
+| `add_role` | Add a role to a key (requires `key_id`, `roles`) |
-| `remove_role` | Remove a role from a key (requires `key_id`, `role`) |
+| `remove_role` | Remove a role from a key (requires `key_id`, `roles`) |
 
 ### `plugin` — Plugins
 | Subaction | Description |
@@ -161,13 +161,13 @@ unraid(action="live",    subaction="cpu")
 |-----------|-------------|
 | `list_remotes` | List configured rclone remotes |
 | `config_form` | Get configuration form for a remote type |
-| `create_remote` | Create a new remote (requires `name`, `type`, `fields`) |
+| `create_remote` | Create a new remote (requires `name`, `provider_type`, `config_data`) |
 | `delete_remote` | ⚠️ Delete a remote (requires `name`, `confirm=True`) |
 
 ### `setting` — System Settings
 | Subaction | Description |
 |-----------|-------------|
-| `update` | Update system settings (requires `settings` object) |
+| `update` | Update system settings (requires `settings_input` object) |
 | `configure_ups` | ⚠️ Configure UPS settings (requires `confirm=True`) |
 
 ### `customization` — Theme & Appearance
@@ -186,7 +186,7 @@ unraid(action="live",    subaction="cpu")
 | `provider` | Single provider details (requires `provider_id`) |
 | `configuration` | OIDC configuration |
 | `public_providers` | Public-facing provider list |
-| `validate_session` | Validate current SSO session |
+| `validate_session` | Validate current SSO session (requires `token`) |
 
 ### `user` — Current User
 | Subaction | Description |
@@ -264,7 +264,7 @@ unraid(action="system", subaction="array")
 ### Read logs
 ```
 unraid(action="disk", subaction="log_files")
-unraid(action="disk", subaction="logs", path="syslog", lines=50)
+unraid(action="disk", subaction="logs", log_path="/var/log/syslog", tail_lines=50)
 ```
 
 ### Live monitoring

skills/unraid/references/endpoints.md

@@ -1,7 +1,7 @@
-> **⚠️ DEVELOPER REFERENCE ONLY** — This file documents raw GraphQL endpoints for development purposes. For MCP tool usage, use `unraid(action=..., subaction=...)` calls as documented in `SKILL.md`.
-
 # Unraid API Endpoints Reference
 
+> **⚠️ DEVELOPER REFERENCE ONLY** — This file documents raw GraphQL endpoints for development purposes. For MCP tool usage, use `unraid(action=..., subaction=...)` calls as documented in `SKILL.md`.
+
 Complete list of available GraphQL read-only endpoints in Unraid 7.2+.
 
 ## System & Metrics (8)

skills/unraid/references/quick-reference.md

@@ -22,7 +22,7 @@ unraid(action="system",  subaction="array")             # Array status overview
 unraid(action="disk",    subaction="disks")             # All disks with temps & health
 unraid(action="array",   subaction="parity_status")     # Current parity check
 unraid(action="array",   subaction="parity_history")    # Past parity results
-unraid(action="array",   subaction="parity_start")      # Start parity check
+unraid(action="array",   subaction="parity_start", correct=False)  # Start parity check
 unraid(action="array",   subaction="stop_array",  confirm=True)   # ⚠️ Stop array
 ```
 

skills/unraid/references/troubleshooting.md

@@ -76,9 +76,9 @@ See the Destructive Actions table in `SKILL.md` for the full list.
 
 **Explanation:** The persistent WebSocket subscription has not yet received its first event. Retry in a moment.
 
-**Known issue:** `live/array_state` uses `arraySubscription` which has a known Unraid API bug (returns null for a non-nullable field). This subscription will always show "connecting."
+**Known issue:** `live/array_state` uses `arraySubscription` which has a known Unraid API bug (returns null for a non-nullable field). This subscription may show "connecting" indefinitely.
 
-**Event-driven subscriptions** (`live/notifications_overview`, `live/owner`, `live/server_status`, `live/ups_status`) only populate when the server emits a change event. If the server is idle, these may never populate during a session.
+**Event-driven subscriptions** (`live/parity_progress`, `live/notifications_overview`, `live/owner`, `live/server_status`, `live/ups_status`) only populate when the server emits a change event. If the server is idle, these may never populate during a session.
 
 **Workaround for array state:** Use `unraid(action="system", subaction="array")` for a synchronous snapshot instead.
 

tests/test_resources.py

@@ -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.
+            resource = _get_resource(mcp, f"unraid://live/{action}")
-            # 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}@"]
             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.
+            resource = _get_resource(mcp, f"unraid://live/{action}")
-            # 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}@"]
             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.
+            resource = _get_resource(mcp, f"unraid://live/{action}")
-            # 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}@"]
             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]
+            resource = _get_resource(mcp, "unraid://logs/stream")
-            # 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@"]
             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]
+            resource = _get_resource(mcp, "unraid://logs/stream")
-            # 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@"]
             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.
+            resource = _get_resource(mcp, f"unraid://live/{action}")
-            # 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}@"]
             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.
+            resource = _get_resource(mcp, f"unraid://live/{action}")
-            # 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}@"]
             result = await resource.fn()
         assert json.loads(result)["status"] == "connecting"