Skip to content

feat(serve): Add daemon workspace voice and control APIs#5765

Merged
wenshao merged 34 commits into
QwenLM:mainfrom
doudouOUC:codex/daemon-workspace-voice
Jun 25, 2026
Merged

feat(serve): Add daemon workspace voice and control APIs#5765
wenshao merged 34 commits into
QwenLM:mainfrom
doudouOUC:codex/daemon-workspace-voice

Conversation

@doudouOUC

Copy link
Copy Markdown
Collaborator

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 /lsp without 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 serve with a token and a configured voice model, then confirm GET /workspace/voice reports voice state and selectable voice models without leaking secrets, POST /workspace/voice validates and persists incremental settings, and POST /workspace/voice/transcribe accepts binary audio/* or application/octet-stream input 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, and GET /session/:id/lsp work through the REST surface and the matching ACP/SDK helpers. For LSP, start serve with --experimental-lsp and 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, and npm run typecheck.

Evidence (Before & After)

N/A. This is an API and SDK change with no TUI screenshot evidence.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

macOS, Node.js v22.22.3.

Risk & Scope

  • Main risk or tradeoff: This broadens the daemon workspace API surface and adds SDK runtime code, including a documented browser daemon bundle budget increase from 122KB to 126KB for the new helper APIs.
  • Not validated / out of scope: Realtime voice WebSocket transcription and daemon-local microphone capture remain out of scope for v1, and Windows/Linux were not manually validated locally.
  • Breaking changes / migration notes: No breaking changes intended. Existing CLI voice and model-selection behavior should continue to work through the shared helper logic.

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/permissionsGET /workspace/trustPOST /workspace/trust/requestGET /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.tscd packages/sdk-typescript && npx vitest run test/unit/DaemonClientVoice.test.ts test/unit/acpRouteTable.test.ts test/unit/daemonEvents.test.tsnpm run build,以及 npm run typecheck

Evidence (Before & After)

N/A。这是 API 和 SDK 改动,没有 TUI 截图证据。

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

macOS,Node.js v22.22.3。

Risk & Scope

  • Main risk or tradeoff: 该改动扩展了 daemon workspace API 表面,并增加了 SDK runtime 代码,包括将 browser daemon bundle 预算从 122KB 提高到 126KB 以覆盖新的 helper API。
  • Not validated / out of scope: realtime voice WebSocket transcription 和 daemon 本机麦克风录音不在 v1 范围内,Windows/Linux 未在本地手动验证。
  • Breaking changes / migration notes: 不预期有 breaking changes。现有 CLI voice 和模型选择行为应继续通过共享 helper 逻辑工作。

Linked Issues

References #4514

@doudouOUC
doudouOUC marked this pull request as ready for review June 23, 2026 09:52
Copilot AI review requested due to automatic review settings June 23, 2026 09:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@qwen-code-ci-bot

qwen-code-ci-bot commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

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 setup-github feature isn't mentioned in the PR title — it's an additional daemon API that's scope-crept into the diff. There are also drive-by formatting changes in unrelated files (doc table width normalization in auth.md/model-providers.md/settings.md, line-wrapping cleanup in workflow-saved.ts/workflow.ts/MessageTimestamp.tsx) that don't belong here.

That said, the core voice API work — the helper extraction (services/voice-*), the route layer, the ACP dispatch, the SDK client additions — is genuinely needed infrastructure, and the code quality is high throughout. I'll review as-is but flag for maintainer awareness.

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 提交和回滚。setup-github 功能在 PR 标题中未提及,是一个额外的 daemon API 被夹带进了 diff。还有无关的格式化改动(auth.md/model-providers.md/settings.md 的文档表格宽度调整,workflow-saved.ts/workflow.ts/MessageTimestamp.tsx 的换行清理)不属于本 PR 范围。

不过,核心 voice API 工作——helper 抽取(services/voice-*)、route 层、ACP dispatch、SDK 客户端添加——是真正需要的基础设施,代码质量始终在线。我会整体审查,但标记给 maintainer 注意。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Code Review

No critical blockers. The implementation is solid across all five API domains:

  • Voice service: clean error handling, secret redaction via sanitizeTranscriptionErrorMessage (strips Bearer tokens and sk- keys), network safety (assertVoiceBaseUrlNetworkAllowed blocks private-network IPs, HTTPS enforced for non-localhost) ✓
  • Permissions: properly refactored from inline logic to workspace service delegation, scope validation accepts both user and workspace now ✓
  • Trust: clean validation chain with desiredState enum check, reason length limit, folder-trust-disabled guard ✓
  • Voice transcription route: content-type gating, empty body rejection, model existence checks, realtime-only model rejection, body size limit ✓
  • Voice helper extraction (services/voice-* → re-exported from ui/voice/) properly decouples daemon from TUI ✓
  • ACP dispatch: new methods registered in read-method set, thorough param validation with typed errors ✓

One remaining nit: copyright headers are still slightly inconsistent — voice-service.ts and voice-keyterms.ts say "Copyright 2025 Qwen", while workspace-trust.ts and workspace-voice.test.ts say "Copyright 2025 Qwen Team". Minor, but worth standardizing.

Scope-creep items confirmed: doc table width normalization in auth.md/model-providers.md/settings.md, line-wrapping cleanup in workflow-saved.ts/workflow.ts/MessageTimestamp.tsx, and the setup-github feature (not mentioned in PR title).

Unit Tests

All passing:

CLI workspace-voice.test.ts:  28 tests passed
CLI workspace-permissions.test.ts: passed (included in 92 total)
CLI facade.test.ts: passed (included in 92 total)
CLI transport.test.ts: 164 tests passed (covers ACP dispatch for trust, permissions, voice, setup-github)
SDK DaemonClientVoice.test.ts: 5 tests passed
SDK acpRouteTable.test.ts: 75 tests passed
SDK daemonEvents.test.ts: 91 tests passed
Build: 0 errors (15 pre-existing vscode warnings)
Typecheck: clean across all packages

Total: 427 unit tests passed, 0 failures.

Real-Scenario Testing (tmux)

Started npm run dev -- serve --token test-token-rerun --port 18888 from the PR worktree and drove all new endpoints with curl:

$ curl -s -H "Authorization: Bearer test-token-rerun" http://127.0.0.1:18888/workspace/voice
{
    "v": 1,
    "workspaceCwd": ".../.qwen/worktrees/triage",
    "enabled": false,
    "mode": "hold",
    "language": "",
    "voiceModel": null,
    "availableVoiceModels": []
}

$ curl -s ... http://127.0.0.1:18888/workspace/permissions
{
    "v": 1,
    "user": { "path": "/home/runner/.qwen/settings.json", "rules": { "allow": [], "ask": [], "deny": [] } },
    "workspace": { "path": ".../.qwen/settings.json", "rules": { "allow": [], "ask": [], "deny": [] } },
    "merged": { "allow": [], "ask": [], "deny": [] },
    "isTrusted": true
}

$ curl -s ... http://127.0.0.1:18888/workspace/trust
{
    "v": 1,
    "workspaceCwd": ".../.qwen/worktrees/triage",
    "folderTrustEnabled": false,
    "effective": { "state": "trusted", "source": "disabled" },
    "explicitTrustLevel": null,
    "requiresDaemonRestartForChanges": true
}

Error paths all return structured JSON with correct status codes and error codes:

$ curl -s -X POST ... -d '{"enabled":"notbool"}' /workspace/voice
{"error":"`enabled` must be a boolean","code":"invalid_enabled"}

$ curl -s -X POST ... -d '{}' /workspace/voice
{"error":"At least one of `enabled`, `mode`, `language`, or `voiceModel` must be provided","code":"invalid_voice_update"}

$ curl -s -X POST ... -H "Content-Type: text/plain" /workspace/voice/transcribe
{"error":"Content-Type must be audio/* or application/octet-stream...","code":"unsupported_voice_content_type"}

$ curl -s -X POST ... -H "Content-Type: audio/wav" --data-binary '' /workspace/voice/transcribe
{"error":"Voice audio body must be non-empty binary data","code":"invalid_voice_audio"}

$ curl -s -X POST ... -d '{"desiredState":"invalid"}' /workspace/trust/request
{"error":"desiredState must be \"trusted\" or \"untrusted\"","code":"invalid_desired_state"}

$ curl -s -X POST ... -d '{"desiredState":"trusted"}' /workspace/trust/request
{"error":"Folder trust is disabled for this workspace","code":"folder_trust_disabled"}

$ curl -s -X POST ... -d '{"scope":"workspace","ruleType":"deny","rules":["Read(.env)"]}' /workspace/permissions
→ 200: {"v":1,...,"workspace":{"rules":{"deny":["Read(.env)"]}},"merged":{"deny":["Read(.env)"]}}

Daemon log confirms clean request handling with no crashes, no leaked secrets, no unexpected stderr:

route=GET /workspace/voice durationMs=4 status=200 request completed
route=GET /workspace/permissions durationMs=4 status=200 request completed
route=GET /workspace/trust durationMs=1 status=200 request completed
route=POST /workspace/voice durationMs=14 status=400 request completed
route=POST /workspace/voice/transcribe durationMs=2 status=415 request completed
route=POST /workspace/trust/request durationMs=3 status=409 request completed
route=POST /workspace/permissions durationMs=8 status=200 request completed
中文说明

代码审查

未发现关键阻断问题。五个 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 serve 并用 curl 测试所有新增端点。三个 GET 端点返回结构正确的 JSON。七个错误路径测试均返回正确的错误码和状态码。permissions POST 成功持久化规则。Daemon 日志确认无崩溃、无 secret 泄露、无异常 stderr。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Reflection

Stepping 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:

  1. Scope — 88 files, ~9,400 additions is well past the contributing guide's 2,000-line ceiling. Five API domains are bundled. Each could be its own PR. The setup-github feature isn't even mentioned in the PR title.
  2. Drive-by changes — doc table formatting and line-wrapping cleanup in unrelated files don't belong here.
  3. Copyright headers — minor inconsistency between "Qwen" and "Qwen Team" across new files.

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 qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread packages/cli/src/serve/routes/workspace-setup-github.ts Outdated
Comment thread packages/cli/src/serve/run-qwen-serve.ts Outdated
Comment thread packages/cli/src/serve/routes/workspace-voice.ts
Comment thread packages/cli/src/serve/routes/workspace-voice.ts
Comment thread packages/cli/src/serve/routes/workspace-voice.ts Outdated
Comment thread packages/sdk-typescript/src/daemon/DaemonClient.ts
Comment thread packages/cli/src/services/voice-service.ts Outdated

@chiga0 chiga0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 presentrun-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() strips url.username/url.password. Solid defense-in-depth.
  • Secret redaction: Both sanitizeResponseDetails (transcriber) and sanitizeTranscriptionErrorMessage (routes) redact Bearer tokens and sk-* patterns. API key redaction in response details uses escapeRegExp for safe regex construction. Error messages truncated to 200 chars.
  • Audio input validation: express.raw({ limit: '10mb' }) caps body size. Content-type restricted to audio/* and application/octet-stream. Empty body check (byteLength === 0). Good DoS protection.
  • isKeytermEcho detection: 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 (persistSettingAvailable for voice/permissions). workspace_voice_transcription has modes: ['batch'] correctly scoping to batch-only.
  • ACP dispatch validation: All new vendor methods validate params before processing, return RPC.INVALID_PARAMS for 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

Comment thread packages/sdk-typescript/src/daemon/DaemonClient.ts
Comment thread packages/cli/src/serve/server.ts Outdated
Comment thread packages/sdk-typescript/src/daemon/DaemonClient.ts
Comment thread packages/cli/src/services/setup-github.ts Outdated

@ytahdn ytahdn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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

Comment thread packages/cli/src/services/voice-transcriber.ts Outdated
Comment thread packages/cli/src/serve/workspace-service/index.ts Outdated
Comment thread packages/cli/src/services/voice-service.ts
Comment thread packages/cli/src/services/setup-github.ts
Comment thread packages/cli/src/serve/routes/workspace-voice.ts Outdated
Comment thread packages/cli/src/serve/routes/workspace-voice.ts Outdated
Comment thread packages/cli/src/services/setup-github.ts
Comment thread packages/cli/src/serve/workspace-service/index.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread packages/cli/src/serve/server.ts Outdated
Comment thread packages/cli/src/serve/routes/workspace-voice.ts
Comment thread packages/cli/src/serve/routes/workspace-voice.ts Outdated
Comment thread packages/cli/src/serve/workspace-service/index.ts

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. Trust state split-brainloadSettings (settings.ts:1452) treats unknown trust state as trusted (trustState === 'untrusted' ? false : true), while runQwenServe (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.

  2. requestWorkspaceTrustChange is a no-op — publishes a trust_change_requested event and returns { accepted: true }, but no handler ever mutates trust state. Clients are told the request was "accepted" when nothing actually changes.

  3. getVoiceSettingsScope bootstrap problem — workspace-scoped voice settings are unreachable from a clean state because the function requires workspace.settings.general.voice.enabled to already be a boolean to select Workspace scope. First enable always writes to User scope.

  4. setWorkspacePermissionRules fallback 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_MAP defined 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.

Comment thread packages/cli/src/config/settings.ts Outdated
Comment thread packages/cli/src/serve/workspace-service/index.ts Outdated
Comment thread packages/cli/src/services/voice-settings.ts
Comment thread packages/cli/src/serve/workspace-service/index.ts Outdated
Comment thread packages/cli/src/serve/server.ts Outdated
Comment thread packages/cli/src/serve/routes/workspace-setup-github.ts Outdated
Comment thread packages/cli/src/serve/routes/workspace-setup-github.ts
Comment thread packages/cli/src/serve/routes/workspace-setup-github.ts Outdated
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review did not complete successfully: Qwen review timed out after 85 minutes. See workflow logs.

Comment thread packages/sdk-typescript/src/daemon/types.ts Outdated
Comment thread packages/cli/src/serve/routes/workspace-setup-github.ts Outdated
Comment thread packages/cli/src/services/setup-github.ts Outdated
Comment thread packages/cli/src/services/setup-github.ts

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Downgraded from Approve to Comment: CI still running.

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

Comment thread packages/cli/src/config/settings.ts
Comment thread packages/cli/src/serve/routes/workspace-voice.ts
Comment thread packages/cli/src/serve/routes/workspace-permissions.ts Outdated
Comment thread packages/cli/src/serve/acp-http/dispatch.ts
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review did not complete successfully: Qwen review timed out after 85 minutes. See workflow logs.

@doudouOUC doudouOUC self-assigned this Jun 24, 2026
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review did not complete successfully: Qwen review timed out after 85 minutes. See workflow logs.

Comment thread packages/cli/src/serve/acp-http/dispatch.ts
Comment thread packages/cli/src/services/voice-transcriber.ts
Comment thread packages/cli/src/services/voice-settings.ts Outdated
Comment thread packages/cli/src/serve/workspace-service/index.ts Outdated
Comment thread packages/cli/src/serve/acp-http/dispatch.ts
Comment thread packages/cli/src/services/voice-service.ts

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 qqqys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Thanks @qqqys for rechecking. CI is green on the latest head (87ce3c7), and all review threads appear resolved. @wenshao, could you please re-review when you have a chance? Happy to address anything still outstanding.

中文说明

感谢 @qqqys 重新检查。最新 head(87ce3c7)上的 CI 是绿色的,所有 review threads 看起来也都已 resolved。@wenshao 方便的话麻烦再复审一下;如果还有未解决的问题,我会继续处理。

@wenshao
wenshao enabled auto-merge June 24, 2026 19:34
@wenshao
wenshao added this pull request to the merge queue Jun 24, 2026
Merged via the queue into QwenLM:main with commit 9254852 Jun 25, 2026
11 of 12 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review did not complete successfully: Qwen review timed out after 85 minutes. See workflow logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants