fix(mattermost): resolve thread follow-ups and tool output leaks (#4221) - #4230
fix(mattermost): resolve thread follow-ups and tool output leaks (#4221)#4230devorun wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing both aspects of Mattermost thread usability. The thread-local delivery half has since landed on current main, but the unmentioned-follow-up behavior remains a distinct salvage target.
Problems
gateway/platforms/mattermost.py:104adds an unbounded, never-expiring_known_threadsset. Every remembered root permanently bypasses the channel mention gate.gateway/platforms/mattermost.py:270forwardsreply_todirectly asroot_id; current main resolves a reply to its actual Mattermost root inplugins/platforms/mattermost/adapter.py:326-340.- The branch silently changes the documented
MATTERMOST_REQUIRE_MENTION=truebehavior (website/docs/reference/environment-variables.md:433) and supplies no regression tests.
Suggested changes
- Port the remaining sticky-follow-up behavior to
plugins/platforms/mattermost/adapter.py; the legacy target path was removed byaf973e407. - Use bounded/explicit sticky ownership and the current root-resolution helper, with tests for expiry and nested replies.
Automated hermes-sweeper review.
| self._SEEN_MAX = 2000 | ||
| self._SEEN_TTL = 300 # 5 minutes | ||
|
|
||
| self._known_threads: set[str] = set() |
There was a problem hiding this comment.
This set is never bounded or expired, so a long-lived gateway retains every thread root and permanently bypasses mention gating for it. Please use bounded ownership/expiry semantics rather than process-lifetime membership.
| formatted = self.format_message(content) | ||
| chunks = self.truncate_message(formatted, MAX_POST_LENGTH) | ||
|
|
||
| thread_root = reply_to |
There was a problem hiding this comment.
reply_to may be a reply post rather than the Mattermost thread root. Resolve it before assigning root_id; current main does this through _resolve_root_id() to avoid invalid-root errors.
What does this PR do?
Fixes #4221 — Mattermost threaded conversations broken in two ways.
1. Tool/reasoning output leaking to main channel
The
sendmethod now falls back tometadata.get("thread_id")whenreply_tois not set, using it asroot_idin the payload. All tool and processing output now stays inside the thread.2. Follow-up messages ignored inside threads
Added
_known_threadsset to the adapter. When the bot posts into a thread, it registers that thread's ID. In_handle_ws_event, incoming messages from known threads bypass the@mentiongate and continue the conversation naturally.Related Issue
Fixes #
Type of Change
Changes Made
How to Test
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AFor New Skills
hermes --toolsets skills -q "Use the X skill to do Y"Screenshots / Logs