diff --git a/stacklets/core/hooks/on_stop.py b/stacklets/core/hooks/on_stop.py index e0b61f0e..22d42079 100644 --- a/stacklets/core/hooks/on_stop.py +++ b/stacklets/core/hooks/on_stop.py @@ -1,14 +1,45 @@ -"""Stop the famstack API socket server.""" +"""Stop the famstack API server and take it out of the login items. +on_start writes the LaunchAgent with RunAtLoad on every up, so the file +only has to exist while core is up. Left in place, launchd would start +the API again at the next login while core reports down, and after a +destroy it would keep respawning a wrapper that no longer exists. +Removing it here covers down, destroy and uninstall, which all run +on_stop. + +Only the agent this instance wrote is touched. Another checkout or data +dir writes the same label with its own wrapper, and that one is left +loaded and in place. +""" + +import plistlib from pathlib import Path +from xml.parsers.expat import ExpatError PLIST_LABEL = "dev.famstack.api" def run(ctx): plist_path = Path.home() / "Library" / "LaunchAgents" / f"{PLIST_LABEL}.plist" - if plist_path.exists(): - try: - ctx.shell(f'launchctl unload "{plist_path}"') - except RuntimeError: - pass + # The wrapper on_start points the agent at. + wrapper = Path(ctx.stack.data) / "core" / "famstack-api" + if not _runs(plist_path, wrapper): + return + + try: + ctx.shell(f'launchctl unload "{plist_path}"') + except RuntimeError: + pass + plist_path.unlink() + ctx.step("famstack API stopped") + + +def _runs(plist_path: Path, wrapper: Path) -> bool: + """True when the agent at plist_path starts this instance's wrapper.""" + try: + with plist_path.open("rb") as f: + plist = plistlib.load(f) + except (OSError, ValueError, ExpatError): + # Missing or unreadable: nothing of ours to remove. + return False + return plist.get("ProgramArguments") == [str(wrapper)] diff --git a/tests/e2e/remote b/tests/e2e/remote index 63357d50..f1fde1bf 100755 --- a/tests/e2e/remote +++ b/tests/e2e/remote @@ -136,12 +136,6 @@ run_stacktests() { collect_logs say "==> stopping the rig" tests/e2e/stacktests down > "$results/down.log" 2>&1 || true - # core's on_start writes a LaunchAgent into the home dir and on_stop - # only unloads it. Remove ours so nothing outside the base dir remains. - local plist="$HOME/Library/LaunchAgents/dev.famstack.api.plist" - if [[ -f "$plist" ]] && grep -q "$base/" "$plist"; then - rm -f "$plist" - fi } # ── install ───────────────────────────────────────────────────────────── diff --git a/tests/unit/stacklets/test_core_launchagent.py b/tests/unit/stacklets/test_core_launchagent.py new file mode 100644 index 00000000..6f2e070e --- /dev/null +++ b/tests/unit/stacklets/test_core_launchagent.py @@ -0,0 +1,170 @@ +"""Core's LaunchAgent exists only while core is up. + +core's on_start writes `dev.famstack.api` into ~/Library/LaunchAgents +with RunAtLoad, so launchd starts the host API at every login. Once +core is down, destroyed or uninstalled, nothing may start it again: a +login after `down` would bring the API back while core reports down, +and one after `destroy` would respawn a wrapper whose data dir is gone. + +The hooks run through the framework's own down and destroy, against a +copy of the core stacklet and a temporary home. launchctl is a stub on +PATH that records its calls; the tests read what is left on disk for +launchd to find at the next login. +""" + +from __future__ import annotations + +import importlib.util +import plistlib +import shutil +import socket +from pathlib import Path + +import pytest + +from stack import Stack +from stack.hooks import build_hook_ctx + +REPO_ROOT = Path(__file__).resolve().parents[3] +CORE = REPO_ROOT / "stacklets" / "core" + +_spec = importlib.util.spec_from_file_location("core_on_start", CORE / "hooks" / "on_start.py") +on_start = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(on_start) + + +# ── Fixtures ────────────────────────────────────────────────────────────── + +@pytest.fixture +def home(tmp_path, monkeypatch): + home = tmp_path / "home" + monkeypatch.setattr(Path, "home", lambda: home) + return home + + +@pytest.fixture +def launchctl(tmp_path, monkeypatch): + """A launchctl on PATH that only records its arguments.""" + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + log = tmp_path / "launchctl.log" + log.touch() + stub = bin_dir / "launchctl" + stub.write_text(f'#!/bin/sh\necho "$*" >> "{log}"\n') + stub.chmod(0o755) + monkeypatch.setenv("PATH", f"{bin_dir}:/usr/bin:/bin") + return log + + +@pytest.fixture(autouse=True) +def api_answering(monkeypatch): + """Something listening where on_start probes for the API. + + on_start then takes its "already running" path instead of waiting + five seconds for an API that launchctl, stubbed, never starts. + """ + with socket.socket() as listener: + listener.bind(("127.0.0.1", 0)) + listener.listen() + monkeypatch.setattr(on_start, "API_PORT", listener.getsockname()[1]) + yield + + +@pytest.fixture +def repo(tmp_path): + """A checkout holding a copy of core, so destroy removes nothing real.""" + repo = tmp_path / "repo" + shutil.copytree(CORE, repo / "stacklets" / "core", + ignore=shutil.ignore_patterns(".env", "__pycache__")) + return repo + + +def _instance(tmp_path, repo, name) -> Stack: + instance = tmp_path / name + (instance / ".stack").mkdir(parents=True) + (instance / ".stack" / "secrets.toml").write_text("") + data = tmp_path / f"{name}-data" + (instance / "stack.toml").write_text(f'[core]\ndomain = ""\ndata_dir = "{data}"\n') + return Stack(root=repo, data=data, instance_dir=instance) + + +@pytest.fixture +def stack(tmp_path, repo, home, launchctl): + return _instance(tmp_path, repo, "instance") + + +def _up(stack: Stack): + on_start.run(build_hook_ctx("core", stack=stack)) + + +def _plist(home: Path) -> Path: + return home / "Library" / "LaunchAgents" / "dev.famstack.api.plist" + + +def _calls(launchctl: Path) -> list[str]: + return launchctl.read_text().splitlines() + + +# ── Behaviour ───────────────────────────────────────────────────────────── + +def test_up_leaves_an_agent_that_starts_at_login(stack, home): + _up(stack) + + agent = plistlib.loads(_plist(home).read_bytes()) + assert agent["RunAtLoad"] is True + assert agent["ProgramArguments"] == [str(stack.data / "core" / "famstack-api")] + + +def test_down_unloads_the_agent_and_leaves_nothing_for_the_next_login(stack, home, launchctl): + _up(stack) + + stack.down("core") + + assert f"unload {_plist(home)}" in _calls(launchctl) + assert not _plist(home).exists() + + +def test_up_after_down_brings_the_agent_back(stack, home): + _up(stack) + stack.down("core") + + _up(stack) + + assert plistlib.loads(_plist(home).read_bytes())["RunAtLoad"] is True + + +def test_destroy_leaves_no_agent_behind(stack, home): + """uninstall destroys each stacklet, so this covers it as well.""" + _up(stack) + + stack.destroy("core") + + assert not (stack.data / "core").exists() + assert not _plist(home).exists() + + +def test_repeating_down_and_destroy_changes_nothing(stack, home, launchctl): + _up(stack) + stack.down("core") + launchctl.write_text("") + + assert stack.down("core")["ok"] + assert stack.destroy("core")["ok"] + assert stack.destroy("core")["ok"] + + assert _calls(launchctl) == [] + assert not _plist(home).exists() + + +def test_the_agent_another_instance_wrote_is_left_alone(stack, tmp_path, repo, home, launchctl): + """Same label, other data dir: that API belongs to the other instance.""" + _up(stack) + other = _instance(tmp_path, repo, "other") + _up(other) + written = _plist(home).read_bytes() + + stack.down("core") + stack.destroy("core") + + assert _plist(home).read_bytes() == written + assert _calls(launchctl) == []