From 5033842802246d1dbf910580d5de2e26cdc74b77 Mon Sep 17 00:00:00 2001 From: Jack Chen Date: Thu, 3 Sep 2026 12:05:03 +0800 Subject: [PATCH] fix(core): make unsupported-video terminal and validate video URL sources (#566) Follow-up to #548, which introduced VideoBlock. Two gaps were missed when that PR was reviewed. The four adapters that cannot map video threw a bare Error. isRetryableError returns true for any Error without a status, so executeWithRetry treated a deterministic capability gap as transient: it ran the full backoff ladder and rewrote a checkpoint per attempt before failing the same way, and the failure was recorded as retryable. UnsupportedContentBlockError makes that terminal, names the adapter that rejected the block, and carries a code alongside the existing UnsupportedToolResultContentError. VideoBlock is the first top-level content block with a URL source, so it inherited no validation. That source is dereferenced by the provider rather than by OMA, and a file: path or a data URL pasted into the field was forwarded verbatim; an unvalidated media_type could also alter the data URL built for inline bytes. Public entry points now run video sources through the same copyMediaSource rules already applied to tool-result media, so the documented HTTP(S) constraint is enforced rather than aspirational. Provider-private upload handles such as MiniMax's mm_file:// are rejected: supporting them needs a source variant of its own, not an unchecked string. Also from the same review: - estimateTokens charged an inline video the 64-character fixed cost images get, hiding a multi-megabyte payload from every context strategy keyed off the estimate. It is now sized like rich tool-result media, so the attachment that needs compacting is the one that triggers compaction. - stripImageBlocksForSummary handles video too, so it is now stripMediaBlocksForSummary. - The two double casts in openai-common are narrowed to a single seam, and the video_url extension type now records that it is not an OpenAI part. - docs/structured-input.md states the rule the adapters actually follow: OpenAI-shaped adapters forward and let the provider decide, adapters with a typed content union reject locally. Bedrock and Gemini support video upstream, so the gap there is OMA's mapping rather than theirs. --- docs/structured-input.md | 38 +++++++++-- packages/core/src/agent/runner.ts | 8 +-- packages/core/src/errors.ts | 28 ++++++++ packages/core/src/index.ts | 1 + packages/core/src/llm/ai-sdk.ts | 7 +- packages/core/src/llm/anthropic.ts | 8 ++- packages/core/src/llm/bedrock.ts | 8 ++- packages/core/src/llm/gemini.ts | 7 +- packages/core/src/llm/openai-common.ts | 33 ++++++--- packages/core/src/llm/validate.ts | 22 ++++-- packages/core/src/tool/result.ts | 7 +- packages/core/src/types.ts | 14 +++- packages/core/src/utils/tokens.ts | 11 ++- packages/core/tests/ai-sdk-adapter.test.ts | 18 ++++- packages/core/tests/anthropic-adapter.test.ts | 24 ++++++- packages/core/tests/bedrock-adapter.test.ts | 23 ++++++- packages/core/tests/context-strategy.test.ts | 59 ++++++++++++++++ .../core/tests/error-classification.test.ts | 13 ++++ packages/core/tests/errors.test.ts | 23 +++++++ .../tests/gemini-adapter-contract.test.ts | 25 +++++++ packages/core/tests/llm-adapters.test.ts | 48 +++++++++++++ .../core/tests/llm-message-validation.test.ts | 68 +++++++++++++++++++ packages/core/tests/tokens.test.ts | 43 ++++++++++++ 23 files changed, 500 insertions(+), 36 deletions(-) diff --git a/docs/structured-input.md b/docs/structured-input.md index 138e265..4cb3228 100644 --- a/docs/structured-input.md +++ b/docs/structured-input.md @@ -36,13 +36,41 @@ const result = await new OpenMultiAgent().runAgent( ) ``` -OpenAI-compatible adapters (including MiniMax) also accept a `video` block; -its source may be inline base64 bytes or an HTTP(S) URL and is sent as a -`video_url` content part. +A `video` block carries either inline base64 bytes or a remote reference: + +```ts +{ + type: 'video', + source: { type: 'url', media_type: 'video/mp4', url: 'https://example.com/clip.mp4' }, +} +``` + +Unlike an image, a `url` source is fetched by the provider rather than by OMA, +so the reference leaves the process. Both source shapes are therefore validated +at every public entry point under the same rules as tool-result media: an +absolute `http:`/`https:` URL, or raw canonical base64 with a parameterless MIME +type. Anything else — a `file:` path, a provider-private scheme, a data URL +pasted into the `url` field — is rejected with `InvalidMessageError` before an +adapter sees it. Providers that expose their own upload handles for large videos +(MiniMax's `mm_file://`, for instance) are not reachable through this block +today. + +## Which adapters accept which blocks The selected model/provider must support every supplied content block. OMA does -not infer a vision-capability flag or silently remove unsupported blocks; -provider errors follow the normal agent failure path. +not infer a vision-capability flag or silently remove unsupported blocks. Two +things follow from that, and they look different at the call site: + +- **OpenAI-shaped adapters forward and let the provider decide.** A `video` + block becomes a `video_url` content part on the Chat Completions route. + MiniMax accepts it; the other OpenAI-compatible providers reject it, and that + provider error follows the normal agent failure path. +- **Adapters with a typed content union reject locally.** Anthropic, Bedrock, + Gemini, and the AI SDK bridge have no `video` mapping, so they raise + `UnsupportedContentBlockError` before the request is sent. That error is + terminal: retrying cannot add a wire-format capability, so orchestrator retry + skips it rather than spending the backoff ladder on it. Bedrock and Gemini do + have native video support upstream — the gap is OMA's mapping, not theirs. ## API boundaries diff --git a/packages/core/src/agent/runner.ts b/packages/core/src/agent/runner.ts index 6c6dc29..dd9d0fc 100644 --- a/packages/core/src/agent/runner.ts +++ b/packages/core/src/agent/runner.ts @@ -397,14 +397,14 @@ function groupIntoTurns(messages: LLMMessage[]): Turn[] { * inlines the result into a text user-message it ships to the summary model. * For an `ImageBlock` or `VideoBlock`, that serialisation includes the full * base64 payload — a 1MB attachment would balloon the "compression" call by - * ~250k tokens, defeating - * its purpose and risking context-limit rejection. + * ~250k tokens, defeating its purpose and risking context-limit rejection. An + * inline video is larger still. * * The placeholder still tells the summariser that media was present at this * turn, so the produced summary can reference it. Modelled on chef Janitor's * `stripAttachmentsForCompression`. */ -function stripImageBlocksForSummary(messages: LLMMessage[]): LLMMessage[] { +function stripMediaBlocksForSummary(messages: LLMMessage[]): LLMMessage[] { return messages.map((msg) => { if (!msg.content.some(b => b.type === 'image' || b.type === 'video' || (b.type === 'tool_result' && toolResultHasMedia(b.content)))) return msg @@ -842,7 +842,7 @@ export class AgentRunner implements AgentBackend { // The placeholder still flags that media existed at this turn so the // summariser can mention it. recentPortion is untouched (returned to // the caller verbatim, never serialised here). - const oldPortionForSummary = stripImageBlocksForSummary(oldPortion) + const oldPortionForSummary = stripMediaBlocksForSummary(oldPortion) const oldSignature = oldPortionForSummary.map(m => this.serializeMessage(m)).join('\n') if (this.summarizeCache !== null && this.summarizeCache.oldSignature === oldSignature) { const derived: SyntheticPrefixRecord | undefined = rewrite ? {} : undefined diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts index 7b8f9c4..2c7e189 100644 --- a/packages/core/src/errors.ts +++ b/packages/core/src/errors.ts @@ -228,6 +228,33 @@ export class UnsupportedToolResultContentError extends Error { } } +/** + * Raised before an SDK request when a built-in adapter has no wire mapping for + * a whole model-visible content block. + * + * Terminal for the same reason {@link UnsupportedToolResultContentError} is: + * the block and the adapter are both fixed for the attempt, so a retry re-runs + * the identical conversion and fails identically. A bare `Error` here would + * instead fall through {@link isRetryableError}'s conservative default and + * spend the whole backoff ladder — plus a checkpoint rewrite per attempt — on + * a capability gap that cannot resolve itself. + */ +export class UnsupportedContentBlockError extends Error { + readonly code = 'UNSUPPORTED_CONTENT_BLOCK' + + constructor( + readonly provider: string, + readonly blockType: ContentBlock['type'], + detail?: string, + ) { + super( + `${provider} cannot represent the "${blockType}" content block` + + (detail ? `: ${detail}` : ''), + ) + this.name = 'UnsupportedContentBlockError' + } +} + /** * Raised before an adapter call when `enforceLineage` is on and a model-visible * block cannot name the journal event it came from. @@ -302,6 +329,7 @@ export function isRetryableError(error: unknown): boolean { if (error instanceof UnsupportedToolCallError) return false if (error instanceof EgressPolicyError) return false if (error instanceof UnsupportedToolResultContentError) return false + if (error instanceof UnsupportedContentBlockError) return false // A lineage gap is a property of the conversation, not of the transport: // the same request would fail the same way on every attempt. if (error instanceof JournalLineageError) return false diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 332e5c2..622461a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -201,6 +201,7 @@ export { LLMCallTimeoutError, UnsupportedToolCallError, UnsupportedToolResultContentError, + UnsupportedContentBlockError, RoutingDeclarationRequiredError, RoutingProfilerFailedError, RoutingTimeoutError, diff --git a/packages/core/src/llm/ai-sdk.ts b/packages/core/src/llm/ai-sdk.ts index 659a626..af9502c 100644 --- a/packages/core/src/llm/ai-sdk.ts +++ b/packages/core/src/llm/ai-sdk.ts @@ -22,6 +22,7 @@ import type { ToolUseBlock, } from '../types.js' import { normalizeFinishReason } from './openai-common.js' +import { UnsupportedContentBlockError } from '../errors.js' import { assertValidMessages } from './validate.js' import { reasoningBlockToInlineText, @@ -178,7 +179,11 @@ export function llmMessagesToAiSdkModelMessages( mediaType: block.source.media_type, }) } else if (block.type === 'video') { - throw new Error('This adapter does not support video content blocks') + throw new UnsupportedContentBlockError( + 'the AI SDK bridge', + 'video', + 'OMA does not map video for this adapter yet', + ) } } if (userParts.length > 0) out.push({ role: 'user', content: userParts } as ModelMessage) diff --git a/packages/core/src/llm/anthropic.ts b/packages/core/src/llm/anthropic.ts index f9aa361..1bb6320 100644 --- a/packages/core/src/llm/anthropic.ts +++ b/packages/core/src/llm/anthropic.ts @@ -64,7 +64,7 @@ import { } from './reasoning-fallback.js' import { assertValidMessages } from './validate.js' import { createEgressFetch } from './egress.js' -import { UnsupportedToolResultContentError } from '../errors.js' +import { UnsupportedContentBlockError, UnsupportedToolResultContentError } from '../errors.js' import { toolResultContentParts } from '../tool/result.js' // --------------------------------------------------------------------------- @@ -222,7 +222,11 @@ function toAnthropicContentBlockParams( return [param] } case 'video': - throw new Error('This adapter does not support video content blocks') + throw new UnsupportedContentBlockError( + 'Anthropic Messages', + 'video', + 'OMA does not map video for this adapter yet', + ) default: { // Exhaustiveness guard — TypeScript will flag this at compile time if a // new variant is added to ContentBlock without updating this switch. diff --git a/packages/core/src/llm/bedrock.ts b/packages/core/src/llm/bedrock.ts index 2d8019d..f71d561 100644 --- a/packages/core/src/llm/bedrock.ts +++ b/packages/core/src/llm/bedrock.ts @@ -61,7 +61,7 @@ import { type ReasoningOutboundOptions, } from './reasoning-fallback.js' import { assertValidMessages } from './validate.js' -import { UnsupportedToolResultContentError } from '../errors.js' +import { UnsupportedContentBlockError, UnsupportedToolResultContentError } from '../errors.js' import { toolResultContentParts } from '../tool/result.js' // --------------------------------------------------------------------------- @@ -216,7 +216,11 @@ function toBedrockContentBlock( } case 'video': - throw new Error('This adapter does not support video content blocks') + throw new UnsupportedContentBlockError( + 'Amazon Bedrock Converse', + 'video', + 'OMA does not map video for this adapter yet', + ) default: { const _exhaustive: never = block diff --git a/packages/core/src/llm/gemini.ts b/packages/core/src/llm/gemini.ts index e061f2c..3672d54 100644 --- a/packages/core/src/llm/gemini.ts +++ b/packages/core/src/llm/gemini.ts @@ -56,6 +56,7 @@ import { resolveReasoningOutboundMaxChars, type ReasoningOutboundOptions, } from './reasoning-fallback.js' +import { UnsupportedContentBlockError } from '../errors.js' import { assertValidMessages } from './validate.js' import { toolResultContentParts, toolResultText } from '../tool/result.js' @@ -222,7 +223,11 @@ function toGeminiContents( break case 'video': - throw new Error('This adapter does not support video content blocks') + throw new UnsupportedContentBlockError( + 'Google Gemini', + 'video', + 'OMA does not map video for this adapter yet', + ) default: { const _exhaustive: never = block diff --git a/packages/core/src/llm/openai-common.ts b/packages/core/src/llm/openai-common.ts index 1521a2a..22a617a 100644 --- a/packages/core/src/llm/openai-common.ts +++ b/packages/core/src/llm/openai-common.ts @@ -96,11 +96,34 @@ function hasToolResults(msg: LLMMessage): boolean { return msg.content.some((b) => b.type === 'tool_result') } +/** + * A user content part, plus the one extension we emit beyond the SDK's union. + * + * `video_url` is not an OpenAI part. MiniMax accepts it on its + * OpenAI-compatible Chat Completions route, so it is emitted for every + * OpenAI-shaped provider and rejected by the ones that do not understand it — + * the same "the provider owns block support" contract documented for images in + * `docs/structured-input.md`. + */ type OpenAIUserContentPart = OpenAI.Chat.ChatCompletionContentPart | { type: 'video_url' video_url: { url: string } } +/** + * Widen built parts to the SDK's content type at the single assignment seam. + * + * The cast is unavoidable while `video_url` sits outside + * `ChatCompletionContentPart`, but keeping it here means every push site stays + * checked against {@link OpenAIUserContentPart} instead of being cast away + * along with it. + */ +function toSDKUserContent( + parts: OpenAIUserContentPart[], +): ChatCompletionUserMessageParam['content'] { + return parts as ChatCompletionUserMessageParam['content'] +} + function toOpenAIToolAttachmentPart(part: Exclude): OpenAIUserContentPart { if (part.type === 'image') { return { @@ -211,10 +234,7 @@ export function toOpenAIMessages( const nonToolBlocks = msg.content.filter((b) => b.type !== 'tool_result') if (attachmentParts.length > 0) { attachmentParts.push(...toOpenAIUserContentParts({ role: 'user', content: nonToolBlocks })) - result.push({ - role: 'user', - content: attachmentParts as unknown as ChatCompletionUserMessageParam['content'], - }) + result.push({ role: 'user', content: toSDKUserContent(attachmentParts) }) } else if (nonToolBlocks.length > 0) { result.push(toOpenAIUserMessage({ role: 'user', content: nonToolBlocks })) } @@ -234,10 +254,7 @@ function toOpenAIUserMessage(msg: LLMMessage): ChatCompletionUserMessageParam { return { role: 'user', content: msg.content[0].text } } - return { - role: 'user', - content: toOpenAIUserContentParts(msg) as unknown as ChatCompletionUserMessageParam['content'], - } + return { role: 'user', content: toSDKUserContent(toOpenAIUserContentParts(msg)) } } function toOpenAIUserContentParts(msg: LLMMessage): OpenAIUserContentPart[] { diff --git a/packages/core/src/llm/validate.ts b/packages/core/src/llm/validate.ts index d889d29..5c6201c 100644 --- a/packages/core/src/llm/validate.ts +++ b/packages/core/src/llm/validate.ts @@ -13,7 +13,7 @@ import type { LLMMessage } from '../types.js' import { InvalidMessageError } from '../errors.js' -import { copyToolResultContent } from '../tool/result.js' +import { copyMediaSource, copyToolResultContent } from '../tool/result.js' function describeType(value: unknown): string { if (value === null) return 'null' @@ -24,10 +24,14 @@ function describeType(value: unknown): string { /** * Assert that `messages` satisfies the shared shape every adapter relies on: * an array of `{ role, content }` objects whose `content` is an array of content - * blocks. Rich `tool_result` content receives full nested validation because a - * malformed media part otherwise fails differently across provider SDKs. - * Other block internals remain the responsibility of their existing adapters. - * Invalid input is rejected rather than coerced or silently reshaped. + * blocks. Rich `tool_result` content and `video` blocks receive full nested + * validation because a malformed media source otherwise fails differently + * across provider SDKs — or, for a `url` source, is handed to the provider to + * dereference on our behalf. Both carry the same source shape and go through + * the same {@link copyMediaSource} rules, so an absolute HTTP(S) reference or + * canonical base64 is the only thing that reaches an adapter. Remaining block + * internals stay the responsibility of their existing adapters. Invalid input + * is rejected rather than coerced or silently reshaped. */ export function assertValidMessages(messages: readonly LLMMessage[]): void { if (!Array.isArray(messages)) { @@ -85,6 +89,14 @@ export function assertValidMessages(messages: readonly LLMMessage[]): void { ) } } + + if (record['type'] === 'video') { + try { + copyMediaSource(record['source'], `messages[${i}].content[${j}].source`) + } catch (error) { + throw new InvalidMessageError(error instanceof Error ? error.message : String(error)) + } + } } } } diff --git a/packages/core/src/tool/result.ts b/packages/core/src/tool/result.ts index 37a5662..e41e0cd 100644 --- a/packages/core/src/tool/result.ts +++ b/packages/core/src/tool/result.ts @@ -22,7 +22,12 @@ function assertNonEmptyString(value: unknown, path: string): asserts value is st } } -function copyMediaSource(value: unknown, path: string): ToolResultMediaSource { +/** + * Validate and copy a media source: an absolute HTTP(S) reference or raw + * canonical base64 with a parameterless MIME type. Shared by tool-result + * media and by `video` content blocks, which carry the same source shape. + */ +export function copyMediaSource(value: unknown, path: string): ToolResultMediaSource { if (value === null || typeof value !== 'object' || Array.isArray(value)) { throw new TypeError(`${path} must be a media source object`) } diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index f69558a..8c21704 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -157,18 +157,30 @@ export interface ImageBlock { } } -/** A video passed to a model as inline bytes or a remote URL. */ +/** + * A video passed to a model as inline bytes or a remote reference. + * + * Unlike {@link ImageBlock}, a `url` source is dereferenced by the provider + * rather than by OMA, so the reference leaves the process. Public entry points + * validate it the same way tool-result media is validated: absolute HTTP(S) + * only, and raw canonical base64 with a parameterless MIME type for inline + * bytes. Provider-private reference schemes are not accepted. + */ export interface VideoBlock { readonly type: 'video' readonly source: | { readonly type: 'base64' + /** IANA media type for the encoded bytes. */ readonly media_type: string + /** Raw base64 data without a data-URL prefix. */ readonly data: string } | { readonly type: 'url' + /** IANA media type expected at the reference. */ readonly media_type: string + /** Absolute HTTP(S) URL. Provider/model support still varies. */ readonly url: string } } diff --git a/packages/core/src/utils/tokens.ts b/packages/core/src/utils/tokens.ts index c069699..dc5e679 100644 --- a/packages/core/src/utils/tokens.ts +++ b/packages/core/src/utils/tokens.ts @@ -22,8 +22,15 @@ export function estimateTokens(messages: LLMMessage[]): number { // Account for non-text payloads with a small fixed cost. chars += 64 } else if (block.type === 'video') { - // Video bytes are opaque to the lightweight estimator. - chars += 64 + // Deliberately not the fixed cost images get. An inline video runs to + // tens of megabytes, and charging it 64 characters leaves the context + // strategies blind to the payload that dominates every request: they + // would never compact the one block worth compacting. Sized the same + // way `toolResultContentSize` sizes rich tool-result media. + chars += block.source.type === 'base64' + ? block.source.data.length + : block.source.url.length + chars += block.source.media_type.length + 32 } } } diff --git a/packages/core/tests/ai-sdk-adapter.test.ts b/packages/core/tests/ai-sdk-adapter.test.ts index 35595f9..0373b86 100644 --- a/packages/core/tests/ai-sdk-adapter.test.ts +++ b/packages/core/tests/ai-sdk-adapter.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import type { LanguageModel } from 'ai' +import { UnsupportedContentBlockError, isRetryableError } from '../src/errors.js' const generateTextMock = vi.fn() const streamTextMock = vi.fn() @@ -117,7 +118,7 @@ describe('llmMessagesToAiSdkModelMessages', () => { }) it('rejects video blocks instead of silently dropping them', () => { - expect(() => llmMessagesToAiSdkModelMessages([ + const convert = () => llmMessagesToAiSdkModelMessages([ { role: 'user', content: [ @@ -127,7 +128,20 @@ describe('llmMessagesToAiSdkModelMessages', () => { }, ], }, - ])).toThrow('This adapter does not support video content blocks') + ]) + + expect(convert).toThrow(UnsupportedContentBlockError) + // Typed, not a bare Error: the run is failed permanently rather than + // retried through the backoff ladder on a capability that cannot appear. + try { + convert() + expect.unreachable('conversion should have thrown') + } catch (error) { + expect(error).toBeInstanceOf(UnsupportedContentBlockError) + expect((error as UnsupportedContentBlockError).blockType).toBe('video') + expect((error as UnsupportedContentBlockError).provider).toContain('AI SDK') + expect(isRetryableError(error)).toBe(false) + } }) it('does not serialize opaque redacted reasoning payloads', () => { diff --git a/packages/core/tests/anthropic-adapter.test.ts b/packages/core/tests/anthropic-adapter.test.ts index 2ae9692..52d3c46 100644 --- a/packages/core/tests/anthropic-adapter.test.ts +++ b/packages/core/tests/anthropic-adapter.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { textMsg, toolUseMsg, toolResultMsg, imageMsg, chatOpts, toolDef, collectEvents } from './helpers/llm-fixtures.js' import type { LLMMessage, LLMResponse, ReasoningBlock, StreamEvent, ToolUseBlock } from '../src/types.js' -import { UnsupportedToolResultContentError } from '../src/errors.js' +import { UnsupportedContentBlockError, UnsupportedToolResultContentError, isRetryableError } from '../src/errors.js' // --------------------------------------------------------------------------- // Mock the Anthropic SDK @@ -206,6 +206,28 @@ describe('AnthropicAdapter', () => { expect(mockCreate).not.toHaveBeenCalled() }) + it('rejects video blocks with a terminal error before sending', async () => { + mockCreate.mockResolvedValue(makeAnthropicResponse()) + + const call = adapter.chat( + [{ + role: 'user', + content: [{ + type: 'video', + source: { type: 'base64', media_type: 'video/mp4', data: 'dmlkZW8=' }, + }], + }], + chatOpts(), + ) + + await expect(call).rejects.toThrow(UnsupportedContentBlockError) + expect(mockCreate).not.toHaveBeenCalled() + await call.catch((error: unknown) => { + expect(isRetryableError(error)).toBe(false) + expect((error as UnsupportedContentBlockError).blockType).toBe('video') + }) + }) + it('converts image blocks to Anthropic format', async () => { mockCreate.mockResolvedValue(makeAnthropicResponse()) diff --git a/packages/core/tests/bedrock-adapter.test.ts b/packages/core/tests/bedrock-adapter.test.ts index 1324583..96eea82 100644 --- a/packages/core/tests/bedrock-adapter.test.ts +++ b/packages/core/tests/bedrock-adapter.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { textMsg, toolUseMsg, toolResultMsg, chatOpts, toolDef, collectEvents } from './helpers/llm-fixtures.js' import type { LLMResponse, StreamEvent, ToolUseBlock } from '../src/types.js' -import { UnsupportedToolResultContentError } from '../src/errors.js' +import { UnsupportedContentBlockError, UnsupportedToolResultContentError, isRetryableError } from '../src/errors.js' // --------------------------------------------------------------------------- // Mock @aws-sdk/client-bedrock-runtime @@ -292,6 +292,27 @@ describe('BedrockAdapter', () => { )).rejects.toThrow(UnsupportedToolResultContentError) expect(mockSend).not.toHaveBeenCalled() }) + + it('rejects video blocks with a terminal error before sending', async () => { + mockSend.mockResolvedValue(makeConverseResponse()) + + const call = adapter.chat( + [{ + role: 'user', + content: [{ + type: 'video', + source: { type: 'base64', media_type: 'video/mp4', data: 'dmlkZW8=' }, + }], + }], + chatOpts(), + ) + + await expect(call).rejects.toThrow(UnsupportedContentBlockError) + expect(mockSend).not.toHaveBeenCalled() + await call.catch((error: unknown) => { + expect(isRetryableError(error)).toBe(false) + }) + }) }) // ========================================================================= diff --git a/packages/core/tests/context-strategy.test.ts b/packages/core/tests/context-strategy.test.ts index f5823e2..4743c28 100644 --- a/packages/core/tests/context-strategy.test.ts +++ b/packages/core/tests/context-strategy.test.ts @@ -356,6 +356,65 @@ describe('AgentRunner contextStrategy', () => { expect(promptText.toLowerCase().includes('image')).toBe(true) }) + it('summarize strategy strips video attachments, and the video alone can trigger it', async () => { + // The image case above needs text FILLER to cross maxTokens because an + // ImageBlock is charged a flat 64 chars. A VideoBlock is charged its real + // payload, so the attachment that actually needs compacting is the thing + // that triggers compaction — no filler turns required. + const FAKE_VIDEO_DATA = 'A'.repeat(100_000) + const calls: { messages: LLMMessage[]; options: LLMChatOptions }[] = [] + let callCount = 0 + const adapter: LLMAdapter = { + name: 'mock', + async chat(messages, options) { + calls.push({ messages, options }) + callCount++ + if (callCount === 1) { + return toolUseResponse('echo', { message: 'x' }) + } + return textResponse('done') + }, + async *stream() { + /* unused */ + }, + } + const { registry, executor } = buildRegistryAndExecutor() + const runner = new AgentRunner(adapter, registry, executor, { + model: 'mock-model', + allowedTools: ['echo'], + maxTurns: 8, + contextStrategy: { type: 'summarize', maxTokens: 100 }, + }) + + const history: LLMMessage[] = [ + { role: 'user', content: [{ type: 'text', text: 'start' }] }, + { role: 'assistant', content: [{ type: 'text', text: 'ok' }] }, + { role: 'user', content: [ + { type: 'text', text: 'analyze this clip' }, + { type: 'video', source: { type: 'base64', media_type: 'video/mp4', data: FAKE_VIDEO_DATA } }, + ] }, + { role: 'assistant', content: [{ type: 'text', text: 'looking' }] }, + { role: 'user', content: [{ type: 'text', text: 'and now' }] }, + { role: 'assistant', content: [{ type: 'text', text: 'done thinking' }] }, + ] + + await runner.run(history) + + const summaryCall = calls.find( + c => c.options.tools === undefined && c.messages.length === 1, + ) + expect(summaryCall).toBeDefined() + + const promptText = summaryCall!.messages[0]!.content + .filter((b): b is import('../src/types.js').TextBlock => b.type === 'text') + .map(b => b.text) + .join('\n') + + expect(promptText.includes(FAKE_VIDEO_DATA)).toBe(false) + expect(promptText.length).toBeLessThan(10_000) + expect(promptText).toContain('[video: video/mp4]') + }) + it('does not drop turns when context strategy shrinks array size', async () => { // The core bug from #152: if the strategy replaces the array with fewer messages than it started with, // the old `slice()` logic would incorrectly drop newly generated turns. diff --git a/packages/core/tests/error-classification.test.ts b/packages/core/tests/error-classification.test.ts index 65cb2c7..b8761d9 100644 --- a/packages/core/tests/error-classification.test.ts +++ b/packages/core/tests/error-classification.test.ts @@ -9,6 +9,7 @@ import { StructuredOutputValidationError, UnsupportedToolCallError, UnsupportedToolResultContentError, + UnsupportedContentBlockError, } from '../src/errors.js' import { classifyRunFailure } from '../src/observability/status.js' @@ -59,6 +60,18 @@ describe('isRetryableError', () => { }]))).toBe(false) }) + it('classifies an unsupported content block as terminal', () => { + // A block the adapter has no mapping for cannot become mappable on a + // second attempt, so retrying only spends the backoff ladder and a + // checkpoint rewrite per attempt before failing the same way. + const error = new UnsupportedContentBlockError('Google Gemini', 'video') + expect(isRetryableError(error)).toBe(false) + expect(error.code).toBe('UNSUPPORTED_CONTENT_BLOCK') + expect(error.message).toContain('Google Gemini') + expect(error.message).toContain('video') + expect(classifyRunFailure(error).errorInfo.retryable).toBe(false) + }) + it('classifies a per-call timeout as retryable', () => { expect(isRetryableError(new LLMCallTimeoutError(1000, 'agent'))).toBe(true) }) diff --git a/packages/core/tests/errors.test.ts b/packages/core/tests/errors.test.ts index 40806c4..cf30d83 100644 --- a/packages/core/tests/errors.test.ts +++ b/packages/core/tests/errors.test.ts @@ -6,6 +6,7 @@ import { StructuredOutputValidationError, UnsupportedToolCallError, UnsupportedToolResultContentError, + UnsupportedContentBlockError, } from '../src/errors.js' describe('TokenBudgetExceededError', () => { @@ -107,6 +108,28 @@ describe('UnsupportedToolResultContentError', () => { }) }) +describe('UnsupportedContentBlockError', () => { + it('identifies the provider and the block it cannot map', () => { + const err = new UnsupportedContentBlockError( + 'Anthropic Messages', + 'video', + 'OMA does not map video for this adapter yet', + ) + expect(err.name).toBe('UnsupportedContentBlockError') + expect(err.code).toBe('UNSUPPORTED_CONTENT_BLOCK') + expect(err.provider).toBe('Anthropic Messages') + expect(err.blockType).toBe('video') + expect(err.message).toContain('Anthropic Messages') + expect(err.message).toContain('video') + expect(err.message).toContain('OMA does not map video for this adapter yet') + }) + + it('omits the detail clause when none is given', () => { + const err = new UnsupportedContentBlockError('Google Gemini', 'video') + expect(err.message).toBe('Google Gemini cannot represent the "video" content block') + }) +}) + describe('LLMCallTimeoutError', () => { it('sets .name to LLMCallTimeoutError', () => { const err = new LLMCallTimeoutError(30_000, 'agent-1') diff --git a/packages/core/tests/gemini-adapter-contract.test.ts b/packages/core/tests/gemini-adapter-contract.test.ts index 2c83f9f..ffa8918 100644 --- a/packages/core/tests/gemini-adapter-contract.test.ts +++ b/packages/core/tests/gemini-adapter-contract.test.ts @@ -23,6 +23,7 @@ vi.mock('@google/genai', () => ({ })) import { GeminiAdapter } from '../src/llm/gemini.js' +import { UnsupportedContentBlockError, isRetryableError } from '../src/errors.js' // --------------------------------------------------------------------------- // Helpers @@ -184,6 +185,30 @@ describe('GeminiAdapter (contract)', () => { data: 'base64data', }) }) + + it('rejects video blocks with a terminal error before sending', async () => { + // Gemini has native video support upstream; the gap is OMA's mapping. + // Either way the caller gets a typed terminal failure, not a retry loop. + mockGenerateContent.mockResolvedValue(makeGeminiResponse([{ text: 'ok' }])) + + const call = adapter.chat( + [{ + role: 'user', + content: [{ + type: 'video', + source: { type: 'base64', media_type: 'video/mp4', data: 'dmlkZW8=' }, + }], + }], + chatOpts(), + ) + + await expect(call).rejects.toThrow(UnsupportedContentBlockError) + expect(mockGenerateContent).not.toHaveBeenCalled() + await call.catch((error: unknown) => { + expect(isRetryableError(error)).toBe(false) + expect((error as UnsupportedContentBlockError).provider).toContain('Gemini') + }) + }) }) // ========================================================================= diff --git a/packages/core/tests/llm-adapters.test.ts b/packages/core/tests/llm-adapters.test.ts index d0fc981..f439054 100644 --- a/packages/core/tests/llm-adapters.test.ts +++ b/packages/core/tests/llm-adapters.test.ts @@ -397,6 +397,54 @@ describe('toOpenAIMessages', () => { { type: 'video_url', video_url: { url: 'data:video/mp4;base64,YWJj' } }, ]) }) + + it('passes a video url source through without rebuilding it as a data URL', () => { + const msgs: LLMMessage[] = [{ + role: 'user', + content: [{ + type: 'video', + source: { + type: 'url', + media_type: 'video/mp4', + url: 'https://example.com/clip.mp4?token=abc', + }, + }], + }] + + const content = (toOpenAIMessages(msgs)[0] as any).content + expect(content).toEqual([ + { type: 'video_url', video_url: { url: 'https://example.com/clip.mp4?token=abc' } }, + ]) + }) + + it('emits video alongside tool-result attachments in one user message', () => { + const msgs: LLMMessage[] = [{ + role: 'user', + content: [ + { + type: 'tool_result', + tool_use_id: 'call-1', + content: [{ + type: 'image', + source: { type: 'base64', media_type: 'image/png', data: 'aW1n' }, + }], + }, + { + type: 'video', + source: { type: 'url', media_type: 'video/mp4', url: 'https://example.com/clip.mp4' }, + }, + ], + }] + + const result = toOpenAIMessages(msgs) + // tool message first, then the attachment-carrying user message. + expect((result[0] as any).role).toBe('tool') + const userContent = (result[1] as any).content + expect(userContent).toContainEqual({ + type: 'video_url', + video_url: { url: 'https://example.com/clip.mp4' }, + }) + }) }) describe('fromOpenAICompletion', () => { diff --git a/packages/core/tests/llm-message-validation.test.ts b/packages/core/tests/llm-message-validation.test.ts index 5d51203..71aa529 100644 --- a/packages/core/tests/llm-message-validation.test.ts +++ b/packages/core/tests/llm-message-validation.test.ts @@ -75,6 +75,74 @@ describe('assertValidMessages', () => { expect(() => assertValidMessages(invalid)).toThrow(/must use HTTP or HTTPS/) }) + it('accepts both video source shapes', () => { + expect(() => assertValidMessages([{ + role: 'user', + content: [ + { + type: 'video', + source: { type: 'url', media_type: 'video/mp4', url: 'https://example.com/clip.mp4' }, + }, + { + type: 'video', + source: { type: 'base64', media_type: 'video/mp4', data: 'dmlkZW8=' }, + }, + ], + }])).not.toThrow() + }) + + it('rejects a video url the provider would dereference off HTTP(S)', () => { + // A `url` video source is fetched by the provider, not by OMA, so an + // unchecked scheme would hand the provider whatever the caller supplied. + const invalid = [{ + role: 'user', + content: [{ + type: 'video', + source: { type: 'url', media_type: 'video/mp4', url: 'file:///etc/passwd' }, + }], + }] as unknown as LLMMessage[] + + expect(() => assertValidMessages(invalid)).toThrow(InvalidMessageError) + expect(() => assertValidMessages(invalid)).toThrow(/must use HTTP or HTTPS/) + }) + + it('rejects a provider-private video reference scheme', () => { + const invalid = [{ + role: 'user', + content: [{ + type: 'video', + source: { type: 'url', media_type: 'video/mp4', url: 'mm_file://abc123' }, + }], + }] as unknown as LLMMessage[] + + expect(() => assertValidMessages(invalid)).toThrow(InvalidMessageError) + expect(() => assertValidMessages(invalid)).toThrow(/must be an absolute HTTP\(S\) URL/) + }) + + it('rejects a video media type that would corrupt the data URL', () => { + const invalid = [{ + role: 'user', + content: [{ + type: 'video', + source: { type: 'base64', media_type: 'video/mp4,evil', data: 'dmlkZW8=' }, + }], + }] as unknown as LLMMessage[] + + expect(() => assertValidMessages(invalid)).toThrow(/MIME type without parameters/) + }) + + it('rejects non-canonical base64 video bytes', () => { + const invalid = [{ + role: 'user', + content: [{ + type: 'video', + source: { type: 'base64', media_type: 'video/mp4', data: 'not base64!' }, + }], + }] as unknown as LLMMessage[] + + expect(() => assertValidMessages(invalid)).toThrow(/must contain raw base64 data/) + }) + it('requires error tool results to remain text-only', () => { const invalid = [{ role: 'user', diff --git a/packages/core/tests/tokens.test.ts b/packages/core/tests/tokens.test.ts index 97c2e7d..f7d82a1 100644 --- a/packages/core/tests/tokens.test.ts +++ b/packages/core/tests/tokens.test.ts @@ -13,4 +13,47 @@ describe('estimateTokens', () => { expect(estimateTokens(messages)).toBe(100) }) + + it('sizes an inline video by its payload, not by the image fixed cost', () => { + // The fixed 64-character charge images get would hide a multi-megabyte + // video from every context strategy that keys off this estimate. + const data = 'A'.repeat(400_000) + const messages: LLMMessage[] = [ + { + role: 'user', + content: [{ + type: 'video', + source: { type: 'base64', media_type: 'video/mp4', data }, + }], + }, + ] + + const image: LLMMessage[] = [ + { + role: 'user', + content: [{ + type: 'image', + source: { type: 'base64', media_type: 'image/png', data }, + }], + }, + ] + + expect(estimateTokens(messages)).toBe(Math.ceil((400_000 + 'video/mp4'.length + 32) / 4)) + expect(estimateTokens(messages)).toBeGreaterThan(estimateTokens(image) * 1000) + }) + + it('sizes a video url source by the reference rather than the fetched bytes', () => { + const url = `https://example.com/${'a'.repeat(80)}.mp4` + const messages: LLMMessage[] = [ + { + role: 'user', + content: [{ + type: 'video', + source: { type: 'url', media_type: 'video/mp4', url }, + }], + }, + ] + + expect(estimateTokens(messages)).toBe(Math.ceil((url.length + 'video/mp4'.length + 32) / 4)) + }) })