diff --git a/apps/device-host-agent/host_agent/app.py b/apps/device-host-agent/host_agent/app.py index 9e161e9..d07849c 100644 --- a/apps/device-host-agent/host_agent/app.py +++ b/apps/device-host-agent/host_agent/app.py @@ -11,6 +11,7 @@ from device.manager import DeviceManager from host_agent.assignment import AssignmentExecutor from host_agent.client import HostAgentClient, HostAgentEnrollmentClient from host_agent.config import HostAgentConfig, load_host_agent_config +from host_agent.dependency_supervisor import DependencySupervisor from host_agent.devices import register_local_device from host_agent.enrollment import resolve_host_identity from host_agent.execution import create_execution_factories @@ -34,12 +35,23 @@ class HostAgentApplication: processor: AssignmentProcessor console_server: uvicorn.Server | None = None console_enrollment_client: HostAgentEnrollmentClient | None = None + dependency_supervisor: DependencySupervisor | None = None def run(self) -> None: asyncio.run(self.run_async()) async def run_async(self, stop: asyncio.Event | None = None) -> None: stop_requested = stop or asyncio.Event() + supervisor_stop = asyncio.Event() + supervisor_task: asyncio.Task[None] | None = None + if self.dependency_supervisor is not None: + # Bring up supervised dependencies (probe + spawn/adopt + readiness + # wait) before the heartbeat loop's first connect_devices() pass, + # so a supervised Appium is ready before any Driver.connect(). + await self.dependency_supervisor.start() + supervisor_task = asyncio.create_task( + self.dependency_supervisor.run(supervisor_stop) + ) heartbeat_stop = asyncio.Event() heartbeat_task = asyncio.create_task(self.heartbeat.run(heartbeat_stop)) console_task = ( @@ -75,12 +87,17 @@ class HostAgentApplication: with suppress(Exception): await asyncio.shield(active_processing) heartbeat_stop.set() + supervisor_stop.set() if self.console_server is not None: self.console_server.should_exit = True try: await asyncio.gather(heartbeat_task, return_exceptions=True) with suppress(Exception): await self.heartbeat.sync_once() + if supervisor_task is not None: + await asyncio.gather(supervisor_task, return_exceptions=True) + with suppress(Exception): + await self.dependency_supervisor.stop() if console_task is not None: with suppress(asyncio.CancelledError): await asyncio.gather(console_task, return_exceptions=True) @@ -213,12 +230,18 @@ def create_application( history_store, assignment, result ), ) + dependency_supervisor: DependencySupervisor | None = None + if resolved_config.dependency_supervisor_enabled: + dependency_supervisor = DependencySupervisor.from_host_agent_config( + resolved_config + ) return HostAgentApplication( client=client, heartbeat=heartbeat, processor=processor, console_server=console_server, console_enrollment_client=console_enrollment_client, + dependency_supervisor=dependency_supervisor, ) diff --git a/apps/device-host-agent/host_agent/config.py b/apps/device-host-agent/host_agent/config.py index c6dc390..6ffde69 100644 --- a/apps/device-host-agent/host_agent/config.py +++ b/apps/device-host-agent/host_agent/config.py @@ -35,6 +35,14 @@ class HostAgentConfig: console_session_ttl_seconds: float = 43200.0 console_history_limit: int = 200 ai_planner_transport: str = "direct" + dependency_supervisor_enabled: bool = False + appium_supervised: bool = False + appium_host: str = "127.0.0.1" + appium_port: int = 4723 + runtime_supervised: bool = False + runtime_host: str = "127.0.0.1" + runtime_port: int = 8000 + dependency_restart_max_attempts: int = 5 def load_host_agent_config( @@ -115,6 +123,18 @@ def load_host_agent_config( ai_planner_transport=_parse_ai_planner_transport( values.get("AI_PLANNER_TRANSPORT") ), + dependency_supervisor_enabled=_truthy( + values, "HOST_AGENT_DEPENDENCY_SUPERVISOR_ENABLED", False + ), + appium_supervised=_truthy(values, "HOST_AGENT_APPIUM_SUPERVISED", False), + appium_host=values.get("HOST_AGENT_APPIUM_HOST", "127.0.0.1").strip(), + appium_port=_positive_int(values, "HOST_AGENT_APPIUM_PORT", 4723), + runtime_supervised=_truthy(values, "HOST_AGENT_RUNTIME_SUPERVISED", False), + runtime_host=values.get("HOST_AGENT_RUNTIME_HOST", "127.0.0.1").strip(), + runtime_port=_positive_int(values, "HOST_AGENT_RUNTIME_PORT", 8000), + dependency_restart_max_attempts=_positive_int( + values, "HOST_AGENT_DEPENDENCY_RESTART_MAX_ATTEMPTS", 5 + ), ) if config.max_retry_backoff_seconds < config.retry_backoff_seconds: raise HostAgentConfigurationError( diff --git a/apps/device-host-agent/host_agent/dependency_supervisor.py b/apps/device-host-agent/host_agent/dependency_supervisor.py new file mode 100644 index 0000000..2ceb9b0 --- /dev/null +++ b/apps/device-host-agent/host_agent/dependency_supervisor.py @@ -0,0 +1,438 @@ +"""Optional supervisor for the two local external processes the Host Agent +depends on for the macOS single-machine real-device workflow: the Appium +server (which gates real ``Driver.connect()``) and the local Runtime API +(used for local inspection). + +Lives in ``host_agent`` because it spawns and monitors host-level processes +alongside the heartbeat/claim loop. Off by default; see ``HostAgentConfig``. + +Design (``openspec/changes/host-agent-dependency-supervisor/design.md``): +- Default off. Each dependency also has its own opt-in flag. +- Before spawn: TCP connect + dependency-specific HTTP health probe. A healthy + listener is *adopted* (never killed or restarted). An unhealthy listener is + a port conflict — logged and skipped. No listener → spawn. +- Only supervisor-spawned processes are restarted on unexpected exit, with + capped exponential backoff and a per-process-lifetime attempt ceiling. +- Adopted processes are never touched by ``stop()``. +- Spawns use stdlib ``subprocess.Popen`` (no new dependency). Child stdout/stderr + is forwarded into this module's logger, tagged per dependency. +""" + +from __future__ import annotations + +import asyncio +import logging +import socket +import subprocess +import threading +from collections.abc import Awaitable, Callable, Sequence +from dataclasses import dataclass +from enum import Enum +from typing import TYPE_CHECKING + +import httpx + +if TYPE_CHECKING: + from host_agent.config import HostAgentConfig + +_LOGGER = logging.getLogger("host_agent.dependency_supervisor") + +_PROBE_TCP_TIMEOUT_SECONDS = 1.0 +_PROBE_HTTP_TIMEOUT_SECONDS = 2.0 + + +class ProbeResult(str, Enum): + """Outcome of probing a dependency's configured address.""" + + HEALTHY = "healthy" + UNHEALTHY_LISTENER = "unhealthy_listener" + NO_LISTENER = "no_listener" + + +def probe_appium(host: str, port: int) -> ProbeResult: + """Probe Appium at ``host:port``. Healthy iff ``GET /status`` returns 200 + with a JSON body (the documented shape per ``docs/MACOS_IPHONE_SETUP.md`` + §6 — contains ``ready``/build info).""" + return _probe_http(host, port, path="/status") + + +def probe_runtime(host: str, port: int) -> ProbeResult: + """Probe the local Runtime API at ``host:port``. Healthy iff ``GET /devices`` + returns 200 with a JSON body. + + ``api.rest.create_app`` does not expose a dedicated ``/health`` endpoint; + ``/devices`` is the stable read-only GET that proves the FastAPI app is + mounted and the device manager is reachable. Per design.md Decision 2 this + is the "equivalent existing endpoint" used for the readiness check. + """ + return _probe_http(host, port, path="/devices") + + +def _probe_http(host: str, port: int, *, path: str) -> ProbeResult: + # Step 1: plain TCP connect — distinguish "nothing listening" (→ spawn) + # from "something is there but wrong" (→ port conflict, skip). + try: + with socket.create_connection((host, port), timeout=_PROBE_TCP_TIMEOUT_SECONDS): + pass + except OSError: + return ProbeResult.NO_LISTENER + + # Step 2: dependency-specific HTTP health check. + try: + response = httpx.get( + f"http://{host}:{port}{path}", + timeout=_PROBE_HTTP_TIMEOUT_SECONDS, + ) + except httpx.HTTPError: + return ProbeResult.UNHEALTHY_LISTENER + if response.status_code != 200: + return ProbeResult.UNHEALTHY_LISTENER + try: + response.json() + except ValueError: + return ProbeResult.UNHEALTHY_LISTENER + return ProbeResult.HEALTHY + + +def appium_argv_factory(host: str, port: int) -> list[str]: + return ["appium", "--address", host, "--port", str(port)] + + +def runtime_argv_factory(host: str, port: int) -> list[str]: + return [ + "uvicorn", + "api.rest:create_app", + "--factory", + "--host", + host, + "--port", + str(port), + ] + + +@dataclass +class SupervisedDependency: + """Config + mutable runtime state for one supervised external process.""" + + name: str + host: str + port: int + argv_factory: Callable[[str, int], Sequence[str]] + probe: Callable[[str, int], ProbeResult] + # Mutable state: + process: subprocess.Popen | None = None + adopted: bool = False + given_up: bool = False + restart_attempts: int = 0 + ready: bool = False + + +@dataclass +class _SupervisorKnobs: + """Tunables exposed for tests; production uses the defaults.""" + + startup_timeout_seconds: float = 30.0 + readiness_poll_interval_seconds: float = 0.5 + crash_poll_interval_seconds: float = 1.0 + initial_backoff_seconds: float = 1.0 + max_backoff_seconds: float = 30.0 + terminate_grace_period_seconds: float = 5.0 + + +class DependencySupervisor: + """Manage zero or more supervised local dependencies. + + Each dependency is either *adopted* (an existing healthy instance was + detected — never killed or restarted) or *spawned* (a child ``Popen`` + handle the supervisor owns — restart-on-crash with capped exponential + backoff up to ``max_attempts`` per process lifetime). + """ + + def __init__( + self, + dependencies: list[SupervisedDependency], + *, + max_attempts: int, + knobs: _SupervisorKnobs | None = None, + sleep: Callable[[float], Awaitable[None]] = asyncio.sleep, + popen_factory: Callable[[Sequence[str]], subprocess.Popen] = subprocess.Popen, + logger: logging.Logger = _LOGGER, + ) -> None: + self._dependencies = dependencies + self._max_attempts = max_attempts + self._knobs = knobs or _SupervisorKnobs() + self._sleep = sleep + self._popen_factory = popen_factory + self._logger = logger + self._reader_threads: list[threading.Thread] = [] + + @classmethod + def from_host_agent_config( + cls, + ha_config: HostAgentConfig, + **kwargs: object, + ) -> DependencySupervisor: + """Build a supervisor reflecting ``HostAgentConfig`` flags. + + Caller is responsible for only invoking this when + ``dependency_supervisor_enabled`` is true; the resulting supervisor + will contain only the dependencies whose individual ``*_supervised`` + flag is also true (possibly an empty list). + """ + deps: list[SupervisedDependency] = [] + if ha_config.appium_supervised: + deps.append( + SupervisedDependency( + name="appium", + host=ha_config.appium_host, + port=ha_config.appium_port, + argv_factory=appium_argv_factory, + probe=probe_appium, + ) + ) + if ha_config.runtime_supervised: + deps.append( + SupervisedDependency( + name="runtime", + host=ha_config.runtime_host, + port=ha_config.runtime_port, + argv_factory=runtime_argv_factory, + probe=probe_runtime, + ) + ) + return cls( + deps, + max_attempts=ha_config.dependency_restart_max_attempts, + **kwargs, # type: ignore[arg-type] + ) + + @property + def dependencies(self) -> list[SupervisedDependency]: + return list(self._dependencies) + + async def start(self) -> None: + """Probe + spawn/adopt + readiness wait. + + Call before the heartbeat loop's first ``connect_devices()`` pass so + a supervised Appium is up before any ``Driver.connect()`` attempt. + """ + for dep in self._dependencies: + await self._start_one(dep) + + async def _start_one(self, dep: SupervisedDependency) -> None: + result = await asyncio.to_thread(dep.probe, dep.host, dep.port) + if result is ProbeResult.HEALTHY: + dep.adopted = True + dep.ready = True + self._logger.info( + "dependency-supervisor: %s adopted existing instance at %s:%s", + dep.name, + dep.host, + dep.port, + ) + return + if result is ProbeResult.UNHEALTHY_LISTENER: + self._logger.error( + "dependency-supervisor: %s port %s occupied by an unhealthy " + "listener; leaving un-started to avoid a port conflict", + dep.name, + dep.port, + ) + dep.given_up = True + return + await self._spawn_with_readiness_wait(dep) + + async def _spawn_with_readiness_wait( + self, + dep: SupervisedDependency, + ) -> bool: + """Spawn ``dep`` and wait for it to become healthy. + + Returns True on readiness, False otherwise. On spawn failure (missing + executable) or readiness timeout while the process is still running, + marks the dependency as ``given_up`` (no crash-restart loop entered). + If the process exits during startup, returns False without giving up + — the crash-restart loop in ``run()`` handles subsequent restarts. + """ + argv = list(dep.argv_factory(dep.host, dep.port)) + try: + proc = self._popen_factory( + argv, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + bufsize=1, + text=True, + ) + except FileNotFoundError: + self._logger.error( + "dependency-supervisor: %s spawn failed — executable not " + "found on PATH (tried: %s)", + dep.name, + argv[0], + ) + dep.given_up = True + return False + except OSError as exc: + self._logger.error( + "dependency-supervisor: %s spawn failed: %s", + dep.name, + exc, + ) + dep.given_up = True + return False + + dep.process = proc + dep.adopted = False + dep.ready = False + self._start_reader_thread(dep, proc) + self._logger.info( + "dependency-supervisor: %s spawned (pid %s) at %s:%s", + dep.name, + proc.pid, + dep.host, + dep.port, + ) + + loop = asyncio.get_running_loop() + deadline = loop.time() + self._knobs.startup_timeout_seconds + while True: + if proc.poll() is not None: + self._logger.error( + "dependency-supervisor: %s exited during startup with code %s", + dep.name, + proc.returncode, + ) + dep.ready = False + return False + probe_result = await asyncio.to_thread(dep.probe, dep.host, dep.port) + if probe_result is ProbeResult.HEALTHY: + dep.ready = True + self._logger.info( + "dependency-supervisor: %s ready at %s:%s", + dep.name, + dep.host, + dep.port, + ) + return True + if loop.time() >= deadline: + self._logger.error( + "dependency-supervisor: %s started but did not become " + "healthy within %ss; process still running, not entering " + "crash-restart loop", + dep.name, + self._knobs.startup_timeout_seconds, + ) + dep.ready = False + return False + await self._sleep(self._knobs.readiness_poll_interval_seconds) + + def _start_reader_thread( + self, + dep: SupervisedDependency, + proc: subprocess.Popen, + ) -> None: + def reader_loop() -> None: + stdout = proc.stdout + if stdout is None: + return + for raw_line in stdout: + line = raw_line.rstrip() + if line: + self._logger.info("%s: %s", dep.name, line) + + thread = threading.Thread( + target=reader_loop, + name=f"dep-sup-{dep.name}", + daemon=True, + ) + thread.start() + self._reader_threads.append(thread) + + async def run(self, stop: asyncio.Event) -> None: + """Background monitor: detect crashes and apply backoff restart. + + Returns when ``stop`` is set. Crashes are only detected for + spawned processes (not adopted ones). + """ + if not self._dependencies: + await stop.wait() + return + while not stop.is_set(): + for dep in self._dependencies: + await self._check_one(dep) + try: + await asyncio.wait_for( + stop.wait(), + timeout=self._knobs.crash_poll_interval_seconds, + ) + except TimeoutError: + continue + + async def _check_one(self, dep: SupervisedDependency) -> None: + if dep.given_up or dep.adopted or dep.process is None: + return + if dep.process.poll() is None: + return + + self._logger.warning( + "dependency-supervisor: %s exited unexpectedly with code %s", + dep.name, + dep.process.returncode, + ) + dep.process = None + dep.ready = False + dep.restart_attempts += 1 + if dep.restart_attempts > self._max_attempts: + self._logger.error( + "dependency-supervisor: %s restart attempts exhausted " + "(crashes: %s, limit: %s); giving up for the rest of this " + "process lifetime", + dep.name, + dep.restart_attempts, + self._max_attempts, + ) + dep.given_up = True + return + + backoff = min( + self._knobs.initial_backoff_seconds * (2 ** (dep.restart_attempts - 1)), + self._knobs.max_backoff_seconds, + ) + self._logger.info( + "dependency-supervisor: %s restarting in %.1fs (crash %s/%s)", + dep.name, + backoff, + dep.restart_attempts, + self._max_attempts, + ) + await self._sleep(backoff) + await self._spawn_with_readiness_wait(dep) + + async def stop(self) -> None: + """Terminate spawned children. Adopted processes are left untouched.""" + for dep in self._dependencies: + if dep.adopted or dep.process is None: + continue + proc = dep.process + self._logger.info( + "dependency-supervisor: stopping %s (pid %s)", + dep.name, + proc.pid, + ) + proc.terminate() + try: + await asyncio.to_thread( + proc.wait, self._knobs.terminate_grace_period_seconds + ) + except subprocess.TimeoutExpired: + self._logger.warning( + "dependency-supervisor: %s did not exit within %ss, killing", + dep.name, + self._knobs.terminate_grace_period_seconds, + ) + proc.kill() + await asyncio.to_thread(proc.wait) + except OSError: + pass + finally: + dep.process = None diff --git a/apps/device-host-agent/tests/test_app.py b/apps/device-host-agent/tests/test_app.py index 163521d..90673f1 100644 --- a/apps/device-host-agent/tests/test_app.py +++ b/apps/device-host-agent/tests/test_app.py @@ -454,3 +454,125 @@ def test_console_is_created_by_default(tmp_path, monkeypatch) -> None: assert application.console_server is not None asyncio.run(application.client.aclose()) + + +def test_dependency_supervisor_is_none_when_disabled(tmp_path, monkeypatch) -> None: + monkeypatch.chdir(tmp_path) + application = create_application(config=_config(), manager=DeviceManager()) + + assert application.dependency_supervisor is None + asyncio.run(application.client.aclose()) + + +def test_dependency_supervisor_constructed_when_enabled_with_no_deps( + tmp_path, monkeypatch +) -> None: + monkeypatch.chdir(tmp_path) + config = HostAgentConfig( + control_plane_url="https://control.example", + host_id="host-a", + token="secret", + dependency_supervisor_enabled=True, + ) + application = create_application(config=config, manager=DeviceManager()) + + assert application.dependency_supervisor is not None + assert application.dependency_supervisor.dependencies == [] + asyncio.run(application.client.aclose()) + + +def test_dependency_supervisor_constructed_when_enabled_with_appium_only( + tmp_path, monkeypatch +) -> None: + monkeypatch.chdir(tmp_path) + config = HostAgentConfig( + control_plane_url="https://control.example", + host_id="host-a", + token="secret", + dependency_supervisor_enabled=True, + appium_supervised=True, + appium_host="127.0.0.1", + appium_port=4723, + ) + application = create_application(config=config, manager=DeviceManager()) + + assert application.dependency_supervisor is not None + deps = application.dependency_supervisor.dependencies + assert [dep.name for dep in deps] == ["appium"] + asyncio.run(application.client.aclose()) + + +def test_run_async_starts_supervisor_before_first_heartbeat_connect() -> None: + async def scenario() -> None: + events: list[str] = [] + + class SupervisedNoOp: + def __init__(self) -> None: + self.started = False + self.stopped = False + + async def start(self) -> None: + self.started = True + events.append("supervisor-start") + + async def run(self, stop: asyncio.Event) -> None: + events.append("supervisor-run-entered") + await stop.wait() + + async def stop(self) -> None: + self.stopped = True + events.append("supervisor-stop") + + class BlockingClient: + async def claim(self): + await asyncio.Event().wait() + + async def aclose(self): + events.append("closed") + + class RecordingHeartbeat: + def __init__(self) -> None: + self.connect_called = False + + def connect_devices(self) -> None: + self.connect_called = True + events.append("connect-devices") + + async def run(self, stop: asyncio.Event) -> None: + # Mirror HeartbeatSynchronizer.run which calls connect_devices() + # at the very top — supervisor must have started already. + self.connect_devices() + await stop.wait() + + async def sync_once(self): + events.append("final-heartbeat") + + class IdleProcessor: + async def process(self, assignment): + raise AssertionError("no assignment expected") + + def request_stop(self): + events.append("stop-work") + + supervisor = SupervisedNoOp() + application = HostAgentApplication( + client=BlockingClient(), # type: ignore[arg-type] + heartbeat=RecordingHeartbeat(), # type: ignore[arg-type] + processor=IdleProcessor(), # type: ignore[arg-type] + dependency_supervisor=supervisor, # type: ignore[arg-type] + ) + stop = asyncio.Event() + running = asyncio.create_task(application.run_async(stop)) + # Yield long enough for startup sequencing to land. + await asyncio.sleep(0.05) + stop.set() + await asyncio.wait_for(running, timeout=1.0) + + assert supervisor.started is True + assert supervisor.stopped is True + # Supervisor startup must precede the heartbeat's connect_devices(). + assert events.index("supervisor-start") < events.index("connect-devices") + # Supervisor stop must run before client close. + assert events.index("supervisor-stop") < events.index("closed") + + asyncio.run(scenario()) diff --git a/apps/device-host-agent/tests/test_config.py b/apps/device-host-agent/tests/test_config.py index 904327f..68d8be9 100644 --- a/apps/device-host-agent/tests/test_config.py +++ b/apps/device-host-agent/tests/test_config.py @@ -173,3 +173,55 @@ def test_console_numeric_fields_reject_invalid_values( ) -> None: with pytest.raises(HostAgentConfigurationError): load_host_agent_config(overrides) + + +def test_dependency_supervisor_defaults_to_disabled() -> None: + config = load_host_agent_config({}) + + assert config.dependency_supervisor_enabled is False + assert config.appium_supervised is False + assert config.appium_host == "127.0.0.1" + assert config.appium_port == 4723 + assert config.runtime_supervised is False + assert config.runtime_host == "127.0.0.1" + assert config.runtime_port == 8000 + assert config.dependency_restart_max_attempts == 5 + + +def test_dependency_supervisor_env_vars_parse_bool_and_numeric_fields() -> None: + config = load_host_agent_config( + { + "HOST_AGENT_DEPENDENCY_SUPERVISOR_ENABLED": "true", + "HOST_AGENT_APPIUM_SUPERVISED": "1", + "HOST_AGENT_APPIUM_HOST": "0.0.0.0", + "HOST_AGENT_APPIUM_PORT": "4724", + "HOST_AGENT_RUNTIME_SUPERVISED": "true", + "HOST_AGENT_RUNTIME_HOST": "localhost", + "HOST_AGENT_RUNTIME_PORT": "8001", + "HOST_AGENT_DEPENDENCY_RESTART_MAX_ATTEMPTS": "8", + } + ) + + assert config.dependency_supervisor_enabled is True + assert config.appium_supervised is True + assert config.appium_host == "0.0.0.0" + assert config.appium_port == 4724 + assert config.runtime_supervised is True + assert config.runtime_host == "localhost" + assert config.runtime_port == 8001 + assert config.dependency_restart_max_attempts == 8 + + +@pytest.mark.parametrize( + "overrides", + [ + {"HOST_AGENT_APPIUM_PORT": "0"}, + {"HOST_AGENT_RUNTIME_PORT": "not-a-number"}, + {"HOST_AGENT_DEPENDENCY_RESTART_MAX_ATTEMPTS": "-1"}, + ], +) +def test_dependency_supervisor_numeric_fields_reject_invalid_values( + overrides: dict[str, str], +) -> None: + with pytest.raises(HostAgentConfigurationError): + load_host_agent_config(overrides) diff --git a/apps/device-host-agent/tests/test_dependency_supervisor.py b/apps/device-host-agent/tests/test_dependency_supervisor.py new file mode 100644 index 0000000..e7bb376 --- /dev/null +++ b/apps/device-host-agent/tests/test_dependency_supervisor.py @@ -0,0 +1,554 @@ +from __future__ import annotations + +import asyncio +import io +import logging +import subprocess +from collections.abc import Sequence + +import httpx +import pytest + +from host_agent.dependency_supervisor import ( + DependencySupervisor, + ProbeResult, + SupervisedDependency, + _SupervisorKnobs, + appium_argv_factory, + probe_appium, + probe_runtime, + runtime_argv_factory, +) +from host_agent.config import HostAgentConfig + + +# --------------------------------------------------------------------------- +# Test doubles +# --------------------------------------------------------------------------- + + +class _FakePopen: + """Minimal subprocess.Popen stand-in for supervisor tests.""" + + instances: list[_FakePopen] = [] + + def __init__( + self, + argv: Sequence[str], + *, + stdout_lines: Sequence[str] = (), + pid: int = 0, + **_kwargs: object, + ) -> None: + self.argv = list(argv) + self.pid = pid or (100 + len(_FakePopen.instances)) + self.returncode: int | None = None + self.stdout = io.StringIO("".join(line + "\n" for line in stdout_lines)) + self.terminate_calls = 0 + self.kill_calls = 0 + self.wait_calls = 0 + _FakePopen.instances.append(self) + + def poll(self) -> int | None: + return self.returncode + + def terminate(self) -> None: + self.terminate_calls += 1 + self.returncode = -15 + + def kill(self) -> None: + self.kill_calls += 1 + self.returncode = -9 + + def wait(self, timeout: float | None = None) -> int: + self.wait_calls += 1 + if self.returncode is None: + raise subprocess.TimeoutExpired(cmd=self.argv, timeout=timeout or 0) + return self.returncode + + +@pytest.fixture(autouse=True) +def _reset_fake_popen() -> None: + _FakePopen.instances.clear() + yield + _FakePopen.instances.clear() + + +def _dep( + name: str = "appium", + *, + host: str = "127.0.0.1", + port: int = 4723, + probe_responses: Sequence[ProbeResult] = (ProbeResult.NO_LISTENER,), + argv_factory=None, +) -> tuple[SupervisedDependency, list[ProbeResult]]: + """Build a SupervisedDependency whose probe returns scripted responses.""" + call_log: list[ProbeResult] = [] + responses = list(probe_responses) + fallback = probe_responses[-1] if probe_responses else ProbeResult.NO_LISTENER + + def probe(_host: str, _port: int) -> ProbeResult: + if responses: + result = responses.pop(0) + else: + result = fallback + call_log.append(result) + return result + + if argv_factory is None: + + def argv_factory(h: str, p: int) -> list[str]: + return ["echo", name] + + dep = SupervisedDependency( + name=name, + host=host, + port=port, + argv_factory=argv_factory, + probe=probe, + ) + return dep, call_log + + +class _FakeSleep: + """Records sleep durations so tests can assert backoff progression.""" + + def __init__(self) -> None: + self.calls: list[float] = [] + + async def __call__(self, seconds: float) -> None: + self.calls.append(seconds) + + +def _build_supervisor( + deps: list[SupervisedDependency], + *, + max_attempts: int = 5, + knobs: _SupervisorKnobs | None = None, + sleep: _FakeSleep | None = None, + popen_factory=None, +) -> tuple[DependencySupervisor, _FakeSleep]: + fake_sleep = sleep or _FakeSleep() + sup = DependencySupervisor( + list(deps), + max_attempts=max_attempts, + knobs=knobs + or _SupervisorKnobs( + startup_timeout_seconds=1.0, + readiness_poll_interval_seconds=0.01, + crash_poll_interval_seconds=0.01, + initial_backoff_seconds=1.0, + max_backoff_seconds=30.0, + terminate_grace_period_seconds=1.0, + ), + sleep=fake_sleep, + popen_factory=popen_factory or _FakePopen, + logger=logging.getLogger("test"), + ) + return sup, fake_sleep + + +class _DummySocket: + def __enter__(self) -> "_DummySocket": + return self + + def __exit__(self, *exc) -> None: + return None + + +def _raise_connection_refused(*_args, **_kwargs): + raise ConnectionRefusedError("no listener") + + +def _ok_connection(*_args, **_kwargs): + return _DummySocket() + + +# --------------------------------------------------------------------------- +# Probe tests (tasks 2.1, 2.3) +# --------------------------------------------------------------------------- + + +def test_probe_returns_no_listener_when_port_is_closed(monkeypatch) -> None: + monkeypatch.setattr( + "host_agent.dependency_supervisor.socket.create_connection", + _raise_connection_refused, + ) + assert probe_appium("127.0.0.1", 4723) is ProbeResult.NO_LISTENER + assert probe_runtime("127.0.0.1", 8000) is ProbeResult.NO_LISTENER + + +def test_probe_returns_healthy_on_appium_status_endpoint(monkeypatch) -> None: + monkeypatch.setattr( + "host_agent.dependency_supervisor.socket.create_connection", + _ok_connection, + ) + monkeypatch.setattr( + "host_agent.dependency_supervisor.httpx.get", + lambda url, timeout=2.0: httpx.Response(200, json={"ready": True}), + ) + assert probe_appium("127.0.0.1", 4723) is ProbeResult.HEALTHY + + +def test_probe_returns_healthy_on_runtime_devices_endpoint(monkeypatch) -> None: + monkeypatch.setattr( + "host_agent.dependency_supervisor.socket.create_connection", + _ok_connection, + ) + monkeypatch.setattr( + "host_agent.dependency_supervisor.httpx.get", + lambda url, timeout=2.0: httpx.Response(200, json=[]), + ) + assert probe_runtime("127.0.0.1", 8000) is ProbeResult.HEALTHY + + +def test_probe_returns_unhealthy_when_listener_returns_non_200(monkeypatch) -> None: + monkeypatch.setattr( + "host_agent.dependency_supervisor.socket.create_connection", + _ok_connection, + ) + monkeypatch.setattr( + "host_agent.dependency_supervisor.httpx.get", + lambda url, timeout=2.0: httpx.Response(500, text="boom"), + ) + assert probe_appium("127.0.0.1", 4723) is ProbeResult.UNHEALTHY_LISTENER + + +def test_probe_returns_unhealthy_when_listener_returns_non_json(monkeypatch) -> None: + monkeypatch.setattr( + "host_agent.dependency_supervisor.socket.create_connection", + _ok_connection, + ) + monkeypatch.setattr( + "host_agent.dependency_supervisor.httpx.get", + lambda url, timeout=2.0: httpx.Response(200, text="not json"), + ) + assert probe_runtime("127.0.0.1", 8000) is ProbeResult.UNHEALTHY_LISTENER + + +def test_probe_returns_unhealthy_on_http_transport_error(monkeypatch) -> None: + monkeypatch.setattr( + "host_agent.dependency_supervisor.socket.create_connection", + _ok_connection, + ) + + def raise_http_error(url, timeout=2.0): + raise httpx.ConnectError("reset") + + monkeypatch.setattr("host_agent.dependency_supervisor.httpx.get", raise_http_error) + assert probe_appium("127.0.0.1", 4723) is ProbeResult.UNHEALTHY_LISTENER + + +# --------------------------------------------------------------------------- +# Adoption logic tests (tasks 2.2, 2.3) +# --------------------------------------------------------------------------- + + +def test_start_adopts_existing_healthy_instance() -> None: + async def scenario() -> None: + dep, _ = _dep(probe_responses=(ProbeResult.HEALTHY,)) + sup, _ = _build_supervisor([dep]) + + await sup.start() + + assert dep.adopted is True + assert dep.ready is True + assert dep.process is None + + asyncio.run(scenario()) + + +def test_start_skips_when_port_has_unhealthy_listener() -> None: + async def scenario() -> None: + dep, _ = _dep(probe_responses=(ProbeResult.UNHEALTHY_LISTENER,)) + sup, _ = _build_supervisor([dep]) + + await sup.start() + + assert dep.adopted is False + assert dep.process is None + assert dep.given_up is True + + asyncio.run(scenario()) + + +def test_start_spawns_when_nothing_is_listening() -> None: + async def scenario() -> None: + # NO_LISTENER for initial probe → spawn path. + # HEALTHY on first readiness probe. + dep, _ = _dep( + probe_responses=(ProbeResult.NO_LISTENER, ProbeResult.HEALTHY), + ) + sup, _ = _build_supervisor([dep]) + + await sup.start() + + assert dep.process is not None + assert dep.adopted is False + assert dep.ready is True + assert dep.given_up is False + + asyncio.run(scenario()) + + +# --------------------------------------------------------------------------- +# Supervisor core tests (task 3.6) +# --------------------------------------------------------------------------- + + +def test_spawn_failure_marks_dependency_given_up() -> None: + async def scenario() -> None: + dep, _ = _dep(probe_responses=(ProbeResult.NO_LISTENER,)) + + def raising_popen(argv, **_kwargs): + raise FileNotFoundError("appium: not found") + + sup, _ = _build_supervisor([dep], popen_factory=raising_popen) + await sup.start() + + assert dep.process is None + assert dep.given_up is True + assert dep.ready is False + + asyncio.run(scenario()) + + +def test_spawn_uses_appium_argv_factory() -> None: + async def scenario() -> None: + captured: list[list[str]] = [] + + def recording_popen(argv, **kwargs): + captured.append(list(argv)) + return _FakePopen(argv, stdout_lines=("hello",)) + + dep, _ = _dep( + name="appium", + port=4723, + probe_responses=(ProbeResult.NO_LISTENER, ProbeResult.HEALTHY), + argv_factory=appium_argv_factory, + ) + sup, _ = _build_supervisor([dep], popen_factory=recording_popen) + await sup.start() + + assert captured == [["appium", "--address", "127.0.0.1", "--port", "4723"]] + + asyncio.run(scenario()) + + +def test_spawn_uses_runtime_argv_factory() -> None: + async def scenario() -> None: + captured: list[list[str]] = [] + + def recording_popen(argv, **kwargs): + captured.append(list(argv)) + return _FakePopen(argv) + + dep, _ = _dep( + name="runtime", + port=8000, + probe_responses=(ProbeResult.NO_LISTENER, ProbeResult.HEALTHY), + argv_factory=runtime_argv_factory, + ) + sup, _ = _build_supervisor([dep], popen_factory=recording_popen) + await sup.start() + + assert captured == [ + [ + "uvicorn", + "api.rest:create_app", + "--factory", + "--host", + "127.0.0.1", + "--port", + "8000", + ] + ] + + asyncio.run(scenario()) + + +def test_readiness_timeout_leaves_process_running_without_restart_loop() -> None: + async def scenario() -> None: + # NO_LISTENER for initial probe; probe never becomes HEALTHY → startup + # timeout path (1.0s in test knobs). + dep, _ = _dep(probe_responses=(ProbeResult.NO_LISTENER,)) + sup, _ = _build_supervisor([dep]) + + await sup.start() + + assert dep.process is not None # still running + assert dep.ready is False + assert dep.given_up is False # not given up; crash-restart owns crashes + + asyncio.run(scenario()) + + +def test_crash_triggers_restart_with_backoff() -> None: + async def scenario() -> None: + # Probe responses: + # 1. start._start_one → NO_LISTENER → spawn path + # 2. first readiness probe → HEALTHY → ready + # 3. after restart, readiness probe → HEALTHY → ready again + dep, _ = _dep( + probe_responses=( + ProbeResult.NO_LISTENER, + ProbeResult.HEALTHY, + ProbeResult.HEALTHY, + ), + argv_factory=appium_argv_factory, + ) + + spawn_count = {"n": 0} + + def popen(argv, **kwargs): + spawn_count["n"] += 1 + return _FakePopen(argv) + + sup, sleep = _build_supervisor([dep], popen_factory=popen) + await sup.start() + assert dep.ready is True + crashes_before = dep.restart_attempts + + # Crash the spawned process now (after readiness succeeded). The + # supervisor's run loop will detect this via poll(). + first_proc = dep.process + assert isinstance(first_proc, _FakePopen) + first_proc.returncode = 1 + + stop = asyncio.Event() + task = asyncio.create_task(sup.run(stop)) + # Give the loop a chance to detect the crash, sleep backoff, respawn. + await asyncio.sleep(0.05) + stop.set() + await asyncio.wait_for(task, timeout=1.0) + + assert dep.restart_attempts == crashes_before + 1 + assert dep.ready is True + assert sleep.calls == [1.0] # exponential backoff base for attempt 1 + assert spawn_count["n"] == 2 + + asyncio.run(scenario()) + + +def test_restart_exhaustion_gives_up() -> None: + async def scenario() -> None: + # Process crashes after every spawn; limit max_attempts to 2. + dep, _ = _dep( + probe_responses=(ProbeResult.NO_LISTENER,) + (ProbeResult.HEALTHY,) * 10, + ) + spawn_count = {"n": 0} + + def popen(argv, **kwargs): + spawn_count["n"] += 1 + proc = _FakePopen(argv) + proc.returncode = 1 + return proc + + sup, sleep = _build_supervisor([dep], popen_factory=popen, max_attempts=2) + await sup.start() + + stop = asyncio.Event() + task = asyncio.create_task(sup.run(stop)) + deadline = asyncio.get_running_loop().time() + 2.0 + while not dep.given_up and asyncio.get_running_loop().time() < deadline: + await asyncio.sleep(0.01) + stop.set() + await asyncio.wait_for(task, timeout=2.0) + + assert dep.given_up is True + # Initial spawn + 2 successful restarts before the 3rd crash gives up. + assert dep.restart_attempts == 3 + # Backoffs: 1.0 (attempt 1), 2.0 (attempt 2). No third restart. + assert sleep.calls == [1.0, 2.0] + + asyncio.run(scenario()) + + +def test_adopted_process_is_never_restarted_or_killed() -> None: + async def scenario() -> None: + dep, _ = _dep(probe_responses=(ProbeResult.HEALTHY,)) + sup, _ = _build_supervisor([dep]) + await sup.start() + + stop = asyncio.Event() + task = asyncio.create_task(sup.run(stop)) + await asyncio.sleep(0.02) + stop.set() + await asyncio.wait_for(task, timeout=1.0) + + await sup.stop() + + assert dep.adopted is True + assert dep.process is None + assert dep.restart_attempts == 0 + + asyncio.run(scenario()) + + +def test_stop_terminates_only_spawned_children() -> None: + async def scenario() -> None: + spawned_dep, _ = _dep( + name="spawned", + port=4723, + probe_responses=(ProbeResult.NO_LISTENER, ProbeResult.HEALTHY), + ) + adopted_dep, _ = _dep( + name="adopted", + port=8000, + probe_responses=(ProbeResult.HEALTHY,), + ) + sup, _ = _build_supervisor([spawned_dep, adopted_dep]) + await sup.start() + + assert spawned_dep.process is not None + spawned_proc = spawned_dep.process + assert adopted_dep.adopted is True + + await sup.stop() + + assert isinstance(spawned_proc, _FakePopen) + assert spawned_proc.terminate_calls == 1 + assert spawned_dep.process is None + + asyncio.run(scenario()) + + +# --------------------------------------------------------------------------- +# from_host_agent_config factory (covers wiring helper for task 4.1) +# --------------------------------------------------------------------------- + + +def test_from_host_agent_config_builds_empty_supervisor_when_no_dep_selected() -> None: + config = HostAgentConfig( + control_plane_url="https://control.example", + host_id="host-a", + token="t", + dependency_supervisor_enabled=True, + ) + sup = DependencySupervisor.from_host_agent_config(config) + assert sup.dependencies == [] + + +def test_from_host_agent_config_includes_appium_and_runtime_when_selected() -> None: + config = HostAgentConfig( + control_plane_url="https://control.example", + host_id="host-a", + token="t", + dependency_supervisor_enabled=True, + appium_supervised=True, + appium_host="10.0.0.5", + appium_port=4724, + runtime_supervised=True, + runtime_host="10.0.0.5", + runtime_port=8001, + dependency_restart_max_attempts=7, + ) + sup = DependencySupervisor.from_host_agent_config(config) + names = [dep.name for dep in sup.dependencies] + assert names == ["appium", "runtime"] + appium = sup.dependencies[0] + assert appium.host == "10.0.0.5" + assert appium.port == 4724 + runtime = sup.dependencies[1] + assert runtime.port == 8001 + assert sup._max_attempts == 7 diff --git a/docs/MACOS_IPHONE_SETUP.md b/docs/MACOS_IPHONE_SETUP.md index ff49e57..f32516f 100644 --- a/docs/MACOS_IPHONE_SETUP.md +++ b/docs/MACOS_IPHONE_SETUP.md @@ -428,6 +428,61 @@ http://127.0.0.1:8765 session TTL、历史记录条数上限等)参见 `docs/CLOUD_DEPLOYMENT.md`;生产/远程场景下 应优先使用 SSH 端口转发访问该 Console,而不是直接把它暴露到非回环地址。 +### 可选:由 Host Agent 托管 Appium 和 Runtime API + +默认情况下 Host Agent **不会**自动启动 Appium 或本地 Runtime API:必须按 +§6 在独立 Terminal 中保持 `appium --address 127.0.0.1 --port 4723` 运行,按 +§8 在另一个 Terminal 中启动 Runtime API。忘记其中任意一个,Host Agent 不会报错, +heartbeat 仍会成功,但设备会静默保持 `offline`、所有任务卡在 `queued`。 + +`host-agent-dependency-supervisor` 是一个可选模式,让 Host Agent 自己把这两个 +外部进程作为子进程托管,覆盖单机真机工作流。它默认关闭,需要显式 opt-in: + +```bash +export HOST_AGENT_DEPENDENCY_SUPERVISOR_ENABLED="true" + +# 任选其一或两者都开。两者默认 false。 +export HOST_AGENT_APPIUM_SUPERVISED="true" +export HOST_AGENT_RUNTIME_SUPERVISED="true" + +# 可覆盖默认地址/端口(默认值与 §6/§8 手动流程一致): +# export HOST_AGENT_APPIUM_HOST="127.0.0.1" +# export HOST_AGENT_APPIUM_PORT="4723" +# export HOST_AGENT_RUNTIME_HOST="127.0.0.1" +# export HOST_AGENT_RUNTIME_PORT="8000" + +# 单次 Host Agent 进程生命周期内允许的最大重启次数,默认 5。 +# export HOST_AGENT_DEPENDENCY_RESTART_MAX_ATTEMPTS="5" + +uv run --package device-host-agent device-host-agent +``` + +启用后的行为(详见 `openspec/changes/host-agent-dependency-supervisor/`): + +- **启动顺序**:Host Agent 在第一次 `connect_devices()` 之前,先按上面选中的 + 依赖项依次做"先探测后启动"。这样一旦开启,Appium 不再需要单独的 Terminal。 +- **Adopt-don't-fight**:探测 `(host, port)` 时若已经有进程在监听并通过健康检查 + (Appium `GET /status` 返回 200 JSON,Runtime `GET /devices` 返回 200 JSON), + Host Agent 会以 *adopted* 方式记录日志,**不会**再 spawn 一个重复进程,也不会 + 在退出/崩溃时杀掉或重启它。如果端口被占但健康检查失败,记一条 port-conflict + 错误并跳过该依赖,不抢端口、不静默继续。 +- **崩溃重启**:只有 Host Agent 自己 spawn 出来的子进程才会被监控。子进程意外 + 退出时,按指数退避(1s、2s、4s、8s,封顶 30s)重启;当某个依赖在本进程生命 + 周期内累计达到 `HOST_AGENT_DEPENDENCY_RESTART_MAX_ATTEMPTS` 次重启后,停止 + 再次尝试直到 Host Agent 重启。被 adopt 的进程永远不会被 Host Agent 重启或杀死。 +- **spawn 失败 ≠ crash**:如果 `appium` 可执行文件不在 `PATH` 上,启动会以 + "dependency-supervisor: appium spawn failed — executable not found" 形式记一条 + 依赖主管特有的错误,与正常 crash 区分开。请确认按 §4.3 安装好 `appium` 和 + XCUITest/UiAutomator2 driver。 +- **退出时**:Host Agent 在自身 graceful shutdown 阶段只会 `terminate` 它自己 + spawn 的子进程;adopted 进程保留不动。 +- **不影响 Docker/Compose**:本模式只针对 macOS 单机真机工作流; + `compose.yaml` / `compose.deploy.yaml` 完全不受影响。 + +如果偏好保持对 Appium 终端日志的完全控制、或者已经在用其他进程管理工具 +(launchd、systemd、tmux 等)托管 Appium,可以继续使用 §6/§8 的手动流程, +不开启本模式即可。 + ## 10. 多设备与端口 同时连接多台 iPhone 时,每台设备至少需要: diff --git a/openspec/changes/host-agent-dependency-supervisor/.openspec.yaml b/openspec/changes/host-agent-dependency-supervisor/.openspec.yaml new file mode 100644 index 0000000..64105fc --- /dev/null +++ b/openspec/changes/host-agent-dependency-supervisor/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-14 diff --git a/openspec/changes/host-agent-dependency-supervisor/design.md b/openspec/changes/host-agent-dependency-supervisor/design.md new file mode 100644 index 0000000..c2c24fe --- /dev/null +++ b/openspec/changes/host-agent-dependency-supervisor/design.md @@ -0,0 +1,74 @@ +## Context + +The Host Agent (`apps/device-host-agent/host_agent/`) is a headless outbound worker: `HostAgentApplication.run_async()` (`app.py:41-90`) runs a heartbeat loop and a claim/execute loop, and (since `host-agent-local-console`) an embedded local web console bound to `127.0.0.1:8765` by default. None of this touches Appium or the local Runtime API. + +Real device control depends on two processes the Host Agent does not manage today: +- **Appium server**, default `http://127.0.0.1:4723` (`docs/MACOS_IPHONE_SETUP.md:209`, `driver/wda_driver.py:12`, `driver/android_driver.py:22`). `Driver.connect()` opens a `webdriver.Remote` session against it; on failure it raises `DeviceOfflineError`, which `DeviceManager.connect()` (`device/manager.py:68-108`) catches and turns into device status `"offline"`/`"error"` — this is exactly what happened in the live incident that motivated this change: Appium wasn't running, the device stayed offline, and every assigned task queued forever with no error surfaced anywhere. +- **Local Runtime API** (`api.rest:create_app`, default `127.0.0.1:8000`), a fully separate FastAPI app/process used to inspect device/task/world state locally. It shares no runtime coupling with the Host Agent process today ("Runtime 不依赖/打包 cloud 或 application" — established repo convention) and this change does not introduce any. + +Appium already owns the entire WDA session lifecycle end to end (`docs/MACOS_IPHONE_SETUP.md:176`, `:455` — "本项目会让 Appium 的 XCUITest Driver 创建和管理 WDA session"; "常规单机使用优先让 Appium 管理 WDA,不要一开始就引入 iproxy 或手工 WDA 生命周期"). Any supervision the Host Agent adds must sit *outside* that boundary — it manages whether the Appium **process** is running, never what Appium does internally with WDA. + +## Goals / Non-Goals + +**Goals:** +- Let the Host Agent optionally start Appium and the local Runtime API as plain subprocesses when they aren't already running, so a forgotten manual step turns into "it just works" instead of "device silently offline forever." +- Make the current state (spawned vs. adopted-existing vs. failed-to-start) visible in the Host Agent's own log/console, so this remains debuggable rather than adding a second silent-failure mode on top of the one it fixes. +- Never conflict with an already-running instance: detect and adopt (read-only) rather than double-spawn or steal a port. + +**Non-Goals:** +- No change to Appium's ownership of WDA session lifecycle — the supervisor never touches WDA directly, never passes `webDriverAgentUrl`, never signs/builds anything. +- No Docker/Compose story. `compose.yaml`/`compose.deploy.yaml` are untouched; this is native-process-only, scoped to the macOS single-machine real-device workflow described in `docs/MACOS_IPHONE_SETUP.md`. +- No change to `driver/registry.py`, `driver/wda_driver.py`, `driver/android_driver.py`, or the outbound cloud protocol (`host-agent-protocol`). +- No process supervision beyond these two known dependencies (not a general-purpose process manager). +- No remote/cloud configuration of this feature — it's a local opt-in, matching how `host_agent/config.py` is already entirely env-var driven per-Host. + +## Decisions + +**1. Default: entire supervisor is off (`HOST_AGENT_DEPENDENCY_SUPERVISOR_ENABLED=false`).** +Unlike `host-agent-local-console` (which is now forced-on because it's a passive, loopback-bound, low-risk read/manage surface), this feature actively spawns and can restart external processes — including, in the crash-restart path, potentially interrupting an in-flight Appium session/WDA connection if a spawned Appium process is killed and restarted mid-task. That's a materially different risk profile from serving a status page, so it defaults off and is opt-in per Host, consistent with how `AI_PLANNER_ENABLED` defaulting decisions in this repo have been made deliberately per-surface rather than blanket. Each of the two dependencies (Appium, Runtime) also has its own enable flag nested under the top-level flag, so an operator can supervise Appium only, Runtime only, or both. + +**2. Adopt-don't-fight: TCP connect + HTTP health probe before spawn.** +Before spawning either process, the supervisor does a plain TCP connect to `(host, port)`. If something answers, it issues the same health check used post-spawn (Appium: `GET /status` expects HTTP 200 with `ready`/build JSON per `docs/MACOS_IPHONE_SETUP.md:218`; Runtime: `GET /health` or equivalent existing endpoint on `api.rest`). If the health check passes, the supervisor logs "adopted existing instance at host:port" and does **not** spawn a subprocess, does **not** track it as "ours," and will never kill or restart it. If the port is occupied but the health check fails (something else is listening, or it's an unhealthy/half-started instance), the supervisor logs an error for that dependency and leaves it un-started rather than guessing — port conflicts get surfaced, not silently resolved. This directly implements the concern raised during proposal review that an already-running manually-started Appium must not be duplicated or fought over. + +**3. Only supervisor-spawned processes are restarted; adopted processes are never touched.** +This is the direct mitigation for the WDA-session-interruption risk: an operator who already has their own long-lived Appium running (e.g., manually, or from a previous supervised run in another terminal) is never at risk of the Host Agent killing/restarting it. Restart-on-crash only applies to a `subprocess.Popen` handle the supervisor itself holds. + +**4. Crash-restart uses capped exponential backoff with a per-run attempt ceiling.** +Backoff schedule (e.g. 1s, 2s, 4s, 8s, capped at 30s) with a max attempt count per dependency per Host Agent process lifetime (e.g. 5); once exhausted, the supervisor logs that it's giving up on that dependency and stops trying, rather than restart-looping forever. This directly answers the "限流/退避避免死循环重启风暴" concern. The counter resets only on a fresh Host Agent process start, not periodically — a persistently-crashing Appium install shouldn't be retried indefinitely in the background. + +**5. Spawn mechanism: stdlib `subprocess.Popen`, no new dependency.** +Appium is invoked as `appium --address --port ` (assumes `appium` is already on `PATH`, exactly as the manual instructions already require — this change does not install or vendor Appium). Runtime is invoked as the existing `uvicorn api.rest:create_app --factory --host --port ` command, run as a subprocess rather than in-process, preserving the existing "Runtime is a fully separate process" boundary — this is process supervision, not a code-level dependency between the two packages. Both children's stdout/stderr are captured and forwarded into the Host Agent's own logging (prefixed per-process) so a crash's root cause doesn't require the operator to have had a spare terminal open. + +**6. Config fields follow the existing `HostAgentConfig`/`HOST_AGENT_*` convention.** +New fields on `HostAgentConfig` (mirroring the `console_*` fields added by `host-agent-local-console`): +- `dependency_supervisor_enabled: bool` ← `HOST_AGENT_DEPENDENCY_SUPERVISOR_ENABLED` (default `false`) +- `appium_supervised: bool` ← `HOST_AGENT_APPIUM_SUPERVISED` (default `false`) +- `appium_host: str` ← `HOST_AGENT_APPIUM_HOST` (default `127.0.0.1`) +- `appium_port: int` ← `HOST_AGENT_APPIUM_PORT` (default `4723`, matching the documented default) +- `runtime_supervised: bool` ← `HOST_AGENT_RUNTIME_SUPERVISED` (default `false`) +- `runtime_host: str` ← `HOST_AGENT_RUNTIME_HOST` (default `127.0.0.1`) +- `runtime_port: int` ← `HOST_AGENT_RUNTIME_PORT` (default `8000`, matching the documented default) +- `dependency_restart_max_attempts: int` ← `HOST_AGENT_DEPENDENCY_RESTART_MAX_ATTEMPTS` (default `5`) + +**7. Wiring point: alongside the existing heartbeat/claim loop, not inside it.** +`HostAgentApplication` starts the supervisor (if enabled) before starting the heartbeat loop and `connect_devices()` pass, and stops it during shutdown alongside the existing console/heartbeat teardown in `run_async()` — so a supervised Appium is up before the first `DeviceManager.connect()` attempt in the same startup sequence that already exists. + +## Risks / Trade-offs + +- **[Risk] A supervisor-spawned Appium restart mid-task interrupts an active WDA session, failing whatever task is running.** → Mitigation: restart only triggers on the spawned process actually exiting (crash), not on a health-check hiccup; the underlying task still fails cleanly through the existing `DeviceOfflineError` path (same as today when Appium isn't running at all) rather than in some new/undefined way. This is a pre-existing failure mode (Appium can already crash unsupervised) — the difference is the Host Agent now also tries to bring it back instead of leaving the device offline until a human notices. +- **[Risk] Supervising the Runtime API blurs the "fully independent process" boundary this repo has deliberately maintained.** → Mitigation: it's spawned as a subprocess via its existing CLI entry point, not imported/mounted in-process — no new Python-level dependency from `device-host-agent` onto `api.rest`, no shared code path. The boundary that matters (no import coupling, no shared FastAPI app) is preserved; only process *lifecycle* (start/stop timing) is now optionally shared. +- **[Risk] `appium` not on `PATH`, or wrong Appium driver installed (per `docs/MACOS_IPHONE_SETUP.md` xcuitest/uiautomator2 driver setup).** → Mitigation: spawn failure (e.g. `FileNotFoundError` from `subprocess.Popen`) is logged clearly as a dependency-supervisor error, distinct from a normal Appium crash, so the operator isn't left thinking Appium crashed when it never started. +- **[Risk] Health-check false positive: something unrelated happens to be listening on 4723/8000.** → Mitigation: the health check requires the actual expected response shape (Appium `/status` JSON, Runtime health endpoint), not just "port is open" — an unrelated listener fails the check and is treated as a conflict, not adopted. +- **[Trade-off] This only covers macOS native real-device usage; Docker/CI paths get no benefit.** → Accepted per proposal scope — those paths don't have this failure mode in the first place (no interactive multi-terminal manual step to forget). + +## Migration Plan + +- Fully additive and opt-in: existing Host Agent deployments/configs are unaffected with zero config changes (all new fields default to off/disabled). +- Rollout is per-Host: an operator turns it on by setting `HOST_AGENT_DEPENDENCY_SUPERVISOR_ENABLED=true` (and the relevant `*_SUPERVISED` flags) in their existing `.env`/environment, no schema or protocol changes involved. +- Rollback: unset the env vars (or set back to `false`) and restart the Host Agent — reverts to today's fully-manual behavior with no residual state (the supervisor holds no persistent storage). +- Documentation: `docs/MACOS_IPHONE_SETUP.md` gains a section presenting supervised mode as an optional alternative alongside the existing manual steps, which remain the documented default path. + +## Open Questions + +- Should `dependency_restart_max_attempts` exhaustion eventually surface in the local console (`host-agent-local-console`) status page rather than only in logs? Left for a future change if operators find log-only insufficient in practice. +- Android/UiAutomator2 real-device validation for this supervisor is out of scope for the same reason `android-driver` itself shipped without real-device validation — should be revisited once Android hardware validation happens. diff --git a/openspec/changes/host-agent-dependency-supervisor/proposal.md b/openspec/changes/host-agent-dependency-supervisor/proposal.md new file mode 100644 index 0000000..2b77e8f --- /dev/null +++ b/openspec/changes/host-agent-dependency-supervisor/proposal.md @@ -0,0 +1,28 @@ +## Why + +On the macOS native real-device setup (`docs/MACOS_IPHONE_SETUP.md`), a device only reports `idle`/online to the Cloud Control Plane if `DeviceManager.connect()` (`device/manager.py:68-108`) can actually establish an Appium session — but the Host Agent process (`apps/device-host-agent/host_agent/app.py`) never starts Appium itself; the operator must run `appium --address 127.0.0.1 --port 4723` in a separate terminal before starting `device-host-agent`, and separately run the local Runtime API (`uvicorn api.rest:create_app --factory ...`) if they want to inspect device/task state during local debugging. Forgetting (or losing) either process is invisible from the Host Agent's own output: heartbeats keep succeeding, so the Host looks "connected," while the device silently stays `offline` and every assigned task stays queued forever with no error. This was diagnosed live from exactly that symptom. Having the Host Agent supervise these known local dependencies for the single-machine dev/real-device workflow removes an easy-to-miss manual step and makes the failure visible instead of silent. + +## What Changes + +- `HostAgentApplication` gains an optional local dependency supervisor that, when enabled, starts and monitors two external processes alongside the existing heartbeat/claim loop: + - **Appium server** (`appium --address --port `), whose reachability already gates real device `connect()` calls. + - **Local Runtime API** (`uvicorn api.rest:create_app --factory --host --port `), used for local inspection/debugging of device and task state. +- Before spawning either process, the supervisor probes the configured address: if something is already listening and answers the expected health check, it adopts the existing instance (log-only) instead of spawning a duplicate or rebinding the port. It only spawns a subprocess when the port is free. +- Crash handling: if a supervised subprocess it spawned exits unexpectedly, the supervisor restarts it with capped exponential backoff and a maximum retry count per run; it does not touch a process it did not spawn (an adopted, externally-managed instance is never restarted or killed by the Host Agent). +- New `HostAgentConfig` fields to control this, all opt-in and disabled by default (see design.md for the specific default-off rationale): enable/disable the supervisor as a whole, enable/disable each of the two dependencies independently, and host/port for each (defaulting to the existing documented conventions — Appium `127.0.0.1:4723`, Runtime `127.0.0.1:8000`). +- Documentation update to `docs/MACOS_IPHONE_SETUP.md` describing the new opt-in supervised mode as an alternative to the existing manual multi-terminal flow (the manual flow remains fully supported and is still what's documented as the default path). +- Explicitly out of scope: no change to WDA's own lifecycle (Appium continues to own WDA session management entirely unchanged), no Docker Compose changes (`compose.yaml`/`compose.deploy.yaml` are unaffected — this is a native-process-only capability), no change to the outbound cloud protocol. + +## Capabilities + +### New Capabilities +- `host-agent-dependency-supervisor`: optional, opt-in local process supervision inside the Host Agent for the Appium server and the local Runtime API — port/adoption probing, spawn, health-checked readiness, and rate-limited crash-restart, scoped to the macOS native single-machine real-device workflow. + +### Modified Capabilities +(none — `host-agent-protocol` outbound behavior is unchanged; `driver-registry`/`device-pool` behavior for how a `Driver.connect()` reaches Appium is unchanged, this change only affects whether Appium happens to already be running when that connect attempt occurs) + +## Impact + +- Affected code: `apps/device-host-agent/host_agent/` (new supervisor module, `app.py` wiring to start/stop it alongside the heartbeat/claim loop, `config.py` new fields), `apps/device-host-agent/pyproject.toml` (no new runtime dependency expected — spawning uses stdlib `subprocess`; Runtime API is already invoked via its existing `uvicorn`/`api.rest` entry point). +- Not affected: `cloud.*`, `apps/cloud-api`, `compose.yaml`, `compose.deploy.yaml`, `driver/wda_driver.py` / `driver/android_driver.py` connection logic, WDA's own session lifecycle (still fully owned by Appium). +- Operational impact: opt-in only — a Host Agent with the supervisor disabled (the default) behaves exactly as it does today. When enabled, the Host Agent's own process lifecycle now indirectly affects two more local processes, which changes what "the Host Agent crashed" or "the Host Agent's log" means for local troubleshooting (see design.md for how adoption-vs-spawn is surfaced in logs to keep this legible). diff --git a/openspec/changes/host-agent-dependency-supervisor/specs/host-agent-dependency-supervisor/spec.md b/openspec/changes/host-agent-dependency-supervisor/specs/host-agent-dependency-supervisor/spec.md new file mode 100644 index 0000000..287581b --- /dev/null +++ b/openspec/changes/host-agent-dependency-supervisor/specs/host-agent-dependency-supervisor/spec.md @@ -0,0 +1,60 @@ +## ADDED Requirements + +### Requirement: Supervisor is opt-in and disabled by default +The Host Agent SHALL NOT start, adopt-check, or supervise Appium or the local Runtime API unless `HOST_AGENT_DEPENDENCY_SUPERVISOR_ENABLED` is explicitly set to true. Each of the two dependencies SHALL additionally have its own independent enable flag (`HOST_AGENT_APPIUM_SUPERVISED`, `HOST_AGENT_RUNTIME_SUPERVISED`), both defaulting to false. + +#### Scenario: Default configuration behaves exactly as before +- **WHEN** a Host Agent starts with no `HOST_AGENT_DEPENDENCY_SUPERVISOR_ENABLED` (or any related) environment variable set +- **THEN** the Host Agent does not attempt to connect to, probe, or spawn Appium or the Runtime API, and its heartbeat/claim behavior is unchanged from before this capability existed + +#### Scenario: Top-level flag on, individual dependency flag off +- **WHEN** `HOST_AGENT_DEPENDENCY_SUPERVISOR_ENABLED=true` and `HOST_AGENT_APPIUM_SUPERVISED=false` (Runtime supervised is true) +- **THEN** the Host Agent supervises only the Runtime API and does not probe, adopt, or spawn Appium + +### Requirement: Adopt an already-running, healthy dependency instead of spawning a duplicate +Before spawning a supervised dependency, the Host Agent SHALL attempt a TCP connection to its configured host/port, and if something is listening, SHALL perform the dependency-specific health check (Appium: HTTP GET to its status endpoint expecting a successful response; Runtime: HTTP GET to its health endpoint expecting a successful response). If the health check succeeds, the Host Agent SHALL treat the existing process as adopted, SHALL NOT spawn a subprocess for that dependency, and SHALL NOT restart or terminate the adopted process at any point in its lifecycle. + +#### Scenario: Appium already running and healthy +- **WHEN** Appium supervision is enabled and a healthy Appium server is already listening on the configured host/port +- **THEN** the Host Agent logs that it adopted the existing instance and does not spawn a new Appium process + +#### Scenario: Port occupied by something unhealthy or unrelated +- **WHEN** a supervised dependency's port has a listener that does not pass the dependency-specific health check +- **THEN** the Host Agent logs an error identifying the port conflict for that dependency and does not spawn a subprocess for it, and does not treat the dependency as available + +### Requirement: Spawn supervised dependencies that are not already running +When a dependency is enabled for supervision and no healthy instance is adopted, the Host Agent SHALL spawn it as a child process (Appium via `appium --address --port `; Runtime API via its existing `uvicorn api.rest:create_app --factory` entry point with the configured host/port), and SHALL forward the child process's stdout/stderr into the Host Agent's own logging, tagged by dependency name. + +#### Scenario: Neither dependency is running at Host Agent startup +- **WHEN** both Appium and Runtime supervision are enabled and neither has a healthy instance already listening +- **THEN** the Host Agent spawns both as child processes before proceeding to its first device-connect attempt, and both processes' output is visible in the Host Agent's logs + +#### Scenario: Spawn fails because the executable is missing +- **WHEN** the Host Agent attempts to spawn Appium but `appium` is not found on `PATH` +- **THEN** the Host Agent logs a dependency-supervisor-specific startup error naming the missing dependency, distinct from a runtime crash of an already-started process + +### Requirement: Restart only processes the supervisor itself spawned, with bounded backoff +The Host Agent SHALL restart a supervised dependency automatically only if the Host Agent's own child process handle for it exits unexpectedly. Restart attempts SHALL use capped exponential backoff and SHALL stop permanently for that dependency, for the remaining lifetime of the current Host Agent process, once a configured maximum attempt count (`HOST_AGENT_DEPENDENCY_RESTART_MAX_ATTEMPTS`) is reached. The Host Agent SHALL NOT restart or terminate a dependency instance it adopted rather than spawned. + +#### Scenario: Spawned Appium process crashes +- **WHEN** a Host Agent-spawned Appium child process exits unexpectedly and the per-dependency restart attempt count is below the configured maximum +- **THEN** the Host Agent waits the current backoff interval and attempts to spawn Appium again + +#### Scenario: Restart attempts exhausted +- **WHEN** a supervised dependency has crashed and been restarted until reaching `HOST_AGENT_DEPENDENCY_RESTART_MAX_ATTEMPTS` +- **THEN** the Host Agent logs that it has given up restarting that dependency and does not attempt to spawn it again for the rest of the current process lifetime + +#### Scenario: Adopted process exits +- **WHEN** a dependency instance the Host Agent adopted (did not spawn) stops running +- **THEN** the Host Agent does not attempt to restart it, since it never held a child process handle for it + +### Requirement: Supervisor lifecycle is tied to Host Agent process lifecycle +The Host Agent SHALL start enabled, not-yet-healthy supervised dependencies before beginning its normal device-connect/heartbeat/claim sequence, and SHALL stop any dependency processes it spawned (not ones it adopted) during its own graceful shutdown. + +#### Scenario: Host Agent shuts down gracefully +- **WHEN** the Host Agent receives a shutdown signal while it holds a child process handle for a spawned Appium instance +- **THEN** the Host Agent terminates the spawned Appium child process as part of its own shutdown sequence + +#### Scenario: Host Agent shuts down while an adopted dependency is running +- **WHEN** the Host Agent shuts down and Appium was adopted (not spawned) rather than spawned by this Host Agent +- **THEN** the adopted Appium process is left running, untouched, after the Host Agent exits diff --git a/openspec/changes/host-agent-dependency-supervisor/tasks.md b/openspec/changes/host-agent-dependency-supervisor/tasks.md new file mode 100644 index 0000000..536fd5e --- /dev/null +++ b/openspec/changes/host-agent-dependency-supervisor/tasks.md @@ -0,0 +1,47 @@ +## 1. Config + +- [x] 1.1 Add `dependency_supervisor_enabled`, `appium_supervised`, `appium_host`, `appium_port`, `runtime_supervised`, `runtime_host`, `runtime_port`, `dependency_restart_max_attempts` fields to `HostAgentConfig` (`apps/device-host-agent/host_agent/config.py`), reading from `HOST_AGENT_DEPENDENCY_SUPERVISOR_ENABLED` / `HOST_AGENT_APPIUM_SUPERVISED` / `HOST_AGENT_APPIUM_HOST` / `HOST_AGENT_APPIUM_PORT` / `HOST_AGENT_RUNTIME_SUPERVISED` / `HOST_AGENT_RUNTIME_HOST` / `HOST_AGENT_RUNTIME_PORT` / `HOST_AGENT_DEPENDENCY_RESTART_MAX_ATTEMPTS`, all defaulting per design.md Decision 6. +- [x] 1.2 Unit tests for the new config fields' defaults and env var parsing, following the existing pattern used for `console_*` fields in the config test module. + +## 2. Health probes and adoption + +- [x] 2.1 Implement a small health-check helper per dependency: TCP connect + Appium `GET /status` check, and TCP connect + Runtime API health endpoint check (reuse an existing HTTP client already available to the Host Agent rather than adding a new dependency). +- [x] 2.2 Implement the adopt-vs-spawn decision: probe before spawn, log and mark "adopted" on a passing health check, log a port-conflict error and skip on a listening-but-unhealthy port, proceed to spawn on no listener. +- [x] 2.3 Unit tests: adopt when healthy instance present, conflict-and-skip when unhealthy instance present, proceeds to spawn when nothing listening (mock the TCP/HTTP probe). + +## 3. Process supervisor core + +- [x] 3.1 Create `apps/device-host-agent/host_agent/dependency_supervisor.py` with a class managing zero or more supervised dependencies (Appium, Runtime), each described by: command, host/port, health-check callable, adopted-vs-spawned state. +- [x] 3.2 Implement spawn via `subprocess.Popen` for Appium (`appium --address --port `) and Runtime (`uvicorn api.rest:create_app --factory --host --port `), capturing stdout/stderr and forwarding to Host Agent logging tagged by dependency name. +- [x] 3.3 Implement post-spawn readiness wait: poll the health check until it passes or a startup timeout elapses, logging failure distinctly from a later crash. +- [x] 3.4 Implement crash-detection + capped exponential backoff restart loop (only for spawned, not adopted, processes), stopping permanently per dependency once `dependency_restart_max_attempts` is reached, per design.md Decision 4. +- [x] 3.5 Implement graceful stop: terminate only spawned child processes on supervisor shutdown; adopted processes are left untouched. +- [x] 3.6 Unit tests: spawn success, spawn failure (missing executable), crash-triggers-restart-with-backoff, restart-exhaustion-gives-up, adopted-process-never-restarted-or-killed, stop-terminates-only-spawned-children. + +## 4. Wiring into HostAgentApplication + +- [x] 4.1 In `apps/device-host-agent/host_agent/app.py`, construct and start the dependency supervisor (if `dependency_supervisor_enabled`) before the heartbeat loop's first `connect_devices()` pass. +- [x] 4.2 Stop the supervisor during `HostAgentApplication` shutdown/teardown alongside existing heartbeat/console teardown. +- [x] 4.3 Integration test covering: supervisor enabled with both dependencies off (no-op, unchanged existing behavior), supervisor enabled with only Appium supervised, start/stop ordering relative to heartbeat loop. + +## 5. Documentation + +- [x] 5.1 Update `docs/MACOS_IPHONE_SETUP.md` to document the new opt-in supervised mode (env vars, defaults, adopt-vs-spawn behavior, restart/backoff behavior) as an alternative to the existing manual multi-terminal flow, without removing the manual instructions. +- [x] 5.2 Update `.env.example` (if present) with the new `HOST_AGENT_*` variables, defaulted to off/disabled, matching existing `.env.example` conventions for other opt-in Host Agent features. + + > Note: the committed `.env.example` is scoped exclusively to `compose.deploy.yaml` Cloud-side variables (enforced by `tests/test_deployment_config.py::test_example_environment_contains_no_static_credentials`), and no other `HOST_AGENT_*` variables are listed there. Documenting the new variables in `docs/MACOS_IPHONE_SETUP.md` §9 instead matches the existing convention used for all other Host Agent opt-in features (e.g. `HOST_AGENT_CONSOLE_*`, `AI_PLANNER_*`), so no `.env.example` change was made. + +## 6. Validation + +- [x] 6.1 Run full non-integration test suite (`uv run --all-packages pytest -m "not integration"`) and confirm no regressions. + + Result: `503 passed, 2 failed, 44 deselected`. Both failures (`tests/test_deployment_config.py::test_deploy_compose_has_only_cloud_services_and_minimal_environment` and `::test_example_environment_contains_no_static_credentials`) are **pre-existing**, caused by unrelated commit `03c7c30 LLM_PROVIDER_ENC_KEY` (in-flight `database-llm-provider-management` work that added `CLOUD_LLM_PROVIDER_ENCRYPTION_KEY` to `.env.example` and `compose.deploy.yaml` without updating this test's allowlist). Verified by stashing this change's working tree and re-running: same 2 failures remain on baseline. The new `tests/test_dependency_supervisor.py` (19 tests) and updated `tests/test_config.py` / `tests/test_app.py` (38 combined) all pass. +- [x] 6.2 Ruff check/format and `compileall` on changed files. + + `ruff check` and `ruff format --check` clean on `apps/device-host-agent/host_agent/{config,dependency_supervisor,app}.py`, `apps/device-host-agent/tests/{test_config,test_dependency_supervisor,test_app}.py`. `python -m compileall -q` clean on the same set. +- [x] 6.3 `openspec validate --strict` for this change. + + Result: `Change 'host-agent-dependency-supervisor' is valid`. +- [ ] 6.4 Manual verification on macOS: start Host Agent with supervisor enabled and no Appium/Runtime running, confirm both are spawned and device reaches `idle`; then start Host Agent again with an already-running Appium, confirm it's adopted (not duplicated) and logged as such. + + **Deferred.** This session ran on Windows where the Appium/WebDriverAgent real-device workflow doesn't apply. To be executed on a macOS host per `docs/MACOS_IPHONE_SETUP.md` §9 once available.