diff --git a/CHANGELOG.md b/CHANGELOG.md index 73d1fef96..d7d516500 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Format adapted from [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), al - the settings page's data included whether cloud thinking keys were set. it no longer does; thinking is the only place that holds provider configuration. - pointing thinking at your own compatible endpoint is more dependable: requests no longer carry local-only fields some endpoints rejected, and an endpoint that can't be reached now says so quickly instead of timing out. - the bundled local model now downloads at full speed on first setup; some installs saw it crawl at a fraction of their connection's speed. +- `journal doctor` now detects when the macOS app and the legacy background service both target one journal, and points to the single service removal step before other repair actions. ## [0.8.7] - 2026-07-15 diff --git a/docs/DOCTOR.md b/docs/DOCTOR.md index c7955e16b..e7093e44d 100644 --- a/docs/DOCTOR.md +++ b/docs/DOCTOR.md @@ -54,11 +54,12 @@ Use the diagnostic command that matches the question: | `disk_space` | advisory | Free-space warning. | | `config_dir_readable` | blocker | Home and service config directory permissions. | | `journal_dir_writable` | blocker | Journal directory writability when the local journal exists. | +| `supervisor_conflict` | blocker | macOS only; detects journal.app and the legacy LaunchAgent both supervising one journal. | | `service_identity` | blocker | Installed service points at this install. | | `service_running` | blocker | Service installed/running/crash-loop diagnosis. | | `journal_sync` | blocker | Concurrent-writer conflict check. | | `stale_alias_symlink` | blocker | Checks only the `journal` wrapper; stale aliases warn, never block, and `journal setup` repairs them. | -| `launchd_stale_plist` | advisory | macOS only; Linux skips it. | +| `launchd_stale_plist` | advisory | macOS only; stale legacy service plists should be removed with `journal service uninstall`, then repaired with `journal service install` only on a confirmed headless host. | | `feature:pdf-import`, `feature:pdf-export`, `feature:whisper` | advisory | Optional extras with exact install commands. | `host_dependencies` fix guidance is: Reinstall the journal host stack: @@ -72,6 +73,15 @@ are blocker failures. An installed service with no supervisor socket is a warning when the OS unit is not failed. Host dependency and feature checks report missing journal-host packaging pieces directly. +On macOS, `supervisor_conflict` fails when `journal.app` is running while the +legacy `org.solpbc.solstone` LaunchAgent is installed or loaded. The proven +conflict remediation is `journal service uninstall`. In that state, other +diagnoses stay visible but their action strings point back to resolving the +supervisor conflict first, so the report does not mix service creation, +restart, setup, upgrade, or deletion advice with the single conflict fix. If the +topology is unknown rather than proven, only service lifecycle actions are +withheld until the topology can be determined. + `journal setup` step 1 runs `journal doctor --readiness`: the client readiness checks (`python_version`, `sol_importable`, `local_bin_sol_reachable`, `stale_alias_symlink`, `disk_space`, `journal_dir_writable`) plus diff --git a/docs/SOLCLI.md b/docs/SOLCLI.md index 71fe3ace9..ba20590d3 100644 --- a/docs/SOLCLI.md +++ b/docs/SOLCLI.md @@ -273,10 +273,16 @@ instead of false failures. Its battery is: - `disk_space` — advisory. - `host_dependencies`, `config_dir_readable`, `journal_dir_writable`, - `service_identity`, `service_running`, `journal_sync`, + `supervisor_conflict`, `service_identity`, `service_running`, `journal_sync`, `stale_alias_symlink` — blockers. Stale `journal` aliases warn, never block, and `journal setup` repairs them. -- `launchd_stale_plist` — advisory on macOS; skipped on Linux. +- `supervisor_conflict` — macOS only; fails when journal.app and the legacy + LaunchAgent are both supervising one journal. The proven-conflict action is + `journal service uninstall`; other diagnoses remain visible with their actions + withheld until the topology is resolved. +- `launchd_stale_plist` — advisory on macOS; skipped on Linux. It advises + removing the legacy service first and reinstalling the headless service only + as a separate step. - `feature:pdf-import`, `feature:pdf-export`, `feature:whisper` — advisories with the exact extra-install command when missing. diff --git a/solstone/think/doctor.py b/solstone/think/doctor.py index 336d435a7..91b8c980a 100644 --- a/solstone/think/doctor.py +++ b/solstone/think/doctor.py @@ -36,7 +36,7 @@ import plistlib import re import sys import time -from dataclasses import dataclass +from dataclasses import dataclass, replace from functools import partial from importlib.metadata import PackageNotFoundError, distribution from importlib.metadata import version as _pkg_version @@ -70,6 +70,7 @@ from solstone.think.probe import ( ) from solstone.think.service import ( check_service_target_identity, + inspect_supervisor_conflict, service_is_failed, service_is_installed, ) @@ -118,6 +119,7 @@ HOST_DEPENDENCY_REINSTALL_GUIDANCE = ( ) SERVICE_IDENTITY_CHECK = Check("service_identity", "blocker", ("linux", "darwin")) SERVICE_RUNNING_CHECK = Check("service_running", "blocker", ("linux", "darwin")) +SUPERVISOR_CONFLICT_CHECK = Check("supervisor_conflict", "blocker", ("darwin",)) LAUNCHD_STALE_PLIST_CHECK = Check("launchd_stale_plist", "advisory", ("darwin",)) JOURNAL_SYNC_CHECK = Check("journal_sync", "blocker", ("linux", "darwin")) DEFAULT_STT_READY_CHECK = Check("default_stt_ready", "advisory", ("linux", "darwin")) @@ -161,6 +163,23 @@ _ORPHAN_SEGMENT_PDF_FIX = ( "journal maint --force settings:007_migrate_pdf_extractions, " "then re-run journal doctor" ) +_SUPERVISOR_CONFLICT_FIX = "journal service uninstall" +_SUPERVISOR_CONFLICT_FIX_POINTER = ( + "resolve the macOS supervisor conflict first: journal service uninstall" +) +_SUPERVISOR_TOPOLOGY_WARN_POINTER = ( + "resolve the macOS supervisor topology warning before changing the journal service" +) +_UNSAFE_SERVICE_ACTIONS = ( + "journal setup", + "journal service install", + "journal service start", + "journal service restart", +) +_LAUNCHD_STALE_PLIST_REPAIR_FIX = ( + "run journal service uninstall, then run journal service install separately " + "to reinstall a headless background service" +) def python_sanity_check(args: Args) -> CheckResult: @@ -543,6 +562,40 @@ def service_running_check(args: Args) -> CheckResult: return make_result(SERVICE_RUNNING_CHECK, "ok", "journal service is running") +def _format_unknown_axes(axes: tuple[str, ...]) -> str: + return ", ".join(axes) + + +def supervisor_conflict_check(args: Args) -> CheckResult: + del args + evidence = inspect_supervisor_conflict() + if evidence.is_conflict: + return make_result( + SUPERVISOR_CONFLICT_CHECK, + "fail", + ( + "macOS supervisor conflict: journal.app is running while the legacy " + f"LaunchAgent is installed or loaded ({evidence.detail})" + ), + _SUPERVISOR_CONFLICT_FIX, + ) + unknown_axes = evidence.unknown_axes + if unknown_axes: + return make_result( + SUPERVISOR_CONFLICT_CHECK, + "warn", + ( + "couldn't fully determine macOS supervisor topology; unknown " + f"axis(es): {_format_unknown_axes(unknown_axes)} ({evidence.detail})" + ), + ) + return make_result( + SUPERVISOR_CONFLICT_CHECK, + "ok", + f"no macOS supervisor conflict ({evidence.detail})", + ) + + def import_install_guard() -> tuple[object, object]: root_text = str(ROOT) if root_text not in sys.path: @@ -748,7 +801,7 @@ def launchd_stale_plist_check(args: Args) -> CheckResult: check, "fail", f"could not parse plist: {type(exc).__name__}: {exc}", - "rm ~/Library/LaunchAgents/org.solpbc.solstone.plist && journal setup", + _LAUNCHD_STALE_PLIST_REPAIR_FIX, ) program_arguments = data.get("ProgramArguments") if not isinstance(program_arguments, list) or not program_arguments: @@ -756,7 +809,7 @@ def launchd_stale_plist_check(args: Args) -> CheckResult: check, "fail", "plist is missing ProgramArguments[0]", - "rm ~/Library/LaunchAgents/org.solpbc.solstone.plist && journal setup", + _LAUNCHD_STALE_PLIST_REPAIR_FIX, ) executable = Path(str(program_arguments[0])) if not executable.exists(): @@ -764,7 +817,7 @@ def launchd_stale_plist_check(args: Args) -> CheckResult: check, "fail", f"plist points to missing executable: {executable}", - "rm ~/Library/LaunchAgents/org.solpbc.solstone.plist && journal setup", + _LAUNCHD_STALE_PLIST_REPAIR_FIX, ) return make_result(check, "ok", f"launchd plist target exists ({executable})") @@ -1103,6 +1156,7 @@ JOURNAL_CHECKS: list[tuple[Check, Runner]] = [ (DISK_SPACE_CHECK, disk_space_check), (CONFIG_DIR_READABLE_CHECK, config_dir_readable_check), (JOURNAL_DIR_WRITABLE_CHECK, journal_dir_writable_journal), + (SUPERVISOR_CONFLICT_CHECK, supervisor_conflict_check), (SERVICE_IDENTITY_CHECK, service_identity_check), (SERVICE_RUNNING_CHECK, service_running_check), (JOURNAL_SYNC_CHECK, journal_sync_check), @@ -1200,6 +1254,35 @@ def select_battery(args: Args) -> list[tuple[Check, Runner]]: return UNIVERSAL_CHECKS +def _fix_mentions_unsafe_service_action(fix: str) -> bool: + return any(action in fix for action in _UNSAFE_SERVICE_ACTIONS) + + +def _apply_supervisor_conflict_fix_policy( + results: list[CheckResult], +) -> list[CheckResult]: + conflict = next( + (result for result in results if result.name == SUPERVISOR_CONFLICT_CHECK.name), + None, + ) + if conflict is None or conflict.status not in {"fail", "warn"}: + return results + + updated: list[CheckResult] = [] + for result in results: + if result.name == SUPERVISOR_CONFLICT_CHECK.name or not result.fix: + updated.append(result) + continue + if conflict.status == "fail": + updated.append(replace(result, fix=_SUPERVISOR_CONFLICT_FIX_POINTER)) + continue + if _fix_mentions_unsafe_service_action(result.fix): + updated.append(replace(result, fix=_SUPERVISOR_TOPOLOGY_WARN_POINTER)) + continue + updated.append(result) + return updated + + def run_checks( args: Args, checks: list[tuple[Check, Runner]] | None = None, @@ -1233,7 +1316,7 @@ def run_checks( ) continue results.append(func(args)) - return results + return _apply_supervisor_conflict_fix_policy(results) def print_result_line(result: CheckResult) -> None: diff --git a/solstone/think/service.py b/solstone/think/service.py index c22e08d72..72787af4b 100644 --- a/solstone/think/service.py +++ b/solstone/think/service.py @@ -24,6 +24,7 @@ from __future__ import annotations import logging import os import plistlib +import re import shlex import subprocess import sys @@ -32,6 +33,8 @@ from dataclasses import dataclass from pathlib import Path from xml.parsers.expat import ExpatError +import psutil + from solstone.think.install_guard import ( parse_wrapper, validate_journal_path_for_wrapper, @@ -49,6 +52,8 @@ _LAUNCHD_UNLOAD_POLL_INTERVAL_S = 0.1 _LAUNCHD_UNLOAD_TIMEOUT_S = 30.0 _LAUNCHD_BOOTSTRAP_MAX_ATTEMPTS = 5 _LAUNCHD_BOOTSTRAP_RETRY_WAIT_S = 1.0 +_SUPERVISOR_CONFLICT_LAUNCHCTL_TIMEOUT_S = 2.0 +_JOURNAL_APP_EXECUTABLE_SUFFIX = "/journal.app/Contents/MacOS/journal" logger = logging.getLogger(__name__) @@ -72,6 +77,41 @@ class ServiceTargetIdentity: detail: str +@dataclass(frozen=True) +class SupervisorConflictEvidence: + """Read-only macOS topology evidence for competing journal supervisors.""" + + plist_path: str + plist_state: str + label_state: str + label_pid: int | None + app_state: str + app_pid: int | None + app_executable: str | None + detail: str + + @property + def plist_installed(self) -> bool: + return self.plist_state in {"present", "malformed"} + + @property + def unknown_axes(self) -> tuple[str, ...]: + axes: list[str] = [] + if self.plist_state == "unknown": + axes.append("plist") + if self.label_state == "unknown": + axes.append("label") + if self.app_state == "unknown": + axes.append("app") + return tuple(axes) + + @property + def is_conflict(self) -> bool: + return self.app_state == "running" and ( + self.plist_installed or self.label_state == "loaded" + ) + + def _ready_timeout_message() -> str: return ( f"Service did not become ready within {READY_TIMEOUT_SECONDS:g}s — " @@ -449,6 +489,135 @@ def check_service_target_identity() -> ServiceTargetIdentity: ) +def _supervisor_conflict_detail( + plist_path: Path, + plist_state: str, + label_state: str, + label_pid: int | None, + app_state: str, + app_pid: int | None, + app_executable: str | None, +) -> str: + label_pid_text = str(label_pid) if label_pid is not None else "none" + app_pid_text = str(app_pid) if app_pid is not None else "none" + app_executable_text = app_executable if app_executable is not None else "none" + return ( + f"plist_path={plist_path} plist_state={plist_state}; " + f"launchd_label={label_state} pid={label_pid_text}; " + f"journal_app={app_state} pid={app_pid_text} exe={app_executable_text}" + ) + + +def _inspect_supervisor_conflict_plist(path: Path) -> str: + try: + os.stat(path) + except (FileNotFoundError, NotADirectoryError): + return "absent" + except OSError: + return "unknown" + return "present" if _launchd_program_arguments(path) else "malformed" + + +def _launchctl_print_pid(stdout: str) -> int | None: + match = re.search(r"^\s*pid = (\d+)\s*$", stdout, re.MULTILINE) + return int(match.group(1)) if match else None + + +def _inspect_supervisor_conflict_label(uid: int) -> tuple[str, int | None]: + try: + result = subprocess.run( + ["launchctl", "print", f"gui/{uid}/{SERVICE_LABEL}"], + capture_output=True, + text=True, + timeout=_SUPERVISOR_CONFLICT_LAUNCHCTL_TIMEOUT_S, + check=False, + ) + except (subprocess.TimeoutExpired, OSError): + return "unknown", None + if result.returncode == 0: + return "loaded", _launchctl_print_pid(result.stdout or "") + output = f"{result.stdout or ''}\n{result.stderr or ''}".lower() + if any(marker in output for marker in _not_loaded_markers()): + return "unloaded", None + return "unknown", None + + +def _inspect_supervisor_conflict_app(uid: int) -> tuple[str, int | None, str | None]: + found_unknown = False + try: + # Do not pass attrs: psutil prefetches them by calling exe() for every process. + processes = psutil.process_iter() + for proc in processes: + try: + uids = proc.uids() + except psutil.Error: + # UID was not readable, so we never confirmed this process is ours. + continue + + if uids.real != uid: + continue + + # Filter by UID before exe(); other users' processes commonly deny exe(). + try: + exe = proc.exe() + except psutil.NoSuchProcess: + continue + except psutil.AccessDenied: + found_unknown = True + continue + except psutil.Error: + found_unknown = True + continue + if exe.endswith(_JOURNAL_APP_EXECUTABLE_SUFFIX): + return "running", proc.pid, exe + except psutil.Error: + return "unknown", None, None + return ("unknown" if found_unknown else "absent"), None, None + + +def inspect_supervisor_conflict() -> SupervisorConflictEvidence: + """Inspect whether journal.app and the legacy LaunchAgent both supervise. + + Read-only. On non-macOS platforms this returns a clean non-conflict result + without invoking launchctl or enumerating processes. + """ + if sys.platform != "darwin": + return SupervisorConflictEvidence( + str(_plist_path()), + "absent", + "unloaded", + None, + "absent", + None, + None, + "macOS supervisor topology is not applicable on this platform", + ) + + uid = os.getuid() + plist_path = _plist_path() + plist_state = _inspect_supervisor_conflict_plist(plist_path) + label_state, label_pid = _inspect_supervisor_conflict_label(uid) + app_state, app_pid, app_executable = _inspect_supervisor_conflict_app(uid) + return SupervisorConflictEvidence( + str(plist_path), + plist_state, + label_state, + label_pid, + app_state, + app_pid, + app_executable, + _supervisor_conflict_detail( + plist_path, + plist_state, + label_state, + label_pid, + app_state, + app_pid, + app_executable, + ), + ) + + def remove_stale_plists() -> tuple[int, int]: """Remove stale launchd plists from prior installs.""" if sys.platform != "darwin": diff --git a/tests/systemd-test/README.md b/tests/systemd-test/README.md index 1a2bab585..dac59c459 100644 --- a/tests/systemd-test/README.md +++ b/tests/systemd-test/README.md @@ -15,7 +15,7 @@ operational playbook in the sol pbc org repo, ```bash make build # build the image make smoke # ~30s — verifies systemd --user works end-to-end -make install # ~3-5min — uv tool host install && journal setup +make install # ~3-5min — uv tool host install, then journal setup make observer-ingest # ~3-5min — install + setup + observer ingest round-trip make legacy-upgrade # ~3-5min — install over a seeded legacy non-symlink wrapper ``` @@ -24,7 +24,7 @@ make legacy-upgrade # ~3-5min — install over a seeded legacy non-symlink `--user` accepts, enables, starts, and reports it active. Use it as a fast pre-flight before chasing solstone-specific failures. -`install` runs the actual journal install path (`uv tool install solstone-journal && journal setup -y --skip-models --skip-skills`) and +`install` runs the actual journal install path (`uv tool install solstone-journal`, then `journal setup -y --skip-models --skip-skills`) and verifies the resulting `solstone.service` reaches `active` plus `journal service status` returns 0. `--skip-models / --skip-skills` are passed by default because faster-whisper / Parakeet / Claude-skill installation is orthogonal to diff --git a/tests/systemd-test/run-test.sh b/tests/systemd-test/run-test.sh index f8b3ba359..637388ef6 100755 --- a/tests/systemd-test/run-test.sh +++ b/tests/systemd-test/run-test.sh @@ -5,7 +5,7 @@ # Usage: # ./run-test.sh # default: smoke (verify systemd --user only) # ./run-test.sh smoke # tiny user unit, no solstone install -# ./run-test.sh install [extra-args] # full: uv tool host install && journal setup +# ./run-test.sh install [extra-args] # full: uv tool host install, then journal setup # ./run-test.sh observer-ingest # install + setup + real observer ingest round-trip # ./run-test.sh legacy-upgrade # install, but seed a legacy non-symlink # # wrapper first; assert setup self-heals it diff --git a/tests/test_journal_doctor.py b/tests/test_journal_doctor.py index a102a8d38..2f1c041d0 100644 --- a/tests/test_journal_doctor.py +++ b/tests/test_journal_doctor.py @@ -6,12 +6,13 @@ from __future__ import annotations import json import os import plistlib +import subprocess from pathlib import Path from types import SimpleNamespace import pytest -from solstone.think import install_guard +from solstone.think import install_guard, service @pytest.fixture @@ -95,6 +96,56 @@ def tree_snapshot(root: Path) -> list[tuple[str, str, str]]: return snapshot +class _FakeUids: + def __init__(self, real: int): + self.real = real + + +class _FakeProcess: + def __init__(self, *, pid: int = 99, uid: int = 501, exe: str = "/tmp/other"): + self.pid = pid + self._uid = uid + self._exe = exe + + def uids(self) -> _FakeUids: + return _FakeUids(self._uid) + + def exe(self) -> str: + return self._exe + + +def force_darwin_supervisor_reader( + doctor, + monkeypatch, + *, + launchctl: subprocess.CompletedProcess, + processes: list[_FakeProcess] | None = None, +) -> None: + monkeypatch.setattr(doctor, "platform_tag", lambda: "darwin") + monkeypatch.setattr(service.sys, "platform", "darwin") + monkeypatch.setattr(service.os, "getuid", lambda: 501) + monkeypatch.setattr(service.subprocess, "run", lambda *args, **kwargs: launchctl) + monkeypatch.setattr( + service.psutil, + "process_iter", + lambda: [] if processes is None else processes, + ) + + +def write_legacy_plist(home_root: Path, argv: list[str]) -> Path: + plist_path = home_root / "Library" / "LaunchAgents" / "org.solpbc.solstone.plist" + plist_path.parent.mkdir(parents=True) + plist_path.write_bytes( + plistlib.dumps( + { + "Label": "org.solpbc.solstone", + "ProgramArguments": argv, + } + ) + ) + return plist_path + + def install_router_skill_links(doctor, journal: Path) -> None: sources = doctor.skills_cli.discover_project_sources(doctor.ROOT) for rel_dir in [Path(".claude/skills"), Path(".agents/skills")]: @@ -389,6 +440,348 @@ def test_service_identity_match_ok(doctor, monkeypatch): assert result.detail == "service target matches current install" +SUPERVISOR_CONFLICT_GRID = [ + ("present", "loaded", "running", "fail", ()), + ("present", "loaded", "absent", "ok", ()), + ("present", "loaded", "unknown", "warn", ("app",)), + ("present", "unloaded", "running", "fail", ()), + ("present", "unloaded", "absent", "ok", ()), + ("present", "unloaded", "unknown", "warn", ("app",)), + ("present", "unknown", "running", "fail", ()), + ("present", "unknown", "absent", "warn", ("label",)), + ("present", "unknown", "unknown", "warn", ("label", "app")), + ("malformed", "loaded", "running", "fail", ()), + ("malformed", "loaded", "absent", "ok", ()), + ("malformed", "loaded", "unknown", "warn", ("app",)), + ("malformed", "unloaded", "running", "fail", ()), + ("malformed", "unloaded", "absent", "ok", ()), + ("malformed", "unloaded", "unknown", "warn", ("app",)), + ("malformed", "unknown", "running", "fail", ()), + ("malformed", "unknown", "absent", "warn", ("label",)), + ("malformed", "unknown", "unknown", "warn", ("label", "app")), + ("absent", "loaded", "running", "fail", ()), + ("absent", "loaded", "absent", "ok", ()), + ("absent", "loaded", "unknown", "warn", ("app",)), + ("absent", "unloaded", "running", "ok", ()), + ("absent", "unloaded", "absent", "ok", ()), + ("absent", "unloaded", "unknown", "warn", ("app",)), + ("absent", "unknown", "running", "warn", ("label",)), + ("absent", "unknown", "absent", "warn", ("label",)), + ("absent", "unknown", "unknown", "warn", ("label", "app")), + ("unknown", "loaded", "running", "fail", ()), + ("unknown", "loaded", "absent", "warn", ("plist",)), + ("unknown", "loaded", "unknown", "warn", ("plist", "app")), + ("unknown", "unloaded", "running", "warn", ("plist",)), + ("unknown", "unloaded", "absent", "warn", ("plist",)), + ("unknown", "unloaded", "unknown", "warn", ("plist", "app")), + ("unknown", "unknown", "running", "warn", ("plist", "label")), + ("unknown", "unknown", "absent", "warn", ("plist", "label")), + ("unknown", "unknown", "unknown", "warn", ("plist", "label", "app")), +] + + +@pytest.mark.parametrize( + ("plist_state", "label_state", "app_state", "expected_status", "unknown_axes"), + SUPERVISOR_CONFLICT_GRID, +) +def test_supervisor_conflict_grid( + doctor, + monkeypatch, + plist_state, + label_state, + app_state, + expected_status, + unknown_axes, +): + plist_path = "/tmp/Library/LaunchAgents/org.solpbc.solstone.plist" + label_pid = 12345 if label_state == "loaded" else None + app_pid = 2468 if app_state == "running" else None + app_executable = ( + "/private/var/folders/xx/AppTranslocation/ABC/d/" + "journal.app/Contents/MacOS/journal" + if app_pid is not None + else None + ) + evidence = service.SupervisorConflictEvidence( + plist_path=plist_path, + plist_state=plist_state, + label_state=label_state, + label_pid=label_pid, + app_state=app_state, + app_pid=app_pid, + app_executable=app_executable, + detail=( + f"plist_path={plist_path} plist_state={plist_state}; " + f"launchd_label={label_state} pid={label_pid or 'none'}; " + f"journal_app={app_state} pid={app_pid or 'none'} " + f"exe={app_executable or 'none'}" + ), + ) + monkeypatch.setattr(doctor, "inspect_supervisor_conflict", lambda: evidence) + + result = doctor.supervisor_conflict_check(args(doctor)) + + assert result.status == expected_status + if expected_status == "fail": + assert result.fix == "journal service uninstall" + assert "macOS supervisor conflict" in result.detail + elif expected_status == "warn": + assert result.fix is None + for axis in unknown_axes: + assert axis in result.detail + else: + assert result.fix is None + assert "no macOS supervisor conflict" in result.detail + + +def test_supervisor_conflict_grid_counts(): + counts = { + status: sum( + 1 + for *_states, row_status, _axes in SUPERVISOR_CONFLICT_GRID + if row_status == status + ) + for status in {"fail", "ok", "warn"} + } + + assert counts == {"fail": 8, "ok": 7, "warn": 21} + + +def test_conflict_report_suppresses_orthogonal_upgrade_fix( + doctor, monkeypatch, home_root +): + plist_path = write_legacy_plist(home_root, ["/tmp/legacy-journal", "start"]) + app_executable = ( + "/private/var/folders/xx/AppTranslocation/ABC/d/" + "journal.app/Contents/MacOS/journal" + ) + force_darwin_supervisor_reader( + doctor, + monkeypatch, + launchctl=subprocess.CompletedProcess( + args=["launchctl"], + returncode=0, + stdout="\tpid = 12345\n", + stderr="", + ), + processes=[_FakeProcess(pid=4242, exe=app_executable)], + ) + monkeypatch.setattr( + doctor, + "_installed_packaging_versions", + lambda: { + "solstone": "2.0.0", + "solstone-journal": "1.0.0", + "solstone-journal-cuda": None, + "solstone-journal-host": None, + }, + ) + before = tree_snapshot(home_root) + + results = doctor.run_checks( + args(doctor), + checks=[ + (doctor.SUPERVISOR_CONFLICT_CHECK, doctor.supervisor_conflict_check), + ( + doctor.JOURNAL_PACKAGE_VERSION_CHECK, + doctor.journal_package_version_check, + ), + ], + ) + + assert tree_snapshot(home_root) == before + by_name = {result.name: result for result in results} + assert by_name["supervisor_conflict"].status == "fail" + assert by_name["supervisor_conflict"].fix == "journal service uninstall" + assert str(plist_path) in by_name["supervisor_conflict"].detail + assert "plist_state=present" in by_name["supervisor_conflict"].detail + assert "launchd_label=loaded pid=12345" in by_name["supervisor_conflict"].detail + assert ( + f"journal_app=running pid=4242 exe={app_executable}" + in by_name["supervisor_conflict"].detail + ) + drift = by_name["journal_package_version"] + assert drift.status == "fail" + assert "journal package version mismatch" in drift.detail + assert "solstone 2.0.0" in drift.detail + assert drift.fix == doctor._SUPERVISOR_CONFLICT_FIX_POINTER + fixes = {result.fix for result in results if result.fix} + assert fixes == { + "journal service uninstall", + doctor._SUPERVISOR_CONFLICT_FIX_POINTER, + } + fix_text = "\n".join(fixes) + assert "pip install --upgrade" not in fix_text + assert "journal setup" not in fix_text + assert "journal service install" not in fix_text + assert "journal service start" not in fix_text + assert "journal service restart" not in fix_text + + +def test_conflict_full_journal_report_suppresses_every_other_fix( + doctor, monkeypatch, tmp_path, home_root +): + from solstone.think import pipeline_health + + journal = tmp_path / "journal" + journal.mkdir() + write_legacy_plist(home_root, ["/tmp/missing-journal", "start"]) + force_darwin_supervisor_reader( + doctor, + monkeypatch, + launchctl=subprocess.CompletedProcess( + args=["launchctl"], + returncode=0, + stdout="\tpid = 12345\n", + stderr="", + ), + processes=[ + _FakeProcess( + pid=4242, + exe="/Applications/journal.app/Contents/MacOS/journal", + ) + ], + ) + monkeypatch.setattr(doctor, "get_journal_info", lambda: (str(journal), "test")) + monkeypatch.setattr(doctor, "_host_module_present", lambda _module: True) + monkeypatch.setattr(doctor, "service_is_installed", lambda: True) + monkeypatch.setattr( + doctor, + "check_service_target_identity", + lambda: SimpleNamespace( + installed=True, + target="/tmp/current/journal", + matches_current_install=True, + detail="service target matches current install", + ), + ) + monkeypatch.setattr( + doctor, + "fetch_supervisor_status", + lambda: {"crashed": [], "tasks": []}, + ) + monkeypatch.setattr( + doctor, + "_installed_packaging_versions", + lambda: { + "solstone": "2.0.0", + "solstone-journal": "1.0.0", + "solstone-journal-cuda": None, + "solstone-journal-host": None, + }, + ) + monkeypatch.setattr( + doctor, + "check_journal_sync", + lambda: SimpleNamespace(is_conflict=False), + ) + monkeypatch.setattr(doctor, "format_doctor_report", lambda _result: "synced") + monkeypatch.setattr( + pipeline_health, + "read_backlog_view", + lambda: SimpleNamespace( + days=[], + errors=[], + pending_days=0, + stuck_days=0, + oldest_pending_day=None, + ), + ) + monkeypatch.setattr("solstone.apps.observer.utils.list_observers", lambda: []) + + results = doctor.run_checks(args(doctor), checks=doctor.JOURNAL_CHECKS) + + by_name = {result.name: result for result in results} + assert by_name["supervisor_conflict"].status == "fail" + for result in results: + if result.name == "supervisor_conflict": + continue + assert result.fix in {None, doctor._SUPERVISOR_CONFLICT_FIX_POINTER} + suppressed = [ + result.name + for result in results + if result.name != "supervisor_conflict" + and result.fix == doctor._SUPERVISOR_CONFLICT_FIX_POINTER + ] + assert suppressed, "no fix was suppressed; the test proves nothing" + + fix_text = "\n".join(result.fix or "" for result in results) + for token in doctor._UNSAFE_SERVICE_ACTIONS: + assert token not in fix_text + assert "rm " not in fix_text + assert "pip install --upgrade" not in fix_text + + +def test_unknown_topology_suppresses_only_service_lifecycle_fixes( + doctor, monkeypatch, home_root +): + write_legacy_plist(home_root, ["/tmp/missing-journal", "start"]) + force_darwin_supervisor_reader( + doctor, + monkeypatch, + launchctl=subprocess.CompletedProcess( + args=["launchctl"], + returncode=5, + stdout="", + stderr="opaque launchctl failure", + ), + ) + monkeypatch.setattr( + service.psutil, + "process_iter", + lambda: (_ for _ in ()).throw(service.psutil.Error("boom")), + ) + monkeypatch.setattr( + doctor, + "_installed_packaging_versions", + lambda: { + "solstone": "2.0.0", + "solstone-journal": "1.0.0", + "solstone-journal-cuda": None, + "solstone-journal-host": None, + }, + ) + before = tree_snapshot(home_root) + + results = doctor.run_checks( + args(doctor), + checks=[ + (doctor.SUPERVISOR_CONFLICT_CHECK, doctor.supervisor_conflict_check), + (doctor.LAUNCHD_STALE_PLIST_CHECK, doctor.launchd_stale_plist_check), + ( + doctor.JOURNAL_PACKAGE_VERSION_CHECK, + doctor.journal_package_version_check, + ), + ], + ) + + assert tree_snapshot(home_root) == before + by_name = {result.name: result for result in results} + assert by_name["supervisor_conflict"].status == "warn" + assert "unknown axis(es): label, app" in by_name["supervisor_conflict"].detail + stale = by_name["launchd_stale_plist"] + assert stale.status == "fail" + assert "plist points to missing executable" in stale.detail + assert stale.fix == doctor._SUPERVISOR_TOPOLOGY_WARN_POINTER + drift = by_name["journal_package_version"] + assert drift.status == "fail" + assert "journal package version mismatch" in drift.detail + assert "pip install --upgrade solstone-journal" in (drift.fix or "") + + fix_text = "\n".join(result.fix or "" for result in results) + assert "journal service uninstall" not in fix_text + for token in doctor._UNSAFE_SERVICE_ACTIONS: + assert token not in fix_text + + +def test_unsafe_service_action_match_does_not_match_uninstall(doctor): + assert not doctor._fix_mentions_unsafe_service_action("journal service uninstall") + assert doctor._fix_mentions_unsafe_service_action("journal service install") + assert doctor._fix_mentions_unsafe_service_action("journal service start") + assert doctor._fix_mentions_unsafe_service_action("journal service restart") + assert doctor._fix_mentions_unsafe_service_action("journal setup") + + def test_role_skip_without_local_journal(doctor, monkeypatch, tmp_path, home_root): journal = tmp_path / "missing-journal" monkeypatch.setattr(doctor, "get_journal_info", lambda: (str(journal), "env")) @@ -544,6 +937,12 @@ class TestLaunchdStalePlist: ) result = doctor.launchd_stale_plist_check(args(doctor)) assert result.status == "fail" + assert result.fix == ( + "run journal service uninstall, then run journal service install " + "separately to reinstall a headless background service" + ) + assert "journal setup" not in (result.fix or "") + assert "&&" not in (result.fix or "") def test_ok_when_target_exists(self, doctor, monkeypatch, home_root, tmp_path): monkeypatch.setattr(doctor, "platform_tag", lambda: "darwin") diff --git a/tests/test_service.py b/tests/test_service.py index 8728a800e..edee51902 100644 --- a/tests/test_service.py +++ b/tests/test_service.py @@ -17,6 +17,67 @@ import pytest from solstone.think import service +LAUNCHCTL_RUNNING_WITH_PID = """gui/501/org.solpbc.solstone = { +\tactive count = 1 +\tpath = /Users/jer/Library/LaunchAgents/org.solpbc.solstone.plist +\ttype = LaunchAgent +\tstate = running +\tprogram = /Users/jer/.local/bin/sol +\tpid = 12345 +\tdomain = gui/501 +\tasid = 100012 +\tlast exit code = 0 +\trun interval = 0 +\tactive transactions = 0 +\tdefault environment = { +\t\tPATH => /usr/bin:/bin +\t} +\tenvironment = { +\t\tHOME => /Users/jer +\t} +\tdomain = gui/501 +\tminimum runtime = 10 +\texit timeout = 5 +\tendpoints = { +\t} +\tevent triggers = { +\t} +\tpid local dispatch queue = { +\t\tjob state = running +\t} +} +""" + +LAUNCHCTL_LOADED_NO_PID = """gui/501/org.solpbc.solstone = { +\tactive count = 0 +\tpath = /Users/jer/Library/LaunchAgents/org.solpbc.solstone.plist +\ttype = LaunchAgent +\tstate = not running +\tprogram = /Users/jer/.local/bin/sol +\tdomain = gui/501 +\tasid = 100012 +\trun interval = 0 +\tactive transactions = 0 +\tdefault environment = { +\t\tPATH => /usr/bin:/bin +\t} +\tenvironment = { +\t\tHOME => /Users/jer +\t} +\tdomain = gui/501 +\tminimum runtime = 10 +\texit timeout = 5 +\tendpoints = { +\t} +\tevent triggers = { +\t} +\tpid local dispatch queue = { +\t\tjob state = exited +\t} +\tlast exit code = 0 +} +""" + def _install_fake_launchd_clock(monkeypatch): fake = [0.0] @@ -29,6 +90,76 @@ def _install_fake_launchd_clock(monkeypatch): return fake +class _FakeUids: + def __init__(self, real: int): + self.real = real + + +class _FakeProcess: + def __init__( + self, + *, + pid: int = 99, + uid: int = 501, + name: str = "other", + exe: str = "/Applications/Other.app/Contents/MacOS/other", + uid_exc: Exception | None = None, + exe_exc: Exception | None = None, + ): + self.pid = pid + self._uid = uid + self._name = name + self._exe = exe + self._uid_exc = uid_exc + self._exe_exc = exe_exc + self.exe_called = False + + def name(self) -> str: + return self._name + + def uids(self) -> _FakeUids: + if self._uid_exc: + raise self._uid_exc + return _FakeUids(self._uid) + + def exe(self) -> str: + self.exe_called = True + if self._exe_exc: + raise self._exe_exc + return self._exe + + +def _patch_supervisor_conflict_inspection( + monkeypatch, + tmp_path: Path, + *, + launchctl_returncode: int = 1, + launchctl_stdout: str = "", + launchctl_stderr: str = "service not found", + processes: list[_FakeProcess] | None = None, +) -> Path: + monkeypatch.setattr(sys, "platform", "darwin") + monkeypatch.setattr(service.os, "getuid", lambda: 501) + plist_path = tmp_path / "org.solpbc.solstone.plist" + monkeypatch.setattr(service, "_plist_path", lambda: plist_path) + monkeypatch.setattr( + service.subprocess, + "run", + lambda *args, **kwargs: subprocess.CompletedProcess( + args=args[0], + returncode=launchctl_returncode, + stdout=launchctl_stdout, + stderr=launchctl_stderr, + ), + ) + monkeypatch.setattr( + service.psutil, + "process_iter", + lambda: [] if processes is None else processes, + ) + return plist_path + + class TestPlatform: def test_darwin(self, monkeypatch): monkeypatch.setattr(sys, "platform", "darwin") @@ -310,38 +441,8 @@ class TestServiceHelpers: monkeypatch.setattr(service, "service_is_installed", lambda: True) monkeypatch.setattr(service, "_platform", lambda: "darwin") monkeypatch.setattr(service.os, "getuid", lambda: 501) - launchctl_stdout = """gui/501/org.solpbc.solstone = { -\tactive count = 1 -\tpath = /Users/jer/Library/LaunchAgents/org.solpbc.solstone.plist -\ttype = LaunchAgent -\tstate = running -\tprogram = /Users/jer/.local/bin/sol -\tpid = 12345 -\tdomain = gui/501 -\tasid = 100012 -\tlast exit code = 0 -\trun interval = 0 -\tactive transactions = 0 -\tdefault environment = { -\t\tPATH => /usr/bin:/bin -\t} -\tenvironment = { -\t\tHOME => /Users/jer -\t} -\tdomain = gui/501 -\tminimum runtime = 10 -\texit timeout = 5 -\tendpoints = { -\t} -\tevent triggers = { -\t} -\tpid local dispatch queue = { -\t\tjob state = running -\t} -} -""" run_mock = MagicMock( - return_value=MagicMock(returncode=0, stdout=launchctl_stdout) + return_value=MagicMock(returncode=0, stdout=LAUNCHCTL_RUNNING_WITH_PID) ) monkeypatch.setattr(service.subprocess, "run", run_mock) assert service.service_is_running() is True @@ -358,42 +459,272 @@ class TestServiceHelpers: monkeypatch.setattr(service, "service_is_installed", lambda: True) monkeypatch.setattr(service, "_platform", lambda: "darwin") monkeypatch.setattr(service.os, "getuid", lambda: 501) - launchctl_stdout = """gui/501/org.solpbc.solstone = { -\tactive count = 0 -\tpath = /Users/jer/Library/LaunchAgents/org.solpbc.solstone.plist -\ttype = LaunchAgent -\tstate = not running -\tprogram = /Users/jer/.local/bin/sol -\tdomain = gui/501 -\tasid = 100012 -\trun interval = 0 -\tactive transactions = 0 -\tdefault environment = { -\t\tPATH => /usr/bin:/bin -\t} -\tenvironment = { -\t\tHOME => /Users/jer -\t} -\tdomain = gui/501 -\tminimum runtime = 10 -\texit timeout = 5 -\tendpoints = { -\t} -\tevent triggers = { -\t} -\tpid local dispatch queue = { -\t\tjob state = exited -\t} -\tlast exit code = 0 -} -""" run_mock = MagicMock( - return_value=MagicMock(returncode=0, stdout=launchctl_stdout) + return_value=MagicMock(returncode=0, stdout=LAUNCHCTL_LOADED_NO_PID) ) monkeypatch.setattr(service.subprocess, "run", run_mock) assert service.service_is_running() is False +class TestSupervisorConflictInspection: + def test_non_darwin_guard_does_not_probe(self, monkeypatch): + monkeypatch.setattr(sys, "platform", "linux") + monkeypatch.setattr( + service.subprocess, + "run", + lambda *args, **kwargs: pytest.fail("launchctl should not be called"), + ) + monkeypatch.setattr( + service.psutil, + "process_iter", + lambda: pytest.fail("processes should not be enumerated"), + ) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.plist_state == "absent" + assert evidence.label_state == "unloaded" + assert evidence.app_state == "absent" + assert not evidence.is_conflict + assert "not applicable" in evidence.detail + + def test_launchctl_with_pid_is_loaded(self, monkeypatch, tmp_path): + _patch_supervisor_conflict_inspection( + monkeypatch, + tmp_path, + launchctl_returncode=0, + launchctl_stdout=LAUNCHCTL_RUNNING_WITH_PID, + ) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.label_state == "loaded" + assert evidence.label_pid == 12345 + + def test_launchctl_without_pid_is_loaded(self, monkeypatch, tmp_path): + _patch_supervisor_conflict_inspection( + monkeypatch, + tmp_path, + launchctl_returncode=0, + launchctl_stdout=LAUNCHCTL_LOADED_NO_PID, + ) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.label_state == "loaded" + assert evidence.label_pid is None + + def test_launchctl_recognized_not_loaded_marker_is_unloaded( + self, monkeypatch, tmp_path + ): + _patch_supervisor_conflict_inspection( + monkeypatch, + tmp_path, + launchctl_returncode=113, + launchctl_stderr="Bootstrap lookup failed: service not found", + ) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.label_state == "unloaded" + + def test_launchctl_opaque_nonzero_is_unknown(self, monkeypatch, tmp_path): + _patch_supervisor_conflict_inspection( + monkeypatch, + tmp_path, + launchctl_returncode=5, + launchctl_stderr="launchctl returned an opaque error", + ) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.label_state == "unknown" + + def test_launchctl_timeout_is_unknown(self, monkeypatch, tmp_path): + _patch_supervisor_conflict_inspection(monkeypatch, tmp_path) + + def timeout(command, **kwargs): + raise subprocess.TimeoutExpired(command, kwargs["timeout"]) + + monkeypatch.setattr(service.subprocess, "run", timeout) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.label_state == "unknown" + + def test_launchctl_oserror_is_unknown(self, monkeypatch, tmp_path): + _patch_supervisor_conflict_inspection(monkeypatch, tmp_path) + monkeypatch.setattr( + service.subprocess, + "run", + lambda *args, **kwargs: (_ for _ in ()).throw(OSError("launchctl failed")), + ) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.label_state == "unknown" + + def test_process_iter_failure_is_app_unknown(self, monkeypatch, tmp_path): + _patch_supervisor_conflict_inspection(monkeypatch, tmp_path) + monkeypatch.setattr( + service.psutil, + "process_iter", + lambda: (_ for _ in ()).throw(service.psutil.Error("boom")), + ) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.app_state == "unknown" + + def test_process_uid_filter_runs_before_exe(self, monkeypatch, tmp_path): + other_user_proc = _FakeProcess( + uid=0, + exe_exc=service.psutil.AccessDenied(pid=1), + ) + _patch_supervisor_conflict_inspection( + monkeypatch, + tmp_path, + processes=[other_user_proc], + ) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.app_state == "absent" + assert other_user_proc.exe_called is False + + def test_current_uid_access_denied_exe_taints_app_unknown( + self, monkeypatch, tmp_path + ): + proc = _FakeProcess( + uid=501, + exe_exc=service.psutil.AccessDenied(pid=2), + ) + _patch_supervisor_conflict_inspection(monkeypatch, tmp_path, processes=[proc]) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.app_state == "unknown" + + def test_process_races_do_not_taint_app_state(self, monkeypatch, tmp_path): + procs = [ + _FakeProcess(uid_exc=service.psutil.NoSuchProcess(pid=1)), + _FakeProcess(uid_exc=service.psutil.ZombieProcess(pid=2)), + ] + _patch_supervisor_conflict_inspection(monkeypatch, tmp_path, processes=procs) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.app_state == "absent" + + def test_app_running_matches_executable_suffix(self, monkeypatch, tmp_path): + executable = ( + "/private/var/folders/xx/AppTranslocation/ABC/d/" + "journal.app/Contents/MacOS/journal" + ) + proc = _FakeProcess( + pid=2468, + uid=501, + exe=executable, + ) + _patch_supervisor_conflict_inspection(monkeypatch, tmp_path, processes=[proc]) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.app_state == "running" + assert evidence.app_pid == 2468 + assert evidence.app_executable == executable + assert executable in evidence.detail + + def test_supervisor_process_title_does_not_match_app(self, monkeypatch, tmp_path): + proc = _FakeProcess( + uid=501, + name="journal:supervisor", + exe="/home/x/.venv/bin/python3.12", + ) + _patch_supervisor_conflict_inspection(monkeypatch, tmp_path, processes=[proc]) + + evidence = service.inspect_supervisor_conflict() + + assert proc.name() == "journal:supervisor" + assert evidence.app_state == "absent" + + def test_matching_app_under_other_uid_does_not_match(self, monkeypatch, tmp_path): + proc = _FakeProcess( + uid=0, + exe="/Applications/journal.app/Contents/MacOS/journal", + ) + _patch_supervisor_conflict_inspection(monkeypatch, tmp_path, processes=[proc]) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.app_state == "absent" + + def test_plist_parse_failure_is_malformed(self, monkeypatch, tmp_path): + plist_path = _patch_supervisor_conflict_inspection(monkeypatch, tmp_path) + plist_path.write_text("not a plist", encoding="utf-8") + + evidence = service.inspect_supervisor_conflict() + + assert evidence.plist_state == "malformed" + + def test_plist_missing_argv_is_malformed(self, monkeypatch, tmp_path): + plist_path = _patch_supervisor_conflict_inspection(monkeypatch, tmp_path) + plist_path.write_bytes(plistlib.dumps({"Label": service.SERVICE_LABEL})) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.plist_state == "malformed" + + def test_plist_wrong_label_is_still_present(self, monkeypatch, tmp_path): + plist_path = _patch_supervisor_conflict_inspection(monkeypatch, tmp_path) + plist_path.write_bytes( + plistlib.dumps( + {"Label": "wrong.label", "ProgramArguments": ["/tmp/missing", "start"]} + ) + ) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.plist_state == "present" + + def test_plist_missing_label_is_still_present(self, monkeypatch, tmp_path): + plist_path = _patch_supervisor_conflict_inspection(monkeypatch, tmp_path) + plist_path.write_bytes( + plistlib.dumps({"ProgramArguments": ["/tmp/missing", "start"]}) + ) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.plist_state == "present" + + def test_plist_missing_executable_is_still_present(self, monkeypatch, tmp_path): + plist_path = _patch_supervisor_conflict_inspection(monkeypatch, tmp_path) + plist_path.write_bytes( + plistlib.dumps( + { + "Label": service.SERVICE_LABEL, + "ProgramArguments": ["/tmp/definitely-missing", "start"], + } + ) + ) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.plist_state == "present" + + def test_stat_permission_error_is_plist_unknown(self, monkeypatch, tmp_path): + _patch_supervisor_conflict_inspection(monkeypatch, tmp_path) + monkeypatch.setattr( + service.os, + "stat", + lambda _path: (_ for _ in ()).throw(PermissionError("denied")), + ) + + evidence = service.inspect_supervisor_conflict() + + assert evidence.plist_state == "unknown" + + class TestStatus: def test_not_installed_linux(self, monkeypatch, tmp_path, capsys): monkeypatch.setattr(sys, "platform", "linux")