Make long-running tools fire-and-return to fix intermittent timeouts
redeploy_project, create_project and pull_image used to block the tool call while polling DSM until the project reached RUNNING / the image appeared - up to 300s and 240s respectively. On large images this regularly ran past Claude Desktop's ~4 min tool-call ceiling (the live MCP log shows redeploy_project calls up to 260s), which the client reported as a timeout even though the server kept working. Whether a call crossed the ceiling depended on image size and NAS load, which is why the failure was intermittent. These tools now fire-and-return: they trigger the operation, consume build_stream only for a short early-error window (20s, to catch fast daemon errors like "manifest unknown"), then return a "running in the background - check get_project_status / check_image_updates" hint. Completion is observed via the existing fast status tools. - DsmClient.trigger_build_stream gains a `budget` parameter - remove _wait_for_project_running and the _POLL_*/_BUILD_POLL_TIMEOUT constants (projects.py) and _PULL_POLL_* constants (registry.py) - update tests, CLAUDE.md DSM quirks and CHANGELOG; bump 0.7.0 -> 0.8.0 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
"""Tests for modules/projects.py."""
|
||||
|
||||
from unittest.mock import AsyncMock, patch
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -215,8 +215,8 @@ def make_stateful_redeploy_mock(
|
||||
"""Create a stateful client mock for redeploy tests.
|
||||
|
||||
Returns (client, calls_list). After ``trigger_build_stream`` is called,
|
||||
subsequent ``list`` calls return RUNNING so the polling loop terminates
|
||||
immediately. asyncio.sleep is NOT patched here — patch it at call-site.
|
||||
subsequent ``list`` calls return RUNNING so the post-build status read sees
|
||||
a finished, running project.
|
||||
"""
|
||||
client = AsyncMock()
|
||||
calls = []
|
||||
@@ -230,12 +230,12 @@ def make_stateful_redeploy_mock(
|
||||
return project_list("RUNNING") if build_done else project_list(initial_status)
|
||||
return {}
|
||||
|
||||
async def mock_trigger_build_stream(project_id):
|
||||
async def mock_trigger_build_stream(project_id, **kwargs):
|
||||
nonlocal build_done
|
||||
calls.append(("SYNO.Docker.Project", "build_stream"))
|
||||
if build_stream_raises:
|
||||
raise build_stream_raises
|
||||
build_done = True # After build_stream, polling returns RUNNING
|
||||
build_done = True # After build_stream, the status read returns RUNNING
|
||||
return build_stream_log
|
||||
|
||||
client.request.side_effect = mock_request
|
||||
@@ -245,12 +245,11 @@ def make_stateful_redeploy_mock(
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redeploy_running_project():
|
||||
"""RUNNING project: stop → build_stream → poll until RUNNING."""
|
||||
"""RUNNING project: stop → build_stream → status read reports RUNNING."""
|
||||
client, calls = make_stateful_redeploy_mock("RUNNING")
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "redeployed successfully" in result
|
||||
methods = [m for _, m in calls]
|
||||
@@ -261,12 +260,11 @@ async def test_redeploy_running_project():
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redeploy_stopped_project_skips_stop():
|
||||
"""STOPPED project: skip stop, call build_stream directly; polls until RUNNING."""
|
||||
"""STOPPED project: skip stop, call build_stream directly; status read RUNNING."""
|
||||
client, calls = make_stateful_redeploy_mock("STOPPED")
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "redeployed successfully" in result
|
||||
methods = [m for _, m in calls]
|
||||
@@ -277,12 +275,11 @@ async def test_redeploy_stopped_project_skips_stop():
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redeploy_build_failed_project():
|
||||
"""BUILD_FAILED project: stop (suppressed) → build_stream → poll until RUNNING."""
|
||||
"""BUILD_FAILED project: stop (suppressed) → build_stream → status read RUNNING."""
|
||||
client, calls = make_stateful_redeploy_mock("BUILD_FAILED")
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "redeployed successfully" in result
|
||||
methods = [m for _, m in calls]
|
||||
@@ -302,8 +299,7 @@ async def test_redeploy_build_failed_stop_error_nonfatal():
|
||||
)
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "redeployed successfully" in result
|
||||
methods = [m for _, m in calls]
|
||||
@@ -321,8 +317,7 @@ async def test_redeploy_build_stream_error_aborts():
|
||||
)
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "redeployed successfully" not in result
|
||||
assert "build failed" in result or "Error during redeploy" in result
|
||||
@@ -333,37 +328,27 @@ async def test_redeploy_build_stream_error_aborts():
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redeploy_poll_timeout():
|
||||
"""If project never reaches RUNNING after build_stream, a warning is emitted."""
|
||||
client = AsyncMock()
|
||||
build_done = False
|
||||
async def test_redeploy_large_image_returns_background_hint():
|
||||
"""When build_stream returns the timeout marker (early window elapsed, large
|
||||
image still pulling), redeploy returns immediately with a 'check status' hint
|
||||
and does NOT block on a polling loop."""
|
||||
from mcp_synology_container.dsm_client import DsmClient
|
||||
|
||||
async def mock_request(api, method, **kwargs):
|
||||
if method == "list":
|
||||
# Before build: RUNNING (so initial status check is valid)
|
||||
# After build: STARTING (simulate stuck containers)
|
||||
return project_list("STARTING") if build_done else project_list("RUNNING")
|
||||
return {}
|
||||
|
||||
async def mock_build_stream(project_id):
|
||||
nonlocal build_done
|
||||
build_done = True
|
||||
return ""
|
||||
|
||||
client.request.side_effect = mock_request
|
||||
client.trigger_build_stream = AsyncMock(side_effect=mock_build_stream)
|
||||
client, calls = make_stateful_redeploy_mock(
|
||||
"RUNNING",
|
||||
build_stream_log=DsmClient.BUILD_STREAM_TIMEOUT_MARKER,
|
||||
)
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
# Use tiny timeout so the test is instant (interval=1, timeout=1 → 1 poll)
|
||||
with (
|
||||
patch("mcp_synology_container.modules.projects.asyncio.sleep"),
|
||||
patch("mcp_synology_container.modules.projects._BUILD_POLL_TIMEOUT", 1),
|
||||
patch("mcp_synology_container.modules.projects._POLL_INTERVAL", 1),
|
||||
):
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "Warning" in result
|
||||
assert "redeploy started" in result
|
||||
assert "get_project_status" in result
|
||||
assert "redeployed successfully" not in result
|
||||
# No post-build status read — the background branch returns immediately.
|
||||
# Only the initial _find_project lookup hit Project/list.
|
||||
methods = [m for _, m in calls]
|
||||
assert [m for m in methods if m == "list"] == ["list"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -405,8 +390,7 @@ async def test_redeploy_build_stream_transport_error_shows_stopped_recovery_hint
|
||||
)
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
# No raw stack trace — clean message
|
||||
assert "transport error" in result
|
||||
@@ -430,8 +414,7 @@ async def test_redeploy_build_stream_error_on_stopped_project_keeps_old_workarou
|
||||
)
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "build failed" in result or "Error during redeploy" in result
|
||||
# Stop was never issued; new recovery hint should not appear
|
||||
@@ -489,8 +472,7 @@ async def test_redeploy_surfaces_build_stream_daemon_error():
|
||||
)
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "Build failed" in result
|
||||
assert "Error response from daemon: manifest for nginx:9.9.9-nonexistent" in result
|
||||
@@ -507,16 +489,15 @@ async def test_redeploy_surfaces_build_stream_daemon_error():
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redeploy_clean_log_proceeds_to_polling():
|
||||
"""A clean build_stream log keeps the original happy-path behavior."""
|
||||
async def test_redeploy_clean_log_reports_success():
|
||||
"""A clean, closed build_stream log → status read RUNNING → success message."""
|
||||
client, calls = make_stateful_redeploy_mock(
|
||||
"RUNNING",
|
||||
build_stream_log="Container myapp Pulling\nContainer myapp Running\n",
|
||||
)
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "redeployed successfully" in result
|
||||
|
||||
@@ -533,8 +514,7 @@ async def test_create_project_surfaces_build_stream_daemon_error():
|
||||
)
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["create_project"]("newapp", SIMPLE_COMPOSE, confirmed=True)
|
||||
result = await tools["create_project"]("newapp", SIMPLE_COMPOSE, confirmed=True)
|
||||
|
||||
assert "Build failed" in result
|
||||
assert "Error response from daemon: manifest for nginx:0.0.0-bad" in result
|
||||
@@ -544,60 +524,14 @@ async def test_create_project_surfaces_build_stream_daemon_error():
|
||||
|
||||
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
# M-5: polling exits early on BUILD_FAILED / ERROR
|
||||
# Post-build status read surfaces BUILD_FAILED / ERROR
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_wait_for_project_running_returns_early_on_build_failed():
|
||||
"""_wait_for_project_running must exit as soon as DSM reports BUILD_FAILED,
|
||||
not wait the full timeout."""
|
||||
from mcp_synology_container.modules.projects import _wait_for_project_running
|
||||
|
||||
client = AsyncMock()
|
||||
|
||||
async def mock_request(api, method, **kwargs):
|
||||
if method == "list":
|
||||
return project_list("BUILD_FAILED")
|
||||
return {}
|
||||
|
||||
client.request.side_effect = mock_request
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
# 100s timeout, 2s interval — if the early-exit isn't there the test
|
||||
# would still terminate quickly because sleep is mocked, but the call
|
||||
# count assertion below catches a non-exiting loop.
|
||||
result = await _wait_for_project_running(client, "myapp", timeout=100, interval=2)
|
||||
|
||||
assert result == "BUILD_FAILED"
|
||||
# Only a few list() calls — exit was on the first poll iteration.
|
||||
list_calls = [c for c in client.request.call_args_list if c.args[1] == "list"]
|
||||
assert len(list_calls) <= 2
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_wait_for_project_running_returns_early_on_error():
|
||||
from mcp_synology_container.modules.projects import _wait_for_project_running
|
||||
|
||||
client = AsyncMock()
|
||||
|
||||
async def mock_request(api, method, **kwargs):
|
||||
if method == "list":
|
||||
return project_list("ERROR")
|
||||
return {}
|
||||
|
||||
client.request.side_effect = mock_request
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await _wait_for_project_running(client, "myapp", timeout=100, interval=2)
|
||||
|
||||
assert result == "ERROR"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redeploy_surfaces_build_failed_with_hint():
|
||||
"""When polling reports BUILD_FAILED, redeploy_project must include a clear
|
||||
hint to inspect the image tag and retry."""
|
||||
"""When the post-build status read reports BUILD_FAILED, redeploy_project must
|
||||
include a clear hint to inspect the image tag and retry."""
|
||||
client = AsyncMock()
|
||||
build_done = False
|
||||
|
||||
@@ -606,17 +540,16 @@ async def test_redeploy_surfaces_build_failed_with_hint():
|
||||
return project_list("BUILD_FAILED") if build_done else project_list("RUNNING")
|
||||
return {}
|
||||
|
||||
async def mock_build_stream(project_id):
|
||||
async def mock_build_stream(project_id, **kwargs):
|
||||
nonlocal build_done
|
||||
build_done = True
|
||||
return "" # Clean stream log — failure surfaces via polling.
|
||||
return "" # Clean stream log — failure surfaces via the status read.
|
||||
|
||||
client.request.side_effect = mock_request
|
||||
client.trigger_build_stream = AsyncMock(side_effect=mock_build_stream)
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "Redeploy failed" in result
|
||||
assert "BUILD_FAILED" in result
|
||||
@@ -694,7 +627,7 @@ def make_create_project_client(
|
||||
return {"id": project_id}
|
||||
return {}
|
||||
|
||||
async def mock_build_stream(pid):
|
||||
async def mock_build_stream(pid, **kwargs):
|
||||
calls.append(("SYNO.Docker.Project", "build_stream", {"id": pid}))
|
||||
if build_stream_raises:
|
||||
raise build_stream_raises
|
||||
@@ -794,8 +727,7 @@ async def test_create_project_happy_path():
|
||||
client, calls = make_create_project_client()
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["create_project"]("newapp", SIMPLE_COMPOSE, confirmed=True)
|
||||
result = await tools["create_project"]("newapp", SIMPLE_COMPOSE, confirmed=True)
|
||||
|
||||
assert "created and started successfully" in result
|
||||
|
||||
@@ -831,13 +763,12 @@ async def test_create_project_explicit_share_path():
|
||||
client, calls = make_create_project_client()
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["create_project"](
|
||||
"newapp",
|
||||
SIMPLE_COMPOSE,
|
||||
share_path="/projects/custom/newapp",
|
||||
confirmed=True,
|
||||
)
|
||||
result = await tools["create_project"](
|
||||
"newapp",
|
||||
SIMPLE_COMPOSE,
|
||||
share_path="/projects/custom/newapp",
|
||||
confirmed=True,
|
||||
)
|
||||
|
||||
assert "created and started successfully" in result
|
||||
cf_params = next(p for api, m, p in calls if api == "SYNO.FileStation.CreateFolder")
|
||||
@@ -859,8 +790,7 @@ async def test_create_project_error_2100_surfaces_hint():
|
||||
)
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["create_project"]("newapp", SIMPLE_COMPOSE, confirmed=True)
|
||||
result = await tools["create_project"]("newapp", SIMPLE_COMPOSE, confirmed=True)
|
||||
|
||||
assert "2100" in result
|
||||
assert "target folder" in result.lower()
|
||||
@@ -879,8 +809,7 @@ async def test_create_project_build_stream_failure_keeps_registration():
|
||||
)
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["create_project"]("newapp", SIMPLE_COMPOSE, confirmed=True)
|
||||
result = await tools["create_project"]("newapp", SIMPLE_COMPOSE, confirmed=True)
|
||||
|
||||
assert "registered but was not started" in result
|
||||
assert "redeploy_project" in result
|
||||
|
||||
Reference in New Issue
Block a user