Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
27 changes: 23 additions & 4 deletions src/services/api/errors.openaiCompatibility.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ test('maps endpoint_not_found category markers to actionable setup guidance', ()
expect(text).toContain('/v1')
})

test('vision_not_supported shows image-specific guidance for remote host', () => {
test('vision_not_supported shows image-specific guidance (issue #1421 canonical message)', () => {
const error = APIError.generate(
404,
undefined,
Expand All @@ -40,12 +40,31 @@ test('vision_not_supported shows image-specific guidance for remote host', () =>
const text = getFirstText(message)

expect(message.isApiErrorMessage).toBe(true)
expect(text).toContain('images')
expect(text).toContain('mimo-v2.5-pro')
expect(text).toContain('opengateway.gitlawb.com')
expect(text).toContain('image')
expect(text).toContain('does not support')
// The command is `/model` in interactive sessions and `--model` in
// non-interactive (test/SDK) sessions — both forms are intentional.
expect(text).toMatch(/(\/model|--model)/)
expect(text).not.toContain('OPENAI_BASE_URL')
})

test('vision_not_supported from Xiaomi Mimo 400 "text is not set" uses the same canonical message (issue #1421)', () => {
const error = APIError.generate(
400,
undefined,
'OpenAI API error 400: {"error":{"code":"400","message":"Param Incorrect","param":"`text` is not set"}} [openai_category=vision_not_supported,host=api.xiaomimimo.com] Hint: The provider rejected an image-bearing request because it lacked a text part.',
new Headers(),
)

const message = getAssistantMessageFromError(error, 'mimo-v2.5-pro')
const text = getFirstText(message)

expect(message.isApiErrorMessage).toBe(true)
expect(text).toContain('image')
expect(text).toContain('does not support')
expect(text).toMatch(/(\/model|--model)/)
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.

test('endpoint_not_found from a remote host shows the actual host, not Ollama (issue #926)', () => {
const error = APIError.generate(
404,
Expand Down
28 changes: 27 additions & 1 deletion src/services/api/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,9 @@ function mapOpenAICompatibilityFailureToAssistantMessage(options: {

case 'vision_not_supported':
return createAssistantAPIErrorMessage({
content: `The provider at ${options.host} returned 404 for a request containing images. The model (${options.model}) may not support image/vision inputs. Try removing images from your message, or ${switchCmd} to a vision-capable model.`,
content: getVisionNotSupportedErrorMessage(),
error: 'invalid_request',
errorDetails: stripOpenAICompatibilityMetadata(options.rawMessage),
})

case 'model_not_found':
Expand Down Expand Up @@ -399,6 +400,31 @@ export function getRequestTooLargeErrorMessage(): string {
? `Request too large (${limits}). Try with a smaller file.`
: `Request too large (${limits}). Double press esc to go back and try with a smaller file.`
}

const VISION_NOT_SUPPORTED_MESSAGE_PREFIX =
'The active model does not support image/vision inputs. The provider rejected the request because it contained an image. Remove the image, or'

export function getVisionNotSupportedErrorMessages(): string[] {
return [
`${VISION_NOT_SUPPORTED_MESSAGE_PREFIX} switch to a vision-capable model with --model.`,
`${VISION_NOT_SUPPORTED_MESSAGE_PREFIX} run /model to switch to a vision-capable model.`,
]
}

/**
* Canonical message for the `vision_not_supported` OpenAI compatibility
* failure (issue #1421). Returned as a stable string so that
* `normalizeMessagesForAPI`'s `errorToBlockTypes` map can self-heal existing
* transcripts by stripping `image` blocks from the preceding user message
* on resume.
*/
export function getVisionNotSupportedErrorMessage(): string {
const [nonInteractiveMessage, interactiveMessage] =
getVisionNotSupportedErrorMessages()
return getIsNonInteractiveSession()
? nonInteractiveMessage!
: interactiveMessage!
}
export const OAUTH_ORG_NOT_ALLOWED_ERROR_MESSAGE =
'Your account does not have access to OpenClaude. Please run /login.'

Expand Down
33 changes: 33 additions & 0 deletions src/services/api/openaiErrorClassification.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,39 @@ test('classifies 404 with images as vision_not_supported', () => {
expect(failure.hint).toContain('image')
})

test('classifies 400 with "text is not set" + images as vision_not_supported (issue #1421)', () => {
const failure = classifyOpenAIHttpFailure({
status: 400,
body: '{"error":{"code":"400","message":"Param Incorrect","param":"`text` is not set","type":""}}',
hasImages: true,
})

expect(failure.category).toBe('vision_not_supported')
expect(failure.retryable).toBe(false)
expect(failure.hint).toContain('image')
})

test('classifies 400 with "text is required" + images as vision_not_supported (issue #1421)', () => {
const failure = classifyOpenAIHttpFailure({
status: 400,
body: '{"error":{"message":"text parameter is required"}}',
hasImages: true,
})

expect(failure.category).toBe('vision_not_supported')
})

test('does not classify 400 with "text is not set" when request has no images', () => {
const failure = classifyOpenAIHttpFailure({
status: 400,
body: '{"error":{"message":"text is not set"}}',
hasImages: false,
})

// Without images, "text is not set" is unrelated to vision capability.
expect(failure.category).not.toBe('vision_not_supported')
})

test('classifies context-overflow responses', () => {
const failure = classifyOpenAIHttpFailure({
status: 500,
Expand Down
48 changes: 48 additions & 0 deletions src/services/api/openaiErrorClassification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,34 @@ function isMalformedProviderResponse(body: string): boolean {
)
}

/**
* Detect provider messages that complain about a missing/required `text`
* field on an otherwise image-bearing payload. Xiaomi Mimo surfaces this as
* `{"error":{"code":"400","message":"Param Incorrect","param":"`text` is not set"}}`
* (with backticks around `text`) when a `role: "tool"` message carries
* images but no text part. Other OpenAI-compatible providers may phrase
* it differently — match liberally.
*
* Only meaningful when `hasImages` is true (we never want this branch to fire
* for text-only requests, which legitimately lack a text field on vision-only
* payloads).
*/
function isMissingTextPartMessage(body: string): boolean {
// Strip backticks so `\`text\` is not set` matches the same patterns as
// `text is not set` — the Xiaomi Mimo 400 body wraps `text` in backticks
// inside the `param` field, which trips naive substring matching.
const lower = body.toLowerCase().replace(/`/g, '')
return (
lower.includes('text is not set') ||
lower.includes('text is required') ||
lower.includes('text parameter is required') ||
lower.includes('text parameter is missing') ||
lower.includes('missing text') ||
lower.includes('"param":"text"') ||
lower.includes('"param": "text"')
)
}

function isModelNotFoundMessage(body: string): boolean {
const lower = body.toLowerCase()
return (
Expand Down Expand Up @@ -344,6 +372,26 @@ export function classifyOpenAIHttpFailure(options: {
}
}

// Xiaomi Mimo and similar OpenAI-compatible providers reject image-bearing
// `role: "tool"` messages with a 400 carrying `text is not set` instead of
// a 404. Classify the same way as the 404 + hasImages branch so the user
// gets actionable guidance rather than the raw API error (issue #1421).
if (
options.status === 400 &&
options.hasImages &&
isMissingTextPartMessage(body)
) {
return {
source: 'http',
category: 'vision_not_supported',
retryable: false,
status: options.status,
message: body,
requestUrl: options.url,
hint: 'The provider rejected a request containing an image (likely a tool result) because it did not include a text part. The model may not support image/vision inputs.',
}
}

if (options.status === 404) {
const isRemote = hostname !== null && !isLocalHost
return {
Expand Down
4 changes: 4 additions & 0 deletions src/services/api/openaiShim.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1950,7 +1950,11 @@ test('preserves image tool results as placeholders in follow-up requests', async
text?: string
image_url?: { url: string }
}>
// Issue #1421: image-only tool results now get a placeholder text part
// prepended so OpenAI-compatible providers that require a `text` field on
// `role: "tool"` messages (e.g. Xiaomi Mimo) don't 400 with "text is not set".
expect(parts).toEqual([
{ type: 'text', text: 'Image attached.' },
{
type: 'image_url',
image_url: { url: 'data:image/png;base64,ZmFrZQ==' },
Expand Down
7 changes: 6 additions & 1 deletion src/services/api/openaiShim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,12 @@ function convertToolResultContent(
parts.unshift({ type: 'text', text: 'Error:' })
}

return parts
// Defense in depth (issue #1421): some OpenAI-compatible providers (e.g.
// Xiaomi Mimo) reject `role: "tool"` messages whose `content` is image-only
// with a 400 "text is not set". Prepend a placeholder text part so the
// payload always carries a text component alongside any images, mirroring
// the existing behavior for user-role messages.
return ensureTextPartForImageContent(parts)
}

function convertContentBlocks(
Expand Down
22 changes: 22 additions & 0 deletions src/tools/FileReadTool/FileReadTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import {
isPDFSupported,
parsePDFPageRange,
} from '../../utils/pdfUtils.js'
import { checkVisionCapabilityForFile } from '../../utils/visionUtils.js'
import {
checkReadPermissionForTool,
matchingRuleForInput,
Expand Down Expand Up @@ -547,6 +548,27 @@ export const FileReadTool = buildTool({
}
}

// Vision-capability gate: refuse image reads when the active model
// explicitly lacks `supportsVision` (e.g. Xiaomi Mimo V2.5 Pro / Flash,
// Llama, Mistral). Returning early surfaces a clear `<tool_use_error>`
// to the model so it can pivot to a text-based approach (Bash `file`,
// `identify`, OCR) or `/model` to switch to a vision-capable model —
// instead of producing an image-only tool result that the provider
// rejects with a generic 400 (issue #1421).
const visionCheck = checkVisionCapabilityForFile(
fullFilePath,
toolUseContext.options.mainLoopModel,
{
baseUrl:
toolUseContext.options.providerOverride?.baseURL ??
process.env.OPENAI_BASE_URL ??
process.env.OPENAI_API_BASE,
},
)
if (visionCheck.result === false) {
return visionCheck
}

// Block specific device files that would hang (infinite output or blocking input).
// This is a path-based check with no I/O — safe special files like /dev/null are allowed.
if (isBlockedDevicePath(fullFilePath)) {
Expand Down
26 changes: 26 additions & 0 deletions src/tools/FileReadTool/prompt.vision.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { describe, expect, test } from 'bun:test'

import { renderPromptTemplate } from './prompt.js'

describe('renderPromptTemplate — vision sentence (issue #1421)', () => {
test('includes the image-reading sentence when the active model supports vision (default Claude)', () => {
const rendered = renderPromptTemplate(
'- Results are returned using cat -n format, with line numbers starting at 1',
'',
'',
)

expect(rendered).toContain('This tool allows Claude Code to read images')
})

test('always includes the Jupyter notebook sentence and the directory-listing hint', () => {
const rendered = renderPromptTemplate(
'- Results are returned using cat -n format, with line numbers starting at 1',
'',
'',
)

expect(rendered).toContain('Jupyter notebooks')
expect(rendered).toContain('not directories')
})
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
12 changes: 12 additions & 0 deletions src/utils/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
getPdfPasswordProtectedErrorMessage,
getPdfTooLargeErrorMessage,
getRequestTooLargeErrorMessage,
getVisionNotSupportedErrorMessages,
} from '../services/api/errors.js'
import type { AnyObject, Progress } from '../Tool.js'
import { isConnectorTextBlock } from '../types/connectorText.js'
Expand Down Expand Up @@ -2069,6 +2070,17 @@ export function normalizeMessagesForAPI(
[getPdfInvalidErrorMessage()]: new Set(['document']),
[getImageTooLargeErrorMessage()]: new Set(['image']),
[getRequestTooLargeErrorMessage()]: new Set(['document', 'image']),
// Issue #1421: existing transcripts poisoned by a 400 "text is not set"
// (Xiaomi Mimo + non-vision model) carry an image-only tool_result that
// would re-trigger the same 400 on every retry. Match the canonical
// message so `normalizeMessagesForAPI` strips the `image` blocks from
// the preceding tool_result user message on resume / next turn.
...Object.fromEntries(
getVisionNotSupportedErrorMessages().map(message => [
message,
new Set(['image']),
]),
),
}

// Walk the reordered messages to build a targeted strip map:
Expand Down
Loading