From d298a89f5f3799100d09a6f78a4b19e7a6a25481 Mon Sep 17 00:00:00 2001 From: Ryan Malloy Date: Fri, 6 Feb 2026 00:48:26 -0700 Subject: [PATCH] refactor: Remove docker_wait tool entirely docker_wait was the same anti-pattern as wait param - it blocked a single tool call for up to 5 minutes with no visibility. LLMs should poll docker_health(port) in their own loop. This gives: - Visibility into progress between polls - Ability to check docker_logs while waiting - Control over timeout and retry logic - Opportunity to bail out early --- CHANGELOG.md | 3 +- src/ghydramcp/mixins/docker.py | 50 ++++------------------------------ 2 files changed, 8 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4490247..2baac1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,7 +59,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Changed - **Docker Port Allocation:** Ports are now auto-allocated from pool (8192-8223) instead of client-specified. Prevents session collisions in multi-agent environments. -- **docker_auto_start:** Removed `wait` and `timeout` parameters entirely. Always returns immediately after starting container. Use `docker_wait(port)` separately to poll for readiness. This prevents LLMs from blocking on a single tool call for minutes. +- **docker_auto_start:** Removed `wait` and `timeout` parameters. Always returns immediately after starting container. +- **Removed docker_wait tool:** This tool blocked for up to 5 minutes in a single call. LLMs should poll `docker_health(port)` in their own loop instead — this gives visibility into progress and ability to check logs between polls. ### Fixed - **instances_use Hanging:** Eliminated 4+ hour hangs by removing blocking HTTP call. Now uses lazy registration — just creates a stub entry, validates on first real tool call. diff --git a/src/ghydramcp/mixins/docker.py b/src/ghydramcp/mixins/docker.py index 15a2cf2..f81e31b 100644 --- a/src/ghydramcp/mixins/docker.py +++ b/src/ghydramcp/mixins/docker.py @@ -897,47 +897,6 @@ class DockerMixin(MCPMixin): None, self._sync_health_check, port, timeout ) - @mcp_tool( - name="docker_wait", - description="Wait for a GhydraMCP container to become healthy", - ) - async def docker_wait( - self, - port: int = 8192, - timeout: float = 300.0, - interval: float = 5.0, - ctx: Optional[Context] = None, - ) -> Dict[str, Any]: - """Wait for a GhydraMCP container to become healthy. - - Polls the API endpoint until it responds or timeout is reached. - - Args: - port: API port to check (default: 8192) - timeout: Maximum time to wait in seconds (default: 300) - interval: Polling interval in seconds (default: 5) - - Returns: - Health status once healthy, or error on timeout - """ - start_time = time.time() - last_error = None - - while (time.time() - start_time) < timeout: - result = await self.docker_health(port=port, timeout=interval, ctx=ctx) - if result.get("healthy"): - result["waited_seconds"] = round(time.time() - start_time, 1) - return result - last_error = result.get("error") - await asyncio.sleep(interval) - - return { - "healthy": False, - "port": port, - "error": f"Timeout after {timeout}s waiting for container", - "last_error": last_error, - } - @mcp_tool( name="docker_auto_start", description="Automatically start a GhydraMCP container with dynamic port allocation", @@ -952,17 +911,20 @@ class DockerMixin(MCPMixin): This is the main entry point for automatic Docker management: 1. Checks if a Ghidra instance with the SAME binary is already running 2. If not, allocates a port from the pool and starts a new container - 3. Returns connection info immediately (use docker_wait to poll for readiness) + 3. Returns connection info immediately Ports are auto-allocated from the pool (8192-8223) to prevent conflicts between concurrent sessions. + After starting, poll docker_health(port) in a loop to check readiness. + This gives you visibility into progress and ability to check logs. + Args: binary_path: Path to the binary to analyze Returns: Instance connection info with session ID and port details. - Use docker_wait(port) or docker_health(port) to check when ready. + Poll docker_health(port) to check when container is ready. """ import os @@ -1020,7 +982,7 @@ class DockerMixin(MCPMixin): "container_name": start_result.get("name"), "port": actual_port, "api_url": f"http://localhost:{actual_port}/", - "message": f"Container starting on port {actual_port}. Use docker_wait(port={actual_port}) to poll for readiness.", + "message": f"Container starting on port {actual_port}. Poll docker_health(port={actual_port}) to check when ready.", } @mcp_tool(