refactor: Remove wait/timeout params from docker_auto_start
The wait parameter was a convenience anti-pattern that caused LLMs to block on a single tool call for up to 5 minutes with no visibility into progress. Now docker_auto_start always returns immediately. Clients should use docker_wait(port) separately to poll for container readiness. This gives visibility into progress and allows early bailout.
This commit is contained in:
parent
6662c8411a
commit
5300fb24b8
@ -59,7 +59,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
|||||||
|
|
||||||
### Changed
|
### 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 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:** Default `wait=False` for immediate return. Use `docker_wait` separately to poll for container readiness.
|
- **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.
|
||||||
|
|
||||||
### Fixed
|
### 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.
|
- **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.
|
||||||
|
|||||||
@ -945,8 +945,6 @@ class DockerMixin(MCPMixin):
|
|||||||
async def docker_auto_start(
|
async def docker_auto_start(
|
||||||
self,
|
self,
|
||||||
binary_path: str,
|
binary_path: str,
|
||||||
wait: bool = False,
|
|
||||||
timeout: float = 300.0,
|
|
||||||
ctx: Optional[Context] = None,
|
ctx: Optional[Context] = None,
|
||||||
) -> Dict[str, Any]:
|
) -> Dict[str, Any]:
|
||||||
"""Automatically start a Docker container with intelligent port allocation.
|
"""Automatically start a Docker container with intelligent port allocation.
|
||||||
@ -954,19 +952,17 @@ class DockerMixin(MCPMixin):
|
|||||||
This is the main entry point for automatic Docker management:
|
This is the main entry point for automatic Docker management:
|
||||||
1. Checks if a Ghidra instance with the SAME binary is already running
|
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
|
2. If not, allocates a port from the pool and starts a new container
|
||||||
3. Optionally waits for the container to become healthy
|
3. Returns connection info immediately (use docker_wait to poll for readiness)
|
||||||
4. Returns connection info for the instance
|
|
||||||
|
|
||||||
Ports are auto-allocated from the pool (8192-8223) to prevent
|
Ports are auto-allocated from the pool (8192-8223) to prevent
|
||||||
conflicts between concurrent sessions.
|
conflicts between concurrent sessions.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
binary_path: Path to the binary to analyze
|
binary_path: Path to the binary to analyze
|
||||||
wait: Wait for container to be ready (default: False, use docker_wait separately)
|
|
||||||
timeout: Max wait time in seconds (default: 300)
|
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
Instance connection info with session ID and port details
|
Instance connection info with session ID and port details.
|
||||||
|
Use docker_wait(port) or docker_health(port) to check when ready.
|
||||||
"""
|
"""
|
||||||
import os
|
import os
|
||||||
|
|
||||||
@ -1017,31 +1013,6 @@ class DockerMixin(MCPMixin):
|
|||||||
|
|
||||||
actual_port = start_result.get("port")
|
actual_port = start_result.get("port")
|
||||||
|
|
||||||
if wait:
|
|
||||||
# Wait for the container to become healthy
|
|
||||||
wait_result = await self.docker_wait(port=actual_port, timeout=timeout, ctx=ctx)
|
|
||||||
if wait_result.get("healthy"):
|
|
||||||
return {
|
|
||||||
"source": "docker",
|
|
||||||
"session_id": self.session_id,
|
|
||||||
"container_id": start_result.get("container_id"),
|
|
||||||
"container_name": start_result.get("name"),
|
|
||||||
"port": actual_port,
|
|
||||||
"api_url": f"http://localhost:{actual_port}/",
|
|
||||||
"program": wait_result.get("program"),
|
|
||||||
"waited_seconds": wait_result.get("waited_seconds"),
|
|
||||||
"message": f"Docker container ready on port {actual_port} after {wait_result.get('waited_seconds')}s",
|
|
||||||
}
|
|
||||||
else:
|
|
||||||
return {
|
|
||||||
"warning": "Container started but not yet healthy",
|
|
||||||
"session_id": self.session_id,
|
|
||||||
"container_id": start_result.get("container_id"),
|
|
||||||
"port": actual_port,
|
|
||||||
"last_error": wait_result.get("error"),
|
|
||||||
"message": "Container may still be analyzing. Check docker_logs() for progress.",
|
|
||||||
}
|
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"source": "docker",
|
"source": "docker",
|
||||||
"session_id": self.session_id,
|
"session_id": self.session_id,
|
||||||
@ -1049,7 +1020,7 @@ class DockerMixin(MCPMixin):
|
|||||||
"container_name": start_result.get("name"),
|
"container_name": start_result.get("name"),
|
||||||
"port": actual_port,
|
"port": actual_port,
|
||||||
"api_url": f"http://localhost:{actual_port}/",
|
"api_url": f"http://localhost:{actual_port}/",
|
||||||
"message": f"Container starting on port {actual_port}. Use docker_wait() or docker_health() to check status.",
|
"message": f"Container starting on port {actual_port}. Use docker_wait(port={actual_port}) to poll for readiness.",
|
||||||
}
|
}
|
||||||
|
|
||||||
@mcp_tool(
|
@mcp_tool(
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user