feat(mattermost): add emoji reactions support#18608
Conversation
|
@codex please review this PR. |
| ws.on("message", async (data) => { | ||
| const raw = rawDataToString(data); | ||
| let payload: MattermostEventPayload; | ||
| try { | ||
| payload = JSON.parse(raw) as MattermostEventPayload; | ||
| } catch { | ||
| return; | ||
| } | ||
|
|
||
| if (payload.event === "reaction_added" || payload.event === "reaction_removed") { | ||
| if (!opts.onReaction) { | ||
| return; | ||
| } | ||
| try { | ||
| await opts.onReaction(payload); | ||
| } catch (err) { | ||
| opts.runtime.error?.(`mattermost reaction handler failed: ${String(err)}`); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| if (payload.event !== "posted") { | ||
| return; | ||
| } | ||
| const parsed = parsePostedEvent(data); |
There was a problem hiding this comment.
Double JSON parsing for every posted event
The new early-parsing block (lines 140-146) parses the raw websocket data and checks the event type. For "posted" events, execution falls through to parsePostedEvent(data) on line 163 which calls rawDataToString(data) and JSON.parse(raw) again — parsing the same message twice for the most common event type.
Consider passing the already-parsed payload into a helper that extracts the post data, instead of re-parsing from data:
| ws.on("message", async (data) => { | |
| const raw = rawDataToString(data); | |
| let payload: MattermostEventPayload; | |
| try { | |
| payload = JSON.parse(raw) as MattermostEventPayload; | |
| } catch { | |
| return; | |
| } | |
| if (payload.event === "reaction_added" || payload.event === "reaction_removed") { | |
| if (!opts.onReaction) { | |
| return; | |
| } | |
| try { | |
| await opts.onReaction(payload); | |
| } catch (err) { | |
| opts.runtime.error?.(`mattermost reaction handler failed: ${String(err)}`); | |
| } | |
| return; | |
| } | |
| if (payload.event !== "posted") { | |
| return; | |
| } | |
| const parsed = parsePostedEvent(data); | |
| ws.on("message", async (data) => { | |
| const raw = rawDataToString(data); | |
| let payload: MattermostEventPayload; | |
| try { | |
| payload = JSON.parse(raw) as MattermostEventPayload; | |
| } catch { | |
| return; | |
| } | |
| if (payload.event === "reaction_added" || payload.event === "reaction_removed") { | |
| if (!opts.onReaction) { | |
| return; | |
| } | |
| try { | |
| await opts.onReaction(payload); | |
| } catch (err) { | |
| opts.runtime.error?.(`mattermost reaction handler failed: ${String(err)}`); | |
| } | |
| return; | |
| } | |
| if (payload.event !== "posted") { | |
| return; | |
| } | |
| const parsed = parsePostedPayload(payload); |
This would need a new parsePostedPayload(payload) helper (extracted from the second half of parsePostedEvent) that takes the already-parsed payload instead of raw data.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/mattermost/src/mattermost/monitor-websocket.ts
Line: 139:163
Comment:
**Double JSON parsing for every posted event**
The new early-parsing block (lines 140-146) parses the raw websocket data and checks the event type. For `"posted"` events, execution falls through to `parsePostedEvent(data)` on line 163 which calls `rawDataToString(data)` and `JSON.parse(raw)` again — parsing the same message twice for the most common event type.
Consider passing the already-parsed payload into a helper that extracts the post data, instead of re-parsing from `data`:
```suggestion
ws.on("message", async (data) => {
const raw = rawDataToString(data);
let payload: MattermostEventPayload;
try {
payload = JSON.parse(raw) as MattermostEventPayload;
} catch {
return;
}
if (payload.event === "reaction_added" || payload.event === "reaction_removed") {
if (!opts.onReaction) {
return;
}
try {
await opts.onReaction(payload);
} catch (err) {
opts.runtime.error?.(`mattermost reaction handler failed: ${String(err)}`);
}
return;
}
if (payload.event !== "posted") {
return;
}
const parsed = parsePostedPayload(payload);
```
This would need a new `parsePostedPayload(payload)` helper (extracted from the second half of `parsePostedEvent`) that takes the already-parsed payload instead of raw data.
How can I resolve this? If you propose a fix, please make it concise.| await createReaction(client, { | ||
| userId, | ||
| postId: params.postId, | ||
| emojiName: params.emojiName, | ||
| }); |
There was a problem hiding this comment.
API errors bypass the Result return type
createReaction (and deleteReaction below) call client.request which throws on HTTP errors. Since those exceptions aren't caught here, addMattermostReaction will throw instead of returning { ok: false, error: "..." }, making the Result return type misleading.
This works in practice because handleAction in channel.ts will propagate the thrown error, but it's inconsistent with the { ok: false, error } handling for config-level issues above. Consider wrapping the API call in a try/catch to return a consistent Result:
| await createReaction(client, { | |
| userId, | |
| postId: params.postId, | |
| emojiName: params.emojiName, | |
| }); | |
| try { | |
| await createReaction(client, { | |
| userId, | |
| postId: params.postId, | |
| emojiName: params.emojiName, | |
| }); | |
| } catch (err) { | |
| return { ok: false, error: `Mattermost add reaction failed: ${String(err)}` }; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/mattermost/src/mattermost/reactions.ts
Line: 53:57
Comment:
**API errors bypass the Result return type**
`createReaction` (and `deleteReaction` below) call `client.request` which throws on HTTP errors. Since those exceptions aren't caught here, `addMattermostReaction` will throw instead of returning `{ ok: false, error: "..." }`, making the `Result` return type misleading.
This works in practice because `handleAction` in `channel.ts` will propagate the thrown error, but it's inconsistent with the `{ ok: false, error }` handling for config-level issues above. Consider wrapping the API call in a try/catch to return a consistent `Result`:
```suggestion
try {
await createReaction(client, {
userId,
postId: params.postId,
emojiName: params.emojiName,
});
} catch (err) {
return { ok: false, error: `Mattermost add reaction failed: ${String(err)}` };
}
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1b56efa42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
extensions/mattermost/src/channel.ts
Outdated
| const acctConfig = accountId && accounts ? accounts[accountId] : undefined; | ||
| const acctActions = acctConfig?.actions as { reactions?: boolean } | undefined; | ||
| const baseActions = mmBase?.actions as { reactions?: boolean } | undefined; | ||
| const reactionsEnabled = acctActions?.reactions ?? baseActions?.reactions ?? true; |
There was a problem hiding this comment.
Respect default account reaction gate when accountId is omitted
When accountId is not provided (the common path for message react unless --account is passed), acctConfig is forced to undefined, so reactionsEnabled ignores channels.mattermost.accounts.default.actions.reactions and falls back to the base setting. In configs where the default account disables reactions under accounts.default, this path still executes add/remove reaction calls, bypassing the per-account disable flag.
Useful? React with 👍 / 👎.
extensions/mattermost/src/channel.ts
Outdated
| const actionsConfig = cfg.channels?.mattermost?.actions as { reactions?: boolean } | undefined; | ||
| const reactionsEnabled = actionsConfig?.reactions !== false; | ||
| if (reactionsEnabled) { |
There was a problem hiding this comment.
Derive listed actions from per-account reaction settings
listActions only checks cfg.channels.mattermost.actions even though this change adds per-account actions.reactions configuration. In multi-account setups this misreports capability: react can be hidden when an enabled account explicitly enables it, or advertised when all enabled accounts have it disabled, causing avoidable runtime failures/confusing UX.
Useful? React with 👍 / 👎.
|
Addressed all current review comments in latest push ():\n\n1. Double JSON parse in websocket handler\n - Added and now reuse already-parsed payload for posted events.\n\n2. Result consistency in reactions API calls\n - Wrapped add/remove API calls in and now return on HTTP/client failures.\n\n3. Default-account reaction gate when accountId omitted\n - now resolves default account id and enforces per-account gate correctly.\n\n4. listActions capability drift in multi-account configs\n - now derives visibility from effective per-account reaction settings (account override > base setting).\n\nAlso added/updated tests for these behaviors.\n\n@codex review |
|
Addressed all current review comments in latest push (commit
Also added/updated tests for these behaviors. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c81da1c8dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }); | ||
|
|
||
| const cacheKey = `${baseUrl}:${botToken}`; | ||
| const userId = await resolveBotUserId(client, cacheKey); |
There was a problem hiding this comment.
Catch bot identity lookup errors in reaction helpers
addMattermostReaction and removeMattermostReaction are declared to return a Result, but resolveBotUserId(...) is awaited outside any try/catch. If /users/me fails (for example due to token expiry, network failures, or 5xx responses), these functions reject instead of returning { ok: false, error }, which breaks the consistency this commit added for reaction API failures. Wrapping bot-user resolution in the same error handling path would keep failures predictable for callers.
Useful? React with 👍 / 👎.
… now upstream via openclaw#18608) Add interactive message buttons and emoji reactions to the Mattermost extension, enabling agents to send messages with clickable action buttons and react to posts with emoji. Interactive buttons: - HMAC-SHA256 token verification for secure button callbacks - HTTP callback handler registered via registerPluginHttpRoute - Button click completion: replaces buttons with confirmation text - Localhost-only validation for callback requests - Stable HMAC secret derived from bot token (works across CLI/gateway) Reactions: - Add/remove emoji reactions via Mattermost REST API - Bot user ID caching with TTL for reaction requests - Reaction event handling in WebSocket monitor with DM/group policy enforcement Channel actions adapter: - supportsButtons, handleAction, listActions for the ChannelPlugin interface - Send action with optional button attachments - React action with add/remove support and emoji colon stripping Also includes: - updateMattermostPost for modifying existing posts (button completion) - props passthrough in createMattermostPost for attachments - parseMattermostTarget with channel-name and isMattermostId support - Comprehensive test coverage (58 new tests across 4 test files)
… now upstream via openclaw#18608) Add interactive message buttons and emoji reactions to the Mattermost extension, enabling agents to send messages with clickable action buttons and react to posts with emoji. Interactive buttons: - HMAC-SHA256 token verification for secure button callbacks - HTTP callback handler registered via registerPluginHttpRoute - Button click completion: replaces buttons with confirmation text - Localhost-only validation for callback requests - Stable HMAC secret derived from bot token (works across CLI/gateway) Reactions: - Add/remove emoji reactions via Mattermost REST API - Bot user ID caching with TTL for reaction requests - Reaction event handling in WebSocket monitor with DM/group policy enforcement Channel actions adapter: - supportsButtons, handleAction, listActions for the ChannelPlugin interface - Send action with optional button attachments - React action with add/remove support and emoji colon stripping Also includes: - updateMattermostPost for modifying existing posts (button completion) - props passthrough in createMattermostPost for attachments - parseMattermostTarget with channel-name and isMattermostId support - Comprehensive test coverage (58 new tests across 4 test files)
… now upstream via openclaw#18608) Add interactive message buttons and emoji reactions to the Mattermost extension, enabling agents to send messages with clickable action buttons and react to posts with emoji. Interactive buttons: - HMAC-SHA256 token verification for secure button callbacks - HTTP callback handler registered via registerPluginHttpRoute - Button click completion: replaces buttons with confirmation text - Localhost-only validation for callback requests - Stable HMAC secret derived from bot token (works across CLI/gateway) Reactions: - Add/remove emoji reactions via Mattermost REST API - Bot user ID caching with TTL for reaction requests - Reaction event handling in WebSocket monitor with DM/group policy enforcement Channel actions adapter: - supportsButtons, handleAction, listActions for the ChannelPlugin interface - Send action with optional button attachments - React action with add/remove support and emoji colon stripping Also includes: - updateMattermostPost for modifying existing posts (button completion) - props passthrough in createMattermostPost for attachments - parseMattermostTarget with channel-name and isMattermostId support - Comprehensive test coverage (58 new tests across 4 test files)
… now upstream via openclaw#18608) Add interactive message buttons and emoji reactions to the Mattermost extension, enabling agents to send messages with clickable action buttons and react to posts with emoji. Interactive buttons: - HMAC-SHA256 token verification for secure button callbacks - HTTP callback handler registered via registerPluginHttpRoute - Button click completion: replaces buttons with confirmation text - Localhost-only validation for callback requests - Stable HMAC secret derived from bot token (works across CLI/gateway) Reactions: - Add/remove emoji reactions via Mattermost REST API - Bot user ID caching with TTL for reaction requests - Reaction event handling in WebSocket monitor with DM/group policy enforcement Channel actions adapter: - supportsButtons, handleAction, listActions for the ChannelPlugin interface - Send action with optional button attachments - React action with add/remove support and emoji colon stripping Also includes: - updateMattermostPost for modifying existing posts (button completion) - props passthrough in createMattermostPost for attachments - parseMattermostTarget with channel-name and isMattermostId support - Comprehensive test coverage (58 new tests across 4 test files)
… now upstream via openclaw#18608) Add interactive message buttons and emoji reactions to the Mattermost extension, enabling agents to send messages with clickable action buttons and react to posts with emoji. Interactive buttons: - HMAC-SHA256 token verification for secure button callbacks - HTTP callback handler registered via registerPluginHttpRoute - Button click completion: replaces buttons with confirmation text - Localhost-only validation for callback requests - Stable HMAC secret derived from bot token (works across CLI/gateway) Reactions: - Add/remove emoji reactions via Mattermost REST API - Bot user ID caching with TTL for reaction requests - Reaction event handling in WebSocket monitor with DM/group policy enforcement Channel actions adapter: - supportsButtons, handleAction, listActions for the ChannelPlugin interface - Send action with optional button attachments - React action with add/remove support and emoji colon stripping Also includes: - updateMattermostPost for modifying existing posts (button completion) - props passthrough in createMattermostPost for attachments - parseMattermostTarget with channel-name and isMattermostId support - Comprehensive test coverage (58 new tests across 4 test files)
… now upstream via openclaw#18608) Add interactive message buttons and emoji reactions to the Mattermost extension, enabling agents to send messages with clickable action buttons and react to posts with emoji. Interactive buttons: - HMAC-SHA256 token verification for secure button callbacks - HTTP callback handler registered via registerPluginHttpRoute - Button click completion: replaces buttons with confirmation text - Localhost-only validation for callback requests - Stable HMAC secret derived from bot token (works across CLI/gateway) Reactions: - Add/remove emoji reactions via Mattermost REST API - Bot user ID caching with TTL for reaction requests - Reaction event handling in WebSocket monitor with DM/group policy enforcement Channel actions adapter: - supportsButtons, handleAction, listActions for the ChannelPlugin interface - Send action with optional button attachments - React action with add/remove support and emoji colon stripping Also includes: - updateMattermostPost for modifying existing posts (button completion) - props passthrough in createMattermostPost for attachments - parseMattermostTarget with channel-name and isMattermostId support - Comprehensive test coverage (58 new tests across 4 test files)
… now upstream via openclaw#18608) Add interactive message buttons and emoji reactions to the Mattermost extension, enabling agents to send messages with clickable action buttons and react to posts with emoji. Interactive buttons: - HMAC-SHA256 token verification for secure button callbacks - HTTP callback handler registered via registerPluginHttpRoute - Button click completion: replaces buttons with confirmation text - Localhost-only validation for callback requests - Stable HMAC secret derived from bot token (works across CLI/gateway) Reactions: - Add/remove emoji reactions via Mattermost REST API - Bot user ID caching with TTL for reaction requests - Reaction event handling in WebSocket monitor with DM/group policy enforcement Channel actions adapter: - supportsButtons, handleAction, listActions for the ChannelPlugin interface - Send action with optional button attachments - React action with add/remove support and emoji colon stripping Also includes: - updateMattermostPost for modifying existing posts (button completion) - props passthrough in createMattermostPost for attachments - parseMattermostTarget with channel-name and isMattermostId support - Comprehensive test coverage (58 new tests across 4 test files)
… now upstream via openclaw#18608) Add interactive message buttons and emoji reactions to the Mattermost extension, enabling agents to send messages with clickable action buttons and react to posts with emoji. Interactive buttons: - HMAC-SHA256 token verification for secure button callbacks - HTTP callback handler registered via registerPluginHttpRoute - Button click completion: replaces buttons with confirmation text - Localhost-only validation for callback requests - Stable HMAC secret derived from bot token (works across CLI/gateway) Reactions: - Add/remove emoji reactions via Mattermost REST API - Bot user ID caching with TTL for reaction requests - Reaction event handling in WebSocket monitor with DM/group policy enforcement Channel actions adapter: - supportsButtons, handleAction, listActions for the ChannelPlugin interface - Send action with optional button attachments - React action with add/remove support and emoji colon stripping Also includes: - updateMattermostPost for modifying existing posts (button completion) - props passthrough in createMattermostPost for attachments - parseMattermostTarget with channel-name and isMattermostId support - Comprehensive test coverage (58 new tests across 4 test files)
… now upstream via openclaw#18608) Add interactive message buttons and emoji reactions to the Mattermost extension, enabling agents to send messages with clickable action buttons and react to posts with emoji. Interactive buttons: - HMAC-SHA256 token verification for secure button callbacks - HTTP callback handler registered via registerPluginHttpRoute - Button click completion: replaces buttons with confirmation text - Localhost-only validation for callback requests - Stable HMAC secret derived from bot token (works across CLI/gateway) Reactions: - Add/remove emoji reactions via Mattermost REST API - Bot user ID caching with TTL for reaction requests - Reaction event handling in WebSocket monitor with DM/group policy enforcement Channel actions adapter: - supportsButtons, handleAction, listActions for the ChannelPlugin interface - Send action with optional button attachments - React action with add/remove support and emoji colon stripping Also includes: - updateMattermostPost for modifying existing posts (button completion) - props passthrough in createMattermostPost for attachments - parseMattermostTarget with channel-name and isMattermostId support - Comprehensive test coverage (58 new tests across 4 test files)
… now upstream via openclaw#18608) Add interactive message buttons and emoji reactions to the Mattermost extension, enabling agents to send messages with clickable action buttons and react to posts with emoji. Interactive buttons: - HMAC-SHA256 token verification for secure button callbacks - HTTP callback handler registered via registerPluginHttpRoute - Button click completion: replaces buttons with confirmation text - Localhost-only validation for callback requests - Stable HMAC secret derived from bot token (works across CLI/gateway) Reactions: - Add/remove emoji reactions via Mattermost REST API - Bot user ID caching with TTL for reaction requests - Reaction event handling in WebSocket monitor with DM/group policy enforcement Channel actions adapter: - supportsButtons, handleAction, listActions for the ChannelPlugin interface - Send action with optional button attachments - React action with add/remove support and emoji colon stripping Also includes: - updateMattermostPost for modifying existing posts (button completion) - props passthrough in createMattermostPost for attachments - parseMattermostTarget with channel-name and isMattermostId support - Comprehensive test coverage (58 new tests across 4 test files)
… now upstream via openclaw#18608) Add interactive message buttons and emoji reactions to the Mattermost extension, enabling agents to send messages with clickable action buttons and react to posts with emoji. Interactive buttons: - HMAC-SHA256 token verification for secure button callbacks - HTTP callback handler registered via registerPluginHttpRoute - Button click completion: replaces buttons with confirmation text - Localhost-only validation for callback requests - Stable HMAC secret derived from bot token (works across CLI/gateway) Reactions: - Add/remove emoji reactions via Mattermost REST API - Bot user ID caching with TTL for reaction requests - Reaction event handling in WebSocket monitor with DM/group policy enforcement Channel actions adapter: - supportsButtons, handleAction, listActions for the ChannelPlugin interface - Send action with optional button attachments - React action with add/remove support and emoji colon stripping Also includes: - updateMattermostPost for modifying existing posts (button completion) - props passthrough in createMattermostPost for attachments - parseMattermostTarget with channel-name and isMattermostId support - Comprehensive test coverage (58 new tests across 4 test files)
Summary
Refresh of Mattermost reactions support on top of current
main.Adds
message(action=react)support for Mattermost with add/remove reaction behavior.Changes
extensions/mattermost/src/mattermost/reactions.tsaddMattermostReactionremoveMattermostReactionreactaction inextensions/mattermost/src/channel.tsreaction_added/reaction_removedchannel.test.tsmattermost/reactions.test.tsmattermost/client.test.tsValidation
pnpm exec vitest run \ extensions/mattermost/src/channel.test.ts \ extensions/mattermost/src/mattermost/reactions.test.ts \ extensions/mattermost/src/mattermost/client.test.tsCloses #14305
Greptile Summary
Adds emoji reaction support (
message(action=react)) to the Mattermost extension, with both outbound (add/remove reactions via API) and inbound (websocket reaction event monitoring) paths.reactions.tsmodule withaddMattermostReaction/removeMattermostReactionfunctions that resolve the bot user ID (with caching) and call the Mattermost reactions APIhandleActionwired inchannel.tsviaChannelMessageActionAdapter, following the same patterns as Discord/Slack/Telegram/Signalactions.reactions) at both base and per-account levels in the schemareaction_added/reaction_removedevents to a newhandleReactionEventhandler with full DM/group policy enforcement mirroring the existing message handlerConfidence Score: 4/5
extensions/mattermost/src/mattermost/monitor-websocket.tshas redundant JSON parsing that could be cleaned up.Last reviewed commit: e1b56ef