feat(mcp): monitoring-config write tools — monitors, alert rules, acks, maintenance

MCP-7 #4971 "Monitoring-config write tools: monitors, alert rules, acks,
maintenance".

The validation the forms did inline (or not at all) moves to the services,
and both the routes and the tools call it, so bad input is refused the same
way with the same reason:
- monitors/service: monitor_fields, build_config, normalise_target. Port
  1-65535, status 100-599, timeout 1-60, method GET/HEAD/POST, headers an
  object of strings, a key the type does not take is named. The forms now
  answer 400 with the reason instead of redirecting silently or a 500.
- alerts/service: METRIC_CATALOG and SOURCE_MODULES, resources_for,
  metrics_for, rule_fields (source, resource and metric from the same
  choices the rule form offers), add_rule, acknowledge, window_fields.

Tools: create_monitor, update_monitor (config merged over the stored one;
a rename warns about rules still watching the old name),
set_monitor_enabled, create_alert_rule, update_alert_rule,
acknowledge_alert, create_maintenance_window (duration or end),
end_maintenance_window (ends one in progress, cancels an upcoming one) —
operator. delete_monitor, delete_alert_rule — admin. get_alert_rule_options
— viewer. Every write is audited as "<user> via MCP".

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

tests/core/test_monitors.py

@@ -8,7 +8,10 @@ import asyncio
 
 from steward.models.monitors import Monitor, MONITOR_TYPE_VALUES
 from steward.monitors import runner as runner_mod
-from steward.monitors.routes import _build_config, _normalise_target
+import pytest
+
+from steward.monitors.routes import _form_opts
+from steward.monitors.service import build_config, monitor_fields, normalise_target
 
 
 def _monitor(mtype, target="example.com", cfg=None):
@@ -85,26 +88,64 @@ def test_run_monitor_dns_passes_expected_ip(monkeypatch):
     assert res["resolved_ip"] == "192.0.2.1"
 
 
-# ── route form helpers ────────────────────────────────────────────────────────
+# ── config validation (shared by the form and the MCP tools) ─────────────────
 
 def test_normalise_target_adds_https_for_bare_http_host():
-    assert _normalise_target("http", "example.com") == "https://example.com"
-    assert _normalise_target("http", "http://x.com") == "http://x.com"
-    assert _normalise_target("tcp", "example.com") == "example.com"
+    assert normalise_target("http", "example.com") == "https://example.com"
+    assert normalise_target("http", "http://x.com") == "http://x.com"
+    assert normalise_target("tcp", "example.com") == "example.com"
 
 
 class _Form(dict):
     """Minimal stand-in for a Quart form (dict + 'in' membership for checkboxes)."""
 
 
-def test_build_config_per_type():
-    assert _build_config(_Form(port="8080"), "tcp") == {"port": 8080}
-    assert _build_config(_Form(), "tcp") == {"port": 80}
-    assert _build_config(_Form(expected_ip="10.0.0.1"), "dns") == {"expected_ip": "10.0.0.1"}
-    assert _build_config(_Form(), "dns") == {"expected_ip": None}
-    assert _build_config(_Form(), "icmp") == {}
+def _from_form(form, mtype):
+    return build_config(mtype, _form_opts(form, mtype))
 
-    http = _build_config(_Form(method="get", expected_status="201", follow_redirects="on"), "http")
+
+def test_build_config_from_the_form_per_type():
+    assert _from_form(_Form(port="8080"), "tcp") == {"port": 8080}
+    assert _from_form(_Form(), "tcp") == {"port": 80}
+    assert _from_form(_Form(expected_ip="10.0.0.1"), "dns") == {"expected_ip": "10.0.0.1"}
+    assert _from_form(_Form(), "dns") == {"expected_ip": None}
+    assert _from_form(_Form(), "icmp") == {}
+
+    http = _from_form(_Form(method="get", expected_status="201", follow_redirects="on"), "http")
     assert http["method"] == "GET" and http["expected_status"] == 201
     assert http["follow_redirects"] is True       # present in form → checked
     assert http["verify_ssl"] is False            # absent from form → unchecked
+
+
+def test_build_config_from_typed_values_defaults_to_safe_checks():
+    http = build_config("http", {"headers": {"X-Key": "v"}})
+    assert http["follow_redirects"] is True and http["verify_ssl"] is True
+    assert http["headers"] == {"X-Key": "v"} and http["timeout_seconds"] == 10
+
+
+@pytest.mark.parametrize("mtype, opts, why", [
+    ("tcp", {"port": 70000}, "port must be between 1 and 65535"),
+    ("tcp", {"port": "http"}, "port must be a whole number"),
+    ("tcp", {"expected_ip": "1.2.3.4"}, "does not take expected_ip; it takes port"),
+    ("icmp", {"port": 22}, "takes no config"),
+    ("http", {"method": "DELETE"}, "not one of GET, HEAD, POST"),
+    ("http", {"expected_status": 42}, "between 100 and 599"),
+    ("http", {"timeout_seconds": 0}, "between 1 and 60"),
+    ("http", {"headers": "{not json"}, "JSON object"),
+    ("http", {"headers": {"X": 1}}, "name: value strings"),
+])
+def test_build_config_refuses_with_a_reason(mtype, opts, why):
+    with pytest.raises(ValueError, match=why):
+        build_config(mtype, opts)
+
+
+def test_monitor_fields():
+    f = monitor_fields("HTTP", target="nas.lan", opts={"expected_status": 204})
+    assert (f["type"], f["target"], f["name"]) == ("http", "https://nas.lan", "https://nas.lan")
+    assert f["check_interval_seconds"] == 0
+    with pytest.raises(ValueError, match="'smtp' is not one of icmp, tcp, dns, http"):
+        monitor_fields("smtp", target="x")
+    with pytest.raises(ValueError, match="needs a target"):
+        monitor_fields("icmp", target="  ")
+    with pytest.raises(ValueError, match="check_interval_seconds must be at least 0"):
+        monitor_fields("icmp", target="x", check_interval_seconds=-5)

tests/integration/test_mcp_monitoring_writes.py

@@ -0,0 +1,146 @@
+"""Integration: the monitoring-config write tools against real Postgres.
+
+A monitor is created on a host, edited (config merged, a rename warning for
+the rule that watches it by name), paused and deleted; a rule is created on
+that monitor, edited, acknowledged once firing, and deleted; a maintenance
+window in progress is ended and an upcoming one cancelled. Every write leaves
+an audit row as "<user> via MCP".
+"""
+from __future__ import annotations
+
+import asyncio
+import os
+import uuid
+
+import pytest
+
+pytestmark = pytest.mark.integration
+
+_NEEDS_DB = pytest.mark.skipif(
+    not os.environ.get("STEWARD_DATABASE_URL"),
+    reason="integration test needs a live Postgres (STEWARD_DATABASE_URL)",
+)
+
+
+@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)
+
+
+@_NEEDS_DB
+def test_monitor_rule_ack_and_maintenance_writes(app):
+    from sqlalchemy import select
+    from steward.core.api_tokens import Caller
+    from steward.mcp import _context
+    from steward.mcp.tools import alerts as a
+    from steward.mcp.tools import monitors as m
+    from steward.models.alerts import AlertRule, AlertState, AlertStateEnum, MaintenanceWindow
+    from steward.models.audit import AuditEvent
+    from steward.models.hosts import Host
+    from steward.models.monitors import Monitor
+    from steward.models.users import User, UserRole
+
+    tag = uuid.uuid4().hex[:8]
+    uid, username, host_name = str(uuid.uuid4()), f"mcp-cfg-{tag}", f"cfg-host-{tag}"
+    mon_name, rule_name = f"web-{tag}", f"web-down-{tag}"
+
+    async def _seed():
+        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.admin, is_active=True))
+                s.add(Host(id=str(uuid.uuid4()), name=host_name, address="10.7.7.7"))
+
+    async def _actions():
+        async with app.db_sessionmaker() as s:
+            rows = (await s.execute(select(AuditEvent).where(AuditEvent.user_id == uid))).scalars()
+            return [(e.action, e.username) for e in rows]
+
+    async def _go():
+        await _seed()
+        _context._app_ctx.set(app)
+        _context._caller_ctx.set(Caller(user_id=uid, username=username,
+                                        role=UserRole.admin, token_id=7))
+
+        # ── monitors ───────────────────────────────────────────────────────
+        with pytest.raises(ValueError, match="does not take port"):
+            await m.create_monitor(mon_name, "http", "nas.lan", config={"port": 80})
+        with pytest.raises(ValueError, match="no host named"):
+            await m.create_monitor(mon_name, "http", "nas.lan", host=f"ghost-{tag}")
+        made = await m.create_monitor(mon_name, "http", "nas.lan", host=host_name,
+                                      config={"expected_status": 204,
+                                              "headers": {"Authorization": "Bearer s3cret"}})
+        assert made["target"] == "https://nas.lan" and made["host_name"] == host_name
+        assert made["config"]["headers"] == {"Authorization": "<redacted>"}
+        assert made["config"]["verify_ssl"] is True
+
+        # ── rules ──────────────────────────────────────────────────────────
+        opts = await a.get_alert_rule_options("http")
+        assert mon_name in opts["resources"] and "is_up" in opts["metrics"]
+        with pytest.raises(ValueError, match="does not report 'up'"):
+            await a.create_alert_rule(rule_name, "http", mon_name, "up", "<", 1)
+        rule = await a.create_alert_rule(rule_name, "http", mon_name, "is_up", "<", 1,
+                                         consecutive_checks=3)
+        assert rule["condition"] == "is_up < 1 for 3 consecutive checks"
+        rule = await a.update_alert_rule(rule_name, threshold=0.5, enabled=False)
+        assert rule["condition"].startswith("is_up < 0.5") and rule["enabled"] is False
+
+        # Editing the config merges: the stored header survives a timeout change.
+        upd = await m.update_monitor(mon_name, config={"timeout_seconds": 5})
+        assert upd["config"]["timeout_seconds"] == 5 and upd["config"]["expected_status"] == 204
+        async with app.db_sessionmaker() as s:
+            row = await s.get(Monitor, made["monitor_id"])
+        assert row.config["headers"] == {"Authorization": "Bearer s3cret"}
+
+        # A rename leaves the rule on the old name, and says so.
+        renamed = await m.update_monitor(mon_name, name=f"{mon_name}-2")
+        assert rule_name in renamed["warning"]
+        await a.update_alert_rule(rule_name, resource=f"{mon_name}-2")
+        assert "warning" not in await m.update_monitor(f"{mon_name}-2", check_interval_s=30)
+        assert (await m.set_monitor_enabled(f"{mon_name}-2", False))["enabled"] is False
+
+        # ── acknowledge ────────────────────────────────────────────────────
+        with pytest.raises(ValueError, match="not firing"):
+            await a.acknowledge_alert(rule_name)
+        async with app.db_sessionmaker() as s:
+            async with s.begin():
+                st = (await s.execute(select(AlertState).where(
+                    AlertState.rule_id == rule["rule_id"]))).scalar_one()
+                st.state = AlertStateEnum.firing
+        ack = await a.acknowledge_alert(rule_name)
+        assert ack["state"] == "acknowledged"
+
+        # ── maintenance ────────────────────────────────────────────────────
+        win = await a.create_maintenance_window(f"patch-{tag}", duration_minutes=30,
+                                                source_module="http", resource=f"{mon_name}-2")
+        assert win["active"] is True
+        ended = await a.end_maintenance_window(f"patch-{tag}")
+        assert ended["outcome"] == "ended" and ended["active"] is True
+        with pytest.raises(ValueError, match="already ended"):
+            await a.end_maintenance_window(win["window_id"])
+        later = await a.create_maintenance_window(f"later-{tag}", start_at="2099-01-01T00:00:00Z",
+                                                  end_at="2099-01-01T02:00:00Z")
+        assert (await a.end_maintenance_window(later["window_id"]))["outcome"] == "cancelled"
+        async with app.db_sessionmaker() as s:
+            assert await s.get(MaintenanceWindow, later["window_id"]) is None
+            assert await s.get(MaintenanceWindow, win["window_id"]) is not None
+
+        # ── deletes ────────────────────────────────────────────────────────
+        await a.delete_alert_rule(rule_name)
+        await m.delete_monitor(f"{mon_name}-2")
+        async with app.db_sessionmaker() as s:
+            assert await s.get(AlertRule, rule["rule_id"]) is None
+            assert await s.get(Monitor, made["monitor_id"]) is None
+
+        actions = await _actions()
+        assert all(user == f"{username} via MCP" for _, user in actions)
+        assert sorted({act for act, _ in actions}) == sorted({
+            "monitor.created", "monitor.updated", "monitor.disabled", "monitor.deleted",
+            "alert_rule.created", "alert_rule.updated", "alert_rule.acknowledged",
+            "alert_rule.deleted", "maintenance_window.created", "maintenance_window.ended",
+            "maintenance_window.cancelled"})
+
+    asyncio.run(_go())

tests/mcp/test_monitoring_writes.py

@@ -0,0 +1,97 @@
+"""The monitoring-config write tools: their validation refusals and their roles.
+
+Monitor-config validation is covered in tests/core/test_monitors.py (the form
+and the tools share it). The writes themselves, their audit rows and the
+rename warning run against Postgres in
+tests/integration/test_mcp_monitoring_writes.py.
+"""
+from __future__ import annotations
+
+from datetime import datetime, timedelta, timezone
+
+import pytest
+
+from steward.alerts import service as alert_svc
+from steward.mcp.tools import alerts
+from steward.models.alerts import AlertOperator
+from steward.models.users import UserRole
+
+NOW = datetime(2026, 10, 6, 12, 0, tzinfo=timezone.utc)
+
+OPERATOR_TOOLS = {"create_monitor", "update_monitor", "set_monitor_enabled", "create_alert_rule",
+                  "update_alert_rule", "acknowledge_alert", "create_maintenance_window",
+                  "end_maintenance_window"}
+ADMIN_TOOLS = {"delete_monitor", "delete_alert_rule"}
+
+
+@pytest.fixture
+def known(monkeypatch):
+    """A source with one resource and its catalog metrics, without a database."""
+    async def resources(db, src):
+        return ["nas"] if src == "host_agent" else []
+    monkeypatch.setattr(alert_svc, "resources_for", resources)
+
+
+async def _fields(**over):
+    args = dict(name="cpu hot", source_module="host_agent", resource_name="nas",
+                metric_name="cpu_pct", operator=">", threshold="90", consecutive_failures_required=2)
+    args.update(over)
+    return await alert_svc.rule_fields(None, **args)
+
+
+async def test_rule_fields_accepts_the_form_choices(known):
+    f = await _fields()
+    assert f["operator"] is AlertOperator.gt and f["threshold"] == 90.0
+    assert f["consecutive_failures_required"] == 2
+
+
+@pytest.mark.parametrize("over, why", [
+    ({"name": " "}, "needs a name"),
+    ({"source_module": "zabbix"}, "'zabbix' is not one of icmp, tcp"),
+    ({"resource_name": "web01"}, "host_agent has no resource 'web01'. Known: nas"),
+    ({"metric_name": "cpu"}, "does not report 'cpu'. Its metrics: cpu_pct"),
+    ({"operator": "=>"}, "'=>' is not one of >, <"),
+    ({"threshold": "high"}, "threshold must be a number"),
+    ({"consecutive_failures_required": 0}, "at least 1"),
+])
+async def test_rule_fields_refuses_with_a_reason(known, over, why):
+    with pytest.raises(ValueError, match=why):
+        await _fields(**over)
+
+
+def test_window_fields():
+    f = alert_svc.window_fields(name="patching", start_at=NOW, end_at=NOW + timedelta(hours=1),
+                                scope_source="", scope_resource="nas")
+    assert f["scope_source"] is None and f["scope_resource"] is None  # resource needs a source
+    with pytest.raises(ValueError, match="must end after it starts"):
+        alert_svc.window_fields(name="x", start_at=NOW, end_at=NOW, scope_source=None,
+                                scope_resource=None)
+    with pytest.raises(ValueError, match="'nope' is not one of"):
+        alert_svc.window_fields(name="x", start_at=NOW, end_at=NOW + timedelta(minutes=1),
+                                scope_source="nope", scope_resource=None)
+
+
+async def test_maintenance_window_needs_exactly_one_end():
+    with pytest.raises(ValueError, match="exactly one of duration_minutes or end_at"):
+        await alerts.create_maintenance_window("x")
+    with pytest.raises(ValueError, match="exactly one of duration_minutes or end_at"):
+        await alerts.create_maintenance_window("x", duration_minutes=30, end_at="2026-10-06T13:00:00Z")
+    with pytest.raises(ValueError, match="not an ISO-8601 time"):
+        await alerts.create_maintenance_window("x", end_at="tonight")
+
+
+def test_when_reads_a_zoneless_time_as_utc():
+    assert alerts._when("2026-10-06T22:00:00", "end_at") == datetime(2026, 10, 6, 22, tzinfo=timezone.utc)
+    assert alerts._when("2026-10-06T22:00:00Z", "end_at").utcoffset() == timedelta(0)
+
+
+def test_write_tools_are_registered_with_their_roles(app):
+    mcp = app.mcp_server
+    for name in OPERATOR_TOOLS:
+        assert mcp.tool_roles[name] is UserRole.operator, name
+        assert mcp._tool_manager.get_tool(name).annotations.read_only_hint is False, name
+    for name in ADMIN_TOOLS:
+        assert mcp.tool_roles[name] is UserRole.admin, name
+    assert mcp.tool_roles["get_alert_rule_options"] is UserRole.viewer
+    for name in OPERATOR_TOOLS | ADMIN_TOOLS | {"get_alert_rule_options"}:
+        assert (mcp._tool_manager.get_tool(name).description or "").strip(), name