feat(serve): Add workspace-qualified Voice - #6839
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Thanks for the PR! Template looks good ✓ — all required sections present, bilingual, linked to #6378 and #6745. Problem: This is an observed architectural gap, not a theoretical concern. Multi-workspace daemons already isolate sessions, REST, ACP, and lifecycle, but Voice remained bound to the primary workspace. Phase 4b of the workspace qualification roadmap is the natural place to close this. The PR description is clear about what's in scope and what isn't. Direction: Aligned. This extends the existing workspace-qualified routing pattern (already used for settings, files, trust, lifecycle) to the Voice surface. Adding Size: 40 changed files. ~1,443 production logic lines (excluding ~1,702 test lines and ~50 docs lines). Core path touch is minimal — Approach: The architecture is clean and follows existing patterns well:
No concerns blocking review. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ — 所有章节齐全，双语，关联了 #6378 和 #6745。 问题：这是一个已识别的架构缺口，不是理论性问题。多 workspace daemon 已经隔离了 sessions、REST、ACP 和生命周期，但 Voice 仍然绑定 primary workspace。Phase 4b 是补齐这个缺口的自然阶段。PR 描述对范围内和范围外的内容都很清晰。 方向：对齐。这延续了已有的 workspace-qualified 路由模式（已用于 settings、files、trust、lifecycle 等），将 Voice 表面纳入覆盖。新增 规模：40 个文件变更，约 1,443 行生产逻辑代码（排除约 1,702 行测试和约 50 行文档）。核心路径触及极少—— 方案：架构干净，很好地遵循了已有模式：
无阻塞性顾虑，进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Code ReviewIndependent proposal before reading the diff: I would have added workspace-qualified REST routes ( The PR's approach matches this proposal and exceeds it in a few ways:
No critical issues found. The code is well-structured, follows existing patterns, and the new abstractions are minimal and well-scoped. Reuse check: The coordinator reuses the existing Test ResultsTmux real-scenario testing: N/A — this is a daemon protocol/lifecycle/SDK change with no TUI or Web Shell UI. The PR author notes this explicitly. Verification is through unit tests. Unit tests (run on the PR branch in an isolated worktree): Typecheck: passed (all 5 packages: cli, core, sdk, web-shell, webui). 中文说明代码审查在阅读 diff 之前我的独立方案：添加 workspace-qualified REST 路由（ PR 的方案与我的方案一致并在几处超越：
未发现关键问题。 代码结构良好，遵循已有模式，新抽象最小且范围明确。 复用检查： coordinator 复用了已有的 测试结果Tmux 真实场景测试：不适用 — 仅涉及 daemon 协议、生命周期和 SDK 变更，无 TUI 或 Web Shell UI 变更。 单元测试（在隔离 worktree 的 PR 分支上运行）：409 项测试全部通过，0 失败。 类型检查： 通过（全部 5 个 package：cli、core、sdk、web-shell、webui）。 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 3/5 — Clean review across every stage, but Stage 0 flagged 1000+ production lines touching core services, which needs a maintainer's sign-off before auto-approval. Stepping back: this PR does exactly what Phase 4b requires — no more, no less. Workspace-qualified Voice routes follow the same pattern already established for settings, files, trust, and lifecycle. The My independent proposal was simpler (bare counter, no lease abstraction) — the PR's approach is better because it gives every consumer (REST routes, WebSocket, coordinator) a uniform admission interface and built-in abort. The code I would have written needed more plumbing to achieve the same disposal semantics. Every change in the diff is needed for the stated goal. No drive-by refactors, no scope creep. The 409 unit tests pass, typecheck and lint are clean. Tests cover the new coordinator lifecycle, qualified route resolution and trust enforcement, abort-during-connect in all three stream openers, and the SDK delegation methods. This is a well-executed Phase 4b feature. Deferring to a maintainer for the final call given the size advisory. 中文说明置信度：3/5 — 每个阶段审查均干净，但 Stage 0 标记了 1000+ 生产行触及核心 services，需要维护者确认后才能自动批准。 退一步看：这个 PR 准确完成了 Phase 4b 的要求——不多不少。Workspace-qualified Voice 路由遵循了 settings、files、trust、lifecycle 已建立的相同模式。 我的独立方案更简单（裸计数器，无 lease 抽象）——PR 的方案更好，因为它给每个消费者（REST 路由、WebSocket、coordinator）提供了统一的 admission 接口和内建的 abort 机制。我写的代码需要更多额外管道才能达到相同的销毁语义。 diff 中的每个改动都是实现目标所必需的。没有顺手重构，没有范围蔓延。 409 项单元测试通过，类型检查和 lint 均干净。测试覆盖了新的 coordinator 生命周期、qualified 路由解析和信任执行、三个 stream opener 的连接中 abort，以及 SDK 委托方法。 这是一个执行良好的 Phase 4b 功能。因规模建议转交维护者做最终决定。 — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
✅ Local verification (maintainer) — build, typecheck, lint & focused tests all greenI built this PR from source and ran the real toolchain in an isolated worktree to corroborate the author's test plan before merge. Environment
Quality gate
Focused test suite — the 15 files this PR touches
These focused tests directly exercise the behaviors in the Reviewer Test Plan:
Controlled experiment — is the invariant load-bearing?To confirm the tests aren't vacuous, I broke only the coordinator's central invariant — the process-global cap of 8 shared across runtimes — by changing
VerdictBuild, typecheck, lint, and all 15 touched test files pass locally, matching the PR's claims. The change is cleanly additive (legacy primary-only routes untouched), the shared-admission invariant is verified load-bearing, and CI is green. LGTM from a local-verification standpoint.
🇨🇳 中文版本✅ 本地验证（维护者）— build、typecheck、lint 与专项测试全部通过我在独立 worktree 中从源码构建了本 PR 并运行真实工具链，在合并前对作者的测试计划进行交叉验证。 环境
质量门禁
专项测试 — 本 PR 改动的 15 个测试文件
（截图见上方英文部分） 这些专项测试直接覆盖了 Reviewer 测试计划中的行为：
受控实验 — 该不变量是否是关键约束？为确认这些测试并非“空测”，我只破坏了 coordinator 的核心不变量——跨 runtime 共享的进程级 8 上限——把 结论build、typecheck、lint 以及全部 15 个受影响测试文件本地均通过，与 PR 声明一致。变更为纯增量（legacy primary-only 路由未动），共享准入不变量被证明是关键约束，且 CI 为绿。从本地验证角度 LGTM。
Local verification on an isolated worktree; Voice upstreams mocked (same as CI). Not an official approval. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
| return; | ||
| } | ||
| wss!.handleUpgrade(req, socket, head, (ws: WebSocket) => { | ||
| if (disposed) { |
There was a problem hiding this comment.
[Suggestion] The disposed check inside the Voice WS upgrade callback correctly closes the socket with code 1012 on server shutdown, but no test exercises this path. The existing test "returns 503 server_disposed after dispose()" only covers REST endpoints, not the Voice WS upgrade flow. Consider adding a test that calls handle.close() then attempts a Voice WS upgrade, asserting the socket closes with code 1012.
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
All 16 previously posted Critical findings have been verified against the current commit and are fixed by this diff (disposeRuntime sets completed, parallel shutdown via Promise.all, WebShell voiceSessions counter, coordinator/removal pairing validation, isPortableAbsolutePath guard, env mode handling, abort signal propagation, REST routing for qualified SDK methods, partial-persist broadcast).
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
@qwen-code /triage |
|
⏸️ Deferring to maintainer — this is a clean Phase 4b implementation (409 tests pass, typecheck/lint clean, no code concerns), but Stage 0 flagged 1,443 production logic lines touching core services ( 中文说明⏸️ 转交维护者 — 这是一个干净的 Phase 4b 实现（409 项测试通过，类型检查/lint 干净，无代码顾虑），但 Stage 0 标记了 1,443 行生产逻辑代码触及核心 services（ |
|
@qwen-code /triage |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no new blockers. 11 of 14 previously reported Criticals are fixed by this diff; 3 still stand (capability scoping in serve-features.ts, active-only resolver for GET/POST settings during drain). Suggestions are inline.
— qwen3.7-max via Qwen Code /review
| const result = deps.acquireVoiceLease?.(); | ||
| if (!result) return true; | ||
| if (result.kind === 'rejected') { | ||
| if (result.reason === 'draining') { |
There was a problem hiding this comment.
[Suggestion] The draining and capacity rejection branches in installAdmissionLease have no test coverage. A mock acquireVoiceLease returning { kind: 'rejected', reason: 'draining' } or { kind: 'rejected', reason: 'capacity' } would verify the 503 response shape, Retry-After header, and that next() is never called.
Concrete cost: a refactor of the rejection response format (e.g. renaming voice_capacity_exceeded) would silently break the client contract without any test catching it.
— qwen3.7-max via Qwen Code /review
| @@ -87,11 +96,13 @@ async function defaultOpenStream( | |||
| voiceModel: ctx.voiceModel, | |||
| env: ctx.env, | |||
| }); | |||
There was a problem hiding this comment.
[Suggestion] No test verifies that operationController.signal actually cancels an in-flight openStream when the lease is aborted. The existing test "aborts an in-flight upstream open" verifies the lease release path, but does not capture the signal passed to openStream and assert signal.aborted after coordinator disposal.
Failure scenario: a future refactor drops the abortSignal parameter from openVoiceStreamWithRetry, and stuck upstream connections survive workspace removal by up to the DNS/HTTP timeout.
— qwen3.7-max via Qwen Code /review
| return { kind: 'admitted', lease }; | ||
| } | ||
|
|
||
| beginWorkspaceDrain(runtime: WorkspaceRuntime): void { |
There was a problem hiding this comment.
[Suggestion] beginWorkspaceDrain and completeWorkspaceDrain call this.stateFor(runtime) without checking the disposed WeakSet first. If called after disposeRuntime (which adds to disposed and may have already removed the entry via deleteIfIdle), stateFor creates a new phantom entry with completed: false that deleteIfIdle can never reclaim — a small but permanent Map leak per occurrence.
| beginWorkspaceDrain(runtime: WorkspaceRuntime): void { | |
| beginWorkspaceDrain(runtime: WorkspaceRuntime): void { | |
| if (this.disposed.has(runtime)) return; | |
| this.stateFor(runtime).draining = true; | |
| } |
— qwen3.7-max via Qwen Code /review
| publishWorkspaceEvent: (event) => { | ||
| if ( | ||
| event.type === 'settings_changed' || | ||
| event.type === 'settings_reloaded' |
There was a problem hiding this comment.
[Suggestion] The publishWorkspaceEvent handler invalidates the serve features cache for both settings_changed and settings_reloaded event types, but the run-qwen-serve test only triggers settings_changed. The settings_reloaded branch is untested.
Failure scenario: if the settings_reloaded string were misspelled or the event shape changed, cache invalidation for that event type would silently break.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
| export async function assertVoiceBaseUrlNetworkAllowed( | ||
| voiceConfig: VoiceTranscriptionConfig, | ||
| lookupHost?: VoiceHostLookup, | ||
| abortSignal?: AbortSignal, |
There was a problem hiding this comment.
[Suggestion] The new abortSignal parameter is forwarded by the serve-side caller (voice-ws.ts) but the TUI caller at InputPrompt.tsx:488 still calls assertVoiceBaseUrlNetworkAllowed(streamConfig) with one argument. If the user cancels voice input during DNS resolution, the lookup will not be interrupted.
Consider forwarding the abort signal from InputPrompt.tsx:
await assertVoiceBaseUrlNetworkAllowed(streamConfig, undefined, abortSignal);— qwen3.7-max via Qwen Code /review
| if (state === 'closed') return; | ||
| cleanup(); | ||
| try { | ||
| ws.close(1012, voiceLeaseCloseReason(lease.signal)); |
There was a problem hiding this comment.
[Suggestion] WebSocket close code 1012 (Service Restart) is used for both workspace removal and daemon shutdown. Per RFC 6455, 1012 signals the client should reconnect — correct for workspace removal (the workspace may be re-added), but misleading for daemon shutdown (permanent). A client that auto-reconnects on 1012 will repeatedly fail against a stopped daemon.
Consider using 1001 (Going Away) for daemon shutdown:
const closeCode = signal.reason instanceof VoiceLeaseAbortError &&
signal.reason.kind === 'daemon_shutdown'
? 1001
: 1012;
ws.close(closeCode, voiceLeaseCloseReason(lease.signal));— qwen3.7-max via Qwen Code /review
|
@qwen-code /resolve |
…ion_management_v2 Both features are independent additions to the serve capability registry: - workspace_qualified_voice (PR branch): workspace-qualified Voice REST/WS routes - extension_management_v2 (main): global extension catalog and mutation surface Resolved by including both capabilities in the registry, docs feature list, and test expectations. Updated the workspace_qualified_rest_core comment to reference both adjacent capabilities.
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge conflict resolution summary — PR #6839Base branch:
|
5 participants
What this PR does
This PR completes Phase 4b workspace-qualified Voice for multi-workspace
qwen servedaemons. Trusted registered runtimes can read and update Voice settings, submit batch transcription, and open streaming transcription through workspace-qualified REST and WebSocket routes selected by workspace id or encoded absolute cwd. Each request uses the selected runtime's cwd, effective environment, bridge, and workspace-scoped settings persistence; legacy Voice routes remain primary-only.The daemon now shares one process-level admission coordinator across legacy and workspace-qualified REST and WebSocket Voice operations, preserving the existing total limit of eight. Workspace drain rejects new Voice work, non-force removal reports active Voice sessions as busy, and force removal or daemon shutdown aborts only the selected runtime's Voice leases while waiting up to five seconds for real operations to settle. The TypeScript workspace client exposes the qualified settings and batch transcription methods, and the protocol advertises the additive
workspace_qualified_voicecapability.Why it's needed
Multi-workspace daemons already isolate sessions, core REST, ACP, channel workers, and runtime lifecycle, but Voice remained bound to primary workspace settings and environment. A secondary workspace therefore could not safely select its own Voice model or credentials, and its active transcription work was invisible to workspace removal. This change closes that Phase 4b gap without changing ACP methods, Voice payload schemas, storage formats, or existing primary-workspace URLs.
Reviewer Test Plan
How to verify
Start a daemon with two trusted workspaces whose Voice models use distinct runtime environment values. Confirm that the qualified settings and transcription routes resolve both workspace id and encoded cwd, persist settings only in the selected workspace scope, send transcription through the selected runtime environment, and never fall back to primary for an unknown or untrusted selector. Confirm that legacy routes still use primary.
Open a mix of legacy and qualified batch and streaming operations. The ninth concurrent operation should return
503 voice_capacity_exceededwithRetry-After: 5for REST or an error frame followed by WebSocket close code1013; completing, failing, disconnecting, or timing out an operation should release exactly one slot.While a secondary Voice operation is active, non-force workspace removal should return
409 workspace_busywithactivity.voiceSessions. Force removal should close only that workspace's stream with code1012, preserve other workspace streams, and allow a later same-path runtime generation to acquire fresh Voice capacity.Locally verified on the final rebased commit with the focused Voice, workspace lifecycle, ACP upgrade, telemetry, realtime retry/session, SDK client, full serve server, and run-qwen-serve test files. The focused CLI matrix passed 267 tests, the SDK matrix passed 352 tests,
server.test.tspassed 693 tests on full rerun, and root build, typecheck, and lint passed.Evidence (Before & After)
N/A — daemon protocol, lifecycle, SDK, documentation, and tests only; no TUI or Web Shell UI changes.
Tested on
Environment (optional)
macOS, Node.js v22.22.3, npm 10.9.8, native local test runtime with mock HTTP/WebSocket Voice providers.
Risk & Scope
/workspace/voice,/workspace/voice/transcribe, and/voice/streamremain primary-only with their existing contracts. Clients should gate qualified settings, batch, and streaming behavior onworkspace_qualified_voiceintersected with the corresponding existing Voice capability.Linked Issues
Part of #6378.
Built on the runtime drain/removal lifecycle merged in #6745.
中文说明
本 PR 做了什么
本 PR 完成多 workspace
qwen servedaemon 的 Phase 4b workspace-qualified Voice。已注册且受信任的 runtime 可以通过按 workspace id 或编码后的绝对 cwd 选择的 workspace-qualified REST 与 WebSocket 路由读取和更新 Voice 设置、提交批量转写以及建立流式转写。每个请求都使用所选 runtime 的 cwd、有效环境、bridge 和 workspace scope 设置持久化；legacy Voice 路由继续只绑定 primary workspace。daemon 现在通过一个进程级 admission coordinator 统一管理 legacy 与 workspace-qualified 的 REST 和 WebSocket Voice 操作，并保持现有总上限 8。workspace drain 会拒绝新的 Voice 工作，非 force removal 会把活动 Voice session 报告为 busy，force removal 或 daemon shutdown 只中止所选 runtime 的 Voice lease，并最多等待 5 秒让真实操作收敛。TypeScript workspace client 暴露 qualified 设置与批量转写方法，协议新增可叠加的
workspace_qualified_voicecapability。为什么需要它
多 workspace daemon 已经隔离 sessions、core REST、ACP、channel workers 与 runtime 生命周期，但 Voice 仍绑定 primary workspace 的设置和环境。因此 secondary workspace 无法安全选择自己的 Voice 模型或凭证，活动转写也不会被 workspace removal 感知。本变更补齐 Phase 4b 缺口，同时不改变 ACP method、Voice payload schema、存储格式或既有 primary-workspace URL。
Reviewer 测试计划
如何验证
使用两个受信任 workspace 启动 daemon，并为它们配置使用不同 runtime 环境值的 Voice 模型。确认 qualified 设置与转写路由都能通过 workspace id 和编码 cwd 解析目标，只向所选 workspace scope 持久化设置，使用所选 runtime 环境发起转写，且 unknown 或 untrusted selector 绝不 fallback 到 primary。确认 legacy 路由仍使用 primary。
同时打开 legacy 与 qualified 的批量和流式操作。第 9 个并发操作在 REST 上应返回带
Retry-After: 5的503 voice_capacity_exceeded，在 WebSocket 上应先发送 error frame 再以1013关闭；操作完成、失败、断连或超时后都应准确释放一个槽位。当 secondary Voice 操作活动时，非 force workspace removal 应返回带
activity.voiceSessions的409 workspace_busy。force removal 应只以1012关闭该 workspace 的 stream，保留其他 workspace 的 stream，并允许随后相同路径的新 runtime generation 获取全新的 Voice 容量。已在最终 rebase 后的提交上本地验证 Voice、workspace 生命周期、ACP upgrade、telemetry、realtime retry/session、SDK client、完整 serve server 和 run-qwen-serve 相关测试文件。CLI 专项矩阵 267 项通过，SDK 矩阵 352 项通过，
server.test.ts完整复跑 693 项通过，根目录 build、typecheck 和 lint 均通过。证据（变更前与变更后）
N/A — 仅涉及 daemon 协议、生命周期、SDK、文档和测试；没有 TUI 或 Web Shell UI 变更。
测试平台
环境（可选）
macOS、Node.js v22.22.3、npm 10.9.8，使用 mock HTTP/WebSocket Voice provider 的原生本地测试 runtime。
风险与范围
/workspace/voice、/workspace/voice/transcribe和/voice/stream继续只绑定 primary，并保持既有契约。客户端应使用workspace_qualified_voice与对应现有 Voice capability 的交集来启用 qualified 设置、批量和流式能力。关联 Issue
属于 #6378 的一部分。
基于 #6745 已合入的 runtime drain/removal 生命周期。