feat(sdk): add serve-bridge MCP server & rename mcp → daemon-mcp#4555
Conversation
📋 Review SummaryThis PR adds an MCP Server bridge ( 🔍 General Feedback
🎯 Specific Feedback🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
jifeng
force-pushed
the
feat/sdk-serve-bridge-pr
branch
from
6ea0f84 to
ea889c9
Compare
🔧 Review Feedback 已修复针对 GitHub Actions Review Summary 的反馈，已在 commit 🟡 High Priority（之前 commit 已修复）
🟢 Medium Priority（本次修复）
🔵 Low Priority（本次修复）
🔵 Low Priority（暂不处理）
|
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Additional findings not covered by inline comments:
[Critical] Lint errors (auto-fixable): workspaceWrite.ts has 7 arrow-body-style violations (lines 55, 92, 105, 118, 129, 142, 167) and 1 missing default case in switch (line 176). Run npm run lint:fix to resolve.
[Suggestion] Test coverage gaps: startEventStream SSE event processing loop (the core data path), stopEventStream side effects, and workspace_agents_manage 5-branch switch have zero test coverage. The current test file covers registration plumbing and core helpers well, but the SSE data path — the most novel and failure-prone part — lacks behavioral tests.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
测试覆盖缺口（无法映射到具体 diff 行）：
- SSE 生命周期模块零测试覆盖 —
sse.ts中的startEventStream、stopEventStream、startSessionCleanup是最复杂的状态管理函数（SSE for-await 循环、idle TTL 清理、collector resolve），但没有任何专项测试。 workspace_agents_manage5 个 switch 分支 + 4 条验证路径全部未测试 — 这是分支最多的工具，handler 从未被调用。- 31 个工具中 27 个 handler 从未在测试中调用 — 测试仅验证注册数量和名称，不验证行为。
- 确定性检查：tsc 报
serve-bridge.test.ts:432缺少lastActivityMs属性（TS2741）；eslint 报workspaceWrite.ts中 7 处arrow-body-style违规和 1 处default-case缺失。
— qwen3.7-max via Qwen Code /review
doudouOUC
force-pushed
the
daemon_mode_b_main
branch
from
f4d03f1 to
1284383
Compare
🔧 P0/P1/P2 Review Feedback 全部修复感谢 @qwen-code-ci-bot 和 @wenshao 的详细评审！已分三个 commit 完成所有修复： 🔴 P0 —
|
| 反馈 | 修复 |
|---|---|
sse.ts _meta 检查在错误层级（检查 content 应为 update），导致每次 prompt 强制等 30s |
改为 '_meta' in update，SSE 事件正确 resolve collector |
workspaceWrite.ts scope: 'global' 允许 MCP 客户端写全局配置（跨 workspace prompt injection） |
默认禁止 global scope，需设置 QWEN_BRIDGE_ALLOW_GLOBAL_SCOPE=true 才可启用 |
serve-bridge.test.ts TS2741 缺少 lastActivityMs |
补全 mock 对象字段 |
🟡 P1 — 8430045d7
| 反馈 | 修复 |
|---|---|
session_load/session_resume SSE 连接泄漏 |
切换 session 前先 stopEventStream 旧 default session |
SSE catch {} 静默吞错 + finally 不 resolve collector |
记录非 AbortError 日志 + finally 中 resolve activeCollector |
| 并发 prompt 竞态（第二个覆盖第一个 collector） | 若 stream.activeCollector 已存在则立即抛错 |
🟢 P2 — 80cdcbd99
| 反馈 | 修复 |
|---|---|
close() 不 abort 活跃 SSE 流 |
shutdown 时遍历 eventStreams 逐个 stopEventStream |
file_write replace 模式空字符串作为 hash |
前置校验：replace 必须传 expected_hash |
Promise.race 中 setTimeout 泄漏 |
正常 resolve 后 clearTimeout(timeoutId!) |
prompt_cancel 不 resolve collector（cancel 后仍等 30s） |
cancel 后立即 stream?.activeCollector?.resolve() |
session_create 先 stop 旧 SSE 再创建新 session（失败则两边都没流） |
先确认新 session 成功，再 stop 旧 SSE |
session_close 先 stop SSE 再 HTTP close（失败则不一致） |
先 HTTP close，再 stop SSE |
daemonFetch/authHeaders 死代码 |
删除函数及对应测试 |
| 超时后部分响应无法区分完整 vs 截断 | 返回 stop_reason: 'timeout' + warning 字段 |
bin.ts 注释路径是旧的 dist/mcp/... |
修正为 dist/daemon-mcp/serve-bridge/bin.js |
session_load/resume 缺少 workspaceCwd 回退 |
添加 ?? state.workspaceCwd |
daemonUrl.replace(/\/+$/, '') ReDoS regex |
替换为手写 stripTrailingSlashes 循环 |
Lint: 7 处 arrow-body-style + switch 缺 default case |
全部修复 |
📝 剩余未处理项
| 反馈 | 说明 |
|---|---|
| 测试覆盖（SSE 数据路径、27 个 handler 行为测试） | 认同需要补充，将在后续 PR 中增加 |
prompt() HTTP 调用无超时上限 |
需要 DaemonClient 层面支持 AbortSignal，后续跟进 |
tsc ✅ 通过，无类型错误。
🔀 Merge Conflicts 已解决
冲突处理策略：
tsc ✅ 通过。 |
doudouOUC
force-pushed
the
daemon_mode_b_main
branch
from
1284383 to
1ca0572
Compare
|
| Item | Value |
|---|---|
| OS | macOS 26.4.1 (Darwin 25.4.0) |
| Node | v22.17.0 |
| Package | @qwen-code/sdk@0.1.8 |
| Tested commit | ea80fe4e9 (PR head) |
| Base | daemon_mode_b_main (40 commits ahead) |
| Runner | tmux session pr4555, vitest 1.6.1 |
Test Plan (PR body) results
| Step | Result |
|---|---|
npx vitest run test/unit/serve-bridge.test.ts |
✅ 18 passed (PR body says 23 — see note below) |
node dist/.../bin.js initialize → serverInfo |
✅ {name: "qwen-serve-bridge", version: "1.0.0"}, protocolVersion: "2025-06-18" (via tsx — see Issue 1) |
tools/list → 31 tools |
✅ confirmed 31 unique tool names |
Note on test count — PR body claims 23 tests; current
serve-bridge.test.tshas 18it()blocks. The descriptive table claim is stale (likely from before the persistent-SSE refactor). All 18 currently present tests pass.
Additional checks
| Step | Result |
|---|---|
Full packages/sdk-typescript test suite (npx vitest run) |
✅ 15 files / 674 tests all pass (24.47s) |
tsc --noEmit (typecheck) |
✅ exit 0, no errors |
eslint packages/sdk-typescript/src/daemon-mcp/serve-bridge ... (root eslint) |
✅ exit 0, no findings |
npm run build --workspace=packages/sdk-typescript |
✅ exit 0, completes in 10.71s |
31-tool registration assertion (should aggregate to exactly 31 tools) |
✅ passes |
Issue 1 — bin entry points to a file the build doesn't produce ⚠️
packages/sdk-typescript/package.json declares:
"bin": {
"qwen-serve-mcp": "./dist/daemon-mcp/serve-bridge/bin.js"
}…but the build pipeline does NOT emit this file:
$ ls packages/sdk-typescript/dist/daemon-mcp/serve-bridge/
bin.d.ts ← only the type declaration
createServeBridgeMcpServer.d.ts
helpers.d.ts
index.d.ts
sse.d.ts
tools/
types.d.ts
$ node packages/sdk-typescript/dist/daemon-mcp/serve-bridge/bin.js
Error: Cannot find module '.../dist/daemon-mcp/serve-bridge/bin.js'
Root cause: tsconfig.build.json sets "emitDeclarationOnly": true, and scripts/build.js only esbuild-bundles src/index.ts and src/daemon/index.ts — the new bin.ts (and the rest of daemon-mcp/serve-bridge/*) gets no JS output. Diff vs base confirms PR 4555 added the bin entry + bin.ts source but did not extend the build to emit them.
Impact: after npm install (or npm publish), invoking the registered bin (npx qwen-serve-mcp or the path documented in the PR body / serve-bridge/README.md) fails immediately with "Cannot find module". The MCP server is effectively un-runnable from a published install.
Suggested fix: extend scripts/build.js with an esbuild entry for src/daemon-mcp/serve-bridge/bin.ts (output dist/daemon-mcp/serve-bridge/bin.js, with a #!/usr/bin/env node shebang and chmod +x), or remove emitDeclarationOnly for the daemon-mcp/serve-bridge/ subtree. Either way, please add a smoke test (e.g. node dist/daemon-mcp/serve-bridge/bin.js < /dev/null) so this regresses loudly next time.
Local protocol verification was done by running
tsx src/daemon-mcp/serve-bridge/bin.tsdirectly. The runtime logic itself is correct — it's purely the build pipeline that's missing the entry.
Issue 2 — Prettier formatting drift in 5 PR files (non-blocking)
$ npx prettier --check packages/sdk-typescript/src/daemon-mcp/serve-bridge ...
[warn] src/daemon-mcp/serve-bridge/README.md
[warn] src/daemon-mcp/serve-bridge/tools/session.ts
[warn] src/daemon-mcp/serve-bridge/tools/workspaceRead.ts
[warn] src/daemon-mcp/serve-bridge/tools/workspaceWrite.ts
[warn] src/daemon-mcp/serve-bridge/types.ts
Differences are line-wrap nits (e.g., 80-char line-length wraps not applied). CI uses prettier --write (auto-fix) rather than --check, so it won't fail CI — but the files don't match the project's own .prettierrc (printWidth: 80). One npx prettier --write packages/sdk-typescript/src/daemon-mcp/serve-bridge resolves it.
Recommendation
Hold on merge until Issue 1 is addressed. Tests, types, and runtime behavior all check out, but shipping a bin entry that points to a non-existent file would mean every external consumer following serve-bridge/README.md hits a "Cannot find module" wall on first run. Issue 2 is purely cosmetic and can be fixed in the same follow-up.
Once bin.js is produced by build (and ideally a smoke-test gate added), this PR is good to land.
— wenshao
jifeng
force-pushed
the
feat/sdk-serve-bridge-pr
branch
from
ea80fe4 to
5b0eee9
Compare
Expose qwen serve's HTTP endpoints as MCP tools via a stdio-based MCP server. This allows any MCP-compatible client (Claude Desktop, Cursor, VS Code, etc.) to interact with a running qwen serve daemon directly through the standard MCP protocol. The bridge provides 31 tools covering session lifecycle, agent interaction (prompt/cancel), workspace file operations, and workspace configuration management. A standalone bin entry (`qwen-serve-mcp`) is included for direct CLI usage.
Includes usage instructions, environment variables, MCP client configuration examples, tool listing, session management notes, and verification commands.
…ignal handling - file_stat now calls GET /stat instead of readWorkspaceFile fallback - dir_list now calls GET /list for proper directory listing - glob now calls GET /glob for pattern matching - Add daemonFetch() helper for raw HTTP calls to endpoints not in DaemonClient - Add SIGINT/SIGTERM graceful shutdown in bin.ts - Add unhandledRejection handler to prevent silent crashes - Exit cleanly when stdin pipe closes (parent process gone)
Document three configuration methods: npx (zero-install), global install, and local path (dev). Clarify Node >=22 requirement and add qwen serve startup options.
The prompt endpoint only returns stopReason synchronously. Actual response content is streamed via session SSE events. Now the prompt tool subscribes to events in parallel, collects agent_message_chunk texts, and returns the full response in the result.
Rename the MCP utilities directory to better reflect its role as daemon-specific MCP tooling. Update all import paths in index.ts, Query.ts, and the bin entry in package.json.
🔬 Maintainer Verification Report环境: macOS Darwin 25.4.0, Node.js v22.17.0 ✅ Build
✅ Unit Tests
✅ E2E: MCP Protocol Handshake (stdio)✅ E2E: Session Lifecycle (qwen serve daemon)通过 tmux 启动
✅ E2E: Workspace Tools
✅ E2E: Security Guards
🐛 Minor Issues Found
结论本 PR 功能验证通过，可以 merge。 核心功能（MCP 协议握手、session 生命周期、workspace 工具、agent prompt/response）和安全防护（全局作用域限制、权限升级阻断）均按预期工作。唯一的 minor issue 是 double shebang 不影响生产使用（MCP 客户端通过 shebang 执行）。 |
- Add allowGlobalScope guard to workspace_mcp_restart (consistent with workspace_tool_toggle — restarting MCP servers is equally disruptive) - Remove scope from hasField check in handleAgentUpdate (scope is a routing parameter, not an update field — passing only scope would POST an empty body to the daemon)
R10 Review Fix Summary (commit
|
| 问题 | 修复 | 文件 |
|---|---|---|
handleAgentUpdate 的 hasField 误含 scope（scope 是路由参数不是更新字段，只传 scope 会导致空 body POST） |
从 hasField 检查中移除 scope，更新错误提示 |
workspaceWrite.ts |
workspace_mcp_restart 缺 allowGlobalScope 守卫（重启 MCP server 可中断运行中的调用，与 workspace_tool_toggle 不一致） |
加 allowGlobalScope 守卫 |
workspaceWrite.ts |
延后处理
| 问题 | 原因 |
|---|---|
fileStat()/dirList()/glob() 返回 Promise<unknown> 缺少类型定义 |
属于 DaemonClient 范围，需定义新类型接口，改动较大，计划下个迭代处理 |
Maintainer Verification ReportPR: #4555 — feat(sdk): add serve-bridge MCP server & rename mcp → daemon-mcp Build & Type Check
Unit Tests
New Test Coverage (30 tests)
Code Review NotesSecurity:
Robustness:
Refactor:
VerdictReady to merge. Comprehensive test coverage (30 new tests), proper security guards, robust SSE lifecycle, clean refactor. All 686 SDK tests pass, no regressions. Verified by: wenshao |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
整体架构清晰，SSE 持久连接 + PromptCollector 模式设计合理，allowGlobalScope 安全守卫覆盖到位，测试覆盖了关键路径（23 tests）。
以下 inline comments 按优先级分 High / Medium / Low 三档，共 8 条。
…sages - Remove runtime re-exports from types.ts; tool files now import directly from sse.js/helpers.js to avoid circular dependency risks - Add best-effort comment on SSE error event regex explaining limitations - Rewrite prompt tool description to clarify 30s is post-response collection timeout, not overall timeout - Split approval mode error messages: distinguish dangerous-mode vs persist-restricted cases - Mark name parameter as (create only) in agents_manage schema - Log close errors in shutdown instead of silently swallowing
jifeng
force-pushed
the
feat/sdk-serve-bridge-pr
branch
from
93fc8e5 to
620b503
Compare
R11 Review Fix Summary (commit
|
| 问题 | 修复 | 文件 |
|---|---|---|
types.ts 同时 re-export 运行时函数，命名与内容不符，有循环依赖风险 |
移除所有运行时 re-export，tool 文件直接从 sse.js/helpers.js 导入，types.ts 仅保留类型定义 |
9 个文件 |
/error|fail/i 正则过于宽泛，可能误匹配或漏检 |
加注释说明 best-effort 匹配限制，待 daemon 定义正式 error 事件枚举后更新 | sse.ts |
Medium 修复
| 问题 | 修复 | 文件 |
|---|---|---|
| prompt 描述 "Times out after 30s" 误导客户端设置过短 timeout | 改为明确说明工具可阻塞数分钟，30s 仅为 post-response completion signal 收集超时 | agent.ts |
persist: true + default 模式的错误消息混淆 |
区分两种 case：dangerous mode restricted vs persisting changes restricted | workspaceWrite.ts |
name 参数在 update 中被忽略但 schema 未标注 |
标注 (create only, required for create) |
workspaceWrite.ts |
Low 修复
| 问题 | 修复 | 文件 |
|---|---|---|
shutdown() catch 吞掉错误，调试无线索 |
改为 process.stderr.write 输出错误详情 |
bin.ts |
无需修复
| 问题 | 原因 |
|---|---|
formatters.ts 新文件删减旧导出可能影响 createSdkMcpServer.ts |
serve-bridge 的 formatters.ts 和 createSdkMcpServer.ts 引用的是不同路径的文件，不存在影响 |
延后处理
| 问题 | 原因 |
|---|---|
fileStat()/dirList()/glob() 返回 Promise<unknown> |
属于 DaemonClient 范围，需定义新类型接口，计划下个迭代处理 |
验证
- tsc: 通过
- vitest: 30/30 测试全部通过
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
二轮 review：补充并发安全、dead code、生命周期方面的额外发现（5 条）。
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
[Suggestion] sse.ts:96-100 — 新增注释承认 /error|fail/i 可能产生 false positive（如 default_fallback），但下游 agent.ts 的 interrupted 分支返回 "SSE stream was closed before the response completed" 警告。在 false positive 触发时，SSE 流实际并未关闭，该警告具有误导性。建议将 warning 改为更准确的措辞，如 "Response collection was interrupted before the completion signal was received."，或在 PromptCollector 上增加 interruptReason 字段以区分 SSE 断连和 regex 误匹配。
— qwen3.7-max via Qwen Code /review
Verification Report — PR #4555Reviewer: wenshao 1. Build & Type Check
2. Unit Tests
Zero regressions from the 3. MCP Protocol Verification (stdio)Tested
4. Code Review Findings🚨 P0 — Blocker
|
| Issue | Details |
|---|---|
DaemonClient new methods return Promise<unknown> |
fileStat(), dirList(), glob() all return Promise<unknown>, unlike every other method in the class which uses typed interfaces. This removes compile-time safety for consumers. Proper response types should be defined. |
Inconsistent path descriptions in tool schemas |
file_read says "relative to workspace root" but file_stat and dir_list just say "File path to stat" / "Directory path to list" — LLM consumers could attempt absolute paths. |
💡 P2 — Nice to Have
| Issue | Details |
|---|---|
| No client-side path sanitization | All file tools trust the daemon entirely for path validation. A lightweight reject-absolute-paths / reject-..-segments check at the SDK level would add defense-in-depth. |
workspace_agents_manage overloads 5 CRUD actions into one flat zod schema |
Required-for-create validation is done manually in the handler, not at the schema level. A discriminated union or per-action tools would catch invalid requests earlier. |
SSE error event detection uses fragile /error|fail/i regex |
Acknowledged in comments. Should be revisited when daemon publishes a formal error event enum. |
file_write in create mode still forwards expected_hash |
Semantically meaningless for creation; SDK should strip it or document the pass-through. |
Missing clientId parameter on new DaemonClient methods |
Existing readWorkspaceFile() accepts optional clientId but new methods omit it. |
✅ Positive Observations
- SSE lifecycle management is well-structured with three cleanup paths (explicit stop, async loop
finally, idle TTL sweep) - Concurrent prompt guard is TOCTOU-safe in Node.js's single-threaded model
allowGlobalScopedefaults tofalse, properly gating dangerous write operations- Token handling via env vars, never logged or exposed in error messages
- Signal handling covers SIGINT, SIGTERM, stdin close, and unhandled rejections
stripTrailingSlashesexplicitly avoids regex to dodge ReDoS concerns- All imports correctly reference
daemon-mcppath — no stalemcp/references remain
5. Summary
The serve-bridge architecture is solid: clean factory pattern, well-separated concerns, comprehensive tool coverage (31 tools), and good SSE lifecycle management. All 686 tests pass with zero regressions.
However, the P0 double-shebang bug makes bin.js non-functional after npm run build. This must be fixed before merge — it's a one-line fix (remove the shebang from src/daemon-mcp/serve-bridge/bin.ts).
Recommendation:
Verified locally on 2026-05-27
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
三轮 review：SSE 韧性、事件可见性、API 一致性方面的补充发现（3 条）。
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
LGTM — 架构清晰，安全防护到位，SSE 生命周期管理完善。
几个非阻塞建议供参考：
workspace_agents_managescope 校验：update/delete操作在scope为 undefined 时会绕过validateGlobalScope检查。建议当allowGlobalScope=false时，对写操作强制 scope 必须为'workspace'（或拒绝 undefined）。state.daemonUrl/state.token疑似冗余：所有 tool handler 已通过state.client.*调用，这两个字段无使用方。- DaemonClient 新方法返回
Promise<unknown>：fileStat/dirList/glob建议定义具体返回类型。 - SSE 错误检测 regex
/error|fail/i：可能误判"default_fallback"等字符串，建议用精确 Set 匹配。 server.instance.server.onclose覆盖：建议链式调用已有 handler 而非直接赋值。
🤖 Generated with Qoder
5 participants
Summary
为
qwen servedaemon 添加 MCP Server 桥接层（qwen-serve-bridge），使任何 MCP 兼容客户端（Qoder、Claude Desktop、Cursor）可通过 stdio 协议与 qwen-code agent 交互。本次改动的 Commits
ce7a8afc50f2f6e6a99081584598d0e287ec0495564e829f91672ee6d20dddf7ccae99712eaef88299b505eb95e133f076e1d6535d95902233b39核心变更
新增: qwen-serve-bridge MCP Server
qwen-serve-mcpbin，通过环境变量配置 (QWEN_DAEMON_URL,QWEN_DAEMON_TOKEN)重构: src/mcp → src/daemon-mcp
DaemonClient 扩展
fileStat(),dirList(),glob()方法外部项目使用方式
{ "mcpServers": { "qwen-serve-bridge": { "command": "node", "args": ["/path/to/packages/sdk-typescript/dist/daemon-mcp/serve-bridge/bin.js"], "env": { "QWEN_DAEMON_URL": "http://127.0.0.1:4170", "QWEN_DAEMON_TOKEN": "qwen-local-dev" } } } }Validation
文件改动范围 (仅列出本次新增/修改)
demo show