Skip to content

fix(images): correct image media types that disagree with their bytes - #176

Open
ian-de-marcellus wants to merge 1 commit into
anima-research:mainfrom
ian-de-marcellus:fix/image-media-type-sniff
Open

ian-de-marcellus wants to merge 1 commit into
anima-research:mainfrom
ian-de-marcellus:fix/image-media-type-sniff

Conversation

@ian-de-marcellus

Copy link
Copy Markdown
Contributor

Problem. A resident's turns all started failing with a provider 400. The context held an image whose media_type said image/webp but whose bytes were a PNG; it arrived through a surface (Portal) that trusted the uploader's label. The API rejects any request where the label and bytes disagree, and the image stays in context, so every later turn failed the same way until an operator intervened.

Fix. src/image-media-type.ts sniffs the magic bytes (PNG, JPEG, GIF, WebP) of base64 image blocks. correctImageMediaTypes rewrites a disagreeing media type on the compiled messages (compileContext / compileWithInjections), including images nested in tool_result, before they go to the provider, and logs how many blocks it corrected. Unrecognized formats and non-base64 sources pass through untouched; stored history is never modified (new objects only for changed blocks).

Tests. test/image-media-type.test.ts: a PNG labeled image/webp is corrected, including inside tool_result, without mutating the input; correct and unknown images are returned as the same objects. Full suite: 956 pass / 0 fail.

(The surface side is being fixed separately in portal. This makes the framework robust to any surface that mislabels.)

🤖 Generated with Claude Code

A Discord attachment labelled image/webp whose bytes were PNG entered a
resident's history; the provider rejects the whole request on such a mismatch,
so every subsequent turn failed (Fable, 2026-09-24). Correct base64 image
blocks' mediaType from their magic bytes when context is compiled for a
request (including images nested in tool results). Stored history is not
modified; compiled messages get new objects only where something changed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5 Tier: apex

[Medium risk] Corrects mismatched image media types at compile time.

The PR should not merge until mislabeled images in live tool-result continuations are corrected.

Findings

  1. P1 Live tool images bypass correction ▶
Fix with agent prompt
### Issue 1
src/agent.ts:546
When an MCPL tool returns a PNG labeled `image/webp`, the framework passes the image directly to the existing stream for the next inference round. That round does not recompile the context, so this correction never runs on the tool result and the provider can still reject the request. Correct image blocks at the live continuation boundary too.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR adds magic-byte detection and corrects mislabeled base64 images in compiled agent messages without changing stored history.

  • The compiled-request path is covered, but native tool-result images supplied during an existing stream are not.
  • The new tests cover PNG correction but not the other three recognized formats.

Reviews (1) · Last reviewed commit: "fix(images): correct image media types t..."

Comment thread src/agent.ts
);
if (!budget) this.settleRuntimeSettingsTransition();
return result;
return { ...result, messages: correctImageMediaTypes(result.messages) };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Live tool images bypass correction When an MCPL tool returns a PNG labeled image/webp, the framework passes the image directly to the existing stream for the next inference round. That round does not recompile the context, so this correction never runs on the tool result and the provider can still reject the request. Correct image blocks at the live continuation boundary too.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agent.ts
Line: 546

Comment:
**Live tool images bypass correction** When an MCPL tool returns a PNG labeled `image/webp`, the framework passes the image directly to the existing stream for the next inference round. That round does not recompile the context, so this correction never runs on the tool result and the provider can still reject the request. Correct image blocks at the live continuation boundary too.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@slimepriestess slimepriestess left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at head a40a09b, trial-merged onto current main (5498805, post-#175): zero conflicts. Head suite 960/956/0/4 + tsc clean; merged state 973/969/0/4 + tsc clean. The incident path was confirmed live: on membrane 0.5.86 as installed, a wire-capture probe shows a top-level PNG labelled image/webp reaches the Anthropic request body as "media_type":"image/webp" under both the native and XML formatters — membrane's detectImageMediaType exists but is not on the top-level request path (only the adapter's nested tool_result converter sniffs). So this PR is the real fix on the main path, not belt-and-braces, and the changelog wording is accurate.

The wiring was verified by walking both inference entry points (startAgentStream and buildActivationRequest/previewActivation all route through the two patched compile methods; the three direct contextManager.compile() call sites are fork-seeding and API introspection, none provider-bound). Field name checked against membrane's Base64Source.mediaType; kv-unified layoutHash is computed after correction and only compared against its own prior heads, so the deterministic rewrite causes no cache churn.

One blocker — the wiring has no test. Reverting just the two agent.ts hunks leaves the whole suite green (verified on a clean dist: 956 pass / 0 fail); only the helper is pinned. House bar here is that a fix carries a test that goes red when the fix is reverted, at the real entry point. It's ~20 lines with the existing activation-request-sanitize.test.ts harness — here's one, verified red on the reverted wiring (0/1) and green on the PR head (961/957/0 with it in):

// test/image-media-type-entrypoint.test.ts
import { describe, it } from 'node:test';
import assert from 'node:assert/strict';
import { Agent } from '../src/agent.js';
import type { ContextManager } from '@animalabs/context-manager';
import type { Membrane } from '@animalabs/membrane';

const PNG_B64 = 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4////fwAJ+wP9KobjigAAAABJRU5ErkJggg==';

function agentCompiling(messages: unknown[]): Agent {
  const cm = {
    compile: async () => ({ messages, systemInjections: [] }),
  } as unknown as ContextManager;
  return new Agent(
    { name: 'tester', model: 'test-model', systemPrompt: 'sys' },
    cm,
    {} as Membrane,
  );
}

describe('buildActivationRequest media-type correction', () => {
  it('a PNG stored as image/webp reaches the request as image/png', async () => {
    const agent = agentCompiling([
      {
        participant: 'human',
        content: [
          { type: 'text', text: 'look' },
          { type: 'image', source: { type: 'base64', data: PNG_B64, mediaType: 'image/webp' } },
        ],
      },
    ]);
    const request = await agent.buildActivationRequest([]);
    const image = request.messages[0]!.content.find((b) => b.type === 'image') as
      { source: { mediaType: string } };
    assert.equal(image.source.mediaType, 'image/png');
  });
});

Take it as-is (or your own shape) and this is an approve.

Non-blocking:

  1. framework.ts:106 already has sniffImageMediaType(Buffer) with identical magic-byte logic (used by read_image). Worth exporting one sniffer from image-media-type.ts and pointing both at it, so the format lists can't drift apart.
  2. Stored history is never corrected, so the console.error fires on every compile for as long as the image stays in the window. Fine as observability; a once-per-block dedupe (or ingest-side correction) would quiet it if it gets chatty.
  3. The helper only reads camelCase mediaType. A block carrying only snake media_type would get a spurious "correction" (adding mediaType beside the stale field) on every compile. No AF producer of that shape found — noting it for the record.

Context, membrane-side (not this PR's problem, being filed upstream): the top-level no-sniff gap above, and a formatter bug the probe surfaced — NativeFormatter.sanitizeToolResultContent checks snake media_type while normalized blocks carry mediaType, so a camelCase tool_result image in compiled history gets replaced with the media type "undefined" placeholder before any label matters. One consequence for this PR: the tool_result branch of the correction is mostly moot under the native formatter today — correct to have, just not load-bearing there yet.

(Weft — reviewed via Ra's account per the usual convention; this review had an adversarial second read by a peer session, which caught the entry-point gap and supplied the wire probe.)

@slimepriestess

Copy link
Copy Markdown
Contributor

Hi Ian! Sorry this sat so long; the last few weeks were busy. Coming back to it, the good news is that main fixed this one underneath you.

Your incident (a Discord PNG labelled image/webp poisoning every turn) and our first review here led to two membrane issues (antra-tess/membrane#81 and #82). nissa's membrane#84 fixed both at the formatter and adapter level, and it shipped in membrane 0.5.87, which agent-framework has depended on since f032271.

We ran a wire probe against current main (c4ba80b) with none of this PR's code. AF still hands membrane image/webp, and membrane puts image/png on the wire under both the native and XML formatters, top-level and inside a tool_result. On membrane 0.5.86 the same probe shows image/webp, so the probe really does catch the bug. The request-path correction here isn't needed any more, and the entry-point test we asked for on 9/29 no longer has anything to protect.

One thing we found while checking, worth having on record in case the pattern comes up again. membrane's sniffer checks the full PNG signature and GIF87a/GIF89a; this PR's sniffer checks a 4-byte PNG prefix and GIF. Running before membrane, the looser check can relabel a non-image: on this head with membrane 0.5.87, a text block starting GIF… and a block of \x89PNG plus junk went out as image/gif and image/png, where main alone strips them as unsupported. That's the poisoned-request shape again, from the other side. When two layers sniff, the stricter one should get the last word.

So we think this can be closed as superseded. Thanks for chasing the original incident down to the bytes; that's what got it fixed properly upstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants