feat(serve): Add daemon workspace voice and control APIs#5765
Conversation
Co-authored-by: Qwen-Coder <[email protected]>
Co-authored-by: Qwen-Coder <[email protected]>
|
Thanks for the updated PR, @doudouOUC! Template looks good ✓ On direction: Voice support is a first-class feature since v0.19.0, and the daemon/serve API surface has been steadily expanding (LSP, workspace settings, etc.). Exposing voice config, transcription, permissions, trust, and LSP status via REST/ACP/SDK for remote clients is clearly aligned with the project's trajectory. On approach: The scope remains the main concern at ~9,400 additions across 88 files. The PR bundles five logically separable pieces: (1) voice config + transcription APIs, (2) permissions APIs, (3) trust APIs, (4) LSP status APIs, and (5) setup-github APIs. Each could land as its own reviewable, revertable PR. The That said, the core voice API work — the helper extraction ( Moving on to code review. 🔍 中文说明感谢更新,@doudouOUC! 模板完整 ✓ 方向:Voice 自 v0.19.0 起是一等公民功能,daemon/serve API 表面持续扩展(LSP、workspace settings 等)。通过 REST/ACP/SDK 暴露 voice 配置、转写、permissions、trust 和 LSP 状态给远程客户端,与项目方向明确一致。 方案:范围仍然是主要关切——88 个文件约 9,400 行新增。PR 捆绑了五个可独立审查的逻辑块:(1) voice 配置 + 转写 API,(2) permissions API,(3) trust API,(4) LSP 状态 API,(5) setup-github API。每个都可以作为独立 PR 提交和回滚。 不过,核心 voice API 工作——helper 抽取( 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewNo critical blockers. The implementation is solid across all five API domains:
One remaining nit: copyright headers are still slightly inconsistent — Scope-creep items confirmed: doc table width normalization in Unit TestsAll passing: Total: 427 unit tests passed, 0 failures. Real-Scenario Testing (tmux)Started Error paths all return structured JSON with correct status codes and error codes: Daemon log confirms clean request handling with no crashes, no leaked secrets, no unexpected stderr: 中文说明代码审查未发现关键阻断问题。五个 API 域实现质量均良好:voice service 错误处理清晰、secret 脱敏到位、网络安全检查完整;permissions 正确重构为 workspace service 委托;trust 验证链干净;voice 转写路由的 content-type/空 body/模型/大小限制全部生效;voice helper 抽取正确解耦 daemon 与 TUI。 小问题:版权头仍略不一致("Copyright 2025 Qwen" vs "Copyright 2025 Qwen Team")。范围夹带确认:文档表格宽度调整、workflow-saved/workflow/MessageTimestamp 换行清理、setup-github 功能(PR 标题未提及)。 单元测试全部通过:CLI 92 个 + transport 164 个 + SDK 171 个 = 427 个测试全部通过。Build 零错误,typecheck 全包通过。 真实场景测试(tmux)从 PR worktree 启动 — Qwen Code · qwen3.7-max |
ReflectionStepping back: this PR does exactly what it says — voice, permissions, trust, LSP status, and setup-github all land as REST + ACP + SDK surfaces with proper security (secret redaction, network safety, HTTPS enforcement), thorough input validation, and comprehensive test coverage (427 unit tests, all passing). Real-scenario testing confirmed every endpoint returns correctly structured JSON and rejects invalid inputs with proper error codes and status codes. No crashes, no leaked secrets, no unexpected output. My independent proposal before reading the diff would have been essentially the same: one route file per domain, shared voice helpers extracted from TUI via re-exports, ACP dispatch extensions, SDK client additions, capability advertising. The PR matches that baseline. The honest concerns:
But the code itself is clean: no speculative abstractions, no unnecessary indirections, every changed file serves a stated goal. The voice helper extraction is genuine infrastructure needed to decouple daemon from TUI. The error handling is thorough and consistent. The security posture is good. 427 tests pass. Real-scenario testing shows it works end-to-end. I'd strongly encourage splitting future work like this, and ask the author to remove the drive-by formatting changes and the setup-github feature from this PR (or at minimum, mention it in the title). But the core voice + permissions + trust + LSP API work earns a pass. Verdict: Approve with the above notes for the author's awareness. 中文说明总结这个 PR 兑现了承诺——voice、permissions、trust、LSP 状态和 setup-github 全部以 REST + ACP + SDK 形式落地,安全到位,输入验证完善,测试覆盖全面(427 个单元测试全部通过)。真实场景测试确认每个端点行为正确。 我在看 diff 之前的独立方案本质上是一样的:每个领域一个 route 文件,共享 voice helper,ACP 扩展,SDK 客户端方法。PR 与这个基线吻合。 真实顾虑:(1) 范围过大——88 个文件 9,400 行新增远超 2,000 行上限,五个 API 域捆绑在一起,setup-github 甚至不在标题中;(2) 夹带无关格式化改动;(3) 版权头略不一致。 但代码本身干净:无投机性抽象,每个改动都服务于目标。Voice helper 抽取是真正需要的基础设施。427 个测试全部通过,真实场景端到端验证通过。 强烈建议后续拆分,并请作者从本 PR 中移除无关格式化改动和 setup-github 功能(或至少在标题中提及)。核心 voice + permissions + trust + LSP API 工作值得通过。 结论:通过,附上述建议供作者参考。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Co-authored-by: Qwen-Coder <[email protected]>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Additional pattern findings (cross-file):
Duplicated validation constants across 3 files: MAX_TRUST_REASON_LENGTH = 1024 is independently defined in dispatch.ts:173, workspace-trust.ts:11, and MAX_LANGUAGE_LENGTH = 1024 in workspace-voice.ts:35 vs MAX_VOICE_LANGUAGE_LENGTH in dispatch.ts:174. If a limit changes in one file but not the others, the ACP and REST transports silently enforce different limits. Extract shared constants to a single module (e.g., serve/validation-limits.ts).
Conflicting readVoiceLanguage definitions: voice-settings.ts:33 (exported, returns string, default '') and voice-transcriber.ts:74 (private, returns string | undefined, default undefined) both read the same settings path with different falsy sentinels. Delete the private copy and import from voice-settings.ts.
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <[email protected]>
chiga0
left a comment
There was a problem hiding this comment.
Overview
Final Verdict: Needs Changes — Large, well-structured PR adding voice, permissions, trust, and LSP workspace APIs. Security-sensitive code (SSRF protection, secret redaction, input validation) is handled correctly. One confirmed breaking change concern remains; the TOCTOU finding has been addressed at HEAD.
Cross-Validation
| # | Finding | Reviewer | My Assessment |
|---|---|---|---|
| C1 | TOCTOU race in ensureDirectoryWithoutSymlink (lstat + mkdir gap) |
qwen-code-ci-bot | Fixed at HEAD — post-mkdir lstat + isSymbolicLink() check added (lines 209-210). The race window is closed. |
| C2 | Breaking change: trustedWorkspace default from true to trust-status check |
qwen-code-ci-bot | Still present — run-qwen-serve.ts:868-873: when bootSettings exists and folder trust is enabled but the workspace is NOT trusted, trustedWorkspace becomes false. Existing deployments relying on the implicit true default will have write operations fail silently. Should be documented as a breaking change or gated behind a migration flag. |
| S1 | Inconsistent client ID validation in voice transcribe | qwen-code-ci-bot | Valid suggestion — defense-in-depth improvement |
| S2 | Duplicated validateResultingVoiceState |
qwen-code-ci-bot | Valid — DRY violation |
| S3 | Non-atomic voice settings persistence | qwen-code-ci-bot | Valid — sequential writes can partially fail |
| S4 | Read-modify-write race in permission helpers | qwen-code-ci-bot | Valid but documented in JSDoc. Acceptable for v1. |
| S5 | listAvailableVoiceModels throws inside .map() |
qwen-code-ci-bot | Not present at HEAD — code uses .filter(isSelectableVoiceModel).map(...) + defensive .filter(transport !== 'unsupported'). No throw path in .map(). |
| S6 | Duplicated validation constants | qwen-code-ci-bot | Valid — validation-limits.ts exists but some constants are still locally defined |
| S7 | Conflicting readVoiceLanguage definitions |
qwen-code-ci-bot | Valid — should consolidate |
Additional Audit Coverage
Areas I independently checked beyond existing findings:
- SSRF protection (
voice-transcriber.ts): Comprehensive —isPrivateNetworkIp()blocks RFC 1918, loopback, link-local, CGNAT, ULA.assertVoiceBaseUrlNetworkAllowed()does DNS resolution check. HTTPS enforced for non-localhost.normalizeBaseUrl()stripsurl.username/url.password. Solid defense-in-depth. - Secret redaction: Both
sanitizeResponseDetails(transcriber) andsanitizeTranscriptionErrorMessage(routes) redact Bearer tokens andsk-*patterns. API key redaction in response details usesescapeRegExpfor safe regex construction. Error messages truncated to 200 chars. - Audio input validation:
express.raw({ limit: '10mb' })caps body size. Content-type restricted toaudio/*andapplication/octet-stream. Empty body check (byteLength === 0). Good DoS protection. isKeytermEchodetection: Well-designed hallucination guard — checks token overlap against keyterm context with dual-ratio thresholds (transcript ratio ≥0.9, keyterm ratio ≥0.3, min 8 overlapping tokens, min 4 total tokens). Prevents keyterm bias from leaking into prompts.- Capability registration: All new capabilities correctly registered with appropriate conditional toggles (
persistSettingAvailablefor voice/permissions).workspace_voice_transcriptionhasmodes: ['batch']correctly scoping to batch-only. - ACP dispatch validation: All new vendor methods validate params before processing, return
RPC.INVALID_PARAMSfor bad input, and check ownership (requireOwned) for session-scoped operations.
Remaining Concern
C2 (breaking change) is the most impactful remaining issue. The trustedWorkspace default change is security-positive (restricting write access to trusted workspaces) but will silently break existing deployments that relied on the implicit true default without configuring folder trust. Recommend documenting this in release notes or adding a deprecation warning log.
This review was generated by QoderWork AI
ytahdn
left a comment
There was a problem hiding this comment.
[Critical] [typecheck] Pre-existing type error in server.test.ts:1141
getWorkspaceMcpStatus does not exist in type FakeBridge — did you mean getWorkspaceMcpToolsStatus? This is not introduced by this PR but was flagged by tsc --noEmit on the changed packages. Mentioning here since it affects the test mock fidelity.
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <[email protected]>
Co-authored-by: Qwen-Coder <[email protected]>
Co-authored-by: Qwen-Coder <[email protected]>
Co-authored-by: Qwen-Coder <[email protected]>
Co-authored-by: Qwen-Coder <[email protected]>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Code Review Summary
4 Critical / 8 Suggestions / 4 Needs Human Review
This PR adds a substantial daemon workspace API surface covering voice configuration, batch transcription, trust management, permission rules, and GitHub setup. The overall architecture is well-structured with clean dependency injection and thoughtful security measures (SSRF guard, credential redaction, input validation). However, I found several correctness issues in the trust model and persistence layer that should be addressed before merge.
Critical findings
-
Trust state split-brain —
loadSettings(settings.ts:1452) treats unknown trust state as trusted (trustState === 'untrusted' ? false : true), whilerunQwenServe(run-qwen-serve.ts:871) treats unknown as untrusted (effective.state === 'trusted'). This means env files are loaded for unknown workspaces but file writes are blocked — an inconsistent trust model. -
requestWorkspaceTrustChangeis a no-op — publishes atrust_change_requestedevent and returns{ accepted: true }, but no handler ever mutates trust state. Clients are told the request was "accepted" when nothing actually changes. -
getVoiceSettingsScopebootstrap problem — workspace-scoped voice settings are unreachable from a clean state because the function requiresworkspace.settings.general.voice.enabledto already be a boolean to select Workspace scope. First enable always writes to User scope. -
setWorkspacePermissionRulesfallback doesn't sync live state — when the ACP session is unavailable, the fallback writes to the settings file but never updates in-memory permission managers. Live permission checks use stale rules until restart.
Patterns
- Duplicated constants:
SCOPE_MAP/PERMISSION_SCOPE_MAPdefined independently in 3+ files - Duplicated redaction logic: two separate credential-redaction functions with subtly different patterns
- Setup-github security: proxy from untrusted workspace settings, error messages leaking filesystem paths, missing root-directory validation
Full details in inline comments below.
Co-authored-by: Qwen-Coder <[email protected]>
|
Qwen Code review did not complete successfully: Qwen review timed out after 85 minutes. See workflow logs. |
Co-authored-by: Qwen-Coder <[email protected]>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Additional finding: packages/cli/src/services/voice-service.ts — this new file exports 11 functions but has no dedicated voice-service.test.ts. Coverage is only indirect via route and facade tests. Consider adding unit tests for buildWorkspaceVoiceSettingsWrites, validateWorkspaceVoiceState, and hasConfiguredBatchVoiceTranscriptionModel.
— qwen3.7-max via Qwen Code /review
|
Qwen Code review did not complete successfully: Qwen review timed out after 85 minutes. See workflow logs. |
Co-authored-by: Qwen-Coder <[email protected]>
|
Qwen Code review did not complete successfully: Qwen review timed out after 85 minutes. See workflow logs. |
Co-authored-by: Qwen-Coder <[email protected]>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
R11 incremental review (qwen3.7-max)
Downgraded from Approve to Comment: CI still running.
Deterministic analysis: tsc reports 41 errors, all in unchanged portions of modified files (server.test.ts lines 2701/2798/4293/9239 are pre-existing Extension type mismatches; MessageTimestamp.tsx and AssistantMessage.tsx TS4111 errors are pre-existing CSS module index-signature access unchanged by the formatting-only diff in those files; settings.test.ts TS2352 errors at lines 192/260 are outside the PR's diff range). ESLint clean (0 findings).
Build: passes cleanly (exit 0, all packages).
Tests: all pass — CLI 104/104 (workspace-voice 29, workspace-permissions 10, facade 56, voice-service 9), SDK 171/171 (DaemonClientVoice 5, acpRouteTable 75, daemonEvents 91).
Reverse audit: no new Critical or Suggestion findings beyond the 92 existing inline comments and 14 review summaries from prior rounds (R1–R10). The latest three feedback-address commits (5c8fbb7, 41ea2b3, 87ce3c7) correctly fix: saveSettings throwOnWriteFailure opt-in behavior, voice trust-aware scope resolution, IPv4-compatible IPv6 SSRF protection, permission rules existing-rules passthrough, shared voice validation refactoring, sanitizeSetupGithubResult intentional field passthrough, streaming download byte-limit enforcement, and requestAbortSignal cleanup.
No new issues found. LGTM pending CI. ✅
— qwen3.7-max via Qwen Code /review
qqqys
left a comment
There was a problem hiding this comment.
Rechecked the latest head after the voice/daemon follow-up fixes. The prior critical redirect and trust-scope issues are addressed, and I did not find a new critical blocker.
|
Qwen Code review did not complete successfully: Qwen review timed out after 85 minutes. See workflow logs. |
What this PR does
This PR adds daemon workspace APIs for voice configuration and batch transcription, plus REST, ACP, and SDK surfaces for workspace trust requests, permission rule management, session LSP status, and related daemon workspace controls. Voice support is implemented as client-side audio capture with daemon-side configuration, model validation, safe ASR request handling, secret redaction, and binary batch transcription.
The implementation also extracts reusable voice settings, model selection, keyterm, and transcription helpers so the daemon does not depend on TUI-only modules, advertises the new capabilities, wires ACP and REST routes consistently, and keeps existing CLI voice behavior on the same validation path.
Why it's needed
Daemon clients need API-level replacements for CLI-only workspace interactions such as
/voice,/permissions,/trust, and/lspwithout depending on interactive slash UI. The new voice API gives browser, desktop, and other remote clients a safe v1 transcription path where the client owns microphone capture and the daemon owns model selection, credential handling, network checks, and ASR invocation.Reviewer Test Plan
How to verify
Start
qwen servewith a token and a configured voice model, then confirmGET /workspace/voicereports voice state and selectable voice models without leaking secrets,POST /workspace/voicevalidates and persists incremental settings, andPOST /workspace/voice/transcribeaccepts binaryaudio/*orapplication/octet-streaminput while rejecting unsupported content types, empty audio, missing models, realtime-only models, oversized bodies, and unsafe upstream errors.Confirm
GET/POST /workspace/permissions,GET /workspace/trust,POST /workspace/trust/request, andGET /session/:id/lspwork through the REST surface and the matching ACP/SDK helpers. For LSP, start serve with--experimental-lspand verify the spawned ACP child receives the flag and reports a sanitized session LSP snapshot.Validated locally with
cd packages/cli && npx vitest run src/serve/routes/workspace-voice.test.ts src/serve/routes/workspace-permissions.test.ts src/serve/workspace-service/__tests__/facade.test.ts,cd packages/sdk-typescript && npx vitest run test/unit/DaemonClientVoice.test.ts test/unit/acpRouteTable.test.ts test/unit/daemonEvents.test.ts,npm run build, andnpm run typecheck.Evidence (Before & After)
N/A. This is an API and SDK change with no TUI screenshot evidence.
Tested on
Environment (optional)
macOS, Node.js v22.22.3.
Risk & Scope
Linked Issues
References #4514.
中文说明
What this PR does
这个 PR 为 daemon 增加 workspace 级 voice 配置与 batch transcription API,同时补齐 workspace trust 请求、permission 规则管理、session LSP 状态以及相关 daemon workspace 控制能力的 REST、ACP 和 SDK 表面。voice 支持采用客户端录音、daemon 侧配置与转写代理的模式,由 daemon 负责模型校验、安全 ASR 请求、secret 脱敏以及二进制 batch transcription。
实现中还抽出了可复用的 voice settings、模型选择、keyterm 和 transcription helper,避免 daemon 依赖 TUI-only 模块;同时补充 capability 广告,统一 ACP 与 REST route,并让现有 CLI voice 行为继续走同一套校验路径。
Why it's needed
daemon 客户端需要 API 级能力来替代
/voice、/permissions、/trust、/lsp等 CLI-only workspace 交互,而不是依赖交互式 slash UI。新的 voice API 为浏览器、桌面和其他远程客户端提供安全的 v1 转写路径:客户端负责麦克风录音,daemon 负责模型选择、凭据处理、网络检查以及 ASR 调用。Reviewer Test Plan
How to verify
使用 token 和已配置的 voice model 启动
qwen serve,确认GET /workspace/voice返回 voice 状态和可选 voice model 且不泄露 secret,POST /workspace/voice能校验并增量持久化设置,POST /workspace/voice/transcribe接受二进制audio/*或application/octet-stream输入,并正确拒绝不支持的 content type、空音频、缺少模型、realtime-only 模型、超大 body 和不安全的上游错误。确认
GET/POST /workspace/permissions、GET /workspace/trust、POST /workspace/trust/request和GET /session/:id/lsp能通过 REST 表面以及对应 ACP/SDK helper 工作。对于 LSP,使用--experimental-lsp启动 serve,并确认 spawned ACP child 收到该 flag 且返回脱敏后的 session LSP snapshot。本地已验证:
cd packages/cli && npx vitest run src/serve/routes/workspace-voice.test.ts src/serve/routes/workspace-permissions.test.ts src/serve/workspace-service/__tests__/facade.test.ts,cd packages/sdk-typescript && npx vitest run test/unit/DaemonClientVoice.test.ts test/unit/acpRouteTable.test.ts test/unit/daemonEvents.test.ts,npm run build,以及npm run typecheck。Evidence (Before & After)
N/A。这是 API 和 SDK 改动,没有 TUI 截图证据。
Tested on
Environment (optional)
macOS,Node.js v22.22.3。
Risk & Scope
Linked Issues
References #4514。