diff --git a/services/agent-system/telegram-bot/bot.py b/services/agent-system/telegram-bot/bot.py index cd2078d..a43ed84 100644 --- a/services/agent-system/telegram-bot/bot.py +++ b/services/agent-system/telegram-bot/bot.py @@ -54,6 +54,36 @@ async def post_api(path, data): logger.error(f"Error posting to {url}: {e}") return False +def _format_pending_action(action_id: str, data: dict) -> str: + """Build the Telegram Markdown message for a pending action notification. + + Extracted so it can be unit-tested without a live Telegram connection. + """ + # Supervisor writes risk_level; action-model.md legacy schema used risk. + risk = data.get("risk_level") or data.get("risk", "unknown") + message = ( + f"⚠️ *Pending Action*\n" + f"ID: `{action_id}`\n" + f"Type: `{data.get('type', 'unknown')}`\n" + f"Service: `{data.get('service', 'unknown')}`\n" + f"Node: `{data.get('node', 'unknown')}`\n" + f"Risk: *{risk}*\n" + ) + # description carries the human-readable substance of the action (required for + # alert_only actions where it is the entire operator-visible message). + description = data.get("description", "") + if description: + truncated = description[:300] + ("..." if len(description) > 300 else "") + message += f"Description: `{truncated}`\n" + # Legacy details block (old action-model.md schema) — kept for backwards compat. + if "details" in data: + details_str = json.dumps(data["details"], indent=2) + if len(details_str) > 1000: + details_str = details_str[:1000] + "..." + message += f"\nDetails:\n```json\n{details_str}\n```" + return message + + class ApprovalBot: def __init__(self): self.pending_dir = ACTIONS_ROOT / "pending" @@ -86,20 +116,7 @@ class ApprovalBot: async def notify_users(self, context: ContextTypes.DEFAULT_TYPE, action_id: str, data: dict): """Sends an approval request message to all allowed users.""" - message = ( - f"⚠️ *Pending Action*\n" - f"ID: `{action_id}`\n" - f"Type: `{data.get('type', 'unknown')}`\n" - f"Service: `{data.get('service', 'unknown')}`\n" - f"Node: `{data.get('node', 'unknown')}`\n" - f"Risk: *{data.get('risk', 'unknown')}*\n" - ) - - if "details" in data: - details_str = json.dumps(data['details'], indent=2) - if len(details_str) > 1000: - details_str = details_str[:1000] + "..." - message += f"\nDetails:\n```json\n{details_str}\n```" + message = _format_pending_action(action_id, data) keyboard = [ [ diff --git a/services/agent-system/telegram-bot/tests/__init__.py b/services/agent-system/telegram-bot/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/services/agent-system/telegram-bot/tests/conftest.py b/services/agent-system/telegram-bot/tests/conftest.py new file mode 100644 index 0000000..9f2eb2b --- /dev/null +++ b/services/agent-system/telegram-bot/tests/conftest.py @@ -0,0 +1,38 @@ +"""Stub telegram before bot.py is imported so pytest doesn't need the real package.""" +from __future__ import annotations + +import sys +import types +from unittest.mock import MagicMock + + +def _make_telegram_stub() -> types.ModuleType: + mod = types.ModuleType("telegram") + mod.Update = MagicMock + mod.InlineKeyboardButton = MagicMock + mod.InlineKeyboardMarkup = MagicMock + return mod + + +def _make_telegram_ext_stub() -> types.ModuleType: + mod = types.ModuleType("telegram.ext") + mod.ApplicationBuilder = MagicMock + + # ContextTypes.DEFAULT_TYPE is referenced as a type annotation at class-body + # evaluation time, so it must be a real attribute, not a dynamic MagicMock attr. + ContextTypesMock = MagicMock() + ContextTypesMock.DEFAULT_TYPE = type(None) + mod.ContextTypes = ContextTypesMock + + mod.CommandHandler = MagicMock + mod.CallbackQueryHandler = MagicMock + mod.MessageHandler = MagicMock + mod.filters = MagicMock() + return mod + + +# Insert before any import of bot.py +if "telegram" not in sys.modules: + sys.modules["telegram"] = _make_telegram_stub() +if "telegram.ext" not in sys.modules: + sys.modules["telegram.ext"] = _make_telegram_ext_stub() diff --git a/services/agent-system/telegram-bot/tests/test_format.py b/services/agent-system/telegram-bot/tests/test_format.py new file mode 100644 index 0000000..add0980 --- /dev/null +++ b/services/agent-system/telegram-bot/tests/test_format.py @@ -0,0 +1,116 @@ +"""Tests for _format_pending_action — no Telegram connection required. + +telegram stubs are set up in conftest.py before this module is imported. +""" +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent)) +from bot import _format_pending_action + + +# --------------------------------------------------------------------------- +# Bug 1 — risk_level field +# --------------------------------------------------------------------------- + +def test_risk_level_shown_when_present(): + data = { + "type": "container_restart", "service": "homeassistant", + "node": "chelsty-ha", "risk_level": "low", + } + msg = _format_pending_action("container-restart-chelsty-ha-homeassistant", data) + assert "Risk: *low*" in msg + assert "unknown" not in msg + + +def test_risk_falls_back_to_legacy_risk_key(): + data = { + "type": "redeploy", "service": "mosquitto", + "node": "chelsty-infra", "risk": "guarded", + } + msg = _format_pending_action("redeploy-chelsty-infra-mosquitto", data) + assert "Risk: *guarded*" in msg + + +def test_risk_unknown_when_both_absent(): + data = {"type": "redeploy", "service": "foo", "node": "bar"} + msg = _format_pending_action("redeploy-bar-foo", data) + assert "Risk: *unknown*" in msg + + +# --------------------------------------------------------------------------- +# Bug 2 — description field +# --------------------------------------------------------------------------- + +def test_description_shown_for_alert_only(): + data = { + "type": "alert_only", "service": "homeassistant", + "node": "chelsty-ha", "risk_level": "info", + "description": "3 entities unavailable for >1h", + } + msg = _format_pending_action("alert-ha-entity-unavailable-chelsty-ha", data) + assert "3 entities unavailable for >1h" in msg + assert "Description:" in msg + + +def test_description_shown_for_container_restart(): + data = { + "type": "container_restart", "service": "homeassistant", + "node": "chelsty-ha", "risk_level": "low", + "description": "Restart 'homeassistant' on chelsty-ha: HA WebSocket unresponsive", + } + msg = _format_pending_action("container-restart-chelsty-ha-homeassistant", data) + assert "HA WebSocket unresponsive" in msg + + +def test_description_absent_no_crash(): + data = {"type": "redeploy", "service": "foo", "node": "bar", "risk_level": "guarded"} + msg = _format_pending_action("redeploy-bar-foo", data) + assert "Description:" not in msg + assert "Risk: *guarded*" in msg + + +def test_description_truncated_at_300_chars(): + long_desc = "x" * 400 + data = { + "type": "alert_only", "service": "homeassistant", + "node": "chelsty-ha", "risk_level": "info", + "description": long_desc, + } + msg = _format_pending_action("alert-ha-foo-chelsty-ha", data) + assert "x" * 300 in msg + assert "..." in msg + assert "x" * 301 not in msg + + +# --------------------------------------------------------------------------- +# Combined — real HA alert_only action shape +# --------------------------------------------------------------------------- + +def test_ha_alert_only_full_action(): + """Mirrors an actual alert_only action written by supervisor._generate_ha_alert_only.""" + data = { + "action_id": "alert-ha-entity-unavailable-chelsty-ha", + "type": "alert_only", + "node": "chelsty-ha", + "service": "homeassistant", + "risk_level": "info", + "confidence": 1.0, + "description": "3 entities unavailable for >1h: sensor.power, binary_sensor.window", + "status": "pending", + "payload": { + "location_tag": "chelsty", + "reason": "ha_entity_unavailable_long", + "count": 3, + }, + } + msg = _format_pending_action(data["action_id"], data) + assert "alert_only" in msg + assert "chelsty-ha" in msg + assert "Risk: *info*" in msg + assert "3 entities unavailable" in msg + assert "unknown" not in msg