From c4c0f95949f826f7ef66687f5ac82a8ee37c608b Mon Sep 17 00:00:00 2001 From: Arthur Date: Fri, 31 Jul 2026 23:06:44 +0200 Subject: [PATCH 1/2] fix: let a stacklet with only optional containers finish setup When COMPOSE_PROFILES excludes every service in a compose file, docker compose exits 1 with "no service selected". That is an empty selection, not a service that failed to start, but the CLI read any non-zero as "Failed to start services" and refused to write the setup marker. The ai stacklet is the case that surfaced it: STACK_AI_NO_VOICE=1 clears the profile and its only container sits behind "voice", so the documented local-dev opt-out could never complete setup, which in turn blocked everything that requires the ai stacklet. --- lib/stack/docker.py | 10 ++++++ tests/framework/test_docker_runtime.py | 47 ++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) 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/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, "") From a5fb07890434bc7a4e624cf5d2bf692049ffb079 Mon Sep 17 00:00:00 2001 From: Arthur Date: Fri, 31 Jul 2026 23:07:10 +0200 Subject: [PATCH 2/2] fix: make stack memory person work on a normal install The command died with "No module named 'frontmatter'" on any host that had not run pip install. It read the vault through loaders that imported the third-party python-frontmatter package, which ships only in the test extra, so the suite stayed green while the command was broken for everyone. stack memory correspondents had the same defect. Both loaders now use lib/stack/frontmatter.py, the stdlib-only parser already built for exactly this, keeping the CLI's zero-pip-deps promise. List fields go through _fm_list so a single value written where a list belongs can no longer iterate into one entry per character. The shipped correspondents seed taught inline "[a, b]" list syntax, which the vault format spec excludes and the parser does not read. Seed and test fixture now use block lists. --- stacklets/memory/lib.py | 33 ++--- .../seeds/_shared/correspondents/README.md | 4 +- tests/stacklets/test_memory_correspondents.py | 4 +- tests/stacklets/test_memory_host_stdlib.py | 116 ++++++++++++++++++ 4 files changed, 135 insertions(+), 22 deletions(-) create mode 100644 tests/stacklets/test_memory_host_stdlib.py 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/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") == []