Fix get_container_status: clean name + dual-format response handling
- get_container_status now strips hash prefix from user input and calls
SYNO.Docker.Container/get with the clean name (e.g. 'jenkins'), not the
hash-prefixed form — the get endpoint accepts only the clean name
- _format_container_detail: unwraps 'container' wrapper key if present
(DSM may return {"container": {State, Config, ...}} at the data level)
- Flat-format fallback: reads lowercase 'status'/'image' fields when
Docker Engine nested format (State/Config) is absent
- Diagnostic stderr logging for data_keys, unwrap, status, image
- 25 container tests all passing
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -4,6 +4,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import logging
|
import logging
|
||||||
import re
|
import re
|
||||||
|
import sys
|
||||||
from typing import TYPE_CHECKING, Any
|
from typing import TYPE_CHECKING, Any
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
@@ -113,21 +114,28 @@ def register_containers(mcp: FastMCP, config: AppConfig, client: DsmClient) -> N
|
|||||||
Args:
|
Args:
|
||||||
container_name: Name of the container to inspect.
|
container_name: Name of the container to inspect.
|
||||||
"""
|
"""
|
||||||
resolved_name = await _resolve_container_name(client, container_name)
|
# Strip hash prefix from user input; SYNO.Docker.Container/get accepts
|
||||||
|
# the clean name directly — do NOT resolve to hash-prefixed name, as
|
||||||
|
# the endpoint may not accept that form.
|
||||||
|
clean_name = _strip_hash_prefix(container_name)
|
||||||
try:
|
try:
|
||||||
data = await client.request(
|
data = await client.request(
|
||||||
"SYNO.Docker.Container",
|
"SYNO.Docker.Container",
|
||||||
"get",
|
"get",
|
||||||
params={"name": resolved_name},
|
params={"name": clean_name},
|
||||||
)
|
)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return f"Error getting container '{container_name}': {e}"
|
return f"Error getting container '{container_name}': {e}"
|
||||||
|
|
||||||
|
sys.stderr.write(
|
||||||
|
f"[get_container_status] name={clean_name!r} data_keys={list(data.keys())}\n"
|
||||||
|
)
|
||||||
|
sys.stderr.flush()
|
||||||
|
|
||||||
if not data:
|
if not data:
|
||||||
return f"Container '{container_name}' not found."
|
return f"Container '{container_name}' not found."
|
||||||
|
|
||||||
display_name = _strip_hash_prefix(resolved_name)
|
return _format_container_detail(clean_name, data)
|
||||||
return _format_container_detail(display_name, data)
|
|
||||||
|
|
||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
async def get_container_logs(
|
async def get_container_logs(
|
||||||
@@ -322,22 +330,47 @@ def _container_in_project(container: dict[str, Any], project_name: str) -> bool:
|
|||||||
|
|
||||||
|
|
||||||
def _format_container_detail(name: str, data: dict[str, Any]) -> str:
|
def _format_container_detail(name: str, data: dict[str, Any]) -> str:
|
||||||
"""Format container inspect data as human-readable text."""
|
"""Format container inspect data as human-readable text.
|
||||||
state = data.get("State", {}) or {}
|
|
||||||
config = data.get("Config", {}) or {}
|
DSM may return the container object under a "container" key, or directly
|
||||||
host_config = data.get("HostConfig", {}) or {}
|
at the top level. Supports both the Docker Engine inspect format
|
||||||
|
(nested State/Config/HostConfig) and the DSM flat format (lowercase
|
||||||
|
status/image fields as returned by the list endpoint).
|
||||||
|
"""
|
||||||
|
# Unwrap "container" wrapper if present (common DSM API pattern)
|
||||||
|
if "container" in data and isinstance(data["container"], dict):
|
||||||
|
sys.stderr.write("[get_container_status] unwrapping 'container' key\n")
|
||||||
|
sys.stderr.flush()
|
||||||
|
raw = data["container"]
|
||||||
|
else:
|
||||||
|
raw = data
|
||||||
|
|
||||||
|
# Docker Engine inspect format: State / Config / HostConfig
|
||||||
|
state = raw.get("State", {}) or {}
|
||||||
|
config = raw.get("Config", {}) or {}
|
||||||
|
host_config = raw.get("HostConfig", {}) or {}
|
||||||
|
|
||||||
|
# DSM flat format fallback (lowercase fields, same as list response)
|
||||||
|
status_str = state.get("Status") or raw.get("status") or "?"
|
||||||
|
running = state.get("Running") if "Running" in state else (status_str == "running")
|
||||||
|
image_str = config.get("Image") or raw.get("image") or "?"
|
||||||
|
|
||||||
|
sys.stderr.write(
|
||||||
|
f"[get_container_status] status={status_str!r} running={running!r} image={image_str!r}\n"
|
||||||
|
)
|
||||||
|
sys.stderr.flush()
|
||||||
|
|
||||||
lines = [
|
lines = [
|
||||||
f"Container: {name}",
|
f"Container: {name}",
|
||||||
f" Status: {state.get('Status', '?')}",
|
f" Status: {status_str}",
|
||||||
f" Running: {state.get('Running', False)}",
|
f" Running: {running}",
|
||||||
f" Image: {config.get('Image', '?')}",
|
f" Image: {image_str}",
|
||||||
]
|
]
|
||||||
|
|
||||||
if state.get("StartedAt"):
|
if state.get("StartedAt"):
|
||||||
lines.append(f" Started: {state.get('StartedAt')}")
|
lines.append(f" Started: {state['StartedAt']}")
|
||||||
if state.get("FinishedAt") and not state.get("Running"):
|
if state.get("FinishedAt") and not running:
|
||||||
lines.append(f" Finished: {state.get('FinishedAt')}")
|
lines.append(f" Finished: {state['FinishedAt']}")
|
||||||
lines.append(f" Exit code: {state.get('ExitCode', '?')}")
|
lines.append(f" Exit code: {state.get('ExitCode', '?')}")
|
||||||
|
|
||||||
memory = host_config.get("Memory", 0)
|
memory = host_config.get("Memory", 0)
|
||||||
|
|||||||
@@ -408,18 +408,16 @@ async def test_container_stats_strips_hash_prefix():
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_get_container_status_resolves_hash_prefix():
|
async def test_get_container_status_uses_clean_name():
|
||||||
"""get_container_status resolves 'jenkins' to 'f93cb8b504f7_jenkins' for DSM call."""
|
"""get_container_status strips hash prefix and calls get with clean name."""
|
||||||
from mcp_synology_container.modules.containers import register_containers
|
from mcp_synology_container.modules.containers import register_containers
|
||||||
|
|
||||||
detail_data = {"State": {"Status": "running", "Running": True}, "Config": {"Image": "jenkins/jenkins:lts"}}
|
detail_data = {"State": {"Status": "running", "Running": True}, "Config": {"Image": "jenkins/jenkins:lts"}}
|
||||||
|
|
||||||
async def mock_request(api, method, **kwargs):
|
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":
|
if api == "SYNO.Docker.Container" and method == "get":
|
||||||
# Must be called with the hash-prefixed name
|
# Must be called with the CLEAN name (no hash prefix)
|
||||||
assert kwargs["params"]["name"] == "f93cb8b504f7_jenkins"
|
assert kwargs["params"]["name"] == "jenkins"
|
||||||
return detail_data
|
return detail_data
|
||||||
return {}
|
return {}
|
||||||
|
|
||||||
@@ -429,12 +427,58 @@ async def test_get_container_status_resolves_hash_prefix():
|
|||||||
mcp, tools = make_mock_mcp()
|
mcp, tools = make_mock_mcp()
|
||||||
register_containers(mcp, make_config(), client)
|
register_containers(mcp, make_config(), client)
|
||||||
|
|
||||||
result = await tools["get_container_status"]("jenkins")
|
# User passes hash-prefixed name → should be stripped before get call
|
||||||
assert "Error" not in result
|
result = await tools["get_container_status"]("f93cb8b504f7_jenkins")
|
||||||
# Display name should be clean (no hash prefix)
|
assert "running" in result
|
||||||
assert "f93cb8b504f7" not in result
|
assert "f93cb8b504f7" not in result
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_get_container_status_container_key_unwrap():
|
||||||
|
"""get_container_status unwraps DSM 'container' wrapper key."""
|
||||||
|
from mcp_synology_container.modules.containers import register_containers
|
||||||
|
|
||||||
|
# DSM may wrap the inspect data under a "container" key
|
||||||
|
wrapped_data = {
|
||||||
|
"container": {
|
||||||
|
"State": {"Status": "running", "Running": True},
|
||||||
|
"Config": {"Image": "jenkins/jenkins:lts"},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
client = AsyncMock()
|
||||||
|
client.request.return_value = wrapped_data
|
||||||
|
|
||||||
|
mcp, tools = make_mock_mcp()
|
||||||
|
register_containers(mcp, make_config(), client)
|
||||||
|
|
||||||
|
result = await tools["get_container_status"]("jenkins")
|
||||||
|
assert "running" in result
|
||||||
|
assert "jenkins/jenkins:lts" in result
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_get_container_status_flat_format():
|
||||||
|
"""get_container_status handles DSM flat format (lowercase status/image)."""
|
||||||
|
from mcp_synology_container.modules.containers import register_containers
|
||||||
|
|
||||||
|
flat_data = {
|
||||||
|
"status": "running",
|
||||||
|
"image": "jenkins/jenkins:lts",
|
||||||
|
"name": "jenkins",
|
||||||
|
}
|
||||||
|
|
||||||
|
client = AsyncMock()
|
||||||
|
client.request.return_value = flat_data
|
||||||
|
|
||||||
|
mcp, tools = make_mock_mcp()
|
||||||
|
register_containers(mcp, make_config(), client)
|
||||||
|
|
||||||
|
result = await tools["get_container_status"]("jenkins")
|
||||||
|
assert "running" in result
|
||||||
|
assert "jenkins/jenkins:lts" in result
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_get_container_logs_resolves_hash_prefix():
|
async def test_get_container_logs_resolves_hash_prefix():
|
||||||
"""get_container_logs resolves 'jenkins' to 'f93cb8b504f7_jenkins' for DSM call."""
|
"""get_container_logs resolves 'jenkins' to 'f93cb8b504f7_jenkins' for DSM call."""
|
||||||
|
|||||||
Reference in New Issue
Block a user