fix(grok): send images as native ACP blocks - #462

Thanks for this one — I went through it carefully and it holds up really well. Rather than take the capability claim on faith I re-verified the premise live against the grok CLI on my machine, and everything you describe reproduces exactly.

What I measured (grok 1.0.0 — the version registry.rs actually pins)

• initialize still advertises promptCapabilities={image:false, audio:false, embeddedContext:true}, yet a native ACP image block is accepted with no error at all.

• 64×64 PNG, four stripes green/orange/purple/black, asking for the colors top to bottom:

  • native image block → green, orange, black, purple (1409 reasoning tokens, 22s)
  • legacy embedded resource blob → red, white, blue, green, i.e. pure hallucination (5645 reasoning tokens, 70s)

  So the old path wasn't merely failing to show the image — it was burning ~4× the reasoning tokens guessing at it.

• Same image at 6000×6000, native block → green, orange, purple, black, exactly right, and no image_dropped fires (grok downscales silently). Good news for the new error arm: no false-positive banner for merely re-encoded images.

• 1×1 PNG → the real payload is _x.ai/session_notification with {"sessionUpdate":"image_dropped","notes":["Image 1 was dropped before send: too small (1×1); images must be at least 8×8 pixels."]} — byte-identical to your fixture, and that method is in GROK_EXT_UPDATE_METHODS, so the new arm genuinely fires.

• grok's on-disk updates.jsonl records the echo as user_message_chunk / content:{type:"image",...} with _meta.promptIndex:0 on both chunks — precisely what the new parser test asserts.

• Bonus data point: the older 0.2.112 binary also accepts native image blocks and reads the pixels, so the un-version-gated override looks safe backwards as well.

On the design

Putting the promotion at the ConnectionCommand::Prompt choke point is the right call. map_prompt_blocks has a single call site and every producer (chat, work-task engine, delegation, queued drafts) funnels through it, so nothing can smuggle the old shape past it — reencode_images in the work-task engine quietly becomes a no-op for grok once both caps are true, and the connection-level lift covers it anyway. And because every capability consumer reads SessionState.prompt_capabilities (populated from the emitted, already-overridden event), there's no split-brain between what the composer encodes and what the dispatcher sends. Nice side effect too: grok_ext_notification_is_turn_output picks up image_dropped for free, so a drop-only turn stops misreporting as turn_failed_empty.

Checks run on the branch

• cargo clippy --all-targets --features test-utils -- -D warnings — clean
• cargo test --features test-utils — 2455 passed / 0 failed
• cargo clippy --no-default-features --bin codeg-server --lib -- -D warnings — clean
• cargo test --no-default-features --bin codeg-server --lib — 2431 passed / 0 failed

Optional nits (nothing blocking)

1. The banner reads a little doubled: grok's note already opens with "Image 1 was dropped before send…", so the UI ends up showing Image dropped: Image 1 was dropped before send: too small (1×1)….
2. Three comments now describe the old world — message-input-attachments.ts:55-65 still says Grok takes the embedded-resource shape and "the sent payload is unchanged for them", plus the reencode_images doc in work_task/engine.rs and the UserMessageBlock doc at acp/types.rs:474. The paths they describe are still needed for old drafts/transcripts, so it's just wording drift.
3. The PR body and the new doc comment say "verified against grok 1.0.2", but registry.rs pins 1.0.0. Worth naming the pinned version so a future reader doesn't assume the behavior is 1.0.2-only — I checked, 1.0.0 behaves identically.
4. session/load replay runs ext notifications through the same mapper, so an old session replayed via that fallback could resurface a historical image-drop alert. Same shape as the existing auto_compact_failed arm and non-terminal, so cosmetic.
5. image/svg+xml / .ico attachments will now be promoted and then rejected by grok's normalizer with a visible error, instead of silently riding along as a binary dump. Arguably an improvement — just flagging it as a user-visible change.

Looks good to me — I'll merge this shortly. Thanks for chasing the actual wire behavior instead of trusting the advertised capability; that's the part that made this fixable at all.

Pushed 45d04133 — the five nits from my last comment are all closed. One of them turned out to be more than a nit, so here's what happened.

The SVG note was an actual regression

I flagged image/svg+xml last time as "arguably an improvement, just flagging it". That was wrong — I went and measured it:

Grok's validator rejects svg outright (image_dropped: "validate: unsupported or unrecognised image format"), so the model gets nothing. Carried as a resource blob the file lands in the session's assets/ and the model just reads the source — SVG is text, so it wins that round. My earlier "visible error beats silent hallucination" reasoning missed that the old path wasn't failing at all here.

The composer only sees one image capability bit, so it can't make this call per mime. Fix is at the same choke point you already picked: promote_grok_image_resources → normalize_grok_image_blocks, now sorting both directions — promote what grok decodes, demote what it doesn't. The allow-list is grok's own raster set (image/{png,jpeg,gif,webp,bmp,tiff}); I verified png, webp, bmp and tiff each round-trip through the describe sidecar with correct answers, and that anything off the list keeps the pre-PR carriage, so a wrong entry there can only ever be conservative.

The other four

• Message stutter — grok's note already opens with "Image 1 was dropped before send…", so it's shown verbatim now. Only the shapeless reason/message fallbacks get a prefix, and a blank one no longer renders as a bare Image dropped: .
• Stale alerts on replay — this one bit harder than expected once I looked: the session/load replay closure forwards ext notifications unconditionally, so a past session's dropped image (or failed compaction, which predates this PR) came back as a live status-bar alert plus an OS notification, just for opening the conversation. Added grok_ext_notification_is_alert, mirroring your grok_ext_notification_is_turn_output, and the replay now skips those — the typed closure right beside it already drew the same line by forwarding only AvailableCommandsUpdate. Compaction cards still replay.
• Version claim — the doc now cites 1.0.0 (what registry.rs pins) and 0.2.112, and says why it's deliberately not version-gated.
• Stale comments — cleaned up in acp/types.rs, work_task/engine.rs, message-input-attachments.ts, plus a few more that still used Grok as the example of an agent receiving images as embedded resources (use-composer-attachments.ts, message-input.tsx, prompt-draft.ts, from-prompt-blocks.ts) and the two mapper doc comments that only mentioned compaction.

Four new tests cover the demotion (including an upper-cased mime, since the two guards have to read the same string the same way), the allow-list boundary, the note-less drop, and the alert predicate.

Checks

• cargo clippy --all-targets --features test-utils -- -D warnings — clean
• cargo test --features test-utils — 2459 passed / 0 failed
• server mode: clippy clean, cargo test --no-default-features --bin codeg-server --lib — 2435 passed / 0 failed
• npx tsc --noEmit, pnpm eslint src — clean
• pnpm test — 3829 passed; pnpm build — green

Thanks again for the original find — the image:false advertisement being simply untrue is the kind of thing nobody catches without actually poking the wire.