fix(discord): preserve content-type and skip optimization for animated/WebP images#41265
fix(discord): preserve content-type and skip optimization for animated/WebP images#41265skidder wants to merge 1 commit intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR fixes three related silent data-loss bugs in the Discord and web media pipeline: (1) The changes are targeted and well-scoped — only the image-optimization branch and the Key observations:
Confidence Score: 4/5
Last reviewed commit: 181720d |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| describe("isAnimatedImage", () => { | ||
| it("returns false for static GIF (single frame)", async () => { | ||
| // Note: Creating true multi-frame animated GIFs with sharp programmatically | ||
| // is complex. This test verifies single-frame images return false. | ||
| // Animated image detection is also tested via integration in web/media.test.ts | ||
| const staticGif = await sharp({ | ||
| create: { width: 10, height: 10, channels: 3, background: "#ff0000" }, | ||
| }) | ||
| .gif() | ||
| .toBuffer(); | ||
|
|
||
| expect(await isAnimatedImage(staticGif)).toBe(false); | ||
| }); | ||
|
|
||
| it("returns false for static JPEG", async () => { | ||
| const jpeg = await sharp({ | ||
| create: { width: 10, height: 10, channels: 3, background: "#ff0000" }, | ||
| }) | ||
| .jpeg() | ||
| .toBuffer(); | ||
|
|
||
| expect(await isAnimatedImage(jpeg)).toBe(false); | ||
| }); | ||
|
|
||
| it("returns false for static PNG", async () => { | ||
| const png = await sharp({ | ||
| create: { width: 10, height: 10, channels: 3, background: "#ff0000" }, | ||
| }) | ||
| .png() | ||
| .toBuffer(); | ||
|
|
||
| expect(await isAnimatedImage(png)).toBe(false); | ||
| }); | ||
|
|
||
| it("returns false for static WebP", async () => { | ||
| const webp = await sharp({ | ||
| create: { width: 10, height: 10, channels: 3, background: "#ff0000" }, | ||
| }) | ||
| .webp() | ||
| .toBuffer(); | ||
|
|
||
| expect(await isAnimatedImage(webp)).toBe(false); | ||
| }); | ||
|
|
||
| it("returns false for invalid/corrupt data", async () => { | ||
| const garbage = Buffer.from("not an image at all"); | ||
| expect(await isAnimatedImage(garbage)).toBe(false); | ||
| }); | ||
|
|
||
| it("returns false for empty buffer", async () => { | ||
| expect(await isAnimatedImage(Buffer.alloc(0))).toBe(false); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The test suite for isAnimatedImage does not exercise the true path — every test case asserts false. While the comment notes that animated detection is "also tested via integration in web/media.test.ts", examining that integration suite shows it only tests static GIFs and static WebPs, not actual multi-frame images.
The core contract of isAnimatedImage — detecting and returning true for a buffer with multiple frames — is untested. If the pages > 1 comparison is accidentally inverted or the metadata key changes, all tests still pass.
Consider adding a programmatic test for a multi-frame image, or at minimum a binary fixture (e.g., a real 2-frame GIF file) to ensure the true path is exercised.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/media/image-ops.helpers.test.ts
Line: 25-77
Comment:
The test suite for `isAnimatedImage` does not exercise the `true` path — every test case asserts `false`. While the comment notes that animated detection is "also tested via integration in web/media.test.ts", examining that integration suite shows it only tests static GIFs and static WebPs, not actual multi-frame images.
The core contract of `isAnimatedImage` — detecting and returning `true` for a buffer with multiple frames — is untested. If the `pages > 1` comparison is accidentally inverted or the metadata key changes, all tests still pass.
Consider adding a programmatic test for a multi-frame image, or at minimum a binary fixture (e.g., a real 2-frame GIF file) to ensure the `true` path is exercised.
How can I resolve this? If you propose a fix, please make it concise.![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 181720dbbc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const sharp = await loadSharp(); | ||
| const meta = await sharp(buffer).metadata(); | ||
| // 'pages' indicates number of frames in multi-frame formats (GIF, WebP, APNG) | ||
| return (meta.pages ?? 1) > 1; |
There was a problem hiding this comment.
isAnimatedImage currently treats any image with meta.pages > 1 as animated, but Sharp uses pages for general multi-page formats (for example TIFF/HEIF) as well, not only animated images. Because loadWebMediaInternal now skips optimization when this helper returns true, multi-page still images can bypass the JPEG/PNG conversion path and either exceed the raw-size cap or be sent in less-compatible source formats that were previously normalized. Please gate this logic by format (or animation-specific metadata) before marking an image as animated.
Useful? React with 👍 / 👎.
openclaw-barnacle
Bot
added
channel: discord
skidder
force-pushed
the
skidder/fix-webp-blob-type
branch
from
52fa769 to
f69d859
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f69d859d99
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const isWebp = params.contentType === "image/webp"; | ||
| const shouldSkipOptimization = isGif || isWebp; | ||
| if (shouldSkipOptimization || !optimizeImages) { |
There was a problem hiding this comment.
loadWebMediaInternal now classifies every image/webp as shouldSkipOptimization, so oversized WebP files throw at the cap check instead of using the existing optimizeAndClampImage compression path. I traced this through extensions/whatsapp/src/send.ts and extensions/whatsapp/src/auto-reply/deliver-reply.ts, both of which call loadWebMedia for capped outbound media, so large WebP messages that previously could be recompressed and sent will now fail with an exceeds ... limit error and fall back to non-media behavior. This is a regression for WhatsApp send/auto-reply flows; skipping optimization should not bypass the reduce-to-fit path when the original WebP is over the size limit.
Useful? React with 👍 / 👎.
skidder
force-pushed
the
skidder/fix-webp-blob-type
branch
from
f69d859 to
97f5e67
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a2bcced74
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -1822,6 +1822,7 @@ async function deliverDiscordInteractionReply(params: { | |||
| mediaList.map(async (url) => { | |||
| const loaded = await loadWebMedia(url, { | |||
| localRoots: params.mediaLocalRoots, | |||
| preserveWebp: true, | |||
There was a problem hiding this comment.
Enabling preserveWebp here allows loadWebMedia to return WebP bytes unchanged, but this interaction-reply flow still wraps buffers as new Blob([arrayBuffer]) without a type and falls back to the generic filename upload. For media URLs that do not provide a filename/extension, Discord receives an application/octet-stream attachment, so preserved WebP replies can lose image typing and show up as generic files instead of inline images.
Useful? React with 👍 / 👎.
skidder
force-pushed
the
skidder/fix-webp-blob-type
branch
from
4a2bcce to
b7fa90e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7fa90ee32
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ); | ||
| const media = await loadWebMedia(mediaUrl, { | ||
| ...buildOutboundMediaLoadOptions({ maxBytes, mediaLocalRoots }), | ||
| preserveWebp: true, |
There was a problem hiding this comment.
preserveWebp flag into the shared loader
loadWebMedia here comes from openclaw/plugin-sdk/web-media, which just re-exports src/media/web-media.ts (src/plugin-sdk/web-media.ts:1-5). That shared loader still has no preserveWebp option and still always normalizes non-GIF images through optimizeAndClampImage (src/media/web-media.ts:26-35, src/media/web-media.ts:239-307). Because only extensions/whatsapp/src/media.ts was updated, this new flag is a no-op for Discord: regular Discord sends will still convert .webp uploads to JPEG, so the feature this commit is trying to add never actually takes effect outside the WhatsApp-local tests.
Useful? React with 👍 / 👎.
skidder
force-pushed
the
skidder/fix-webp-blob-type
branch
from
b7fa90e to
6ef346b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de719075a1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const isGif = params.contentType === "image/gif"; | ||
| if (isGif || !optimizeImages) { | ||
| const isWebp = params.contentType === "image/webp"; | ||
| const skipOptimization = isGif || (isWebp && preserveWebp) || !optimizeImages; |
There was a problem hiding this comment.
This branch still only bypasses optimization for image/gif and image/webp. Animated PNGs come through as image/png, so Discord message sends still fall into optimizeAndClampImage, and the optimizeImageToPng / resizeToJpeg pipelines in src/media/image-ops.ts re-encode a single static frame. In other words, APNG attachments are still silently flattened in the flow this change is trying to preserve for animated images.
Useful? React with 👍 / 👎.
| const skipOptimization = isGif || (isWebp && preserveWebp) || !optimizeImages; | ||
| if (skipOptimization) { | ||
| if (params.buffer.length > cap) { | ||
| throw new Error(formatCapLimit(isGif ? "GIF" : "Media", cap, params.buffer.length)); |
There was a problem hiding this comment.
The new WebP-preservation path rejects against the loader's default image cap, but the Discord interaction-reply and component-send call sites now opt into preserveWebp: true without supplying maxBytes (extensions/discord/src/monitor/native-command.ts:942-945, extensions/discord/src/send.components.ts:101-104). Because maxBytesForKind("image") is only 6 MB in src/media/constants.ts, valid 6–8 MB WebPs that Discord would accept now start failing in those flows instead of uploading, whereas the regular Discord send path still uses the larger channel cap.
Useful? React with 👍 / 👎.
skidder
force-pushed
the
skidder/fix-webp-blob-type
branch
from
de71907 to
4f82827
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f828273bc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| maxBytes: params.opts.mediaMaxBytes, | ||
| preserveWebp: true, | ||
| preserveAvif: true, |
There was a problem hiding this comment.
This code now preserves WebP/AVIF (preserveWebp/preserveAvif) but still relies on params.opts.mediaMaxBytes, which is usually undefined, so loadWebMedia falls back to the global 6MB image cap instead of Discord’s per-account cap (default 8MB). Fresh evidence in this commit is that mediaMaxBytes was added to DiscordComponentSendOpts but production callsites still don’t populate it (checked extensions/discord/src/outbound-adapter.ts and extensions/discord/src/actions/runtime.messaging.ts), so component sends with 6–8MB WebP/AVIF attachments can now fail with Media exceeds 6MB limit where normal Discord sends still succeed.
Useful? React with 👍 / 👎.
skidder
force-pushed
the
skidder/fix-webp-blob-type
branch
from
4f82827 to
c62292f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c62292f20f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const isPreservedFormat = | ||
| (params.contentType === "image/webp" && preserveWebp) || | ||
| (params.contentType === "image/avif" && preserveAvif); | ||
| const skipOptimization = isGif || isPreservedFormat || !optimizeImages; |
There was a problem hiding this comment.
This new skipOptimization path now includes preserved WebP/AVIF images, but remote fetching still derives fetchCap from optimizeImages alone (up to the 100MB document cap), so Discord uploads with an 8MB channel limit can download and buffer very large preserved images before failing at the later size check. In practice, a remote 30–100MB WebP/AVIF now incurs unnecessary bandwidth, latency, and memory pressure before returning the same cap error; preserved formats should be treated as non-optimizable when computing fetch limits.
Useful? React with 👍 / 👎.
skidder
force-pushed
the
skidder/fix-webp-blob-type
branch
from
c62292f to
d9e5436
Compare
openclaw-barnacle
Bot
removed
the
channel: whatsapp-web
1 participant
Summary
toDiscordFileBlobnow accepts and forwards acontentTypeto theBlobconstructor;loadWebMediaInternalskips image optimization for GIF, WebP, and any animated image (detected via sharp'spagesmetadata); a newisAnimatedImagehelper was added toimage-ops.ts.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
Blob, so recipients see the right file format.GIF,WebP,Animated image) instead of the genericMedia.Security Impact (required)
Repro + Verification
Environment
Steps
Expected
Actual (before fix)
Blobhad notype, so Discord received files with an unknown MIME type.Evidence
src/discord/send.shared.test.ts,src/media/image-ops.helpers.test.ts, andsrc/web/media.test.tscovering all three fix areas.Human Verification (required)
Review Conversations
Compatibility / Migration
Failure Recovery (if this breaks)
src/web/media.tsto re-enable optimization for WebP/animated; revertsrc/discord/send.shared.tsto drop content-type fromBlob.src/web/media.ts,src/discord/send.shared.ts,src/discord/send.components.tsapplication/octet-stream.Risks and Mitigations
WebP/Animated imagelabel) rather than silent conversion.