diff --git a/src/macos_mcp/__main__.py b/src/macos_mcp/__main__.py index c8ef076..75d1b7f 100644 --- a/src/macos_mcp/__main__.py +++ b/src/macos_mcp/__main__.py @@ -46,6 +46,7 @@ import signal import subprocess import sys +import time from threading import Lock import click @@ -907,15 +908,33 @@ def auth(transport: str, host: str, port: int, with_tls: bool, force: bool) -> N _PLIST_PATH = _LAUNCH_AGENTS_DIR / f"{_AGENT_LABEL}.plist" +def _port_available(host: str, port: int) -> bool: + """Return whether a TCP port can be bound on host.""" + try: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.bind((host, port)) + except OSError: + return False + return True + + +def _wait_for_port_available(host: str, port: int, timeout: float = 2.0) -> bool: + """Wait briefly for a recently stopped server to release its TCP port.""" + deadline = time.monotonic() + timeout + while True: + if _port_available(host, port): + return True + now = time.monotonic() + if now >= deadline: + return False + time.sleep(min(0.1, deadline - now)) + + def _find_free_port(host: str, preferred: int, max_tries: int = 100) -> int: """Return the first free TCP port at or above preferred on host.""" for port in range(preferred, preferred + max_tries): - try: - with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: - s.bind((host, port)) - return port - except OSError: - continue + if _port_available(host, port): + return port raise click.ClickException( f"No free port found in range {preferred}–{preferred + max_tries - 1} on {host}." ) @@ -955,6 +974,164 @@ def _launchctl(*args: str) -> subprocess.CompletedProcess: return subprocess.run(["launchctl", *args], capture_output=True, text=True) +def _launchctl_field(output: str, field: str) -> str | None: + """Return a top-level field from `launchctl print` output.""" + prefix = f"{field} = " + depth = 0 + for line in output.splitlines(): + stripped = line.strip() + if depth == 1 and stripped.startswith(prefix): + return stripped[len(prefix) :] + depth += line.count("{") - line.count("}") + return None + + +def _server_accepting_connections(host: str, port: int) -> bool: + """Return whether the configured server endpoint accepts TCP connections.""" + probe_host = {"0.0.0.0": "127.0.0.1", "::": "::1"}.get(host, host) + try: + with socket.create_connection((probe_host, port), timeout=0.1): + return True + except OSError: + return False + + +def _launch_agent_loaded(domain: str, timeout: float = 0.5) -> bool: + """Check whether the existing agent is loaded, tolerating transient print failures.""" + deadline = time.monotonic() + timeout + while True: + result = _launchctl("print", f"{domain}/{_AGENT_LABEL}") + if result.returncode == 0: + return True + now = time.monotonic() + if now >= deadline: + return False + time.sleep(min(0.1, deadline - now)) + + +def _wait_for_launch_agent_start( + domain: str, host: str, port: int, timeout: float = 10.0 +) -> tuple[bool, str]: + """Wait for a freshly bootstrapped agent to reach its listen endpoint.""" + deadline = time.monotonic() + timeout + saw_running = False + last_print_error = "launchctl print could not find the service" + + while True: + result = _launchctl("print", f"{domain}/{_AGENT_LABEL}") + if result.returncode == 0: + state = _launchctl_field(result.stdout, "state") + last_exit = _launchctl_field(result.stdout, "last exit code") + last_signal = _launchctl_field(result.stdout, "last terminating signal") + if last_exit and last_exit != "(never exited)": + return False, f"state={state or 'unknown'}, last exit code={last_exit}" + if last_signal: + return False, f"state={state or 'unknown'}, last terminating signal={last_signal}" + if state == "running": + saw_running = True + if _server_accepting_connections(host, port): + return True, "accepting connections" + last_print_error = f"state={state or 'unknown'}" + else: + last_print_error = ( + result.stderr.strip() or "launchctl print could not find the service" + ) + + now = time.monotonic() + if now >= deadline: + break + time.sleep(min(0.1, deadline - now)) + + if saw_running: + return False, "process ran but endpoint did not accept connections before timeout" + return False, last_print_error + + +def _wait_for_launch_agent_running( + domain: str, timeout: float = 10.0 +) -> tuple[bool, str]: + """Wait for a restored launch agent to report a running state.""" + deadline = time.monotonic() + timeout + last_detail = "launchctl print could not find the service" + + while True: + result = _launchctl("print", f"{domain}/{_AGENT_LABEL}") + if result.returncode == 0: + state = _launchctl_field(result.stdout, "state") + last_exit = _launchctl_field(result.stdout, "last exit code") + last_signal = _launchctl_field(result.stdout, "last terminating signal") + if last_exit and last_exit != "(never exited)": + return False, f"state={state or 'unknown'}, last exit code={last_exit}" + if last_signal: + return False, f"state={state or 'unknown'}, last terminating signal={last_signal}" + if state == "running": + return True, "running" + last_detail = f"state={state or 'unknown'}" + else: + last_detail = result.stderr.strip() or last_detail + + now = time.monotonic() + if now >= deadline: + return False, last_detail + time.sleep(min(0.1, deadline - now)) + + +def _wait_for_launch_agent_unloaded( + domain: str, timeout: float = 2.0 +) -> tuple[bool, str]: + """Wait until launchd no longer reports the agent after bootout.""" + deadline = time.monotonic() + timeout + last_state = "unknown" + + while True: + result = _launchctl("print", f"{domain}/{_AGENT_LABEL}") + if result.returncode != 0: + return True, "unloaded" + last_state = _launchctl_field(result.stdout, "state") or "unknown" + now = time.monotonic() + if now >= deadline: + return False, f"state={last_state}" + time.sleep(min(0.1, deadline - now)) + + +def _rollback_launch_agent( + domain: str, + previous_plist: bytes | None, + previous_loaded: bool, + bootstrap_attempted: bool, +) -> str | None: + """Restore the pre-install launchd state after a failed install transaction.""" + errors: list[str] = [] + + if bootstrap_attempted: + _launchctl("bootout", f"{domain}/{_AGENT_LABEL}") + unloaded, detail = _wait_for_launch_agent_unloaded(domain) + if not unloaded: + errors.append(f"new launch agent did not unload ({detail})") + + if previous_plist is None: + _PLIST_PATH.unlink(missing_ok=True) + return "; ".join(errors) or None + + try: + _PLIST_PATH.write_bytes(previous_plist) + except OSError as exc: + errors.append(f"could not restore previous plist: {exc}") + return "; ".join(errors) + + if previous_loaded: + restored = _launchctl("bootstrap", domain, str(_PLIST_PATH)) + if restored.returncode != 0: + detail = restored.stderr.strip() or f"exit status {restored.returncode}" + errors.append(f"could not restore previous launch agent: {detail}") + else: + running, detail = _wait_for_launch_agent_running(domain) + if not running: + errors.append(f"restored launch agent did not reach running state ({detail})") + + return "; ".join(errors) or None + + @main.command() @click.option( "--transport", @@ -973,38 +1150,81 @@ def install(transport: str, host: str, port: int, force: bool) -> None: click.echo("Use --force to reinstall.") return - # Auto-select a free port when the user didn't explicitly pass --port. - ctx = click.get_current_context() - if ctx.get_parameter_source("port") == click.core.ParameterSource.DEFAULT: - selected = _find_free_port(host, port) - if selected != port: - click.echo(f"Port {port} is in use — using {selected} instead.") - port = selected - CONFIG_DIR.mkdir(parents=True, exist_ok=True) _LAUNCH_AGENTS_DIR.mkdir(parents=True, exist_ok=True) exe = _resolve_program() - args = exe + ["serve", "--transport", transport, "--host", host, "--port", str(port)] - _PLIST_PATH.write_text(_build_plist(args)) - click.echo(f"Wrote {_PLIST_PATH}") - uid = os.getuid() domain = f"gui/{uid}" + previous_plist = _PLIST_PATH.read_bytes() if _PLIST_PATH.exists() else None + previous_loaded = ( + _launch_agent_loaded(domain) if force and previous_plist is not None else False + ) + bootstrap_attempted = False - # Unload first if already running (needed for --force) - _launchctl("bootout", f"{domain}/{_AGENT_LABEL}") + try: + if previous_loaded: + result = _launchctl("bootout", f"{domain}/{_AGENT_LABEL}") + if result.returncode != 0: + detail = result.stderr.strip() or f"exit status {result.returncode}" + raise click.ClickException(f"launchctl bootout failed:\n{detail}") + unloaded, detail = _wait_for_launch_agent_unloaded(domain) + if not unloaded: + raise click.ClickException(f"launch agent did not unload ({detail})") + _wait_for_port_available(host, port) + + # Auto-select a free port when the user didn't explicitly pass --port. + ctx = click.get_current_context() + if ctx.get_parameter_source("port") == click.core.ParameterSource.DEFAULT: + selected = _find_free_port(host, port) + if selected != port: + click.echo(f"Port {port} is in use — using {selected} instead.") + port = selected + elif not _port_available(host, port): + raise click.ClickException(f"Port {port} is already in use on {host}.") + + args = exe + [ + "serve", + "--transport", + transport, + "--host", + host, + "--port", + str(port), + ] + _PLIST_PATH.write_text(_build_plist(args)) + click.echo(f"Wrote {_PLIST_PATH}") + + bootstrap_attempted = True + result = _launchctl("bootstrap", domain, str(_PLIST_PATH)) + if result.returncode != 0: + raise click.ClickException( + f"launchctl bootstrap failed:\n{result.stderr.strip()}" + ) - result = _launchctl("bootstrap", domain, str(_PLIST_PATH)) - if result.returncode != 0: - raise click.ClickException(f"launchctl bootstrap failed:\n{result.stderr.strip()}") + started, detail = _wait_for_launch_agent_start(domain, host, port) + if not started: + raise click.ClickException( + "Launch agent loaded but the server did not start " + f"({detail}).\nCheck {CONFIG_DIR / 'server.error.log'} for startup errors." + ) + except Exception as exc: + rollback_error = _rollback_launch_agent( + domain, + previous_plist, + previous_loaded, + bootstrap_attempted, + ) + if rollback_error: + raise click.ClickException(f"{exc}\nRollback failed: {rollback_error}") from exc + raise - click.echo(f"Launch agent loaded — server is starting now.") + click.echo("Launch agent loaded — accepting connections.") click.echo(f" Transport : {transport}") click.echo(f" Address : {host}:{port}") click.echo(f" Logs : {CONFIG_DIR / 'server.log'}") - click.echo(f"\nThe server will restart automatically at every login.") - click.echo(f"Run `macos-mcp uninstall` to remove it.") + click.echo("\nThe server will restart automatically at every login.") + click.echo("Run `macos-mcp uninstall` to remove it.") @main.command() diff --git a/tests/test_launchd_install.py b/tests/test_launchd_install.py new file mode 100644 index 0000000..1f67dd0 --- /dev/null +++ b/tests/test_launchd_install.py @@ -0,0 +1,345 @@ +from types import SimpleNamespace + +from click.testing import CliRunner + +import macos_mcp.__main__ as server + + +def _completed(returncode=0, stdout="", stderr=""): + return SimpleNamespace(returncode=returncode, stdout=stdout, stderr=stderr) + + +def _launchd_output(state="running", last_exit="(never exited)", signal=None): + lines = [ + "gui/501/com.macos-mcp.server = {", + f" state = {state}", + f" last exit code = {last_exit}", + ] + if signal is not None: + lines.append(f" last terminating signal = {signal}") + lines.append("}") + return "\n".join(lines) + + +class FakeClock: + def __init__(self): + self.now = 0.0 + + def monotonic(self): + return self.now + + def sleep(self, duration): + self.now += duration + + +def _patch_clock(mocker): + clock = FakeClock() + mocker.patch.object(server.time, "monotonic", side_effect=clock.monotonic) + mocker.patch.object(server.time, "sleep", side_effect=clock.sleep) + return clock + + +def _patch_install_paths(mocker, tmp_path, previous=None): + config_dir = tmp_path / "config" + agents_dir = tmp_path / "LaunchAgents" + plist_path = agents_dir / "com.macos-mcp.server.plist" + agents_dir.mkdir(parents=True) + if previous is not None: + plist_path.write_bytes(previous) + mocker.patch.object(server, "CONFIG_DIR", config_dir) + mocker.patch.object(server, "_LAUNCH_AGENTS_DIR", agents_dir) + mocker.patch.object(server, "_PLIST_PATH", plist_path) + mocker.patch.object(server, "_resolve_program", return_value=["/tmp/macos-mcp"]) + return plist_path + + +def test_launchctl_field_ignores_nested_state(): + output = """gui/501/com.macos-mcp.server = { + state = running + endpoints = { + state = active + } + last exit code = (never exited) +} +""" + + assert server._launchctl_field(output, "state") == "running" + assert server._launchctl_field(output, "last exit code") == "(never exited)" + + +def test_wait_for_launch_agent_accepts_delayed_healthy_endpoint(mocker): + clock = _patch_clock(mocker) + mocker.patch.object( + server, + "_launchctl", + return_value=_completed(stdout=_launchd_output()), + ) + mocker.patch.object( + server, + "_server_accepting_connections", + side_effect=lambda host, port: clock.now >= 3.4, + ) + + started, detail = server._wait_for_launch_agent_start( + "gui/501", "127.0.0.1", 8000, timeout=10.0 + ) + + assert started is True + assert detail == "accepting connections" + assert 3.4 <= clock.now < 3.6 + + +def test_wait_for_launch_agent_reports_delayed_exit(mocker): + clock = _patch_clock(mocker) + + def launchctl(*args): + if clock.now < 3.8: + return _completed(stdout=_launchd_output()) + return _completed(stdout=_launchd_output("spawn scheduled", "3")) + + mocker.patch.object(server, "_launchctl", side_effect=launchctl) + mocker.patch.object(server, "_server_accepting_connections", return_value=False) + + started, detail = server._wait_for_launch_agent_start( + "gui/501", "127.0.0.1", 8000, timeout=10.0 + ) + + assert started is False + assert "last exit code=3" in detail + assert 3.8 <= clock.now < 4.0 + + +def test_wait_for_launch_agent_times_out_without_endpoint(mocker): + clock = _patch_clock(mocker) + mocker.patch.object( + server, + "_launchctl", + return_value=_completed(stdout=_launchd_output()), + ) + mocker.patch.object(server, "_server_accepting_connections", return_value=False) + + started, detail = server._wait_for_launch_agent_start( + "gui/501", "127.0.0.1", 8000, timeout=0.5 + ) + + assert started is False + assert detail == "process ran but endpoint did not accept connections before timeout" + assert clock.now == 0.5 + + +def test_wait_for_launch_agent_retries_transient_print_failure(mocker): + clock = _patch_clock(mocker) + launchctl = mocker.patch.object( + server, + "_launchctl", + side_effect=[ + _completed(returncode=1, stderr="temporary launchctl failure"), + _completed(stdout=_launchd_output()), + ], + ) + mocker.patch.object(server, "_server_accepting_connections", return_value=True) + + started, detail = server._wait_for_launch_agent_start( + "gui/501", "127.0.0.1", 8000, timeout=1.0 + ) + + assert started is True + assert detail == "accepting connections" + assert launchctl.call_count == 2 + assert clock.now == 0.1 + + +def test_launch_agent_loaded_retries_transient_print_failure(mocker): + clock = _patch_clock(mocker) + launchctl = mocker.patch.object( + server, + "_launchctl", + side_effect=[ + _completed(returncode=1, stderr="temporary launchctl failure"), + _completed(stdout=_launchd_output()), + ], + ) + + loaded = server._launch_agent_loaded("gui/501", timeout=0.5) + + assert loaded is True + assert launchctl.call_count == 2 + assert clock.now == 0.1 + + +def test_wait_for_launch_agent_reports_signal_termination(mocker): + mocker.patch.object( + server, + "_launchctl", + return_value=_completed( + stdout=_launchd_output("spawn scheduled", signal="Terminated: 15") + ), + ) + listener = mocker.patch.object( + server, "_server_accepting_connections", return_value=False + ) + + started, detail = server._wait_for_launch_agent_start( + "gui/501", "127.0.0.1", 8000, timeout=0 + ) + + assert started is False + assert "last terminating signal=Terminated: 15" in detail + listener.assert_not_called() + + +def test_wait_for_launch_agent_does_not_accept_unrelated_listener(mocker): + mocker.patch.object( + server, + "_launchctl", + return_value=_completed(stdout=_launchd_output("not running")), + ) + listener = mocker.patch.object( + server, "_server_accepting_connections", return_value=True + ) + + started, detail = server._wait_for_launch_agent_start( + "gui/501", "127.0.0.1", 8000, timeout=0 + ) + + assert started is False + assert detail == "state=not running" + listener.assert_not_called() + + +def test_wait_for_launch_agent_unloaded_waits_through_sigtermed(mocker): + clock = _patch_clock(mocker) + launchctl = mocker.patch.object( + server, + "_launchctl", + side_effect=[ + _completed(stdout=_launchd_output("SIGTERMed")), + _completed(returncode=1, stderr="Could not find service"), + ], + ) + + unloaded, detail = server._wait_for_launch_agent_unloaded( + "gui/501", timeout=1.0 + ) + + assert unloaded is True + assert detail == "unloaded" + assert launchctl.call_count == 2 + assert clock.now == 0.1 + + +def test_install_rejects_explicit_port_conflict(mocker, tmp_path): + plist_path = _patch_install_paths(mocker, tmp_path) + mocker.patch.object(server, "_port_available", return_value=False) + launchctl = mocker.patch.object(server, "_launchctl") + + result = CliRunner().invoke(server.main, ["install", "--port", "18133"]) + + assert result.exit_code != 0 + assert "Port 18133 is already in use on 127.0.0.1." in result.output + assert not plist_path.exists() + launchctl.assert_not_called() + + +def test_fresh_install_failure_cleans_up_job_and_plist(mocker, tmp_path): + plist_path = _patch_install_paths(mocker, tmp_path) + mocker.patch.object(server, "_port_available", return_value=True) + mocker.patch.object( + server, + "_wait_for_launch_agent_start", + return_value=(False, "state=spawn scheduled, last exit code=3"), + ) + mocker.patch.object( + server, "_wait_for_launch_agent_unloaded", return_value=(True, "unloaded") + ) + launchctl = mocker.patch.object(server, "_launchctl", return_value=_completed()) + + result = CliRunner().invoke(server.main, ["install", "--port", "18133"]) + + assert result.exit_code != 0 + assert not plist_path.exists() + assert any(call.args[0] == "bootout" for call in launchctl.call_args_list) + + +def test_force_same_port_waits_for_old_listener_to_release(mocker, tmp_path): + _patch_install_paths(mocker, tmp_path, previous=b"old plist") + mocker.patch.object(server, "_launch_agent_loaded", return_value=True) + mocker.patch.object( + server, "_wait_for_launch_agent_unloaded", return_value=(True, "unloaded") + ) + wait_port = mocker.patch.object( + server, "_wait_for_port_available", return_value=True + ) + mocker.patch.object(server, "_port_available", return_value=True) + mocker.patch.object( + server, + "_wait_for_launch_agent_start", + return_value=(True, "accepting connections"), + ) + mocker.patch.object(server, "_launchctl", return_value=_completed()) + + result = CliRunner().invoke( + server.main, ["install", "--force", "--port", "18134"] + ) + + assert result.exit_code == 0 + wait_port.assert_called_once_with("127.0.0.1", 18134) + + +def test_force_failure_restores_old_plist_without_loading(mocker, tmp_path): + previous = b"old plist bytes" + plist_path = _patch_install_paths(mocker, tmp_path, previous=previous) + mocker.patch.object(server, "_launch_agent_loaded", return_value=False) + mocker.patch.object(server, "_port_available", return_value=True) + mocker.patch.object( + server, + "_wait_for_launch_agent_start", + return_value=(False, "state=spawn scheduled, last exit code=3"), + ) + mocker.patch.object( + server, "_wait_for_launch_agent_unloaded", return_value=(True, "unloaded") + ) + launchctl = mocker.patch.object(server, "_launchctl", return_value=_completed()) + + result = CliRunner().invoke( + server.main, ["install", "--force", "--port", "18134"] + ) + + assert result.exit_code != 0 + assert plist_path.read_bytes() == previous + bootstrap_calls = [ + call for call in launchctl.call_args_list if call.args[0] == "bootstrap" + ] + assert len(bootstrap_calls) == 1 + + +def test_force_failure_restores_and_restarts_loaded_service(mocker, tmp_path): + previous = b"old loaded plist bytes" + plist_path = _patch_install_paths(mocker, tmp_path, previous=previous) + mocker.patch.object(server, "_launch_agent_loaded", return_value=True) + mocker.patch.object(server, "_wait_for_port_available", return_value=True) + mocker.patch.object(server, "_port_available", return_value=True) + mocker.patch.object( + server, "_wait_for_launch_agent_unloaded", return_value=(True, "unloaded") + ) + mocker.patch.object( + server, + "_wait_for_launch_agent_start", + return_value=(False, "state=spawn scheduled, last exit code=3"), + ) + running = mocker.patch.object( + server, "_wait_for_launch_agent_running", return_value=(True, "running") + ) + launchctl = mocker.patch.object(server, "_launchctl", return_value=_completed()) + + result = CliRunner().invoke( + server.main, ["install", "--force", "--port", "18134"] + ) + + assert result.exit_code != 0 + assert plist_path.read_bytes() == previous + bootstrap_calls = [ + call for call in launchctl.call_args_list if call.args[0] == "bootstrap" + ] + assert len(bootstrap_calls) == 2 + running.assert_called_once()