diff --git a/docs/stack-reference.md b/docs/stack-reference.md index 12c2b280..cb803c95 100644 --- a/docs/stack-reference.md +++ b/docs/stack-reference.md @@ -205,6 +205,25 @@ expect = "pong" | `expect` | Expected HTTP status code (as string) or response body value. | | `path` | JSONPath into the response body. If set, `expect` is compared against the extracted value instead of the status code. | +For several endpoints, use `[[health.checks]]`, which additionally takes a +`name`, a `hint` shown when the check fails, and `[health.checks.headers]`. + +```toml +[[health.checks]] +name = "TTS" +url = "http://localhost:42063/" +hint = "TTS not running — check 'docker logs stack-ai-speech'" +skip_when_env = "STACK_AI_NO_VOICE" +``` + +`skip_when_env` names an environment variable that, when set to `"1"`, +drops the check entirely. Use it for a part of a stacklet the user can +choose not to start: `stack up ai --no-voice` leaves the speech containers +out, and without this the start sequence waits a full timeout for each of +them and then reports containers it was told to skip as broken. Any other +value, including `"0"` or empty, leaves the check in place, so an +accidentally exported variable cannot quietly disable monitoring. + ### Native Services (host stacklets) Host stacklets (`type = "host"`) declare native macOS services that run diff --git a/lib/stack/stack.py b/lib/stack/stack.py index 5517d3e4..643cece5 100644 --- a/lib/stack/stack.py +++ b/lib/stack/stack.py @@ -13,6 +13,7 @@ import collections import json +import os import platform import shutil import subprocess @@ -585,8 +586,16 @@ def _resolve_health_checks(self, manifest, template_vars): url = "..." name = "..." hint = "..." + skip_when_env = "STACK_AI_NO_VOICE" # drop when that var is "1" [health.checks.headers] Authorization = "Bearer {api_key}" + + `skip_when_env` exists because a stacklet can be told to start only + part of itself, and a check for the part that was deliberately left + out is not a failure, it is a question nobody asked. Waiting on it + costs the full per-check timeout and then reports a container that + was never meant to be running as broken. The manifest names the + condition; the framework only reads it. """ health = manifest.get("health", {}) checks_list = health.get("checks", []) @@ -594,6 +603,9 @@ def _resolve_health_checks(self, manifest, template_vars): if checks_list: result = [] for c in checks_list: + skip_env = c.get("skip_when_env", "") + if skip_env and os.environ.get(skip_env) == "1": + continue url_tpl = c.get("url", "") name = c.get("name", "") hint = c.get("hint", "") diff --git a/stacklets/ai/hooks/on_start.py b/stacklets/ai/hooks/on_start.py index 7b8c5002..bf8aa1a8 100644 --- a/stacklets/ai/hooks/on_start.py +++ b/stacklets/ai/hooks/on_start.py @@ -6,6 +6,7 @@ """ import os +import shutil from stack.prompt import out, nl, warn, dim, TEAL, RESET @@ -31,6 +32,24 @@ def run(ctx): nl() raise RuntimeError("AI provider not configured") + if provider == "managed" and shutil.which("omlx") is None: + # Setup ran once and left a marker, so the install hook will not run + # again on its own. If the binary has since gone -- a Python upgrade + # that broke its virtualenv, a brew cleanup, a migrated machine -- + # nothing notices: the containers start, `stack up` reports success, + # and the only symptom is an LLM health check failing with no stated + # cause. Say what is actually wrong and how to undo it, using the + # same two-command recovery this hook already teaches above. + nl() + warn("oMLX is not installed (no `omlx` command found).") + out("The AI engine was set up before but is no longer on this Mac.") + out("Reinstall it with:") + nl() + out(f" {TEAL}stack destroy ai{RESET} (keeps your downloaded models)") + out(f" {TEAL}stack up ai{RESET} (reinstalls oMLX)") + nl() + raise RuntimeError("oMLX missing for managed provider") + if provider == "external": url = ctx.cfg("openai_url", default="") if not url: diff --git a/stacklets/ai/stacklet.toml b/stacklets/ai/stacklet.toml index 405356fd..a4697672 100644 --- a/stacklets/ai/stacklet.toml +++ b/stacklets/ai/stacklet.toml @@ -21,15 +21,20 @@ hints = [ "Try: stack ai models", ] +# Both belong to the voice profile. `stack up ai --no-voice` never starts +# them, so without the skip the start sequence waits out two full timeouts +# and then reports containers it was told to leave alone as failures. [[health.checks]] name = "TTS" url = "http://localhost:42063/" hint = "TTS not running — check 'docker logs stack-ai-speech'" +skip_when_env = "STACK_AI_NO_VOICE" [[health.checks]] name = "Whisper" url = "http://localhost:42062/" hint = "Whisper not running — check whisper-server logs" +skip_when_env = "STACK_AI_NO_VOICE" [[health.checks]] name = "LLM" diff --git a/tests/framework/test_ai_on_start.py b/tests/framework/test_ai_on_start.py index 6a4e89ca..3bd88df0 100644 --- a/tests/framework/test_ai_on_start.py +++ b/tests/framework/test_ai_on_start.py @@ -35,6 +35,20 @@ def _ctx(make_stack, env): return StackContext(stck, "ai", env) +@pytest.fixture(autouse=True) +def _engine_installed(monkeypatch): + """Pretend oMLX is on the machine, because that is not what these cover. + + on_start refuses to start a managed provider whose oMLX binary has gone + missing; that behaviour has its own tests. Without this stub these would + pass or fail on whether the machine running the suite happens to have + oMLX installed -- green on a developer Mac, red on CI, for reasons that + have nothing to do with compose profiles. + """ + import shutil + monkeypatch.setattr(shutil, "which", lambda _cmd: "/usr/local/bin/omlx") + + @pytest.fixture(autouse=True) def _restore_no_voice(): old = os.environ.get("STACK_AI_NO_VOICE") diff --git a/tests/framework/test_health_check_skip.py b/tests/framework/test_health_check_skip.py new file mode 100644 index 00000000..82388797 --- /dev/null +++ b/tests/framework/test_health_check_skip.py @@ -0,0 +1,83 @@ +"""Health checks for parts a stacklet was told not to start. + +`stack up ai --no-voice` starts the LLM side and deliberately leaves the +speech containers out. The health checks, though, are declared in the +manifest and knew nothing about that, so the start sequence waited out a +full timeout per missing container and then reported two services as +broken that were never meant to be running. + +A manifest can now mark a check as belonging to an optional part, naming +the environment variable that turns that part off. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +_REPO_ROOT = Path(__file__).resolve().parent.parent.parent +sys.path.insert(0, str(_REPO_ROOT / "lib")) + +from stack.stack import Stack # noqa: E402 + + +def _stack(tmp_path) -> Stack: + return Stack(root=tmp_path, data=tmp_path / "data") + + +MANIFEST = { + "health": { + "checks": [ + {"name": "TTS", "url": "http://localhost:42063/", + "skip_when_env": "STACK_AI_NO_VOICE"}, + {"name": "Whisper", "url": "http://localhost:42062/", + "skip_when_env": "STACK_AI_NO_VOICE"}, + {"name": "LLM", "url": "http://localhost:42060/v1/models"}, + ], + }, +} + + +class TestOptionalChecks: + + def test_all_checks_run_when_nothing_is_disabled(self, tmp_path, monkeypatch): + monkeypatch.delenv("STACK_AI_NO_VOICE", raising=False) + checks = _stack(tmp_path)._resolve_health_checks(MANIFEST, {}) + assert [c["name"] for c in checks] == ["TTS", "Whisper", "LLM"] + + def test_disabled_parts_are_not_waited_on(self, tmp_path, monkeypatch): + """The whole point: no timeout spent on a container we chose not + to start, and no failure reported for its absence.""" + monkeypatch.setenv("STACK_AI_NO_VOICE", "1") + checks = _stack(tmp_path)._resolve_health_checks(MANIFEST, {}) + assert [c["name"] for c in checks] == ["LLM"] + + def test_the_variable_must_say_one_not_merely_exist(self, tmp_path, monkeypatch): + """An empty or "0" value is not opting out. Anything else would + make an accidentally-exported variable silently hide checks.""" + monkeypatch.setenv("STACK_AI_NO_VOICE", "0") + checks = _stack(tmp_path)._resolve_health_checks(MANIFEST, {}) + assert [c["name"] for c in checks] == ["TTS", "Whisper", "LLM"] + + def test_checks_without_the_key_are_unaffected(self, tmp_path, monkeypatch): + monkeypatch.setenv("STACK_AI_NO_VOICE", "1") + manifest = {"health": {"checks": [ + {"name": "LLM", "url": "http://localhost:42060/v1/models"}, + ]}} + checks = _stack(tmp_path)._resolve_health_checks(manifest, {}) + assert [c["name"] for c in checks] == ["LLM"] + + +class TestTheAiManifestUsesIt: + """The bug was in the ai stacklet specifically; pin that its manifest + actually carries the marks, not just that the framework supports them.""" + + def test_voice_checks_are_marked_optional(self): + import tomllib + manifest = tomllib.loads( + (_REPO_ROOT / "stacklets" / "ai" / "stacklet.toml").read_text()) + by_name = {c["name"]: c for c in manifest["health"]["checks"]} + assert by_name["TTS"].get("skip_when_env") == "STACK_AI_NO_VOICE" + assert by_name["Whisper"].get("skip_when_env") == "STACK_AI_NO_VOICE" + assert "skip_when_env" not in by_name["LLM"], \ + "the LLM is the point of the stacklet; it is never optional" diff --git a/tests/stacklets/test_ai_start_guard.py b/tests/stacklets/test_ai_start_guard.py new file mode 100644 index 00000000..0a9d5bd8 --- /dev/null +++ b/tests/stacklets/test_ai_start_guard.py @@ -0,0 +1,73 @@ +"""Starting the AI stacklet when its engine is no longer on the Mac. + +First-run setup installs oMLX and records that setup is done. The install +hook is gated on that marker, so it never runs again -- which is fine +until the binary disappears underneath it. A Python upgrade that breaks +its virtualenv, a brew cleanup, a migrated machine: all leave a stacklet +that reports itself set up and starts its containers happily, with a +failing LLM health check as the only clue. + +`stack up ai` now says what is wrong before starting anything. +""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path + +import pytest + +_REPO_ROOT = Path(__file__).resolve().parent.parent.parent +sys.path.insert(0, str(_REPO_ROOT / "lib")) + +# Every stacklet has hooks with the same module names, so load this one +# from its path under its own name rather than by bare import. +_spec = importlib.util.spec_from_file_location( + "ai_hooks_on_start", + _REPO_ROOT / "stacklets" / "ai" / "hooks" / "on_start.py") +on_start = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(on_start) + + +class FakeCtx: + def __init__(self, **cfg): + self._cfg = cfg + self.env: dict = {} + + def cfg(self, key, default=None): + return self._cfg.get(key, default) + + +class TestManagedProviderNeedsItsEngine: + + def test_a_missing_engine_stops_the_start(self, monkeypatch): + monkeypatch.setattr(on_start.shutil, "which", lambda _cmd: None) + with pytest.raises(RuntimeError, match="oMLX missing"): + on_start.run(FakeCtx(provider="managed")) + + def test_an_installed_engine_starts_normally(self, monkeypatch): + monkeypatch.setattr(on_start.shutil, "which", + lambda _cmd: "/opt/homebrew/bin/omlx") + on_start.run(FakeCtx(provider="managed")) # must not raise + + def test_an_external_endpoint_does_not_need_a_local_engine(self, monkeypatch): + """Someone pointing at their own server has no business being told + to install oMLX.""" + monkeypatch.setattr(on_start.shutil, "which", lambda _cmd: None) + on_start.run(FakeCtx(provider="external", + openai_url="https://ai.example.test/v1")) + + +class TestExistingGuardsStillHold: + + def test_no_provider_configured_still_stops(self, monkeypatch): + monkeypatch.setattr(on_start.shutil, "which", lambda _cmd: None) + with pytest.raises(RuntimeError, match="provider not configured"): + on_start.run(FakeCtx()) + + def test_external_without_a_url_still_stops(self, monkeypatch): + monkeypatch.setattr(on_start.shutil, "which", + lambda _cmd: "/opt/homebrew/bin/omlx") + with pytest.raises(RuntimeError, match="Missing openai_url"): + on_start.run(FakeCtx(provider="external"))