fix(mattermost): extend session-continuity root-id fix to DM channels - #54229
fix(mattermost): extend session-continuity root-id fix to DM channels#54229wernerhp wants to merge 4 commits into
Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Clean DM session-continuity fix (57 additions). Extends root-id seeding to DM channels so threaded DM replies land in the same session. Includes regression test and reply test. Simple, well-targeted.
Reviewed by Hermes Agent
c7b24d2 to
91e327c
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the Mattermost session-continuity fix (originally added for non-DM threads) to DM threads when reply_mode == "thread", ensuring a DM thread’s root post and its replies map to the same gateway session key and therefore keep agent context across threaded follow-ups.
Changes:
- Update the Mattermost adapter’s inbound
thread_idderivation so that, in thread reply mode, a root post falls back to using its ownpost_idasthread_id(now applied to DMs as well). - Replace the prior DM-specific expectation in tests with new assertions that DM root posts seed
thread_id, and DM threaded replies continue to useroot_idasthread_id.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| plugins/platforms/mattermost/adapter.py | Extends thread-mode root-post thread_id seeding to include DM channels, preventing DM thread session splits. |
| tests/gateway/test_mattermost.py | Adds/updates regression tests covering DM root seeding and DM reply thread_id behavior in thread mode. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
afe111f to
077e5d9
Compare
|
Thanks for the focused regression fix. Current main still excludes DM roots from thread-id seeding at The existing test at Automated hermes-sweeper review. |
fcf320b to
2c78cab
Compare
… (PR NousResearch#37144) Root DM posts had thread_id=None; replies carried root_id=<root>. Result: two different session keys → agent lost all context on first reply. Prior fix excluded DM channels (channel_type_raw != 'D' guard). This removes that exclusion so DMs get the same treatment: thread_id = post.get('root_id') or post_id (in thread mode) Port of upstream PR NousResearch#37144 extended to cover DM channel type.
…ix (NousResearch#37144) The prior test asserted thread_id=None for DM root posts — which was the old broken behaviour that caused session context loss on first threaded reply. Commit a062424fb intentionally removed the 'channel_type_raw != D' guard so DMs get the same root-post seeding as channels. Update the test to: - Rename and doc the test to describe the fixed behaviour - Assert thread_id == post_id for DM root posts (correct) - Add companion test: DM replies carry root_id → same session key
a9d1849 to
a4fcd8f
Compare
What does this PR do?
Extends the session-continuity fix from #37144 to DM channels, which the original PR excluded via
channel_type_raw != "D".Root DM posts had
thread_id=None; replies carriedroot_id=<root post id>. This produced two different session keys, so the agent lost all context on the first threaded DM reply. #37144 fixed this for channels but left DMs broken.Removing the
channel_type_raw != "D"guard gives DMs the same treatment:thread_id = post.get("root_id") or post_idwhenreply_mode == "thread".Related Issue
Fixes #18279
Relates to #37144
Type of Change
Changes Made
plugins/platforms/mattermost/adapter.py: removechannel_type_raw != "D"guard from thread-id seeding logic.tests/gateway/test_mattermost.py: 2 new tests — DM root post seeding and DM thread reply resolves to same session.How to Test
reply_mode: thread.pytest tests/gateway/test_mattermost.py -v -k "dm"— both new DM tests pass.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — N/Acli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/A