feat: add qwen-omni-mcp bailian video/image server - #1
Conversation
Node/TS MCP server exposing 8 video/image understanding tools backed by Bailian (DashScope) OpenAI-compatible endpoint, model qwen3.7-plus. Native video — no client-side frame extraction. - strict TS + eslint strictTypeChecked + prettier + vitest(msw) + husky - pre-commit secret guard (check-secrets + gitleaks) blocks .env/sk-ws- - CI: node 20/22 matrix, gitleaks, weekly live smoke - AGENTS.md hard rules: no --no-verify, no key commits, env-only
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe project adds a DashScope-backed MCP server for image and video analysis, with environment configuration, prompt handling, MCP tools, stdio startup, tests, repository documentation, secret protection, and CI workflows. ChangesMultimodal MCP server
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant MCPServer
participant DashScope
MCPClient->>MCPServer: Call analysis tool
MCPServer->>DashScope: Send image/video prompt
DashScope-->>MCPServer: Return model response
MCPServer-->>MCPClient: Return tool result
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 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 @.github/PULL_REQUEST_TEMPLATE.md:
- Line 1: Update the first heading in the pull request template from a level-two
heading to a top-level H1 heading, preserving the existing “Summary” text so the
template satisfies Markdownlint rule MD041.
In @.github/workflows/ci.yml:
- Around line 19-23: Pin the actions/checkout, actions/setup-node, and
actions/upload-artifact uses to immutable commit SHAs while preserving their
current versions and configuration. In the checkout step, set
persist-credentials to false so subsequent steps cannot reuse the checkout
token.
- Around line 3-6: Add a workflow-level concurrency configuration near the
existing on trigger in the CI workflow, using a group key that distinguishes the
relevant workflow and ref while canceling in-progress runs when superseded. Keep
the current push and pull_request triggers unchanged.
In @.github/workflows/secrets-scan.yml:
- Around line 15-18: Update the workflow steps using actions/checkout and
gitleaks/gitleaks-action to immutable commit SHAs instead of mutable version
tags, and set persist-credentials to false in the checkout step while preserving
fetch-depth: 0.
- Around line 3-6: Add a concurrency configuration to the workflow alongside the
existing “on” triggers, using a stable group key that distinguishes the relevant
branch or pull request context and cancels or prevents redundant in-progress
secrets scans consistently with ci.yml.
In @.github/workflows/smoke-live.yml:
- Around line 19-20: Update the workflow steps using actions/checkout and
actions/setup-node to pin both actions to immutable commit SHAs, preserving
their current major-version behavior. Add persist-credentials: false to the
actions/checkout step.
- Around line 3-7: Add a workflow-level concurrency group to smoke-live.yml so
workflow_dispatch and scheduled runs share the same serialized execution group
and cannot overlap. Configure the group to use the workflow identity and
cancel-in-progress behavior appropriate for preventing duplicate live API calls,
without changing the existing triggers.
In @.husky/pre-commit:
- Around line 6-11: Make secret scanning fail closed: in .husky/pre-commit,
require gitleaks to be installed or provisioned and exit nonzero when
unavailable instead of skipping. In scripts/check-secrets.mjs, inspect every
staged .env* path, including nested files and .env.test, and expand credential
detection beyond root .env files and sk-ws- keys to cover relevant secret
patterns.
In @.husky/pre-push:
- Around line 4-5: Update the pre-push hook commands alongside the existing
typecheck and test gates to also run lint, format:check, and build, ensuring
each command exits nonzero on failure so every quality gate must pass before
pushing.
In `@src/bailian.ts`:
- Around line 103-106: Update the response parsing in the surrounding try block
to validate and narrow the unknown JSON with runtime type guards before
assigning it to ChatResponse. Ensure nested fields, including
AnalyzeResult.model, are checked for their declared types so malformed values
such as numeric model data are rejected rather than accepted via a type
assertion.
In `@src/server.ts`:
- Line 69: Introduce a shared HTTP(S)-only URL schema in src/server.ts and reuse
it for every media field: video_url and image_url at src/server.ts lines 69, 91,
113, 136, 147, and 160. Ensure the schema accepts only http and https protocols
while retaining URL validation, and add coverage confirming non-HTTP schemes
such as file and ftp are rejected at all affected tool inputs.
In `@test/bailian.test.ts`:
- Around line 123-134: Add a test alongside the existing timeout test that makes
the mocked POST handler return MSW v2’s HttpResponse.error(), then call analyze
with the same request shape and assert rejection with status "network". Keep the
existing abort/timeout test unchanged and target the plain fetch-rejection
branch in analyze.
In `@test/live.test.ts`:
- Line 8: Replace the hardcoded value in LOCAL_ASSET_DIR with an
environment-variable-based configuration, using the current path only as a
documented example or fallback rather than embedding personal filesystem details
in the test. Preserve the existing existsSync gating and local asset test
behavior for contributors who provide the directory.
In `@test/tools.test.ts`:
- Around line 121-146: Add a dedicated behavioral test for the
extract_video_text tool alongside the existing tool tests, invoking it with a
video_url and asserting the response, captured request video kind,
TEXT_EXTRACTION_PROMPT content, and max_tokens of 1024. Keep the test consistent
with the existing mockCapture, withClient, and textOf patterns.
🪄 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: 90cd018a-d948-4c6c-a896-28d11b898bb4
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsontest/fixtures/sample.pngis excluded by!**/*.png
📒 Files selected for processing (27)
.env.example.github/PULL_REQUEST_TEMPLATE.md.github/workflows/ci.yml.github/workflows/secrets-scan.yml.github/workflows/smoke-live.yml.gitignore.husky/pre-commit.husky/pre-push.prettierignore.prettierrcAGENTS.mdREADME.mdeslint.config.jspackage.jsonscripts/check-secrets.mjssrc/bailian.tssrc/config.tssrc/index.tssrc/prompts.tssrc/server.tstest/bailian.test.tstest/config.test.tstest/live.test.tstest/tools.test.tstsconfig.build.jsontsconfig.jsonvitest.config.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
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 andunknownoveranywhen parsing external JSON, particularly insrc/bailian.ts.
Match the existing TypeScript source style and use Prettier and ESLint fixes for formatting.
check_endpoint_statusmust redact the API key usingredactKey; do not allow the key to leak.
Use the Bailian DashScope OpenAI-compatible endpoint${DASHSCOPE_BASE_URL}/chat/completions, defaulting tohttps://dashscope.aliyuncs.com/compatible-mode/v1.
Use modelqwen3.7-plusfor multimodal video support; do not switch multimodal tools to the Anthropic-compatible/apps/anthropicendpoint.
Do not add client-side video frame extraction; video frame sampling is server-side at 0.5 seconds per frame in OpenAI-compatible mode.
Do not send local videos larger than 10 MB as base64 data URLs; use a public URL for large local videos.
Files:
src/index.tssrc/config.tssrc/prompts.tssrc/server.tssrc/bailian.ts
test/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Never place real API keys in test fixtures; use dummy values such as
sk-testorsk-secret-key-….
Files:
test/config.test.tstest/live.test.tstest/bailian.test.tstest/tools.test.ts
test/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
test/**/*.{ts,tsx}:anymay be used sparingly in tests for fixture typing, but should not be used as a general practice.
Unit and mocked end-to-end tests must use MSW to mockfetchand must not make real API calls.
Add a test for every new tool or branch of logic; maintain at least 85% coverage.
Files:
test/config.test.tstest/live.test.tstest/bailian.test.tstest/tools.test.ts
src/config.ts
📄 CodeRabbit inference engine (AGENTS.md)
Read the DashScope API key from
DASHSCOPE_API_KEYviasrc/config.ts; never hardcode keys in source, tests, configs, or documentation.
Files:
src/config.ts
test/live.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Live tests run only when
LIVE=1with a realDASHSCOPE_API_KEY; they must not be part of defaultnpm test.
Files:
test/live.test.ts
src/server.ts
📄 CodeRabbit inference engine (AGENTS.md)
Preserve the names and argument schemas of the eight MCP tools; add new tools instead of silently renaming or changing existing ones.
Files:
src/server.ts
src/bailian.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/bailian.ts: Keep the DashScope payload builderbuildPayloadinjectable, and changevideo_url/image_urlcontent block shapes in that single location.
If thevideo_urlcontent block is rejected, changecontentBlock()to use native DashScopevideocontent or the documented fallback model; verify the exactqwen3.7-plusmodel ID before changing it.
Files:
src/bailian.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: sommio/qwen-omni-mcp
Timestamp: 2026-07-28T09:26:04.972Z
Learning: Never commit secrets, API keys, tokens, or `.env` files; keep keys only in gitignored `.env` files or environment variables.
Learnt from: CR
Repo: sommio/qwen-omni-mcp
Timestamp: 2026-07-28T09:26:04.972Z
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-28T09:26:04.972Z
Learning: Never bypass Git hooks with `git commit --no-verify` or `git push --no-verify`; fix hook failures instead.
Learnt from: CR
Repo: sommio/qwen-omni-mcp
Timestamp: 2026-07-28T09:26:04.972Z
Learning: After the first clone, run `npm install` so the `prepare` script installs Husky hooks, and verify `core.hooksPath` is `.husky`.
Learnt from: CR
Repo: sommio/qwen-omni-mcp
Timestamp: 2026-07-28T09:26:04.972Z
Learning: Before push, all quality gates must pass: typecheck, lint, format check, tests, and build.
Learnt from: CR
Repo: sommio/qwen-omni-mcp
Timestamp: 2026-07-28T09:26:04.972Z
Learning: Do not add a new runtime, language, or heavy dependency without explicit maintainer approval.
🪛 LanguageTool
AGENTS.md
[style] ~9-~9: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...SCOPE_API_KEYviasrc/config.ts`. - Never paste a real key into a fixture. Test...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
README.md
[uncategorized] ~106-~106: The official name of this software platform is spelled with a capital “H”.
Context: ... real API calls (costs tokens) ``` CI (.github/workflows/ci.yml) runs the same gates ...
(GITHUB)
🪛 markdownlint-cli2 (0.23.0)
.github/PULL_REQUEST_TEMPLATE.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🪛 zizmor (1.26.1)
.github/workflows/secrets-scan.yml
[warning] 15-17: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 15-15: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 18-18: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[info] 12-12: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
[warning] 3-6: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
.github/workflows/ci.yml
[warning] 19-19: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 19-19: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 20-20: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 33-33: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[info] 12-12: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
[warning] 3-6: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
.github/workflows/smoke-live.yml
[warning] 19-19: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 19-19: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 20-20: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[info] 13-13: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
[warning] 3-7: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
🔇 Additional comments (21)
AGENTS.md (1)
1-69: LGTM!README.md (1)
1-113: LGTM!package.json (1)
1-62: LGTM!tsconfig.json (1)
1-23: LGTM!.env.example (1)
1-13: LGTM!src/config.ts (1)
1-48: LGTM!src/bailian.ts (1)
1-102: LGTM!Also applies to: 107-121
tsconfig.build.json (1)
1-13: LGTM!eslint.config.js (1)
1-42: LGTM!.prettierrc (1)
1-8: LGTM!.prettierignore (1)
1-15: LGTM!.gitignore (1)
1-39: LGTM!src/prompts.ts (1)
1-44: LGTM!src/index.ts (1)
1-7: LGTM!test/bailian.test.ts (1)
7-55: LGTM!Also applies to: 57-122, 146-157
test/config.test.ts (1)
1-56: LGTM!test/tools.test.ts (1)
12-39: LGTM!Also applies to: 76-119, 148-195
vitest.config.ts (1)
1-20: LGTM!.github/workflows/ci.yml (1)
8-18: LGTM!Also applies to: 24-32
.github/workflows/secrets-scan.yml (1)
1-2: LGTM!Also applies to: 8-14, 19-20
.github/workflows/smoke-live.yml (1)
1-2: LGTM!Also applies to: 9-18, 21-31
| @@ -0,0 +1,18 @@ | |||
| ## Summary | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a top-level heading on the first line.
Markdownlint rule MD041 requires the first line to be an H1, so this template can fail the documented formatting gate.
Proposed fix
-## Summary
+# Pull Request Summary📝 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.
| ## Summary | |
| # Pull Request Summary |
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 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 @.github/PULL_REQUEST_TEMPLATE.md at line 1, Update the first heading in the
pull request template from a level-two heading to a top-level H1 heading,
preserving the existing “Summary” text so the template satisfies Markdownlint
rule MD041.
Source: Linters/SAST tools
| on: | ||
| push: | ||
| branches: [main, develop] | ||
| pull_request: |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
No concurrency group defined.
zizmor flags the workflow for missing a concurrency setting; overlapping pushes/PRs can run redundant CI jobs concurrently, wasting compute.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 3-6: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
🤖 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 @.github/workflows/ci.yml around lines 3 - 6, Add a workflow-level
concurrency configuration near the existing on trigger in the CI workflow, using
a group key that distinguishes the relevant workflow and ref while canceling
in-progress runs when superseded. Keep the current push and pull_request
triggers unchanged.
Source: Linters/SAST tools
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: ${{ matrix.node }} | ||
| cache: npm |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
Actions unpinned and checkout doesn't disable credential persistence.
zizmor flags actions/checkout@v4, actions/setup-node@v4, and actions/upload-artifact@v4 as unpinned (mutable tag rather than a commit SHA), and checkout doesn't set persist-credentials: false, leaving the git credential available to later steps/artifacts unnecessarily.
Also applies to: 33-37
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 19-19: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 19-19: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 20-20: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/ci.yml around lines 19 - 23, Pin the actions/checkout,
actions/setup-node, and actions/upload-artifact uses to immutable commit SHAs
while preserving their current versions and configuration. In the checkout step,
set persist-credentials to false so subsequent steps cannot reuse the checkout
token.
Source: Linters/SAST tools
| on: | ||
| push: | ||
| branches: [main, develop] | ||
| pull_request: |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
No concurrency group defined.
Same finding as ci.yml — no concurrency setting, so parallel pushes/PRs can run duplicate scans.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 3-6: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
🤖 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 @.github/workflows/secrets-scan.yml around lines 3 - 6, Add a concurrency
configuration to the workflow alongside the existing “on” triggers, using a
stable group key that distinguishes the relevant branch or pull request context
and cancels or prevents redundant in-progress secrets scans consistently with
ci.yml.
Source: Linters/SAST tools
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| - uses: gitleaks/gitleaks-action@v2 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
Actions unpinned and checkout doesn't disable credential persistence.
Same as flagged in ci.yml: actions/checkout@v4 and gitleaks/gitleaks-action@v2 are unpinned mutable tags, and checkout doesn't set persist-credentials: false.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 15-17: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 15-15: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 18-18: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/secrets-scan.yml around lines 15 - 18, Update the workflow
steps using actions/checkout and gitleaks/gitleaks-action to immutable commit
SHAs instead of mutable version tags, and set persist-credentials to false in
the checkout step while preserving fetch-depth: 0.
Source: Linters/SAST tools
| let data: ChatResponse; | ||
| try { | ||
| const json: unknown = await res.json(); | ||
| data = json as ChatResponse; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate the response before treating it as ChatResponse.
The assertion accepts malformed API JSON; for example, a numeric model is returned through the AnalyzeResult.model: string contract. Parse and narrow the response with runtime type guards before assigning it to ChatResponse.
🤖 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/bailian.ts` around lines 103 - 106, Update the response parsing in the
surrounding try block to validate and narrow the unknown JSON with runtime type
guards before assigning it to ChatResponse. Ensure nested fields, including
AnalyzeResult.model, are checked for their declared types so malformed values
such as numeric model data are rejected rather than accepted via a type
assertion.
Source: Coding guidelines
| description: | ||
| "Analyze a video via URL using Qwen3.7-Plus (multimodal). The model reads the video natively — no client-side frame extraction. URL must be publicly reachable (http/https).", | ||
| inputSchema: { | ||
| video_url: z.string().url().describe("Public URL of the video to analyze"), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
node --input-type=module - <<'NODE'
import { z } from "zod";
const schema = z.string().url();
for (const value of [
"https://example.com/video.mp4",
"file:///tmp/video.mp4",
"ftp://example.com/video.mp4",
]) {
console.log(value, schema.safeParse(value).success);
}
NODERepository: sommio/qwen-omni-mcp
Length of output: 1158
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate server and package files =="
git ls-files | rg '(^|/)(src/server\.ts|package\.json|package-lock\.json|yarn\.lock|pnpm-lock\.yaml)$' || true
echo
echo "== relevant server snippets =="
if [ -f src/server.ts ]; then
nl -ba src/server.ts | sed -n '55,170p'
fi
echo
echo "== zod/version references =="
if [ -f package.json ]; then
node - <<'JS'
const fs = require('fs');
const pkg = JSON.parse(fs.readFileSync('package.json','utf8'));
console.log(JSON.stringify({dependencies: pkg.dependencies, devDependencies: pkg.devDependencies}, null, 2));
JS
fi
echo
echo "== URL validator references in server =="
rg -n "video_url|image_url|z\.string\(\)\.url\(\)|dashscope|DASHSCOPE" src/server.ts || true
echo
echo "== read-only Node URL behavior probe =="
node - <<'JS'
for (const value of [
"https://example.com/video.mp4",
"file:///tmp/video.mp4",
"ftp://example.com/video.mp4",
]) {
try {
const u = new URL(value);
console.log(value, "ok", u.protocol, u.href);
} catch (e) {
console.log(value, "reject", e.message);
}
}
JSRepository: sommio/qwen-omni-mcp
Length of output: 314
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant server snippets =="
awk '{printf "%6d %s\n", NR, $0}' src/server.ts | sed -n '55,170p'
echo
echo "== zod/version references in package files =="
node - <<'JS'
const fs = require('fs');
const pkg = JSON.parse(fs.readFileSync('package.json','utf8'));
console.log(JSON.stringify({dependencies: pkg.dependencies, devDependencies: pkg.devDependencies}, null, 2));
const lock = fs.readFileSync('package-lock.json','utf8');
for (const needle of ['node_modules/zod', '"version": "3.23.8"']) {
const idx = lock.indexOf(needle);
console.log(`${needle}: ${idx === -1 ? 'not found' : 'found at ' + idx}`);
}
// find zod version entries directly
const re = /"node_modules\/zod":\s*\{(?:.|\n){0,200}/g;
while ((m = re.exec(lock)) !== null) console.log(m[0].split('\n').filter(l => /"name"|"version"|"resolved"|node_modules\/zod/.test(l)).join('\n'));
JS
echo
echo "== schema and DashScope usage =="
rg -n "video_url|image_url|z\.string\(\)\.url\(\)|dashscope|DASHSCOPE|openai" src/server.ts || true
echo
echo "== read-only Node URL behavior probe =="
node - <<'JS'
for (const value of [
"https://example.com/video.mp4",
"file:///tmp/video.mp4",
"ftp://example.com/video.mp4",
]) {
try {
const u = new URL(value);
console.log(value, "ok", u.protocol, u.href);
} catch (e) {
console.log(value, "reject", e.name + ": " + e.message);
}
}
JSRepository: sommio/qwen-omni-mcp
Length of output: 6858
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== imports and mediaCall implementation =="
awk '{printf "%6d %s\n", NR, $0}' src/server.ts | sed -n '1,50p'
awk '{printf "%6d %s\n", NR, $0}' src/server.ts | sed -n '170,340p'
echo
echo "== existing tests mentioning video/image/url =="
printf '%s\n' "$(git ls-files)" | rg -n "(^|/)(test|spec|__tests__|vitest|tests)" || true
if [ -d tests ] || [ -d test ]; then
find tests test -maxdepth 3 -type f 2>/dev/null | sort
fi
rg -n "analyze_video|analyze_image|summarize_video|video_url|image_url|https|file:|ftp:" . || trueRepository: sommio/qwen-omni-mcp
Length of output: 4420
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tests/tools snippets =="
for f in test/tools.test.ts test/bailian.test.ts test/config.test.ts vitest.config.ts; do
[ -f "$f" ] || continue
echo "--- $f ---"
wc -l "$f"
rg -n "analyze_video|analyze_image|summarize_video|extract_video_text|video_qa|compare_video_frames|check_endpoint_status|list_capabilities|video_url|image_url|https|file:|ftp:|zod|url|inputSchema" "$f" || true
echo
done
echo "== bailian url implementation =="
if [ -f src/bailian.ts ]; then
awk '{printf "%6d %s\n", NR, $0}' src/bailian.ts | sed -n '1,260p'
fiRepository: sommio/qwen-omni-mcp
Length of output: 8093
Restrict media inputs to HTTP(S) URLs.
These schemas describe media as public HTTP/HTTPS URLs, but z.string().url() only checks URL syntax and accepts schemes like file:/ftp:. Introduce a shared HTTP(S)-only schema and apply it to video_url and image_url across the MCP tools, plus add non-HTTP scheme rejection coverage.
📍 Affects 1 file
src/server.ts#L69-L69(this comment)src/server.ts#L91-L91src/server.ts#L113-L113src/server.ts#L136-L136src/server.ts#L147-L147src/server.ts#L160-L160
🤖 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` at line 69, Introduce a shared HTTP(S)-only URL schema in
src/server.ts and reuse it for every media field: video_url and image_url at
src/server.ts lines 69, 91, 113, 136, 147, and 160. Ensure the schema accepts
only http and https protocols while retaining URL validation, and add coverage
confirming non-HTTP schemes such as file and ftp are rejected at all affected
tool inputs.
Source: Coding guidelines
| it("maps an abort to a timeout error", async () => { | ||
| server.use( | ||
| http.post(endpoint, async () => { | ||
| await new Promise((r) => setTimeout(r, 1000)); | ||
| return jsonOk(); | ||
| }), | ||
| ); | ||
| const slowCfg = { ...cfg, timeoutMs: 50 }; | ||
| await expect( | ||
| analyze(slowCfg, { kind: "image", url: "https://v/i.png", prompt: "p", maxTokens: 5 }), | ||
| ).rejects.toMatchObject({ status: "timeout" }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Missing test for the "network" error branch.
analyze() maps a plain fetch rejection (non-AbortError) to BailianError with status: "network", but no test exercises that path — only the abort/timeout branch is covered here. MSW v2's HttpResponse.error() simulates a network failure and would close this gap.
🧪 Suggested additional test
+ it("maps a network failure to a network error", async () => {
+ server.use(http.post(endpoint, () => HttpResponse.error()));
+ await expect(
+ analyze(cfg, { kind: "image", url: "https://v/i.png", prompt: "p", maxTokens: 5 }),
+ ).rejects.toMatchObject({ status: "network" });
+ });As per coding guidelines, "Add a test for every new tool or branch of logic; maintain at least 85% coverage."
📝 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.
| it("maps an abort to a timeout error", async () => { | |
| server.use( | |
| http.post(endpoint, async () => { | |
| await new Promise((r) => setTimeout(r, 1000)); | |
| return jsonOk(); | |
| }), | |
| ); | |
| const slowCfg = { ...cfg, timeoutMs: 50 }; | |
| await expect( | |
| analyze(slowCfg, { kind: "image", url: "https://v/i.png", prompt: "p", maxTokens: 5 }), | |
| ).rejects.toMatchObject({ status: "timeout" }); | |
| }); | |
| it("maps an abort to a timeout error", async () => { | |
| server.use( | |
| http.post(endpoint, async () => { | |
| await new Promise((r) => setTimeout(r, 1000)); | |
| return jsonOk(); | |
| }), | |
| ); | |
| const slowCfg = { ...cfg, timeoutMs: 50 }; | |
| await expect( | |
| analyze(slowCfg, { kind: "image", url: "https://v/i.png", prompt: "p", maxTokens: 5 }), | |
| ).rejects.toMatchObject({ status: "timeout" }); | |
| }); | |
| it("maps a network failure to a network error", async () => { | |
| server.use(http.post(endpoint, () => HttpResponse.error())); | |
| await expect( | |
| analyze(cfg, { kind: "image", url: "https://v/i.png", prompt: "p", maxTokens: 5 }), | |
| ).rejects.toMatchObject({ status: "network" }); | |
| }); |
🤖 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/bailian.test.ts` around lines 123 - 134, Add a test alongside the
existing timeout test that makes the mocked POST handler return MSW v2’s
HttpResponse.error(), then call analyze with the same request shape and assert
rejection with status "network". Keep the existing abort/timeout test unchanged
and target the plain fetch-rejection branch in analyze.
Source: Coding guidelines
| import { loadConfig } from "../src/config.js"; | ||
|
|
||
| const LIVE = process.env.LIVE === "1" && !!process.env.DASHSCOPE_API_KEY; | ||
| const LOCAL_ASSET_DIR = "/home/sommio/Downloads/test10-v3/cat_dialogue_10/cat_dialogue_000001"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Hardcoded personal local path.
LOCAL_ASSET_DIR embeds a specific developer's home directory and folder layout directly in a committed test file. It's gated by existsSync so it won't break other contributors' runs, but it's dead code for everyone else and bakes personal filesystem details into the repo. Prefer sourcing it from an env var with this as a documented example/fallback.
🧹 Suggested fix
-const LOCAL_ASSET_DIR = "/home/sommio/Downloads/test10-v3/cat_dialogue_10/cat_dialogue_000001";
+const LOCAL_ASSET_DIR = process.env.LIVE_LOCAL_ASSET_DIR ?? "";📝 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 LOCAL_ASSET_DIR = "/home/sommio/Downloads/test10-v3/cat_dialogue_10/cat_dialogue_000001"; | |
| const LOCAL_ASSET_DIR = process.env.LIVE_LOCAL_ASSET_DIR ?? ""; |
🤖 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 8, Replace the hardcoded value in LOCAL_ASSET_DIR
with an environment-variable-based configuration, using the current path only as
a documented example or fallback rather than embedding personal filesystem
details in the test. Preserve the existing existsSync gating and local asset
test behavior for contributors who provide the directory.
| it("summarize_video detailed uses the detailed prompt and 1024 tokens", async () => { | ||
| const cap = mockCapture(); | ||
| await withClient(async (client) => { | ||
| const r = await client.callTool({ | ||
| name: "summarize_video", | ||
| arguments: { video_url: "https://v/x.mp4", style: "detailed" }, | ||
| }); | ||
| expect(textOf(r)).toBe("answer"); | ||
| }); | ||
| const body = await cap.body(); | ||
| expect(body.max_tokens).toBe(1024); | ||
| const prompt = (body.messages as { content: { text?: string }[] }[])[0]!.content[0]!.text ?? ""; | ||
| expect(prompt).toContain("comprehensive"); | ||
| }); | ||
|
|
||
| it("maps a backend 500 to an isError tool result", async () => { | ||
| server.use(http.post(endpoint, () => new HttpResponse(null, { status: 500 }))); | ||
| await withClient(async (client) => { | ||
| const r = await client.callTool({ | ||
| name: "analyze_video", | ||
| arguments: { video_url: "https://v/x.mp4" }, | ||
| }); | ||
| expect(r.isError).toBe(true); | ||
| expect(textOf(r)).toContain("HTTP 500"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
extract_video_text tool has no test coverage.
Every other one of the 8 registered tools has a dedicated behavioral test, but extract_video_text is only referenced by name in the "exposes all 8 tools" list — its request wiring (video kind, TEXT_EXTRACTION_PROMPT, 1024 max tokens per src/server.ts) is never exercised.
🧪 Suggested additional test
+ it("extract_video_text sends the extraction prompt and 1024 tokens", async () => {
+ const cap = mockCapture();
+ await withClient(async (client) => {
+ const r = await client.callTool({
+ name: "extract_video_text",
+ arguments: { video_url: "https://v/x.mp4" },
+ });
+ expect(textOf(r)).toBe("answer");
+ });
+ const body = await cap.body();
+ expect(body.max_tokens).toBe(1024);
+ });As per coding guidelines, "Add a test for every new tool or branch of logic; maintain at least 85% coverage."
📝 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.
| it("summarize_video detailed uses the detailed prompt and 1024 tokens", async () => { | |
| const cap = mockCapture(); | |
| await withClient(async (client) => { | |
| const r = await client.callTool({ | |
| name: "summarize_video", | |
| arguments: { video_url: "https://v/x.mp4", style: "detailed" }, | |
| }); | |
| expect(textOf(r)).toBe("answer"); | |
| }); | |
| const body = await cap.body(); | |
| expect(body.max_tokens).toBe(1024); | |
| const prompt = (body.messages as { content: { text?: string }[] }[])[0]!.content[0]!.text ?? ""; | |
| expect(prompt).toContain("comprehensive"); | |
| }); | |
| it("maps a backend 500 to an isError tool result", async () => { | |
| server.use(http.post(endpoint, () => new HttpResponse(null, { status: 500 }))); | |
| await withClient(async (client) => { | |
| const r = await client.callTool({ | |
| name: "analyze_video", | |
| arguments: { video_url: "https://v/x.mp4" }, | |
| }); | |
| expect(r.isError).toBe(true); | |
| expect(textOf(r)).toContain("HTTP 500"); | |
| }); | |
| }); | |
| it("extract_video_text sends the extraction prompt and 1024 tokens", async () => { | |
| const cap = mockCapture(); | |
| await withClient(async (client) => { | |
| const r = await client.callTool({ | |
| name: "extract_video_text", | |
| arguments: { video_url: "https://v/x.mp4" }, | |
| }); | |
| expect(textOf(r)).toBe("answer"); | |
| }); | |
| const body = await cap.body(); | |
| expect(body.max_tokens).toBe(1024); | |
| }); | |
| it("summarize_video detailed uses the detailed prompt and 1024 tokens", async () => { | |
| const cap = mockCapture(); | |
| await withClient(async (client) => { | |
| const r = await client.callTool({ | |
| name: "summarize_video", | |
| arguments: { video_url: "https://v/x.mp4", style: "detailed" }, | |
| }); | |
| expect(textOf(r)).toBe("answer"); | |
| }); | |
| const body = await cap.body(); | |
| expect(body.max_tokens).toBe(1024); | |
| const prompt = (body.messages as { content: { text?: string }[] }[])[0]!.content[0]!.text ?? ""; | |
| expect(prompt).toContain("comprehensive"); | |
| }); | |
| it("maps a backend 500 to an isError tool result", async () => { | |
| server.use(http.post(endpoint, () => new HttpResponse(null, { status: 500 }))); | |
| await withClient(async (client) => { | |
| const r = await client.callTool({ | |
| name: "analyze_video", | |
| arguments: { video_url: "https://v/x.mp4" }, | |
| }); | |
| expect(r.isError).toBe(true); | |
| expect(textOf(r)).toContain("HTTP 500"); | |
| }); | |
| }); |
🤖 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/tools.test.ts` around lines 121 - 146, Add a dedicated behavioral test
for the extract_video_text tool alongside the existing tool tests, invoking it
with a video_url and asserting the response, captured request video kind,
TEXT_EXTRACTION_PROMPT content, and max_tokens of 1024. Keep the test consistent
with the existing mockCapture, withClient, and textOf patterns.
Source: Coding guidelines
The dummy value `sk-secret-key-1234567890` in test/tools.test.ts trips gitleaks' default `generic-api-key` rule (entropy 4.02) and fails the Secret Scan CI job on PR #1. This is a test-only fixture explicitly endorsed by AGENTS.md ("Tests use dummy values ... sk-secret-key-…"), not a real key — the value is also asserted by the redaction test at line 183, so it cannot be changed. Suppress the single line with a `gitleaks:allow` inline comment rather than weakening the rule set globally. Verified locally with gitleaks 8.24.3 (matching CI): test/ scans clean, .env (gitignored, real key) is the only remaining finding and is not tracked.
The dummy value `sk-secret-key-1234567890` in test/tools.test.ts trips gitleaks' default `generic-api-key` rule (entropy 4.02) and fails the Secret Scan job on PR #1. It is a test-only fixture explicitly endorsed by AGENTS.md ("Tests use dummy values ... sk-secret-key-…"), and the literal value is asserted by the redaction test at line 183, so it cannot be changed. An inline `// gitleaks:allow` comment suppresses file-level scans but NOT gitleaks' PR-range scan: gitleaks inspects each commit patch separately, and the secret was introduced in 17822e7 before any allow comment existed, so that patch is still flagged. A per-line comment cannot retroactively suppress a finding in an earlier commit's patch. Fix: add .gitleaks.toml that extends the built-in default config (`extend = { useDefault = true }`, keeping every default rule) and allowlists only the exact dummy string via a global [allowlist] regex. Real Bailian keys (sk-ws-…) are never allowlisted. Verified locally with gitleaks 8.24.3 (matching CI): the PR commit range scans clean.
4463fcc to
4c1e331
Compare
1 participant
Summary
Node/TypeScript MCP server exposing 8 video/image understanding tools backed by Bailian (DashScope) OpenAI-compatible endpoint, model
qwen3.7-plus(native multimodal — no client-side frame extraction). Replaces the reference Modal-basedqwen-video-mcp-serverwith anpx-launchable, agent-friendly server.What's included
analyze_video,analyze_image,summarize_video,extract_video_text,video_qa,compare_video_frames,check_endpoint_status,list_capabilities${DASHSCOPE_BASE_URL}/chat/completions(OpenAI-compatible),video_url/image_urlcontent blocks; env-driven model, defaultqwen3.7-plusnoUncheckedIndexedAccess+exactOptionalPropertyTypes), eslintstrictTypeChecked(--max-warnings 0), prettier, vitest + msw (in-memory MCP e2e), coverage ≥85%check-secrets.mjsblocks.envfiles +sk-ws-…keys + lint-staged + gitleaks-if-present), pre-push (typecheck + test);check_endpoint_statusredacts the key; leak-assertion testci.yml(Node 20/22 matrix),secrets-scan.yml(gitleaks),smoke-live.yml(weekly + manual, real image call)AGENTS.mdhard rules (no--no-verify, no key commits, env-only,trashnotrm),README.md, PR templateVerification
npm run typecheck/lint/format:check/test/build/coverage— all greenLIVE=1qwen3.7-plus(bundled 256×256 fixture + local cat image)Out of scope
npx tsxworks now; publish is a follow-up)npm auditreports 13 dev-dep transitive advisories (msw/vitest chain); non-blockingFragile assumptions (see AGENTS.md)
video_urlforqwen3.7-plus— image path verified live; video unverified.qwen3.7-plus— verified working.Checklist
npm run typecheckpassesnpm run lintpasses (--max-warnings 0)npm run format:checkpassesnpm testpasses.envfiles committedgit commit --no-verifyLIVE=1(image path)