Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/refuse-unsupported-image-formats.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@moonshot-ai/kimi-code": patch
---

Stop unsupported image formats (AVIF, BMP, TIFF, ICO, …) from breaking sessions at every entry point — including remote image URLs and images mislabeled by a tool — and recover an already-stuck session by dropping the offending image and retrying, so one such image can no longer make every later request fail.
20 changes: 13 additions & 7 deletions packages/acp-adapter/src/convert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import {
log,
buildImageCompressionCaption,
compressBase64ForModel,
gateImageFormatParts,
parseImageDataUrl,
persistOriginalImage,
type PromptPart,
type TelemetryClient,
Expand All @@ -16,6 +18,9 @@ import { isHideOutputMarker } from './marker';
* Convert an array of ACP {@link ContentBlock}s into the SDK's
* {@link PromptPart} array.
*
* Image parts are built from the client-declared MIME verbatim; run the
* result through {@link compressPromptImageParts} before submitting so
* unsupported formats are dropped and MIME aliases canonicalized.
*/
export function acpBlocksToPromptParts(
blocks: readonly ContentBlock[],
Expand Down Expand Up @@ -81,6 +86,13 @@ export function acpBlocksToPromptParts(
* server's upload-time step. Best effort: a part that cannot be compressed is
* passed through unchanged.
*
* The format gate (`gateImageFormatParts`) runs first: parts whose MIME is
* outside the provider-accepted set are never forwarded — the part is
* dropped and a text notice stands in, so one unsupported image cannot
* poison the session history; accepted MIME aliases (`image/jpg`,
* case/whitespace variants) are rewritten to the canonical form strict
* provider whitelists require.
*
* Compression is never silent: a re-encoded image gains a caption text part
* immediately before it stating what the original was, and the original bytes
* are persisted (into `originalsDir` — typically the session's
Expand All @@ -102,7 +114,7 @@ export async function compressPromptImageParts(
} = {},
): Promise<PromptPart[]> {
const out: PromptPart[] = [];
for (const part of parts) {
for (const part of gateImageFormatParts(parts) as PromptPart[]) {
if (part.type === 'image_url') {
const parsed = parseImageDataUrl(part.imageUrl.url);
if (parsed !== null) {
Expand Down Expand Up @@ -150,12 +162,6 @@ export async function compressPromptImageParts(
return out;
}

function parseImageDataUrl(url: string): { mimeType: string; base64: string } | null {
const match = /^data:([^;,]+);base64,(.*)$/s.exec(url);
if (match === null) return null;
return { mimeType: match[1]!, base64: match[2]! };
}

/**
* Minimum-viable XML-attribute escaping for prompt-embedded resource
* wrappers. The output is consumed by an LLM, not parsed by a canonical
Expand Down
28 changes: 28 additions & 0 deletions packages/acp-adapter/test/convert.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,4 +426,32 @@ describe('compressPromptImageParts', () => {
const compressed = await compressPromptImageParts(parts);
expect(compressed).toEqual(parts);
});

it('replaces an image the provider cannot accept with a text notice', async () => {
// An AVIF image must never reach the session history — the provider
// rejects it and every later request would fail. A notice stands in.
const parts = acpBlocksToPromptParts([
textBlock('look at this'),
imageBlock(Buffer.from([1, 2, 3]).toString('base64'), 'image/avif'),
]);
const compressed = await compressPromptImageParts(parts);

expect(compressed).toHaveLength(2);
expect(compressed[0]).toEqual({ type: 'text', text: 'look at this' });
const notice = compressed[1];
if (notice?.type !== 'text') throw new Error('expected a text notice');
expect(notice.text).toContain('image/avif');
});

it('forwards accepted MIME aliases in canonical form', async () => {
// Strict provider whitelists reject the raw `image/jpg` alias — the part
// must land in the session with the canonical MIME.
const base64 = Buffer.from([1, 2, 3]).toString('base64');
const parts = acpBlocksToPromptParts([imageBlock(base64, 'image/jpg')]);
const compressed = await compressPromptImageParts(parts);

expect(compressed).toEqual([
{ type: 'image_url', imageUrl: { url: `data:image/jpeg;base64,${base64}` } },
]);
});
});
25 changes: 15 additions & 10 deletions packages/agent-core/src/agent/compaction/full.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
APIRequestTooLargeError,
APIStatusError,
createUserMessage,
isImageFormatError,
} from '@moonshot-ai/kosong';

import type { Agent } from '..';
Expand Down Expand Up @@ -468,16 +469,20 @@ export class FullCompaction {
summary = extractCompactionSummary(response);
break;
} catch (error) {
// A request-body-size rejection (HTTP 413) is first retried with
// media parts replaced by text markers: accumulated base64 payloads
// are the usual culprit, and a text summary does not need them —
// the conversation already narrates what was seen, and the
// ReadMediaFile `<image path="...">` text wrapper survives. Only
// the summarizer input copy is rewritten; the real history keeps
// its media. A 413 after the strip (or with no media to strip)
// falls through to the overflow shrink below — dropping oldest
// messages shrinks the body too.
if (error instanceof APIRequestTooLargeError && !mediaStripAttempted) {
// A request-body-size rejection (HTTP 413) or an image-format
// rejection is first retried with media parts replaced by text
// markers: accumulated base64 payloads are the usual 413 culprit,
// a poisoned image the format-rejection culprit, and a text summary
// needs neither — the conversation already narrates what was seen,
// and the ReadMediaFile `<image path="...">` text wrapper survives.
// Only the summarizer input copy is rewritten; the real history
// keeps its media. A rejection after the strip (or with no media to
// strip) falls through to the overflow shrink below for a 413, and
// propagates for a format error — dropping oldest messages cannot
// fix a poisoned image's format.
const mediaRejected =
error instanceof APIRequestTooLargeError || isImageFormatError(error);
if (mediaRejected && !mediaStripAttempted) {
mediaStripAttempted = true;
const stripped = replaceMediaPartsWithMarkers(historyForModel);
if (stripped !== historyForModel) {
Expand Down
11 changes: 11 additions & 0 deletions packages/agent-core/src/agent/context/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
import {
degradeOlderMediaParts,
MEDIA_DEGRADE_KEEP_RECENT,
MEDIA_STRIPPED_PLACEHOLDERS,
project,
type ProjectionAnomaly,
type ProjectOptions,
Expand Down Expand Up @@ -501,6 +502,16 @@ export class ContextMemory {
return degradeOlderMediaParts(this.messages, MEDIA_DEGRADE_KEEP_RECENT);
}

// Fallback projection for the image-format resend: EVERY media part
// replaced by a text marker. Unlike the 413 case (too MUCH media), a
// format rejection means at least one image is poison and the error never
// says which — only a full strip guarantees the resend carries none.
// Purely read-side, and only used after the provider already rejected an
// image; see the image-format fallback in `turn-step`.
get mediaStrippedMessages(): Message[] {
return degradeOlderMediaParts(this.messages, 0, MEDIA_STRIPPED_PLACEHOLDERS);
}

useProjectedHistoryFrom(source: ContextMemory): void {
this.clear();
this.pushHistory(...trimTrailingOpenToolExchange(source.project(source.history)));
Expand Down
38 changes: 29 additions & 9 deletions packages/agent-core/src/agent/context/projector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -482,26 +482,46 @@ const MEDIA_DEGRADED_PLACEHOLDERS = {
'[video omitted: dropped to fit the provider request size limit; re-read the file to view it]',
} as const;

/**
* Markers for the media-stripped resend after the provider rejected an
* image's FORMAT (not its size): the image marker points the model at
* re-reading the file, whose refusal carries per-OS conversion instructions;
* audio/video are collateral of the full strip and say so.
*/
export const MEDIA_STRIPPED_PLACEHOLDERS = {
image_url:
'[image omitted: the provider rejected this image; re-read the file for conversion instructions]',
audio_url:
'[audio omitted: dropped along with a rejected image; re-read the file to hear it]',
video_url:
'[video omitted: dropped along with a rejected image; re-read the file to view it]',
} as const;

type MediaPlaceholderSet = typeof MEDIA_DEGRADED_PLACEHOLDERS | typeof MEDIA_STRIPPED_PLACEHOLDERS;

function isDegradableMediaPart(
part: ContentPart,
): part is ContentPart & { type: keyof typeof MEDIA_DEGRADED_PLACEHOLDERS } {
): part is ContentPart & { type: keyof MediaPlaceholderSet } {
return part.type in MEDIA_DEGRADED_PLACEHOLDERS;
}

/**
* Replace all but the `keepRecent` most recent media parts with deterministic
* text markers. This is the media-degraded projection used to resend a request
* the provider rejected as too large (HTTP 413 on accumulated base64 media):
* a purely read-side transform — the underlying history is left untouched —
* that trades old pixels for bytes while the surrounding text (including
* ReadMediaFile's `<image path="...">` wrapper) survives, so the model can
* re-read any file it still needs. Untouched messages are returned by
* reference, and when nothing needs degrading the input array itself is
* returned.
* the provider rejected as too large (HTTP 413 on accumulated base64 media)
* and — with `keepRecent = 0` and `MEDIA_STRIPPED_PLACEHOLDERS` — the resend
* after an image-format rejection, where the poisoned image could be anywhere
* and only a full strip guarantees a clean request. A purely read-side
* transform — the underlying history is left untouched — that trades pixels
* for deliverability while the surrounding text (including ReadMediaFile's
* `<image path="...">` wrapper) survives, so the model can re-read any file
* it still needs. Untouched messages are returned by reference, and when
* nothing needs degrading the input array itself is returned.
*/
export function degradeOlderMediaParts(
messages: readonly Message[],
keepRecent: number,
placeholders: MediaPlaceholderSet = MEDIA_DEGRADED_PLACEHOLDERS,
): Message[] {
const mediaCount = messages.reduce(
(count, message) => count + message.content.filter(isDegradableMediaPart).length,
Expand All @@ -515,7 +535,7 @@ export function degradeOlderMediaParts(
const content = message.content.map((part): ContentPart => {
if (toDegrade === 0 || !isDegradableMediaPart(part)) return part;
toDegrade -= 1;
return { type: 'text', text: MEDIA_DEGRADED_PLACEHOLDERS[part.type] };
return { type: 'text', text: placeholders[part.type] };
});
return { ...message, content };
});
Expand Down
6 changes: 3 additions & 3 deletions packages/agent-core/src/agent/records/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,9 @@ export interface AgentRecordEvents {
messageCount: number;
turnStep?: string;
attempt?: string;
/** Set when this request is a fallback resend (strict rebuild or
* media-degraded rebuild). */
projection?: 'strict' | 'media-degraded';
/** Set when this request is a fallback resend (strict rebuild,
* media-degraded rebuild, or media-stripped rebuild). */
projection?: 'strict' | 'media-degraded' | 'media-stripped';
/** Compaction only: messages dropped so far by overflow/empty shrinking. */
droppedCount?: number;
};
Expand Down
19 changes: 14 additions & 5 deletions packages/agent-core/src/agent/turn/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
} from '../../loop/index';
import type { AgentEvent, TurnEndedEvent, TurnEndReason } from '../../rpc';
import type { TelemetryPropertyValue } from '../../telemetry';
import { gateImageFormatParts } from '../../tools/support/image-compress';
import { abortable, isUserCancellation, userCancellationReason } from '../../utils/abort';
import { USER_PROMPT_ORIGIN, type PromptOrigin } from '../context';
import { renderUserPromptHookBlockResult, renderUserPromptHookResult } from '../../session/hooks';
Expand Down Expand Up @@ -134,31 +135,38 @@

// Returns the new turnId, or null if the turn was marked as resuming.
prompt(input: readonly ContentPart[], origin: PromptOrigin = USER_PROMPT_ORIGIN): number | null {
// The last funnel before a prompt lands in the session history: images
// in formats providers reject (AVIF, HEIC, …) become text notices here,
// so no caller — the SDK/RPC prompt path included — can poison the
// session. Upstream ingestion points already gate; this is the backstop.
const gated = gateImageFormatParts(input);
this.agent.records.logRecord({
type: 'turn.prompt',
input,
input: gated,
origin,
});
return this.launch(input, origin);
return this.launch(gated, origin);
}

// Returns the new turnId, or null if the input was buffered as a steer
// message or the turn was marked as resuming.
steer(input: readonly ContentPart[], origin: PromptOrigin = USER_PROMPT_ORIGIN): number | null {
// Same format gate as prompt() — steer input enters the history too.
const gated = gateImageFormatParts(input);
this.agent.records.logRecord({
type: 'turn.steer',
input,
input: gated,
origin,
});
// Buffer while a turn is active OR a manual compaction holds the context;
// `onCompactionFinished` replays the buffer once compaction's full lifecycle
// (summary + reinjection) is done. Returning null means "buffered" — which is
// exactly what fire-and-forget callers (background notifications, cron) assume.
if (this.activeTurn || this.agent.fullCompaction.isCompacting) {
this.steerBuffer.push({ input, origin });
this.steerBuffer.push({ input: gated, origin });
return null;
}
return this.launch(input, origin);
return this.launch(gated, origin);
}

retry(trigger?: string): number | null {
Expand Down Expand Up @@ -725,6 +733,7 @@
buildMessages: () => this.agent.context.messages,
buildMessagesStrict: () => this.agent.context.strictMessages,
buildMessagesMediaDegraded: () => this.agent.context.mediaDegradedMessages,
buildMessagesMediaStripped: () => this.agent.context.mediaStrippedMessages,
dispatchEvent: this.buildDispatchEvent(turnId),
// Re-read per step (not snapshotted per turn) so a select_tools load
// is dispatchable on the very next step of the same turn.
Expand Down Expand Up @@ -858,39 +867,39 @@
authorizeToolExecution: async (ctx) => {
return this.agent.permission.beforeToolCall(ctx);
},
finalizeToolResult: async (ctx) => {
// Resolve dedup BEFORE firing the PostToolUse hook so same-step
// dups (whose ctx.result is the dedup placeholder) report the
// original's real outcome, not an empty success.
const finalResult = await deduper.finalizeResult(
ctx.toolCall.id,
ctx.toolCall.name,
ctx.args,
ctx.result,
);
const { isError, output } = finalResult;
const event = isError === true ? 'PostToolUseFailure' : 'PostToolUse';
void this.agent.hooks?.fireAndForgetTrigger(event, {
matcherValue: ctx.toolCall.name,
inputData: {
toolName: ctx.toolCall.name,
toolInput: toolInputRecord(ctx.args),
toolCallId: ctx.toolCall.id,
error: isError === true ? toKimiErrorPayload(toolOutputText(output)) : undefined,
toolOutput: isError === true ? undefined : toolOutputText(output).slice(0, 2000),
},
});
const modelResult = await budgetToolResultForModel({
homedir: this.agent.homedir,
toolName: ctx.toolCall.name,
toolCallId: ctx.toolCall.id,
result: finalResult,
});
if (isTerminalUpdateGoalResult(ctx.toolCall.name, ctx.args, finalResult)) {
goalOutcomeToolResultPending = true;
}
return modelResult;
},

Check warning on line 902 in packages/agent-core/src/agent/turn/index.ts

View workflow job for this annotation

GitHub Actions / lint

eslint(no-loop-func)

Function declared in a loop contains unsafe references to variable(s)
},
});

Expand Down
12 changes: 12 additions & 0 deletions packages/agent-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,24 @@ export {
compressImageContentParts,
cropImageForModel,
formatByteSize,
gateImageFormatParts,
resolveMaxImageEdgePx,
resolveReadImageByteBudget,
IMAGE_BYTE_BUDGET,
MAX_IMAGE_EDGE_PX,
READ_IMAGE_BYTE_BUDGET,
} from './tools/support/image-compress';
export {
MODEL_ACCEPTED_IMAGE_MIMES,
buildImageConversionGuidance,
buildUnsupportedImageNotice,
decodeBase64Prefix,
isModelAcceptedImageMime,
normalizeImageMime,
parseImageDataUrl,
resolveEffectiveImageMime,
unsupportedImageMimeFromUrl,
} from './tools/support/image-format-policy';
export { ImageLimits } from './tools/support/image-limits';
export type {
CompressAnnotateOptions,
Expand Down
7 changes: 4 additions & 3 deletions packages/agent-core/src/loop/llm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@ export interface LLMRequestLogFields {
/** Request purpose; absent means a regular loop step. */
readonly kind?: 'loop' | 'compaction';
/** Set when the messages are a fallback resend projection: the strict
* wire-compliant rebuild, or the media-degraded rebuild after a
* request-too-large rejection. */
readonly projection?: 'strict' | 'media-degraded';
* wire-compliant rebuild, the media-degraded rebuild after a
* request-too-large rejection, or the media-stripped rebuild after an
* image-format rejection. */
readonly projection?: 'strict' | 'media-degraded' | 'media-stripped';
/** Compaction only: messages dropped so far by overflow/empty shrinking. */
readonly droppedCount?: number;
}
Expand Down
23 changes: 20 additions & 3 deletions packages/agent-core/src/loop/run-turn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ export interface RunTurnInput {
* so each step does not pay a fresh rejection.
*/
readonly buildMessagesMediaDegraded?: LoopMessageBuilder | undefined;
/**
* Optional media-stripped rebuild of the request messages: EVERY media
* part replaced by a text marker. Used to resend once after the provider
* rejects an image's format (see `executeLoopStep`); the poisoned image
* could be anywhere in the history, so only a full strip guarantees a
* clean request. After a successful stripped resend, later steps of the
* same turn build from this projection directly.
*/
readonly buildMessagesMediaStripped?: LoopMessageBuilder | undefined;
readonly dispatchEvent: LoopEventDispatcher;
readonly tools?: readonly ExecutableTool[] | undefined;
/**
Expand Down Expand Up @@ -84,6 +93,7 @@ export async function runTurn(input: RunTurnInput): Promise<TurnResult> {
buildMessages,
buildMessagesStrict,
buildMessagesMediaDegraded,
buildMessagesMediaStripped,
dispatchEvent,
tools,
buildTools,
Expand All @@ -104,6 +114,9 @@ export async function runTurn(input: RunTurnInput): Promise<TurnResult> {
// history is deterministically over the provider's body-size limit, so
// rebuilding it would pay a fresh rejection on every step.
let mediaDegradedActive = false;
// Same for the media-stripped resend after an image-format rejection: the
// poison is still in the full history, so later steps stay stripped.
let mediaStrippedActive = false;
const recordStepUsage = async (
stepUsage: TokenUsage,
): Promise<RecordStepUsageResult | void> => {
Expand All @@ -125,11 +138,14 @@ export async function runTurn(input: RunTurnInput): Promise<TurnResult> {
turnId,
signal,
buildMessages:
mediaDegradedActive && buildMessagesMediaDegraded !== undefined
? buildMessagesMediaDegraded
: buildMessages,
mediaStrippedActive && buildMessagesMediaStripped !== undefined
? buildMessagesMediaStripped
: mediaDegradedActive && buildMessagesMediaDegraded !== undefined
? buildMessagesMediaDegraded
: buildMessages,
buildMessagesStrict,
buildMessagesMediaDegraded,
buildMessagesMediaStripped,
dispatchEvent,
llm,
tools,
Expand All @@ -147,6 +163,7 @@ export async function runTurn(input: RunTurnInput): Promise<TurnResult> {
});
activeStep = undefined;
mediaDegradedActive = mediaDegradedActive || stepResult.mediaDegradedResendUsed === true;
mediaStrippedActive = mediaStrippedActive || stepResult.mediaStrippedResendUsed === true;

if (stepResult.stopReason === 'tool_use') {
continue;
Expand Down
Loading
Loading