fix(mattermost): seed first thread turn context - #38152
Conversation
alt-glitch
added
type/feature
e0842b2 to
86b1049
Compare
…ity)
The Mattermost adapter only saw the single message it was @mentioned in and
required a fresh @mention every turn. The Slack adapter already solves both;
this ports that pattern to Mattermost.
- In-thread auto-response: after the first @mention in a thread, subsequent
messages in that thread auto-trigger the bot. Opt out with
MATTERMOST_STRICT_MENTION=true (mirrors SLACK_STRICT_MENTION).
- Thread context: on the first turn in a pre-existing thread, prior messages
are fetched (GET /posts/{root}/thread) and injected via channel_context.
- Security: injected thread history is filtered to allowlisted authors
(MATTERMOST_ALLOWED_USERS / *_ALLOW_ALL_USERS). The authz layer only checks
the triggering author; channel_context bypasses it, so unfiltered history
would leak non-allowlisted users' messages into the model.
- Session-key parity: _has_active_session_for_thread passes the adapter's real
chat_type so the key matches what handle_message persists.
Adds TestMattermostThreadBehavior (12 cases) and documents
MATTERMOST_STRICT_MENTION + the allowlist-filtered context behavior.
Related: NousResearch#38362, NousResearch#38152, NousResearch#37144 (thread-context PRs without the allowlist filter).
|
Thanks for this 🙏 Same note as on #38362: #43805 also seeds first-turn thread context, and adds an allowlist filter on the seeded messages. Why it matters: |
When @mentioned inside an existing thread for the first time, seed the prior
thread history as context (GET /posts/{root}/thread) via MessageEvent.channel_context,
so the agent sees the whole conversation, not just the triggering message.
Security: the authz layer only checks the *triggering* message's author;
injected channel_context bypasses it. So seeded history is filtered to authors
in MATTERMOST_ALLOWED_USERS (or everyone when *_ALLOW_ALL_USERS is set) to avoid
leaking a non-allowlisted user's messages into the model. MATTERMOST_THREAD_CONTEXT
controls the policy: allowlisted (default), off, or all (full thread without
widening who may invoke the bot). Fails open on fetch error.
Stacked on the in-thread auto-response change (uses _has_active_session_for_thread
as the first-turn guard).
Adds TestMattermostThreadContext and documents MATTERMOST_THREAD_CONTEXT.
Supersedes NousResearch#38362, NousResearch#38152 (thread context without the allowlist filter).
86b1049 to
74a8691
Compare
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for implementing the missing Mattermost thread-history path. The premise remains valid on current main: plugins/platforms/mattermost/adapter.py:867-960 keys threads but does not fetch or attach prior posts.
Problems
plugins/platforms/mattermost/adapter.py:983prepends history intotext. In shared threads,gateway/run.py:10388-10395prefixes that whole value with the triggering sender. UseMessageEvent.channel_contextso backfill retains its own attribution and the gateway inserts[New message]correctly.plugins/platforms/mattermost/adapter.py:969checks for/before current main's leading-space command normalization (plugins/platforms/mattermost/adapter.py:881-884,1197d2bc9). A/newcan receive injected context and cease to be a command.- The new user-facing
.envsetting athermes_cli/config.py:3282should instead use the existing YAML path added at adapter lines 1314-1316.
Suggested changes
- Attach seeded history through
channel_context, normalize command text before the fetch guard, and add a gateway-level regression covering shared-thread attribution plus/new.
This is an automated hermes-sweeper review.
| current_post_id=post_id, | ||
| ) | ||
| if thread_context: | ||
| message_text = thread_context + message_text |
There was a problem hiding this comment.
Blocking: keep this backfill out of message_text. Current GatewayRunner._prepare_inbound_message_text() prefixes shared-session text with the triggering sender before it handles event.channel_context (gateway/run.py:10388-10395), so this assigns every recovered post to the trigger author. Pass the block through MessageEvent.channel_context instead.
| thread_id = post.get("root_id") or None | ||
| if ( | ||
| thread_id | ||
| and not message_text.startswith("/") |
There was a problem hiding this comment.
Blocking on current main: leading-space slash commands are normalized later (plugins/platforms/mattermost/adapter.py:881-884, commit 1197d2bc9). For /new, this guard fetches context and line 983 changes the leading character, so command classification no longer occurs. Normalize/check lstrip() before this guard and add a regression test.
| "password": False, | ||
| "category": "messaging", | ||
| }, | ||
| "MATTERMOST_THREAD_CONTEXT": { |
There was a problem hiding this comment.
This is a new user-facing non-secret behavior setting in the environment configuration. Please make mattermost.thread_context in config.yaml the canonical setup/documented surface instead; the adapter patch already includes a YAML bridge for that key.
teknium1
added
sweeper:risk-session-state
|
Closing in favor of #43805. That PR covers the same Mattermost first-turn thread-context gap and adds the necessary allowlist filtering so injected thread history cannot bypass the triggering-author authorization boundary. |
4 participants
Summary
GET /api/v4/posts/{root_id}/threadon the first Hermes turn in a thread.Fixes #37695.
Tests
.\.venv\Scripts\python.exe -m pytest tests/gateway/test_mattermost.py::TestMattermostWebSocketParsing::test_first_thread_turn_omits_prior_slash_commands_from_context -q --timeout-method=threadfailed because only the triggering message reached the agent..\.venv\Scripts\python.exe -m pytest tests/gateway/test_mattermost.py::TestMattermostWebSocketParsing::test_first_thread_turn_omits_prior_slash_commands_from_context -q --timeout-method=thread->1 passeduv run --extra dev --extra messaging python -m pytest tests/gateway/test_mattermost.py -q --timeout-method=thread->46 passed.\.venv\Scripts\python.exe -m ruff check plugins/platforms/mattermost/adapter.py tests/gateway/test_mattermost.py->All checks passed!.\.venv\Scripts\python.exe -m py_compile plugins/platforms/mattermost/adapter.py tests/gateway/test_mattermost.py-> passedgit diff --check-> passed