Skip to content

Commit 7be9dce

Browse files
authored
fix: vision handling for OpenAI-compatible models (Gitlawb#1663)
* Fix vision handling for OpenAI-compatible models Add route-aware vision capability checks for image reads so registered non-vision models get an actionable refusal before sending image content. Classify provider-side image/text errors as canonical vision_not_supported responses, preserve image-only tool results for OpenAI-compatible shims, and strip rejected images from retry messages. Add focused coverage for Xiaomi MiMo/OpenGateway route collisions, canonical errors, shim image handling, and the Read prompt. * Address vision review findings Move the Read tool vision gate before the UNC no-I/O early return so UNC image paths cannot bypass non-vision model checks. Add direct FileReadTool.validateInput coverage for non-vision denials, provider override/env precedence, and UNC image paths. Add the missing OPENAI_BASE_URL exclusion assertion for the Xiaomi MiMo canonical error path. * Isolate vision gate tests from provider env Clear OPENAI_BASE_URL and OPENAI_API_BASE before each FileReadTool vision-gate test so full-suite provider tests cannot leak route state into these cases. * Fix vision gate test full-suite isolation Import FileReadTool and prompt with a cache-busted module id so compact.test's process-global mock cannot replace validateInput during test:full. Invoke validateInput directly instead of optional chaining, matching the review finding and making missing exports fail clearly. * Lock vision prompt env mutations Acquire the shared mutation lock before mutating OPENAI_BASE_URL and OPENAI_API_BASE in the FileReadTool vision prompt tests, and release it after restoring the environment.
1 parent bac74aa commit 7be9dce

11 files changed

Lines changed: 705 additions & 14 deletions

src/services/api/errors.openaiCompatibility.test.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ test('maps endpoint_not_found category markers to actionable setup guidance', ()
2828
expect(text).toContain('/v1')
2929
})
3030

31-
test('vision_not_supported shows image-specific guidance for remote host', () => {
31+
test('vision_not_supported shows image-specific guidance (issue #1421 canonical message)', () => {
3232
const error = APIError.generate(
3333
404,
3434
undefined,
@@ -40,9 +40,29 @@ test('vision_not_supported shows image-specific guidance for remote host', () =>
4040
const text = getFirstText(message)
4141

4242
expect(message.isApiErrorMessage).toBe(true)
43-
expect(text).toContain('images')
44-
expect(text).toContain('mimo-v2.5-pro')
45-
expect(text).toContain('opengateway.gitlawb.com')
43+
expect(text).toContain('image')
44+
expect(text).toContain('does not support')
45+
// The command is `/model` in interactive sessions and `--model` in
46+
// non-interactive (test/SDK) sessions — both forms are intentional.
47+
expect(text).toMatch(/(\/model|--model)/)
48+
expect(text).not.toContain('OPENAI_BASE_URL')
49+
})
50+
51+
test('vision_not_supported from Xiaomi Mimo 400 "text is not set" uses the same canonical message (issue #1421)', () => {
52+
const error = APIError.generate(
53+
400,
54+
undefined,
55+
'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.',
56+
new Headers(),
57+
)
58+
59+
const message = getAssistantMessageFromError(error, 'mimo-v2.5-pro')
60+
const text = getFirstText(message)
61+
62+
expect(message.isApiErrorMessage).toBe(true)
63+
expect(text).toContain('image')
64+
expect(text).toContain('does not support')
65+
expect(text).toMatch(/(\/model|--model)/)
4666
expect(text).not.toContain('OPENAI_BASE_URL')
4767
})
4868

src/services/api/errors.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,9 @@ function mapOpenAICompatibilityFailureToAssistantMessage(options: {
9999

100100
case 'vision_not_supported':
101101
return createAssistantAPIErrorMessage({
102-
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.`,
102+
content: getVisionNotSupportedErrorMessage(),
103103
error: 'invalid_request',
104+
errorDetails: stripOpenAICompatibilityMetadata(options.rawMessage),
104105
})
105106

106107
case 'model_not_found':
@@ -399,6 +400,31 @@ export function getRequestTooLargeErrorMessage(): string {
399400
? `Request too large (${limits}). Try with a smaller file.`
400401
: `Request too large (${limits}). Double press esc to go back and try with a smaller file.`
401402
}
403+
404+
const VISION_NOT_SUPPORTED_MESSAGE_PREFIX =
405+
'The active model does not support image/vision inputs. The provider rejected the request because it contained an image. Remove the image, or'
406+
407+
export function getVisionNotSupportedErrorMessages(): string[] {
408+
return [
409+
`${VISION_NOT_SUPPORTED_MESSAGE_PREFIX} switch to a vision-capable model with --model.`,
410+
`${VISION_NOT_SUPPORTED_MESSAGE_PREFIX} run /model to switch to a vision-capable model.`,
411+
]
412+
}
413+
414+
/**
415+
* Canonical message for the `vision_not_supported` OpenAI compatibility
416+
* failure (issue #1421). Returned as a stable string so that
417+
* `normalizeMessagesForAPI`'s `errorToBlockTypes` map can self-heal existing
418+
* transcripts by stripping `image` blocks from the preceding user message
419+
* on resume.
420+
*/
421+
export function getVisionNotSupportedErrorMessage(): string {
422+
const [nonInteractiveMessage, interactiveMessage] =
423+
getVisionNotSupportedErrorMessages()
424+
return getIsNonInteractiveSession()
425+
? nonInteractiveMessage!
426+
: interactiveMessage!
427+
}
402428
export const OAUTH_ORG_NOT_ALLOWED_ERROR_MESSAGE =
403429
'Your account does not have access to OpenClaude. Please run /login.'
404430

src/services/api/openaiErrorClassification.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,39 @@ test('classifies 404 with images as vision_not_supported', () => {
7373
expect(failure.hint).toContain('image')
7474
})
7575

76+
test('classifies 400 with "text is not set" + images as vision_not_supported (issue #1421)', () => {
77+
const failure = classifyOpenAIHttpFailure({
78+
status: 400,
79+
body: '{"error":{"code":"400","message":"Param Incorrect","param":"`text` is not set","type":""}}',
80+
hasImages: true,
81+
})
82+
83+
expect(failure.category).toBe('vision_not_supported')
84+
expect(failure.retryable).toBe(false)
85+
expect(failure.hint).toContain('image')
86+
})
87+
88+
test('classifies 400 with "text is required" + images as vision_not_supported (issue #1421)', () => {
89+
const failure = classifyOpenAIHttpFailure({
90+
status: 400,
91+
body: '{"error":{"message":"text parameter is required"}}',
92+
hasImages: true,
93+
})
94+
95+
expect(failure.category).toBe('vision_not_supported')
96+
})
97+
98+
test('does not classify 400 with "text is not set" when request has no images', () => {
99+
const failure = classifyOpenAIHttpFailure({
100+
status: 400,
101+
body: '{"error":{"message":"text is not set"}}',
102+
hasImages: false,
103+
})
104+
105+
// Without images, "text is not set" is unrelated to vision capability.
106+
expect(failure.category).not.toBe('vision_not_supported')
107+
})
108+
76109
test('classifies context-overflow responses', () => {
77110
const failure = classifyOpenAIHttpFailure({
78111
status: 500,

src/services/api/openaiErrorClassification.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,34 @@ function isMalformedProviderResponse(body: string): boolean {
158158
)
159159
}
160160

161+
/**
162+
* Detect provider messages that complain about a missing/required `text`
163+
* field on an otherwise image-bearing payload. Xiaomi Mimo surfaces this as
164+
* `{"error":{"code":"400","message":"Param Incorrect","param":"`text` is not set"}}`
165+
* (with backticks around `text`) when a `role: "tool"` message carries
166+
* images but no text part. Other OpenAI-compatible providers may phrase
167+
* it differently — match liberally.
168+
*
169+
* Only meaningful when `hasImages` is true (we never want this branch to fire
170+
* for text-only requests, which legitimately lack a text field on vision-only
171+
* payloads).
172+
*/
173+
function isMissingTextPartMessage(body: string): boolean {
174+
// Strip backticks so `\`text\` is not set` matches the same patterns as
175+
// `text is not set` — the Xiaomi Mimo 400 body wraps `text` in backticks
176+
// inside the `param` field, which trips naive substring matching.
177+
const lower = body.toLowerCase().replace(/`/g, '')
178+
return (
179+
lower.includes('text is not set') ||
180+
lower.includes('text is required') ||
181+
lower.includes('text parameter is required') ||
182+
lower.includes('text parameter is missing') ||
183+
lower.includes('missing text') ||
184+
lower.includes('"param":"text"') ||
185+
lower.includes('"param": "text"')
186+
)
187+
}
188+
161189
function isModelNotFoundMessage(body: string): boolean {
162190
const lower = body.toLowerCase()
163191
return (
@@ -344,6 +372,26 @@ export function classifyOpenAIHttpFailure(options: {
344372
}
345373
}
346374

375+
// Xiaomi Mimo and similar OpenAI-compatible providers reject image-bearing
376+
// `role: "tool"` messages with a 400 carrying `text is not set` instead of
377+
// a 404. Classify the same way as the 404 + hasImages branch so the user
378+
// gets actionable guidance rather than the raw API error (issue #1421).
379+
if (
380+
options.status === 400 &&
381+
options.hasImages &&
382+
isMissingTextPartMessage(body)
383+
) {
384+
return {
385+
source: 'http',
386+
category: 'vision_not_supported',
387+
retryable: false,
388+
status: options.status,
389+
message: body,
390+
requestUrl: options.url,
391+
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.',
392+
}
393+
}
394+
347395
if (options.status === 404) {
348396
const isRemote = hostname !== null && !isLocalHost
349397
return {

src/services/api/openaiShim.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1950,7 +1950,11 @@ test('preserves image tool results as placeholders in follow-up requests', async
19501950
text?: string
19511951
image_url?: { url: string }
19521952
}>
1953+
// Issue #1421: image-only tool results now get a placeholder text part
1954+
// prepended so OpenAI-compatible providers that require a `text` field on
1955+
// `role: "tool"` messages (e.g. Xiaomi Mimo) don't 400 with "text is not set".
19531956
expect(parts).toEqual([
1957+
{ type: 'text', text: 'Image attached.' },
19541958
{
19551959
type: 'image_url',
19561960
image_url: { url: 'data:image/png;base64,ZmFrZQ==' },

src/services/api/openaiShim.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,12 @@ function convertToolResultContent(
408408
parts.unshift({ type: 'text', text: 'Error:' })
409409
}
410410

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

414419
function convertContentBlocks(

src/tools/FileReadTool/FileReadTool.ts

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ import {
6464
isPDFSupported,
6565
parsePDFPageRange,
6666
} from '../../utils/pdfUtils.js'
67+
import { checkVisionCapabilityForFile } from '../../utils/visionUtils.js'
6768
import {
6869
checkReadPermissionForTool,
6970
matchingRuleForInput,
@@ -524,14 +525,6 @@ export const FileReadTool = buildTool({
524525
}
525526
}
526527

527-
// SECURITY: UNC path check (no I/O) — defer filesystem operations
528-
// until after user grants permission to prevent NTLM credential leaks
529-
const isUncPath =
530-
fullFilePath.startsWith('\\\\') || fullFilePath.startsWith('//')
531-
if (isUncPath) {
532-
return { result: true }
533-
}
534-
535528
// Binary extension check (string check on extension only, no I/O).
536529
// PDF, images, and SVG are excluded - this tool renders them natively.
537530
const ext = path.extname(fullFilePath).toLowerCase()
@@ -547,6 +540,35 @@ export const FileReadTool = buildTool({
547540
}
548541
}
549542

543+
// Vision-capability gate: refuse image reads when the active model
544+
// explicitly lacks `supportsVision` (e.g. Xiaomi Mimo V2.5 Pro / Flash,
545+
// Llama, Mistral). Returning early surfaces a clear `<tool_use_error>`
546+
// to the model so it can pivot to a text-based approach (Bash `file`,
547+
// `identify`, OCR) or `/model` to switch to a vision-capable model —
548+
// instead of producing an image-only tool result that the provider
549+
// rejects with a generic 400 (issue #1421).
550+
const visionCheck = checkVisionCapabilityForFile(
551+
fullFilePath,
552+
toolUseContext.options.mainLoopModel,
553+
{
554+
baseUrl:
555+
toolUseContext.options.providerOverride?.baseURL ??
556+
process.env.OPENAI_BASE_URL ??
557+
process.env.OPENAI_API_BASE,
558+
},
559+
)
560+
if (visionCheck.result === false) {
561+
return visionCheck
562+
}
563+
564+
// SECURITY: UNC path check (no I/O) — defer filesystem operations
565+
// until after user grants permission to prevent NTLM credential leaks
566+
const isUncPath =
567+
fullFilePath.startsWith('\\\\') || fullFilePath.startsWith('//')
568+
if (isUncPath) {
569+
return { result: true }
570+
}
571+
550572
// Block specific device files that would hang (infinite output or blocking input).
551573
// This is a path-based check with no I/O — safe special files like /dev/null are allowed.
552574
if (isBlockedDevicePath(fullFilePath)) {

0 commit comments

Comments
 (0)