Skip to content

Commit 3c03d39

Browse files
howieclaude
andcommitted
fix(adapters): inject system note when image validation fails so agent reply aligns with warning
When download_and_encode_image rejects an attachment, the Slack/Discord adapter already sends the user a visible ⚠️ message. Without this change, the agent sees no attachment and may reply "I don't see any image attached" -- contradicting the warning the user just received. - Add media::format_failed_attachment_note() as a shared helper (single source of truth) for the ContentBlock::Text injected into extra_blocks. - Slack: push the note after the existing warning send (pack_arrival_event partitions Text blocks before the typed prompt; stable iteration order means push trails any STT transcript, which is correct). - Discord: introduce failed_image_files Vec, split the catch-all Err arm into variant-specific arms matching Slack, send a Discord-flavored user warning (without the Slack files:read scope hint), and inject the note. - Add three unit tests for format_failed_attachment_note in media::tests. Addresses the minor UX observation from the openabdev#776 final review comment (antigenius0910, 2026-05-14). Depends on PR openabdev#793 (failed_image_files Vec and MediaFetchError variants introduced there). Fixes openabdev#776 (UX follow-up). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 6d7fd5f commit 3c03d39

3 files changed

Lines changed: 105 additions & 0 deletions

File tree

src/discord.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,7 @@ impl EventHandler for Handler {
647647
let mut echo_entries: Vec<crate::stt::EchoEntry> = Vec::new();
648648
let mut text_file_bytes: u64 = 0;
649649
let mut text_file_count: u32 = 0;
650+
let mut failed_image_files: Vec<String> = Vec::new();
650651
const TEXT_TOTAL_CAP: u64 = 1024 * 1024; // 1 MB total for all text file attachments
651652
const TEXT_FILE_COUNT_CAP: u32 = 5;
652653

@@ -738,6 +739,39 @@ impl EventHandler for Handler {
738739
));
739740
}
740741
}
742+
Err(media::MediaFetchError::SizeExceeded { actual, limit }) => {
743+
tracing::warn!(
744+
url = %attachment.url,
745+
filename = %attachment.filename,
746+
actual,
747+
limit,
748+
"image exceeds size limit"
749+
);
750+
failed_image_files.push(format!(
751+
"{} (exceeds {limit} byte limit)",
752+
attachment.filename
753+
));
754+
}
755+
Err(
756+
media::MediaFetchError::UnsupportedResponseType { .. }
757+
| media::MediaFetchError::InvalidImageBody { .. },
758+
) => {
759+
tracing::warn!(
760+
url = %attachment.url,
761+
filename = %attachment.filename,
762+
"image validation failed; body is not a supported image"
763+
);
764+
failed_image_files.push(attachment.filename.clone());
765+
}
766+
Err(media::MediaFetchError::ProcessingFailed(ref e)) => {
767+
tracing::warn!(
768+
url = %attachment.url,
769+
filename = %attachment.filename,
770+
error = %e,
771+
"image post-processing failed"
772+
);
773+
failed_image_files.push(attachment.filename.clone());
774+
}
741775
Err(e) => {
742776
tracing::warn!(
743777
url = %attachment.url,
@@ -750,6 +784,28 @@ impl EventHandler for Handler {
750784
}
751785
}
752786

787+
if !failed_image_files.is_empty() {
788+
let warn_channel = ChannelRef {
789+
platform: "discord".into(),
790+
channel_id: msg.channel_id.get().to_string(),
791+
thread_id: None,
792+
parent_id: None,
793+
origin_event_id: None,
794+
};
795+
let file_list = failed_image_files.join("`, `");
796+
let warn_msg = format!(
797+
":warning: I couldn't process the file(s) you shared (`{file_list}`). \
798+
Supported formats are PNG / JPEG / GIF / WebP up to 10 MB."
799+
);
800+
if let Err(e) = adapter.send_message(&warn_channel, &warn_msg).await {
801+
warn!(error = %e, "failed to send image validation warning to user");
802+
}
803+
let names: Vec<&str> = failed_image_files.iter().map(String::as_str).collect();
804+
extra_blocks.push(ContentBlock::Text {
805+
text: media::format_failed_attachment_note(&names),
806+
});
807+
}
808+
753809
tracing::debug!(
754810
num_extra_blocks = extra_blocks.len(),
755811
num_attachments = msg.attachments.len(),

src/media.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,23 @@ impl std::fmt::Display for MediaFetchError {
6767
}
6868
}
6969

70+
/// Build a `[Attachment validation failed]` note for injection into the agent prompt
71+
/// so the agent's reply acknowledges the failure instead of asking "where's the image?".
72+
/// Caller must guarantee `filenames` is non-empty.
73+
pub fn format_failed_attachment_note(filenames: &[&str]) -> String {
74+
let list = filenames
75+
.iter()
76+
.map(|n| format!("`{n}`"))
77+
.collect::<Vec<_>>()
78+
.join(", ");
79+
format!(
80+
"[Attachment validation failed]: the user attempted to attach {list} but \
81+
the file(s) could not be processed (unsupported format, corrupt body, or \
82+
size limit exceeded). The user has been notified separately — acknowledge \
83+
the failure and proceed without apologising for not seeing the file."
84+
)
85+
}
86+
7087
/// Strip MIME parameters and trim whitespace. `"image/png; charset=binary"` → `"image/png"`.
7188
pub(crate) fn strip_mime_params(mime: &str) -> &str {
7289
mime.split(';').next().unwrap_or(mime).trim()
@@ -793,4 +810,28 @@ mod tests {
793810
let bytes = [0xffu8, 0xd8];
794811
assert_eq!(hex_prefix(&bytes), "ffd8");
795812
}
813+
814+
// --- format_failed_attachment_note tests ---
815+
816+
#[test]
817+
fn format_failed_attachment_note_single() {
818+
let note = super::format_failed_attachment_note(&["photo.png"]);
819+
assert!(note.contains("`photo.png`"));
820+
assert!(note.starts_with("[Attachment validation failed]:"));
821+
assert!(note.contains("The user has been notified separately"));
822+
}
823+
824+
#[test]
825+
fn format_failed_attachment_note_multiple() {
826+
let note = super::format_failed_attachment_note(&["a.png", "b.jpg"]);
827+
assert!(note.contains("`a.png`"));
828+
assert!(note.contains("`b.jpg`"));
829+
assert!(note.contains(", "));
830+
}
831+
832+
#[test]
833+
fn format_failed_attachment_note_preserves_size_suffix() {
834+
let note = super::format_failed_attachment_note(&["big.png (exceeds 10000000 byte limit)"]);
835+
assert!(note.contains("`big.png (exceeds 10000000 byte limit)`"));
836+
}
796837
}

src/slack.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,6 +1103,14 @@ async fn handle_message(
11031103
if let Err(e) = adapter.send_message(&warn_channel, &msg).await {
11041104
warn!(error = %e, "failed to send image validation warning to user");
11051105
}
1106+
// Inject a system note so the agent's reply acknowledges the failure
1107+
// instead of saying "I don't see any image". pack_arrival_event partitions
1108+
// Text blocks before the prompt (stable order), so push trails any STT
1109+
// transcript but precedes the typed prompt — correct position for meta-info.
1110+
let names: Vec<&str> = failed_image_files.iter().map(String::as_str).collect();
1111+
extra_blocks.push(ContentBlock::Text {
1112+
text: media::format_failed_attachment_note(&names),
1113+
});
11061114
}
11071115

11081116
// Resolve Slack display name (best-effort, fallback to user_id)

0 commit comments

Comments
 (0)