diff --git a/lib/stack/docker.py b/lib/stack/docker.py index 6be39edf..8766bf39 100644 --- a/lib/stack/docker.py +++ b/lib/stack/docker.py @@ -59,12 +59,22 @@ def compose_up(compose_file: str | Path, env: dict = None) -> tuple[int, str]: `up -d`, leaving running containers stuck on stale env. `stack up` is a deliberate user action, so bouncing healthy containers is an acceptable cost for a reliable config-propagation contract. + + Selecting no services at all is success, not failure. When + COMPOSE_PROFILES excludes every service in the file, compose exits + 1 with "no service selected" — an empty selection, not a service + that refused to start. Treating it as an error meant a stacklet + whose containers are all optional could never finish setup, and + anything depending on it stayed blocked. The ai stacklet under + STACK_AI_NO_VOICE=1 is exactly that shape. """ full_env = {**__import__("os").environ, **(env or {})} result = _docker( "compose", "-f", str(compose_file), "up", "-d", "--force-recreate", capture_output=True, text=True, timeout=300, env=full_env, ) + if result.returncode != 0 and (result.stderr or "").strip() == "no service selected": + return 0, "" return result.returncode, result.stderr diff --git a/stacklets/memory/lib.py b/stacklets/memory/lib.py index ca3295ee..b55ab183 100644 --- a/stacklets/memory/lib.py +++ b/stacklets/memory/lib.py @@ -462,19 +462,13 @@ def load_correspondents_from_vault( if not folder.exists(): return [] - # Lazy import: keeps the CLI install path stdlib-only. Callers of - # this function (archivist, `stack memory correspondents`) bring - # `python-frontmatter` on their PYTHONPATH. - import frontmatter - result: List[Correspondent] = [] for md_path in sorted(folder.glob("*.md")): try: - with open(md_path, "r", encoding="utf-8") as f: - post = frontmatter.load(f) - except (OSError, ValueError): + text = md_path.read_text(encoding="utf-8") + except OSError: continue - meta = post.metadata or {} + meta = _parse_frontmatter(text) if not meta: # No frontmatter at all — likely a README or stray note, skip. continue @@ -485,8 +479,12 @@ def load_correspondents_from_vault( continue result.append(Correspondent( canonical=str(canonical), - aliases=[str(a) for a in (meta.get("aliases") or [])], - topics=[str(t) for t in (meta.get("topics") or [])], + # _fm_list, not a comprehension: a bare string iterates into + # one entry per character, so a hand edit that writes a + # single value where a list belongs turns "insurance" into + # eleven topics instead of one. + aliases=_fm_list(meta, "aliases"), + topics=_fm_list(meta, "topics"), address=meta.get("address"), phone=meta.get("phone"), email=meta.get("email"), @@ -602,10 +600,6 @@ def load_persons_from_vault( if not vault_path.exists(): return [] - # Lazy import: keeps the CLI install path stdlib-only (see the - # module-level note above `load_correspondents_from_vault`). - import frontmatter - skip = _NON_MEMBER_DIRS | {shared_bucket} result: List[Person] = [] for about in sorted(vault_path.glob("*/about.md")): @@ -613,11 +607,10 @@ def load_persons_from_vault( if slug in skip or slug.startswith("."): continue try: - with open(about, "r", encoding="utf-8") as f: - post = frontmatter.load(f) - except (OSError, ValueError): + text = about.read_text(encoding="utf-8") + except OSError: continue - meta = post.metadata or {} + meta = _parse_frontmatter(text) if meta.get("kind") and meta.get("kind") != "person": continue canonical = meta.get("canonical") or meta.get("title") or slug @@ -626,7 +619,7 @@ def load_persons_from_vault( result.append(Person( canonical=str(canonical), slug=str(meta.get("slug") or slug), - synonyms=[str(s) for s in (meta.get("synonyms") or [])], + synonyms=_fm_list(meta, "synonyms"), source_path=about, )) return result diff --git a/stacklets/memory/seeds/_shared/correspondents/README.md b/stacklets/memory/seeds/_shared/correspondents/README.md index b80b069e..87b1e536 100644 --- a/stacklets/memory/seeds/_shared/correspondents/README.md +++ b/stacklets/memory/seeds/_shared/correspondents/README.md @@ -28,7 +28,9 @@ canonical: Duff Insurance aliases: - "Duff Insurance Ortsverband Springfield" - "Duff Insurance Versicherung AG" -topics: [insurance, vehicle] +topics: + - insurance + - vehicle address: "Hansastraße 19, 80686 München" website: "https://www.duff-insurance.example" --- diff --git a/tests/framework/test_docker_runtime.py b/tests/framework/test_docker_runtime.py index a268e026..b9b4421c 100644 --- a/tests/framework/test_docker_runtime.py +++ b/tests/framework/test_docker_runtime.py @@ -130,3 +130,50 @@ def test_force_recreate_is_unconditional(self): cmd = run.call_args[0][0] assert "--force-recreate" in cmd assert cmd.index("up") < cmd.index("--force-recreate") + + +class TestComposeUpWithNoActiveServices: + """A stacklet whose every service is profile-gated off starts cleanly. + + When COMPOSE_PROFILES excludes all of a compose file's services, + `docker compose up` exits 1 with "no service selected" on stderr. + That is compose reporting an empty selection, not a failure to + start anything, but the CLI reads any non-zero as "Failed to start + services" and refuses to write the setup marker. + + The ai stacklet is the live example: STACK_AI_NO_VOICE=1 clears the + profile, and its only service (Piper TTS) sits behind `voice`. The + documented local-dev opt-out could therefore never finish setup, + which in turn blocked every stacklet that `requires = ["ai"]`. + + The exit code and message below were taken from a real + `docker compose up -d --force-recreate` run, not from reading the + source, so this pins compose's actual contract. + """ + + def _run(self, returncode, stderr): + from stack import docker + docker._context = None + + mock = MagicMock() + mock.returncode = returncode + mock.stderr = stderr + with patch("subprocess.run", return_value=mock): + return docker.compose_up("/tmp/compose.yml") + + def test_empty_selection_is_success(self): + assert self._run(1, "no service selected") == (0, "") + + def test_message_is_matched_regardless_of_padding(self): + """Compose has moved this text between streams and added + whitespace across versions; match on content, not layout.""" + assert self._run(1, " no service selected\n") == (0, "") + + def test_real_failures_still_propagate(self): + """The narrow allowance must not swallow a genuine error.""" + code, err = self._run(1, "network stack declared as external, but could not be found") + assert code == 1 + assert "could not be found" in err + + def test_success_is_untouched(self): + assert self._run(0, "") == (0, "") diff --git a/tests/stacklets/test_memory_correspondents.py b/tests/stacklets/test_memory_correspondents.py index 20103f55..b8b1839f 100644 --- a/tests/stacklets/test_memory_correspondents.py +++ b/tests/stacklets/test_memory_correspondents.py @@ -73,7 +73,9 @@ def test_loads_aliases_topics_and_contact_fields(self, vault): aliases: - "Duff Insurance Ortsverband Springfield" - "Duff Insurance Versicherung AG" -topics: [insurance, vehicle] +topics: + - insurance + - vehicle address: "Hansastraße 19, 80686 München" phone: "089 7676 0" website: "https://www.duff-insurance.de" diff --git a/tests/stacklets/test_memory_host_stdlib.py b/tests/stacklets/test_memory_host_stdlib.py new file mode 100644 index 00000000..c633330c --- /dev/null +++ b/tests/stacklets/test_memory_host_stdlib.py @@ -0,0 +1,116 @@ +"""The memory lib reads the vault on a host with no pip packages. + +`./stack` runs under the system interpreter with `PYTHONPATH=lib` and +nothing else. README.md calls the CLI "zero pip deps" and +docs/admin-guide.md says "no virtualenvs, no pip install. That is +intentional." Every host-side read path therefore has to work with the +stdlib plus `stack.*`. + +This is easy to break without noticing, because the `test` extra +installs `python-frontmatter` for the bot suites. A loader that reaches +for it stays green here and fails for every real user the moment they +run the command. That is exactly what happened: `stack memory person` +shipped and could not run on any clean host. + +So these tests make the *production* environment the thing under test. +Blocking the module in `sys.modules` is what a machine that never ran +`pip install` looks like from inside an import statement. Assert on the +data the loaders return, not on which parser they chose, so a future +swap to another stdlib parser keeps them passing. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent.parent + / "stacklets" / "memory")) + +from lib import ( # noqa: E402 + load_correspondents_from_vault, + load_persons_from_vault, +) + + +@pytest.fixture +def bare_host(monkeypatch): + """A host where `import frontmatter` fails, as on a real install. + + Setting the entry to None is how CPython represents "this import + has already been tried and there is nothing there": the next + `import frontmatter` raises ImportError without touching the disk. + """ + monkeypatch.setitem(sys.modules, "frontmatter", None) + with pytest.raises(ImportError): + import frontmatter # noqa: F401 + return True + + +def _write(path: Path, text: str) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + return path + + +# ── persons: what `stack memory person` walks ──────────────────────── + +def test_persons_load_without_the_pip_package(bare_host, tmp_path): + """`stack memory person ` resolves a name on a clean host. + + Name resolution is the only reason that command parses frontmatter + at all -- the profile body is stripped with a regex. If this raises, + the command is dead on arrival for everyone. + """ + _write(tmp_path / "marge" / "about.md", + "---\ntitle: Marge\nslug: marge\ncanonical: Marge\n" + "synonyms:\n - Marjorie\n - Marge Bouvier\n---\n\n# Marge\n") + + [person] = load_persons_from_vault(tmp_path) + + assert person.canonical == "Marge" + assert person.slug == "marge" + assert person.synonyms == ["Marjorie", "Marge Bouvier"] + + +def test_person_kind_filter_survives_on_a_bare_host(bare_host, tmp_path): + """A non-person page at a member path stays excluded. + + Worth pinning separately: if the parser returned nothing on a bare + host, `kind` would read as absent, the page would fall back to its + slug, and a correspondent would quietly enter the family roster. + Degrading to an empty dict is not a safe failure here. + """ + _write(tmp_path / "duff-insurance" / "about.md", + "---\nkind: correspondent\ncanonical: Duff Insurance\n---\n") + + assert load_persons_from_vault(tmp_path) == [] + + +# ── correspondents: same import, same exposure ─────────────────────── + +def test_correspondents_load_without_the_pip_package(bare_host, tmp_path): + """`stack memory correspondents` shares the defect and the fix.""" + _write(tmp_path / "family" / "correspondents" / "duff.md", + "---\nkind: correspondent\ncanonical: Duff Brewery\n" + "aliases:\n - Duff Beer\n---\n\n# Duff Brewery\n") + + [correspondent] = load_correspondents_from_vault(tmp_path, + shared_bucket="family") + + assert correspondent.canonical == "Duff Brewery" + assert correspondent.aliases == ["Duff Beer"] + + +def test_a_vault_with_nothing_in_it_is_not_an_error(bare_host, tmp_path): + """The empty case ran before the import did, which is what hid this. + + `stack memory person` looked fine against an empty vault because it + returned before reaching the import. Pin both loaders on an empty + vault so that early return can never again pass for proof that the + populated path works. + """ + assert load_persons_from_vault(tmp_path) == [] + assert load_correspondents_from_vault(tmp_path, shared_bucket="family") == []