fix(adapters): notify agent of failed image attachments so reply aligns with warning

src/discord.rs

@@ -89,7 +89,10 @@ impl ChatAdapter for DiscordAdapter {
         let builder = serenity::builder::CreateMessage::new()
             .content(content)
             .reference_message((ChannelId::new(ch_id), MessageId::new(msg_id)));
-        match ChannelId::new(ch_id).send_message(&self.http, builder).await {
+        match ChannelId::new(ch_id)
+            .send_message(&self.http, builder)
+            .await
+        {
             Ok(msg) => Ok(MessageRef {
                 channel: channel.clone(),
                 message_id: msg.id.to_string(),
@@ -105,7 +108,9 @@ impl ChatAdapter for DiscordAdapter {
     async fn delete_message(&self, msg: &MessageRef) -> anyhow::Result<()> {
         let ch_id: u64 = Self::resolve_channel(&msg.channel).parse()?;
         let msg_id: u64 = msg.message_id.parse()?;
-        self.http.delete_message(ChannelId::new(ch_id), MessageId::new(msg_id), None).await?;
+        self.http
+            .delete_message(ChannelId::new(ch_id), MessageId::new(msg_id), None)
+            .await?;
         Ok(())
     }
 
@@ -415,10 +420,13 @@ impl EventHandler for Handler {
         let in_allowed_channel =
             self.allow_all_channels || self.allowed_channels.contains(&channel_id);
 
-        let is_mentioned =
-            msg.mentions_user_id(bot_id) || msg.content.contains(&format!("<@{}>", bot_id))
+        let is_mentioned = msg.mentions_user_id(bot_id)
+            || msg.content.contains(&format!("<@{}>", bot_id))
             || (!self.allowed_role_ids.is_empty()
-                && msg.mention_roles.iter().any(|r| self.allowed_role_ids.contains(&r.get())));
+                && msg
+                    .mention_roles
+                    .iter()
+                    .any(|r| self.allowed_role_ids.contains(&r.get())));
 
         // Bot message gating (from upstream #321)
         if msg.author.bot {
@@ -639,6 +647,7 @@ impl EventHandler for Handler {
         let mut echo_entries: Vec<crate::stt::EchoEntry> = Vec::new();
         let mut text_file_bytes: u64 = 0;
         let mut text_file_count: u32 = 0;
+        let mut failed_image_files: Vec<String> = Vec::new();
         const TEXT_TOTAL_CAP: u64 = 1024 * 1024; // 1 MB total for all text file attachments
         const TEXT_FILE_COUNT_CAP: u32 = 5;
 
@@ -702,25 +711,56 @@ impl EventHandler for Handler {
                     debug!(filename = %attachment.filename, "adding text file attachment");
                     extra_blocks.push(block);
                 }
-            } else if let Some(block) = media::download_and_encode_image(
-                &attachment.url,
-                attachment.content_type.as_deref(),
-                &attachment.filename,
-                u64::from(attachment.size),
-                None,
-            )
-            .await
-            {
-                debug!(url = %attachment.url, filename = %attachment.filename, "adding image attachment");
-                extra_blocks.push(block);
-            } else if media::is_video_file(&attachment.filename, attachment.content_type.as_deref()) {
-                debug!(url = %attachment.url, filename = %attachment.filename, "adding video attachment link");
-                extra_blocks.push(video_attachment_block(
-                    &attachment.filename,
+            } else {
+                match media::download_and_encode_image(
+                    &attachment.url,
                     attachment.content_type.as_deref(),
+                    &attachment.filename,
                     u64::from(attachment.size),
-                    &attachment.url,
-                ));
+                    None,
+                )
+                .await
+                {
+                    Ok(block) => {
+                        debug!(url = %attachment.url, filename = %attachment.filename, "adding image attachment");
+                        extra_blocks.push(block);
+                    }
+                    Err(media::MediaFetchError::NotAnImage) => {
+                        if media::is_video_file(
+                            &attachment.filename,
+                            attachment.content_type.as_deref(),
+                        ) {
+                            debug!(url = %attachment.url, filename = %attachment.filename, "adding video attachment link");
+                            extra_blocks.push(video_attachment_block(
+                                &attachment.filename,
+                                attachment.content_type.as_deref(),
+                                u64::from(attachment.size),
+                                &attachment.url,
+                            ));
+                        }
+                    }
+                    Err(e) => {
+                        if let Some(entry) =
+                            media::failed_attachment_entry(&attachment.filename, &e)
+                        {
+                            tracing::warn!(
+                                url = %attachment.url,
+                                filename = %attachment.filename,
+                                error = %e,
+                                "image attachment failed"
+                            );
+                            failed_image_files.push(entry);
+                        } else {
+                            // Network/HTTP 5xx failures: transient; warn in logs only.
+                            tracing::warn!(
+                                url = %attachment.url,
+                                filename = %attachment.filename,
+                                error = %e,
+                                "image download failed"
+                            );
+                        }
+                    }
+                }
             }
         }
 
@@ -744,12 +784,47 @@ impl EventHandler for Handler {
             match get_or_create_thread(&ctx, &adapter, &msg, &prompt).await {
                 Ok(ch) => ch,
                 Err(e) => {
+                    // Thread creation failed — entire message is dropped. No reply,
+                    // no warning, no agent note. The failed_image_files warning and
+                    // agent note are NOT sent here: we have no thread to target and
+                    // the agent won't run anyway.
                     error!("failed to create thread: {e}");
                     return;
                 }
             }
         };
 
+        // Agent note and user warning are sent together or not at all.
+        // Push the agent note here (after thread_channel is resolved) so a
+        // thread-creation failure above silences both, keeping them in sync.
+        if !failed_image_files.is_empty() {
+            extra_blocks.push(ContentBlock::Text {
+                text: media::format_failed_attachment_note(&failed_image_files),
+            });
+        }
+
+        // Send user-visible warning into the correct thread now that we know where it is.
+        // For top-level channel messages, thread_channel is the newly-created thread —
+        // not msg.channel_id. Sending before get_or_create_thread would route to the parent.
+        if !failed_image_files.is_empty() {
+            let file_list = failed_image_files
+                .iter()
+                .map(|n| sanitize_discord_filename(n))
+                .collect::<Vec<_>>()
+                .join("`, `");
+            let warn_msg = format!(
+                ":warning: I couldn't process the file(s) you shared (`{file_list}`). \
+                 Supported formats are PNG / JPEG / GIF / WebP up to 10 MB."
+            );
+            if let Err(e) = adapter.send_message(&thread_channel, &warn_msg).await {
+                warn!(
+                    channel_id = %msg.channel_id,
+                    error = %e,
+                    "failed to send image validation warning to user"
+                );
+            }
+        }
+
         let trigger_msg = discord_msg_ref(&msg);
 
         // Per-thread streaming: check if another bot is present in this thread
@@ -1323,14 +1398,24 @@ fn resolve_mentions(content: &str, bot_id: UserId, allowed_role_ids: &HashSet<u6
     let out = if allowed_role_ids.is_empty() {
         out
     } else {
-        allowed_role_ids.iter().fold(out, |s, id| s.replace(&format!("<@&{}>", id), ""))
+        allowed_role_ids
+            .iter()
+            .fold(out, |s, id| s.replace(&format!("<@&{}>", id), ""))
     };
     // 3. Other user mentions: keep <@UID> as-is so the LLM can mention back
     // 4. Fallback: replace remaining role mentions only (user mentions are preserved)
     let out = ROLE_MENTION_RE.replace_all(&out, "@(role)").to_string();
     out.trim().to_string()
 }
 
+/// Sanitize a filename for safe embedding in a Discord message.
+///
+/// Delegates to `media::sanitize_attachment_filename` so the escaping rules
+/// stay in one place for both Slack and Discord.
+fn sanitize_discord_filename(s: &str) -> String {
+    media::sanitize_attachment_filename(s)
+}
+
 fn video_attachment_block(
     filename: &str,
     content_type: Option<&str>,
@@ -1488,6 +1573,24 @@ mod tests {
     use super::*;
     use crate::bot_turns::{TurnResult, HARD_BOT_TURN_LIMIT};
 
+    // --- sanitize_discord_filename tests ---
+
+    #[test]
+    fn sanitize_discord_leaves_normal_filename_unchanged() {
+        assert_eq!(sanitize_discord_filename("photo.png"), "photo.png");
+        assert_eq!(sanitize_discord_filename("my file (1).jpg"), "my file (1).jpg");
+    }
+
+    #[test]
+    fn sanitize_discord_replaces_backtick() {
+        assert_eq!(sanitize_discord_filename("file`name.png"), "file'name.png");
+    }
+
+    #[test]
+    fn sanitize_discord_combined_injection_attempt() {
+        assert_eq!(sanitize_discord_filename("`<!here>`"), "'(!here)'");
+    }
+
     // --- resolve_mentions tests ---
 
     /// Bot's own <@UID> mention is stripped from the prompt.
@@ -2125,4 +2228,5 @@ mod tests {
     fn normal_channel_creates_thread() {
         assert!(!should_skip_thread_creation(false, false));
     }
+
 }

src/dispatch.rs

@@ -796,6 +796,37 @@ mod tests {
         assert!(matches!(&blocks[3], ContentBlock::Image { .. }));
     }
 
+    #[test]
+    fn pack_arrival_event_keeps_text_extras_order_before_prompt() {
+        let extra = vec![
+            ContentBlock::Text {
+                text: "[Voice message transcript]: hello".into(),
+            },
+            ContentBlock::Text {
+                text: "[Attachment validation failed]: the user attempted to attach `photo.png`"
+                    .into(),
+            },
+            ContentBlock::Image {
+                media_type: "image/png".into(),
+                data: "abc".into(),
+            },
+        ];
+        let blocks = AdapterRouter::pack_arrival_event("{}", "typed prompt", extra);
+
+        assert_eq!(blocks.len(), 5);
+        assert!(
+            matches!(&blocks[0], ContentBlock::Text { text } if text.contains("<sender_context>"))
+        );
+        assert!(
+            matches!(&blocks[1], ContentBlock::Text { text } if text.contains("Voice message transcript"))
+        );
+        assert!(
+            matches!(&blocks[2], ContentBlock::Text { text } if text.contains("Attachment validation failed"))
+        );
+        assert!(matches!(&blocks[3], ContentBlock::Text { text } if text == "typed prompt"));
+        assert!(matches!(&blocks[4], ContentBlock::Image { .. }));
+    }
+
     #[test]
     fn pack_arrival_event_batch_n2() {
         // Two arrival events concatenated → 2 (header + prompt) pairs = 4 blocks.