fix: vision handling for OpenAI-compatible models - #1663
Conversation
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (5)**/*.{ts,tsx,js,jsx,py}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**⚙️ CodeRabbit configuration file
Files:
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}⚙️ CodeRabbit configuration file
Files:
**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughAdds end-to-end handling for OpenAI-compatible providers that reject image payloads with HTTP 400 "text is not set" (issue ChangesVision capability gating (issue
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
jatmn
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tools/FileReadTool/FileReadTool.ts (1)
530-534:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove the vision gate before the UNC early-return path.
Line 533 returns early for UNC paths before the new vision check runs, so
\\server\share\image.pngcan bypass the pre-flight denial and still hit the provider with an image payload on non-vision models.Proposed fix
- const isUncPath = - fullFilePath.startsWith('\\\\') || fullFilePath.startsWith('//') - if (isUncPath) { - return { result: true } - } - // Binary extension check (string check on extension only, no I/O). // PDF, images, and SVG are excluded - this tool renders them natively. const ext = path.extname(fullFilePath).toLowerCase() @@ const visionCheck = checkVisionCapabilityForFile( fullFilePath, toolUseContext.options.mainLoopModel, { @@ if (visionCheck.result === false) { return visionCheck } + + const isUncPath = + fullFilePath.startsWith('\\\\') || fullFilePath.startsWith('//') + if (isUncPath) { + return { result: true } + }Also applies to: 551-570
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/FileReadTool/FileReadTool.ts` around lines 530 - 534, The vision gate pre-flight check is running after the UNC path early-return, allowing UNC paths with image payloads to bypass the vision validation on non-vision models. Move the vision gate check to execute before the UNC path detection logic (the isUncPath check with startsWith('\\\\') or startsWith('//')), ensuring that image payload validation happens before allowing UNC paths to return early.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/api/errors.openaiCompatibility.test.ts`:
- Around line 51-66: The test titled vision_not_supported from Xiaomi Mimo 400
claims to use the same canonical message as the test at line 48, but it lacks
the OPENAI_BASE_URL exclusion assertion present in that canonical test. Add a
negative expectation assertion to the test (after the existing expectations on
the text variable) to verify that OPENAI_BASE_URL is not contained in the text,
matching the canonical test's assertion for parity.
In `@src/tools/FileReadTool/prompt.vision.test.ts`:
- Around line 5-26: Add focused regression tests to cover the new runtime
behavior in FileReadTool.validateInput method that handles vision model
validation. Specifically, create test cases that verify when an image path is
provided to validateInput with a non-vision-capable model, the method returns a
structured vision_not_supported denial response. Additionally, add tests to
validate the base URL override and environment variable precedence behavior for
this vision gate logic. These tests should go beyond the existing
renderPromptTemplate assertions and directly test the user-visible behavior
change at the validateInput level.
---
Outside diff comments:
In `@src/tools/FileReadTool/FileReadTool.ts`:
- Around line 530-534: The vision gate pre-flight check is running after the UNC
path early-return, allowing UNC paths with image payloads to bypass the vision
validation on non-vision models. Move the vision gate check to execute before
the UNC path detection logic (the isUncPath check with startsWith('\\\\') or
startsWith('//')), ensuring that image payload validation happens before
allowing UNC paths to return early.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d6b81b48-b2ad-4822-8ae9-7f8b5a50b377
📒 Files selected for processing (11)
src/services/api/errors.openaiCompatibility.test.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.tssrc/tools/FileReadTool/FileReadTool.tssrc/tools/FileReadTool/prompt.vision.test.tssrc/utils/messages.tssrc/utils/visionUtils.test.tssrc/utils/visionUtils.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise in code
Files:
src/tools/FileReadTool/prompt.vision.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/utils/messages.tssrc/services/api/errors.openaiCompatibility.test.tssrc/utils/visionUtils.tssrc/tools/FileReadTool/FileReadTool.tssrc/utils/visionUtils.test.tssrc/services/api/openaiShim.tssrc/services/api/errors.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/tools/FileReadTool/prompt.vision.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/utils/messages.tssrc/services/api/errors.openaiCompatibility.test.tssrc/utils/visionUtils.tssrc/tools/FileReadTool/FileReadTool.tssrc/utils/visionUtils.test.tssrc/services/api/openaiShim.tssrc/services/api/errors.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/FileReadTool/prompt.vision.test.tssrc/tools/FileReadTool/FileReadTool.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/tools/FileReadTool/prompt.vision.test.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/services/api/errors.openaiCompatibility.test.tssrc/utils/visionUtils.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/tools/FileReadTool/prompt.vision.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/utils/messages.tssrc/services/api/errors.openaiCompatibility.test.tssrc/utils/visionUtils.tssrc/tools/FileReadTool/FileReadTool.tssrc/utils/visionUtils.test.tssrc/services/api/openaiShim.tssrc/services/api/errors.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/openaiErrorClassification.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/services/api/errors.openaiCompatibility.test.tssrc/services/api/openaiShim.tssrc/services/api/errors.ts
🔇 Additional comments (8)
src/utils/visionUtils.ts (1)
1-199: LGTM!src/utils/visionUtils.test.ts (1)
1-194: LGTM!src/services/api/openaiErrorClassification.ts (1)
161-187: LGTM!Also applies to: 375-393
src/services/api/openaiErrorClassification.test.ts (1)
76-107: LGTM!src/services/api/openaiShim.ts (1)
411-417: LGTM!src/services/api/openaiShim.test.ts (1)
1953-1958: LGTM!src/services/api/errors.ts (1)
100-105: LGTM!Also applies to: 404-427
src/utils/messages.ts (1)
39-40: LGTM!Also applies to: 2073-2084
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.
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.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tools/FileReadTool/prompt.vision.test.ts`:
- Around line 64-108: The vision-gate tests are failing because
FileReadTool.validateInput calls use optional chaining (?.operator), which
returns undefined when the method exists but assertions expect an object. Remove
the optional chaining operator from all FileReadTool.validateInput?. calls (in
the test cases checking mime-v2.5-pro, provider override, OPENAI_BASE_URL
fallback, and UNC image paths) and invoke FileReadTool.validateInput() directly
without the question mark, ensuring the method returns a deterministic value
that can be asserted against with expect().toEqual(), expect().toMatchObject(),
etc.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 05954514-616f-42ad-8f58-30926f875ddd
📒 Files selected for processing (3)
src/services/api/errors.openaiCompatibility.test.tssrc/tools/FileReadTool/FileReadTool.tssrc/tools/FileReadTool/prompt.vision.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise in code
Files:
src/tools/FileReadTool/prompt.vision.test.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/api/errors.openaiCompatibility.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/tools/FileReadTool/prompt.vision.test.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/api/errors.openaiCompatibility.test.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/FileReadTool/prompt.vision.test.tssrc/tools/FileReadTool/FileReadTool.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/tools/FileReadTool/prompt.vision.test.tssrc/services/api/errors.openaiCompatibility.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/tools/FileReadTool/prompt.vision.test.tssrc/tools/FileReadTool/FileReadTool.tssrc/services/api/errors.openaiCompatibility.test.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/errors.openaiCompatibility.test.ts
🪛 GitHub Check: smoke-and-tests
src/tools/FileReadTool/prompt.vision.test.ts
[failure] 108-108: error: expect(received).toMatchObject(expected)
Matcher error: received value must be a non-null object
at <anonymous> (/home/runner/work/openclaude/openclaude/src/tools/FileReadTool/prompt.vision.test.ts:108:20)
[failure] 99-99: error: expect(received).toMatchObject(expected)
Matcher error: received value must be a non-null object
at <anonymous> (/home/runner/work/openclaude/openclaude/src/tools/FileReadTool/prompt.vision.test.ts:99:20)
[failure] 88-88: error: expect(received).toMatchObject(expected)
Matcher error: received value must be a non-null object
at <anonymous> (/home/runner/work/openclaude/openclaude/src/tools/FileReadTool/prompt.vision.test.ts:88:20)
[failure] 69-69: error: expect(received).toEqual(expected)
- {
- "errorCode": 10,
- "message": StringContaining "does not support image inputs",
- "result": false,
- }
- undefined
- Expected - 5
-
Received + 1
at <anonymous> (/home/runner/work/openclaude/openclaude/src/tools/FileReadTool/prompt.vision.test.ts:69:20)
🔇 Additional comments (2)
src/services/api/errors.openaiCompatibility.test.ts (1)
66-66: LGTM!src/tools/FileReadTool/FileReadTool.ts (1)
564-570: LGTM!
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.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tools/FileReadTool/prompt.vision.test.ts`:
- Line 1: The beforeEach and afterEach test hooks are mutating process.env
globally without acquiring or releasing the shared mutation lock, which causes
test flakiness in concurrent Bun test execution. Import
acquireSharedMutationLock and releaseSharedMutationLock from the appropriate
shared test utilities, then modify the beforeEach hook to call
acquireSharedMutationLock() at the start and modify the afterEach hook to call
releaseSharedMutationLock() to ensure the lock is properly released after each
test completes. Apply this fix to all env-mutating test hooks in the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 276eae22-8d0d-4d17-8c86-acb57c2610d6
📒 Files selected for processing (1)
src/tools/FileReadTool/prompt.vision.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise in code
Files:
src/tools/FileReadTool/prompt.vision.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/tools/FileReadTool/prompt.vision.test.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/FileReadTool/prompt.vision.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/tools/FileReadTool/prompt.vision.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/tools/FileReadTool/prompt.vision.test.ts
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.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tools/FileReadTool/prompt.vision.test.ts (1)
102-111: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd the missing
OPENAI_API_BASEfallback regression case.This block validates fallback via
OPENAI_BASE_URL, but the runtime precedence also includesOPENAI_API_BASE. Add a focused test whereOPENAI_BASE_URLis unset andOPENAI_API_BASEis set.Suggested test addition
+ test('falls back to OPENAI_API_BASE when OPENAI_BASE_URL is unset', async () => { + delete process.env.OPENAI_BASE_URL + process.env.OPENAI_API_BASE = 'https://opencode.ai/zen/go/v1' + + const result = await FileReadTool.validateInput( + { file_path: 'fixture.png' }, + createToolUseContext('mimo-v2.5'), + ) + + expect(result).toMatchObject({ result: false, errorCode: 10 }) + })As per coding guidelines, “Review tests for meaningful coverage of the changed behavior…”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/FileReadTool/prompt.vision.test.ts` around lines 102 - 111, The existing test validates the fallback behavior when OPENAI_BASE_URL is set, but the runtime precedence also includes OPENAI_API_BASE as a fallback option. Add a new test case after the current fallback test that verifies FileReadTool.validateInput correctly handles the scenario where OPENAI_BASE_URL is unset and OPENAI_API_BASE is set instead. This test should set process.env.OPENAI_API_BASE to a test URL, ensure OPENAI_BASE_URL is not set, call FileReadTool.validateInput with the same parameters (file_path: 'fixture.png' and the tool context), and verify the result matches the expected behavior, providing complete coverage of all fallback precedence paths.Source: Coding guidelines
♻️ Duplicate comments (1)
src/tools/FileReadTool/prompt.vision.test.ts (1)
17-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard global env mutations with the shared test-mutation lock.
Line 17 through Line 33 mutate
process.envwithout a shared lock, which can still cause cross-file flakiness under concurrent test execution.As per coding guidelines, “Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state…”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/FileReadTool/prompt.vision.test.ts` around lines 17 - 33, The beforeEach and afterEach hooks in this test file are mutating process.env (specifically OPENAI_BASE_URL and OPENAI_API_BASE) without protection from a shared test-mutation lock, which causes cross-file test flakiness under concurrent execution. Wrap both the beforeEach and afterEach hooks with the shared test-mutation lock to guard these process.env mutations. This ensures that environment variable changes are serialized across all test files and prevents race conditions when tests run in parallel.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/tools/FileReadTool/prompt.vision.test.ts`:
- Around line 102-111: The existing test validates the fallback behavior when
OPENAI_BASE_URL is set, but the runtime precedence also includes OPENAI_API_BASE
as a fallback option. Add a new test case after the current fallback test that
verifies FileReadTool.validateInput correctly handles the scenario where
OPENAI_BASE_URL is unset and OPENAI_API_BASE is set instead. This test should
set process.env.OPENAI_API_BASE to a test URL, ensure OPENAI_BASE_URL is not
set, call FileReadTool.validateInput with the same parameters (file_path:
'fixture.png' and the tool context), and verify the result matches the expected
behavior, providing complete coverage of all fallback precedence paths.
---
Duplicate comments:
In `@src/tools/FileReadTool/prompt.vision.test.ts`:
- Around line 17-33: The beforeEach and afterEach hooks in this test file are
mutating process.env (specifically OPENAI_BASE_URL and OPENAI_API_BASE) without
protection from a shared test-mutation lock, which causes cross-file test
flakiness under concurrent execution. Wrap both the beforeEach and afterEach
hooks with the shared test-mutation lock to guard these process.env mutations.
This ensures that environment variable changes are serialized across all test
files and prevents race conditions when tests run in parallel.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fd45e5cf-c8a4-4fb0-b1eb-18c9acb076e3
📒 Files selected for processing (1)
src/tools/FileReadTool/prompt.vision.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise in code
Files:
src/tools/FileReadTool/prompt.vision.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/tools/FileReadTool/prompt.vision.test.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/FileReadTool/prompt.vision.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/tools/FileReadTool/prompt.vision.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/tools/FileReadTool/prompt.vision.test.ts
🔇 Additional comments (1)
src/tools/FileReadTool/prompt.vision.test.ts (1)
5-12: LGTM!Also applies to: 75-100, 113-120
2 participants
Summary
vision_not_supportedmessage before sending image contentFixes #1421
Impact
Users on OpenAI-compatible non-vision models such as Xiaomi MiMo V2.5 Pro / Flash now get a clear model capability error instead of a confusing provider-side
text is not setfailure. Unknown/custom models still fail open so third-party providers are not blocked unless the registry says they do not support vision.Provider Path Tested
Validation
bun run build- passedbun run smoke- passedbun test src/utils/visionUtils.test.ts src/tools/FileReadTool/prompt.vision.test.ts src/services/api/openaiErrorClassification.test.ts src/services/api/errors.openaiCompatibility.test.ts src/services/api/openaiShim.test.ts- passed, 176 pass / 0 failbun run test:provider- passed, 748 pass / 0 failbun run test:provider-recommendation- passed, 88 pass / 0 failpython -m pytest -q python/tests- passed, 44 passedbun run typecheck:type-tests- passedbun run security:pr-scan- passed, no suspicious additions foundKnown local failures unrelated to this change:
bun run checkfails intest:full: 4044 pass / 11 fail / 1 error. Failures are in/export direct filename,/lsp recommend, marketplace Windows cache finalization / EXDEV fallback, and secure storage platform tests.bun run typecheckstill fails on existing repo-wide diagnostics outside the touched vision files, mostly strictunknown/undefined issues in plugin, memory, MCP, output style, and session storage code.Screenshots
Not applicable; this change does not touch UI, terminal presentation, or the VS Code extension.
Summary by CodeRabbit
Release Notes
text.