From 82e8167f6754b827e43704275435dfc102b55712 Mon Sep 17 00:00:00 2001 From: Marcus van Elst Date: Mon, 18 May 2026 13:13:42 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20v0.4.3=20=E2=80=94=20inspect=5Fimage=20r?= =?UTF-8?q?endering=20polish=20(follow-up=20to=20#4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three live-NAS observations that the 0.4.2 implementation got wrong: 1. Header showed "?:?" — DSM Image/get returns image="" and tag="" for name:tag lookups; the response fields are unreliable. The header now echoes the user-supplied image_id, falling back to the sha256 id only when image_id itself is empty. 2. Ports printed as raw Python dicts ({'port':'22','protocol':'tcp'}). The ports array is a list of {port, protocol} objects — each entry now renders as "22/tcp". 3. Environment printed as raw Python dicts ({'key':'PATH','value':...}). The env array is a list of {key, value} objects — each entry now renders as "PATH=/usr/local/...". cmd, entrypoint, and volumes (plain string arrays) were already correct and are untouched. Tests updated to match the live shape; two new tests guard the header fallback. References #4 (already closed by 0.4.2 — no need to reopen for a rendering polish). Co-Authored-By: Claude Opus 4.7 --- CHANGELOG.md | 23 +++++++ pyproject.toml | 2 +- src/mcp_synology_container/modules/images.py | 30 +++++--- tests/test_modules/test_images.py | 72 ++++++++++++++++---- uv.lock | 2 +- 5 files changed, 106 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e0b14d..a7adec1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,29 @@ All notable changes to this project will be documented in this file. +## [0.4.3] - 2026-05-18 + +### Fixed + +- `inspect_image` rendering polish (follow-up to #4 after the 0.4.2 + parameter-contract fix). Three live-NAS observations that the + initial implementation got wrong: + - **Header showed `?:?`** — DSM `SYNO.Docker.Image/get` returns + `image=""` and `tag=""` when the lookup is by name:tag, so the + response fields are unreliable. The header now echoes the user- + supplied `image_id` (e.g. `gitea/gitea:1.26.1`), falling back to + the sha256 `id` only if `image_id` itself were empty. + - **Ports rendered as raw Python dicts** like + `{'port': '22', 'protocol': 'tcp'}`. The `ports` array is actually + a list of `{"port", "protocol"}` objects; each entry is now + formatted as `22/tcp`. + - **Environment rendered as raw Python dicts** like + `{'key': 'PATH', 'value': '...'}`. The `env` array is actually a + list of `{"key", "value"}` objects; each entry is now formatted + as `PATH=/usr/local/...`. + `cmd`, `entrypoint`, and `volumes` (plain string arrays) were + already correct and are unchanged. Referencing #4 (already closed). + ## [0.4.2] - 2026-05-18 ### Fixed diff --git a/pyproject.toml b/pyproject.toml index 20b9077..76c6299 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "mcp-synology-container" -version = "0.4.2" +version = "0.4.3" description = "MCP server for Synology Container Manager" requires-python = ">=3.12" dependencies = [ diff --git a/src/mcp_synology_container/modules/images.py b/src/mcp_synology_container/modules/images.py index ac76de0..78af1d0 100644 --- a/src/mcp_synology_container/modules/images.py +++ b/src/mcp_synology_container/modules/images.py @@ -290,10 +290,12 @@ def register_images(mcp: FastMCP, config: AppConfig, client: DsmClient) -> None: if not isinstance(data, dict) or not data: return f"Image '{image_id}' not found." - repo = data.get("image") or "?" - tag = data.get("tag") or "?" - display_name = f"{repo}:{tag}" img_hash = data.get("id") or "" + # The DSM Image/get response leaves `image` and `tag` empty when the + # lookup is by name:tag (live-confirmed against the NAS), so the + # response fields are unreliable for the header. Prefer the user- + # supplied image_id, then fall back to the id hash. + display_name = image_id or img_hash or "?" digest = data.get("digest") or "" size_val = data.get("size") or 0 virtual_size_val = data.get("virtual_size") or 0 @@ -301,9 +303,9 @@ def register_images(mcp: FastMCP, config: AppConfig, client: DsmClient) -> None: docker_version = data.get("docker_version") or "" cmd = data.get("cmd") or [] entrypoint = data.get("entrypoint") or [] - env_list: list[str] = data.get("env") or [] - ports: list[str] = data.get("ports") or [] - volumes: list[str] = data.get("volumes") or [] + env_list: list[Any] = data.get("env") or [] + ports: list[Any] = data.get("ports") or [] + volumes: list[Any] = data.get("volumes") or [] lines = [f"Image: {display_name}"] if img_hash: @@ -337,7 +339,13 @@ def register_images(mcp: FastMCP, config: AppConfig, client: DsmClient) -> None: lines.append("") lines.append(f"Exposed ports ({len(ports)}):") for port in ports: - lines.append(f" {port}") + # Live shape: {"port": "22", "protocol": "tcp"}. + if isinstance(port, dict): + port_num = port.get("port", "?") + proto = port.get("protocol", "tcp") + lines.append(f" {port_num}/{proto}") + else: + lines.append(f" {port}") if volumes: lines.append("") @@ -349,7 +357,13 @@ def register_images(mcp: FastMCP, config: AppConfig, client: DsmClient) -> None: lines.append("") lines.append(f"Environment ({len(env_list)}):") for var in env_list: - lines.append(f" {var}") + # Live shape: {"key": "PATH", "value": "/usr/local/sbin"}. + if isinstance(var, dict): + key = var.get("key", "?") + value = var.get("value", "") + lines.append(f" {key}={value}") + else: + lines.append(f" {var}") return "\n".join(lines) diff --git a/tests/test_modules/test_images.py b/tests/test_modules/test_images.py index 2fa32da..9531526 100644 --- a/tests/test_modules/test_images.py +++ b/tests/test_modules/test_images.py @@ -416,12 +416,17 @@ async def test_delete_image_api_error(): # ────────────────────────────────────────────────────────────────────────────── -# Authoritative DSM SYNO.Docker.Image/get response shape (v1). Top-level -# fields only — there is NO "layers" field on this endpoint, confirmed -# via API capture against the live NAS. +# Authoritative DSM SYNO.Docker.Image/get response shape (v1). +# Live-captured peculiarities: +# - `image` and `tag` come back as empty strings when the lookup is +# by name:tag — the header therefore has to fall back to image_id. +# - `ports` is an array of {"port", "protocol"} dicts, NOT strings. +# - `env` is an array of {"key", "value"} dicts, NOT strings. +# - `volumes` and `cmd`/`entrypoint` are plain string arrays. +# - There is no `layers` field on this endpoint. SAMPLE_INSPECT = { - "image": "nginx", - "tag": "1.24", + "image": "", + "tag": "", "id": "sha256:aaaa1234567890abcdef", "digest": "sha256:digestabcdef", "size": 50 * 1024 * 1024, @@ -430,8 +435,14 @@ SAMPLE_INSPECT = { "docker_version": "20.10.0", "cmd": ["nginx", "-g", "daemon off;"], "entrypoint": ["/docker-entrypoint.sh"], - "env": ["NGINX_VERSION=1.24", "PATH=/usr/local/sbin"], - "ports": ["80/tcp", "443/tcp"], + "env": [ + {"key": "NGINX_VERSION", "value": "1.24"}, + {"key": "PATH", "value": "/usr/local/sbin"}, + ], + "ports": [ + {"port": "80", "protocol": "tcp"}, + {"port": "443", "protocol": "tcp"}, + ], "volumes": ["/var/cache/nginx"], } @@ -467,10 +478,12 @@ async def test_inspect_image_by_name_tag(): async def test_inspect_image_by_hash(): from mcp_synology_container.modules.images import register_images - # Inspect data shaped for redis returned by hash lookup + # Inspect data shaped for redis returned by hash lookup. + # image/tag come back empty (matches live DSM behavior) — display + # falls back to image_id, then to the id hash. redis_inspect = { - "image": "redis", - "tag": "7", + "image": "", + "tag": "", "id": "sha256:ccccredis", "digest": "sha256:rdigest", "size": 30 * 1024 * 1024, @@ -478,7 +491,7 @@ async def test_inspect_image_by_hash(): "cmd": ["redis-server"], "entrypoint": [], "env": [], - "ports": ["6379/tcp"], + "ports": [{"port": "6379", "protocol": "tcp"}], "volumes": [], } client = _make_inspect_client(inspect_payload=redis_inspect) @@ -486,7 +499,39 @@ async def test_inspect_image_by_hash(): register_images(mcp, make_config(), client) result = await tools["inspect_image"](image_id="sha256:cccc") + # The header echoes the user-supplied image_id; "redis" surfaces via the cmd line. + assert "sha256:cccc" in result assert "redis" in result + assert "6379/tcp" in result + + +@pytest.mark.asyncio +async def test_inspect_image_header_uses_image_id_when_fields_empty(): + """DSM Image/get returns image='' and tag='' for name:tag lookups — the header + must fall back to the user-supplied image_id, NOT render '?:?'.""" + from mcp_synology_container.modules.images import register_images + + client = _make_inspect_client() # SAMPLE_INSPECT has image='' / tag='' + mcp, tools = make_mock_mcp() + register_images(mcp, make_config(), client) + + result = await tools["inspect_image"](image_id="gitea/gitea:1.26.1") + assert "Image: gitea/gitea:1.26.1" in result + assert "?:?" not in result + + +@pytest.mark.asyncio +async def test_inspect_image_header_falls_back_to_id_when_image_id_empty(): + """If image_id were empty (defensive — should not happen in practice), the + header falls back to the sha256 id field.""" + from mcp_synology_container.modules.images import register_images + + client = _make_inspect_client() # SAMPLE_INSPECT.id == sha256:aaaa1234567890abcdef + mcp, tools = make_mock_mcp() + register_images(mcp, make_config(), client) + + result = await tools["inspect_image"](image_id="") + assert "Image: sha256:aaaa1234567890abcdef" in result @pytest.mark.asyncio @@ -605,8 +650,9 @@ async def test_inspect_image_registry_prefixed(): from mcp_synology_container.modules.images import register_images registry_inspect = { - "image": "ghcr.io/foo/bar", - "tag": "v1", + # image / tag come back empty for name:tag lookups (live DSM behavior). + "image": "", + "tag": "", "id": "sha256:dddd", "digest": "sha256:gdigest", "size": 100 * 1024 * 1024, diff --git a/uv.lock b/uv.lock index 719fcbc..0489d4b 100644 --- a/uv.lock +++ b/uv.lock @@ -362,7 +362,7 @@ wheels = [ [[package]] name = "mcp-synology-container" -version = "0.4.2" +version = "0.4.3" source = { editable = "." } dependencies = [ { name = "click" },