diff --git a/docs/guides/mattermost-ai-agent.md b/docs/guides/mattermost-ai-agent.md index f399d5da7..af049e02b 100644 --- a/docs/guides/mattermost-ai-agent.md +++ b/docs/guides/mattermost-ai-agent.md @@ -41,6 +41,7 @@ Merge this snippet into `~/.nanobot/config.json`: "token": "YOUR_MATTERMOST_TOKEN", "teamId": "YOUR_TEAM_ID", "groupPolicy": "mention", + "groupPolicyInThread": "open", "replyInThread": true, "dm": { "policy": "allowlist" @@ -51,7 +52,15 @@ Merge this snippet into `~/.nanobot/config.json`: ``` `teamId` scopes the channel to a Mattermost team. Keep `groupPolicy` as -`mention` for the first test. +`mention` for the first test. `groupPolicyInThread` can be `"mention"`, +`"open"`, or `"allowlist"` and controls messages that reply inside a +thread. If it is omitted, it inherits `groupPolicy`, preserving the behavior +of existing configurations. Set it to `"open"` explicitly when follow-up +messages in threads should not require another @mention. + +When `groupPolicy` is `"allowlist"`, `groupAllowFrom` remains the outer +channel boundary for root posts and thread replies. A thread policy cannot open +a channel that is not on that allowlist. Mattermost DMs are open by default. Setting `dm.policy` to `"allowlist"` with no `dm.allowFrom` entries makes new DM senders receive a pairing code. Approve the @@ -93,8 +102,8 @@ Then DM the bot again, or mention it in a channel where the bot has access: - If DMs are ignored, review the `dm` policy and pairing approval state. - If channel messages are ignored, confirm the bot is mentioned and belongs to the team/channel. -- If thread replies are surprising, review `replyInThread` and - `includeThreadContext`. +- If thread replies are surprising, review `groupPolicyInThread`, + `replyInThread`, and `includeThreadContext`. ## Next: memory, automations, MCP tools diff --git a/nanobot/channels/mattermost/manifest.py b/nanobot/channels/mattermost/manifest.py index 575e09565..4061a3513 100644 --- a/nanobot/channels/mattermost/manifest.py +++ b/nanobot/channels/mattermost/manifest.py @@ -10,6 +10,7 @@ SETUP_SPEC = ChannelSetupSpec( "token": field("secret"), "teamId": field(), "groupPolicy": field("enum", choices=GROUP_POLICIES, default="mention"), + "groupPolicyInThread": field("enum", choices=GROUP_POLICIES, default="mention"), "allowFrom": field("list"), }, required=required_fields("serverUrl", "token"), diff --git a/nanobot/channels/mattermost/runtime.py b/nanobot/channels/mattermost/runtime.py index 141c82452..6cd32fce3 100644 --- a/nanobot/channels/mattermost/runtime.py +++ b/nanobot/channels/mattermost/runtime.py @@ -9,7 +9,7 @@ from pathlib import Path from typing import Any, cast import httpx -from pydantic import Field +from pydantic import Field, model_validator from nanobot.bus.events import OutboundMessage from nanobot.bus.queue import MessageBus @@ -60,6 +60,22 @@ class MattermostConfig(Base): send_tool_hints: bool = True dm: MattermostDMConfig = Field(default_factory=MattermostDMConfig) + @model_validator(mode="before") + @classmethod + def _inherit_thread_policy(cls, data: Any) -> Any: + """Preserve the existing group policy unless a thread override is set.""" + if not isinstance(data, dict): + return data + raw = cast(dict[str, Any], data) + if "groupPolicyInThread" in raw or "group_policy_in_thread" in raw: + return raw + values = dict(raw) + values["group_policy_in_thread"] = values.get( + "groupPolicy", + values.get("group_policy", "mention"), + ) + return values + def _server_url_to_ws_url(server_url: str) -> str: if server_url.startswith("https://"): diff --git a/nanobot/channels/mattermost/tests/test_mattermost_channel.py b/nanobot/channels/mattermost/tests/test_mattermost_channel.py index a1fdefea0..1960e10b3 100644 --- a/nanobot/channels/mattermost/tests/test_mattermost_channel.py +++ b/nanobot/channels/mattermost/tests/test_mattermost_channel.py @@ -12,6 +12,7 @@ import pytest from nanobot.bus.events import OutboundMessage from nanobot.bus.queue import MessageBus +from nanobot.channels.mattermost.manifest import SETUP_SPEC from nanobot.channels.mattermost.runtime import ( MATTERMOST_MAX_MESSAGE_LEN, MattermostChannel, @@ -123,6 +124,25 @@ def test_config_defaults(): assert config.dm.enabled is True assert config.dm.policy == "open" assert config.reply_in_thread is True + assert config.group_policy_in_thread == "mention" + + +def test_thread_policy_inherits_group_policy_when_omitted(): + config = MattermostConfig.model_validate({"groupPolicy": "open"}) + assert config.group_policy_in_thread == "open" + + explicit = MattermostConfig.model_validate({ + "groupPolicy": "open", + "groupPolicyInThread": "mention", + }) + assert explicit.group_policy_in_thread == "mention" + + +def test_setup_contract_exposes_thread_policy(): + field = SETUP_SPEC.fields["groupPolicyInThread"] + assert field.kind == "enum" + assert field.choices == {"open", "mention", "allowlist"} + assert field.default == "mention" def test_config_camelcase_aliases(): @@ -376,15 +396,16 @@ async def test_group_policy_allowlist(): @pytest.mark.asyncio -async def test_group_policy_in_thread_default_open(): - """Thread uses open policy by default, so no mention needed in threads.""" +async def test_group_policy_in_thread_defaults_to_group_policy(): + """Existing configs keep their main-channel behavior in threads.""" channel, fake = _make_channel({"groupPolicy": "mention"}) channel._self_username = "nanobot" # In a main channel (not thread), mention is required assert channel._should_respond_in_channel("hello", "c1", in_thread=False) is False assert channel._should_respond_in_channel("@nanobot hello", "c1", in_thread=False) is True - # In a thread, no mention needed (default open policy) - assert channel._should_respond_in_channel("hello", "c1", in_thread=True) is True + # In a thread, the omitted override inherits mention policy. + assert channel._should_respond_in_channel("hello", "c1", in_thread=True) is False + assert channel._should_respond_in_channel("@nanobot hello", "c1", in_thread=True) is True @pytest.mark.asyncio @@ -410,6 +431,38 @@ async def test_group_policy_in_thread_open(): assert channel._should_respond_in_channel("hello", "c1", in_thread=True) is True +@pytest.mark.asyncio +async def test_posted_thread_event_uses_thread_policy(): + """A real posted event derives thread policy from its root_id.""" + channel, fake = _make_channel({ + "groupPolicy": "mention", + "groupPolicyInThread": "open", + "includeThreadContext": False, + }) + channel._self_id = "bot_id" + channel._self_username = "nanobot" + with patch.object(channel, "_handle_message", AsyncMock()) as mock_handle: + ws_msg = { + "event": "posted", + "data": { + "channel_type": "O", + "post": json.dumps({ + "id": "reply_1", + "user_id": "user_1", + "channel_id": "channel_1", + "message": "follow up without a mention", + "root_id": "root_1", + }), + }, + "broadcast": {}, + } + + await channel._handle_ws_message(ws_msg) + + mock_handle.assert_awaited_once() + assert mock_handle.call_args.kwargs["session_key"] == "mattermost:channel_1:root_1" + + @pytest.mark.asyncio async def test_group_policy_in_thread_allowlist(): """Thread uses allowlist policy when configured."""