fix(slm): declaration-gate every group the update playbook deploys to (#14552)

autobot-slm-backend/api/nodes.py

@@ -828,6 +828,34 @@ async def _run_preflight(node_id: str, role_name: str, db: AsyncSession) -> Pref
     )
 
 
+async def _declare_role_on_node(db: AsyncSession, node_id: str, role_name: str) -> None:
+    """Record the role on ``Node.roles`` as well as the NodeRole table (#14552).
+
+    Assigning a role through the UI IS a declaration -- but this endpoint only
+    ever wrote a NodeRole row, leaving ``Node.roles`` untouched.
+
+    That was invisible until deploy groups became declaration-gated (#14513).
+    Now ``_strip_undeclared_privileged_groups`` reads ``Node.roles`` as the sole
+    record of operator intent, so a role assigned through "Assign Role Manually"
+    would be treated as undeclared: the group is stripped, the update playbook's
+    tasks for it never fire, and the component silently never installs. The
+    admin sees the assignment succeed and nothing happens.
+
+    ``api/npu.py`` already writes ``node.roles`` for its dedicated NPU modal,
+    which is why npu-worker never showed the problem. This brings the generic
+    path in line with it.
+    """
+    result = await db.execute(select(Node).where(Node.node_id == node_id))
+    node = result.scalar_one_or_none()
+    if node is None:
+        return
+
+    current = list(node.roles or [])
+    if role_name not in current:
+        node.roles = current + [role_name]
+        logger.info("Declared role on node: %s -> %s", node_id, role_name)
+
+
 async def _upsert_node_role(
     db: AsyncSession,
     node_id: str,
@@ -844,6 +872,7 @@ async def _upsert_node_role(
 
     if existing:
         existing.assignment_type = role_request.assignment_type
+        await _declare_role_on_node(db, node_id, role_request.role_name)
         await db.commit()
         await db.refresh(existing)
         logger.info(
@@ -861,6 +890,7 @@ async def _upsert_node_role(
         status="not_installed",
     )
     db.add(node_role)
+    await _declare_role_on_node(db, node_id, role_request.role_name)
     await db.commit()
     await db.refresh(node_role)
     logger.info(

autobot-slm-backend/api/nodes_role_declaration_test.py

@@ -0,0 +1,102 @@
+# Copyright 2025-2026 mrveiss
+# SPDX-License-Identifier: Apache-2.0
+# AutoBot - AI-Powered Automation Platform
+# Author: mrveiss
+"""Assigning a role through the UI must declare it on the node (#14552).
+
+"Assign Role Manually" wrote a `NodeRole` row and nothing else. `Node.roles`
+was left untouched, which did not matter until deploy groups became
+declaration-gated (#14513): `_strip_undeclared_privileged_groups` reads
+`Node.roles` as the sole record of operator intent.
+
+After that, a role assigned through the supported UI counted as UNDECLARED. The
+group was stripped, the update playbook's tasks for it never fired, and the
+component silently never installed — while the admin saw the assignment
+succeed. `api/npu.py` already wrote `node.roles` for its dedicated NPU modal,
+which is why npu-worker alone never showed the problem.
+
+The function is AST-extracted and executed rather than imported, following
+`nodes_test.py`: importing `api/nodes.py` registers FastAPI routes.
+"""
+
+from __future__ import annotations
+
+import ast
+import asyncio
+from pathlib import Path
+from types import SimpleNamespace
+
+_NODES_PY = Path(__file__).parent / "nodes.py"
+_WANTED = {"_declare_role_on_node"}
+
+
+def _load() -> dict:
+    tree = ast.parse(_NODES_PY.read_text(encoding="utf-8"), filename=str(_NODES_PY))
+    wanted = [n for n in tree.body if isinstance(n, ast.AsyncFunctionDef) and n.name in _WANTED]
+    assert len(wanted) == len(
+        _WANTED
+    ), f"expected {sorted(_WANTED)} in {_NODES_PY}, found {sorted(n.name for n in wanted)} — rename tracking broke"
+    module = ast.Module(body=wanted, type_ignores=[])
+    ast.fix_missing_locations(module)
+
+    # `select(Node).where(Node.node_id == ...)` must be constructible: the query
+    # object is handed straight to the fake session, so only its shape matters.
+    class _Query:
+        def where(self, *_a, **_k):
+            return self
+
+    namespace: dict = {
+        "select": lambda *_a, **_k: _Query(),
+        "Node": SimpleNamespace(node_id=object()),
+        "logger": SimpleNamespace(info=lambda *_a, **_k: None),
+        "AsyncSession": object,
+    }
+    exec(compile(module, filename=str(_NODES_PY), mode="exec"), namespace)  # noqa: S102
+    return namespace
+
+
+_declare_role_on_node = _load()["_declare_role_on_node"]
+
+
+class _FakeDb:
+    """Minimal stand-in: the function only executes a select and reads one row."""
+
+    def __init__(self, node):
+        self._node = node
+
+    async def execute(self, _query):
+        return SimpleNamespace(scalar_one_or_none=lambda: self._node)
+
+
+def _run(node, role_name):
+    asyncio.run(_declare_role_on_node(_FakeDb(node), "node-under-test", role_name))
+    return node
+
+
+def test_assigning_a_role_declares_it():
+    """The #14552 regression: the assignment must reach `Node.roles`."""
+    node = _run(SimpleNamespace(node_id="node-under-test", roles=["vnc"]), "frontend")
+
+    assert "frontend" in node.roles, (
+        "a manually assigned role never reached Node.roles, so the deploy group is stripped "
+        "and the component silently never installs (#14552)"
+    )
+    assert "vnc" in node.roles, "existing declared roles were discarded"
+
+
+def test_assigning_twice_does_not_duplicate():
+    node = _run(SimpleNamespace(node_id="node-under-test", roles=["frontend"]), "frontend")
+
+    assert node.roles.count("frontend") == 1
+
+
+def test_a_node_with_no_roles_yet_is_handled():
+    """`roles` is nullable, and `None` must not raise."""
+    node = _run(SimpleNamespace(node_id="node-under-test", roles=None), "ai-stack")
+
+    assert node.roles == ["ai-stack"]
+
+
+def test_a_missing_node_is_a_no_op_not_a_crash():
+    """The caller already 404s on an unknown node; this must not raise first."""
+    asyncio.run(_declare_role_on_node(_FakeDb(None), "gone", "frontend"))

autobot-slm-backend/services/inventory_builder.py

@@ -248,7 +248,16 @@ def _union_roles(node: Any) -> list[str]:
 # node genuinely running redis should keep receiving redis updates. The line is
 # drawn at groups whose plays were checked and found to deploy a tree or migrate
 # a database, not at "detection is untrusted".
-_DECLARED_ONLY_GROUPS = frozenset({"slm_server", "backend", "main"})
+# #14552: `frontend` was missing, and the omission is instructive. Play 2 gates
+# its deploy tasks on exactly two groups -- `backend` and `frontend` -- and
+# #14513 gated only the first, so a vnc node still ran `npx vite build` and
+# failed for want of Node.js. Hand-picking the members is what let that recur;
+# `inventory_deploy_groups_test.py` now DERIVES this set from the playbook and
+# fails if a component is added there without being listed here. That
+# derivation immediately found three MORE gates I had also missed by reading
+# one range of the file -- `aiml`, `npu` and `browser` -- which is the whole
+# argument against maintaining this list by eye.
+_DECLARED_ONLY_GROUPS = frozenset({"slm_server", "backend", "main", "frontend", "aiml", "npu", "browser"})
 
 
 def _strip_undeclared_privileged_groups(node: Any, node_groups: set[str]) -> set[str]:

autobot-slm-backend/services/inventory_deploy_groups_test.py

@@ -0,0 +1,134 @@
+# Copyright 2025-2026 mrveiss
+# SPDX-License-Identifier: Apache-2.0
+# AutoBot - AI-Powered Automation Platform
+# Author: mrveiss
+"""Every group whose play deploys a component must be declaration-gated (#14552).
+
+#14513 stopped detection granting `slm_server`, `backend` and `main`, because
+those plays unpack a tree and run migrations. It missed `frontend`, and the gap
+showed up in production as::
+
+    TASK [[PLAY 2] Frontend | Rebuild production dist]
+    fatal: cmd "npx vite build" -> [Errno 2] No such file or directory: b'npx'
+
+on a `[vnc, slm-agent]` node with no Node.js. Play 2 gates its deploy tasks on
+exactly two groups, `backend` and `frontend`; gating one of them left the other
+half of the same play wide open.
+
+The lesson is not "add frontend" -- it is that a hand-picked list drifts from
+the playbook it is supposed to mirror. So the expected set is DERIVED here from
+`update-all-nodes.yml`, and this fails if a component is ever added to that play
+without being added to `_DECLARED_ONLY_GROUPS`.
+
+The constant stays in code: production must not parse a playbook to build an
+inventory. Only the assertion is derived.
+"""
+
+from __future__ import annotations
+
+import importlib.util
+import re
+import sys
+from pathlib import Path
+
+import pytest
+
+yaml = pytest.importorskip("yaml")
+
+_SLM_ROOT = Path(__file__).resolve().parent.parent
+_PLAYBOOK = _SLM_ROOT / "ansible" / "playbooks" / "update-all-nodes.yml"
+
+
+def _load_inventory_builder():
+    name = "_inventory_builder_deploy_groups"
+    spec = importlib.util.spec_from_file_location(name, _SLM_ROOT / "services" / "inventory_builder.py")
+    module = importlib.util.module_from_spec(spec)
+    sys.modules[name] = module
+    try:
+        spec.loader.exec_module(module)
+    finally:
+        sys.modules.pop(name, None)
+    return module
+
+
+inventory_builder = _load_inventory_builder()
+
+
+def _groups_gating_deploy_tasks() -> set[str]:
+    """Groups the update playbook gates component deploys on.
+
+    Read as text rather than parsed: the gates are Jinja expressions inside
+    `when:`, and what matters is which group name each one tests.
+    """
+    text = _PLAYBOOK.read_text(encoding="utf-8")
+
+    # Every `'x' in group_names` occurrence, whatever syntax carries it.
+    #
+    # Review of #14552: an earlier version anchored on `when:` and double
+    # quotes, which misses the folded-scalar and list forms -- already the
+    # dominant style for the npu and browser gates in this very file. It
+    # happened to produce the right answer only because each of those groups
+    # also appears once in the single-line form. A new gate written in the
+    # folded style would have been omitted silently, and the rule would have
+    # reported "missing: none" while the hole it exists to catch was open.
+    return set(re.findall(r"'([a-z_]+)'\s+in\s+group_names", text))
+
+
+def _plays_targeting_a_group_directly() -> set[str]:
+    """Groups a whole play targets via `hosts:` — Play 1 uses this for slm_server."""
+    documents = list(yaml.safe_load_all(_PLAYBOOK.read_text(encoding="utf-8")))
+    hosts = set()
+    for document in documents:
+        for play in document if isinstance(document, list) else [document]:
+            if isinstance(play, dict) and isinstance(play.get("hosts"), str):
+                hosts.add(play["hosts"])
+    return hosts
+
+
+def test_the_derivation_finds_the_playbook_and_its_gates():
+    """A derivation that finds nothing would make the rule below vacuous."""
+    assert _PLAYBOOK.is_file(), f"{_PLAYBOOK} is gone — this rule is pinned to the wrong path"
+
+    gates = _groups_gating_deploy_tasks()
+
+    assert gates, "no `'x' in group_names` gates parsed — the playbook's shape changed"
+    assert (
+        "backend" in gates and "frontend" in gates
+    ), f"expected the two known deploy gates in Play 2, found {sorted(gates)}"
+
+
+def test_every_deploy_gated_group_is_declaration_gated():
+    """The #14552 invariant.
+
+    If the update playbook will deploy a component to whoever is in group X,
+    then landing in X must require the operator to have declared it.
+    """
+    missing = sorted(_groups_gating_deploy_tasks() - set(inventory_builder._DECLARED_ONLY_GROUPS))
+
+    assert not missing, (
+        f"group(s) receive a component deploy but detection can still grant them: {missing}. "
+        "A node that never declared the role gets the tree unpacked and its build/restart tasks run "
+        "(#14552 — this is how `frontend` was missed after #14513)"
+    )
+
+
+def test_play_one_target_is_declaration_gated():
+    """Play 1 targets `slm_server` by `hosts:`, not by a group_names gate."""
+    slm_plays = _plays_targeting_a_group_directly() & {"slm_server"}
+
+    assert slm_plays, "no play targets slm_server directly — Play 1's shape changed"
+    assert "slm_server" in inventory_builder._DECLARED_ONLY_GROUPS
+
+
+def test_ordinary_groups_are_not_swept_in():
+    """The gate must stay narrow enough that detection still adds ordinary groups.
+
+    `redis`/`database` are granted by detection on purpose — a node genuinely
+    running redis should keep receiving redis updates, which
+    `test_detected_roles_merged_with_roles` pins independently.
+    """
+    gated = set(inventory_builder._DECLARED_ONLY_GROUPS)
+
+    assert (
+        "redis" not in gated and "database" not in gated
+    ), "redis/database became declaration-gated — that is a larger behaviour change than #14552"