fix: plain replies deletion and edits #3416

lorenzo132 · 2025-12-13T14:05:10Z

No description provided.

StephenDaDev

Using color to check message status causes issue of mod_color is changed. A fix is being worked on.

This will avoid crashes when the mod_color get changed.

lorenzo132 · 2025-12-13T22:12:49Z

Dont touch this yet. Working on resolving some issues.

martinbndr · 2025-12-13T22:18:50Z

Dont touch this yet. Working on resolving some issues.

Nvm just saw it yet. I have re requested for my review to remove my approval

StephenDaDev

Seems to be solved.

sebkuip

Currently when deleting a message that is a note through discord's built in delete function, it causes an error. I'm unsure if this is something existing on main branch as well, but as this PR addresses deleting its best if this is fixed as well.

sebkuip

Tested this throughoutly now without any errors appearing on my side anymore. Issue above seems to be resolved.

LGTM

Copilot

Pull request overview

This PR fixes the deletion and editing functionality for plain replies in the modmail system. Previously, plain messages (messages sent without embeds) were not supported for deletion and editing operations. The changes implement support for finding, matching, and editing plain messages by using footer markers and content matching instead of relying on embed author URLs.

Changes:

Refactored find_linked_messages to support plain message detection and matching using footer text markers and content comparison
Updated edit_message to reconstruct plain message content format when editing plain messages in DMs
Improved error messages in edit and delete commands to show actual error details instead of generic messages

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 15 comments.

File	Description
core/thread.py	Refactored message finding logic to detect and match plain messages using footer markers; added plain message editing support with content reconstruction
cogs/modmail.py	Changed error handling to display specific error messages from exceptions instead of generic hardcoded messages
bot.py	Added new error message strings to the set of expected benign deletion errors

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-16T12:49:54Z

core/thread.py

+                    elif msg.embeds[0].author.url and msg.embeds[0].author.url.split("#")[-1].isdigit():
+                        is_valid_candidate = True


There's a missing check for whether msg.embeds[0].author exists before accessing msg.embeds[0].author.url. If the embed exists but doesn't have an author field, this will raise an AttributeError. The code should verify that msg.embeds[0].author is not None before attempting to access its url attribute.

Suggested change

elif msg.embeds[0].author.url and msg.embeds[0].author.url.split("#")[-1].isdigit():

is_valid_candidate = True

else:

author = msg.embeds[0].author

if author and author.url and author.url.split("#")[-1].isdigit():

is_valid_candidate = True

Copilot · 2026-01-16T12:49:54Z

core/thread.py

+            mod_tag = message1.embeds[0].footer.text.replace("[PLAIN]", "", 1).strip()
+            author_name = message1.embeds[0].author.name
+            desc = message1.embeds[0].description or ""
+            prefix = f"**{mod_tag} " if mod_tag else "**"
+            plain_content_expected = f"{prefix}{author_name}:** {desc}"
+            creation_time = message1.created_at

        messages = [message1]
-        for user in self.recipients:
-            async for msg in user.history():
-                if either_direction:
-                    if msg.id == joint_id:
-                        return message1, msg

-                if not (msg.embeds and msg.embeds[0].author.url):
-                    continue
-                try:
-                    if int(msg.embeds[0].author.url.split("#")[-1]) == joint_id:
+        if is_plain:
+            for user in self.recipients:
+                async for msg in user.history(limit=50, around=creation_time):
+                    if abs((msg.created_at - creation_time).total_seconds()) > 15:
+                        continue
+                    if msg.author != self.bot.user:
+                        continue
+                    if msg.embeds:
+                        continue
+
+                    if msg.content == plain_content_expected:


The code constructs the expected plain content to match against, but this logic is brittle. If the mod_tag contains special characters or if there are any formatting differences in how the message was originally constructed versus how it's being reconstructed here, the match will fail. Consider a more robust matching strategy, such as checking for key components separately or using fuzzy matching for the content.

Copilot · 2026-01-16T12:49:55Z

core/thread.py

+                    if is_plain:
+                        # Reconstruct the plain message format to preserve matching capability
+                        mod_tag = embed1.footer.text.replace("[PLAIN]", "", 1).strip()
+                        author_name = embed1.author.name


When reconstructing the plain message content for editing, the code directly accesses embed1.author.name without checking if embed1.author exists. If the embed doesn't have an author field, this will raise an AttributeError. Add a check to ensure embed1.author is not None before accessing its name attribute.

Suggested change

author_name = embed1.author.name

author_name = embed1.author.name if embed1.author and embed1.author.name else ""

Copilot · 2026-01-16T12:49:55Z

core/thread.py

+        else:
+            joint_id = None
+            mod_tag = message1.embeds[0].footer.text.replace("[PLAIN]", "", 1).strip()
+            author_name = message1.embeds[0].author.name


Similar to the issue in find_linked_messages, accessing embed1.author.name without first checking if embed1.author exists can cause an AttributeError. Verify that embed1.author is not None before accessing its name attribute.

Copilot · 2026-01-16T12:49:55Z

core/thread.py

+            return message1, None

-            if message1.embeds[0].color.value != self.bot.mod_color and not (
-                either_direction and message1.embeds[0].color.value == self.bot.recipient_color
-            ):
-                logger.warning("Message color does not match mod/recipient colors.")
-                raise ValueError("Thread message not found.")
-        else:
-            async for message1 in self.channel.history():
-                if (
-                    message1.embeds
-                    and message1.embeds[0].author.url
-                    and message1.embeds[0].color
-                    and (
-                        message1.embeds[0].color.value == self.bot.mod_color
-                        or (either_direction and message1.embeds[0].color.value == self.bot.recipient_color)
-                    )
-                    and message1.embeds[0].author.url.split("#")[-1].isdigit()
-                    and message1.author == self.bot.user
-                ):
-                    break
-            else:
+        if not note and is_note:
+            raise ValueError("Thread message is an internal message, not a note.")
+
+        if is_note:
+            return message1, None


The return type annotation indicates the method returns a Tuple containing a discord.Message and a List of Optional discord.Messages. However, when returning for notes (line 1381 and 1387), the method returns (message1, None) where None is not a List. This violates the type contract. The return should be (message1, []) or the type annotation should be updated to allow None as the second element.

Copilot · 2026-01-16T12:49:56Z

core/thread.py

+                            messages.append(msg)
+                            break
+
+                    if not (msg.embeds and msg.embeds[0].author.url):


The code checks msg.embeds[0].author.url without first verifying that msg.embeds[0].author exists. If an embed doesn't have an author field, this will raise an AttributeError. Add a check to ensure msg.embeds[0].author is not None before accessing its url attribute.

Copilot · 2026-01-16T12:49:57Z

core/thread.py


        if len(messages) > 1:
            return messages



The error "Linked DM message not found." is raised when len(messages) <= 1, but this doesn't distinguish between the case where no linked messages were found versus when we're dealing with a note that doesn't have linked messages. For notes that return early with (message1, None), this is fine, but for regular messages, this error assumes that a linked message should always exist. Consider whether this is the intended behavior for all message types, particularly for plain messages where finding the linked DM might legitimately fail.

Suggested change

if is_plain:

# For plain messages, it's acceptable that no linked DM is found;

# in that case, just return the original message.

return messages

Copilot · 2026-01-16T12:49:57Z

core/thread.py

+            for user in self.recipients:
+                async for msg in user.history(limit=50, around=creation_time):
+                    if abs((msg.created_at - creation_time).total_seconds()) > 15:


The time window check of 15 seconds may be too restrictive for matching plain messages. If there's any network delay, message processing delay, or if the bot is under load, the DM message could be created slightly more than 15 seconds after the thread message, causing the match to fail. Consider increasing this tolerance to at least 30-60 seconds to account for potential delays.

Suggested change

for user in self.recipients:

async for msg in user.history(limit=50, around=creation_time):

if abs((msg.created_at - creation_time).total_seconds()) > 15:

tolerance_seconds = 60

for user in self.recipients:

async for msg in user.history(limit=50, around=creation_time):

if abs((msg.created_at - creation_time).total_seconds()) > tolerance_seconds:

Copilot · 2026-01-16T12:49:57Z

core/thread.py

+            except (ValueError, AttributeError, IndexError):
+                raise ValueError("Malformed thread message.")
+        else:
+            joint_id = None


Variable joint_id is not used.

Suggested change

joint_id = None

Copilot · 2026-01-16T12:49:57Z

core/thread.py

+        if is_note:
+            return message1, None
+


This statement is unreachable.

Suggested change

if is_note:

return message1, None

lorenzo132 added 2 commits December 13, 2025 14:59

support to edit and delete plain reply messages

f33b06c

fix linting

20af225

lorenzo132 mentioned this pull request Dec 13, 2025

Support editing and deleting plain messages #2876

Closed

lorenzo132 requested review from a team, StephenDaDev, Taaku18, martinbndr and sebkuip December 13, 2025 14:43

StephenDaDev requested changes Dec 13, 2025

View reviewed changes

fix: not rely on mod_color as originally was made.

e620a03

This will avoid crashes when the mod_color get changed.

martinbndr previously approved these changes Dec 13, 2025

View reviewed changes

martinbndr self-requested a review December 13, 2025 22:17

fix: typeerror / refactor

4e38eaa

lorenzo132 dismissed martinbndr’s stale review via 4e38eaa December 13, 2025 22:23

fix linting

420ece0

lorenzo132 requested a review from StephenDaDev December 13, 2025 22:27

martinbndr previously approved these changes Dec 13, 2025

View reviewed changes

StephenDaDev previously approved these changes Dec 14, 2025

View reviewed changes

Merge branch 'development' into users/lorenzo132/plainmsg-edit-deletion

2992eb0

StephenDaDev added the changelog Changes in PR have been added to draft release that will be used for the changelog on the next ver. label Dec 18, 2025

sebkuip requested changes Dec 18, 2025

View reviewed changes

silent unneeded noise

2758afe

lorenzo132 dismissed stale reviews from StephenDaDev and martinbndr via 2758afe December 18, 2025 19:00

lorenzo132 requested review from StephenDaDev, martinbndr and sebkuip December 18, 2025 19:07

sebkuip approved these changes Dec 18, 2025

View reviewed changes

StephenDaDev approved these changes Dec 19, 2025

View reviewed changes

martinbndr approved these changes Jan 16, 2026

View reviewed changes

martinbndr self-requested a review January 16, 2026 12:41

Merge branch 'development' into users/lorenzo132/plainmsg-edit-deletion

9e549a4

Copilot AI review requested due to automatic review settings January 16, 2026 12:43

Copilot started reviewing on behalf of lorenzo132 January 16, 2026 12:44 View session

lorenzo132 enabled auto-merge January 16, 2026 12:44

lorenzo132 added this pull request to the merge queue Jan 16, 2026

Merged via the queue into development with commit 040195d Jan 16, 2026
9 checks passed

lorenzo132 deleted the users/lorenzo132/plainmsg-edit-deletion branch January 16, 2026 12:46

Copilot AI reviewed Jan 16, 2026

View reviewed changes

martinbndr approved these changes Jan 16, 2026

View reviewed changes

		elif msg.embeds[0].author.url and msg.embeds[0].author.url.split("#")[-1].isdigit():
		is_valid_candidate = True

	author_name = embed1.author.name
	author_name = embed1.author.name if embed1.author and embed1.author.name else ""

Uh oh!

fix: plain replies deletion and edits #3416

fix: plain replies deletion and edits #3416

Conversation

lorenzo132 commented Dec 13, 2025

Uh oh!

StephenDaDev left a comment

Choose a reason for hiding this comment

Uh oh!

lorenzo132 commented Dec 13, 2025

Uh oh!

martinbndr commented Dec 13, 2025

Uh oh!

StephenDaDev left a comment

Choose a reason for hiding this comment

Uh oh!

sebkuip left a comment

Choose a reason for hiding this comment

Uh oh!

sebkuip left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants