diff --git a/src/mcp_synology_container/modules/containers.py b/src/mcp_synology_container/modules/containers.py index 280b531..0decf18 100644 --- a/src/mcp_synology_container/modules/containers.py +++ b/src/mcp_synology_container/modules/containers.py @@ -312,6 +312,58 @@ def register_containers(mcp: FastMCP, config: AppConfig, client: DsmClient) -> N return "\n".join(result_lines) + @mcp.tool() + async def delete_container(container_name: str, confirmed: bool = False) -> str: + """Delete a container. + + Container must be stopped before deletion. Without confirmed=True, + returns a preview only. + + Args: + container_name: Name of the container. + confirmed: Must be True to proceed. Set to True to confirm deletion. + """ + if not confirmed: + return ( + f"Preview: would delete container '{container_name}'.\n" + f"Call this tool again with confirmed=True to proceed." + ) + + resolved_name = await _resolve_container_name(client, container_name) + display_name = _strip_hash_prefix(resolved_name) + + try: + data = await client.request( + "SYNO.Docker.Container", + "get", + params={"name": resolved_name}, + ) + except Exception as e: + return f"Error inspecting container '{container_name}': {e}" + + if not data: + return f"Container '{container_name}' not found." + + details = data.get("details", {}) or {} + state = details.get("State", {}) or {} + running = state.get("Running", False) + + if running: + return ( + f"Cannot delete '{display_name}': container is still running.\n" + f"Stop the container first with stop_project or stop_container." + ) + + try: + await client.request( + "SYNO.Docker.Container", + "delete", + params={"name": resolved_name}, + ) + return f"Deleted container '{display_name}'." + except Exception as e: + return f"Error deleting '{display_name}': {e}" + def _container_in_project(container: dict[str, Any], project_name: str) -> bool: """Check if a container belongs to a project based on its labels.""" @@ -350,9 +402,7 @@ def _format_container_detail(name: str, data: dict[str, Any]) -> str: lines.append(f" Exit code: {state.get('ExitCode', '?')}") # IP addresses from all attached networks - networks: dict[str, Any] = ( - details.get("NetworkSettings", {}) or {} - ).get("Networks", {}) or {} + networks: dict[str, Any] = (details.get("NetworkSettings", {}) or {}).get("Networks", {}) or {} for net_name, net in networks.items(): ip = (net or {}).get("IPAddress", "") if ip: diff --git a/src/mcp_synology_container/modules/images.py b/src/mcp_synology_container/modules/images.py index a74f828..b8a8514 100644 --- a/src/mcp_synology_container/modules/images.py +++ b/src/mcp_synology_container/modules/images.py @@ -217,7 +217,8 @@ def register_images(mcp: FastMCP, config: AppConfig, client: DsmClient) -> None: img_hash = target.get("id", "") # Check if image is in use by any container - in_use_by: list[str] = [] + in_use_running: list[str] = [] + in_use_stopped: list[str] = [] try: ctr_data = await client.request( "SYNO.Docker.Container", @@ -231,12 +232,25 @@ def register_images(mcp: FastMCP, config: AppConfig, client: DsmClient) -> None: ctr_img_id == img_hash or (hash_prefix and ctr_img_id.startswith(hash_prefix)) ): ctr_name = ctr.get("name") or ctr.get("Names", ["?"])[0] - in_use_by.append(ctr_name) + status = ctr.get("status", ctr.get("state", "")).lower() + if status == "running": + in_use_running.append(ctr_name) + else: + in_use_stopped.append(ctr_name) except Exception as e: logger.debug("Could not fetch containers for in-use check: %s", e) - if in_use_by: - return f"Cannot delete '{display_name}': image is used by " + ", ".join(in_use_by) + if in_use_running: + return ( + f"Cannot delete '{display_name}': image is used by running container(s): " + + ", ".join(in_use_running) + ) + + if in_use_stopped: + return ( + f"Cannot delete '{display_name}': image is used by stopped container '{in_use_stopped[0]}'.\n" + f"Delete the container first or run system_prune to clean up stopped containers." + ) if not confirmed: return ( diff --git a/src/mcp_synology_container/modules/projects.py b/src/mcp_synology_container/modules/projects.py index 1969d57..ed12326 100644 --- a/src/mcp_synology_container/modules/projects.py +++ b/src/mcp_synology_container/modules/projects.py @@ -122,7 +122,7 @@ def register_projects(mcp: FastMCP, config: AppConfig, client: DsmClient) -> Non Checks the current project status to determine the correct action: - RUNNING → stop, then start - STOPPED → start directly (nothing to stop) - - BUILD_FAILED → force-stop, then start + - BUILD_FAILED → stop, pull images, then start This operation will briefly take the project offline. Requires confirmation before executing. @@ -153,19 +153,23 @@ def register_projects(mcp: FastMCP, config: AppConfig, client: DsmClient) -> Non await client.request("SYNO.Docker.Project", "start", params={"id": project_id}) results.append(" Project started.") - elif status in ("RUNNING", "BUILD_FAILED", ""): - if status == "RUNNING": - results.append("Step 1/2: Stopping project...") + elif status == "BUILD_FAILED": + results.append("Step 1/3: Stopping failed build...") + with contextlib.suppress(Exception): await client.request("SYNO.Docker.Project", "stop", params={"id": project_id}) - results.append(" Project stopped.") - elif status == "BUILD_FAILED": - results.append("Step 1/2: Stopping failed build...") - with contextlib.suppress(Exception): - await client.request( - "SYNO.Docker.Project", "stop", params={"id": project_id} - ) - results.append(" Build stopped.") + results.append(" Build stopped.") + results.append("Step 2/3: Pulling updated images...") + with contextlib.suppress(Exception): + await client.request("SYNO.Docker.Image", "pull", params={"id": project_id}) + results.append(" Images pulled.") + results.append("Step 3/3: Starting project...") + await client.request("SYNO.Docker.Project", "start", params={"id": project_id}) + results.append(" Project started.") + elif status in ("RUNNING", ""): + results.append("Step 1/2: Stopping project...") + await client.request("SYNO.Docker.Project", "stop", params={"id": project_id}) + results.append(" Project stopped.") results.append("Step 2/2: Starting project...") await client.request("SYNO.Docker.Project", "start", params={"id": project_id}) results.append(" Project started.") diff --git a/tests/test_modules/test_containers.py b/tests/test_modules/test_containers.py index b320196..81b755d 100644 --- a/tests/test_modules/test_containers.py +++ b/tests/test_modules/test_containers.py @@ -247,6 +247,76 @@ async def test_container_stats_found(): assert "Block I/O" in result +# ────────────────────────────────────────────────────────────────────────────── +# delete_container +# ────────────────────────────────────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_delete_container_preview(): + from mcp_synology_container.modules.containers import register_containers + + client = AsyncMock() + + mcp, tools = make_mock_mcp() + register_containers(mcp, make_config(), client) + + result = await tools["delete_container"]("myapp_web", confirmed=False) + assert "Preview" in result + assert "myapp_web" in result + client.request.assert_not_called() + + +@pytest.mark.asyncio +async def test_delete_container_not_found(): + from mcp_synology_container.modules.containers import register_containers + + client = AsyncMock() + client.request.return_value = None + + mcp, tools = make_mock_mcp() + register_containers(mcp, make_config(), client) + + result = await tools["delete_container"]("nonexistent", confirmed=True) + assert "not found" in result + + +@pytest.mark.asyncio +async def test_delete_container_running_blocked(): + from mcp_synology_container.modules.containers import register_containers + + client = AsyncMock() + client.request.return_value = { + "details": {"State": {"Running": True, "Status": "running"}}, + "profile": {"image": "nginx:latest"}, + } + + mcp, tools = make_mock_mcp() + register_containers(mcp, make_config(), client) + + result = await tools["delete_container"]("myapp_web", confirmed=True) + assert "running" in result.lower() + assert "Cannot delete" in result + + +@pytest.mark.asyncio +async def test_delete_container_stopped_confirmed(): + 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) + + result = await tools["delete_container"]("myapp_web", confirmed=True) + assert "Deleted" in result + assert "myapp_web" in result + + @pytest.mark.asyncio async def test_container_stats_cpu_calculation(): """CPU% is computed via the standard Docker formula.""" @@ -537,7 +607,10 @@ async def test_get_container_logs_resolves_hash_prefix(): return HASH_PREFIXED_CONTAINERS_DATA if api == "SYNO.Docker.Container.Log" and method == "get": assert kwargs["params"]["name"] == "f93cb8b504f7_jenkins" - return {"logs": [{"created": "2025-01-01", "stream": "stdout", "text": "started"}], "total": 1} + return { + "logs": [{"created": "2025-01-01", "stream": "stdout", "text": "started"}], + "total": 1, + } return {} client = AsyncMock() diff --git a/tests/test_modules/test_images.py b/tests/test_modules/test_images.py index a3c9b23..588f38f 100644 --- a/tests/test_modules/test_images.py +++ b/tests/test_modules/test_images.py @@ -255,7 +255,7 @@ async def test_delete_image_not_found(): @pytest.mark.asyncio -async def test_delete_image_in_use_blocked(): +async def test_delete_image_in_use_by_running_blocked(): from mcp_synology_container.modules.images import register_images client = AsyncMock() @@ -265,7 +265,7 @@ async def test_delete_image_in_use_blocked(): if api == "SYNO.Docker.Image" and method == "list": return SAMPLE_IMAGES if api == "SYNO.Docker.Container": - return SAMPLE_CONTAINERS # nginx is in use + return SAMPLE_CONTAINERS # nginx is in use (running) return {} client.request.side_effect = mock_request @@ -274,10 +274,41 @@ async def test_delete_image_in_use_blocked(): result = await tools["delete_image"](image_id="nginx:1.24", confirmed=True) assert "Cannot delete" in result + assert "running" in result.lower() assert "my-nginx" in result client.post_request.assert_not_called() +@pytest.mark.asyncio +async def test_delete_image_in_use_by_stopped_blocked(): + from mcp_synology_container.modules.images import register_images + + client = AsyncMock() + client.post_request = AsyncMock(return_value={}) + + async def mock_request(api, method, **kwargs): + if api == "SYNO.Docker.Image" and method == "list": + return SAMPLE_IMAGES + if api == "SYNO.Docker.Container": + return { + "containers": [ + {"name": "stopped-nginx", "image_id": "sha256:aaaa", "status": "exited"} + ] + } + return {} + + client.request.side_effect = mock_request + mcp, tools = make_mock_mcp() + register_images(mcp, make_config(), client) + + result = await tools["delete_image"](image_id="nginx:1.24", confirmed=True) + assert "Cannot delete" in result + assert "stopped" in result.lower() + assert "stopped-nginx" in result + assert "system_prune" in result + client.post_request.assert_not_called() + + @pytest.mark.asyncio async def test_delete_image_by_hash(): import json diff --git a/tests/test_modules/test_projects.py b/tests/test_modules/test_projects.py index 21e9e01..087d621 100644 --- a/tests/test_modules/test_projects.py +++ b/tests/test_modules/test_projects.py @@ -100,6 +100,7 @@ async def test_list_projects_tool(): def decorator(fn): tools[fn.__name__] = fn return fn + return decorator register_projects(MockMCP(), config, client) @@ -128,6 +129,7 @@ async def test_stop_project_requires_confirmation(): def decorator(fn): tools[fn.__name__] = fn return fn + return decorator register_projects(MockMCP(), config, client) @@ -154,6 +156,7 @@ async def test_redeploy_project_requires_confirmation(): def decorator(fn): tools[fn.__name__] = fn return fn + return decorator register_projects(MockMCP(), config, client) @@ -183,6 +186,7 @@ def make_projects_tools(client): def decorator(fn): tools[fn.__name__] = fn return fn + return decorator register_projects(MockMCP(), config, client) @@ -249,7 +253,7 @@ async def test_redeploy_stopped_project_starts_directly(): @pytest.mark.asyncio async def test_redeploy_build_failed_project(): - """BUILD_FAILED project: force-stop (non-fatal), then start.""" + """BUILD_FAILED project: stop, pull images, then start (3 steps).""" client = AsyncMock() calls = [] @@ -265,13 +269,16 @@ async def test_redeploy_build_failed_project(): assert "redeployed successfully" in result methods = [m for _, m in calls] assert "stop" in methods + assert "pull" in methods # New: pull step assert "start" in methods - assert methods.index("stop") < methods.index("start") + # Order: stop → pull → start + assert methods.index("stop") < methods.index("pull") + assert methods.index("pull") < methods.index("start") @pytest.mark.asyncio async def test_redeploy_build_failed_stop_error_nonfatal(): - """BUILD_FAILED: stop failure must not abort the redeploy.""" + """BUILD_FAILED: stop/pull failure must not abort the redeploy.""" from mcp_synology_container.dsm_client import SynologyError client = AsyncMock() @@ -281,6 +288,8 @@ async def test_redeploy_build_failed_stop_error_nonfatal(): return project_list("BUILD_FAILED") if method == "stop": raise SynologyError("already stopped", code=2101) + if method == "pull": + raise SynologyError("pull failed", code=2102) return {} # start succeeds client.request.side_effect = mock_request diff --git a/uv.lock b/uv.lock index 1d4970e..eac7600 100644 --- a/uv.lock +++ b/uv.lock @@ -362,7 +362,7 @@ wheels = [ [[package]] name = "mcp-synology-container" -version = "0.1.0" +version = "0.2.0" source = { editable = "." } dependencies = [ { name = "click" },