fix: v0.3.3 — delete_container params (error 114) + delete_project orphan guard
Bug 1 — delete_container (DSM error 114):
SYNO.Docker.Container/delete requires three parameters: name
(JSON-encoded), force=false, and preserve_profile=false. Previously
only a bare `name` string was sent, causing DSM to reject the call
with error 114. Added the two missing fields and JSON-encode name to
match the DSM convention. The connector-side running-container guard
is unchanged; force stays hard-coded to false.
Bug 2 — delete_project orphan containers:
Production test revealed that DSM does NOT reject Project/delete on a
running project — it silently removes the registration and leaves the
containers running without any project context. The previous
implementation tried to handle this via a caught SynologyError that
never actually fires. Fix: check the project status from _find_project
connector-side before issuing any DSM call; if RUNNING, return an
error pointing at stop_project. The delete request is never sent for
a running project.
The corresponding unit test (test_delete_project_running_returns_stop_hint)
was a false positive — it mocked a DSM rejection that real DSM never
produces. Replaced with test_delete_project_running_blocked_connector_side
which asserts that client.request("delete") is never called when the
project is RUNNING.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
+20
-1
@@ -2,7 +2,26 @@
|
|||||||
|
|
||||||
All notable changes to this project will be documented in this file.
|
All notable changes to this project will be documented in this file.
|
||||||
|
|
||||||
## [0.3.2] - 2026-05-18
|
## [0.3.3] - 2026-05-18
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
|
||||||
|
- `delete_container`: `SYNO.Docker.Container/delete` requires three
|
||||||
|
parameters — `name` (JSON-encoded), `force=false`, and
|
||||||
|
`preserve_profile=false`. Previously only `name` was sent (without
|
||||||
|
JSON-encoding), causing DSM to reject the call with error 114.
|
||||||
|
- `delete_project`: DSM does **not** reject `Project/delete` on a
|
||||||
|
running project — it silently removes the registration and leaves
|
||||||
|
the containers running as orphans. The connector now blocks the call
|
||||||
|
itself when the project status is `RUNNING`, before issuing any DSM
|
||||||
|
request, and tells the user to `stop_project` first. The previous
|
||||||
|
implementation relied on a DSM-level rejection that never occurs in
|
||||||
|
practice; the corresponding unit test was a false positive (mocked
|
||||||
|
an error that real DSM never returns) and has been replaced with a
|
||||||
|
test that asserts `Project/delete` is never called for a `RUNNING`
|
||||||
|
project.
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
### Added
|
### Added
|
||||||
|
|
||||||
|
|||||||
+1
-1
@@ -1,6 +1,6 @@
|
|||||||
[project]
|
[project]
|
||||||
name = "mcp-synology-container"
|
name = "mcp-synology-container"
|
||||||
version = "0.3.2"
|
version = "0.3.3"
|
||||||
description = "MCP server for Synology Container Manager"
|
description = "MCP server for Synology Container Manager"
|
||||||
requires-python = ">=3.12"
|
requires-python = ">=3.12"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import json
|
||||||
import logging
|
import logging
|
||||||
import re
|
import re
|
||||||
from typing import TYPE_CHECKING, Any
|
from typing import TYPE_CHECKING, Any
|
||||||
@@ -319,7 +320,11 @@ def register_containers(mcp: FastMCP, config: AppConfig, client: DsmClient) -> N
|
|||||||
await client.request(
|
await client.request(
|
||||||
"SYNO.Docker.Container",
|
"SYNO.Docker.Container",
|
||||||
"delete",
|
"delete",
|
||||||
params={"name": resolved_name},
|
params={
|
||||||
|
"name": json.dumps(resolved_name),
|
||||||
|
"force": "false",
|
||||||
|
"preserve_profile": "false",
|
||||||
|
},
|
||||||
)
|
)
|
||||||
return f"Deleted container '{display_name}'."
|
return f"Deleted container '{display_name}'."
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
|
|||||||
@@ -403,6 +403,15 @@ def register_projects(mcp: FastMCP, config: AppConfig, client: DsmClient) -> Non
|
|||||||
f"Call this tool again with confirmed=True to proceed."
|
f"Call this tool again with confirmed=True to proceed."
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Guard: DSM does NOT reject a delete call on a running project — it
|
||||||
|
# removes the registration silently and leaves the containers running
|
||||||
|
# as orphans. Reject here so we never put the NAS into that state.
|
||||||
|
if status == "RUNNING":
|
||||||
|
return (
|
||||||
|
f"Error: project '{project_name}' is RUNNING. "
|
||||||
|
f"Stop it first with stop_project, then delete_project."
|
||||||
|
)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
await client.request(
|
await client.request(
|
||||||
"SYNO.Docker.Project",
|
"SYNO.Docker.Project",
|
||||||
@@ -411,14 +420,6 @@ def register_projects(mcp: FastMCP, config: AppConfig, client: DsmClient) -> Non
|
|||||||
params={"id": json.dumps(project_id)},
|
params={"id": json.dumps(project_id)},
|
||||||
)
|
)
|
||||||
except SynologyError as e:
|
except SynologyError as e:
|
||||||
# DSM refuses to delete a running project. We deliberately do NOT
|
|
||||||
# auto-stop — that would be too destructive for a delete tool —
|
|
||||||
# but we tell the user how to proceed.
|
|
||||||
if status == "RUNNING":
|
|
||||||
return (
|
|
||||||
f"Cannot delete project '{project_name}' while it is running ({e}).\n"
|
|
||||||
f"Stop the project first with stop_project."
|
|
||||||
)
|
|
||||||
return f"Error deleting project '{project_name}': {e}"
|
return f"Error deleting project '{project_name}': {e}"
|
||||||
|
|
||||||
return (
|
return (
|
||||||
|
|||||||
@@ -317,6 +317,32 @@ async def test_delete_container_stopped_confirmed():
|
|||||||
assert "myapp_web" in result
|
assert "myapp_web" in result
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_delete_container_sends_required_params():
|
||||||
|
"""SYNO.Docker.Container/delete must include name (json-encoded), force=false,
|
||||||
|
and preserve_profile=false — without them DSM returns error 114."""
|
||||||
|
from mcp_synology_container.modules.containers import register_containers
|
||||||
|
|
||||||
|
client = AsyncMock()
|
||||||
|
client.request.return_value = {
|
||||||
|
"details": {"State": {"Running": False, "Status": "exited"}},
|
||||||
|
"profile": {"image": "nginx:latest"},
|
||||||
|
}
|
||||||
|
|
||||||
|
mcp, tools = make_mock_mcp()
|
||||||
|
register_containers(mcp, make_config(), client)
|
||||||
|
|
||||||
|
await tools["delete_container"]("myapp_web", confirmed=True)
|
||||||
|
|
||||||
|
# Find the delete call (second call — first is Container/get)
|
||||||
|
delete_calls = [c for c in client.request.call_args_list if c.args[1] == "delete"]
|
||||||
|
assert len(delete_calls) == 1
|
||||||
|
params = delete_calls[0].kwargs.get("params") or {}
|
||||||
|
assert params["name"] == '"myapp_web"'
|
||||||
|
assert params["force"] == "false"
|
||||||
|
assert params["preserve_profile"] == "false"
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_container_stats_cpu_calculation():
|
async def test_container_stats_cpu_calculation():
|
||||||
"""CPU% is computed via the standard Docker formula."""
|
"""CPU% is computed via the standard Docker formula."""
|
||||||
|
|||||||
@@ -898,20 +898,17 @@ async def test_delete_project_happy_path():
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_delete_project_running_returns_stop_hint():
|
async def test_delete_project_running_blocked_connector_side():
|
||||||
"""DSM refusing to delete a running project produces a clean 'stop_project' hint
|
"""Live test showed that DSM does NOT reject Project/delete on a running project —
|
||||||
rather than a raw error."""
|
it silently orphans the containers. The connector must therefore block the call
|
||||||
from mcp_synology_container.dsm_client import SynologyError
|
itself when the project is RUNNING, without ever calling client.request(delete)."""
|
||||||
|
client, calls = make_delete_project_client(project=SAMPLE_PROJECT_RUNNING)
|
||||||
client, calls = make_delete_project_client(
|
|
||||||
project=SAMPLE_PROJECT_RUNNING,
|
|
||||||
delete_raises=SynologyError("Project is running", code=2103),
|
|
||||||
)
|
|
||||||
tools = make_projects_tools(client)
|
tools = make_projects_tools(client)
|
||||||
|
|
||||||
result = await tools["delete_project"]("myapp", confirmed=True)
|
result = await tools["delete_project"]("myapp", confirmed=True)
|
||||||
|
|
||||||
|
assert "RUNNING" in result
|
||||||
assert "stop_project" in result
|
assert "stop_project" in result
|
||||||
assert "running" in result.lower()
|
# The delete endpoint must NOT have been called — no orphaned containers.
|
||||||
# No "deleted" success line
|
delete_calls = [m for _, m, _ in calls if m == "delete"]
|
||||||
assert "deleted (registration removed)" not in result
|
assert delete_calls == []
|
||||||
|
|||||||
@@ -362,7 +362,7 @@ wheels = [
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "mcp-synology-container"
|
name = "mcp-synology-container"
|
||||||
version = "0.3.2"
|
version = "0.3.3"
|
||||||
source = { editable = "." }
|
source = { editable = "." }
|
||||||
dependencies = [
|
dependencies = [
|
||||||
{ name = "click" },
|
{ name = "click" },
|
||||||
|
|||||||
Reference in New Issue
Block a user