mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-05-23 10:02:34 +00:00
feat(msteams): make ref pruning configurable and atomic
This commit is contained in:
parent
106ae2cf1f
commit
15e9d0471f
@ -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 (`<at>Nanobot</at>`). 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**
|
||||
|
||||
|
||||
@ -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)
|
||||
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user