feat(mattermost): interactive permission-request buttons (Approve/Deny, slash-confirm, clarify) - #66823
Conversation
Competing Mattermost interactive-action work: #26537 uses an earlier standalone callback server, while #29373 uses WebhookAdapter reuse and covers clarify. This PR's threaded clarify scope is related rather than duplicate; a maintainer should select the implementation. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for adding native Mattermost controls. The capability is still absent on current main, but this revision has blockers.
Problems
plugins/platforms/mattermost/adapter.py:1471-1478does not acceptallow_permanentorsmart_denied; currentgateway/run.py:19479-19487always passes both. That makes exec approvals raiseTypeErrorand fall back to text.- The new listener defaults to
0.0.0.0atadapter.py:140-146, but callbackuser_idis only read (adapter.py:1580), not authorized beforeresolve_gateway_approval()(adapter.py:1657).MATTERMOST_ALLOWED_USERSalready exists in config (hermes_cli/config.py:4256-4261). - The clarify Other branch uses undefined
chat_idatadapter.py:1632; regular clicks resolve the index rather than the selected string atadapter.py:1634-1635, contrary to the adapter contract ingateway/platforms/base.py:3167-3174. - The PR contains no tests or Mattermost callback setup/security documentation.
Suggested changes
- Align the approval method with the current gateway kwargs and cover that integration path.
- Authorize callbacks before consuming pending state; use a safe listener/configuration model.
- Fix clarify routing and add adapter tests for callback authorization, stale/double clicks, threading, and all prompt types.
Automated hermes-sweeper review.
| session_key: str, | ||
| description: str = "dangerous command", | ||
| metadata: Optional[Dict[str, Any]] = None, | ||
| ) -> "SendResult": |
There was a problem hiding this comment.
Current gateway/run.py:19479-19487 passes allow_permanent and smart_denied to this method. Add those keyword parameters (and honor them in the buttons), otherwise this override raises TypeError and the gateway silently falls back to the text approval path.
| ).strip() | ||
| channel_id = str(ctx.get("channel_id") or body.get("channel_id") or "") | ||
| root_id = str(ctx.get("root_id") or body.get("root_id") or "") | ||
| user_id = str(body.get("user_id") or "") |
There was a problem hiding this comment.
user_id is parsed but never authorized before _route_action_click() resolves approvals. Because this PR defaults the listener to 0.0.0.0, validate it against the existing MATTERMOST_ALLOWED_USERS policy before consuming any pending action.
| except Exception: # noqa: BLE001 | ||
| self._clarify_state[clarify_id] = session_key | ||
| return False | ||
| await self._post_outcome(chat_id, "✏️ Type your answer:", root_id) |
There was a problem hiding this comment.
chat_id is undefined in _route_action_click; this raises after mark_awaiting_text() succeeds. Use the channel_id parameter and preserve/recover pending state if posting the outcome fails.
| return False | ||
| await self._post_outcome(chat_id, "✏️ Type your answer:", root_id) | ||
| return True | ||
| response_text = str(choice) |
There was a problem hiding this comment.
The action encodes a zero-based index, but resolve_gateway_clarify() must receive the chosen response string. Retain the choices with pending state or encode the selected value so the agent receives the actual option rather than "0", "1", or "2".
ad9f5cb to
f9eca7e
Compare
Summary
Mattermost was the only major chat platform without native interactive buttons for the agent's permission/confirmation prompts. This adds them to the Mattermost adapter, mirroring the proven WhatsApp Cloud clickback pattern (gateway/platforms/whatsapp_cloud.py).
send_exec_approval()- Approve / Deny buttons for permission requests (e.g. terminal command approval), resolved via tools.approval.send_slash_confirm()- Approve Once / Always Approve / Cancel buttons, resolved via tools.slash_confirm.send_clarify()- multiple-choice buttons, resolved via tools.clarify_gateway.Threading requirement
Prompts AND their click outcomes are posted in the SAME thread as the originating message (using root_id from the message metadata), so a permission request never spawns a stray top-level post.
Mechanism
Each button carries an action_id encoding the pending request (appr::approve|deny, sc:<once|always|cancel>:, cl::<idx|other>) and an integration.url pointing at a small loopback aiohttp callback server started in connect() (default http://127.0.0.1:8731/mattermost/actions). The handler parses the click, resolves the pending request, and posts the outcome in the same thread. Fails soft if the port can't bind.
Test plan
Generated with Hermes Agent (https://hermes-agent.nousresearch.com).