feat: Telegram messages get real formatting + push DMs at draft origination (#568)

* feat(telegram): HTML-styled bot messages + push DMs at held-draft origination

* fix(telegram): attr-context escaping, balance-aware truncation, send observability; docs

---------

Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
Renzo F
2026-07-18 15:54:49 +02:00
committed by GitHub
co-authored by Renn F
parent f0782cb858
commit ff78618b76
17 changed files with 1077 additions and 83 deletions
@@ -28,7 +28,7 @@ class _FakeTask:
self.orchestration_markers = orchestration_markers
def _actions(role: str) -> ContentActions:
def _actions(role: str, *, notification_delivery: Any = None) -> ContentActions:
task = MagicMock()
agent = MagicMock()
agent.role = role
@@ -41,6 +41,7 @@ def _actions(role: str) -> ContentActions:
journal=MagicMock(),
workspace=MagicMock(),
notifications=MagicMock(),
notification_delivery=notification_delivery,
)
return ContentActions(deps)
@@ -175,6 +176,64 @@ async def test_propose_roadmap_persists_cycle_onto_open_task(
actions.task.session.flush.assert_awaited_once()
@pytest.mark.asyncio
async def test_propose_roadmap_sends_telegram_push_per_item(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""Roadmap items only become CEO-actionable once propose_roadmap lands
(the engine's exploration-task origination has nothing to review yet),
so the push DM fires once per item here, not from RoadmapEngine."""
monkeypatch.setattr(cfg, "roadmap_min_items_per_cycle", 1)
monkeypatch.setattr(cfg, "roadmap_max_items_per_cycle", 7)
agent_id = uuid4()
cycle_task = _FakeTask(assigned_to=agent_id)
task_svc = MagicMock()
task_svc.list_open_roadmap_cycles = AsyncMock(return_value=[cycle_task])
monkeypatch.setattr("roboco.services.task.get_task_service", lambda _s: task_svc)
notify = AsyncMock()
actions = _actions("product_owner", notification_delivery=notify)
actions.task.session.flush = AsyncMock()
items = _valid_items(3)
env = await actions.propose_roadmap(
agent_id=agent_id, cycle_goal="Close onboarding friction", items=items
)
assert env.error is None
assert notify.notify_ceo_of_queue_item.await_count == len(items)
id8 = str(cycle_task.id)[:8]
for i, call in enumerate(notify.notify_ceo_of_queue_item.await_args_list):
assert call.kwargs["kind"] == "roadmap"
assert call.kwargs["id8"] == id8
assert call.kwargs["extra"] == f"item-{i}"
assert call.kwargs["title"] == f"Item {i}"
@pytest.mark.asyncio
async def test_propose_roadmap_survives_telegram_push_failure(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""A Telegram send failure must never block propose_roadmap itself."""
monkeypatch.setattr(cfg, "roadmap_min_items_per_cycle", 1)
monkeypatch.setattr(cfg, "roadmap_max_items_per_cycle", 7)
agent_id = uuid4()
cycle_task = _FakeTask(assigned_to=agent_id)
task_svc = MagicMock()
task_svc.list_open_roadmap_cycles = AsyncMock(return_value=[cycle_task])
monkeypatch.setattr("roboco.services.task.get_task_service", lambda _s: task_svc)
notify = MagicMock()
notify.notify_ceo_of_queue_item = AsyncMock(side_effect=RuntimeError("boom"))
actions = _actions("product_owner", notification_delivery=notify)
actions.task.session.flush = AsyncMock()
env = await actions.propose_roadmap(
agent_id=agent_id, cycle_goal="Close onboarding friction", items=_valid_items(2)
)
assert env.error is None
assert env.status == "roadmap_proposed"
@pytest.mark.asyncio
async def test_propose_roadmap_ignores_cycle_assigned_to_another_agent(
monkeypatch: pytest.MonkeyPatch,
@@ -219,6 +219,49 @@ async def test_loop_never_publishes_or_approves(
assert proposals[0].status == TS.PENDING # never advanced by the loop
@pytest.mark.asyncio
async def test_proposes_sends_telegram_push(
db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch
) -> None:
"""A freshly-originated release proposal fires the styled push DM
(release kind, the proposal's id8, its version) alongside the existing
in-app notification."""
await _seed(db_session)
_enable(monkeypatch)
notify = AsyncMock()
monkeypatch.setattr(
"roboco.services.notification_delivery.NotificationDeliveryService."
"notify_ceo_of_queue_item",
notify,
)
engine = ReleaseManagerEngine(db_session, assessor=_assessor(_report()))
task = await engine.run_cycle()
assert task is not None
notify.assert_awaited_once()
_args, kwargs = notify.await_args
assert kwargs["kind"] == "release"
assert kwargs["id8"] == str(task.id)[:8]
assert _VERSION in kwargs["title"]
@pytest.mark.asyncio
async def test_proposes_survives_telegram_push_failure(
db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch
) -> None:
"""A Telegram send failure must never block origination itself."""
await _seed(db_session)
_enable(monkeypatch)
monkeypatch.setattr(
"roboco.services.notification_delivery.NotificationDeliveryService."
"notify_ceo_of_queue_item",
AsyncMock(side_effect=RuntimeError("boom")),
)
engine = ReleaseManagerEngine(db_session, assessor=_assessor(_report()))
task = await engine.run_cycle()
assert task is not None
assert await get_task_service(db_session).list_open_release_proposals()
@pytest.mark.asyncio
async def test_none_assessment_no_proposal(
db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch
+55 -1
View File
@@ -93,6 +93,59 @@ async def test_live_client_send_message_with_reply_markup_and_message_id() -> No
await client.close()
@pytest.mark.asyncio
async def test_live_client_send_message_with_formatting_fields() -> None:
def handler(request: httpx.Request) -> httpx.Response:
body = json.loads(request.content.decode())
assert body["parse_mode"] == "HTML"
assert body["link_preview_options"] == {"is_disabled": True}
return httpx.Response(200, json={"ok": True, "result": {"message_id": 1}})
transport = httpx.MockTransport(handler)
http_client = httpx.AsyncClient(transport=transport)
client = LiveTelegramClient(_CREDS, timeout=5.0, client=http_client)
result = await client.send_message(
"<b>hi</b>", parse_mode="HTML", disable_link_preview=True
)
assert result.sent is True
await client.close()
@pytest.mark.asyncio
async def test_live_client_send_message_omits_formatting_fields_when_unset() -> None:
def handler(request: httpx.Request) -> httpx.Response:
body = json.loads(request.content.decode())
assert "parse_mode" not in body
assert "link_preview_options" not in body
return httpx.Response(200, json={"ok": True, "result": {"message_id": 1}})
transport = httpx.MockTransport(handler)
http_client = httpx.AsyncClient(transport=transport)
client = LiveTelegramClient(_CREDS, timeout=5.0, client=http_client)
result = await client.send_message("hi")
assert result.sent is True
await client.close()
@pytest.mark.asyncio
async def test_live_client_edit_message_text_with_parse_mode() -> None:
def handler(request: httpx.Request) -> httpx.Response:
assert request.url.path == "/bot123456:ABC/editMessageText"
body = json.loads(request.content.decode())
assert body["text"] == "<b>done</b>"
assert body["parse_mode"] == "HTML"
assert body["link_preview_options"] == {"is_disabled": True}
return httpx.Response(200, json={"ok": True, "result": True})
transport = httpx.MockTransport(handler)
http_client = httpx.AsyncClient(transport=transport)
client = LiveTelegramClient(_CREDS, timeout=5.0, client=http_client)
await client.edit_message_text(
7, "<b>done</b>", parse_mode="HTML", disable_link_preview=True
)
await client.close()
@pytest.mark.asyncio
async def test_live_client_get_updates_success() -> None:
def handler(request: httpx.Request) -> httpx.Response:
@@ -161,4 +214,5 @@ async def test_null_client_v2_methods_are_all_noops() -> None:
# None of these raise — that's the whole contract.
await client.answer_callback_query("cq1", "text")
await client.edit_message_reply_markup(1, None)
await client.edit_message_text(1, "done")
await client.edit_message_text(1, "done", parse_mode="HTML")
await client.send_message("hi", parse_mode="HTML", disable_link_preview=True)
@@ -253,7 +253,7 @@ async def test_expired_reply_prompt_sends_notice(
)
client.send_message.assert_awaited_once_with(
"That prompt expired — tap the button again."
"That prompt expired — tap the button again.", parse_mode="HTML"
)
dispatch.assert_not_called()
assert ("777", 42) not in ti._PENDING_REPLIES
@@ -743,3 +743,207 @@ async def test_mark_audit_adds_via_telegram_row() -> None:
assert added.agent_id == CEO_UUID
assert added.target_id == task_id
assert added.details["via"] == "telegram"
# ---------------------------------------------------------------------------
# /queue rendering — pluralization + HTML formatting
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_send_queue_empty_says_nothing_awaiting(
monkeypatch: pytest.MonkeyPatch,
) -> None:
engine = _engine()
monkeypatch.setattr(engine, "_collect_queue_items", AsyncMock(return_value=[]))
client = AsyncMock()
await engine._send_queue(client)
client.send_message.assert_awaited_once_with(
"✅ Nothing awaiting your approval.", parse_mode="HTML"
)
@pytest.mark.asyncio
async def test_send_queue_singular_item_pluralization(
monkeypatch: pytest.MonkeyPatch,
) -> None:
engine = _engine()
monkeypatch.setattr(
engine,
"_collect_queue_items",
AsyncMock(return_value=[("task", "a1b2c3d4", "", "Ship it")]),
)
client = AsyncMock()
await engine._send_queue(client)
header = client.send_message.await_args_list[0].args[0]
assert header == "<b>🔔 Awaiting your approval</b> — 1 item"
@pytest.mark.asyncio
async def test_send_queue_plural_items_pluralization(
monkeypatch: pytest.MonkeyPatch,
) -> None:
engine = _engine()
monkeypatch.setattr(
engine,
"_collect_queue_items",
AsyncMock(
return_value=[
("task", "a1b2c3d4", "", "Ship it"),
("release", "deadbeef", "", "v1.0.0 ready"),
]
),
)
client = AsyncMock()
await engine._send_queue(client)
header = client.send_message.await_args_list[0].args[0]
assert header == "<b>🔔 Awaiting your approval</b> — 2 items"
@pytest.mark.asyncio
async def test_send_queue_item_line_escapes_title_and_carries_keyboard(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""Injection regression: a malicious task title must arrive HTML-escaped
— never as live markup — in the /queue item line's sent payload."""
engine = _engine()
monkeypatch.setattr(
engine,
"_collect_queue_items",
AsyncMock(return_value=[("task", "a1b2c3d4", "", "<b>bold&joke</b>")]),
)
client = AsyncMock()
await engine._send_queue(client)
item_call = client.send_message.await_args_list[1]
text = item_call.args[0]
assert "&lt;b&gt;bold&amp;joke&lt;/b&gt;" in text
assert "<b>bold&joke</b>" not in text
assert text.startswith("📋 <b>Task</b> — ")
assert item_call.kwargs["parse_mode"] == "HTML"
assert "reply_markup" in item_call.kwargs
# ---------------------------------------------------------------------------
# /task — link preview disabled, title/status/team escaping
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_dispatch_command_task_disables_link_preview(
monkeypatch: pytest.MonkeyPatch,
) -> None:
engine = _engine()
monkeypatch.setattr(engine, "_render_task", AsyncMock(return_value="detail"))
client = AsyncMock()
await engine._dispatch_command("task", "a1b2c3d4", client)
client.send_message.assert_awaited_once_with(
"detail", parse_mode="HTML", disable_link_preview=True
)
def test_format_task_detail_escapes_html_in_title() -> None:
"""Injection regression: a task titled ``<b>bold&joke</b>`` must render
HTML-escaped, not as live markup, in /task's detail view."""
engine = _engine()
task = _fake_task(title="<b>bold&joke</b>")
rendered = engine._format_task_detail(task)
assert "&lt;b&gt;bold&amp;joke&lt;/b&gt;" in rendered
assert "<b>bold&joke</b>" not in rendered
def test_format_task_detail_pr_url_is_a_named_link() -> None:
engine = _engine()
task = _fake_task()
task.pr_url = "https://github.com/example/repo/pull/1"
rendered = engine._format_task_detail(task)
assert '<a href="https://github.com/example/repo/pull/1">View PR</a>' in rendered
def test_format_task_detail_pr_url_quote_cannot_break_out_of_href() -> None:
"""Injection regression: a pr_url containing a literal '"' must not be
able to close the href attribute early and inject a bogus attribute —
_esc_attr (quote=True) turns it into '&quot;', keeping the whole value
inside the attribute."""
engine = _engine()
task = _fake_task()
task.pr_url = 'https://evil.example/x" onmouseover="alert(1)'
rendered = engine._format_task_detail(task)
assert (
'<a href="https://evil.example/x&quot; onmouseover=&quot;alert(1)">'
"View PR</a>" in rendered
)
assert 'onmouseover="alert(1)"' not in rendered
# ---------------------------------------------------------------------------
# outcome confirmations (_finish_action / _consume_reply) — escaping
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_finish_action_escapes_text_and_edits_origin() -> None:
engine = _engine()
client = AsyncMock()
await engine._finish_action(client, 42, True, "Rejected: <script>xss</script>")
client.edit_message_reply_markup.assert_awaited_once_with(42, None)
call = client.edit_message_text.await_args
assert call.args == (42, "✅ Rejected: &lt;script&gt;xss&lt;/script&gt;")
assert call.kwargs["parse_mode"] == "HTML"
@pytest.mark.asyncio
async def test_finish_action_escapes_text_without_origin() -> None:
engine = _engine()
client = AsyncMock()
await engine._finish_action(client, None, False, "<script>alert(1)</script>")
call = client.send_message.await_args
assert call.args == ("❌ &lt;script&gt;alert(1)&lt;/script&gt;",)
assert call.kwargs["parse_mode"] == "HTML"
@pytest.mark.asyncio
async def test_consume_reply_reject_outcome_arrives_escaped(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""End-to-end regression: a reject reason that reaches the CEO through
whatever text a dispatch handler returns must never arrive as live HTML —
the funnel (_finish_action) escapes it regardless of the handler."""
engine = _engine()
client = AsyncMock()
dispatch = AsyncMock(return_value=(True, "Rejected: <script>xss</script>"))
monkeypatch.setattr(engine, "_dispatch_reject", dispatch)
pending = ti._PendingAction(
kind="xpost",
id8="a1b2c3d4",
extra="",
action="reject",
origin_message_id=None,
expires_at=time.monotonic() + 60,
)
await engine._consume_reply(pending, "<script>xss</script>", client)
dispatch.assert_awaited_once_with("xpost", "a1b2c3d4", "", "<script>xss</script>")
sent_text = client.send_message.await_args.args[0]
assert "&lt;script&gt;xss&lt;/script&gt;" in sent_text
assert "<script>xss</script>" not in sent_text
@@ -6,12 +6,17 @@ from __future__ import annotations
import pytest
from roboco.config import settings as cfg
from roboco.services.telegram_inbound import (
_MESSAGE_CHAR_LIMIT,
ParsedCallback,
_authorized_chat,
_esc,
_esc_attr,
_truncate,
build_action_keyboard,
build_callback,
parse_callback,
parse_command,
render_queue_item_text,
)
_CALLBACK_DATA_MAX_BYTES = 64 # mirrors Telegram's own callback_data cap
@@ -134,3 +139,94 @@ class TestAuthorizedChat:
def test_empty_chat_id_rejected(self) -> None:
assert _authorized_chat("", "12345") is False
class TestEsc:
def test_escapes_angle_brackets_and_ampersand(self) -> None:
assert _esc("<b>bold&joke</b>") == "&lt;b&gt;bold&amp;joke&lt;/b&gt;"
def test_quotes_are_left_alone(self) -> None:
# quote=False — _esc renders HTML text nodes, where quotes need no
# escaping. Attribute values (e.g. href="...") go through _esc_attr
# instead, which does escape them.
assert _esc('it\'s "fine"') == 'it\'s "fine"'
def test_stringifies_non_str_values(self) -> None:
assert _esc(42) == "42"
class TestEscAttr:
def test_escapes_quotes_and_angle_brackets(self) -> None:
assert _esc_attr("""a"b'c<d>e""") == "a&quot;b&#x27;c&lt;d&gt;e"
def test_stringifies_non_str_values(self) -> None:
assert _esc_attr(42) == "42"
class TestTruncateHtml:
def test_short_text_is_untouched(self) -> None:
assert _truncate("hello") == "hello"
def test_backs_off_before_an_unclosed_angle_bracket(self) -> None:
# A naive slice at `limit - 1` would land inside "<code>", leaving a
# bare '<' Telegram's HTML parser can't make sense of — back off to
# before it instead.
text = ("x" * 4093) + "<code>"
result = _truncate(text)
assert result.endswith("")
assert not result.rstrip("").endswith("<")
def test_backs_off_before_an_unclosed_entity(self) -> None:
text = ("x" * 4093) + "&amp;"
result = _truncate(text)
assert result.endswith("")
assert "&am" not in result
@pytest.mark.parametrize("title_len", range(4048, 4069))
def test_render_queue_item_truncation_balances_code_tag(
self, title_len: int
) -> None:
# Regression: a naive char-count slice landed inside the trailing
# `<code>id8</code>` span, shipping an unclosed `<code>` Telegram's
# HTML parser rejects outright.
text = render_queue_item_text("roadmap", "abc12345", "item-0", "A" * title_len)
assert text.count("<code>") == text.count("</code>")
assert len(text) <= _MESSAGE_CHAR_LIMIT
def test_truncate_balances_a_bold_wrapped_tag(self) -> None:
text = "<b>" + ("y" * 4200) + "</b>"
result = _truncate(text)
assert result.count("<b>") == result.count("</b>")
assert len(result) <= _MESSAGE_CHAR_LIMIT
class TestRenderQueueItemText:
def test_escapes_html_in_title(self) -> None:
text = render_queue_item_text("task", "a1b2c3d4", "", "<b>bold&joke</b>")
assert "&lt;b&gt;bold&amp;joke&lt;/b&gt;" in text
assert "<b>bold&joke</b>" not in text
def test_kind_emoji_and_label_per_kind(self) -> None:
assert render_queue_item_text("release", "a1b2c3d4", "", "x").startswith(
"🚀 <b>Release</b>"
)
assert render_queue_item_text("video", "a1b2c3d4", "", "x").startswith(
"🎬 <b>Video</b>"
)
assert render_queue_item_text("xpost", "a1b2c3d4", "", "x").startswith(
"✕ <b>Post</b>"
)
assert render_queue_item_text("roadmap", "a1b2c3d4", "", "x").startswith(
"🗺️ <b>Roadmap</b>"
)
assert render_queue_item_text("task", "a1b2c3d4", "", "x").startswith(
"📋 <b>Task</b>"
)
def test_id8_and_extra_render_as_code_span(self) -> None:
text = render_queue_item_text("roadmap", "a1b2c3d4", "item-2", "x")
assert "<code>a1b2c3d4:item-2</code>" in text
def test_no_extra_omits_suffix(self) -> None:
text = render_queue_item_text("task", "a1b2c3d4", "", "x")
assert "<code>a1b2c3d4</code>" in text
+62
View File
@@ -456,6 +456,68 @@ async def test_originate_video_post_holds_draft_for_secretary(
assert draft["source_task_id"] == str(source_task.id) # traceability back-ref
@pytest.mark.asyncio
async def test_originate_video_post_sends_telegram_push(
db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch
) -> None:
await _seed(db_session)
_enable(monkeypatch)
notify = AsyncMock()
monkeypatch.setattr(
"roboco.services.notification_delivery.NotificationDeliveryService."
"notify_ceo_of_queue_item",
notify,
)
engine = video_engine_module.VideoEngine(db_session)
source_task = await engine.open_video_task(
occasion="release v1.0.0",
script="Here's what shipped",
platforms=["x", "tiktok"],
brief="Announce the release",
)
assert source_task is not None
draft_task = await engine._originate_video_post(
source_task=source_task,
mp4_paths={"vertical": "/a.mp4", "square": "/b.mp4"},
captions={"x": "shipped!", "tiktok": "shipped!"},
platforms=["x", "tiktok"],
)
notify.assert_awaited_once()
_args, kwargs = notify.await_args
assert kwargs["kind"] == "video"
assert kwargs["id8"] == str(draft_task.id)[:8]
assert kwargs["title"] == "release v1.0.0"
@pytest.mark.asyncio
async def test_originate_video_post_survives_telegram_push_failure(
db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch
) -> None:
"""A Telegram send failure must never block the draft itself."""
await _seed(db_session)
_enable(monkeypatch)
monkeypatch.setattr(
"roboco.services.notification_delivery.NotificationDeliveryService."
"notify_ceo_of_queue_item",
AsyncMock(side_effect=RuntimeError("boom")),
)
engine = video_engine_module.VideoEngine(db_session)
source_task = await engine.open_video_task(
occasion="release v1.0.0", script="s", platforms=["x"], brief="b"
)
assert source_task is not None
draft_task = await engine._originate_video_post(
source_task=source_task,
mp4_paths={"vertical": "/a.mp4", "square": "/b.mp4"},
captions={"x": "shipped!"},
platforms=["x"],
)
assert draft_task is not None
@pytest.mark.asyncio
async def test_originate_video_post_not_counted_by_dedupe_against_new_occasion(
db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch
+48
View File
@@ -252,6 +252,54 @@ async def test_draft_release_post_dedupes_same_version(
assert len(open_posts) == ONE
@pytest.mark.asyncio
async def test_originate_post_sends_telegram_push(
db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch
) -> None:
"""``_originate_post`` is the shared chokepoint for all three X sources
(release/reply/feature) a freshly-drafted post fires the styled push
DM (xpost kind, the draft's id8, its body)."""
await _seed(db_session)
_enable(monkeypatch)
_mock_local_model(monkeypatch, "RoboCo just shipped a great new feature!")
notify = AsyncMock()
monkeypatch.setattr(
"roboco.services.notification_delivery.NotificationDeliveryService."
"notify_ceo_of_queue_item",
notify,
)
engine = x_engine_module.XEngine(db_session, client=_FakeClient())
task = await engine.draft_release_post(
version=_VERSION, highlights=["feat: new thing"]
)
assert task is not None
notify.assert_awaited_once()
_args, kwargs = notify.await_args
assert kwargs["kind"] == "xpost"
assert kwargs["id8"] == str(task.id)[:8]
body = markers.get_x_draft_body(task)
assert body is not None
assert kwargs["title"] == body[:100]
@pytest.mark.asyncio
async def test_originate_post_survives_telegram_push_failure(
db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch
) -> None:
"""A Telegram send failure must never block the draft itself."""
await _seed(db_session)
_enable(monkeypatch)
_mock_local_model(monkeypatch, "shipped!")
monkeypatch.setattr(
"roboco.services.notification_delivery.NotificationDeliveryService."
"notify_ceo_of_queue_item",
AsyncMock(side_effect=RuntimeError("boom")),
)
engine = x_engine_module.XEngine(db_session, client=_FakeClient())
task = await engine.draft_release_post(version=_VERSION, highlights=[])
assert task is not None
@pytest.mark.asyncio
async def test_draft_release_post_respects_open_cap(
db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch