Fix delete_image: POST with images JSON array (DevTools-confirmed format)
DSM Container Manager rejects name+tag and sha256 id params (error 114).
Browser DevTools capture shows the correct call is:
POST SYNO.Docker.Image / delete / version=1
images=[{"repository":"nouchka/sqlite3","tags":["latest"]}]
Changes:
- Add DsmClient.post_request() for form-encoded POST requests
- delete_image now calls post_request with version=1 and the images
JSON array built from the resolved repository name and tag
- Remove unused docker error-code dict from _error_message()
- Tests mock post_request and assert the images param content
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -61,12 +61,6 @@ def _error_message(code: int, api: str = "") -> str:
|
|||||||
407: "Too many failed login attempts — account temporarily locked",
|
407: "Too many failed login attempts — account temporarily locked",
|
||||||
408: "IP blocked due to excessive failed attempts",
|
408: "IP blocked due to excessive failed attempts",
|
||||||
}
|
}
|
||||||
# Docker API codes
|
|
||||||
docker = {
|
|
||||||
1: "Project not found",
|
|
||||||
2: "Container not found",
|
|
||||||
}
|
|
||||||
|
|
||||||
if "Auth" in api and code in auth:
|
if "Auth" in api and code in auth:
|
||||||
return auth[code]
|
return auth[code]
|
||||||
if code in common:
|
if code in common:
|
||||||
@@ -269,7 +263,9 @@ class DsmClient:
|
|||||||
# Log with sensitive fields masked
|
# Log with sensitive fields masked
|
||||||
log_params = {k: ("***" if k in _SENSITIVE_PARAMS else v) for k, v in req_params.items()}
|
log_params = {k: ("***" if k in _SENSITIVE_PARAMS else v) for k, v in req_params.items()}
|
||||||
retry_tag = " (retry)" if _is_retry else ""
|
retry_tag = " (retry)" if _is_retry else ""
|
||||||
logger.debug("DSM GET%s: %s/%s v%d — %s", retry_tag, api, method, resolved_version, log_params)
|
logger.debug(
|
||||||
|
"DSM GET%s: %s/%s v%d — %s", retry_tag, api, method, resolved_version, log_params
|
||||||
|
)
|
||||||
|
|
||||||
resp = await http.get(url, params=req_params)
|
resp = await http.get(url, params=req_params)
|
||||||
resp.raise_for_status()
|
resp.raise_for_status()
|
||||||
@@ -296,6 +292,72 @@ class DsmClient:
|
|||||||
|
|
||||||
raise SynologyError(_error_message(code, api), code=code)
|
raise SynologyError(_error_message(code, api), code=code)
|
||||||
|
|
||||||
|
async def post_request(
|
||||||
|
self,
|
||||||
|
api: str,
|
||||||
|
method: str,
|
||||||
|
version: int | None = None,
|
||||||
|
params: dict[str, Any] | None = None,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
"""Make a POST (form-encoded) request to the DSM API.
|
||||||
|
|
||||||
|
Identical semantics to request(), but sends params as a form body
|
||||||
|
instead of query-string — required by some Container Manager endpoints
|
||||||
|
(e.g. SYNO.Docker.Image/delete).
|
||||||
|
|
||||||
|
Args:
|
||||||
|
api: DSM API name (e.g. "SYNO.Docker.Image").
|
||||||
|
method: API method (e.g. "delete").
|
||||||
|
version: API version. Defaults to maxVersion from API info.
|
||||||
|
params: Additional form fields.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
Response data dict from the "data" field of the envelope.
|
||||||
|
|
||||||
|
Raises:
|
||||||
|
SynologyError: On API errors.
|
||||||
|
"""
|
||||||
|
sys.stderr.write(f"[dsm] post_request: {api}/{method}\n")
|
||||||
|
sys.stderr.flush()
|
||||||
|
await self._ensure_initialized()
|
||||||
|
http = self._get_http()
|
||||||
|
|
||||||
|
if api not in self._api_cache:
|
||||||
|
raise SynologyError(
|
||||||
|
f"API '{api}' not found. Call query_api_info() first.",
|
||||||
|
code=102,
|
||||||
|
)
|
||||||
|
|
||||||
|
info = self._api_cache[api]
|
||||||
|
resolved_version = version if version is not None else info["maxVersion"]
|
||||||
|
url = f"{self._base_url}/webapi/{info['path']}"
|
||||||
|
|
||||||
|
form: dict[str, Any] = {
|
||||||
|
"api": api,
|
||||||
|
"version": str(resolved_version),
|
||||||
|
"method": method,
|
||||||
|
}
|
||||||
|
if params:
|
||||||
|
form.update(params)
|
||||||
|
|
||||||
|
query_params: dict[str, str] = {}
|
||||||
|
if self._sid:
|
||||||
|
query_params["_sid"] = self._sid
|
||||||
|
|
||||||
|
log_form = {k: ("***" if k in _SENSITIVE_PARAMS else v) for k, v in form.items()}
|
||||||
|
logger.debug("DSM POST: %s/%s v%d — %s", api, method, resolved_version, log_form)
|
||||||
|
|
||||||
|
resp = await http.post(url, params=query_params, data=form)
|
||||||
|
resp.raise_for_status()
|
||||||
|
body = resp.json()
|
||||||
|
|
||||||
|
if body.get("success"):
|
||||||
|
return body.get("data") or {}
|
||||||
|
|
||||||
|
code = body.get("error", {}).get("code", 0)
|
||||||
|
logger.debug("DSM POST response: %s/%s — error code %d", api, method, code)
|
||||||
|
raise SynologyError(_error_message(code, api), code=code)
|
||||||
|
|
||||||
async def upload_text(
|
async def upload_text(
|
||||||
self,
|
self,
|
||||||
dest_folder: str,
|
dest_folder: str,
|
||||||
@@ -340,7 +402,13 @@ class DsmClient:
|
|||||||
if self._sid:
|
if self._sid:
|
||||||
query_params["_sid"] = self._sid
|
query_params["_sid"] = self._sid
|
||||||
|
|
||||||
logger.debug("DSM POST: %s/upload v%d — path=%s filename=%s", api, resolved_version, dest_folder, filename)
|
logger.debug(
|
||||||
|
"DSM POST: %s/upload v%d — path=%s filename=%s",
|
||||||
|
api,
|
||||||
|
resolved_version,
|
||||||
|
dest_folder,
|
||||||
|
filename,
|
||||||
|
)
|
||||||
|
|
||||||
encoded = content.encode("utf-8")
|
encoded = content.encode("utf-8")
|
||||||
resp = await http.post(
|
resp = await http.post(
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import json
|
||||||
import logging
|
import logging
|
||||||
import sys
|
import sys
|
||||||
from datetime import UTC, datetime
|
from datetime import UTC, datetime
|
||||||
@@ -243,17 +244,22 @@ def register_images(mcp: FastMCP, config: AppConfig, client: DsmClient) -> None:
|
|||||||
f"Call delete_image(image_id={image_id!r}, confirmed=True) to confirm."
|
f"Call delete_image(image_id={image_id!r}, confirmed=True) to confirm."
|
||||||
)
|
)
|
||||||
|
|
||||||
# DSM requires the sha256 image ID for deletion, not name+tag.
|
# DSM Container Manager expects a POST with version=1 and an
|
||||||
if not img_hash:
|
# "images" JSON array — confirmed via browser DevTools capture.
|
||||||
return f"Cannot delete '{display_name}': image ID (sha256) not found in list."
|
# Format: images=[{"repository": "nginx", "tags": ["1.24"]}]
|
||||||
|
delete_repo = repo
|
||||||
sys.stderr.write(f"[delete_image] api=SYNO.Docker.Image method=delete id={img_hash!r}\n")
|
delete_tag = img_tags[0] if img_tags else tag
|
||||||
|
images_param = json.dumps([{"repository": delete_repo, "tags": [delete_tag]}])
|
||||||
|
sys.stderr.write(
|
||||||
|
f"[delete_image] POST SYNO.Docker.Image/delete v1 images={images_param!r}\n"
|
||||||
|
)
|
||||||
sys.stderr.flush()
|
sys.stderr.flush()
|
||||||
try:
|
try:
|
||||||
await client.request(
|
await client.post_request(
|
||||||
"SYNO.Docker.Image",
|
"SYNO.Docker.Image",
|
||||||
"delete",
|
"delete",
|
||||||
params={"id": img_hash},
|
version=1,
|
||||||
|
params={"images": images_param},
|
||||||
)
|
)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
code = getattr(e, "code", "?")
|
code = getattr(e, "code", "?")
|
||||||
|
|||||||
@@ -203,17 +203,18 @@ async def test_delete_image_preview():
|
|||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_delete_image_confirmed():
|
async def test_delete_image_confirmed():
|
||||||
|
import json
|
||||||
|
|
||||||
from mcp_synology_container.modules.images import register_images
|
from mcp_synology_container.modules.images import register_images
|
||||||
|
|
||||||
client = AsyncMock()
|
client = AsyncMock()
|
||||||
|
client.post_request = AsyncMock(return_value={})
|
||||||
|
|
||||||
async def mock_request(api, method, **kwargs):
|
async def mock_request(api, method, **kwargs):
|
||||||
if api == "SYNO.Docker.Image" and method == "list":
|
if api == "SYNO.Docker.Image" and method == "list":
|
||||||
return SAMPLE_IMAGES
|
return SAMPLE_IMAGES
|
||||||
if api == "SYNO.Docker.Container":
|
if api == "SYNO.Docker.Container":
|
||||||
return {"containers": []}
|
return {"containers": []}
|
||||||
if api == "SYNO.Docker.Image" and method == "delete":
|
|
||||||
return {}
|
|
||||||
return {}
|
return {}
|
||||||
|
|
||||||
client.request.side_effect = mock_request
|
client.request.side_effect = mock_request
|
||||||
@@ -225,12 +226,11 @@ async def test_delete_image_confirmed():
|
|||||||
assert "redis:7" in result
|
assert "redis:7" in result
|
||||||
assert "freed" in result
|
assert "freed" in result
|
||||||
|
|
||||||
# Delete must use the sha256 id, not name+tag
|
# post_request must be called with images JSON param, not name/tag/id
|
||||||
delete_call = next(c for c in client.request.call_args_list if c.args[1] == "delete")
|
client.post_request.assert_called_once()
|
||||||
params = delete_call.kwargs.get("params") or {}
|
params = client.post_request.call_args.kwargs.get("params", {})
|
||||||
assert params.get("id") == "sha256:cccc"
|
images = json.loads(params["images"])
|
||||||
assert "name" not in params
|
assert images == [{"repository": "redis", "tags": ["7"]}]
|
||||||
assert "tag" not in params
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
@@ -238,6 +238,7 @@ async def test_delete_image_not_found():
|
|||||||
from mcp_synology_container.modules.images import register_images
|
from mcp_synology_container.modules.images import register_images
|
||||||
|
|
||||||
client = AsyncMock()
|
client = AsyncMock()
|
||||||
|
client.post_request = AsyncMock(return_value={})
|
||||||
|
|
||||||
async def mock_request(api, method, **kwargs):
|
async def mock_request(api, method, **kwargs):
|
||||||
if api == "SYNO.Docker.Image" and method == "list":
|
if api == "SYNO.Docker.Image" and method == "list":
|
||||||
@@ -250,6 +251,7 @@ async def test_delete_image_not_found():
|
|||||||
|
|
||||||
result = await tools["delete_image"](image_id="nonexistent:latest", confirmed=True)
|
result = await tools["delete_image"](image_id="nonexistent:latest", confirmed=True)
|
||||||
assert "not found" in result
|
assert "not found" in result
|
||||||
|
client.post_request.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
@@ -257,6 +259,7 @@ async def test_delete_image_in_use_blocked():
|
|||||||
from mcp_synology_container.modules.images import register_images
|
from mcp_synology_container.modules.images import register_images
|
||||||
|
|
||||||
client = AsyncMock()
|
client = AsyncMock()
|
||||||
|
client.post_request = AsyncMock(return_value={})
|
||||||
|
|
||||||
async def mock_request(api, method, **kwargs):
|
async def mock_request(api, method, **kwargs):
|
||||||
if api == "SYNO.Docker.Image" and method == "list":
|
if api == "SYNO.Docker.Image" and method == "list":
|
||||||
@@ -272,21 +275,23 @@ async def test_delete_image_in_use_blocked():
|
|||||||
result = await tools["delete_image"](image_id="nginx:1.24", confirmed=True)
|
result = await tools["delete_image"](image_id="nginx:1.24", confirmed=True)
|
||||||
assert "Cannot delete" in result
|
assert "Cannot delete" in result
|
||||||
assert "my-nginx" in result
|
assert "my-nginx" in result
|
||||||
|
client.post_request.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_delete_image_by_hash():
|
async def test_delete_image_by_hash():
|
||||||
|
import json
|
||||||
|
|
||||||
from mcp_synology_container.modules.images import register_images
|
from mcp_synology_container.modules.images import register_images
|
||||||
|
|
||||||
client = AsyncMock()
|
client = AsyncMock()
|
||||||
|
client.post_request = AsyncMock(return_value={})
|
||||||
|
|
||||||
async def mock_request(api, method, **kwargs):
|
async def mock_request(api, method, **kwargs):
|
||||||
if api == "SYNO.Docker.Image" and method == "list":
|
if api == "SYNO.Docker.Image" and method == "list":
|
||||||
return SAMPLE_IMAGES
|
return SAMPLE_IMAGES
|
||||||
if api == "SYNO.Docker.Container":
|
if api == "SYNO.Docker.Container":
|
||||||
return {"containers": []}
|
return {"containers": []}
|
||||||
if api == "SYNO.Docker.Image" and method == "delete":
|
|
||||||
return {}
|
|
||||||
return {}
|
return {}
|
||||||
|
|
||||||
client.request.side_effect = mock_request
|
client.request.side_effect = mock_request
|
||||||
@@ -297,10 +302,18 @@ async def test_delete_image_by_hash():
|
|||||||
assert "Deleted" in result
|
assert "Deleted" in result
|
||||||
assert "redis" in result
|
assert "redis" in result
|
||||||
|
|
||||||
|
# Verify images param uses the resolved name+tag (not the hash)
|
||||||
|
call_kwargs = client.post_request.call_args
|
||||||
|
params = call_kwargs.kwargs.get("params") or {}
|
||||||
|
images = json.loads(params.get("images", "[]"))
|
||||||
|
assert images == [{"repository": "redis", "tags": ["7"]}]
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_delete_image_registry_prefixed_name():
|
async def test_delete_image_registry_prefixed_name():
|
||||||
"""Registry-prefixed image names (e.g. ghcr.io/foo/bar:v1) must split at last ':'."""
|
"""Registry-prefixed image names (e.g. ghcr.io/foo/bar:v1) must split at last ':'."""
|
||||||
|
import json
|
||||||
|
|
||||||
from mcp_synology_container.modules.images import register_images
|
from mcp_synology_container.modules.images import register_images
|
||||||
|
|
||||||
registry_images = {
|
registry_images = {
|
||||||
@@ -317,14 +330,13 @@ async def test_delete_image_registry_prefixed_name():
|
|||||||
}
|
}
|
||||||
|
|
||||||
client = AsyncMock()
|
client = AsyncMock()
|
||||||
|
client.post_request = AsyncMock(return_value={})
|
||||||
|
|
||||||
async def mock_request(api, method, **kwargs):
|
async def mock_request(api, method, **kwargs):
|
||||||
if api == "SYNO.Docker.Image" and method == "list":
|
if api == "SYNO.Docker.Image" and method == "list":
|
||||||
return registry_images
|
return registry_images
|
||||||
if api == "SYNO.Docker.Container":
|
if api == "SYNO.Docker.Container":
|
||||||
return {"containers": []}
|
return {"containers": []}
|
||||||
if api == "SYNO.Docker.Image" and method == "delete":
|
|
||||||
return {}
|
|
||||||
return {}
|
return {}
|
||||||
|
|
||||||
client.request.side_effect = mock_request
|
client.request.side_effect = mock_request
|
||||||
@@ -337,12 +349,11 @@ async def test_delete_image_registry_prefixed_name():
|
|||||||
assert "Deleted" in result
|
assert "Deleted" in result
|
||||||
assert "open-webui" in result
|
assert "open-webui" in result
|
||||||
|
|
||||||
# Verify delete was called with the sha256 ID, not name+tag
|
# images param must use full registry-prefixed repository name
|
||||||
delete_call = next(c for c in client.request.call_args_list if c.args[1] == "delete")
|
call_kwargs = client.post_request.call_args
|
||||||
params = delete_call.kwargs.get("params") or {}
|
params = call_kwargs.kwargs.get("params") or {}
|
||||||
assert params.get("id") == "sha256:dddd"
|
images = json.loads(params.get("images", "[]"))
|
||||||
assert "name" not in params
|
assert images == [{"repository": "ghcr.io/open-webui/open-webui", "tags": ["v0.8.10"]}]
|
||||||
assert "tag" not in params
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
@@ -351,14 +362,13 @@ async def test_delete_image_api_error():
|
|||||||
from mcp_synology_container.modules.images import register_images
|
from mcp_synology_container.modules.images import register_images
|
||||||
|
|
||||||
client = AsyncMock()
|
client = AsyncMock()
|
||||||
|
client.post_request = AsyncMock(side_effect=SynologyError("delete failed", code=114))
|
||||||
|
|
||||||
async def mock_request(api, method, **kwargs):
|
async def mock_request(api, method, **kwargs):
|
||||||
if api == "SYNO.Docker.Image" and method == "list":
|
if api == "SYNO.Docker.Image" and method == "list":
|
||||||
return SAMPLE_IMAGES
|
return SAMPLE_IMAGES
|
||||||
if api == "SYNO.Docker.Container":
|
if api == "SYNO.Docker.Container":
|
||||||
return {"containers": []}
|
return {"containers": []}
|
||||||
if api == "SYNO.Docker.Image" and method == "delete":
|
|
||||||
raise SynologyError("delete failed", code=1)
|
|
||||||
return {}
|
return {}
|
||||||
|
|
||||||
client.request.side_effect = mock_request
|
client.request.side_effect = mock_request
|
||||||
@@ -367,6 +377,7 @@ async def test_delete_image_api_error():
|
|||||||
|
|
||||||
result = await tools["delete_image"](image_id="redis:7", confirmed=True)
|
result = await tools["delete_image"](image_id="redis:7", confirmed=True)
|
||||||
assert "Error" in result
|
assert "Error" in result
|
||||||
|
assert "114" in result
|
||||||
|
|
||||||
|
|
||||||
# ──────────────────────────────────────────────────────────────────────────────
|
# ──────────────────────────────────────────────────────────────────────────────
|
||||||
|
|||||||
Reference in New Issue
Block a user