Bridge Gitlab comment replies as Matrix threads (#758)

* Bridge Gitlab comment replies as Matrix threads

* Persistently store Gitlab Discussion-Thread mapping

* Remove leftover debug line

* Denoise comment descriptions when they happen in Matrix threads

* Make comment debouncing time configurable

* Add some tests for Gitlab comments

* De-only Gitlab comment tests

* Linting

* Changelog

* Map multiple Gitlab discussions to a single Matrix thread

We debounce Gitlab comments, so multiple discussions can end up in one thread.
This ensures that replies to *any* of these discussions end up in the same thread.

* Add tests for the many-to-one reply case

* Move SerializedGitlabDiscussionThreads to Types

* Update changelog.d/758.feature

Co-authored-by: Will Hunt <will@half-shot.uk>

* Fix instructions for validating your config using Docker (#794)

* Fix instructions for validating your config using Docker

Fixes GH-787

* Changelog

---------

Co-authored-by: Tadeusz Sośnierz <tadeusz@sosnierz.com>

* Add more icons to GitHub messages (#795)

* Add more icons to GitHub messages

* Add merged icon

* Lint

* Add changelog

* Bump word-wrap from 1.2.3 to 1.2.4 (#799)

* Bump word-wrap from 1.2.3 to 1.2.4

Bumps [word-wrap](https://github.com/jonschlinkert/word-wrap) from 1.2.3 to 1.2.4.
- [Release notes](https://github.com/jonschlinkert/word-wrap/releases)
- [Commits](https://github.com/jonschlinkert/word-wrap/compare/1.2.3...1.2.4)

---
updated-dependencies:
- dependency-name: word-wrap
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

* Add changelog

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Andrew Ferrazzutti <andrewf@element.io>

* Update matrix-appservice-bridge to 9.0.1 (#800)

* Bump semver from 5.7.1 to 5.7.2 (#797)

Bumps [semver](https://github.com/npm/node-semver) from 5.7.1 to 5.7.2.
- [Release notes](https://github.com/npm/node-semver/releases)
- [Changelog](https://github.com/npm/node-semver/blob/v5.7.2/CHANGELOG.md)
- [Commits](https://github.com/npm/node-semver/compare/v5.7.1...v5.7.2)

---
updated-dependencies:
- dependency-name: semver
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Will Hunt <will@half-shot.uk>

* 4.4.0

* 4.4.1

* Set the default commentDebouncMs for Gitlab in its Config

* Rename `approvalState` to something more fitting

* Update sample config

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Tadeusz Sośnierz <tadeusz@sosnierz.com>
Co-authored-by: Will Hunt <will@half-shot.uk>
Co-authored-by: Connor Davis <mail@connordav.is>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Andrew Ferrazzutti <andrewf@element.io>

changelog.d/758.feature

@@ -0,0 +1 @@
+Bridge Gitlab comment replies as Matrix threads.

config.sample.yml

@@ -49,6 +49,10 @@ gitlab:
     # (Optional) Prefix used when creating ghost users for GitLab accounts.
 
     _gitlab_
+  commentDebounceMs:
+    # (Optional) Aggregate comments by waiting this many miliseconds before posting them to Matrix. Defaults to 5000 (5 seconds)
+
+    5000
 figma:
   # (Optional) Configure this to enable Figma support
 

src/Connections/GitlabRepo.ts

@@ -12,7 +12,7 @@ import { CommandConnection } from "./CommandConnection";
 import { Connection, IConnection, IConnectionState, InstantiateConnectionOpts, ProvisionConnectionOpts } from "./IConnection";
 import { ConnectionWarning, GetConnectionsResponseItem } from "../provisioning/api";
 import { ErrCode, ApiError, ValidatorApiError } from "../api"
-import { AccessLevel } from "../Gitlab/Types";
+import { AccessLevel, SerializedGitlabDiscussionThreads } from "../Gitlab/Types";
 import Ajv, { JSONSchemaType } from "ajv";
 import { CommandError } from "../errors";
 import QuickLRU from "@alloc/quick-lru";
@@ -59,8 +59,6 @@ const log = new Logger("GitLabRepoConnection");
 const md = new markdown();
 
 const PUSH_MAX_COMMITS = 5;
-const MRRCOMMENT_DEBOUNCE_MS = 5000;
-
 
 export type GitLabRepoResponseItem = GetConnectionsResponseItem<GitLabRepoConnectionState>;
 
@@ -205,7 +203,7 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
         throw new ValidatorApiError(validator.errors);
     }
 
-    static async createConnectionForState(roomId: string, event: StateEvent<Record<string, unknown>>, {as, intent, tokenStore, config}: InstantiateConnectionOpts) {
+    static async createConnectionForState(roomId: string, event: StateEvent<Record<string, unknown>>, {as, intent, storage, tokenStore, config}: InstantiateConnectionOpts) {
         if (!config.gitlab) {
             throw Error('GitLab is not configured');
         }
@@ -214,7 +212,13 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
         if (!instance) {
             throw Error('Instance name not recognised');
         }
-        return new GitLabRepoConnection(roomId, event.stateKey, as, config.gitlab, intent, state, tokenStore, instance);
+
+        const connection = new GitLabRepoConnection(roomId, event.stateKey, as, config.gitlab, intent, state, tokenStore, instance, storage);
+
+        const discussionThreads = await storage.getGitlabDiscussionThreads(connection.connectionId);
+        connection.setDiscussionThreads(discussionThreads);
+
+        return connection;
     }
 
     public static async assertUserHasAccessToProject(
@@ -242,7 +246,12 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
         return permissionLevel;
     }
 
-    public static async provisionConnection(roomId: string, requester: string, data: Record<string, unknown>, { as, config, intent, tokenStore, getAllConnectionsOfType }: ProvisionConnectionOpts) {
+    public static async provisionConnection(
+        roomId: string,
+        requester: string,
+        data: Record<string, unknown>,
+        { as, config, intent, storage, tokenStore, getAllConnectionsOfType }: ProvisionConnectionOpts
+    ) {
         if (!config.gitlab) {
             throw Error('GitLab is not configured');
         }
@@ -260,7 +269,8 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
 
         const project = await client.projects.get(validData.path);
         const stateEventKey = `${validData.instance}/${validData.path}`;
-        const connection = new GitLabRepoConnection(roomId, stateEventKey, as, gitlabConfig, intent, validData, tokenStore, instance);
+        const connection = new GitLabRepoConnection(roomId, stateEventKey, as, gitlabConfig, intent, validData, tokenStore, instance, storage);
+
         const existingConnections = getAllConnectionsOfType(GitLabRepoConnection);
         const existing = existingConnections.find(c => c.roomId === roomId && c.instance.url === connection.instance.url && c.path === connection.path);
 
@@ -382,21 +392,19 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
     private readonly debounceMRComments = new Map<string, {
         commentCount: number,
         commentNotes?: string[],
+        discussions: string[],
         author: string,
         timeout: NodeJS.Timeout,
         approved?: boolean,
         skip?: boolean,
     }>();
 
-    /**
+    private readonly discussionThreads = new QuickLRU<string, Promise<string|undefined>>({ maxSize: 100});
-     * GitLab provides NO threading information in its webhook response objects,
+
-     * so we need to determine if we've seen a comment for a line before, and
-     * skip it if we have (because it's probably a reply).
-     */
-    private readonly mergeRequestSeenDiscussionIds = new QuickLRU<string, undefined>({ maxSize: 100 });
     private readonly hookFilter: HookFilter<AllowedEventsNames>;
 
     private readonly grantChecker;
+    private readonly commentDebounceMs: number;
 
     constructor(
         roomId: string,
@@ -407,6 +415,7 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
         state: ConnectionStateValidated,
         private readonly tokenStore: UserTokenStore,
         private readonly instance: GitLabInstance,
+        private readonly storage: IBridgeStorageProvider,
     ) {
         super(
             roomId,
@@ -427,7 +436,8 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
         this.hookFilter = new HookFilter(
             state.enableHooks ?? DefaultHooks,
         );
-}
+        this.commentDebounceMs = config.commentDebounceMs;
+    }
 
     public get path() {
         return this.state.path.toLowerCase();
@@ -723,7 +733,7 @@ ${data.description}`;
         });
     }
 
-    private renderDebouncedMergeRequest(uniqueId: string, mergeRequest: IGitlabMergeRequest, project: IGitlabProject) {
+    private async renderDebouncedMergeRequest(uniqueId: string, mergeRequest: IGitlabMergeRequest, project: IGitlabProject) {
         const result = this.debounceMRComments.get(uniqueId);
         if (!result) {
             // Always defined, but for type checking purposes.
@@ -739,26 +749,52 @@ ${data.description}`;
             comments = ` with ${result.commentCount} comments`;
         }
 
-        let approvalState = 'commented on';
+        let relation;
-        if (result.approved === true) {
+        const discussionWithThread = result.discussions.find(discussionId => this.discussionThreads.has(discussionId));
-            approvalState = '✅ approved'
+        if (discussionWithThread) {
-        } else if (result.approved === false) {
+            const threadEventId = await this.discussionThreads.get(discussionWithThread)!.catch(_ => { /* already logged */ });
-            approvalState = '🔴 unapproved';
+            if (threadEventId) {
+                relation = {
+                    "m.relates_to": {
+                        "event_id": threadEventId,
+                        "rel_type": "m.thread"
+                    },
+                };
+            }
         }
 
-        let content = `**${result.author}** ${approvalState} MR [${orgRepoName}#${mergeRequest.iid}](${mergeRequest.url}): "${mergeRequest.title}"${comments}`;
+        let action = relation ? 'replied' : 'commented on'; // this is the only place we need this, approve/unapprove don't appear in discussions
+        if (result.approved === true) {
+            action = '✅ approved'
+        } else if (result.approved === false) {
+            action = '🔴 unapproved';
+        }
+
+        const target = relation ? '' : ` MR [${orgRepoName}#${mergeRequest.iid}](${mergeRequest.url}): "${mergeRequest.title}"`;
+        let content = `**${result.author}** ${action}${target} ${comments}`;
 
         if (result.commentNotes) {
             content += "\n\n> " + result.commentNotes.join("\n\n> ");
         }
 
-        this.intent.sendEvent(this.roomId, {
+        const eventPromise = this.intent.sendEvent(this.roomId, {
             msgtype: "m.notice",
             body: content,
             formatted_body: md.renderInline(content),
             format: "org.matrix.custom.html",
+            ...relation,
         }).catch(ex  => {
             log.error('Failed to send MR review message', ex);
+            return undefined;
+        });
+
+        for (const discussionId of result.discussions) {
+            if (!this.discussionThreads.has(discussionId)) {
+                this.discussionThreads.set(discussionId, eventPromise);
+            }
+        }
+        void this.persistDiscussionThreads().catch(ex => {
+            log.error(`Failed to persistently store Gitlab discussion threads for connection ${this.connectionId}:`, ex);
         });
     }
 
@@ -770,6 +806,7 @@ ${data.description}`;
             commentCount: number,
             commentNotes?: string[],
             approved?: boolean,
+            discussionId?: string,
             /**
              * If the MR contains only comments, skip it.
              */
@@ -789,16 +826,20 @@ ${data.description}`;
             if (!opts.skip) {
                 existing.skip = false;
             }
-            existing.timeout = setTimeout(() => this.renderDebouncedMergeRequest(uniqueId, mergeRequest, project), MRRCOMMENT_DEBOUNCE_MS);
+            if (opts.discussionId) {
+                existing.discussions.push(opts.discussionId);
+            }
+            existing.timeout = setTimeout(() => this.renderDebouncedMergeRequest(uniqueId, mergeRequest, project), this.commentDebounceMs);
             return;
         }
         this.debounceMRComments.set(uniqueId, {
             commentCount: commentCount,
             commentNotes: commentNotes,
+            discussions: opts.discussionId ? [opts.discussionId] : [],
             skip: opts.skip,
             approved,
             author: user.name,
-            timeout: setTimeout(() => this.renderDebouncedMergeRequest(uniqueId, mergeRequest, project), MRRCOMMENT_DEBOUNCE_MS),
+            timeout: setTimeout(() => this.renderDebouncedMergeRequest(uniqueId, mergeRequest, project), this.commentDebounceMs),
         });
     }
 
@@ -840,19 +881,6 @@ ${data.description}`;
         );
     }
 
-    private shouldHandleMRComment(event: IGitLabWebhookNoteEvent) {
-        // Check to see if this line has had a comment before
-        if (event.object_attributes.discussion_id) {
-            if (this.mergeRequestSeenDiscussionIds.has(event.object_attributes.discussion_id)) {
-                // If it has, this is probably a reply. Skip repeated replies.
-                return false;
-            }
-            // Otherwise, record that we have seen the line and continue (it's probably a genuine comment).
-            this.mergeRequestSeenDiscussionIds.set(event.object_attributes.discussion_id, undefined);
-        }
-        return true;
-    }
-
     public async onCommentCreated(event: IGitLabWebhookNoteEvent) {
         if (this.hookFilter.shouldSkip('merge_request', 'merge_request.review')) {
             return;
@@ -862,13 +890,11 @@ ${data.description}`;
             // Not a MR comment
             return;
         }
-        if (!this.shouldHandleMRComment(event)) {
+
-            // Skip it.
-            return;
-        }
         this.debounceMergeRequestReview(event.user, event.merge_request, event.project, {
             commentCount: 1,
             commentNotes: this.state.includeCommentBody ? [event.object_attributes.note] : undefined,
+            discussionId: event.object_attributes.discussion_id,
             skip: this.hookFilter.shouldSkip('merge_request.review.comments'),
         });
     }
@@ -912,6 +938,24 @@ ${data.description}`;
         }
         // TODO: Clean up webhooks
     }
+
+    private setDiscussionThreads(discussionThreads: SerializedGitlabDiscussionThreads): void {
+        for (const { discussionId, eventId } of discussionThreads) {
+            this.discussionThreads.set(discussionId, Promise.resolve(eventId));
+        }
+    }
+
+    private async persistDiscussionThreads(): Promise<void> {
+        const serialized: SerializedGitlabDiscussionThreads = [];
+        for (const [discussionId, eventIdPromise] of this.discussionThreads.entriesAscending()) {
+            const eventId = await eventIdPromise.catch(_ => { /* logged elsewhere */ });
+            if (eventId) {
+                serialized.push({ discussionId, eventId });
+            }
+
+        }
+        return this.storage.setGitlabDiscussionThreads(this.connectionId, serialized);
+    }
 }
 
 // Typescript doesn't understand Prototypes very well yet.

src/Gitlab/Types.ts

@@ -220,6 +220,9 @@ export interface ProjectHook extends ProjectHookOpts {
     created_at?: string;
 }
 
+/** newest last, to enable feeding it straight into an LRU cache */
+export type SerializedGitlabDiscussionThreads = { discussionId: string, eventId: string }[];
+
 export interface SimpleProject {
     avatar_url?: string;
     description?: string;

src/Stores/MemoryStorageProvider.ts

@@ -3,6 +3,7 @@ import { IBridgeStorageProvider, MAX_FEED_ITEMS } from "./StorageProvider";
 import { IssuesGetResponseData } from "../github/Types";
 import { ProvisionSession } from "matrix-appservice-bridge";
 import QuickLRU from "@alloc/quick-lru";
+import { SerializedGitlabDiscussionThreads } from "../Gitlab/Types";
 
 export class MemoryStorageProvider extends MSP implements IBridgeStorageProvider {
     private issues: Map<string, IssuesGetResponseData> = new Map();
@@ -11,6 +12,7 @@ export class MemoryStorageProvider extends MSP implements IBridgeStorageProvider
     private figmaCommentIds: Map<string, string> = new Map();
     private widgetSessions: Map<string, ProvisionSession> = new Map();
     private storedFiles = new QuickLRU<string, string>({ maxSize: 128 });
+    private gitlabDiscussionThreads = new Map<string, SerializedGitlabDiscussionThreads>();
     private feedGuids = new Map<string, Array<string>>();
 
     constructor() {
@@ -97,4 +99,12 @@ export class MemoryStorageProvider extends MSP implements IBridgeStorageProvider
     public async setStoredTempFile(key: string, value: string) {
         this.storedFiles.set(key, value);
     }
+
+    public async getGitlabDiscussionThreads(connectionId: string): Promise<SerializedGitlabDiscussionThreads> {
+        return this.gitlabDiscussionThreads.get(connectionId) ?? [];
+    }
+
+    public async setGitlabDiscussionThreads(connectionId: string, value: SerializedGitlabDiscussionThreads): Promise<void> {
+        this.gitlabDiscussionThreads.set(connectionId, value);
+    }
 }

src/Stores/RedisStorageProvider.ts

@@ -5,6 +5,7 @@ import { Logger } from "matrix-appservice-bridge";
 import { IBridgeStorageProvider, MAX_FEED_ITEMS } from "./StorageProvider";
 import { IFilterInfo, IStorageProvider } from "matrix-bot-sdk";
 import { ProvisionSession } from "matrix-appservice-bridge";
+import { SerializedGitlabDiscussionThreads } from "../Gitlab/Types";
 
 const BOT_SYNC_TOKEN_KEY = "bot.sync_token.";
 const BOT_FILTER_KEY = "bot.filter.";
@@ -16,6 +17,7 @@ const GH_ISSUES_LAST_COMMENT_KEY = "gh.issues.last_comment";
 const GH_ISSUES_REVIEW_DATA_KEY = "gh.issues.review_data";
 const FIGMA_EVENT_COMMENT_ID = "figma.comment_event_id";
 const STORED_FILES_KEY = "storedfiles.";
+const GL_DISCUSSIONTHREADS_KEY = "gl.discussion-threads";
 const STORED_FILES_EXPIRE_AFTER = 24 * 60 * 60; // 24 hours
 const COMPLETED_TRANSACTIONS_EXPIRE_AFTER = 24 * 60 * 60; // 24 hours
 const ISSUES_EXPIRE_AFTER = 7 * 24 * 60 * 60; // 7 days
@@ -204,6 +206,16 @@ export class RedisStorageProvider extends RedisStorageContextualProvider impleme
         await this.redis.set(STORED_FILES_KEY + key, value);
     }
 
+    public async getGitlabDiscussionThreads(connectionId: string): Promise<SerializedGitlabDiscussionThreads> {
+        const key = `${GL_DISCUSSIONTHREADS_KEY}:${connectionId}`;
+        return JSON.parse(await this.redis.get(key) ?? '[]');
+    }
+
+    public async setGitlabDiscussionThreads(connectionId: string, value: SerializedGitlabDiscussionThreads): Promise<void> {
+        const key = `${GL_DISCUSSIONTHREADS_KEY}:${connectionId}`;
+        await this.redis.set(key, JSON.stringify(value));
+    }
+
     public async storeFeedGuids(url: string, ...guid: string[]): Promise<void> {
         const feedKey = `${FEED_GUIDS}${url}`;
         await this.redis.lpush(feedKey, ...guid);

src/Stores/StorageProvider.ts

@@ -1,6 +1,7 @@
 import { ProvisioningStore } from "matrix-appservice-bridge";
 import { IAppserviceStorageProvider, IStorageProvider } from "matrix-bot-sdk";
 import { IssuesGetResponseData } from "../github/Types";
+import { SerializedGitlabDiscussionThreads } from "../Gitlab/Types";
 
 // Some RSS feeds can return a very small number of items then bounce
 // back to their "normal" size, so we cannot just clobber the recent GUID list per request or else we'll
@@ -22,6 +23,8 @@ export interface IBridgeStorageProvider extends IAppserviceStorageProvider, ISto
     getFigmaCommentEventId(roomId: string, figmaCommentId: string): Promise<string|null>;
     getStoredTempFile(key: string): Promise<string|null>;
     setStoredTempFile(key: string, value: string): Promise<void>;
+    getGitlabDiscussionThreads(connectionId: string): Promise<SerializedGitlabDiscussionThreads>;
+    setGitlabDiscussionThreads(connectionId: string, value: SerializedGitlabDiscussionThreads): Promise<void>;
     storeFeedGuids(url: string, ...guid: string[]): Promise<void>;
     hasSeenFeed(url: string, ...guid: string[]): Promise<boolean>;
     hasSeenFeedGuid(url: string, guid: string): Promise<boolean>;

src/config/Config.ts

@@ -193,6 +193,7 @@ export interface BridgeConfigGitLabYAML {
     },
     instances: {[name: string]: GitLabInstance};
     userIdPrefix: string;
+    commentDebounceMs?: number;
 }
 
 export class BridgeConfigGitLab {
@@ -205,6 +206,9 @@ export class BridgeConfigGitLab {
     @configKey("Prefix used when creating ghost users for GitLab accounts.", true)
     readonly userIdPrefix: string;
 
+    @configKey("Aggregate comments by waiting this many miliseconds before posting them to Matrix. Defaults to 5000 (5 seconds)", true)
+    readonly commentDebounceMs: number;
+
     constructor(yaml: BridgeConfigGitLabYAML) {
         this.instances = yaml.instances;
         this.webhook = yaml.webhook;
@@ -216,6 +220,12 @@ export class BridgeConfigGitLab {
                 this.instances[name].url = url.slice(0, -1);
             }
         }
+
+        if (yaml.commentDebounceMs === undefined) {
+            this.commentDebounceMs = 5000;
+        } else {
+            this.commentDebounceMs = yaml.commentDebounceMs;
+        }
     }
 
     @hideKey()

tests/connections/GitlabRepoTest.ts

@@ -5,6 +5,8 @@ import { ApiError, ErrCode, ValidatorApiError } from "../../src/api";
 import { GitLabRepoConnection, GitLabRepoConnectionState } from "../../src/Connections";
 import { expect } from "chai";
 import { BridgeConfigGitLab } from "../../src/config/Config";
+import { IBridgeStorageProvider } from "../../src/Stores/StorageProvider";
+import { IntentMock } from "../utils/IntentMock";
 
 const ROOM_ID = "!foo:bar";
 
@@ -20,20 +22,40 @@ const GITLAB_MR = {
 	title: "My MR",
 };
 
-const GITLAB_ISSUE_CREATED_PAYLOAD = {
+const GITLAB_USER = {
-    object_kind: "merge_request",
+	name: "Alice",
-	user: {
+	username: "alice",
-		name: "Alice",
-		username: "alice",
-	},
-	object_attributes: GITLAB_MR,
-	project: {
-		path_with_namespace: `${GITLAB_ORG_REPO.org}/${GITLAB_ORG_REPO.repo}`,
-		web_url: `https://gitlab.example.com/${GITLAB_ORG_REPO.org}/${GITLAB_ORG_REPO.repo}`,
-	}
 };
 
-function createConnection(state: Record<string, unknown> = {}, isExistingState=false) {
+const GITLAB_PROJECT = {
+	path_with_namespace: `${GITLAB_ORG_REPO.org}/${GITLAB_ORG_REPO.repo}`,
+	web_url: `https://gitlab.example.com/${GITLAB_ORG_REPO.org}/${GITLAB_ORG_REPO.repo}`,
+};
+
+const GITLAB_ISSUE_CREATED_PAYLOAD = {
+    object_kind: "merge_request",
+	user: GITLAB_USER,
+	object_attributes: GITLAB_MR,
+	project: GITLAB_PROJECT,
+};
+
+const GITLAB_MR_COMMENT = {
+	'object_kind': 'note',
+	'event_type': 'note',
+	'merge_request': GITLAB_MR,
+	'object_attributes': {
+        'discussion_id': '6babfc4ad3be2355db286ed50be111a5220d5751',
+        'note': 'I am starting a new thread',
+        'noteable_type': 'MergeRequest',
+        'url': 'https://gitlab.com/tadeuszs/my-awesome-project/-/merge_requests/2#note_1455087141'
+	},
+	'project': GITLAB_PROJECT,
+	'user': GITLAB_USER,
+};
+
+const COMMENT_DEBOUNCE_MS = 25;
+
+function createConnection(state: Record<string, unknown> = {}, isExistingState=false): { connection: GitLabRepoConnection, intent: IntentMock } {
 	const mq = createMessageQueue({
 		monolithic: true
 	});
@@ -44,7 +66,9 @@ function createConnection(state: Record<string, unknown> = {}, isExistingState=f
 		ROOM_ID,
 		"state_key",
 		as,
-		{} as BridgeConfigGitLab,
+		{
+			commentDebounceMs: COMMENT_DEBOUNCE_MS,
+		} as BridgeConfigGitLab,
 		intent,
 		GitLabRepoConnection.validateState({
 			instance: "bar",
@@ -55,10 +79,18 @@ function createConnection(state: Record<string, unknown> = {}, isExistingState=f
 		{
 			url: "https://gitlab.example.com"
 		},
+		{
+			setGitlabDiscussionThreads: () => Promise.resolve(),
+			getGitlabDiscussionThreads: () => Promise.resolve([]),
+		} as unknown as IBridgeStorageProvider,
 	);
 	return {connection, intent};
 }
 
+async function waitForDebouncing(): Promise<void> {
+	return new Promise(resolve => setTimeout(resolve, COMMENT_DEBOUNCE_MS * 2));
+}
+
 describe("GitLabRepoConnection", () => {
 	describe("validateState", () => {
 		it("can validate a completes state config", () => {
@@ -128,6 +160,98 @@ describe("GitLabRepoConnection", () => {
 			}, true);
 		});
 	});
+	describe("onCommentCreated", () => {
+		it("will handle an MR comment", async () => {
+			const { connection, intent } = createConnection();
+			await connection.onCommentCreated(GITLAB_MR_COMMENT as never);
+			await waitForDebouncing();
+			intent.expectEventMatches(
+				(ev: any) => ev.content.body.includes('**Alice** commented on MR'),
+				'event body indicates MR comment'
+			);
+		});
+		it("will debounce MR comments", async () => {
+			const { connection, intent } = createConnection();
+			await connection.onCommentCreated(GITLAB_MR_COMMENT as never);
+			await connection.onCommentCreated({
+				...GITLAB_MR_COMMENT,
+				'object_attributes': {
+					...GITLAB_MR_COMMENT.object_attributes,
+					'discussion_id': 'fa5d',
+					'note': 'different comment',
+				},
+			} as never);
+			await waitForDebouncing();
+			expect(intent.sentEvents.length).to.equal(1);
+			intent.expectEventMatches(
+				(ev: any) => ev.content.body.includes('with 2 comments'),
+				'one event sent for both comments',
+				0,
+			);
+		});
+		it("will add new comments in a Matrix thread", async () => {
+			const { connection, intent } = createConnection();
+			await connection.onCommentCreated(GITLAB_MR_COMMENT as never);
+			await waitForDebouncing();
+			await connection.onCommentCreated(GITLAB_MR_COMMENT as never);
+			await waitForDebouncing();
+			expect(intent.sentEvents.length).to.equal(2);
+			intent.expectEventMatches(
+				(ev: any) => ev.content['m.relates_to'].event_id === 'event_0',
+				'one event sent for both comments',
+				1,
+			);
+		});
+		it("will correctly map new comments to aggregated discussions", async () => {
+			const { connection, intent } = createConnection();
+			await connection.onCommentCreated({
+				...GITLAB_MR_COMMENT,
+				'object_attributes': {
+					...GITLAB_MR_COMMENT.object_attributes,
+					'discussion_id': 'disc1',
+				},
+			} as never);
+			await connection.onCommentCreated({
+				...GITLAB_MR_COMMENT,
+				'object_attributes': {
+					...GITLAB_MR_COMMENT.object_attributes,
+					'discussion_id': 'disc2',
+				},
+			} as never);
+			await waitForDebouncing();
+			expect(intent.sentEvents.length).to.equal(1);
+
+			await connection.onCommentCreated({
+				...GITLAB_MR_COMMENT,
+				'object_attributes': {
+					...GITLAB_MR_COMMENT.object_attributes,
+					'discussion_id': 'disc1',
+				},
+			} as never);
+			await waitForDebouncing();
+			expect(intent.sentEvents.length).to.equal(2);
+			intent.expectEventMatches(
+				(ev: any) => ev.content['m.relates_to'].event_id === 'event_0',
+				'disc1 reply goes to existing thread',
+				1
+			);
+
+			await connection.onCommentCreated({
+				...GITLAB_MR_COMMENT,
+				'object_attributes': {
+					...GITLAB_MR_COMMENT.object_attributes,
+					'discussion_id': 'disc2',
+				},
+			} as never);
+			await waitForDebouncing();
+			expect(intent.sentEvents.length).to.equal(3);
+			intent.expectEventMatches(
+				(ev: any) => ev.content['m.relates_to'].event_id === 'event_0',
+				'disc2 reply also goes to existing thread',
+				2
+			);
+		});
+	});
 	describe("onIssueCreated", () => {
 		it("will handle a simple issue", async () => {
 			const { connection, intent } = createConnection();

tests/utils/IntentMock.ts

@@ -68,11 +68,12 @@ export class IntentMock {
         });
     }
 
-    sendEvent(roomId: string, content: any) {
+    sendEvent(roomId: string, content: any): Promise<string> {
         this.sentEvents.push({
             roomId,
             content,
         });
+        return Promise.resolve(`event_${this.sentEvents.length - 1}`);
     }
 
     expectNoEvent() {
@@ -87,10 +88,20 @@ export class IntentMock {
                 body.includes(matcher),
                 `Expected event body ${eventIndex} to match '${matcher}'.\nMessage was: '${body}'`
             ).to.be.true;
+            return;
         }
         expect(!!this.sentEvents.find(ev => ev.content.body.includes(matcher)), `Expected any event body to match '${matcher}'`).to.be.true;
     }
 
+    expectEventMatches(matcher: (content: any) => boolean, description: string, eventIndex?: number) {
+        if (eventIndex !== undefined) {
+            expect(this.sentEvents[eventIndex], `Expected event ${eventIndex} to exist`).to.not.be.undefined;
+            expect(matcher(this.sentEvents[eventIndex]), description).to.be.true;
+            return;
+        }
+        expect(this.sentEvents.some(ev => matcher(ev)), description).to.be.true;
+    }
+
     async ensureJoined() {
         return true;
     }