feat: mattermost connector bot - #1142
Conversation
|
This PR was auto-closed. Only contributors approved with Maintainers review auto-closed issues daily. Issues that do not meet the quality bar in CONTRIBUTING.md will not be reopened or receive a reply. If a maintainer replies See CONTRIBUTING.md. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
12 issues found across 23 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/frontend/src/components/settings/mattermost-config-section.tsx">
<violation number="1" location="apps/frontend/src/components/settings/mattermost-config-section.tsx:6">
P2: Admins configuring Mattermost never get the project-specific webhook URL to register in Mattermost, so saved credentials alone leave the connector without an obvious way to receive events. The component imports `CopyableUrl` but never reads or renders `mattermostConfig.data.webhookUrl`; consider adding the same webhook card used by the other connector settings.</violation>
</file>
<file name="apps/backend/package.json">
<violation number="1" location="apps/backend/package.json:81">
P1: The Mattermost adapter requires chat@^4.29.0 as a peer dependency, but the project currently specifies chat@^4.15.0. If the resolved chat version is older than 4.29.0 the adapter may encounter missing APIs or behavioral changes at runtime. Consider bumping the project's chat dependency to ^4.29.0 to ensure compatibility with chat-adapter-mattermost@^1.1.3.</violation>
</file>
<file name="apps/backend/src/trpc/project.routes.ts">
<violation number="1" location="apps/backend/src/trpc/project.routes.ts:626">
P2: Mattermost configuration accepts non-URL values such as `foo`, which can be persisted and then passed to the Mattermost adapter during bot startup. Consider validating `baseUrl` as a URL at the API boundary so admins get a clear validation error before saving a broken integration.</violation>
</file>
<file name="apps/backend/migrations-postgres/0056_add_mattermost_channel.sql">
<violation number="1" location="apps/backend/migrations-postgres/0056_add_mattermost_channel.sql:1">
P1: Postgres migration checks will fail for this new migration because the matching Drizzle meta snapshot/journal entry was not added. Regenerate or commit the 0056 snapshot and _journal.json update with this SQL migration.</violation>
</file>
<file name="apps/backend/migrations-sqlite/0056_add_mattermost_channel.sql">
<violation number="1" location="apps/backend/migrations-sqlite/0056_add_mattermost_channel.sql:1">
P2: SQLite migration checks will fail because the new 0056 migration has no matching `migrations-sqlite/meta/0056_snapshot.json` or journal entry. Adding the generated Drizzle meta files alongside this SQL keeps CI and migration discovery in sync.</violation>
</file>
<file name="apps/backend/src/queries/project-mattermost-config.queries.ts">
<violation number="1" location="apps/backend/src/queries/project-mattermost-config.queries.ts:11">
P3: This adds another copy of the same LLM model-selection parser used by other messaging config queries. Consider moving `toLlmSelectedModel` to a shared utility so provider parsing behavior stays consistent across connectors.</violation>
</file>
<file name="apps/frontend/src/components/settings/mattermost-form.tsx">
<violation number="1" location="apps/frontend/src/components/settings/mattermost-form.tsx:33">
P2: The close/dismiss button only renders an X icon with no `aria-label`. Screen readers will announce it as a generic "button" with no indication of its action. Add `aria-label='Close'` (or `aria-label='Cancel'` to match `onCancel`) so assistive technology users can identify the control.</violation>
</file>
<file name="apps/backend/src/routes/mattermost.ts">
<violation number="1" location="apps/backend/src/routes/mattermost.ts:12">
P1: Mattermost outgoing webhooks configured as `application/x-www-form-urlencoded` can fail to parse or validate because this re-serializes Fastify's parsed body as JSON while preserving the original form content-type. Consider enabling `rawBody` like the Slack route and forwarding the exact raw payload, or updating the content-type to match the serialized body.</violation>
</file>
<file name="apps/frontend/src/components/settings-search-index.ts">
<violation number="1" location="apps/frontend/src/components/settings-search-index.ts:382">
P2: The search index description includes a `/` prefix (`/login`) for the Mattermost linking command, but the component's actual `loginHint` text shows `login <code>` (without the slash). This inconsistency will confuse users who search for one format and see the other in the UI.</violation>
</file>
<file name="apps/backend/src/services/mattermost.ts">
<violation number="1" location="apps/backend/src/services/mattermost.ts:55">
P1: `_startProject(config)` is not awaited inside `startForAllProjects()`. This means errors from bot initialization become unhandled promise rejections (the inner try-catch won't catch them), and the `logger.info` on the next line logs success before initialization completes. Add `await` before `this._startProject(config)`.</violation>
<violation number="2" location="apps/backend/src/services/mattermost.ts:119">
P1: `onNewMessage` uses `/.*/` (matches empty strings) and lacks a `message.isMention` guard. Since `onNewMention` and `onNewMessage` have identical copied logic, every @-mention fires both handlers, causing duplicate LLM streams, duplicate chat records, and duplicate responses. Follow the Slack pattern: use `[\s\S]+` as the regex and add `if (message.isMention) return;` at the top of the handler. Also consider extracting the shared handler body to avoid code duplication.</violation>
<violation number="3" location="apps/backend/src/services/mattermost.ts:404">
P2: Responses that finish after a tool call can leave the Mattermost message stuck in the live “Exploring...” state. Flushing the pending tool group before the final edit keeps the final message summarized even when no text part follows the tool result.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| "better-auth": "^1.6.3", | ||
| "better-sqlite3": "^12.5.0", | ||
| "chat": "^4.15.0", | ||
| "chat-adapter-mattermost": "^1.1.3", |
There was a problem hiding this comment.
P1: The Mattermost adapter requires chat@^4.29.0 as a peer dependency, but the project currently specifies chat@^4.15.0. If the resolved chat version is older than 4.29.0 the adapter may encounter missing APIs or behavioral changes at runtime. Consider bumping the project's chat dependency to ^4.29.0 to ensure compatibility with chat-adapter-mattermost@^1.1.3.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/package.json, line 81:
<comment>The Mattermost adapter requires chat@^4.29.0 as a peer dependency, but the project currently specifies chat@^4.15.0. If the resolved chat version is older than 4.29.0 the adapter may encounter missing APIs or behavioral changes at runtime. Consider bumping the project's chat dependency to ^4.29.0 to ensure compatibility with chat-adapter-mattermost@^1.1.3.</comment>
<file context>
@@ -78,6 +78,7 @@
"better-auth": "^1.6.3",
"better-sqlite3": "^12.5.0",
"chat": "^4.15.0",
+ "chat-adapter-mattermost": "^1.1.3",
"cheerio": "^1.2.0",
"cron-parser": "^5.5.0",
</file context>
| @@ -0,0 +1,3 @@ | |||
| ALTER TABLE "chat" ADD COLUMN "mattermost_thread_id" text;--> statement-breakpoint | |||
There was a problem hiding this comment.
P1: Postgres migration checks will fail for this new migration because the matching Drizzle meta snapshot/journal entry was not added. Regenerate or commit the 0056 snapshot and _journal.json update with this SQL migration.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/migrations-postgres/0056_add_mattermost_channel.sql, line 1:
<comment>Postgres migration checks will fail for this new migration because the matching Drizzle meta snapshot/journal entry was not added. Regenerate or commit the 0056 snapshot and _journal.json update with this SQL migration.</comment>
<file context>
@@ -0,0 +1,3 @@
+ALTER TABLE "chat" ADD COLUMN "mattermost_thread_id" text;--> statement-breakpoint
+ALTER TABLE "project" ADD COLUMN "mattermost_settings" jsonb;--> statement-breakpoint
+CREATE INDEX "chat_mattermost_thread_idx" ON "chat" USING btree ("mattermost_thread_id");
</file context>
| const webRequest = new Request(`http://localhost${request.url}`, { | ||
| method: request.method, | ||
| headers: convertHeaders(request.headers), | ||
| body: JSON.stringify(request.body), |
There was a problem hiding this comment.
P1: Mattermost outgoing webhooks configured as application/x-www-form-urlencoded can fail to parse or validate because this re-serializes Fastify's parsed body as JSON while preserving the original form content-type. Consider enabling rawBody like the Slack route and forwarding the exact raw payload, or updating the content-type to match the serialized body.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/src/routes/mattermost.ts, line 12:
<comment>Mattermost outgoing webhooks configured as `application/x-www-form-urlencoded` can fail to parse or validate because this re-serializes Fastify's parsed body as JSON while preserving the original form content-type. Consider enabling `rawBody` like the Slack route and forwarding the exact raw payload, or updating the content-type to match the serialized body.</comment>
<file context>
@@ -0,0 +1,35 @@
+ const webRequest = new Request(`http://localhost${request.url}`, {
+ method: request.method,
+ headers: convertHeaders(request.headers),
+ body: JSON.stringify(request.body),
+ });
+
</file context>
| const configs = await listMattermostConfigs(); | ||
| for (const config of configs) { | ||
| try { | ||
| this._startProject(config); |
There was a problem hiding this comment.
P1: _startProject(config) is not awaited inside startForAllProjects(). This means errors from bot initialization become unhandled promise rejections (the inner try-catch won't catch them), and the logger.info on the next line logs success before initialization completes. Add await before this._startProject(config).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/src/services/mattermost.ts, line 55:
<comment>`_startProject(config)` is not awaited inside `startForAllProjects()`. This means errors from bot initialization become unhandled promise rejections (the inner try-catch won't catch them), and the `logger.info` on the next line logs success before initialization completes. Add `await` before `this._startProject(config)`.</comment>
<file context>
@@ -0,0 +1,559 @@
+ const configs = await listMattermostConfigs();
+ for (const config of configs) {
+ try {
+ this._startProject(config);
+ logger.info(`Mattermost bot started for project ${config.projectId}`, { source: 'system' });
+ } catch (error) {
</file context>
| ); | ||
| }); | ||
|
|
||
| bot.onNewMessage(/.*/, async (thread, message) => { |
There was a problem hiding this comment.
P1: onNewMessage uses /.*/ (matches empty strings) and lacks a message.isMention guard. Since onNewMention and onNewMessage have identical copied logic, every @-mention fires both handlers, causing duplicate LLM streams, duplicate chat records, and duplicate responses. Follow the Slack pattern: use [\s\S]+ as the regex and add if (message.isMention) return; at the top of the handler. Also consider extracting the shared handler body to avoid code duplication.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/src/services/mattermost.ts, line 119:
<comment>`onNewMessage` uses `/.*/` (matches empty strings) and lacks a `message.isMention` guard. Since `onNewMention` and `onNewMessage` have identical copied logic, every @-mention fires both handlers, causing duplicate LLM streams, duplicate chat records, and duplicate responses. Follow the Slack pattern: use `[\s\S]+` as the regex and add `if (message.isMention) return;` at the top of the handler. Also consider extracting the shared handler body to avoid code duplication.</comment>
<file context>
@@ -0,0 +1,559 @@
+ );
+ });
+
+ bot.onNewMessage(/.*/, async (thread, message) => {
+ if (message.text.startsWith('login ')) {
+ await this._handleLoginCommand(thread, message, userByMattermostId);
</file context>
| @@ -0,0 +1,3 @@ | |||
| ALTER TABLE `chat` ADD `mattermost_thread_id` text;--> statement-breakpoint | |||
There was a problem hiding this comment.
P2: SQLite migration checks will fail because the new 0056 migration has no matching migrations-sqlite/meta/0056_snapshot.json or journal entry. Adding the generated Drizzle meta files alongside this SQL keeps CI and migration discovery in sync.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/migrations-sqlite/0056_add_mattermost_channel.sql, line 1:
<comment>SQLite migration checks will fail because the new 0056 migration has no matching `migrations-sqlite/meta/0056_snapshot.json` or journal entry. Adding the generated Drizzle meta files alongside this SQL keeps CI and migration discovery in sync.</comment>
<file context>
@@ -0,0 +1,3 @@
+ALTER TABLE `chat` ADD `mattermost_thread_id` text;--> statement-breakpoint
+CREATE INDEX `chat_mattermost_thread_idx` ON `chat` (`mattermost_thread_id`);--> statement-breakpoint
+ALTER TABLE `project` ADD `mattermost_settings` text;
</file context>
| > | ||
| <div className='flex items-center justify-between'> | ||
| <span className='text-sm font-medium text-foreground'>Mattermost</span> | ||
| <Button variant='ghost' size='icon-sm' type='button' onClick={onCancel}> |
There was a problem hiding this comment.
P2: The close/dismiss button only renders an X icon with no aria-label. Screen readers will announce it as a generic "button" with no indication of its action. Add aria-label='Close' (or aria-label='Cancel' to match onCancel) so assistive technology users can identify the control.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/frontend/src/components/settings/mattermost-form.tsx, line 33:
<comment>The close/dismiss button only renders an X icon with no `aria-label`. Screen readers will announce it as a generic "button" with no indication of its action. Add `aria-label='Close'` (or `aria-label='Cancel'` to match `onCancel`) so assistive technology users can identify the control.</comment>
<file context>
@@ -0,0 +1,86 @@
+ >
+ <div className='flex items-center justify-between'>
+ <span className='text-sm font-medium text-foreground'>Mattermost</span>
+ <Button variant='ghost' size='icon-sm' type='button' onClick={onCancel}>
+ <X className='size-4' />
+ </Button>
</file context>
| page: '/settings/project/mattermost', | ||
| pageLabel: 'Mattermost', | ||
| title: 'Linking Code', | ||
| description: 'Send /login <code> to the Mattermost bot you want to link.', |
There was a problem hiding this comment.
P2: The search index description includes a / prefix (/login) for the Mattermost linking command, but the component's actual loginHint text shows login <code> (without the slash). This inconsistency will confuse users who search for one format and see the other in the UI.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/frontend/src/components/settings-search-index.ts, line 382:
<comment>The search index description includes a `/` prefix (`/login`) for the Mattermost linking command, but the component's actual `loginHint` text shows `login <code>` (without the slash). This inconsistency will confuse users who search for one format and see the other in the UI.</comment>
<file context>
@@ -366,6 +366,23 @@ export const settingsSearchIndex: SettingsSearchEntry[] = [
+ page: '/settings/project/mattermost',
+ pageLabel: 'Mattermost',
+ title: 'Linking Code',
+ description: 'Send /login <code> to the Mattermost bot you want to link.',
+ keywords: ['link', 'login', 'mattermost'],
+ },
</file context>
| lastMessage = uiMessage; | ||
| } | ||
|
|
||
| await this._sendFinalText(ctx); |
There was a problem hiding this comment.
P2: Responses that finish after a tool call can leave the Mattermost message stuck in the live “Exploring...” state. Flushing the pending tool group before the final edit keeps the final message summarized even when no text part follows the tool result.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/src/services/mattermost.ts, line 404:
<comment>Responses that finish after a tool call can leave the Mattermost message stuck in the live “Exploring...” state. Flushing the pending tool group before the final edit keeps the final message summarized even when no text part follows the tool result.</comment>
<file context>
@@ -0,0 +1,559 @@
+ lastMessage = uiMessage;
+ }
+
+ await this._sendFinalText(ctx);
+ return { ...state, lastMessage };
+ }
</file context>
| import { MatterMostReplyMode } from '../types/messaging-provider'; | ||
| import { takeFirstOrThrow } from '../utils/queries'; | ||
|
|
||
| function toLlmSelectedModel( |
There was a problem hiding this comment.
P3: This adds another copy of the same LLM model-selection parser used by other messaging config queries. Consider moving toLlmSelectedModel to a shared utility so provider parsing behavior stays consistent across connectors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/src/queries/project-mattermost-config.queries.ts, line 11:
<comment>This adds another copy of the same LLM model-selection parser used by other messaging config queries. Consider moving `toLlmSelectedModel` to a shared utility so provider parsing behavior stays consistent across connectors.</comment>
<file context>
@@ -0,0 +1,134 @@
+import { MatterMostReplyMode } from '../types/messaging-provider';
+import { takeFirstOrThrow } from '../utils/queries';
+
+function toLlmSelectedModel(
+ provider: string | null | undefined,
+ modelId: string | null | undefined,
</file context>
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
0738307 to
1678103
Compare
2 participants
Closes: #1090
This PR adds Mattermost as a new messaging provider for nao, following the same architecture as Slack, Telegram, and WhatsApp.
What's included