mirror of
https://github.com/block/buzz.git
synced 2026-10-27 12:40:30 +00:00
fix(desktop): downscale large avatars for agent-share PNG body (#6260)
Sharing an agent encodes a `.agent.png` snapshot with the avatar as the PNG image body. Non-PNG avatars were transcoded at full resolution and already-PNG avatars were carried through unchanged, so a large source avatar could produce a PNG that exceeds the 10 MiB `MAX_SNAPSHOT_PNG_BYTES` cap and fail the send. A 2764×4096 webp avatar encoded to ~26 MB — over 2.5× the ceiling — which is why sharing that agent hung for a few seconds (fetching and transcoding the image) and then failed. The share dialog compounded the problem: on failure it discarded the pipeline's real error (`Snapshot exceeds the 10 MiB size limit…`) and toasted a generic `Couldn't send … Try again.`, hiding the actual cause. ## Changes - **Downscale the avatar for the snapshot PNG body.** The body is only a card thumbnail, so raster avatars are now downscaled to a 512px longest edge (`MAX_PNG_BODY_EDGE`) before PNG re-encoding, mirroring the frontend SVG rasterizer's 512×512 cap in `snapshotAvatarPng.ts`. Already-PNG avatars over the dimension cap route through the same downscaling transcode path instead of a straight tEXt-chunk injection. The manifest's `avatar_url`/`avatar_data_url` source reference is untouched — only the PNG image body is downscaled. - **Surface the real error in the share dialog.** `PersonaShareDialog` (used by `SnapshotShareDialog`/`TeamShareDialog`) and `AgentCardViewerDialog` now toast the send controller's actual error message, falling back to the generic string only when it is empty. A `getCurrentError()` accessor reads the error through a ref because the render-captured `state.error` is stale in the closure immediately after `beginSend` resolves. A unit test pins the invariant: an oversize (2764×4096) avatar produces a snapshot that stays far under the cap with body dimensions clamped to 512px. --------- Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> Co-authored-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -56,6 +56,13 @@ pub const PNG_CHUNK_KEYWORD: &str = "buzz_agent_snapshot";
|
||||
/// this are stored as a URL reference instead.
|
||||
const MAX_AVATAR_INLINE_BYTES: usize = 2 * 1024 * 1024; // 2 MB
|
||||
|
||||
/// Maximum edge (px) for the PNG image body. The body is only a card
|
||||
/// thumbnail — the manifest keeps the full-resolution source reference — so a
|
||||
/// large avatar is downscaled here to keep the encoded snapshot well under
|
||||
/// `MAX_SNAPSHOT_PNG_BYTES`. Mirrors the frontend SVG rasterizer's 512×512 cap
|
||||
/// in `snapshotAvatarPng.ts`.
|
||||
const MAX_PNG_BODY_EDGE: u32 = 512;
|
||||
|
||||
/// Format discriminator — used for sniffing and validation.
|
||||
pub const FORMAT_DISCRIMINATOR: &str = "buzz-agent-snapshot";
|
||||
|
||||
@@ -328,7 +335,7 @@ pub(crate) fn encode_chunk_payload_png(
|
||||
// there is no avatar or it cannot be decoded.
|
||||
let png_bytes = match avatar_bytes.filter(|bytes| !bytes.is_empty()) {
|
||||
Some(bytes) => {
|
||||
let encoded_avatar = if bytes.starts_with(b"\x89PNG") {
|
||||
let encoded_avatar = if bytes.starts_with(b"\x89PNG") && png_within_body_cap(bytes) {
|
||||
inject_text_chunk(bytes, PNG_CHUNK_KEYWORD, &chunk_text).or_else(|_| {
|
||||
transcode_avatar_to_png_with_text(bytes, PNG_CHUNK_KEYWORD, &chunk_text)
|
||||
})
|
||||
@@ -449,6 +456,11 @@ pub(crate) fn make_png_with_text(keyword: &str, text: &str) -> Result<Vec<u8>, S
|
||||
}
|
||||
|
||||
/// Transcode a decodable avatar to PNG and add the snapshot manifest chunk.
|
||||
///
|
||||
/// The decoded image is downscaled so its longest edge is at most
|
||||
/// `MAX_PNG_BODY_EDGE` before PNG re-encoding. The body is only a card
|
||||
/// thumbnail — this keeps a large source avatar (e.g. a 4K webp) from
|
||||
/// producing a PNG that blows `MAX_SNAPSHOT_PNG_BYTES`.
|
||||
fn transcode_avatar_to_png_with_text(
|
||||
avatar_bytes: &[u8],
|
||||
keyword: &str,
|
||||
@@ -456,6 +468,7 @@ fn transcode_avatar_to_png_with_text(
|
||||
) -> Result<Vec<u8>, String> {
|
||||
let image = image::load_from_memory(avatar_bytes)
|
||||
.map_err(|e| format!("Failed to decode avatar image: {e}"))?;
|
||||
let image = downscale_to_body_cap(image);
|
||||
let mut png_bytes = Vec::new();
|
||||
image
|
||||
.write_to(&mut Cursor::new(&mut png_bytes), image::ImageFormat::Png)
|
||||
@@ -463,6 +476,32 @@ fn transcode_avatar_to_png_with_text(
|
||||
inject_text_chunk(&png_bytes, keyword, text)
|
||||
}
|
||||
|
||||
/// Downscale so the longest edge is at most `MAX_PNG_BODY_EDGE`, preserving
|
||||
/// aspect ratio. Images already within the cap are returned untouched.
|
||||
fn downscale_to_body_cap(image: image::DynamicImage) -> image::DynamicImage {
|
||||
if image.width() <= MAX_PNG_BODY_EDGE && image.height() <= MAX_PNG_BODY_EDGE {
|
||||
return image;
|
||||
}
|
||||
image.resize(
|
||||
MAX_PNG_BODY_EDGE,
|
||||
MAX_PNG_BODY_EDGE,
|
||||
image::imageops::FilterType::Lanczos3,
|
||||
)
|
||||
}
|
||||
|
||||
/// Whether an already-PNG avatar is within the body dimension cap and can be
|
||||
/// carried as-is (via a cheap tEXt-chunk injection) instead of being decoded
|
||||
/// and downscaled. Undecodable headers fall through to the transcode path.
|
||||
fn png_within_body_cap(png_bytes: &[u8]) -> bool {
|
||||
Decoder::new(Cursor::new(png_bytes))
|
||||
.read_info()
|
||||
.map(|reader| {
|
||||
let info = reader.info();
|
||||
info.width <= MAX_PNG_BODY_EDGE && info.height <= MAX_PNG_BODY_EDGE
|
||||
})
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// Inject a tEXt chunk into an existing PNG by re-encoding it.
|
||||
///
|
||||
/// Re-decodes the image data via the `png` crate and writes a fresh PNG with
|
||||
|
||||
@@ -234,7 +234,60 @@ fn png_snapshot_transcodes_jpeg_avatar_into_image_body() {
|
||||
assert_eq!((reader.info().width, reader.info().height), (3, 2));
|
||||
}
|
||||
|
||||
// ── PNG memory parity ─────────────────────────────────────────────────────
|
||||
#[test]
|
||||
fn png_snapshot_downscales_oversize_avatar_under_cap() {
|
||||
// A large avatar (mirrors Gurney's 2764×4096 image that encoded to ~26 MB)
|
||||
// must be downscaled for the PNG body so the snapshot stays under the
|
||||
// 10 MiB cap — while the manifest keeps the untouched source reference.
|
||||
// An already-PNG oversize avatar exercises the `png_within_body_cap` guard
|
||||
// that routes it through the downscaling transcode path.
|
||||
let avatar = image::DynamicImage::ImageRgb8(image::RgbImage::from_fn(2764, 4096, |x, y| {
|
||||
image::Rgb([(x % 256) as u8, (y % 256) as u8, ((x + y) % 256) as u8])
|
||||
}));
|
||||
let mut source_bytes = Vec::new();
|
||||
avatar
|
||||
.write_to(&mut Cursor::new(&mut source_bytes), image::ImageFormat::Png)
|
||||
.unwrap();
|
||||
|
||||
let snapshot = build_snapshot(
|
||||
&minimal_record(),
|
||||
MemoryLevel::None,
|
||||
vec![],
|
||||
Some(&source_bytes),
|
||||
);
|
||||
let png_bytes = encode_snapshot_png(&snapshot, Some(&source_bytes)).unwrap();
|
||||
|
||||
assert!(
|
||||
png_bytes.len()
|
||||
<= super::MAX_PNG_BODY_EDGE as usize * super::MAX_PNG_BODY_EDGE as usize * 4,
|
||||
"downscaled snapshot ({} bytes) must be far under the 10 MiB cap",
|
||||
png_bytes.len()
|
||||
);
|
||||
|
||||
let reader = Decoder::new(Cursor::new(png_bytes)).read_info().unwrap();
|
||||
let (width, height) = (reader.info().width, reader.info().height);
|
||||
assert!(
|
||||
width <= 512 && height <= 512,
|
||||
"body dimensions {width}×{height} must fit the 512px cap"
|
||||
);
|
||||
// Aspect ratio preserved: the longest edge (height) is clamped to the cap.
|
||||
assert_eq!(height, 512, "longest edge should hit the 512px cap");
|
||||
|
||||
// The manifest keeps the untouched full-resolution source reference — only
|
||||
// the PNG body is downscaled. The oversize source bytes exceed the inline
|
||||
// cap, so the manifest falls back to the record's `avatar_url`.
|
||||
let manifest =
|
||||
decode_snapshot_png(&encode_snapshot_png(&snapshot, Some(&source_bytes)).unwrap()).unwrap();
|
||||
assert_eq!(
|
||||
manifest.profile.avatar_url.as_deref(),
|
||||
Some("https://example.com/avatar.png"),
|
||||
"manifest must preserve the untouched source avatar reference"
|
||||
);
|
||||
assert!(
|
||||
manifest.profile.avatar_data_url.is_none(),
|
||||
"oversize source bytes must not be inlined into the manifest"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn png_round_trip_with_core_memory() {
|
||||
|
||||
@@ -143,7 +143,10 @@ function AgentCardViewerContent({
|
||||
toast.success(`Sent ${agentName}'s card.`);
|
||||
closeCardViewer();
|
||||
} else if (sent === false) {
|
||||
toast.error("Couldn’t send the card. Try again.");
|
||||
toast.error(
|
||||
sendController.getCurrentError() ??
|
||||
"Couldn’t send the card. Try again.",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -391,7 +391,10 @@ export function SnapshotShareDialog({
|
||||
toast.success(`Sent a copy of ${displayName}`);
|
||||
onOpenChange(false);
|
||||
} else if (sent === false) {
|
||||
toast.error(`Couldn’t send ${itemLabel}. Try again.`);
|
||||
toast.error(
|
||||
snapshotSendController.getCurrentError() ??
|
||||
`Couldn’t send ${itemLabel}. Try again.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -350,6 +350,12 @@ export type UseSnapshotSendControllerResult = {
|
||||
/** Relay moderation identity to exclude from the people picker. */
|
||||
relaySelfPubkey: string | null;
|
||||
state: SnapshotSendState;
|
||||
/**
|
||||
* Read the latest error synchronously — right after `beginSend` resolves the
|
||||
* render-captured `state.error` is stale until the next commit, so callers
|
||||
* that toast on failure must read through here.
|
||||
*/
|
||||
getCurrentError: () => string | null;
|
||||
/**
|
||||
* Execute destination creation plus prepare → encode → upload → send behind
|
||||
* one concurrency guard. A second call while the first is in flight returns
|
||||
@@ -390,6 +396,15 @@ export function useSnapshotSendController(
|
||||
error: null,
|
||||
});
|
||||
|
||||
// Mirror `state` into a ref so callers can read the latest error
|
||||
// synchronously right after `beginSend` resolves — the render-captured
|
||||
// `state` in their closure is stale until the next render commits.
|
||||
const stateRef = React.useRef(state);
|
||||
const commitState = React.useCallback((next: SnapshotSendState) => {
|
||||
stateRef.current = next;
|
||||
setState(next);
|
||||
}, []);
|
||||
|
||||
// Single-concurrency guard covering the full encode → upload → send action.
|
||||
// Stored in a ref so it survives re-renders without triggering effects.
|
||||
const guardRef = React.useRef(createSendGuard());
|
||||
@@ -417,7 +432,7 @@ export function useSnapshotSendController(
|
||||
checkEligibilityFn: () => checkSendEligibility(queryClient, channelId),
|
||||
uploadFn: (bytes, filename) => uploadMediaBytes(bytes, filename),
|
||||
sendFn: (args) => sendMutation.mutateAsync(args),
|
||||
setStateFn: setState,
|
||||
setStateFn: commitState,
|
||||
buildMessageFn: (descriptor) => {
|
||||
const message = buildOutgoingMessage("", [descriptor]);
|
||||
return attachmentLabel?.trim()
|
||||
@@ -430,15 +445,15 @@ export function useSnapshotSendController(
|
||||
: message;
|
||||
},
|
||||
}),
|
||||
setState,
|
||||
commitState,
|
||||
);
|
||||
}
|
||||
|
||||
const reset = React.useCallback(() => {
|
||||
if (!guardRef.current.inFlight) {
|
||||
setState({ phase: "idle", error: null });
|
||||
commitState({ phase: "idle", error: null });
|
||||
}
|
||||
}, []);
|
||||
}, [commitState]);
|
||||
|
||||
return {
|
||||
isDmSafetyReady:
|
||||
@@ -447,6 +462,7 @@ export function useSnapshotSendController(
|
||||
relaySelfQuery.status === "success"),
|
||||
relaySelfPubkey: relaySelfQuery.data ?? null,
|
||||
state,
|
||||
getCurrentError: () => stateRef.current.error,
|
||||
beginSend,
|
||||
reset,
|
||||
};
|
||||
|
||||
@@ -2287,7 +2287,9 @@ test("people sharing blocks a timeout before encoding or upload", async ({
|
||||
});
|
||||
|
||||
await page.getByTestId("persona-share-send").click();
|
||||
await expect(page.getByText("Couldn’t send agent. Try again.")).toBeVisible();
|
||||
await expect(
|
||||
page.getByText("You are currently timed out and cannot send messages."),
|
||||
).toBeVisible();
|
||||
|
||||
const commands = await readAgentShareCommands(page);
|
||||
expect(
|
||||
@@ -2332,7 +2334,11 @@ test("people sharing rechecks destination eligibility after encoding", async ({
|
||||
return testWindow.__BUZZ_E2E_INVALIDATE_CHANNELS__?.();
|
||||
});
|
||||
|
||||
await expect(page.getByText("Couldn’t send agent. Try again.")).toBeVisible({
|
||||
await expect(
|
||||
page.getByText(
|
||||
"The selected destination is no longer available. Please pick another.",
|
||||
),
|
||||
).toBeVisible({
|
||||
timeout: 5_000,
|
||||
});
|
||||
const commands = await readAgentShareCommands(page);
|
||||
|
||||
Reference in New Issue
Block a user