From 313efb29d81c7817e028d9b3c2388f2aadfca835 Mon Sep 17 00:00:00 2001 From: Ellie Algase Date: Mon, 1 Jun 2026 04:54:38 +0000 Subject: [PATCH 1/2] Fix m->d reaction deletion counting (#85) Fixes a bug where, if multiple Matrix users had used the same reaction on a message, and then one of those Matrix users removed their reactions, the bot would forcibly remove all of that reactions. Now, we check and make sure there are no remaining reactions from Matrix before removal. This also rewrote the retrigger system to be more generic and to use promises instead of re-entry (would lose call stack). Co-authored-by: Cadence Ember Reviewed-on: https://gitdab.com/cadence/out-of-your-element/pulls/85 --- src/d2m/actions/retrigger.js | 175 ++++++++++++++++++-------- src/d2m/converters/remove-reaction.js | 2 +- src/d2m/event-dispatcher.js | 23 +++- src/db/orm.js | 10 ++ src/db/orm.test.js | 5 + src/m2d/actions/add-reaction.js | 4 +- src/m2d/actions/redact.js | 47 +++++-- src/m2d/event-dispatcher.js | 4 +- src/stdin.js | 3 +- 9 files changed, 196 insertions(+), 77 deletions(-) diff --git a/src/d2m/actions/retrigger.js b/src/d2m/actions/retrigger.js index 66ef19e..43f400d 100644 --- a/src/d2m/actions/retrigger.js +++ b/src/d2m/actions/retrigger.js @@ -2,7 +2,15 @@ const {EventEmitter} = require("events") const passthrough = require("../../passthrough") -const {select} = passthrough +const {select, sync, from} = passthrough +/** @type {import("../../matrix/utils")} */ +const utils = sync.require("../../matrix/utils") + +/* + Due to Eventual Consistency(TM) an update/delete may arrive before the original message arrives + (or before the it has finished being bridged to an event). + In this case, wait until the original message has finished bridging, then retrigger the passed function. +*/ const DEBUG_RETRIGGER = false @@ -12,81 +20,140 @@ function debugRetrigger(message) { } } -const paused = new Set() -const emitter = new EventEmitter() +const storage = new class { + /** @private @type {Set} */ + paused = new Set() + /** @private @type {Map any)[]>} id -> list of resolvers */ + resolves = new Map() + /** @private @type {Map>} id -> timer */ + timers = new Map() -/** - * Due to Eventual Consistency(TM) an update/delete may arrive before the original message arrives - * (or before the it has finished being bridged to an event). - * In this case, wait until the original message has finished bridging, then retrigger the passed function. - * @template {(...args: any[]) => any} T - * @param {string} inputID - * @param {T} fn - * @param {Parameters} rest - * @returns {boolean} false if the event was found and the function will be ignored, true if the event was not found and the function will be retriggered - */ -function eventNotFoundThenRetrigger(inputID, fn, ...rest) { - if (!paused.has(inputID)) { - if (inputID.match(/^[0-9]+$/)) { - const eventID = select("event_message", "event_id", {message_id: inputID}).pluck().get() - if (eventID) { - debugRetrigger(`[retrigger] OK mid <-> eid = ${inputID} <-> ${eventID}`) - return false // event was found so don't retrigger - } - } else if (inputID.match(/^\$/)) { - const messageID = select("event_message", "message_id", {event_id: inputID}).pluck().get() - if (messageID) { - debugRetrigger(`[retrigger] OK eid <-> mid = ${inputID} <-> ${messageID}`) - return false // message was found so don't retrigger - } + /** + * The purpose of storage is to store `resolve` and call it at a later time. + * @param {string} id + * @param {(found: Boolean) => any} resolve + */ + store(id, resolve) { + debugRetrigger(`[retrigger] STORE id = ${id}`) + this.resolves.set(id, (this.resolves.get(id) || []).concat(resolve)) // add to list in map value + if (!this.timers.has(id)) { + debugRetrigger(`[retrigger] SET TIMER id = ${id}`) + this.timers.set(id, setTimeout(() => this.resolve(id, false), 60 * 1000).unref()) // 1 minute } } + + /** @param {string} id */ + isNotPaused(id) { + return !storage.paused.has(id) + } - debugRetrigger(`[retrigger] WAIT id = ${inputID}`) - emitter.once(inputID, () => { - debugRetrigger(`[retrigger] TRIGGER id = ${inputID}`) - fn(...rest) - }) - // if the event never arrives, don't trigger the callback, just clean up - setTimeout(() => { - if (emitter.listeners(inputID).length) { - debugRetrigger(`[retrigger] EXPIRE id = ${inputID}`) + /** @param {string} id */ + pause(id) { + debugRetrigger(`[retrigger] PAUSE id = ${id}`) + this.paused.add(id) + } + + /** + * Go through `resolves` storage and resolve them all. (Also resets timer/paused.) + * @param {string} id + * @param {boolean} value + */ + resolve(id, value) { + if (this.paused.has(id)) { + debugRetrigger(`[retrigger] RESUME id = ${id}`) + this.paused.delete(id) } - emitter.removeAllListeners(inputID) - }, 60 * 1000) // 1 minute - return true // event was not found, then retrigger + + if (this.resolves.has(id)) { + debugRetrigger(`[retrigger] RESOLVE ${value} id = ${id}`) + const fns = this.resolves.get(id) || [] + this.resolves.delete(id) + for (const fn of fns) { + fn(value) + } + } + + if (this.timers.has(id)) { + clearTimeout(this.timers.get(id)) + this.timers.delete(id) + } + } +} + +/** + * @param {string} id + * @param {(found: Boolean) => any} resolve + * @param {boolean} existsInDatabase + */ +function waitFor(id, resolve, existsInDatabase) { + if (existsInDatabase && storage.isNotPaused(id)) { // if event already exists and isn't paused then resolve immediately + debugRetrigger(`[retrigger] EXISTS id = ${id}`) + return resolve(true) + } + + // doesn't exist. wait for it to exist. storage will resolve true if it exists or false if it timed out + return storage.store(id, resolve) +} + +const GET_EVENT_PREPARED = from("event_message").select("event_id").and("WHERE event_id = ?").prepare().raw() +/** + * @param {string} eventID + * @returns {Promise} if true then the message did not arrive + */ +function waitForEvent(eventID) { + const {promise, resolve} = Promise.withResolvers() + waitFor(eventID, resolve, !!GET_EVENT_PREPARED.get(eventID)) + return promise +} + +const GET_MESSAGE_PREPARED = from("event_message").select("message_id").and("WHERE message_id = ?").prepare().raw() +/** + * @param {string} messageID + * @returns {Promise} if true then the message did not arrive + */ +function waitForMessage(messageID) { + const {promise, resolve} = Promise.withResolvers() + waitFor(messageID, resolve, !!GET_MESSAGE_PREPARED.get(messageID)) + return promise +} + +const GET_REACTION_EVENT_PREPARED = from("reaction").select("hashed_event_id").and("WHERE hashed_event_id = ?").prepare().raw() +/** + * @param {string} eventID + * @returns {Promise} if true then the message did not arrive + */ +function waitForReactionEvent(eventID) { + const {promise, resolve} = Promise.withResolvers() + waitFor(eventID, resolve, !!GET_REACTION_EVENT_PREPARED.get(utils.getEventIDHash(eventID))) + return promise } /** * Anything calling retrigger during the callback will be paused and retriggered after the callback resolves. * @template T - * @param {string} messageID + * @param {string} id * @param {Promise} promise * @returns {Promise} */ -async function pauseChanges(messageID, promise) { +async function pauseChanges(id, promise) { try { - debugRetrigger(`[retrigger] PAUSE id = ${messageID}`) - paused.add(messageID) + storage.pause(id) return await promise } finally { - debugRetrigger(`[retrigger] RESUME id = ${messageID}`) - paused.delete(messageID) - messageFinishedBridging(messageID) + finishedBridging(id) } } /** * Triggers any pending operations that were waiting on the corresponding event ID. - * @param {string} messageID + * @param {string} id */ -function messageFinishedBridging(messageID) { - if (emitter.listeners(messageID).length) { - debugRetrigger(`[retrigger] EMIT id = ${messageID}`) - } - emitter.emit(messageID) +function finishedBridging(id) { + storage.resolve(id, true) } -module.exports.eventNotFoundThenRetrigger = eventNotFoundThenRetrigger -module.exports.messageFinishedBridging = messageFinishedBridging +module.exports.waitForMessage = waitForMessage +module.exports.waitForEvent = waitForEvent +module.exports.waitForReactionEvent = waitForReactionEvent module.exports.pauseChanges = pauseChanges +module.exports.finishedBridging = finishedBridging \ No newline at end of file diff --git a/src/d2m/converters/remove-reaction.js b/src/d2m/converters/remove-reaction.js index 4ca22b6..b6b0407 100644 --- a/src/d2m/converters/remove-reaction.js +++ b/src/d2m/converters/remove-reaction.js @@ -34,7 +34,7 @@ function removeReaction(data, reactions, key) { // Even though the bridge bot only reacted once on Discord-side, multiple Matrix users may have // reacted on Matrix-side. Semantically, we want to remove the reaction from EVERY Matrix user. // Also need to clean up the database. - const hash = utils.getEventIDHash(event.event_id) + const hash = utils.getEventIDHash(eventID) removals.push({eventID, mxid: null, hash}) } if (!lookingAtMatrixReaction && !wantToRemoveMatrixReaction) { diff --git a/src/d2m/event-dispatcher.js b/src/d2m/event-dispatcher.js index 90824ac..8101a03 100644 --- a/src/d2m/event-dispatcher.js +++ b/src/d2m/event-dispatcher.js @@ -2,6 +2,7 @@ const assert = require("assert").strict const DiscordTypes = require("discord-api-types/v10") +const {id: botID} = require("../../addbot") const {sync, db, select, from} = require("../passthrough") /** @type {import("./actions/send-message")}) */ @@ -38,6 +39,8 @@ const removeMember = sync.require("./actions/remove-member") const vote = sync.require("./actions/poll-vote") /** @type {import("../m2d/event-dispatcher")} */ const matrixEventDispatcher = sync.require("../m2d/event-dispatcher") +/** @type {import("../m2d/actions/redact.js")} */ +const redact = sync.require("../m2d/actions/redact.js") /** @type {import("../discord/interactions/matrix-info")} */ const matrixInfoInteraction = sync.require("../discord/interactions/matrix-info") @@ -316,7 +319,7 @@ module.exports = { // @ts-ignore await sendMessage.sendMessage(message, channel, guild, row) - retrigger.messageFinishedBridging(message.id) + retrigger.finishedBridging(message.id) }, /** @@ -337,7 +340,7 @@ module.exports = { if (!row) { // Check that the sending-to room exists, and deal with Eventual Consistency(TM) - if (retrigger.eventNotFoundThenRetrigger(data.id, module.exports.MESSAGE_UPDATE, client, data)) return + if (!await retrigger.waitForMessage(data.id)) return } /** @type {DiscordTypes.GatewayMessageCreateDispatchData} */ @@ -375,6 +378,16 @@ module.exports = { * @param {DiscordTypes.GatewayMessageReactionRemoveDispatchData | DiscordTypes.GatewayMessageReactionRemoveEmojiDispatchData | DiscordTypes.GatewayMessageReactionRemoveAllDispatchData} data */ async onSomeReactionsRemoved(client, data) { + // Don't attempt to double-bridge our own m2d deleted reactions back to Matrix + if ("user_id" in data && data.user_id === botID) { + const emojiIdOrName = data.emoji.id || data.emoji.name + const i = redact.m2dDeletedReactions.findIndex(x => data.message_id === x.messageID && emojiIdOrName === x.emojiIdOrName) + if (i !== -1) { + redact.m2dDeletedReactions.splice(i, 1) + return + } + } + await removeReaction.removeSomeReactions(data) }, @@ -384,7 +397,7 @@ module.exports = { */ async MESSAGE_DELETE(client, data) { speedbump.onMessageDelete(data.id) - if (retrigger.eventNotFoundThenRetrigger(data.id, module.exports.MESSAGE_DELETE, client, data)) return + if (!await retrigger.waitForMessage(data.id)) return await deleteMessage.deleteMessage(data) }, @@ -432,12 +445,12 @@ module.exports = { * @param {DiscordTypes.GatewayMessagePollVoteDispatchData} data */ async MESSAGE_POLL_VOTE_ADD(client, data) { - if (retrigger.eventNotFoundThenRetrigger(data.message_id, module.exports.MESSAGE_POLL_VOTE_ADD, client, data)) return + if (!await retrigger.waitForMessage(data.message_id)) return await vote.addVote(data) }, async MESSAGE_POLL_VOTE_REMOVE(client, data) { - if (retrigger.eventNotFoundThenRetrigger(data.message_id, module.exports.MESSAGE_POLL_VOTE_REMOVE, client, data)) return + if (!await retrigger.waitForMessage(data.message_id)) return await vote.removeVote(data) }, diff --git a/src/db/orm.js b/src/db/orm.js index 4d9b6f1..8763314 100644 --- a/src/db/orm.js +++ b/src/db/orm.js @@ -104,6 +104,16 @@ class From { return r } + pluckUnsafe(col) { + /** @type {Pluck} */ + // @ts-ignore + const r = this + r.cols = [col] + r.makeColsSafe = false + r.isPluck = true + return r + } + /** * @param {string} sql */ diff --git a/src/db/orm.test.js b/src/db/orm.test.js index 6f6018e..4639090 100644 --- a/src/db/orm.test.js +++ b/src/db/orm.test.js @@ -68,3 +68,8 @@ test("orm: select unsafe works (to select complex column names that can't be typ .all() t.equal(results[0].power_level, 150) }) + +test("orm: pluck unsafe works (to select complex column names that can't be type verified)", t => { + const result = from("channel_room").where({guild_id: "112760669178241024"}).pluckUnsafe("count(*)").get() + t.equal(result, 7) +}) diff --git a/src/m2d/actions/add-reaction.js b/src/m2d/actions/add-reaction.js index e4981fb..c453244 100644 --- a/src/m2d/actions/add-reaction.js +++ b/src/m2d/actions/add-reaction.js @@ -17,7 +17,7 @@ const retrigger = sync.require("../../d2m/actions/retrigger") */ async function addReaction(event) { // Wait until the corresponding channel and message have already been bridged - if (retrigger.eventNotFoundThenRetrigger(event.content["m.relates_to"].event_id, () => as.emit("type:m.reaction", event))) return + if (!await retrigger.waitForEvent(event.content["m.relates_to"].event_id)) return // These will exist because it passed retrigger const row = from("event_message").join("message_room", "message_id").join("historical_channel_room", "historical_room_index") @@ -50,6 +50,8 @@ async function addReaction(event) { } db.prepare("REPLACE INTO reaction (hashed_event_id, message_id, encoded_emoji, original_encoding) VALUES (?, ?, ?, ?)").run(utils.getEventIDHash(event.event_id), messageID, discordPreferredEncoding, key) + + retrigger.finishedBridging(event.event_id) } module.exports.addReaction = addReaction diff --git a/src/m2d/actions/redact.js b/src/m2d/actions/redact.js index 3135d31..7e49753 100644 --- a/src/m2d/actions/redact.js +++ b/src/m2d/actions/redact.js @@ -10,6 +10,9 @@ const utils = sync.require("../../matrix/utils") /** @type {import("../../d2m/actions/retrigger")} */ const retrigger = sync.require("../../d2m/actions/retrigger") +/** @type {{messageID: string, emojiIdOrName: string}[]} */ +const m2dDeletedReactions = [] + /** * @param {Ty.Event.Outer_M_Room_Redaction} event */ @@ -24,6 +27,21 @@ async function deleteMessage(event) { db.prepare("DELETE FROM message_room WHERE message_id = ?").run(rows[0].message_id) } +/** + * @param {Ty.Event.Outer_M_Room_Redaction} event + */ +async function removeMessageEvent(event) { + // Could be for removing a message or suppressing embeds. For more information, the message needs to be bridged first. + if (!await retrigger.waitForEvent(event.redacts)) return + + const row = select("event_message", ["event_type", "event_subtype", "part"], {event_id: event.redacts}).get() + if (row && row.event_type === "m.room.message" && row.event_subtype === "m.notice" && row.part === 1) { + await suppressEmbeds(event) + } else { + await deleteMessage(event) + } +} + /** * @param {Ty.Event.Outer_M_Room_Redaction} event */ @@ -41,11 +59,20 @@ async function suppressEmbeds(event) { * @param {Ty.Event.Outer_M_Room_Redaction} event */ async function removeReaction(event) { + if (!await retrigger.waitForReactionEvent(event.redacts)) return + const hash = utils.getEventIDHash(event.redacts) const row = from("reaction").join("message_room", "message_id").join("historical_channel_room", "historical_room_index") .select("reference_channel_id", "message_id", "encoded_emoji").where({hashed_event_id: hash}).get() if (!row) return - await discord.snow.channel.deleteReactionSelf(row.reference_channel_id, row.message_id, row.encoded_emoji) + // See how many Matrix-side reactions there are, and delete if it's the last one + const numberOfReactions = from("reaction").where({message_id: row.message_id, encoded_emoji: row.encoded_emoji}).pluckUnsafe("count(*)").get() + if (numberOfReactions === 1) { + // If a unicode emoji, the name is already the Discord preferred version because that's what was added and stored to encoded_emoji + const emojiIdOrName = decodeURIComponent(row.encoded_emoji).split(":").slice(-1)[0] + m2dDeletedReactions.push({messageID: row.message_id, emojiIdOrName}) + await discord.snow.channel.deleteReactionSelf(row.reference_channel_id, row.message_id, row.encoded_emoji) + } db.prepare("DELETE FROM reaction WHERE hashed_event_id = ?").run(hash) } @@ -54,18 +81,12 @@ async function removeReaction(event) { * @param {Ty.Event.Outer_M_Room_Redaction} event */ async function handle(event) { - // If this is for removing a reaction, try it - await removeReaction(event) - - // Or, it might be for removing a message or suppressing embeds. But to do that, the message needs to be bridged first. - if (retrigger.eventNotFoundThenRetrigger(event.redacts, () => as.emit("type:m.room.redaction", event))) return - - const row = select("event_message", ["event_type", "event_subtype", "part"], {event_id: event.redacts}).get() - if (row && row.event_type === "m.room.message" && row.event_subtype === "m.notice" && row.part === 1) { - await suppressEmbeds(event) - } else { - await deleteMessage(event) - } + // Don't know if it's a redaction for a reaction or an event, try both at the same time (otherwise waitFor will block) + await Promise.all([ + removeMessageEvent(event), + removeReaction(event) + ]) } module.exports.handle = handle +module.exports.m2dDeletedReactions = m2dDeletedReactions \ No newline at end of file diff --git a/src/m2d/event-dispatcher.js b/src/m2d/event-dispatcher.js index 352ca41..3580d1b 100644 --- a/src/m2d/event-dispatcher.js +++ b/src/m2d/event-dispatcher.js @@ -225,7 +225,7 @@ async event => { // @ts-ignore await matrixCommandHandler.execute(event) } - retrigger.messageFinishedBridging(event.event_id) + retrigger.finishedBridging(event.event_id) await api.ackEvent(event) })) @@ -236,7 +236,7 @@ sync.addTemporaryListener(as, "type:m.sticker", guard("m.sticker", async event => { if (utils.eventSenderIsFromDiscord(event.sender)) return const messageResponses = await sendEvent.sendEvent(event) - retrigger.messageFinishedBridging(event.event_id) + retrigger.finishedBridging(event.event_id) await api.ackEvent(event) })) diff --git a/src/stdin.js b/src/stdin.js index 04b0151..43f9607 100644 --- a/src/stdin.js +++ b/src/stdin.js @@ -15,6 +15,7 @@ const mreq = sync.require("./matrix/mreq") const api = sync.require("./matrix/api") const file = sync.require("./matrix/file") const sendEvent = sync.require("./m2d/actions/send-event") +const redact = sync.require("./m2d/actions/redact") const eventDispatcher = sync.require("./d2m/event-dispatcher") const updatePins = sync.require("./d2m/actions/update-pins") const speedbump = sync.require("./d2m/actions/speedbump") @@ -22,7 +23,7 @@ const ks = sync.require("./matrix/kstate") const setPresence = sync.require("./d2m/actions/set-presence") const channelWebhook = sync.require("./m2d/actions/channel-webhook") const dUtils = sync.require("./discord/utils") -const mUtils = sync.require("./matrix/utils") +const mxUtils = sync.require("./matrix/utils") const guildID = "112760669178241024" async function ping() { From 18b6efdd1863dbd88519305862fcfb2587fb5eb4 Mon Sep 17 00:00:00 2001 From: Ellie Algase Date: Mon, 1 Jun 2026 00:08:36 -0400 Subject: [PATCH 2/2] Fix editing permissions interactions not working Co-authored-by: Cadence Ember --- src/discord/register-interactions.js | 54 ++++++++++++---------------- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/src/discord/register-interactions.js b/src/discord/register-interactions.js index e3d58c4..66012b4 100644 --- a/src/discord/register-interactions.js +++ b/src/discord/register-interactions.js @@ -91,40 +91,32 @@ function registerInteractions() { async function dispatchInteraction(interaction) { const interactionId = interaction.data?.["custom_id"] || interaction.data?.["name"] try { - if (interaction.type === DiscordTypes.InteractionType.MessageComponent || interaction.type === DiscordTypes.InteractionType.ModalSubmit) { - // All we get is custom_id, don't know which context the button was clicked in. - // So we namespace these ourselves in the custom_id. Currently the only existing namespace is POLL_. - if (interaction.data.custom_id.startsWith("POLL_")) { - await poll.interact(interaction) + if (interactionId === "Matrix info") { + await matrixInfo.interact(interaction) + } else if (interactionId === "invite") { + await invite.interact(interaction) + } else if (interactionId === "invite_channel") { + await invite.interactButton(interaction) + } else if (interactionId === "Permissions") { + await permissions.interact(interaction) + } else if (interactionId === "permissions_edit") { + await permissions.interactEdit(interaction) + } else if (interactionId === "Responses") { + /** @type {DiscordTypes.APIMessageApplicationCommandGuildInteraction} */ // @ts-ignore + const messageInteraction = interaction + if (select("poll", "message_id", {message_id: messageInteraction.data.target_id}).get()) { + await pollResponses.interact(messageInteraction) } else { - throw new Error(`Unknown message component ${interaction.data.custom_id}`) + await reactions.interact(messageInteraction) } + } else if (interactionId === "ping") { + await ping.interact(interaction) + } else if (interactionId === "privacy") { + await privacy.interact(interaction) + } else if (interactionId.startsWith("POLL_")) { + await poll.interact(interaction) } else { - if (interactionId === "Matrix info") { - await matrixInfo.interact(interaction) - } else if (interactionId === "invite") { - await invite.interact(interaction) - } else if (interactionId === "invite_channel") { - await invite.interactButton(interaction) - } else if (interactionId === "Permissions") { - await permissions.interact(interaction) - } else if (interactionId === "permissions_edit") { - await permissions.interactEdit(interaction) - } else if (interactionId === "Responses") { - /** @type {DiscordTypes.APIMessageApplicationCommandGuildInteraction} */ // @ts-ignore - const messageInteraction = interaction - if (select("poll", "message_id", {message_id: messageInteraction.data.target_id}).get()) { - await pollResponses.interact(messageInteraction) - } else { - await reactions.interact(messageInteraction) - } - } else if (interactionId === "ping") { - await ping.interact(interaction) - } else if (interactionId === "privacy") { - await privacy.interact(interaction) - } else { - throw new Error(`Unknown interaction ${interactionId}`) - } + throw new Error(`Unknown interaction ${interactionId}`) } } catch (e) { let stackLines = null