diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c2baaa..df6d0f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,38 @@ All notable changes to this project will be documented in this file. +## [0.8.0] - 2026-06-16 + +### Changed (fixes intermittent "MCP not responding" timeouts) + +The long-running tools no longer block the tool call until the operation +finishes. They now **fire-and-return**: trigger the work, surface any fast +failure, then return immediately so Claude can poll status with the existing +fast tools. This removes the root cause of the intermittent timeouts — +analysis of the live MCP log showed `redeploy_project` calls running up to +**260 s**, which crosses Claude Desktop's ~4 min tool-call ceiling. Whether a +call crossed it depended on image size and NAS load, which is exactly why the +failure was intermittent ("works fine for a while, then suddenly times out"). + +- **`redeploy_project` / `create_project`** — `build_stream` is now consumed + only for a short early-error window (`_BUILD_EARLY_BUDGET`, 20 s) to catch + fast daemon errors (e.g. `manifest unknown`). If the build is still running + after that window (large image), the tool returns a "redeploy/create + started — check `get_project_status`" hint instead of polling for up to + 300 s. The pull and container start continue on the NAS. +- **`pull_image`** — calls `pull_start` and returns immediately with a + "pull started in the background — verify with `check_image_updates` / + `list_images`" hint, instead of polling `Image/list` for up to 240 s. +- **`DsmClient.trigger_build_stream`** — gained a `budget` parameter so + callers can cap how long the streamed log is consumed. + +### Removed + +- `_wait_for_project_running` and the `_POLL_INTERVAL` / `_POLL_TIMEOUT` / + `_BUILD_POLL_TIMEOUT` constants in `projects.py`, and the + `_PULL_POLL_TIMEOUT` / `_PULL_POLL_INTERVALS` constants in `registry.py` — + the minutes-long polling loops they drove are gone. + ## [0.7.0] - 2026-05-18 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index e54f8a1..79ad06e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,22 +54,30 @@ Only a second consecutive failure is treated as a real auth problem. `container_name`. All container tools strip this prefix transparently via `_strip_hash_prefix` / `_resolve_container_name`. - **Async project start** — `SYNO.Docker.Project/start` returns immediately - while containers are still initialising. `redeploy_project` polls - `SYNO.Docker.Project/list` every 2 s for up to 30 s after issuing start. + while containers are still initialising. The long-running tools + (`redeploy_project`, `create_project`, `pull_image`) are **fire-and-return**: + they trigger the operation, surface any fast failure, then return so the + tool call never approaches the Claude Desktop ~4 min ceiling. Completion is + observed by the model polling `get_project_status` / `check_image_updates`, + not by the tool blocking. (Blocking until RUNNING was the root cause of the + intermittent "MCP not responding" timeouts — large-image pulls regularly + ran 4+ min; see CHANGELOG 0.8.0.) - **`SYNO.Docker.Project/build_stream`** — returns a streamed plaintext build log (content-type `text/html`), one short line per step: `Container Running` on success, ` Error` followed by `Error response from daemon: ` on failure. The stream closes - when the build is done. `DsmClient.trigger_build_stream` consumes the - body line-by-line with a 210 s wall-clock budget (under the Claude - Desktop ~4 min ceiling) and returns the log as a string; on timeout - the partial log is returned with a marker appended so callers know - the build is still running server-side. `redeploy_project` and - `create_project` grep the returned log for daemon errors and abort - early — these errors are much more actionable than the eventual - `BUILD_FAILED` polling status. The log is **live-only**: it cannot - be re-fetched after the build ends, which is why no standalone - `get_project_build_log` tool exists. + when the build is done. `DsmClient.trigger_build_stream(project_id, + budget=…)` consumes the body line-by-line up to a configurable wall-clock + budget and returns the log so far; if the budget elapses before the stream + closes, the partial log is returned with `BUILD_STREAM_TIMEOUT_MARKER` + appended so callers know the build is still running server-side. + `redeploy_project` / `create_project` pass a **short** budget + (`_BUILD_EARLY_BUDGET`, 20 s) purely to catch fast daemon errors + (e.g. `manifest unknown`) — they grep the returned log and abort early on + those, otherwise return a "build running in the background, check + `get_project_status`" hint. They do **not** block until RUNNING. The log is + **live-only**: it cannot be re-fetched after the build ends, which is why no + standalone `get_project_build_log` tool exists. - **Image delete** — requires a form-encoded POST with a JSON `images` array (confirmed via browser DevTools); uses `DsmClient.post_request()`. - **`SYNO.Docker.Image/pull` vs. `pull_start`** — the legacy `pull` method @@ -80,11 +88,13 @@ Only a second consecutive failure is treated as a real auth problem. `SYNO.Docker.Registry` — the Registry API only exposes the synchronous read-only methods (`search`, `tags`, `get/set/create/delete`, `using`); calling `Registry/pull_start` returns "Method does not exist". No - matching `pull_status` method is confirmed on either API, so completion - is detected by polling `SYNO.Docker.Image/list` until `repository:tag` - appears (2–10 s backoff, 240 s budget). Timeout returns a "still - running" hint instead of raising — DSM keeps pulling server-side - regardless of the HTTP response. + matching `pull_status` method is confirmed on either API. `pull_image` is + **fire-and-return**: it short-circuits if the tag is already local, calls + `pull_start`, then returns a "pull started in the background, verify with + `check_image_updates` / `list_images`" hint. DSM keeps pulling server-side + regardless of the HTTP response, so the tool does not block polling + `Image/list` for the tag to appear (that loop used to run the full 240 s on + large images and trip the Claude Desktop tool-call timeout). - **`SYNO.Docker.Registry/tags`** — uses `repo` (JSON-encoded) as the parameter name; the n4s4 reference's `name` does not work on this DSM version. Returns the tag list as the envelope's `data` field directly, diff --git a/pyproject.toml b/pyproject.toml index 2ca7661..2dc7f81 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "mcp-synology-container" -version = "0.7.0" +version = "0.8.0" description = "MCP server for Synology Container Manager" requires-python = ">=3.12" dependencies = [ diff --git a/src/mcp_synology_container/dsm_client.py b/src/mcp_synology_container/dsm_client.py index 0a5903d..4f3180a 100644 --- a/src/mcp_synology_container/dsm_client.py +++ b/src/mcp_synology_container/dsm_client.py @@ -431,8 +431,8 @@ class DsmClient: # whether the log they got is complete. BUILD_STREAM_TIMEOUT_MARKER = "[build_stream: timeout — stream still open server-side]" - async def trigger_build_stream(self, project_id: str) -> str: - """Trigger SYNO.Docker.Project/build_stream and return the full log text. + async def trigger_build_stream(self, project_id: str, *, budget: float | None = None) -> str: + """Trigger SYNO.Docker.Project/build_stream and return the log text so far. This is the proper way to force an image pull and project restart in DSM Container Manager (confirmed via browser DevTools). The endpoint @@ -457,14 +457,22 @@ class DsmClient: Args: project_id: Project UUID from SYNO.Docker.Project/list. + budget: Wall-clock seconds to keep consuming the streamed log + before giving up and returning the partial log + timeout + marker. Defaults to :attr:`BUILD_STREAM_BUDGET`. Callers + that must stay well under the MCP client's tool-call timeout + (``redeploy_project`` / ``create_project``) pass a short value + so the tool returns quickly while DSM keeps building + server-side; completion is then observed via + ``get_project_status`` rather than by blocking here. Returns: The collected build log as a single string (may be empty if DSM returned a non-JSON empty body). Appended with :attr:`BUILD_STREAM_TIMEOUT_MARKER` when the wall-clock budget ran out before DSM closed the stream — in that case the build is - still running server-side and the caller's polling step is the - authoritative status source. + still running server-side and the caller should poll project + status for the authoritative result. Raises: SynologyError: If DSM rejects the build with a JSON error body, or @@ -496,7 +504,7 @@ class DsmClient: # generous per-read timeout because DSM may pause between status lines # during a slow image pull. loop = asyncio.get_event_loop() - deadline = loop.time() + self.BUILD_STREAM_BUDGET + deadline = loop.time() + (self.BUILD_STREAM_BUDGET if budget is None else budget) try: async with http.stream( diff --git a/src/mcp_synology_container/modules/projects.py b/src/mcp_synology_container/modules/projects.py index 68de9ac..83eb601 100644 --- a/src/mcp_synology_container/modules/projects.py +++ b/src/mcp_synology_container/modules/projects.py @@ -2,7 +2,6 @@ from __future__ import annotations -import asyncio import contextlib import json import logging @@ -10,22 +9,26 @@ from typing import TYPE_CHECKING, Any import yaml +from mcp_synology_container.dsm_client import DsmClient + if TYPE_CHECKING: from mcp.server.fastmcp import FastMCP from mcp_synology_container.config import AppConfig - from mcp_synology_container.dsm_client import DsmClient logger = logging.getLogger(__name__) -_POLL_INTERVAL = 2 # seconds between status checks -_POLL_TIMEOUT = 30 # seconds for ordinary start polling -_BUILD_POLL_TIMEOUT = 300 # seconds for build_stream polling (image pull can be slow) +# Short wall-clock window (seconds) for the build_stream early-error read in +# redeploy_project / create_project. We consume the stream only long enough +# to catch fast daemon errors (e.g. "manifest unknown"), then return and let +# DSM finish the pull/start in the background. Blocking until RUNNING used to +# run for minutes and intermittently exceed Claude Desktop's ~4 min tool-call +# ceiling on large images — the root cause of the "MCP not responding" +# timeouts. Completion is observed via get_project_status instead. +_BUILD_EARLY_BUDGET = 20.0 -# Statuses that mean "stop polling now — this redeploy is not coming back." -# DSM signals these typically within seconds of build_stream when the image -# pull or container start fails; without an early exit the caller would wait -# the full _BUILD_POLL_TIMEOUT for nothing. +# Statuses that mean "this redeploy is not coming back." DSM reports these on +# the post-build status read when the image pull or container start fails. _TERMINAL_FAILURE_STATUSES = frozenset({"BUILD_FAILED", "ERROR"}) # Substrings that mark a line in the build_stream log as a failure. Live @@ -189,15 +192,18 @@ def register_projects(mcp: FastMCP, config: AppConfig, client: DsmClient) -> Non results.append(" Stopped.") # ── Step 2: build_stream (pull images + start) ──────────────────── - results.append("Step 2/3: Triggering image pull and project start (build_stream)...") - build_log = await client.trigger_build_stream(project_id) + # Consume only a short early-error window, then return. The image + # pull and container start continue on the NAS; blocking the tool + # call until RUNNING used to run for minutes and intermittently + # exceed Claude Desktop's ~4 min tool-call ceiling on large images. + # Claude polls get_project_status for completion instead. + results.append("Step 2/2: Triggering image pull and project start (build_stream)...") + build_log = await client.trigger_build_stream(project_id, budget=_BUILD_EARLY_BUDGET) build_errors, _ = _parse_build_stream_log(build_log) if build_errors: # Live build_stream reported a daemon error (e.g. manifest # not found, container exited). Surface the cause now — - # no need to wait for polling to flip the project to - # BUILD_FAILED, and the daemon line is much more - # actionable than the bare status. + # the daemon line is much more actionable than a bare status. results.append(" Build failed — DSM reported:") results.extend(f" {line}" for line in build_errors) results.append( @@ -210,23 +216,30 @@ def register_projects(mcp: FastMCP, config: AppConfig, client: DsmClient) -> Non f"redeploy_project to recover." ) return "\n".join(results) - results.append(" Build request accepted by DSM.") - # ── Step 3: Poll ────────────────────────────────────────────────── - results.append( - f"Step 3/3: Waiting for project to reach RUNNING state " - f"(up to {_BUILD_POLL_TIMEOUT}s)..." - ) - final_status = await _wait_for_project_running( - client, project_name, timeout=_BUILD_POLL_TIMEOUT - ) + if DsmClient.BUILD_STREAM_TIMEOUT_MARKER in build_log: + # Early window elapsed with no error — a large image is still + # pulling/starting in the background. Return now; do NOT block. + results.append(" Build accepted — image pull and start running in the background.") + results.append( + f"\nProject '{project_name}' redeploy started. The image pull and " + f"container start continue on the NAS (this can take several minutes " + f"for large images).\n" + f"Check progress with get_project_status('{project_name}') — " + f"RUNNING means done, BUILD_FAILED means the image tag is wrong." + ) + return "\n".join(results) + + # Stream closed within the early window → the build already + # finished server-side. Report the outcome from a single status + # read (no minutes-long polling loop). + results.append(" Build request accepted by DSM.") + final = await _find_project(client, project_name) + final_status = (final.get("status") or "UNKNOWN").upper() if final else "UNKNOWN" if final_status == "RUNNING": results.append(" Project is RUNNING.") results.append(f"\nProject '{project_name}' redeployed successfully.") elif final_status in _TERMINAL_FAILURE_STATUSES: - # M-5: DSM signalled a hard failure during polling. The - # build_stream log above was clean, so this is a late - # failure (e.g. container exited after start). results.append(f" Redeploy failed — project status is '{final_status}'.") if final_status == "BUILD_FAILED": results.append( @@ -237,12 +250,11 @@ def register_projects(mcp: FastMCP, config: AppConfig, client: DsmClient) -> Non f"\nProject '{project_name}' redeploy aborted (status: {final_status})." ) else: + results.append(f" Build finished — current status: {final_status}.") results.append( - f" Warning: project status is '{final_status}' after " - f"{_BUILD_POLL_TIMEOUT}s. " - f"Containers may still be starting — check with get_project_status." + f"\nProject '{project_name}' redeploy started (status: {final_status}). " + f"Check get_project_status('{project_name}')." ) - results.append(f"\nProject '{project_name}' build issued (status: {final_status}).") except Exception as e: results.append(f"Error during redeploy: {e}") @@ -404,14 +416,23 @@ def register_projects(mcp: FastMCP, config: AppConfig, client: DsmClient) -> Non f"redeploy_project('{project_name}', confirmed=True) to retry." ) return "\n".join(results) - results.append(" Build request accepted by DSM.") - results.append( - f"Waiting for project to reach RUNNING state (up to {_BUILD_POLL_TIMEOUT}s)..." - ) - final_status = await _wait_for_project_running( - client, project_name, timeout=_BUILD_POLL_TIMEOUT - ) + if DsmClient.BUILD_STREAM_TIMEOUT_MARKER in build_log: + # Large image still pulling/starting in the background — return now. + results.append(" Build accepted — image pull and start running in the background.") + results.append( + f"\nProject '{project_name}' created. The image pull and container " + f"start continue on the NAS (this can take several minutes for large " + f"images).\n" + f"Check progress with get_project_status('{project_name}') — " + f"RUNNING means done, BUILD_FAILED means an image tag is wrong." + ) + return "\n".join(results) + + # Stream closed within the early window → the build already finished. + results.append(" Build request accepted by DSM.") + final = await _find_project(client, project_name) + final_status = (final.get("status") or "UNKNOWN").upper() if final else "UNKNOWN" if final_status == "RUNNING": results.append(" Project is RUNNING.") results.append(f"\nProject '{project_name}' created and started successfully.") @@ -427,12 +448,11 @@ def register_projects(mcp: FastMCP, config: AppConfig, client: DsmClient) -> Non f"(status: {final_status})." ) else: + results.append(f" Build finished — current status: {final_status}.") results.append( - f" Warning: project status is '{final_status}' after " - f"{_BUILD_POLL_TIMEOUT}s. " - f"Containers may still be starting — check with get_project_status." + f"\nProject '{project_name}' created (status: {final_status}). " + f"Check get_project_status('{project_name}')." ) - results.append(f"\nProject '{project_name}' created (final status: {final_status}).") return "\n".join(results) @@ -515,45 +535,6 @@ async def _find_project(client: DsmClient, name: str) -> dict[str, Any] | None: return None -async def _wait_for_project_running( - client: DsmClient, - name: str, - timeout: int = _POLL_TIMEOUT, - interval: int = _POLL_INTERVAL, -) -> str: - """Poll until the project reaches RUNNING status or timeout expires. - - Args: - client: DsmClient instance. - name: Project name to watch. - timeout: Maximum seconds to wait (default 30). - interval: Seconds between polls (default 2). - - Returns: - Final project status string (may not be "RUNNING" on timeout). - """ - elapsed = 0 - while elapsed < timeout: - await asyncio.sleep(interval) - elapsed += interval - project = await _find_project(client, name) - if project is None: - continue - current = (project.get("status") or "").upper() - logger.debug("Polling '%s': status=%s elapsed=%ds", name, current, elapsed) - if current == "RUNNING": - return current - if current in _TERMINAL_FAILURE_STATUSES: - # DSM has reported a hard failure (e.g. image pull failed, - # container exited immediately). Returning early lets the - # caller surface the real cause instead of waiting out the - # full timeout. - return current - # Return whatever status we last saw (or UNKNOWN on repeated failures) - project = await _find_project(client, name) - return (project.get("status") or "UNKNOWN").upper() if project else "UNKNOWN" - - def _format_project_detail(project: dict[str, Any]) -> str: """Format project details as human-readable text.""" lines = [ diff --git a/src/mcp_synology_container/modules/registry.py b/src/mcp_synology_container/modules/registry.py index 1704045..0c70292 100644 --- a/src/mcp_synology_container/modules/registry.py +++ b/src/mcp_synology_container/modules/registry.py @@ -2,7 +2,6 @@ from __future__ import annotations -import asyncio import json import logging from typing import TYPE_CHECKING, Any @@ -15,11 +14,6 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) -# Pull polling: backoff schedule capped at 10 s between checks. Total budget -# stays under the Claude Desktop ~4 min tool-call ceiling. -_PULL_POLL_TIMEOUT = 240.0 -_PULL_POLL_INTERVALS: tuple[float, ...] = (2.0, 3.0, 5.0, 8.0, 10.0) - def _truncate(text: str, limit: int = 80) -> str: """Truncate text to limit with an ellipsis.""" @@ -155,7 +149,7 @@ def register_registry(mcp: FastMCP, config: AppConfig, client: DsmClient) -> Non @mcp.tool() async def pull_image(repository: str, tag: str = "latest", confirmed: bool = False): - """Pull image from the active registry. Requires confirmed=True; polls for completion.""" + """Pull image from the active registry in the background. Requires confirmed=True.""" target = f"{repository}:{tag}" if not confirmed: @@ -186,21 +180,15 @@ def register_registry(mcp: FastMCP, config: AppConfig, client: DsmClient) -> Non except Exception as e: return f"Error starting pull for '{target}': {e}" - # Async pull — DSM exposes no confirmed pull_status method, so we poll - # Image/list for the new tag. Backoff schedule capped at 10 s; total - # budget under the Claude Desktop ~4 min tool-call ceiling. - loop = asyncio.get_event_loop() - deadline = loop.time() + _PULL_POLL_TIMEOUT - attempt = 0 - while loop.time() < deadline: - interval = _PULL_POLL_INTERVALS[min(attempt, len(_PULL_POLL_INTERVALS) - 1)] - remaining = deadline - loop.time() - await asyncio.sleep(min(interval, max(remaining, 0.0))) - if await _image_present(client, repository, tag): - return f"Pulled {target} successfully." - attempt += 1 - + # Fire-and-return: the pull runs asynchronously on the NAS. We do NOT + # poll Image/list until the tag appears — for large images that took + # minutes and intermittently exceeded Claude Desktop's ~4 min tool-call + # ceiling, making the whole MCP appear unresponsive. The pull continues + # server-side; completion is observed via check_image_updates / + # list_images, which show the new tag once it finishes. return ( - f"Pull of {target} started, still running — " - f"verify later with list_images or check_image_updates." + f"Pull of {target} started in the background on the NAS.\n" + f"This can take a while for large images. Check completion with " + f"check_image_updates or list_images — the tag appears once the " + f"pull finishes." ) diff --git a/tests/test_modules/test_projects.py b/tests/test_modules/test_projects.py index 5abb23b..016522c 100644 --- a/tests/test_modules/test_projects.py +++ b/tests/test_modules/test_projects.py @@ -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 diff --git a/tests/test_modules/test_registry.py b/tests/test_modules/test_registry.py index c24201f..711eea3 100644 --- a/tests/test_modules/test_registry.py +++ b/tests/test_modules/test_registry.py @@ -285,30 +285,15 @@ async def test_pull_image_already_present(): @pytest.mark.asyncio -async def test_pull_image_confirmed_success(monkeypatch): - """pull_start succeeds, then Image/list shows the new tag on first poll.""" - import mcp_synology_container.modules.registry as registry_mod +async def test_pull_image_confirmed_starts_background(): + """pull_start is invoked, then the tool returns immediately with a background hint — + it does NOT poll Image/list until the tag appears (fire-and-return).""" from mcp_synology_container.modules.registry import register_registry - # Make polling instant - async def fake_sleep(_): - return None - - monkeypatch.setattr(registry_mod.asyncio, "sleep", fake_sleep) - - state = {"pulled": False} - async def mock_request(api, method, **kwargs): if api == "SYNO.Docker.Image" and method == "list": - if state["pulled"]: - return { - "images": [ - {"id": "sha256:aaaa", "repository": "nginx", "tags": ["1.24"]}, - ] - } - return {"images": []} + return {"images": []} # not present locally → proceed to pull if api == "SYNO.Docker.Image" and method == "pull_start": - state["pulled"] = True return {} raise AssertionError(f"Unexpected call: {api}/{method}") @@ -319,11 +304,12 @@ async def test_pull_image_confirmed_success(monkeypatch): register_registry(mcp, make_config(), client) result = await tools["pull_image"](repository="nginx", tag="1.24", confirmed=True) - assert "Pulled" in result + assert "started in the background" in result assert "nginx:1.24" in result + assert "check_image_updates" in result # Verify pull_start was invoked on SYNO.Docker.Image (not Registry) - # with JSON-encoded params. + # with JSON-encoded params, exactly once. pull_calls = [c for c in client.request.call_args_list if c.args[1] == "pull_start"] assert len(pull_calls) == 1 assert pull_calls[0].args[0] == "SYNO.Docker.Image" @@ -332,37 +318,9 @@ async def test_pull_image_confirmed_success(monkeypatch): assert json.loads(params["tag"]) == "1.24" assert pull_calls[0].kwargs.get("version") == 1 - -@pytest.mark.asyncio -async def test_pull_image_timeout(monkeypatch): - """If the image never appears, the tool returns a still-running hint.""" - import mcp_synology_container.modules.registry as registry_mod - from mcp_synology_container.modules.registry import register_registry - - # Make polling instant - async def fake_sleep(_): - return None - - monkeypatch.setattr(registry_mod.asyncio, "sleep", fake_sleep) - # Shrink the budget so the loop exits quickly - monkeypatch.setattr(registry_mod, "_PULL_POLL_TIMEOUT", 0.05) - - async def mock_request(api, method, **kwargs): - if api == "SYNO.Docker.Image" and method == "list": - return {"images": []} - if api == "SYNO.Docker.Image" and method == "pull_start": - return {} - raise AssertionError(f"Unexpected call: {api}/{method}") - - client = AsyncMock() - client.request.side_effect = mock_request - - mcp, tools = make_mock_mcp() - register_registry(mcp, make_config(), client) - - result = await tools["pull_image"](repository="nginx", tag="1.24", confirmed=True) - assert "still running" in result - assert "nginx:1.24" in result + # Exactly one Image/list pre-check — no post-pull polling loop. + list_calls = [c for c in client.request.call_args_list if c.args[1] == "list"] + assert len(list_calls) == 1 @pytest.mark.asyncio diff --git a/uv.lock b/uv.lock index e57d10b..b36fd98 100644 --- a/uv.lock +++ b/uv.lock @@ -362,7 +362,7 @@ wheels = [ [[package]] name = "mcp-synology-container" -version = "0.7.0" +version = "0.8.0" source = { editable = "." } dependencies = [ { name = "click" },