From 5bf4d4d8acbc446cbfb896df29e9bb68ac0d32d7 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Wed, 2 Sep 2026 01:01:14 +0200 Subject: [PATCH] refactor(variants)!: let an MCP round decide for itself what it carries The mcp-consumer-variant guard demanded three things of a provider's MCP round: every admitted consumer enabled, each of them pinned `services: null` so it dragged no dependencies along, and no other service left enabled. Two of those were the rule legislating a round's contents rather than checking that the pairing gets proven at all. Keep the first and drop the rest. What else the round carries is the role's own call, so the tor/litellm exemption, the datastore exemption and the `allowed` set they fed have no reader left and go with them. Strip the 63 `services: null` pins the rule forced across 20 variants, all written bare as `services:`. The topic-emptying capability behind them stays: apply_topic still normalises None to {} for the inventory payload, and the unit tests still cover it. Only the uses and the mandate are gone. That leaves overridden_providers scanning a repository with nothing left to find, so its three assertions passed over an empty dict and would have stayed green however the function broke. Rebuild them on a synthetic role tree, which also covers the case the repository scan never held: a variant naming a plain flag is not an override. Removing the pins changes what deploys. Measured across all 20 variants: 18 identical, prometheus#1 grows 23 to 31 roles, moodle#2 grows 31 to 32 and crosses the memory budget until the following commit shrinks it back. Co-Authored-By: Claude Opus 5 (1M context) --- roles/svc-db-qdrant/meta/variants.yml | 4 - roles/web-app-baserow/meta/variants.yml | 3 - roles/web-app-checkmk/meta/variants.yml | 3 - roles/web-app-fider/meta/variants.yml | 3 - roles/web-app-gitea/meta/variants.yml | 3 - roles/web-app-gitlab/meta/variants.yml | 3 - roles/web-app-homeassistant/meta/variants.yml | 4 - roles/web-app-jellyfin/meta/variants.yml | 3 - roles/web-app-jenkins/meta/variants.yml | 3 - roles/web-app-listmonk/meta/variants.yml | 3 - roles/web-app-mattermost/meta/variants.yml | 3 - roles/web-app-moodle/meta/variants.yml | 4 - roles/web-app-n8n/meta/variants.yml | 3 - roles/web-app-nextcloud/meta/variants.yml | 3 - roles/web-app-pretix/meta/variants.yml | 3 - roles/web-app-prometheus/meta/variants.yml | 4 - roles/web-app-snipe-it/meta/variants.yml | 3 - roles/web-app-wordpress/meta/variants.yml | 2 - roles/web-app-zammad/meta/variants.yml | 3 - .../web-svc-libretranslate/meta/variants.yml | 3 - .../variants/test_mcp_consumer_variant.py | 72 ++++-------------- .../utils/roles/applications/test_topics.py | 73 +++++++++++++++---- 22 files changed, 70 insertions(+), 138 deletions(-) diff --git a/roles/svc-db-qdrant/meta/variants.yml b/roles/svc-db-qdrant/meta/variants.yml index 85878423d4..0546253d99 100644 --- a/roles/svc-db-qdrant/meta/variants.yml +++ b/roles/svc-db-qdrant/meta/variants.yml @@ -23,16 +23,12 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: flowise: enabled: true shared: true - services: diff --git a/roles/web-app-baserow/meta/variants.yml b/roles/web-app-baserow/meta/variants.yml index 032260691f..9037a469d0 100644 --- a/roles/web-app-baserow/meta/variants.yml +++ b/roles/web-app-baserow/meta/variants.yml @@ -83,12 +83,9 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: diff --git a/roles/web-app-checkmk/meta/variants.yml b/roles/web-app-checkmk/meta/variants.yml index ea78be6498..7f02bdf42c 100644 --- a/roles/web-app-checkmk/meta/variants.yml +++ b/roles/web-app-checkmk/meta/variants.yml @@ -72,15 +72,12 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: - services: container_backup: enabled: false diff --git a/roles/web-app-fider/meta/variants.yml b/roles/web-app-fider/meta/variants.yml index 837322f47e..3af4570eeb 100644 --- a/roles/web-app-fider/meta/variants.yml +++ b/roles/web-app-fider/meta/variants.yml @@ -78,12 +78,9 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: diff --git a/roles/web-app-gitea/meta/variants.yml b/roles/web-app-gitea/meta/variants.yml index 8d2ed419cf..5da73eb89a 100644 --- a/roles/web-app-gitea/meta/variants.yml +++ b/roles/web-app-gitea/meta/variants.yml @@ -89,15 +89,12 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: - services: container_backup: enabled: false diff --git a/roles/web-app-gitlab/meta/variants.yml b/roles/web-app-gitlab/meta/variants.yml index fc409d21ea..96606d26fd 100644 --- a/roles/web-app-gitlab/meta/variants.yml +++ b/roles/web-app-gitlab/meta/variants.yml @@ -83,12 +83,9 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: diff --git a/roles/web-app-homeassistant/meta/variants.yml b/roles/web-app-homeassistant/meta/variants.yml index 64421bc9a7..50ff180866 100644 --- a/roles/web-app-homeassistant/meta/variants.yml +++ b/roles/web-app-homeassistant/meta/variants.yml @@ -41,16 +41,12 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: flowise: enabled: true shared: true - services: diff --git a/roles/web-app-jellyfin/meta/variants.yml b/roles/web-app-jellyfin/meta/variants.yml index 51bf9ff461..71c2ab67a0 100644 --- a/roles/web-app-jellyfin/meta/variants.yml +++ b/roles/web-app-jellyfin/meta/variants.yml @@ -66,15 +66,12 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: - services: container_backup: enabled: false diff --git a/roles/web-app-jenkins/meta/variants.yml b/roles/web-app-jenkins/meta/variants.yml index e741b63b9e..72b1bead94 100644 --- a/roles/web-app-jenkins/meta/variants.yml +++ b/roles/web-app-jenkins/meta/variants.yml @@ -71,15 +71,12 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: - services: container_backup: enabled: false diff --git a/roles/web-app-listmonk/meta/variants.yml b/roles/web-app-listmonk/meta/variants.yml index fbdbdb2d85..61308c1211 100644 --- a/roles/web-app-listmonk/meta/variants.yml +++ b/roles/web-app-listmonk/meta/variants.yml @@ -80,12 +80,9 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: diff --git a/roles/web-app-mattermost/meta/variants.yml b/roles/web-app-mattermost/meta/variants.yml index ed775be250..4207624f41 100644 --- a/roles/web-app-mattermost/meta/variants.yml +++ b/roles/web-app-mattermost/meta/variants.yml @@ -89,15 +89,12 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: - services: container_backup: diff --git a/roles/web-app-moodle/meta/variants.yml b/roles/web-app-moodle/meta/variants.yml index 38ad4dfa31..73e8b1cf5f 100644 --- a/roles/web-app-moodle/meta/variants.yml +++ b/roles/web-app-moodle/meta/variants.yml @@ -129,18 +129,14 @@ litellm: enabled: true shared: true - services: tor: enabled: true openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: diff --git a/roles/web-app-n8n/meta/variants.yml b/roles/web-app-n8n/meta/variants.yml index 3bcc556118..adf775c880 100644 --- a/roles/web-app-n8n/meta/variants.yml +++ b/roles/web-app-n8n/meta/variants.yml @@ -75,12 +75,9 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: diff --git a/roles/web-app-nextcloud/meta/variants.yml b/roles/web-app-nextcloud/meta/variants.yml index 10d5a4f763..9c7c418051 100644 --- a/roles/web-app-nextcloud/meta/variants.yml +++ b/roles/web-app-nextcloud/meta/variants.yml @@ -191,7 +191,6 @@ openwebui: enabled: true shared: true - services: litellm: enabled: false shared: false # nocheck: variants-static-override backup shared stays literally true @@ -206,11 +205,9 @@ hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: addons: &addons_static_off appointments: enabled: false diff --git a/roles/web-app-pretix/meta/variants.yml b/roles/web-app-pretix/meta/variants.yml index ca3d40d1ee..14d24d0757 100644 --- a/roles/web-app-pretix/meta/variants.yml +++ b/roles/web-app-pretix/meta/variants.yml @@ -77,12 +77,9 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: diff --git a/roles/web-app-prometheus/meta/variants.yml b/roles/web-app-prometheus/meta/variants.yml index a2f43d14f1..7fb470edd3 100644 --- a/roles/web-app-prometheus/meta/variants.yml +++ b/roles/web-app-prometheus/meta/variants.yml @@ -67,16 +67,12 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: flowise: enabled: true shared: true - services: diff --git a/roles/web-app-snipe-it/meta/variants.yml b/roles/web-app-snipe-it/meta/variants.yml index 6bbfde59fd..cd0b18b92a 100644 --- a/roles/web-app-snipe-it/meta/variants.yml +++ b/roles/web-app-snipe-it/meta/variants.yml @@ -89,15 +89,12 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: - services: container_backup: enabled: false diff --git a/roles/web-app-wordpress/meta/variants.yml b/roles/web-app-wordpress/meta/variants.yml index 0e59d2b295..aeb0a46fac 100644 --- a/roles/web-app-wordpress/meta/variants.yml +++ b/roles/web-app-wordpress/meta/variants.yml @@ -91,11 +91,9 @@ hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: addons: activitypub: enabled: false diff --git a/roles/web-app-zammad/meta/variants.yml b/roles/web-app-zammad/meta/variants.yml index c9629385c9..225c028f71 100644 --- a/roles/web-app-zammad/meta/variants.yml +++ b/roles/web-app-zammad/meta/variants.yml @@ -94,15 +94,12 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: - services: container_backup: enabled: false diff --git a/roles/web-svc-libretranslate/meta/variants.yml b/roles/web-svc-libretranslate/meta/variants.yml index f5b7b94d00..998cdc6080 100644 --- a/roles/web-svc-libretranslate/meta/variants.yml +++ b/roles/web-svc-libretranslate/meta/variants.yml @@ -51,12 +51,9 @@ openwebui: enabled: true shared: true - services: hermes: enabled: true shared: true - services: openclaw: enabled: true shared: true - services: diff --git a/tests/integration/roles/meta/variants/test_mcp_consumer_variant.py b/tests/integration/roles/meta/variants/test_mcp_consumer_variant.py index da8da1d61a..68e9d3590b 100644 --- a/tests/integration/roles/meta/variants/test_mcp_consumer_variant.py +++ b/tests/integration/roles/meta/variants/test_mcp_consumer_variant.py @@ -1,19 +1,9 @@ -"""Integration guard: an MCP provider ships one round that is only MCP. +"""Integration guard: an MCP provider ships a round that proves the pairing. -A provider's clients cost a full application stack each, so the round that -proves the MCP pairing must not also carry the role's SSO, mail, metrics and -dashboard surfaces. Every role with a ``meta/mcp.yml`` that admits at least -one consumer therefore needs one variant that is exactly that pairing: - -* every admitted consumer pinned ``enabled: true`` with ``services: null``, - so the client is deployed but drags none of its own dependencies along, -* every other service pinned ``enabled: false``. - -``tor`` is exempt because the onion round is a deploy axis rather than a -feature of the role, ``litellm`` because the gateway is what the clients -answer prompts through, and so are the role's own entity and any service its -``meta/services.yml`` pins to a literal ``true`` — the datastores, which a -variant cannot switch off without taking the application with them. +A surface nobody deploys against is never exercised, so every role with a +``meta/mcp.yml`` that admits at least one consumer needs one variant pinning +every admitted consumer to ``enabled: true``. What else that variant carries +is the role's own call. Suppression (see ``docs/contributing/actions/testing/suppression.md``): @@ -37,37 +27,17 @@ from . import PROJECT_ROOT _RULE = "mcp-consumer-variant" _SERVER_DIRECTIONS = frozenset({"server", "both"}) -_ALWAYS_ALLOWED = frozenset({"tor", "litellm"}) ROLES_DIR = PROJECT_ROOT / "roles" -def _statically_on(base_services: Mapping) -> set[str]: - """Return service keys a variant cannot switch off. - - Args: - base_services: the role's ``meta/services.yml`` mapping. - - A literal ``enabled: true`` in the base is the datastore shape: it states - a fact about what the application needs, not a per-round choice. - """ - return { - key - for key, entry in base_services.items() - if isinstance(entry, Mapping) and entry.get("enabled") is True - } - - -def _variant_findings( - role: str, consumers: set[str], variant: Mapping, allowed: set[str] -) -> list[str]: - """Return why one variant is not the role's MCP-only round. +def _variant_findings(role: str, consumers: set[str], variant: Mapping) -> list[str]: + """Return why one variant does not prove the role's MCP pairing. Args: role: the provider role id. consumers: entity keys of every admitted consumer. variant: one ``meta/variants.yml`` entry. - allowed: keys that may stay on besides the consumers. """ services = variant.get("services") if not isinstance(services, Mapping): @@ -77,21 +47,11 @@ def _variant_findings( entry = services.get(key) if not isinstance(entry, Mapping) or entry.get("enabled") is not True: findings.append(f"{role}: {key} is not enabled") - elif "services" not in entry or entry["services"] is not None: - findings.append(f"{role}: {key} lacks `services: null`") - findings.extend( - f"{role}: {key} stays enabled" - for key, entry in sorted(services.items()) - if isinstance(entry, Mapping) - and entry.get("enabled") is True - and key not in consumers - and key not in allowed - ) return findings def offenders() -> list[str]: - """Return one finding per provider without an MCP-only variant.""" + """Return one finding per provider that never deploys its consumers.""" defaults = get_application_defaults(roles_dir=ROLES_DIR) services_by_role = { role: (config or {}).get("services") @@ -114,14 +74,8 @@ def offenders() -> list[str]: } if not consumers: continue - base = services_by_role.get(role) - allowed = ( - _ALWAYS_ALLOWED - | {get_entity_name(role)} - | (_statically_on(base) if isinstance(base, Mapping) else set()) - ) per_variant = [ - _variant_findings(role, consumers, v, allowed) + _variant_findings(role, consumers, v) for v in variants.get(role, []) if isinstance(v, Mapping) ] @@ -129,19 +83,19 @@ def offenders() -> list[str]: continue closest = min(per_variant, key=len) if per_variant else [f"{role}: no variants"] findings.append( - f"{role}: no variant is the MCP-only round; the closest one still " - f"reports: {'; '.join(closest)}" + f"{role}: no variant enables every admitted consumer; the closest " + f"one still reports: {'; '.join(closest)}" ) return findings class TestMcpConsumerVariant(unittest.TestCase): - def test_every_provider_ships_an_mcp_only_variant(self) -> None: + def test_every_provider_ships_a_variant_enabling_its_consumers(self) -> None: findings = offenders() self.assertEqual( [], findings, - f"MCP provider(s) without an MCP-only variant ({len(findings)}):\n" + f"MCP provider(s) whose consumers never deploy ({len(findings)}):\n" + "\n".join(f" - {f}" for f in findings), ) diff --git a/tests/unit/python/utils/roles/applications/test_topics.py b/tests/unit/python/utils/roles/applications/test_topics.py index 80d2fc625f..22a729ec8b 100644 --- a/tests/unit/python/utils/roles/applications/test_topics.py +++ b/tests/unit/python/utils/roles/applications/test_topics.py @@ -7,8 +7,9 @@ The three override shapes have to stay distinguishable: ``null`` empties, from __future__ import annotations import unittest +from pathlib import Path +from tempfile import TemporaryDirectory -from utils.cache.files import PROJECT_ROOT from utils.roles.applications.topics import ( CONFIG_TOPICS, apply_topic, @@ -41,25 +42,65 @@ class TestApplyTopic(unittest.TestCase): class TestOverriddenProviders(unittest.TestCase): - @classmethod - def setUpClass(cls) -> None: - cls.found = overridden_providers(PROJECT_ROOT / "roles") + """The scan runs against a synthetic tree. - def test_the_scan_finds_the_declared_overrides(self) -> None: - self.assertTrue(self.found) + No variant in ``roles/`` dictates a provider's config today, so scanning + the real one would assert nothing and stay green however the function + breaks. + """ + + def setUp(self) -> None: + self._tmp = TemporaryDirectory() + self.roles = Path(self._tmp.name) + self.addCleanup(self._tmp.cleanup) + + def _role(self, name: str, services: str = "", variants: str = "") -> None: + """Write the minimum a role needs to reach the service registry. + + Args: + name: role directory name, also its ``application_id``. + services: body of ``meta/services.yml``. + variants: body of ``meta/variants.yml``, omitted when empty. + """ + meta = self.roles / name / "meta" + meta.mkdir(parents=True) + variables = self.roles / name / "vars" + variables.mkdir(parents=True) + (variables / "main.yml").write_text( + f"application_id: {name}\n", encoding="utf-8" + ) + (meta / "services.yml").write_text(services or "{}\n", encoding="utf-8") + if variants: + (meta / "variants.yml").write_text(variants, encoding="utf-8") + + def test_it_records_the_provider_a_variant_dictates(self) -> None: + self._role("svc-provider", "provider:\n shared: true\n") + self._role( + "web-app-consumer", + "provider:\n enabled: true\n", + "- services:\n provider:\n enabled: true\n services: null\n", + ) + self.assertEqual( + {("web-app-consumer", 0): {"svc-provider"}}, + overridden_providers(self.roles), + ) + + def test_a_plain_flag_is_not_an_override(self) -> None: + self._role("svc-provider", "provider:\n shared: true\n") + self._role( + "web-app-consumer", + "provider:\n enabled: true\n", + "- services:\n provider:\n enabled: true\n", + ) + self.assertEqual({}, overridden_providers(self.roles)) def test_a_role_never_records_itself(self) -> None: - self.assertEqual( - [], - [(role, i) for (role, i), targets in self.found.items() if role in targets], + self._role( + "svc-provider", + "provider:\n shared: true\n", + "- services:\n provider:\n services: null\n", ) - - def test_every_target_is_a_role_that_exists(self) -> None: - roles = {p.name for p in (PROJECT_ROOT / "roles").iterdir() if p.is_dir()} - missing = sorted( - {target for targets in self.found.values() for target in targets} - roles - ) - self.assertEqual([], missing) + self.assertEqual({}, overridden_providers(self.roles)) if __name__ == "__main__":