diff --git a/docs/DOCTOR.md b/docs/DOCTOR.md index b34e3c4e9..4401eafba 100644 --- a/docs/DOCTOR.md +++ b/docs/DOCTOR.md @@ -37,6 +37,13 @@ Use the diagnostic command that matches the question: `journal`-prefixed commands, including `journal doctor` and `journal setup`, require a journal-host install because the `journal` executable ships in the `solstone-journal` distribution, not in the thin `sol` client. +Each doctor and preflight check is an independent observation. If a check raises +an ordinary execution exception, the row is reported as `ERROR`, the check result +uses status `fail`, and the aggregate fails independently of that check's +severity. The public result includes the exception type and a truncated message, +not a traceback. Summary `errors` are a subset of `failed`; consumers that want +completed health failures should compute `failed - errors`. + `sol doctor` runs four checks: | Check | Severity | Notes | @@ -98,14 +105,16 @@ checks (`python_version`, `sol_importable`, `local_bin_sol_reachable`, `host_dependencies`, `default_stt_ready`, `feature:pdf-import`, `feature:pdf-export`, and `feature:whisper`. It does not run runtime service, sync, config-dir, or launchd checks. A blocker failure still stops setup early; feature advisories stay advisory and include -the exact extra-install command. +the exact extra-install command. An execution error in any readiness check also +stops setup early, even when that check is advisory. `make preflight` runs `scripts/preflight.py`, the stdlib-only source-checkout readiness battery that is valid before `.venv`/`uv` exist: `python_version`, `uv_installed`, `venv_consistent`, `local_bin_sol_reachable`, `disk_space`, and `config_dir_readable`. It shares probe primitives with doctor through `solstone/think/probe.py`, but its -behavior is unchanged. +execution-error rows follow the same aggregate rules: they are labeled `ERROR`, +counted in both `failed` and `errors`, and exit nonzero regardless of severity. --- diff --git a/docs/SOLCLI.md b/docs/SOLCLI.md index aead545d4..fe0697676 100644 --- a/docs/SOLCLI.md +++ b/docs/SOLCLI.md @@ -283,7 +283,13 @@ before `.venv`/`uv` exist, and `journal health` for the live supervisor status v ## Structured output: `journal setup --jsonl` and doctor `--jsonl` -Use `--jsonl` when another process needs progress events as they happen. The contract is one JSON object per stdout line, flushed immediately; doctor `--jsonl` is mutually exclusive with doctor `--json`, and the existing doctor `--json` payload keeps its short statuses (`ok`, `warn`, `fail`, `skip`). +Use `--jsonl` when another process needs progress events as they happen. The +contract is one JSON object per stdout line, flushed immediately; doctor +`--jsonl` is mutually exclusive with doctor `--json`, and the existing doctor +`--json` payload keeps its short statuses (`ok`, `warn`, `fail`, `skip`). A +per-check `execution_error` field is always present in doctor and preflight JSON: +`null` when the check completed normally, or `{"type": "...", "message": "..."}` +when the check runner raised an ordinary exception. | Event | Emitted by | When | |-------|------------|------| @@ -297,6 +303,10 @@ Use `--jsonl` when another process needs progress events as they happen. The con | `check.completed` | doctor `--jsonl` | One diagnostic check finishes. Status is long form: `ok`, `warning`, `failed`, or `skipped`. | | `doctor.completed` | doctor `--jsonl` | Doctor diagnostics finish with `status: "ok"`, `"warning"`, or `"failed"`. | +An execution error fails the doctor or preflight aggregate independently of the +check's severity. Summary `errors` are a subset of `failed`, so a consumer that +wants completed health failures should compute `failed - errors`. + | Code | When | |------|------| | `doctor_failed` | Doctor reports a blocking failure or cannot start. | @@ -322,7 +332,7 @@ the wrappers. ### Doctor pass-through -`journal setup --jsonl` runs `journal doctor --readiness --jsonl` for the doctor step and forwards `doctor.started`, `check.completed`, and `doctor.completed` lines verbatim. The readiness battery is the client readiness checks (`python_version`, `sol_importable`, `local_bin_sol_reachable`, `stale_alias_symlink`, `disk_space`, `journal_dir_writable`) plus `host_dependencies`, `default_stt_ready`, `feature:pdf-import`, `feature:pdf-export`, and `feature:whisper`; it does not run runtime service, sync, config-dir, or launchd checks. Advisory doctor checks are also translated into setup-level `step.warning` events so consumers can handle setup warnings uniformly. +`journal setup --jsonl` runs `journal doctor --readiness --jsonl` for the doctor step and forwards `doctor.started`, `check.completed`, and `doctor.completed` lines verbatim. The readiness battery is the client readiness checks (`python_version`, `sol_importable`, `local_bin_sol_reachable`, `stale_alias_symlink`, `disk_space`, `journal_dir_writable`) plus `host_dependencies`, `default_stt_ready`, `feature:pdf-import`, `feature:pdf-export`, and `feature:whisper`; it does not run runtime service, sync, config-dir, or launchd checks. Advisory doctor checks are also translated into setup-level `step.warning` events so consumers can handle setup warnings uniformly. Execution-error doctor failures remain `doctor_failed` step failures, not warnings. Example stream excerpt for setup readiness: @@ -330,8 +340,8 @@ Example stream excerpt for setup readiness: {"event":"setup.started","ts":"2026-05-11T20:00:00Z","version":"0.0.0+source","mode":"non_interactive"} {"event":"step.started","ts":"2026-05-11T20:00:00Z","step":"doctor","index":1,"total":8} {"event":"doctor.started","ts":"2026-05-11T20:00:00Z","version":"0.0.0+source","port":5015,"feature":""} -{"event":"check.completed","ts":"2026-05-11T20:00:01Z","name":"python_version","severity":"blocker","status":"ok","detail":"Python version ok","fix":""} -{"event":"doctor.completed","ts":"2026-05-11T20:00:01Z","status":"ok","duration_ms":120,"summary":{"total":10,"failed":0,"warnings":0,"skipped":0}} +{"event":"check.completed","ts":"2026-05-11T20:00:01Z","name":"python_version","severity":"blocker","status":"ok","detail":"Python version ok","fix":"","execution_error":null} +{"event":"doctor.completed","ts":"2026-05-11T20:00:01Z","status":"ok","duration_ms":120,"summary":{"total":10,"failed":0,"warnings":0,"skipped":0,"errors":0}} {"event":"step.completed","ts":"2026-05-11T20:00:01Z","step":"doctor","outcome":"ok","duration_ms":121} {"event":"step.completed","ts":"2026-05-11T20:00:04Z","step":"service","outcome":"ok","duration_ms":900} {"event":"setup.completed","ts":"2026-05-11T20:00:04Z","status":"ok","duration_ms":4000} diff --git a/solstone/think/doctor.py b/solstone/think/doctor.py index b79690632..2985756df 100644 --- a/solstone/think/doctor.py +++ b/solstone/think/doctor.py @@ -7,8 +7,9 @@ journal-less machine. `journal doctor` runs journal-host service, folder, and processing-health checks. `--readiness` runs the setup step-1 battery. -Exit code `0` means no blocker failed; exit code `1` means at least one -blocker-severity check failed. +Exit code `0` means no blocker failed and no check raised during execution; +exit code `1` means at least one blocker-severity check failed or any check +raised during execution. Decision log: - Universal python check reads installed package metadata (with a static @@ -61,13 +62,18 @@ from solstone.think.probe import ( Check, CheckResult, Status, + check_result_to_json_dict, compare_versions, config_dir_readable_check, disk_space_check, + has_execution_error, local_bin_sol_reachable_check, make_result, platform_tag, + run_check, run_probe, + status_label, + summary_counts, truncate, version_text, ) @@ -1577,12 +1583,13 @@ def _apply_supervisor_conflict_fix_policy( if conflict is None or conflict.status not in {"fail", "warn"}: return results + conflict_execution_error = has_execution_error(conflict) 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": + if conflict.status == "fail" and not conflict_execution_error: pointer = _SUPERVISOR_CONFLICT_FIX_POINTER_TEMPLATE.format(fix=conflict.fix) updated.append(replace(result, fix=pointer)) continue @@ -1610,7 +1617,7 @@ def run_checks( platform=current_platform, ) ] - return [runner(args)] + return [run_check(check, runner, args)] selected_checks = select_battery(args) if checks is None else checks results: list[CheckResult] = [] @@ -1625,26 +1632,17 @@ def run_checks( ) ) continue - results.append(func(args)) + results.append(run_check(check, func, args)) return _apply_supervisor_conflict_fix_policy(results) def print_result_line(result: CheckResult) -> None: - label = result.status.upper() + label = status_label(result) print(f" {label} {result.name} — {result.detail}") if result.fix: print(f" → {result.fix}") -def summary_counts(results: Sequence[CheckResult]) -> dict[str, int]: - return { - "total": len(results), - "failed": sum(1 for result in results if result.status == "fail"), - "warnings": sum(1 for result in results if result.status == "warn"), - "skipped": sum(1 for result in results if result.status == "skip"), - } - - def emit_text(results: Sequence[CheckResult], *, verbose: bool) -> None: if verbose: for result in results: @@ -1659,22 +1657,14 @@ def emit_text(results: Sequence[CheckResult], *, verbose: bool) -> None: f"{summary['total']} checks, " f"{summary['failed']} failed, " f"{summary['warnings']} warnings, " - f"{summary['skipped']} skipped" + f"{summary['skipped']} skipped, " + f"{summary['errors']} errors" ) def emit_json(results: Sequence[CheckResult]) -> None: payload = { - "checks": [ - { - "name": result.name, - "severity": result.severity, - "status": result.status, - "detail": result.detail, - "fix": result.fix, - } - for result in results - ], + "checks": [check_result_to_json_dict(result) for result in results], "summary": summary_counts(results), } print(json.dumps(payload)) @@ -1688,6 +1678,8 @@ def solstone_version() -> str: def jsonl_summary_status(results: Sequence[CheckResult]) -> str: + if any(has_execution_error(result) for result in results): + return "failed" if any( result.severity == "blocker" and result.status == "fail" for result in results ): @@ -1718,6 +1710,11 @@ def emit_jsonl( status=STATUS_TRANSLATION[result.status], detail=result.detail or "", fix=result.fix or "", + execution_error=( + result.execution_error.to_dict() + if has_execution_error(result) + else None + ), ) emitter.emit( "doctor.completed", @@ -1757,4 +1754,5 @@ def main(argv: Sequence[str] | None = None) -> int: blocker_failed = any( result.severity == "blocker" and result.status == "fail" for result in results ) - return 1 if blocker_failed else 0 + execution_failed = any(has_execution_error(result) for result in results) + return 1 if execution_failed or blocker_failed else 0 diff --git a/solstone/think/preflight.py b/solstone/think/preflight.py index 5259d1d01..f5ee0f534 100644 --- a/solstone/think/preflight.py +++ b/solstone/think/preflight.py @@ -6,8 +6,9 @@ This battery can run before `.venv` or `uv` exist. It composes the stdlib-only checks from `solstone.think.probe`. -Exit code `0` means no blocker-severity check failed; exit code `1` means at -least one blocker-severity check failed. +Exit code `0` means no blocker-severity check failed and no check raised during +execution; exit code `1` means at least one blocker-severity check failed or any +check raised during execution. """ from __future__ import annotations @@ -27,13 +28,18 @@ from solstone.think.probe import ( VENV_CONSISTENT_CHECK, Check, CheckResult, + check_result_to_json_dict, config_dir_readable_check, disk_space_check, + has_execution_error, local_bin_sol_reachable_check, make_result, platform_tag, python_version_check, + run_check, solstone_core_rust_toolchain_check, + status_label, + summary_counts, uv_installed_check, venv_consistent_check, ) @@ -87,26 +93,17 @@ def run_checks(args: Args) -> list[CheckResult]: ) ) continue - results.append(func(args)) + results.append(run_check(check, func, args)) return results def print_result_line(result: CheckResult) -> None: - label = result.status.upper() + label = status_label(result) print(f" {label} {result.name} — {result.detail}") if result.fix: print(f" → {result.fix}") -def summary_counts(results: Sequence[CheckResult]) -> dict[str, int]: - return { - "total": len(results), - "failed": sum(1 for result in results if result.status == "fail"), - "warnings": sum(1 for result in results if result.status == "warn"), - "skipped": sum(1 for result in results if result.status == "skip"), - } - - def emit_text(results: Sequence[CheckResult], *, verbose: bool) -> None: if verbose: for result in results: @@ -121,22 +118,14 @@ def emit_text(results: Sequence[CheckResult], *, verbose: bool) -> None: f"{summary['total']} checks, " f"{summary['failed']} failed, " f"{summary['warnings']} warnings, " - f"{summary['skipped']} skipped" + f"{summary['skipped']} skipped, " + f"{summary['errors']} errors" ) def emit_json(results: Sequence[CheckResult]) -> None: payload = { - "checks": [ - { - "name": result.name, - "severity": result.severity, - "status": result.status, - "detail": result.detail, - "fix": result.fix, - } - for result in results - ], + "checks": [check_result_to_json_dict(result) for result in results], "summary": summary_counts(results), } print(json.dumps(payload)) @@ -152,4 +141,5 @@ def main(argv: Sequence[str] | None = None) -> int: blocker_failed = any( result.severity == "blocker" and result.status == "fail" for result in results ) - return 1 if blocker_failed else 0 + execution_failed = any(has_execution_error(result) for result in results) + return 1 if execution_failed or blocker_failed else 0 diff --git a/solstone/think/probe.py b/solstone/think/probe.py index 7c8d46541..7df30b11c 100644 --- a/solstone/think/probe.py +++ b/solstone/think/probe.py @@ -18,7 +18,7 @@ import sys from dataclasses import dataclass from importlib.metadata import PackageNotFoundError, distribution from pathlib import Path -from typing import Literal, Sequence +from typing import Callable, Literal, Sequence, TypeVar # See doctor.py's decision-log for the MIN_UV=0.7.12 and MIN_FREE_GIB=10 # rationale. @@ -37,6 +37,7 @@ Severity = Literal["blocker", "advisory"] Status = Literal["ok", "fail", "warn", "skip"] Platform = Literal["linux", "darwin"] CorePlatform = tuple[Platform, str] +ArgsT = TypeVar("ArgsT") @dataclass(frozen=True) @@ -46,6 +47,18 @@ class Check: platforms: tuple[Platform, ...] +@dataclass(frozen=True) +class ExecutionError: + type: str + message: str + + def to_dict(self) -> dict[str, str]: + return { + "type": self.type, + "message": self.message, + } + + @dataclass(frozen=True) class CheckResult: name: str @@ -54,6 +67,7 @@ class CheckResult: detail: str fix: str | None platform: str | None = None + execution_error: ExecutionError | None = None @dataclass(frozen=True) @@ -194,6 +208,7 @@ def make_result( fix: str | None = None, *, platform: str | None = None, + execution_error: ExecutionError | None = None, ) -> CheckResult: return CheckResult( name=check.name, @@ -202,9 +217,68 @@ def make_result( detail=detail, fix=fix, platform=platform, + execution_error=execution_error, ) +def has_execution_error(result: CheckResult) -> bool: + return result.execution_error is not None + + +def run_check( + check: Check, + runner: Callable[[ArgsT], CheckResult], + args: ArgsT, +) -> CheckResult: + try: + return runner(args) + except Exception as exc: + message = truncate(str(exc), 512) + exc_type = type(exc).__name__ + detail = f"check execution failed: {exc_type}" + if message: + detail = f"{detail}: {message}" + return make_result( + check, + "fail", + detail, + execution_error=ExecutionError(type=exc_type, message=message), + ) + + +def summary_counts(results: Sequence[CheckResult]) -> dict[str, int]: + return { + "total": len(results), + "failed": sum( + 1 + for result in results + if result.status == "fail" or has_execution_error(result) + ), + "warnings": sum(1 for result in results if result.status == "warn"), + "skipped": sum(1 for result in results if result.status == "skip"), + "errors": sum(1 for result in results if has_execution_error(result)), + } + + +def status_label(result: CheckResult) -> str: + if has_execution_error(result): + return "ERROR" + return result.status.upper() + + +def check_result_to_json_dict(result: CheckResult) -> dict[str, object]: + return { + "name": result.name, + "severity": result.severity, + "status": result.status, + "detail": result.detail, + "fix": result.fix, + "execution_error": ( + result.execution_error.to_dict() if has_execution_error(result) else None + ), + } + + def truncate(text: str, limit: int) -> str: text = " ".join(text.split()) if len(text) <= limit: diff --git a/tests/test_doctor.py b/tests/test_doctor.py index aae243ba3..16d68acbb 100644 --- a/tests/test_doctor.py +++ b/tests/test_doctor.py @@ -13,7 +13,7 @@ from types import SimpleNamespace import pytest from solstone.think import install_guard -from solstone.think.probe import ProbeOutput +from solstone.think.probe import ExecutionError, ProbeOutput ROOT = Path(__file__).resolve().parent.parent @@ -33,8 +33,14 @@ def home_root(monkeypatch, tmp_path): return home -def args(doctor, *, port: int = 5015): - return doctor.Args(verbose=False, json=False, jsonl=False, port=port) +def args(doctor, *, port: int = 5015, feature: str | None = None): + return doctor.Args( + verbose=False, + json=False, + jsonl=False, + port=port, + feature=feature, + ) def make_repo(tmp_path: Path, *, worktree: bool = False) -> Path: @@ -1338,7 +1344,17 @@ class TestJsonAndExitCodes: "status", "detail", "fix", + "execution_error", } + assert payload["checks"][0]["execution_error"] is None + assert list(payload["summary"]) == [ + "total", + "failed", + "warnings", + "skipped", + "errors", + ] + assert payload["summary"]["errors"] == 0 def test_exit_code_matrix(self, doctor, monkeypatch, capsys): monkeypatch.setattr( @@ -1364,6 +1380,31 @@ class TestJsonAndExitCodes: ) assert doctor.main([]) == 0 + def test_advisory_execution_error_returns_nonzero( + self, doctor, monkeypatch, capsys + ): + monkeypatch.setattr( + doctor, + "run_checks", + lambda _args: [ + doctor.CheckResult( + "a", + "advisory", + "fail", + "check execution failed: RuntimeError: boom", + None, + execution_error=ExecutionError("RuntimeError", "boom"), + ) + ], + ) + + assert doctor.main([]) == 1 + output = capsys.readouterr().out + assert "ERROR a" in output + assert output.rstrip().endswith( + "doctor: 1 checks, 1 failed, 0 warnings, 0 skipped, 1 errors" + ) + def test_summary_line_format(self, doctor, monkeypatch, capsys): monkeypatch.setattr( doctor, @@ -1376,7 +1417,104 @@ class TestJsonAndExitCodes: ) doctor.main([]) output = capsys.readouterr().out.strip().splitlines() - assert output[-1] == "doctor: 3 checks, 1 failed, 1 warnings, 1 skipped" + assert ( + output[-1] == "doctor: 3 checks, 1 failed, 1 warnings, 1 skipped, 0 errors" + ) + + def test_run_checks_isolates_exception_and_continues(self, doctor): + before_check = doctor.Check("before_check", "blocker", ("linux", "darwin")) + raising_check = doctor.Check("raising_check", "advisory", ("linux", "darwin")) + after_check = doctor.Check("after_check", "blocker", ("linux", "darwin")) + calls: list[str] = [] + + def before(_args): + calls.append("before") + return doctor.make_result(before_check, "ok", "before complete") + + def raising(_args): + calls.append("raising") + raise RuntimeError("boom") + + def after(_args): + calls.append("after") + return doctor.make_result(after_check, "ok", "after complete") + + results = doctor.run_checks( + args(doctor), + checks=[ + (before_check, before), + (raising_check, raising), + (after_check, after), + ], + ) + + assert [result.name for result in results] == [ + "before_check", + "raising_check", + "after_check", + ] + assert calls == ["before", "raising", "after"] + error = results[1] + assert error.status == "fail" + assert error.severity == "advisory" + assert error.fix is None + assert error.execution_error == ExecutionError("RuntimeError", "boom") + assert "RuntimeError: boom" in error.detail + summary = doctor.summary_counts(results) + assert summary["errors"] == 1 + assert summary["failed"] >= summary["errors"] + + @pytest.mark.parametrize("exception", [KeyboardInterrupt(), SystemExit(7)]) + def test_run_checks_propagates_base_exceptions(self, doctor, exception): + raising_check = doctor.Check("raising_check", "blocker", ("linux", "darwin")) + + def raising(_args): + raise exception + + with pytest.raises(type(exception)): + doctor.run_checks(args(doctor), checks=[(raising_check, raising)]) + + def test_run_checks_truncates_execution_error_without_traceback(self, doctor): + raising_check = doctor.Check("raising_check", "blocker", ("linux", "darwin")) + long_message = " ".join(["overflow"] * 120) + + def raising(_args): + raise PermissionError(long_message) + + result = doctor.run_checks( + args(doctor), + checks=[(raising_check, raising)], + )[0] + + assert doctor.has_execution_error(result) + assert result.execution_error.type == "PermissionError" + assert len(result.execution_error.message) <= 512 + assert result.execution_error.message.endswith("...") + for public_text in [result.detail, result.execution_error.message]: + assert "Traceback" not in public_text + assert "line " not in public_text + assert str(Path(__file__)) not in public_text + + def test_feature_run_isolates_exception(self, doctor, monkeypatch): + feature_check = doctor.Check( + "feature:raising-feature", "advisory", ("linux", "darwin") + ) + + def raising(_args): + raise RuntimeError("feature boom") + + monkeypatch.setitem( + doctor.FEATURE_CHECKS, + "raising-feature", + (feature_check, raising), + ) + + result = doctor.run_checks(args(doctor, feature="raising-feature"))[0] + + assert result.name == "feature:raising-feature" + assert result.status == "fail" + assert result.severity == "advisory" + assert result.execution_error == ExecutionError("RuntimeError", "feature boom") def test_doctor_jsonl_emits_started_and_completed( self, doctor, monkeypatch, capsys @@ -1394,6 +1532,7 @@ class TestJsonAndExitCodes: assert events[0]["event"] == "doctor.started" assert events[-1]["event"] == "doctor.completed" assert events[-1]["status"] == "ok" + assert events[-1]["summary"]["errors"] == 0 def test_doctor_jsonl_emits_check_completed_per_check( self, doctor, monkeypatch, capsys @@ -1412,6 +1551,7 @@ class TestJsonAndExitCodes: checks = [event for event in events if event["event"] == "check.completed"] assert len(checks) == len(doctor.UNIVERSAL_CHECKS) + assert all(check["execution_error"] is None for check in checks) def test_doctor_jsonl_status_translates_short_to_long( self, doctor, monkeypatch, capsys @@ -1443,6 +1583,42 @@ class TestJsonAndExitCodes: "skip": "skipped", } assert events[-1]["status"] == "warning" + assert events[-1]["summary"]["errors"] == 0 + + def test_doctor_jsonl_execution_error_fails_terminal_status( + self, doctor, monkeypatch, capsys + ): + monkeypatch.setattr( + doctor, + "run_checks", + lambda _args: [ + doctor.CheckResult( + "advisory_error", + "advisory", + "fail", + "check execution failed: RuntimeError: boom", + None, + execution_error=ExecutionError("RuntimeError", "boom"), + ) + ], + ) + + rc = doctor.main(["--jsonl"]) + events = [json.loads(line) for line in capsys.readouterr().out.splitlines()] + + completed = [event for event in events if event["event"] == "check.completed"][ + 0 + ] + terminal = events[-1] + assert rc == 1 + assert completed["status"] == "failed" + assert completed["execution_error"] == { + "type": "RuntimeError", + "message": "boom", + } + assert terminal["event"] == "doctor.completed" + assert terminal["status"] == "failed" + assert terminal["summary"]["errors"] == 1 def test_doctor_jsonl_json_and_jsonl_mutually_exclusive(self, doctor): with pytest.raises(SystemExit) as raised: @@ -1466,6 +1642,7 @@ class TestJsonAndExitCodes: assert rc == 0 assert payload["checks"][0]["status"] == "warn" + assert payload["checks"][0]["execution_error"] is None def test_doctor_jsonl_subprocess_e2e(self): result = subprocess.run( @@ -1611,6 +1788,26 @@ def test_journal_doctor_readiness_subprocess_json_shape(): payload = json.loads(result.stdout) assert "checks" in payload and isinstance(payload["checks"], list) assert "summary" in payload and isinstance(payload["summary"], dict) + assert list(payload["summary"]) == [ + "total", + "failed", + "warnings", + "skipped", + "errors", + ] + assert payload["summary"]["errors"] <= payload["summary"]["failed"] + assert all( + set(check) + == { + "name", + "severity", + "status", + "detail", + "fix", + "execution_error", + } + for check in payload["checks"] + ) from solstone.think import doctor as doctor_module assert {check["name"] for check in payload["checks"]} == { diff --git a/tests/test_doctor_features.py b/tests/test_doctor_features.py index 13f1df969..f6d76b3a3 100644 --- a/tests/test_doctor_features.py +++ b/tests/test_doctor_features.py @@ -83,4 +83,6 @@ def test_emit_json_filtered_summary(doctor, capsys): payload = json.loads(capsys.readouterr().out) assert payload["summary"]["total"] == 1 + assert payload["summary"]["errors"] == 0 assert len(payload["checks"]) == 1 + assert payload["checks"][0]["execution_error"] is None diff --git a/tests/test_journal_caught_up.py b/tests/test_journal_caught_up.py index 6dbb241be..ccc2c02af 100644 --- a/tests/test_journal_caught_up.py +++ b/tests/test_journal_caught_up.py @@ -424,7 +424,10 @@ def test_journal_caught_up_rides_existing_emission_paths(doctor, capsys): ok_result = doctor.make_result(doctor.JOURNAL_CAUGHT_UP_CHECK, "ok", "caught up") doctor.emit_json([warn_result]) - assert "journal_caught_up" in capsys.readouterr().out + payload = json.loads(capsys.readouterr().out) + assert payload["checks"][0]["name"] == "journal_caught_up" + assert payload["checks"][0]["execution_error"] is None + assert payload["summary"]["errors"] == 0 doctor.emit_jsonl( [warn_result], @@ -432,7 +435,11 @@ def test_journal_caught_up_rides_existing_emission_paths(doctor, capsys): duration_ms=0, summary_status="warning", ) - assert "journal_caught_up" in capsys.readouterr().out + events = [json.loads(line) for line in capsys.readouterr().out.splitlines()] + check_event = [event for event in events if event["event"] == "check.completed"][0] + assert check_event["name"] == "journal_caught_up" + assert check_event["execution_error"] is None + assert events[-1]["summary"]["errors"] == 0 doctor.emit_text([ok_result], verbose=False) assert "journal_caught_up" not in capsys.readouterr().out diff --git a/tests/test_journal_doctor.py b/tests/test_journal_doctor.py index 28837416c..67499c39e 100644 --- a/tests/test_journal_doctor.py +++ b/tests/test_journal_doctor.py @@ -1693,6 +1693,54 @@ def test_unknown_topology_suppresses_only_service_lifecycle_fixes( assert token not in fix_text +def test_supervisor_conflict_execution_error_uses_unknown_topology_policy(doctor): + conflict_check = doctor.Check( + doctor.SUPERVISOR_CONFLICT_CHECK.name, + doctor.SUPERVISOR_CONFLICT_CHECK.severity, + ("linux", "darwin"), + ) + unsafe_check = doctor.Check("unsafe_fix", "blocker", ("linux", "darwin")) + unrelated_check = doctor.Check("unrelated_fix", "blocker", ("linux", "darwin")) + unrelated_fix = "pip install --upgrade solstone-journal" + + def raising_conflict(_args): + raise RuntimeError("launchctl unavailable") + + def unsafe(_args): + return doctor.make_result( + unsafe_check, + "fail", + "unsafe lifecycle fix", + "journal service restart", + ) + + def unrelated(_args): + return doctor.make_result( + unrelated_check, + "fail", + "unrelated fix", + unrelated_fix, + ) + + results = doctor.run_checks( + args(doctor), + checks=[ + (conflict_check, raising_conflict), + (unsafe_check, unsafe), + (unrelated_check, unrelated), + ], + ) + + by_name = {result.name: result for result in results} + conflict = by_name["supervisor_conflict"] + assert conflict.status == "fail" + assert doctor.has_execution_error(conflict) + assert conflict.fix is None + assert by_name["unsafe_fix"].fix == doctor._SUPERVISOR_TOPOLOGY_WARN_POINTER + assert by_name["unrelated_fix"].fix == unrelated_fix + assert all("None" not in result.fix for result in results if result.fix) + + 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") diff --git a/tests/test_preflight.py b/tests/test_preflight.py index 4ac1e3464..3c6b645be 100644 --- a/tests/test_preflight.py +++ b/tests/test_preflight.py @@ -10,6 +10,7 @@ from unittest.mock import Mock import pytest +from solstone.think.probe import ExecutionError from tests.helpers.module_mocks import module_mock @@ -106,10 +107,69 @@ def test_main_json_passes_when_blockers_pass( assert rc == 0 assert payload["summary"]["failed"] == 0 + assert payload["summary"]["errors"] == 0 + assert list(payload["summary"]) == [ + "total", + "failed", + "warnings", + "skipped", + "errors", + ] assert isinstance(payload["checks"], list) + assert all(check["execution_error"] is None for check in payload["checks"]) assert isinstance(payload["summary"], dict) +def test_main_isolates_execution_error_and_continues(preflight, monkeypatch, capsys): + before_check = preflight.Check("before_check", "blocker", ("linux", "darwin")) + raising_check = preflight.Check("raising_check", "advisory", ("linux", "darwin")) + after_check = preflight.Check("after_check", "blocker", ("linux", "darwin")) + calls: list[str] = [] + + def before(_args): + calls.append("before") + return preflight.make_result(before_check, "ok", "before complete") + + def raising(_args): + calls.append("raising") + raise RuntimeError("boom") + + def after(_args): + calls.append("after") + return preflight.make_result(after_check, "ok", "after complete") + + monkeypatch.setattr( + preflight, + "CHECKS", + [ + (before_check, before), + (raising_check, raising), + (after_check, after), + ], + ) + + results = preflight.run_checks(args(preflight)) + + assert [result.name for result in results] == [ + "before_check", + "raising_check", + "after_check", + ] + assert calls == ["before", "raising", "after"] + assert results[1].execution_error == ExecutionError("RuntimeError", "boom") + + calls.clear() + rc = preflight.main([]) + output = capsys.readouterr().out + + assert rc == 1 + assert calls == ["before", "raising", "after"] + assert "ERROR raising_check" in output + assert output.rstrip().endswith( + "preflight: 3 checks, 1 failed, 0 warnings, 0 skipped, 1 errors" + ) + + def test_python_version_ok(preflight, probe, monkeypatch, tmp_path): repo = make_repo(tmp_path) monkeypatch.setattr(probe, "ROOT", repo) @@ -287,3 +347,4 @@ def test_main_returns_one_when_uv_missing( assert rc == 1 assert payload["summary"]["failed"] >= 1 + assert payload["summary"]["errors"] == 0 diff --git a/tests/test_setup_jsonl.py b/tests/test_setup_jsonl.py index 73e1d1a16..19dc1cfc8 100644 --- a/tests/test_setup_jsonl.py +++ b/tests/test_setup_jsonl.py @@ -61,13 +61,20 @@ def doctor_ok_lines() -> list[dict]: "status": "ok", "detail": "fine", "fix": "", + "execution_error": None, }, { "event": "doctor.completed", "ts": "2026-05-11T00:00:00Z", "status": "ok", "duration_ms": 1, - "summary": {"total": 1, "failed": 0, "warnings": 0, "skipped": 0}, + "summary": { + "total": 1, + "failed": 0, + "warnings": 0, + "skipped": 0, + "errors": 0, + }, }, ] @@ -83,13 +90,20 @@ def doctor_non_port_warning_lines() -> list[dict]: "status": "warning", "detail": ".local/bin/sol is not reachable", "fix": "uv tool install solstone", + "execution_error": None, }, { "event": "doctor.completed", "ts": "2026-05-11T00:00:00Z", "status": "warning", "duration_ms": 1, - "summary": {"total": 1, "failed": 0, "warnings": 1, "skipped": 0}, + "summary": { + "total": 1, + "failed": 0, + "warnings": 1, + "skipped": 0, + "errors": 0, + }, }, ] @@ -105,13 +119,20 @@ def stale_alias_warning_lines() -> list[dict]: "status": "warning", "detail": "~/.local/bin/sol is a legacy uv-tool install", "fix": "run journal setup", + "execution_error": None, }, { "event": "doctor.completed", "ts": "2026-05-11T00:00:00Z", "status": "warning", "duration_ms": 1, - "summary": {"total": 1, "failed": 0, "warnings": 1, "skipped": 0}, + "summary": { + "total": 1, + "failed": 0, + "warnings": 1, + "skipped": 0, + "errors": 0, + }, }, ] @@ -130,13 +151,52 @@ def host_dependency_failed_lines() -> list[dict]: "the journal host is not installed or is incomplete." ), "fix": doctor.HOST_DEPENDENCY_REINSTALL_GUIDANCE, + "execution_error": None, }, { "event": "doctor.completed", "ts": "2026-05-11T00:00:00Z", "status": "failed", "duration_ms": 1, - "summary": {"total": 1, "failed": 1, "warnings": 0, "skipped": 0}, + "summary": { + "total": 1, + "failed": 1, + "warnings": 0, + "skipped": 0, + "errors": 0, + }, + }, + ] + + +def doctor_execution_error_lines() -> list[dict]: + return [ + doctor_ok_lines()[0], + { + "event": "check.completed", + "ts": "2026-05-11T00:00:00Z", + "name": "local_bin_sol_reachable", + "severity": "advisory", + "status": "failed", + "detail": "check execution failed: RuntimeError: boom", + "fix": "", + "execution_error": { + "type": "RuntimeError", + "message": "boom", + }, + }, + { + "event": "doctor.completed", + "ts": "2026-05-11T00:00:00Z", + "status": "failed", + "duration_ms": 1, + "summary": { + "total": 1, + "failed": 1, + "warnings": 0, + "skipped": 0, + "errors": 1, + }, }, ] @@ -320,6 +380,48 @@ def test_setup_jsonl_host_dependency_blocker_fails_doctor_step( assert "Traceback" not in out +def test_setup_jsonl_doctor_execution_error_fails_doctor_step( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + doctor_lines = doctor_execution_error_lines() + expected_failed_check = json.dumps(doctor_lines[1]) + + rc, events, out = run_setup_jsonl( + tmp_path, + monkeypatch, + capsys, + ["--skip-models", "--skip-skills", "--skip-service"], + doctor_lines=doctor_lines, + ) + + for line in out.splitlines(): + json.loads(line) + failed = [event for event in events if event["event"] == "step.failed"][-1] + completed = [event for event in events if event["event"] == "setup.completed"][-1] + terminal_doctor = [ + event for event in events if event["event"] == "doctor.completed" + ][-1] + + assert rc == 1 + assert expected_failed_check in out.splitlines() + assert failed["step"] == "doctor" + assert failed["error"]["code"] == "doctor_failed" + assert completed["status"] == "failed" + assert terminal_doctor["summary"]["errors"] == 1 + assert not [ + event + for event in events + if event["event"] == "step.warning" and event.get("step") == "doctor" + ] + assert not [ + event + for event in events + if event["event"] == "step.completed" and event.get("step") == "doctor" + ] + + def test_setup_jsonl_translates_doctor_advisories_to_step_warning( tmp_path: Path, monkeypatch: pytest.MonkeyPatch,