feat: support local file paths in media tools - #3
Conversation
Six content tools now accept a local file path in addition to a public URL; local files are sent inline as base64 data URLs (verified live: 14MB video / ~18MB body, HTTP 200). toDataUrl blocks unrestricted file exfiltration to DashScope via a 25MB cap, extension whitelist, and magic-byte signature check — rejects non-media files (.env, id_rsa, renamed secrets) before encoding. bailian.ts unchanged; it passes the url string through as-is.
📝 WalkthroughSummary by CodeRabbit
WalkthroughLocal image and video paths can now be validated and converted to base64 data URLs. MCP tools accept either public HTTP(S) URLs or local paths, while remote URLs pass through unchanged. Unit, tool, and live tests cover the new behavior and limits. ChangesLocal media support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MCP Client
participant MCP Server
participant resolveMedia
participant Local Filesystem
participant Analyze Backend
MCP Client->>MCP Server: Submit URL or local path
MCP Server->>resolveMedia: Resolve media input
resolveMedia->>Local Filesystem: Read and validate local file
Local Filesystem-->>resolveMedia: Validated file bytes
resolveMedia-->>MCP Server: Data URL or unchanged remote URL
MCP Server->>Analyze Backend: Analyze resolved media
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/server.ts`:
- Around line 33-37: Update the mediaInput schema predicate to accept only valid
remote HTTP(S) URLs or local paths, explicitly rejecting non-HTTP(S) schemes
such as ftp:// and file:// while preserving the existing validation message and
isLocalPath classifier.
In `@test/live.test.ts`:
- Line 12: Update the live-test fixture path initialization around
BUNDLED_SAMPLE so it does not use import.meta.dirname on unsupported Node
versions, or raise the package.json Node engine floor to the first version
supporting that API. Preserve live-test loading and fixture resolution across
the declared runtime range.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: be9ede19-2154-4298-87e0-ae38bd149cc3
📒 Files selected for processing (6)
AGENTS.mdsrc/media.tssrc/server.tstest/live.test.tstest/media.test.tstest/tools.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,mjs,json,md,env}
📄 CodeRabbit inference engine (AGENTS.md)
Never commit or hardcode secrets, API keys, tokens, or
.envfiles. Read the DashScope key fromDASHSCOPE_API_KEYviasrc/config.ts; use dummy values such assk-testin fixtures.
Files:
AGENTS.mdtest/live.test.tssrc/media.tstest/media.test.tstest/tools.test.tssrc/server.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Delete files with
trash, neverrm.
Files:
AGENTS.mdtest/live.test.tssrc/media.tstest/media.test.tstest/tools.test.tssrc/server.ts
test/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
test/**/*.{ts,tsx}: Unit and mocked end-to-end tests must use MSW to mockfetchand must not make real API calls.
Every new tool or branch of logic must have a test, and test coverage must remain at least 85%.
Files:
test/live.test.tstest/media.test.tstest/tools.test.ts
test/live.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Live tests may call the real API only when
LIVE=1with a realDASHSCOPE_API_KEY; they must remain excluded from defaultnpm test.
Files:
test/live.test.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Use TypeScript strict typing insrc/: do not useany,@ts-ignore, or non-null assertions; prefer narrow types andunknownwhen parsing external JSON.
Keep the DashScope payload builderbuildPayloadinjectable, and make content-block shape changes forvideo_urlorimage_urlin one place.
Preserve the eight MCP tool names and argument schemas insrc/server.ts:analyze_video,analyze_image,summarize_video,extract_video_text,video_qa,compare_video_frames,check_endpoint_status, andlist_capabilities. Add tools rather than silently renaming or changing existing ones.
Ensurecheck_endpoint_statusredacts the API key usingredactKey; its no-leak test must continue passing.
Use the Bailian DashScope OpenAI-compatible endpoint${DASHSCOPE_BASE_URL}/chat/completions, defaulting tohttps://dashscope.aliyuncs.com/compatible-mode/v1, with modelqwen3.7-plus.
Do not switch multimodal tools to the Anthropic-compatible/apps/anthropicendpoint, because it does not support video input.
Do not add client-side video frame extraction; video frame sampling is server-side at 0.5 seconds per frame in OpenAI-compatible mode.
Keep the OpenAI-compatiblevideo_urlassumption and fallback logic centralized incontentBlock()insrc/bailian.ts; use the documented fallback only if the endpoint rejects it.
Use the exact model IDqwen3.7-plus, verifying it against the Bailian model list if a model-not-found error occurs.
Enforce the 25MB local-media guardrail, validate files by extension and magic-byte signature, and encode eligible local files as base64 data URLs; larger files must use a public URL.
Files:
src/media.tssrc/server.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: sommio/qwen-omni-mcp
Timestamp: 2026-07-29T04:30:37.233Z
Learning: If a secret is accidentally staged, unstage it, rotate the key immediately, and notify the maintainer.
Learnt from: CR
Repo: sommio/qwen-omni-mcp
Timestamp: 2026-07-29T04:30:37.233Z
Learning: Never use `git commit --no-verify` or `git push --no-verify`; fix hook failures instead of bypassing them.
Learnt from: CR
Repo: sommio/qwen-omni-mcp
Timestamp: 2026-07-29T04:30:37.233Z
Learning: After cloning, run `npm install` to install Husky hooks and verify `git config core.hooksPath` is `.husky`.
Learnt from: CR
Repo: sommio/qwen-omni-mcp
Timestamp: 2026-07-29T04:30:37.233Z
Learning: Before push, all quality gates must pass: `npm run typecheck`, `npm run lint`, `npm run format:check`, `npm test`, and `npm run build`.
Learnt from: CR
Repo: sommio/qwen-omni-mcp
Timestamp: 2026-07-29T04:30:37.233Z
Learning: Do not add a new runtime, language, or heavy dependency without explicit maintainer approval.
🔇 Additional comments (22)
AGENTS.md (1)
68-68: LGTM!src/media.ts (6)
6-16: LGTM!
18-45: LGTM!
47-56: LGTM!
58-87: LGTM!
89-126: LGTM!
128-138: LGTM!test/media.test.ts (6)
1-23: LGTM!
25-78: LGTM!
80-148: LGTM!
150-183: LGTM!
198-218: LGTM!
185-195: 📐 Maintainability & Code QualityNo change needed.
context.skip()/t.skip()is the supported runtime skip API for conditional test skipping in Vitest 2, and the trailingreturndoes not prevent the skip behavior.src/server.ts (3)
64-65: LGTM!
82-84: LGTM!Also applies to: 104-106, 128-128, 151-151, 162-162, 175-175
232-233: LGTM!test/tools.test.ts (3)
1-6: LGTM!Also applies to: 201-210
212-265: LGTM!
267-293: LGTM!test/live.test.ts (3)
8-11: LGTM!Also applies to: 14-18
21-24: LGTM!Also applies to: 34-39
51-66: LGTM!
| const mediaInput = (description: string) => | ||
| z | ||
| .string() | ||
| .refine((v) => isRemoteUrl(v) || isLocalPath(v), "Must be a public URL or a local file path") | ||
| .describe(description); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
The refine predicate is tautological — it only rejects empty strings.
isLocalPath(v) is v.length > 0 && !isRemoteUrl(v), so isRemoteUrl(v) || isLocalPath(v) reduces to v.length > 0. Non-file schemes like ftp://h/x.mp4 or file:///x.mp4 pass validation and later fail in toDataUrl as Cannot read local file: file:///x.mp4, which reads as a filesystem error rather than a rejected input.
Consider rejecting non-http(s) URL schemes explicitly so the message matches the actual contract.
♻️ Tighten the predicate
const mediaInput = (description: string) =>
z
.string()
- .refine((v) => isRemoteUrl(v) || isLocalPath(v), "Must be a public URL or a local file path")
+ .refine(
+ (v) => isRemoteUrl(v) || (isLocalPath(v) && !/^[a-z][a-z0-9+.-]*:\/\//i.test(v)),
+ "Must be a public http(s) URL or a local file path",
+ )
.describe(description);This keeps isLocalPath as the shared classifier while making the scheme rule explicit at the tool boundary.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const mediaInput = (description: string) => | |
| z | |
| .string() | |
| .refine((v) => isRemoteUrl(v) || isLocalPath(v), "Must be a public URL or a local file path") | |
| .describe(description); | |
| const mediaInput = (description: string) => | |
| z | |
| .string() | |
| .refine( | |
| (v) => isRemoteUrl(v) || (isLocalPath(v) && !/^[a-z][a-z0-9+.-]*:\/\//i.test(v)), | |
| "Must be a public http(s) URL or a local file path", | |
| ) | |
| .describe(description); |
🤖 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/server.ts` around lines 33 - 37, Update the mediaInput schema predicate
to accept only valid remote HTTP(S) URLs or local paths, explicitly rejecting
non-HTTP(S) schemes such as ftp:// and file:// while preserving the existing
validation message and isLocalPath classifier.
| const LOCAL_ASSET_DIR = | ||
| process.env.QWEN_LIVE_ASSET_DIR ?? | ||
| "/home/sommio/Downloads/2026-07-08-01-test10_V1_fix1/cat_dialogue_10/cat_dialogue_000001"; | ||
| const BUNDLED_SAMPLE = join(import.meta.dirname, "fixtures", "sample.png"); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check declared Node version floor across manifests and CI.
fd -H -t f 'package.json' -E node_modules --exec jq -r '{path: input_filename, engines: .engines, volta: .volta}' {} \;
fd -H -t f '.nvmrc|.node-version|.tool-versions' -E node_modules --exec sh -c 'echo "--- $1"; cat "$1"' _ {} \;
rg -n --iglob '.github/workflows/*.y*ml' -C2 'node-version|setup-node'
# Other usages of import.meta.dirname / import.meta.filename
rg -n 'import\.meta\.(dirname|filename)' -g '!node_modules'Repository: sommio/qwen-omni-mcp
Length of output: 245
🏁 Script executed:
#!/bin/bash
set -u
echo "--- package and lockfiles"
for f in package.json package-lock.json pnpm-lock.yaml yarn.lock bun.lockb bun.lock; do
if [ -e "$f" ]; then
echo "### $f"
case "$f" in
package.json) jq '{engines, type}' "$f";;
*) head -80 "$f";;
esac
fi
done
echo "--- Node files"
fd -H -t f '.nvmrc|.node-version|.tool-versions|package.json' -E node_modules
echo "--- CI node-version references"
rg -n --iglob '.github/workflows/*.y*ml' -C2 'node-version|setup-node' || true
echo "--- import.meta.dirname/filename usages"
rg -n 'import\.meta\.(dirname|filename)' -g '!node_modules' || trueRepository: sommio/qwen-omni-mcp
Length of output: 2961
🌐 Web query:
Node.js CHANGELOG import.meta.dirname added 20.11.0
💡 Result:
The import.meta.dirname property (along with import.meta.filename) was added to Node.js in version 20.11.0 [1][2][3]. This addition was tracked in the Node.js CHANGELOG under the esm module section as a semver-minor change [1][4]. These properties serve as the ESM equivalents to the CommonJS dirname and filename variables, respectively [5][6].
Citations:
- 1: https://github.com/nodejs/node/blob/v20.11.1/doc/changelogs/CHANGELOG_V20.md
- 2: https://github.com/nodejs/nodejs.org/blob/main/apps/site/pages/en/blog/release/v20.11.0.md
- 3: https://nodejs.org/es/blog/release/v20.11.0
- 4: nodejs/node@2d74e77
- 5: https://nodejs.org/api/esm.html
- 6: https://nodejs.github.io/package-examples/05-cjs-esm-migration/migrating-context-local-variables/
🏁 Script executed:
#!/bin/bash
set -u
echo "--- lockfile `@types/node` and vitest/test runner entries"
python3 - <<'PY'
import json
from pathlib import Path
p = Path("package-lock.json")
data = json.loads(p.read_text())
packages = data.get("packages", {})
for key in [
"node_modules/@types/node",
"node_modules/vitest",
"node_modules/@vitest/runner",
"node_modules/@types/estree",
"node_modules/rollup",
]:
val = packages.get(key)
if val:
print("###", key)
print(json.dumps({k: val.get(k) for k in ("name","version","engines","dependencies","devDependencies") if k in val}, indent=2))
PY
echo "--- lockfile `@types/node` version"
python3 - <<'PY'
import json
print(json.load(open("package-lock.json"))["packages"]["node_modules/@types/node"]["version"])
PY
echo "--- package scripts"
jq '.scripts' package.jsonRepository: sommio/qwen-omni-mcp
Length of output: 1891
Narrow the Node engine floor to where import.meta.dirname is available.
import.meta.dirname is only available from Node 20.11+/21.2+, but package.json currently declares >=20, so Node 20.9/20.10 can throw at live-test module load instead of skipping the tests. Raise the runtime floor to cover it or avoid this API in test/live.test.ts.
🤖 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 `@test/live.test.ts` at line 12, Update the live-test fixture path
initialization around BUNDLED_SAMPLE so it does not use import.meta.dirname on
unsupported Node versions, or raise the package.json Node engine floor to the
first version supporting that API. Preserve live-test loading and fixture
resolution across the declared runtime range.
Summary
The 6 content tools (
analyze_video,analyze_image,summarize_video,extract_video_text,video_qa,compare_video_frames) now accept a local file path in addition to a public URL. Local files are read and sent inline as base64 data URLs via the OpenAI-compatible endpoint — verified live (14MB video / ~18MB body, HTTP 200 onqwen3.7-plus).This removes the previous requirement to host every media file at a public URL before the tools could touch it.
Changes
src/media.ts(new):isRemoteUrl,isLocalPath,mimeFromExt,toDataUrl,resolveMedia. Local path → base64 data URL; remote URL → passthrough.src/server.ts:mediaCallresolves input viaresolveMediabeforeanalyze; the 6 tool schemas relaxz.string().url()to accept a URL or local path (field name + string type unchanged — backward compatible). Tool descriptions andlist_capabilitiesnotes updated.src/bailian.ts: untouched — it already passes theurlstring transparently into the content block, so data URLs work as-is.test/media.test.ts(new, 32 cases) +test/tools.test.ts(+5 msw e2e) +test/live.test.ts(rewritten to exerciseresolveMediaend-to-end against the real API).AGENTS.md: fragile assumption feat: support local file paths in media tools #3 updated — local video ≤25MB can be base64 (the prior “>10MB cannot” claim was disproven live).Security
toDataUrlguards against unrestricted local-file exfiltration to DashScope (a prompt-injected/malicious MCP client could otherwise passimage_url="/home/.../.env"or~/.ssh/id_rsaand ship the bytes to a third-party endpoint):.env,id_rsa,/etc/passwd,.bashrc, etc. before reading.secret.png). Also defeats symlink-to-secret attacks, so nolstatguard is needed (which would have rejected legitimate symlinked media).Surfaced by an adversarial review workflow; also fixed a TOCTOU in the size guard (re-check bytes read) and wrapped
readFilefailures into theCannot read local filecontract.Verification
npm run typecheck && lint && format:check && test && build— all green.LIVE=1end-to-end against real DashScope: bundled PNG (5.8s), local图1.jpg(19s), local 14MBvideo.mp4(79s) — all return real answers through the fullresolveMedia→analyzepath.