Repository navigation
fix(images): correct image media types that disagree with their bytes - #176
ian-de-marcellus wants to merge 1 commit into
Conversation
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>
|
| ); | ||
| if (!budget) this.settleRuntimeSettingsTransition(); | ||
| return result; | ||
| return { ...result, messages: correctImageMediaTypes(result.messages) }; |
There was a problem hiding this 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.
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
left a comment
There was a problem hiding this comment.
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:
framework.ts:106already hassniffImageMediaType(Buffer)with identical magic-byte logic (used byread_image). Worth exporting one sniffer fromimage-media-type.tsand pointing both at it, so the format lists can't drift apart.- Stored history is never corrected, so the
console.errorfires 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. - The helper only reads camelCase
mediaType. A block carrying only snakemedia_typewould get a spurious "correction" (addingmediaTypebeside 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.)
|
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 We ran a wire probe against current main (c4ba80b) with none of this PR's code. AF still hands membrane 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 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. |
Problem. A resident's turns all started failing with a provider 400. The context held an image whose
media_typesaidimage/webpbut 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.tssniffs the magic bytes (PNG, JPEG, GIF, WebP) of base64 image blocks.correctImageMediaTypesrewrites a disagreeing media type on the compiled messages (compileContext/compileWithInjections), including images nested intool_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 labeledimage/webpis corrected, including insidetool_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