diff --git a/CLAUDE.md b/CLAUDE.md index 58279dc..1267e2d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -45,10 +45,8 @@ via Container Manager. Der MCP-Server ist in Claude Desktop aktiv verbunden. ### Bekannte Bugs -- Container-Name wird manchmal mit Hash-Präfix zurückgegeben (z.B. `f93cb8b504f7_jenkins`) - → Tritt auf wenn Service-Name in compose.yaml von `container_name` abweicht -- `redeploy_project` schlägt mit DSM 2101/1202 fehl bei falschem Projektstatus - → Workaround: `stop_project` + `start_project` separat oder `docker compose` per SSH +~~- Container-Name mit Hash-Präfix (`f93cb8b504f7_jenkins`) → behoben: `_strip_hash_prefix` + `_resolve_container_name` in allen Container-Tools~~ +~~- `redeploy_project` DSM 2101/1202 bei falschem Status → behoben: status-aware Logik (RUNNING/STOPPED/BUILD_FAILED)~~ --- diff --git a/src/mcp_synology_container/modules/containers.py b/src/mcp_synology_container/modules/containers.py index 0f31d08..2a268a3 100644 --- a/src/mcp_synology_container/modules/containers.py +++ b/src/mcp_synology_container/modules/containers.py @@ -3,6 +3,7 @@ from __future__ import annotations import logging +import re from typing import TYPE_CHECKING, Any if TYPE_CHECKING: @@ -13,6 +14,51 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) +# Matches DSM hash-prefixed container names like "f93cb8b504f7_jenkins" +_HASH_PREFIX_RE = re.compile(r"^[a-f0-9]{12}_(.+)$") + + +def _strip_hash_prefix(name: str) -> str: + """Strip 12-char hex hash prefix from container names. + + DSM sometimes returns names like 'f93cb8b504f7_jenkins' when the + service name in compose.yaml differs from container_name. Returns the + clean name (also strips a leading slash if present). + """ + clean = name.lstrip("/") + match = _HASH_PREFIX_RE.match(clean) + return match.group(1) if match else clean + + +async def _resolve_container_name(client: DsmClient, user_name: str) -> str: + """Resolve a user-supplied name to the actual DSM container name. + + Needed because DSM may store the container as 'f93cb8b504f7_jenkins' + while the user passes 'jenkins'. Falls back to user_name unchanged if + the list cannot be fetched or no match is found. + + Args: + client: DsmClient instance. + user_name: Name as provided by the user (may or may not have prefix). + + Returns: + Actual container name as registered in DSM. + """ + clean_user = _strip_hash_prefix(user_name) + try: + data = await client.request( + "SYNO.Docker.Container", + "list", + params={"limit": "-1", "offset": "0", "type": "all"}, + ) + for c in data.get("containers", []): + actual = c.get("name", "") + if actual == user_name or _strip_hash_prefix(actual) == clean_user: + return actual + except Exception: + pass + return user_name + def register_containers(mcp: FastMCP, config: AppConfig, client: DsmClient) -> None: """Register all container management tools with the MCP server.""" @@ -50,7 +96,7 @@ def register_containers(mcp: FastMCP, config: AppConfig, client: DsmClient) -> N lines = [f"Containers ({len(containers)} total):", ""] for container in sorted(containers, key=lambda c: c.get("name", "")): - name = container.get("name", "?") + name = _strip_hash_prefix(container.get("name", "?")) state = container.get("status", container.get("state", "?")) image = container.get("image", "?") lines.append(f" {name}") @@ -67,11 +113,12 @@ def register_containers(mcp: FastMCP, config: AppConfig, client: DsmClient) -> N Args: container_name: Name of the container to inspect. """ + resolved_name = await _resolve_container_name(client, container_name) try: data = await client.request( "SYNO.Docker.Container", "get", - params={"name": container_name}, + params={"name": resolved_name}, ) except Exception as e: return f"Error getting container '{container_name}': {e}" @@ -79,7 +126,8 @@ def register_containers(mcp: FastMCP, config: AppConfig, client: DsmClient) -> N if not data: return f"Container '{container_name}' not found." - return _format_container_detail(container_name, data) + display_name = _strip_hash_prefix(resolved_name) + return _format_container_detail(display_name, data) @mcp.tool() async def get_container_logs( @@ -94,8 +142,9 @@ def register_containers(mcp: FastMCP, config: AppConfig, client: DsmClient) -> N tail: Number of recent log lines to return (default 100). keyword: Optional keyword to filter log lines. """ + resolved_name = await _resolve_container_name(client, container_name) params: dict[str, Any] = { - "name": container_name, + "name": resolved_name, "limit": tail, "offset": 0, "sort_dir": "DESC", @@ -117,7 +166,8 @@ def register_containers(mcp: FastMCP, config: AppConfig, client: DsmClient) -> N return f"No logs found for container '{container_name}'." total = data.get("total", len(logs)) - header = f"Logs for {container_name} (showing {len(logs)} of {total}):\n" + display_name = _strip_hash_prefix(container_name) + header = f"Logs for {display_name} (showing {len(logs)} of {total}):\n" # Logs are returned in DESC order, reverse for chronological display lines = [] @@ -149,19 +199,18 @@ def register_containers(mcp: FastMCP, config: AppConfig, client: DsmClient) -> N return "No stats data returned." # Response is a dict keyed by container ID hash; each entry has "name" - # with a leading slash (e.g. "/jenkins"). + # with a leading slash (e.g. "/jenkins") and may have a hash prefix. + clean_query = _strip_hash_prefix(container_name) target: dict[str, Any] | None = None for entry in data.values(): - entry_name = entry.get("name", "").lstrip("/") - if entry_name == container_name.lstrip("/"): + entry_name = _strip_hash_prefix(entry.get("name", "")) + if entry_name == clean_query: target = entry break if target is None: - return ( - f"Container '{container_name}' not found in stats. " - f"Available: {', '.join(v.get('name', '?').lstrip('/') for v in data.values())}" - ) + available = ", ".join(_strip_hash_prefix(v.get("name", "?")) for v in data.values()) + return f"Container '{container_name}' not found in stats. Available: {available}" # ── CPU % ──────────────────────────────────────────────────────────── cpu_stats = target.get("cpu_stats", {}) @@ -239,12 +288,13 @@ def register_containers(mcp: FastMCP, config: AppConfig, client: DsmClient) -> N f"Call this tool again with confirmed=True to proceed." ) + resolved_name = await _resolve_container_name(client, container_name) try: data = await client.request( "SYNO.Docker.Container", "exec", params={ - "name": container_name, + "name": resolved_name, "command": command, }, ) diff --git a/src/mcp_synology_container/modules/projects.py b/src/mcp_synology_container/modules/projects.py index 4ebf1a2..1969d57 100644 --- a/src/mcp_synology_container/modules/projects.py +++ b/src/mcp_synology_container/modules/projects.py @@ -2,11 +2,13 @@ from __future__ import annotations +import contextlib import logging from typing import TYPE_CHECKING, Any if TYPE_CHECKING: from mcp.server.fastmcp import FastMCP + from mcp_synology_container.config import AppConfig from mcp_synology_container.dsm_client import DsmClient @@ -33,7 +35,7 @@ def register_projects(mcp: FastMCP, config: AppConfig, client: DsmClient) -> Non return "No projects found." lines = ["Projects:", ""] - for project_id, proj in sorted(projects.items(), key=lambda x: x[1].get("name", "")): + for _project_id, proj in sorted(projects.items(), key=lambda x: x[1].get("name", "")): name = proj.get("name", "?") status = proj.get("status", "?") path = proj.get("path", "?") @@ -115,7 +117,12 @@ def register_projects(mcp: FastMCP, config: AppConfig, client: DsmClient) -> Non @mcp.tool() async def redeploy_project(project_name: str, confirmed: bool = False) -> str: - """Redeploy a project: pull latest images, stop, and restart. + """Redeploy a project by stopping and restarting it. + + 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 This operation will briefly take the project offline. Requires confirmation before executing. @@ -126,10 +133,8 @@ def register_projects(mcp: FastMCP, config: AppConfig, client: DsmClient) -> Non """ if not confirmed: return ( - f"Redeploying project '{project_name}' will:\n" - f" 1. Pull latest images\n" - f" 2. Stop all containers\n" - f" 3. Restart with new images\n\n" + f"Redeploying project '{project_name}' will stop and restart all its " + f"containers (auto-detects current state).\n\n" f"Call this tool again with confirmed=True to proceed." ) @@ -138,43 +143,44 @@ def register_projects(mcp: FastMCP, config: AppConfig, client: DsmClient) -> Non return f"Project '{project_name}' not found." project_id = project.get("id", "") + status = (project.get("status") or "").upper() results = [] try: - # Step 1: Pull latest images via build (triggers compose pull) - results.append("Step 1/3: Pulling latest images...") - try: - await client.request( - "SYNO.Docker.Project", - "build", - params={"id": project_id, "force": "true"}, + if status == "STOPPED": + results.append("Project is STOPPED — starting directly.") + results.append("Step 1/1: Starting project...") + 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...") + 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("Step 2/2: Starting project...") + await client.request("SYNO.Docker.Project", "start", params={"id": project_id}) + results.append(" Project started.") + + else: + return ( + f"Cannot redeploy '{project_name}': unexpected status '{status}'.\n" + f"Workaround: use stop_project + start_project separately." ) - results.append(" Images pulled.") - except Exception as e: - results.append(f" Warning: pull step failed ({e}), continuing with restart.") - - # Step 2: Stop the project - results.append("Step 2/3: Stopping project...") - await client.request( - "SYNO.Docker.Project", - "stop", - params={"id": project_id}, - ) - results.append(" Project stopped.") - - # Step 3: Start the project - results.append("Step 3/3: Starting project...") - await client.request( - "SYNO.Docker.Project", - "start", - params={"id": project_id}, - ) - results.append(" Project started.") results.append(f"\nProject '{project_name}' redeployed successfully.") except Exception as e: results.append(f"Error during redeploy: {e}") + results.append("Workaround: use stop_project + start_project separately.") return "\n".join(results) diff --git a/tests/test_modules/test_containers.py b/tests/test_modules/test_containers.py index 04a442a..a878bc6 100644 --- a/tests/test_modules/test_containers.py +++ b/tests/test_modules/test_containers.py @@ -51,6 +51,18 @@ SAMPLE_CONTAINERS_DATA = { ] } +# Container data where DSM returns hash-prefixed names +HASH_PREFIXED_CONTAINERS_DATA = { + "containers": [ + { + "name": "f93cb8b504f7_jenkins", + "status": "running", + "image": "jenkins/jenkins:lts", + "project_name": "frostiq", + }, + ] +} + SAMPLE_LOGS_DATA = { "logs": [ { @@ -315,6 +327,161 @@ async def test_container_stats_api_error(): assert "Error" in result +# ────────────────────────────────────────────────────────────────────────────── +# Bug 1: hash-prefix stripping +# ────────────────────────────────────────────────────────────────────────────── + + +def test_strip_hash_prefix_strips_prefix(): + from mcp_synology_container.modules.containers import _strip_hash_prefix + + assert _strip_hash_prefix("f93cb8b504f7_jenkins") == "jenkins" + + +def test_strip_hash_prefix_no_prefix(): + from mcp_synology_container.modules.containers import _strip_hash_prefix + + assert _strip_hash_prefix("jenkins") == "jenkins" + + +def test_strip_hash_prefix_leading_slash(): + from mcp_synology_container.modules.containers import _strip_hash_prefix + + assert _strip_hash_prefix("/jenkins") == "jenkins" + + +def test_strip_hash_prefix_slash_with_hash(): + from mcp_synology_container.modules.containers import _strip_hash_prefix + + assert _strip_hash_prefix("/f93cb8b504f7_jenkins") == "jenkins" + + +@pytest.mark.asyncio +async def test_list_containers_strips_hash_prefix(): + """list_containers must display the clean name without the hash prefix.""" + from mcp_synology_container.modules.containers import register_containers + + client = AsyncMock() + client.request.return_value = HASH_PREFIXED_CONTAINERS_DATA + + mcp, tools = make_mock_mcp() + register_containers(mcp, make_config(), client) + + result = await tools["list_containers"]() + assert "jenkins" in result + assert "f93cb8b504f7_jenkins" not in result + + +@pytest.mark.asyncio +async def test_container_stats_strips_hash_prefix(): + """container_stats must match even when DSM returns a hash-prefixed name.""" + from mcp_synology_container.modules.containers import register_containers + + stats_with_hash = { + "abc123": { + "name": "/f93cb8b504f7_jenkins", + "cpu_stats": { + "cpu_usage": {"total_usage": 500_000, "percpu_usage": [500_000]}, + "system_cpu_usage": 100_000_000_000, + "online_cpus": 1, + }, + "precpu_stats": { + "cpu_usage": {"total_usage": 0}, + "system_cpu_usage": 0, + }, + "memory_stats": {"usage": 1024, "limit": 2048}, + "networks": {}, + "blkio_stats": {"io_service_bytes_recursive": []}, + } + } + + client = AsyncMock() + client.request.return_value = stats_with_hash + + mcp, tools = make_mock_mcp() + register_containers(mcp, make_config(), client) + + # User passes clean name; must still match the hash-prefixed entry + result = await tools["container_stats"]("jenkins") + assert "not found" not in result.lower() + assert "CPU" in result + + +@pytest.mark.asyncio +async def test_get_container_status_resolves_hash_prefix(): + """get_container_status resolves 'jenkins' to 'f93cb8b504f7_jenkins' for DSM call.""" + from mcp_synology_container.modules.containers import register_containers + + detail_data = {"State": {"Status": "running", "Running": True}, "Config": {"Image": "jenkins/jenkins:lts"}} + + async def mock_request(api, method, **kwargs): + if api == "SYNO.Docker.Container" and method == "list": + return HASH_PREFIXED_CONTAINERS_DATA + if api == "SYNO.Docker.Container" and method == "get": + # Must be called with the hash-prefixed name + assert kwargs["params"]["name"] == "f93cb8b504f7_jenkins" + return detail_data + return {} + + client = AsyncMock() + client.request.side_effect = mock_request + + mcp, tools = make_mock_mcp() + register_containers(mcp, make_config(), client) + + result = await tools["get_container_status"]("jenkins") + assert "Error" not in result + # Display name should be clean (no hash prefix) + assert "f93cb8b504f7" not in result + + +@pytest.mark.asyncio +async def test_get_container_logs_resolves_hash_prefix(): + """get_container_logs resolves 'jenkins' to 'f93cb8b504f7_jenkins' for DSM call.""" + from mcp_synology_container.modules.containers import register_containers + + async def mock_request(api, method, **kwargs): + if api == "SYNO.Docker.Container" and method == "list": + 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 {} + + client = AsyncMock() + client.request.side_effect = mock_request + + mcp, tools = make_mock_mcp() + register_containers(mcp, make_config(), client) + + result = await tools["get_container_logs"]("jenkins") + assert "started" in result + assert "f93cb8b504f7" not in result + + +@pytest.mark.asyncio +async def test_exec_in_container_resolves_hash_prefix(): + """exec_in_container resolves 'jenkins' to 'f93cb8b504f7_jenkins' for DSM call.""" + from mcp_synology_container.modules.containers import register_containers + + async def mock_request(api, method, **kwargs): + if api == "SYNO.Docker.Container" and method == "list": + return HASH_PREFIXED_CONTAINERS_DATA + if api == "SYNO.Docker.Container" and method == "exec": + assert kwargs["params"]["name"] == "f93cb8b504f7_jenkins" + return {"output": "ok", "exit_code": 0} + return {} + + client = AsyncMock() + client.request.side_effect = mock_request + + mcp, tools = make_mock_mcp() + register_containers(mcp, make_config(), client) + + result = await tools["exec_in_container"]("jenkins", "echo ok", confirmed=True) + assert "ok" in result + + @pytest.mark.asyncio async def test_container_stats_no_precpu_graceful(): """When precpu_stats has no system_cpu_usage (first poll), CPU% = 0.""" diff --git a/tests/test_modules/test_projects.py b/tests/test_modules/test_projects.py index 113870d..21e9e01 100644 --- a/tests/test_modules/test_projects.py +++ b/tests/test_modules/test_projects.py @@ -161,3 +161,150 @@ async def test_redeploy_project_requires_confirmation(): result = await tools["redeploy_project"]("myapp", confirmed=False) assert "confirmed=True" in result client.request.assert_not_called() + + +# ────────────────────────────────────────────────────────────────────────────── +# Bug 2: status-aware redeploy +# ────────────────────────────────────────────────────────────────────────────── + + +def make_projects_tools(client): + from mcp_synology_container.modules.projects import register_projects + from mcp_synology_container.config import AppConfig, ConnectionConfig + + config = AppConfig( + schema_version=1, + connection=ConnectionConfig(host="nas.local", port=443, https=True, verify_ssl=True), + ) + tools: dict = {} + + class MockMCP: + def tool(self): + def decorator(fn): + tools[fn.__name__] = fn + return fn + return decorator + + register_projects(MockMCP(), config, client) + return tools + + +def project_list(status: str) -> dict: + return { + "uuid-1": { + "id": "uuid-1", + "name": "myapp", + "status": status, + "path": "/volume1/docker/myapp", + "containerIds": ["abc123"], + "services": [], + } + } + + +@pytest.mark.asyncio +async def test_redeploy_running_project(): + """RUNNING project: stop then start (2 steps).""" + client = AsyncMock() + calls = [] + + async def mock_request(api, method, **kwargs): + calls.append((api, method)) + return project_list("RUNNING") if method == "list" else {} + + client.request.side_effect = mock_request + tools = make_projects_tools(client) + + result = await tools["redeploy_project"]("myapp", confirmed=True) + + assert "redeployed successfully" in result + methods = [m for _, m in calls] + assert "stop" in methods + assert "start" in methods + # stop must come before start + assert methods.index("stop") < methods.index("start") + + +@pytest.mark.asyncio +async def test_redeploy_stopped_project_starts_directly(): + """STOPPED project: skip stop, just start.""" + client = AsyncMock() + calls = [] + + async def mock_request(api, method, **kwargs): + calls.append((api, method)) + return project_list("STOPPED") if method == "list" else {} + + client.request.side_effect = mock_request + tools = make_projects_tools(client) + + result = await tools["redeploy_project"]("myapp", confirmed=True) + + assert "redeployed successfully" in result + methods = [m for _, m in calls] + assert "stop" not in methods + assert "start" in methods + assert "STOPPED" in result or "starting directly" in result.lower() + + +@pytest.mark.asyncio +async def test_redeploy_build_failed_project(): + """BUILD_FAILED project: force-stop (non-fatal), then start.""" + client = AsyncMock() + calls = [] + + async def mock_request(api, method, **kwargs): + calls.append((api, method)) + return project_list("BUILD_FAILED") if method == "list" else {} + + client.request.side_effect = mock_request + tools = make_projects_tools(client) + + result = await tools["redeploy_project"]("myapp", confirmed=True) + + assert "redeployed successfully" in result + methods = [m for _, m in calls] + assert "stop" in methods + assert "start" in methods + assert methods.index("stop") < methods.index("start") + + +@pytest.mark.asyncio +async def test_redeploy_build_failed_stop_error_nonfatal(): + """BUILD_FAILED: stop failure must not abort the redeploy.""" + from mcp_synology_container.dsm_client import SynologyError + + client = AsyncMock() + + async def mock_request(api, method, **kwargs): + if method == "list": + return project_list("BUILD_FAILED") + if method == "stop": + raise SynologyError("already stopped", code=2101) + return {} # start succeeds + + client.request.side_effect = mock_request + tools = make_projects_tools(client) + + result = await tools["redeploy_project"]("myapp", confirmed=True) + + assert "redeployed successfully" in result + + +@pytest.mark.asyncio +async def test_redeploy_unknown_status_returns_error(): + """Unknown status must return a clear error with a workaround hint.""" + client = AsyncMock() + + async def mock_request(api, method, **kwargs): + if method == "list": + return project_list("UPDATING") + return {} + + client.request.side_effect = mock_request + tools = make_projects_tools(client) + + result = await tools["redeploy_project"]("myapp", confirmed=True) + + assert "UPDATING" in result + assert "Workaround" in result or "stop_project" in result