feat(mcp): update_group_members puts hosts into an inventory group or takes them out (#5487)

#5487 "MCP: edit inventory group membership". Group membership decides which
hosts a group alert rule covers, and the MCP could only read it. The new
operator tool takes a group and lists to add and remove, each a target's
name or id or the Steward host it is linked to; an unknown name refuses the
whole change. It answers with the group's targets after and the alert rules
covering the group (whose per-host rules follow within a minute), and audits
as ansible.group.updated by "<user> via MCP", as the Inventory page does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs/core/mcp.md

@@ -84,7 +84,7 @@ Each role includes the tools of the roles above it in this list.
 
 | Area | Tools |
 |---|---|
-| Automation | `run_playbook`, `cancel_ansible_run`, `create_schedule`, `update_schedule`, `delete_schedule` |
+| Automation | `run_playbook`, `cancel_ansible_run`, `create_schedule`, `update_schedule`, `delete_schedule`, `update_group_members` (put hosts into an inventory group or take them out; a group alert rule follows within a minute) |
 | Monitors | `create_monitor`, `update_monitor`, `set_monitor_enabled` |
 | Alerts | `create_alert_rule`, `update_alert_rule`, `acknowledge_alert`, `send_test_alert`, `create_maintenance_window`, `end_maintenance_window` |
 

steward/mcp/tools/automation.py

@@ -1,4 +1,5 @@
-"""Automation tools: playbooks, inventory, runs, schedules and alert actions.
+"""Automation tools: playbooks, inventory and its groups, runs, schedules and
+alert actions.
 
 Reads are viewer tools. Anything that starts a run or changes what runs is an
 operator tool (set_alert_action is admin, as on the alert rule form), and each
@@ -267,6 +268,77 @@ async def list_inventory() -> dict:
     }
 
 
+def member_targets(names: list[str] | None, targets: list, host_names: dict[str, str]) -> list:
+    """The targets `names` mean, each a target's name or id, or the name of the
+    Steward host a target is linked to. Refuses the whole list when one is
+    unknown, so nothing changes on a typo."""
+    by_host = {host_names[t.host_id].lower(): t for t in targets
+               if t.host_id and host_names.get(t.host_id)}
+    found, unknown = [], []
+    for want in names or []:
+        want = (want or "").strip()
+        if not want:
+            continue
+        t = next((t for t in targets if t.id == want or t.name.lower() == want.lower()), None) \
+            or by_host.get(want.lower())
+        (found if t else unknown).append(t or want)
+    if unknown:
+        raise ValueError(f"no target or linked host named {', '.join(map(repr, unknown))}; "
+                         "list_inventory lists them. Nothing was changed.")
+    return list({t.id: t for t in found}.values())
+
+
+async def update_group_members(group: str, add: list[str] | None = None,
+                               remove: list[str] | None = None) -> dict:
+    """Put targets into an inventory group, or take them out.
+
+    Inventory groups are Steward's grouping system: an alert rule covering a
+    group watches every host in it, the Hosts list filters by them, and a run
+    can be scoped to one. A host is in a group through its linked target.
+
+    group: the group's name or id (list_inventory).
+    add, remove: targets by name or id, or by the name of the Steward host a
+      target is linked to. One already in (or out of) the group is left as it
+      is; an unknown name refuses the whole change.
+
+    Returns the group's targets after the change and the alert rules that
+    cover it; their per-host rules follow within a minute. Recorded in the
+    audit log as "<you> via MCP".
+    """
+    if not (add or remove):
+        raise ValueError("pass add and/or remove: the targets to put in or take out.")
+    async with db_session() as db, db.begin():
+        g = pick(group, (await db.execute(
+            select(AnsibleGroup).options(selectinload(AnsibleGroup.targets)))).scalars(),
+                 kind="group", list_tool="list_inventory")
+        targets = list((await db.execute(select(AnsibleTarget))).scalars())
+        host_names = dict((await db.execute(select(Host.id, Host.name))).all())
+        to_add = member_targets(add, targets, host_names)
+        to_remove = member_targets(remove, targets, host_names)
+        both = {t.name for t in to_add} & {t.name for t in to_remove}
+        if both:
+            raise ValueError(f"{', '.join(sorted(both))} is in both add and remove. Nothing was changed.")
+        have = {t.id for t in g.targets}
+        added = [t for t in to_add if t.id not in have]
+        removed = [t for t in to_remove if t.id in have]
+        g.targets = [t for t in g.targets if t.id not in {r.id for r in removed}] + added
+        members = sorted(t.name for t in g.targets)
+        covering = sorted((await db.execute(select(AlertRule.name).where(
+            AlertRule.covers == "group", AlertRule.cover_group == g.name))).scalars())
+        name, gid = g.name, g.id
+    if added or removed:
+        # The Inventory page's own record of a group's membership changing.
+        await audit_mcp("ansible.group.updated", "ansible_group", gid,
+                        {"name": name, "targets": members,
+                         "added": sorted(t.name for t in added),
+                         "removed": sorted(t.name for t in removed)})
+    return {"group": name, "group_id": gid,
+            "added": sorted(t.name for t in added), "removed": sorted(t.name for t in removed),
+            "targets": members, "alert_rules": covering,
+            **({"note": "Alert rules covering this group pick up the change within a minute."}
+               if covering and (added or removed) else {})}
+
+
 # ── runs ─────────────────────────────────────────────────────────────────────
 
 def recap_totals(results: dict | None) -> dict | None:
@@ -712,6 +784,7 @@ def register(register_tool) -> None:
     for fn in (list_playbooks, get_playbook, list_inventory, list_ansible_runs,
                get_ansible_run, list_schedules, wait_for_run):
         register_tool(fn, role=UserRole.viewer, read_only=True)
-    for fn in (run_playbook, cancel_ansible_run, create_schedule, update_schedule, delete_schedule):
+    for fn in (run_playbook, cancel_ansible_run, create_schedule, update_schedule, delete_schedule,
+               update_group_members):
         register_tool(fn, role=UserRole.operator, read_only=False)
     register_tool(set_alert_action, role=UserRole.admin, read_only=False)

tests/integration/test_mcp_group_members.py

@@ -0,0 +1,80 @@
+"""Integration: update_group_members over MCP (#5487) against Postgres. A host
+joins a group by its linked target, a group rule covering that group is named
+in the answer, and the change is audited as "<user> via MCP"."""
+from __future__ import annotations
+
+import asyncio
+import json
+import os
+import uuid
+
+import pytest
+
+pytestmark = pytest.mark.integration
+
+
+@pytest.fixture
+def app():
+    if not os.environ.get("STEWARD_DATABASE_URL"):
+        pytest.skip("needs Postgres")
+    from steward.app import create_app
+    return create_app(testing=False)
+
+
+def test_a_host_joins_and_leaves_a_group_through_mcp(app):
+    from sqlalchemy import select
+    from steward.core.api_tokens import Caller
+    from steward.mcp import _context
+    from steward.mcp.tools import automation as a
+    from steward.models.alerts import AlertOperator, AlertRule
+    from steward.models.ansible_inventory import AnsibleGroup, AnsibleTarget
+    from steward.models.audit import AuditEvent
+    from steward.models.hosts import Host
+    from steward.models.users import User, UserRole
+
+    tag = uuid.uuid4().hex[:8]
+    uid, username = str(uuid.uuid4()), f"gm-op-{tag}"
+    group, host = f"gm-group-{tag}", f"gm-host-{tag}"
+
+    async def _go():
+        async with app.db_sessionmaker() as s:
+            async with s.begin():
+                s.add(User(id=uid, username=username, email=f"{tag}@e.test",
+                           password_hash="x", role=UserRole.operator, is_active=True))
+                h = Host(name=host, address="10.9.9.9")
+                s.add(h)
+                await s.flush()
+                s.add(AnsibleTarget(name=f"gm-target-{tag}", address="10.9.9.9", host_id=h.id))
+                s.add(AnsibleTarget(name=f"gm-other-{tag}", address="10.9.9.8"))
+                s.add(AnsibleGroup(name=group))
+                s.add(AlertRule(name=f"gm-rule-{tag}", source_module="host_agent", resource_name="",
+                                metric_name="cpu_pct", operator=AlertOperator.gt, threshold=90.0,
+                                covers="group", cover_group=group, created_by=uid))
+        _context._app_ctx.set(app)
+        _context._caller_ctx.set(Caller(user_id=uid, username=username,
+                                        role=UserRole.operator, token_id=7))
+
+        got = await a.update_group_members(group, add=[host, f"gm-other-{tag}"])
+        assert got["targets"] == sorted([f"gm-other-{tag}", f"gm-target-{tag}"])
+        assert got["added"] == got["targets"] and got["alert_rules"] == [f"gm-rule-{tag}"]
+        assert "within a minute" in got["note"]
+        inv = await a.list_inventory()
+        assert next(t for t in inv["targets"] if t["host_name"] == host)["groups"] == [group]
+
+        # Adding again changes nothing; an unknown name refuses the lot.
+        again = await a.update_group_members(group, add=[f"gm-target-{tag}"])
+        assert again["added"] == [] and "note" not in again
+        with pytest.raises(ValueError, match="Nothing was changed"):
+            await a.update_group_members(group, remove=[host, "no-such-host"])
+
+        got = await a.update_group_members(group, remove=[host])
+        assert got["removed"] == [f"gm-target-{tag}"] and got["targets"] == [f"gm-other-{tag}"]
+
+        async with app.db_sessionmaker() as s:
+            rows = list((await s.execute(select(AuditEvent).where(
+                AuditEvent.user_id == uid, AuditEvent.action == "ansible.group.updated")
+                .order_by(AuditEvent.timestamp))).scalars())
+        assert len(rows) == 2 and rows[0].username == f"{username} via MCP"
+        assert json.loads(rows[1].detail_json)["removed"] == [f"gm-target-{tag}"]
+
+    asyncio.run(_go())

tests/mcp/test_automation.py

@@ -24,7 +24,7 @@ VARS = [
 VIEWER_TOOLS = {"list_playbooks", "get_playbook", "list_inventory", "list_ansible_runs",
                 "get_ansible_run", "list_schedules", "wait_for_run"}
 OPERATOR_TOOLS = {"run_playbook", "cancel_ansible_run", "create_schedule", "update_schedule",
-                  "delete_schedule"}
+                  "delete_schedule", "update_group_members"}
 
 
 def test_split_vars_keeps_secrets_apart_from_what_is_stored():
@@ -156,3 +156,15 @@ def test_schedule_update_holds_only_changed_variables(monkeypatch):
     automation.check_caller_vars({**stored, "release": "2"}, stored)
     with pytest.raises(ValueError, match="Only an admin"):
         automation.check_caller_vars({"ansible_ssh_common_args": "-o ProxyCommand=x"}, stored)
+
+
+def test_group_members_are_named_by_target_or_by_linked_host():
+    """#5487: a target by its name or id, or the Steward host linked to it; an
+    unknown name refuses the lot, so a typo changes nothing."""
+    web = NS(id="t1", name="web01", host_id="h1")
+    db = NS(id="t2", name="db01", host_id=None)
+    hosts = {"h1": "VDNT-Docker02"}
+    got = automation.member_targets(["vdnt-docker02", "t2", "web01", " "], [web, db], hosts)
+    assert [t.name for t in got] == ["web01", "db01"]
+    with pytest.raises(ValueError, match="no target or linked host named 'nas'.*Nothing was changed"):
+        automation.member_targets(["web01", "nas"], [web, db], hosts)