mirror of
https://github.com/jmagar/unraid-mcp.git
synced 2026-03-23 04:29:17 -07:00
fix: address remaining PR review threads (docs, test-destructive, rclone test)
Resolves review threads: - PRRT_kwDOO6Hdxs50T0Wp (test-destructive.sh: add key cleanup on failure path) - PRRT_kwDOO6Hdxs50T0Ws (test-destructive.sh: pick last notification match by title) - PRRT_kwDOO6Hdxs50T0Wt (README.md: correct test-actions.sh coverage description) - PRRT_kwDOO6Hdxs50T0Wv (CLAUDE.md: add info.update_ssh to destructive actions list) - PRRT_kwDOO6Hdxs50T0Wy (http_layer test: inp["config"] -> inp["parameters"]) - PRRT_kwDOO6Hdxs50T0Wz (DESTRUCTIVE_ACTIONS.md: key ID extraction key.id not top-level) - PRRT_kwDOO6Hdxs50T0W2 (DESTRUCTIVE_ACTIONS.md: delete_archived — add archive step) - PRRT_kwDOO6Hdxs50T0W3 (DESTRUCTIVE_ACTIONS.md: rclone params provider_type/config_data/name) - PRRT_kwDOO6Hdxs50T0W4 (DESTRUCTIVE_ACTIONS.md: notification delete list+match pattern) - PRRT_kwDOO6Hdxs50T0W5 (DESTRUCTIVE_ACTIONS.md: create_folder uses folder_name param) - PRRT_kwDOO6Hdxs50T0W7 (README.md: cleanup note — test-tools.sh may write tmp log file) Changes: - test-destructive.sh keys test: attempt key delete cleanup when delete step fails - test-destructive.sh notifications test: reverse list to pick most-recent title match - tests/mcporter/README.md: accurate coverage claim; accurate cleanup section - CLAUDE.md: info.update_ssh added to destructive actions list - tests/http_layer/test_request_construction.py: assert parameters not config field - docs/DESTRUCTIVE_ACTIONS.md: all 5 example code blocks corrected with right parameter names, correct ID extraction paths, and proper sequencing Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -104,6 +104,7 @@ docker compose down
|
||||
- **rclone**: delete_remote
|
||||
- **keys**: delete
|
||||
- **storage**: flash_backup
|
||||
- **info**: update_ssh
|
||||
- **settings**: configure_ups, setup_remote_access, enable_dynamic_remote_access
|
||||
|
||||
### Environment Variable Hierarchy
|
||||
|
||||
@@ -43,9 +43,15 @@ No safe live isolation — this hits every running container. Test via `tests/sa
|
||||
|
||||
```bash
|
||||
# 1. Create a throwaway organizer folder
|
||||
# Parameter: folder_name (str); ID is in organizer.views.flatEntries[type==FOLDER]
|
||||
FOLDER=$(mcporter call --http-url "$MCP_URL" --tool unraid_docker \
|
||||
--args '{"action":"create_folder","name":"mcp-test-delete-me"}' --output json)
|
||||
FID=$(echo "$FOLDER" | python3 -c "import json,sys; print(json.load(sys.stdin).get('id',''))")
|
||||
--args '{"action":"create_folder","folder_name":"mcp-test-delete-me"}' --output json)
|
||||
FID=$(echo "$FOLDER" | python3 -c "
|
||||
import json,sys
|
||||
data=json.load(sys.stdin)
|
||||
entries=(data.get('organizer',{}).get('views',{}).get('flatEntries') or [])
|
||||
match=next((e['id'] for e in entries if e.get('type')=='FOLDER' and 'mcp-test' in e.get('name','')),'' )
|
||||
print(match)")
|
||||
|
||||
# 2. Delete it
|
||||
mcporter call --http-url "$MCP_URL" --tool unraid_docker \
|
||||
@@ -107,14 +113,21 @@ mcporter call --http-url "$MCP_URL" --tool unraid_vm \
|
||||
### `delete` — Permanently delete a notification
|
||||
|
||||
```bash
|
||||
# 1. Create a test notification
|
||||
NID=$(mcporter call --http-url "$MCP_URL" --tool unraid_notifications \
|
||||
--args '{"action":"create","title":"mcp-test-delete","subject":"safe to delete","description":"MCP destructive action test","importance":"normal"}' --output json \
|
||||
| python3 -c "import json,sys; print(json.load(sys.stdin).get('id',''))")
|
||||
|
||||
# 2. Delete it
|
||||
# 1. Create a test notification, then list to get the real stored ID (create response
|
||||
# ID is ULID-based; stored filename uses a unix timestamp, so IDs differ)
|
||||
mcporter call --http-url "$MCP_URL" --tool unraid_notifications \
|
||||
--args "{\"action\":\"delete\",\"notification_id\":\"$NID\",\"confirm\":true}" --output json
|
||||
--args '{"action":"create","title":"mcp-test-delete","subject":"safe to delete","description":"MCP destructive action test","importance":"INFO"}' --output json
|
||||
NID=$(mcporter call --http-url "$MCP_URL" --tool unraid_notifications \
|
||||
--args '{"action":"list","notification_type":"UNREAD"}' --output json \
|
||||
| python3 -c "
|
||||
import json,sys
|
||||
notifs=json.load(sys.stdin).get('notifications',[])
|
||||
matches=[n['id'] for n in reversed(notifs) if n.get('title')=='mcp-test-delete']
|
||||
print(matches[0] if matches else '')")
|
||||
|
||||
# 2. Delete it (notification_type required)
|
||||
mcporter call --http-url "$MCP_URL" --tool unraid_notifications \
|
||||
--args "{\"action\":\"delete\",\"notification_id\":\"$NID\",\"notification_type\":\"UNREAD\",\"confirm\":true}" --output json
|
||||
|
||||
# 3. Verify
|
||||
mcporter call --http-url "$MCP_URL" --tool unraid_notifications \
|
||||
@@ -127,10 +140,18 @@ mcporter call --http-url "$MCP_URL" --tool unraid_notifications \
|
||||
### `delete_archived` — Wipe all archived notifications (bulk, irreversible)
|
||||
|
||||
```bash
|
||||
# 1. Create and archive a test notification first
|
||||
# 1. Create and archive a test notification
|
||||
mcporter call --http-url "$MCP_URL" --tool unraid_notifications \
|
||||
--args '{"action":"create","title":"mcp-test-archive-wipe","subject":"archive me","description":"safe to delete","importance":"normal"}' --output json
|
||||
# (then archive it via action=archive if needed)
|
||||
--args '{"action":"create","title":"mcp-test-archive-wipe","subject":"archive me","description":"safe to delete","importance":"INFO"}' --output json
|
||||
AID=$(mcporter call --http-url "$MCP_URL" --tool unraid_notifications \
|
||||
--args '{"action":"list","notification_type":"UNREAD"}' --output json \
|
||||
| python3 -c "
|
||||
import json,sys
|
||||
notifs=json.load(sys.stdin).get('notifications',[])
|
||||
matches=[n['id'] for n in reversed(notifs) if n.get('title')=='mcp-test-archive-wipe']
|
||||
print(matches[0] if matches else '')")
|
||||
mcporter call --http-url "$MCP_URL" --tool unraid_notifications \
|
||||
--args "{\"action\":\"archive\",\"notification_id\":\"$AID\"}" --output json
|
||||
|
||||
# 2. Wipe all archived
|
||||
# NOTE: this deletes ALL archived notifications, not just the test one
|
||||
@@ -148,12 +169,13 @@ mcporter call --http-url "$MCP_URL" --tool unraid_notifications \
|
||||
|
||||
```bash
|
||||
# 1. Create a throwaway local remote (points to /tmp — no real data)
|
||||
# Parameters: name (str), provider_type (str), config_data (dict)
|
||||
mcporter call --http-url "$MCP_URL" --tool unraid_rclone \
|
||||
--args '{"action":"create_remote","name":"mcp-test-remote","remote_type":"local","config":{"root":"/tmp"}}' --output json
|
||||
--args '{"action":"create_remote","name":"mcp-test-remote","provider_type":"local","config_data":{"root":"/tmp"}}' --output json
|
||||
|
||||
# 2. Delete it
|
||||
mcporter call --http-url "$MCP_URL" --tool unraid_rclone \
|
||||
--args '{"action":"delete_remote","remote_name":"mcp-test-remote","confirm":true}' --output json
|
||||
--args '{"action":"delete_remote","name":"mcp-test-remote","confirm":true}' --output json
|
||||
|
||||
# 3. Verify
|
||||
mcporter call --http-url "$MCP_URL" --tool unraid_rclone \
|
||||
@@ -170,10 +192,10 @@ mcporter call --http-url "$MCP_URL" --tool unraid_rclone \
|
||||
### `delete` — Delete an API key (immediately revokes access)
|
||||
|
||||
```bash
|
||||
# 1. Create a test key
|
||||
# 1. Create a test key (names cannot contain hyphens; ID is at key.id)
|
||||
KID=$(mcporter call --http-url "$MCP_URL" --tool unraid_keys \
|
||||
--args '{"action":"create","name":"mcp-test-key","description":"Safe to delete — MCP destructive test"}' --output json \
|
||||
| python3 -c "import json,sys; print(json.load(sys.stdin).get('id',''))")
|
||||
--args '{"action":"create","name":"mcp test key","roles":["VIEWER"]}' --output json \
|
||||
| python3 -c "import json,sys; print(json.load(sys.stdin).get('key',{}).get('id',''))")
|
||||
|
||||
# 2. Delete it
|
||||
mcporter call --http-url "$MCP_URL" --tool unraid_keys \
|
||||
|
||||
@@ -1044,7 +1044,7 @@ class TestRCloneToolRequests:
|
||||
inp = body["variables"]["input"]
|
||||
assert inp["name"] == "my-s3"
|
||||
assert inp["type"] == "s3"
|
||||
assert inp["config"] == {"bucket": "my-bucket"}
|
||||
assert inp["parameters"] == {"bucket": "my-bucket"}
|
||||
|
||||
@respx.mock
|
||||
async def test_delete_remote_requires_confirm(self) -> None:
|
||||
|
||||
@@ -33,7 +33,7 @@ Launches `uv run unraid-mcp-server` in stdio mode for each tool call. Requires `
|
||||
UNRAID_MCP_URL=http://10.1.0.2:6970/mcp ./tests/mcporter/test-actions.sh
|
||||
```
|
||||
|
||||
Connects to an already-running streamable-http server. More up-to-date coverage — includes `unraid_settings`, all docker organizer mutations, and the full notification action set.
|
||||
Connects to an already-running streamable-http server. Covers all read-only actions across 10 tools (`unraid_settings` is all-mutations and skipped; all destructive mutations are explicitly skipped).
|
||||
|
||||
---
|
||||
|
||||
@@ -148,4 +148,4 @@ uv run unraid-mcp-server
|
||||
|
||||
## Cleanup
|
||||
|
||||
Both scripts create **no temporary files and no background processes**. `test-actions.sh` connects to an existing server and leaves it running. `test-tools.sh` spawns stdio server subprocesses per call; they exit when mcporter finishes each invocation.
|
||||
`test-actions.sh` connects to an existing server and leaves it running; it creates no temporary files. `test-tools.sh` spawns stdio server subprocesses per call — they exit when mcporter finishes each invocation — and may write a timestamped log file under `${TMPDIR:-/tmp}`. Neither script leaves background processes.
|
||||
|
||||
@@ -158,15 +158,17 @@ test_notifications_delete() {
|
||||
return
|
||||
fi
|
||||
|
||||
# The create response ID doesn't match the stored filename — list and find by title
|
||||
# The create response ID doesn't match the stored filename — list and find by title.
|
||||
# Use the LAST match so a stale notification with the same title is bypassed.
|
||||
local list_raw nid
|
||||
list_raw="$(mcall unraid_notifications '{"action":"list","notification_type":"UNREAD"}')"
|
||||
nid="$(python3 -c "
|
||||
import json,sys
|
||||
d = json.loads('''${list_raw}''')
|
||||
notifs = d.get('notifications', [])
|
||||
match = next((n['id'] for n in notifs if n.get('title') == 'mcp-test-delete'), '')
|
||||
print(match)
|
||||
# Reverse so the most-recent match wins over any stale leftover
|
||||
matches = [n['id'] for n in reversed(notifs) if n.get('title') == 'mcp-test-delete']
|
||||
print(matches[0] if matches else '')
|
||||
" 2>/dev/null)"
|
||||
|
||||
if [[ -z "${nid}" ]]; then
|
||||
@@ -255,7 +257,9 @@ sys.exit(1 if any(k.get('name') == 'mcp test key' for k in keys) else 0)
|
||||
success="$(python3 -c "import json,sys; d=json.loads('''${del_raw}'''); print(d.get('success', False))" 2>/dev/null)"
|
||||
|
||||
if [[ "${success}" != "True" ]]; then
|
||||
fail_test "${label}" "delete did not return success=true: ${del_raw}"
|
||||
# Cleanup: attempt to delete the leaked key so future runs are not blocked
|
||||
mcall unraid_keys "{\"action\":\"delete\",\"key_id\":\"${kid}\",\"confirm\":true}" &>/dev/null || true
|
||||
fail_test "${label}" "delete did not return success=true: ${del_raw} (key delete re-attempted as fallback cleanup)"
|
||||
return
|
||||
fi
|
||||
|
||||
|
||||
Reference in New Issue
Block a user