From 15e9d0471f2694b564d6b8c63b3d7be1fe922d1a Mon Sep 17 00:00:00 2001 From: zhuzhh Date: Sat, 25 Apr 2026 12:58:04 +0800 Subject: [PATCH] feat(msteams): make ref pruning configurable and atomic --- docs/chat-apps.md | 9 ++- nanobot/channels/msteams.py | 38 ++++++++++-- tests/test_msteams.py | 112 ++++++++++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+), 7 deletions(-) diff --git a/docs/chat-apps.md b/docs/chat-apps.md index 3bf2bee29..3d5e4dbd5 100644 --- a/docs/chat-apps.md +++ b/docs/chat-apps.md @@ -642,7 +642,10 @@ Create or reuse a Microsoft Teams / Azure bot app registration. Set the bot mess "allowFrom": ["*"], "replyInThread": true, "mentionOnlyResponse": "Hi — what can I help with?", - "validateInboundAuth": true + "validateInboundAuth": true, + "refTtlDays": 30, + "pruneWebChatRefs": true, + "pruneNonPersonalRefs": true } } } @@ -651,7 +654,9 @@ Create or reuse a Microsoft Teams / Azure bot app registration. Set the bot mess > - `replyInThread: true` replies to the triggering Teams activity when a stored `activity_id` is available. > - `mentionOnlyResponse` controls what Nanobot receives when a user sends only a bot mention (`Nanobot`). Set to `""` to ignore mention-only messages. > - `validateInboundAuth: true` enables inbound Bot Framework bearer-token validation (signature, issuer, audience, lifetime, `serviceUrl`). This is the safe default for public deployments. Only set it to `false` for local development or tightly controlled testing. -> - Conversation refs are auto-pruned to avoid bad outbound routing: Web Chat refs, non-`personal` refs, and refs older than 30 days are removed. +> - `refTtlDays` (default `30`) controls how old stored conversation refs can be before they are pruned. +> - `pruneWebChatRefs` (default `true`) drops refs with `webchat.botframework.com` service URLs. +> - `pruneNonPersonalRefs` (default `true`) drops refs whose `conversation_type` is not `personal`. **4. Run** diff --git a/nanobot/channels/msteams.py b/nanobot/channels/msteams.py index bdbdf8c8a..7b294a830 100644 --- a/nanobot/channels/msteams.py +++ b/nanobot/channels/msteams.py @@ -15,7 +15,9 @@ import asyncio import html import importlib.util import json +import os import re +import tempfile import threading import time from dataclasses import dataclass @@ -63,6 +65,9 @@ class MSTeamsConfig(Base): reply_in_thread: bool = True mention_only_response: str = "Hi — what can I help with?" validate_inbound_auth: bool = True + ref_ttl_days: int = Field(default=MSTEAMS_REF_TTL_DAYS, ge=1) + prune_web_chat_refs: bool = True + prune_non_personal_refs: bool = True @dataclass @@ -516,16 +521,17 @@ class MSTeamsChannel(BaseChannel): return False now_ts = time.time() if now is None else now - stale_before = now_ts - MSTEAMS_REF_TTL_S + ttl_days = int(self.config.ref_ttl_days) + stale_before = now_ts - (ttl_days * 24 * 60 * 60) keys_to_drop: list[str] = [] for key, ref in self._conversation_refs.items(): - if self._is_webchat_service_url(ref.service_url): + if self.config.prune_web_chat_refs and self._is_webchat_service_url(ref.service_url): keys_to_drop.append(key) continue conv_type = str(ref.conversation_type or "").strip().lower() - if conv_type and conv_type != "personal": + if self.config.prune_non_personal_refs and conv_type and conv_type != "personal": keys_to_drop.append(key) continue @@ -544,10 +550,32 @@ class MSTeamsChannel(BaseChannel): logger.info( "MSTeams pruned {} stale/unsupported conversation refs (ttl={} days)", len(keys_to_drop), - MSTEAMS_REF_TTL_DAYS, + ttl_days, ) return True + def _write_refs_atomically(self, data: dict[str, Any]) -> None: + """Write refs JSON atomically to reduce corruption risk during crashes.""" + payload = json.dumps(data, indent=2) + tmp_path: str | None = None + try: + fd, tmp_path = tempfile.mkstemp( + dir=str(self._refs_path.parent), + prefix=f"{self._refs_path.name}.", + suffix=".tmp", + ) + with os.fdopen(fd, "w", encoding="utf-8") as f: + f.write(payload) + f.flush() + os.fsync(f.fileno()) + os.replace(tmp_path, self._refs_path) + finally: + if tmp_path and os.path.exists(tmp_path): + try: + os.unlink(tmp_path) + except OSError: + pass + def _save_refs(self, *, prune: bool = True) -> None: """Persist conversation references.""" try: @@ -565,7 +593,7 @@ class MSTeamsChannel(BaseChannel): } for key, ref in self._conversation_refs.items() } - self._refs_path.write_text(json.dumps(data, indent=2), encoding="utf-8") + self._write_refs_atomically(data) except Exception as e: logger.warning("Failed to save MSTeams conversation refs: {}", e) diff --git a/tests/test_msteams.py b/tests/test_msteams.py index 4febd7915..da6bf511c 100644 --- a/tests/test_msteams.py +++ b/tests/test_msteams.py @@ -206,6 +206,115 @@ def test_save_prunes_unsupported_conversation_refs(make_channel, tmp_path, monke assert set(saved.keys()) == {"conv-valid"} +def test_init_respects_prune_toggle_flags(make_channel, tmp_path, monkeypatch): + now = 1_800_000_000.0 + monkeypatch.setattr(msteams_module.time, "time", lambda: now) + + state_dir = tmp_path / "state" + state_dir.mkdir(parents=True, exist_ok=True) + refs_path = state_dir / "msteams_conversations.json" + refs_path.write_text( + json.dumps( + { + "conv-webchat": { + "service_url": "https://webchat.botframework.com/", + "conversation_id": "conv-webchat", + "conversation_type": "personal", + "updated_at": now - 60, + }, + "conv-group": { + "service_url": "https://smba.trafficmanager.net/amer/", + "conversation_id": "conv-group", + "conversation_type": "channel", + "updated_at": now - 60, + }, + }, + indent=2, + ), + encoding="utf-8", + ) + + ch = make_channel(pruneWebChatRefs=False, pruneNonPersonalRefs=False) + + assert set(ch._conversation_refs.keys()) == {"conv-webchat", "conv-group"} + persisted = json.loads(refs_path.read_text(encoding="utf-8")) + assert set(persisted.keys()) == {"conv-webchat", "conv-group"} + + +def test_init_respects_custom_ref_ttl_days(make_channel, tmp_path, monkeypatch): + now = 1_800_000_000.0 + monkeypatch.setattr(msteams_module.time, "time", lambda: now) + + state_dir = tmp_path / "state" + state_dir.mkdir(parents=True, exist_ok=True) + refs_path = state_dir / "msteams_conversations.json" + refs_path.write_text( + json.dumps( + { + "conv-fresh": { + "service_url": "https://smba.trafficmanager.net/amer/", + "conversation_id": "conv-fresh", + "conversation_type": "personal", + "updated_at": now - 12 * 60 * 60, + }, + "conv-old": { + "service_url": "https://smba.trafficmanager.net/amer/", + "conversation_id": "conv-old", + "conversation_type": "personal", + "updated_at": now - 10 * 24 * 60 * 60, + }, + }, + indent=2, + ), + encoding="utf-8", + ) + + ch = make_channel(refTtlDays=1) + + assert set(ch._conversation_refs.keys()) == {"conv-fresh"} + persisted = json.loads(refs_path.read_text(encoding="utf-8")) + assert set(persisted.keys()) == {"conv-fresh"} + + +def test_save_uses_atomic_replace_and_keeps_existing_file_on_replace_error(make_channel, tmp_path, monkeypatch): + ch = make_channel() + refs_path = tmp_path / "state" / "msteams_conversations.json" + refs_path.write_text( + json.dumps( + { + "conv-old": { + "service_url": "https://smba.trafficmanager.net/amer/", + "conversation_id": "conv-old", + "conversation_type": "personal", + "updated_at": 1_700_000_000.0, + } + }, + indent=2, + ), + encoding="utf-8", + ) + + ch._conversation_refs = { + "conv-new": ConversationRef( + service_url="https://smba.trafficmanager.net/amer/", + conversation_id="conv-new", + conversation_type="personal", + updated_at=1_800_000_000.0, + ) + } + + def _raise_replace(_src, _dst): + raise OSError("replace failed") + + monkeypatch.setattr(msteams_module.os, "replace", _raise_replace) + ch._save_refs() + + persisted = json.loads(refs_path.read_text(encoding="utf-8")) + assert set(persisted.keys()) == {"conv-old"} + tmp_files = list((tmp_path / "state").glob("msteams_conversations.json.*.tmp")) + assert tmp_files == [] + + @pytest.mark.asyncio async def test_handle_activity_ignores_group_messages(make_channel): ch = make_channel() @@ -644,6 +753,9 @@ def test_msteams_default_config_includes_restart_notify_fields(): cfg = MSTeamsChannel.default_config() assert cfg["validateInboundAuth"] is True + assert cfg["refTtlDays"] == msteams_module.MSTEAMS_REF_TTL_DAYS + assert cfg["pruneWebChatRefs"] is True + assert cfg["pruneNonPersonalRefs"] is True assert "restartNotifyEnabled" not in cfg assert "restartNotifyPreMessage" not in cfg assert "restartNotifyPostMessage" not in cfg