feat(mattermost): add rich posts and passive observation - #68980
feat(mattermost): add rich posts and passive observation#68980nawaf-inter4 wants to merge 3 commits into
Conversation
Related: #59401 (feature request), #59420 and #60464 (open rich-markdown alternatives), and #26901 (ambient ingestion). This PR has a broader structured-post, interaction, and passive-observation contract; maintainer guidance is needed on the rendering and ingestion policy. |
75196bc to
34118c1
Compare
|
Thanks for the triage and related references. The intended policy boundaries are:
I’m happy to split passive observation and/or interactions into separate PRs, or narrow this PR to the preferred rendering policy, based on maintainer guidance. In particular, guidance on whether native attachments should remain additive to canonical Markdown—and whether ambient Mattermost ingestion should be reviewed separately alongside #26901—would be appreciated. |
34118c1 to
2a323d6
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the substantial Mattermost work. The opt-in/fallback design is thoughtful, but three issues need resolution before this can be safely salvaged.
Problems
gateway/stream_consumer.py:419and:446directly send transformed appendices/replacements. The normal overflow paths at:950-973and final fallback at:1531-1549split to platform limits; these new paths do not. An oversized transformed response can therefore fail delivery.plugins/platforms/mattermost/adapter.py:1611-1613makes every top-level post its own thread scope inreply_mode="thread". Observed top-level content is saved with that scope at:1665, while a later top-level mention gets a different scope at:1822; it cannot receive the intended channel context.plugins/platforms/mattermost/plugin.yaml:56-59adds a behavioral.envsetting.AGENTS.md:102-107requires user-facing behavioral settings to live inconfig.yaml.
Suggested changes
- Chunk transformed replacements/appendices through the existing safe-limit logic and test an over-limit non-prefix transform.
- Keep top-level observation channel-scoped; use thread-root scope only for replies, with a
reply_mode=threadregression test. - Remove the new observation env var and document/configure it only through YAML.
Automated hermes-sweeper review.
| obsolete_ids = set(self._segment_preview_message_ids) | ||
| if self._message_id and self._message_id != "__no_edit__": | ||
| obsolete_ids.add(str(self._message_id)) | ||
| result = await self.adapter.send( |
There was a problem hiding this comment.
This direct send bypasses the safe-limit splitting used by normal overflow and final-fallback delivery. A non-prefix transform can be larger than the platform limit, so split transformed (and the analogous appendix path) before sending and add an over-limit regression test.
There was a problem hiding this comment.
Addressed in 9607554. Transformed appendices and non-prefix replacements now use platform-safe chunking before delivery. Replacement cleanup runs only after every chunk succeeds, so a partial-send failure preserves the existing streamed messages. Added regressions for oversized appendices, oversized non-prefix replacements, send-before-delete ordering, and failure on a later replacement chunk.
|
|
||
| def _mattermost_thread_id(self, post: Dict[str, Any], chat_type: str) -> Optional[str]: | ||
| thread_id = post.get("root_id") or None | ||
| if not thread_id and self._reply_mode == "thread" and chat_type != "dm": |
There was a problem hiding this comment.
This makes each top-level post a distinct session key in thread mode. Since observed top-level posts and later top-level mentions have different post IDs, channel observation cannot be retrieved by the later mention. Keep top-level observation channel-scoped and reserve thread-root scope for replies.
There was a problem hiding this comment.
Addressed in 9607554. Mattermost session scope now uses root_id only for actual replies. Top-level posts remain channel-scoped even when reply_mode is thread, while response rendering can still use the top-level post as its delivery thread. Added a regression covering an observed top-level post followed by a later top-level mention.
| @@ -47,3 +53,7 @@ optional_env: | |||
| description: "If set, the bot only responds in these channels (whitelist)." | |||
| prompt: "Allowed channel IDs (comma-separated)" | |||
| password: false | |||
| - name: MATTERMOST_OBSERVE_UNMENTIONED_CHANNEL_MESSAGES | |||
There was a problem hiding this comment.
This is a non-secret behavioral setting exposed through setup/.env. Per AGENTS.md:102-107, make observe_unmentioned_channel_messages config.yaml-only and remove this optional env variable and its documented equivalent.
There was a problem hiding this comment.
Addressed in 9607554. observe_unmentioned_channel_messages is now config.yaml-only: the plugin setup entry, adapter environment lookup, English/Chinese documented equivalent, and environment-variable reference were removed. Added a regression proving the former environment setting is ignored.
Add opt-in Mattermost-native structured messages, secure interactive approvals and feedback, context-only allowlisted channel observation, and safe plain streaming fallbacks.
2a323d6 to
9607554
Compare
SummaryTwo open PRs reference #59401 but address different causes: #60464 only marks Mattermost payloads as Markdown, while #68980 adds opt-in attachment-based rich posts with Markdown fallback plus substantially broader interaction and passive-observation functionality. The visible diffs confirm that neither adds the requested Mattermost-flavored Markdown transformer for mentions, emoji, math, or diagrams. Related pull requests
Duplicates#60464 and #68980 are not duplicates: #60464 adds an unconditional payload-format property, whereas #68980 implements opt-in attachment rendering and a much broader interaction/observation contract. Suggested consolidationKeep #68980 open with a salvage path focused on its opt-in rich-post renderer, Markdown-preserving fallback, and associated delivery tests; split the interaction and passive-observation work if maintainers do not approve that broader scope. Keep #60464 separate only if its payload-format property is independently wanted, with the failing exact-payload test updated and the PR respecified away from closing #59401; otherwise close it as not addressing this issue rather than as a duplicate. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I59401(["issue #59401 (open)"])
P68980["PR #68980 (open)"]
P68980 -->|best fix| I59401
class I59401 open
class P68980 open
class P68980 best
class P68980 target
click I59401 "https://github.com/NousResearch/hermes-agent/issues/59401"
click P68980 "https://github.com/NousResearch/hermes-agent/pull/68980"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 2 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 198 kB of PR diffs, 10 kB of issue/PR text, 5 kB of discussion (10 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Summary
Adds opt-in Mattermost-native rich posts, secure interactive actions, and passive
allowlisted channel observation. This provides parity with Hermes's Slack rich-output
experience and Telegram's context-only group observation without sending Slack Block
Kit payloads to Mattermost.
props.attachmentsmessageand attachmentfallbackdispatching the agent
passive content is persisted
Closes #59401.
Related work
PRs #59420 and #60464 address Markdown normalization/formatting. This change keeps ordinary Markdown lossless and additive, while implementing the broader Mattermost-native structured-post, final-edit, approval, feedback, fallback, and callback-security contract requested in #59401.
Configuration
Interactive actions also require
MATTERMOST_INTERACTION_SECRET.interaction_allowed_cidrsauthenticates the direct Mattermost-to-Hermes network peer. Reverse-proxy deployments must restrict or authenticate the proxy route independently; Hermes deliberately does not trust caller-supplied forwarding headers. Non-loopback callback URLs require HTTPS.Security
approval_idbinding so a button resolves only the command it displayedBehavior and compatibility
messageremains intentionally populated alongsideattachments for search, notifications, accessibility clients, and API consumers
Passive channel observation
observe_unmentioned_channel_messagesdefaults to falseexplicit
allowed_channelslistobserved: true; they do not invokethe agent or produce a response
before passive persistence or shared-session dispatch; webhook, bot, system,
command-shaped, and out-of-scope posts remain ignored
rather than replaying them as pending requests
attribution
Tests
The repository-wide suite was also exercised, but this checkout reports unrelated baseline/environment failures across ACP, WSL/service-manager, Matrix, file-tool, macOS
/tmppath normalization, system-guard, and other suites. The focused feature, routing, streaming, and exact-approval suites above pass.