feat(cli): forward ask_user_question answers from SDK can_use_tool - #6655
Conversation
SDK-hosted agents could receive ask_user_question calls through the can_use_tool callback and approve them, but the user's answers never reached the tool: the CLI called onConfirm(ProceedOnce) with no payload, so the tool read an empty answers map and the model never got the decisions. Route updatedInput.answers from the SDK's allow response into the tool confirmation payload so the collected answers reach the tool. Reuses the existing updatedInput channel — no new SDK API or types. Document the pattern in the TypeScript and Python SDK READMEs.
|
Thanks for the PR! Template looks good ✓ Problem: this is an observed SDK gap with a linked feature request (#6647, now closed by this PR). The SDK's Direction: aligned. The PR reuses the existing Size: 86 production logic lines (71 additions + 15 deletions in Approach: the scope is tight — one extracted helper method, two call sites refactored to use it, SDK docs for both languages. Every edit serves the stated goal. The Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是一个已观测到的 SDK 缺陷，有关联的 feature request（#6647，已被此 PR 关闭）。SDK 的 方向：对齐。PR 复用现有的 规模：86 行生产逻辑（ 方案：范围紧凑——提取一个 helper 方法，重构两个调用点，为两种语言补充 SDK 文档。每处改动都服务于既定目标。 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewThe change is clean. The key design decision is correct: The Reuse check: the existing Independent proposal comparison: I would have done exactly this — extract a shared helper, gate No critical blockers. No AGENTS.md violations. Unit TestsAll pass:
Real-Scenario TestingN/A — this is SDK/host wiring plus docs, not user-visible TUI behavior. There is no CLI invocation to drive in tmux. The unit tests are the correct verification mechanism here. 中文说明代码审查改动干净。 关键设计决策正确：仅当
复用检查： 复用了 独立方案对比： 我的方案与此完全一致——提取共享 helper、按工具名称守卫 无关键阻塞项。无 AGENTS.md 违规。 单元测试全部通过：
真实场景测试不适用——这是 SDK/宿主连线加文档，不是用户可见的 TUI 行为。没有可以在 tmux 中驱动的 CLI 调用。单元测试是此处的正确验证机制。 — Qwen Code · qwen3.7-max |
|
This is a clean, well-scoped fix for a real SDK gap. The The fix is minimal: one extracted method, two call sites unified, SDK docs for both TypeScript and Python. It reuses the existing My independent proposal matched the PR's approach exactly. No simpler path was missed. The change solves something SDK users actually need (the linked issue has clear demand), and I'd be happy maintaining this code in six months. LGTM — approving. ✅ 中文说明这是一个干净、范围良好的修复，解决了真实的 SDK 缺陷。 修复最小化：一个提取的方法、两个统一调用的调用点、TypeScript 和 Python 的 SDK 文档。复用现有的 我的独立方案与 PR 的方案完全一致。未发现更简路径。该变更解决了 SDK 用户实际需要的问题（关联 issue 有明确需求），六个月后维护此代码我不会有任何问题。 LGTM — 批准。✅ — Qwen Code · qwen3.7-max |
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. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
|
No new Suggestion-level findings this round — all prior suggestions have been addressed or superseded. — qwen3.7-max via Qwen Code /review |
Address review feedback on QwenLM#6655: - handleTeammateApproval now mirrors the leader path and promotes the user's answers from updatedInput into the confirmation payload, so ask_user_question calls approved through a teammate no longer drop the user's choices (wenshao). - Extract a shared buildAllowConfirmationPayload helper used by both the leader and teammate paths, and only promote `answers` for ask_user_question so a same-named field on any other tool's input can't leak into the payload. - Add tests for the teammate path and the defensive guards (array updatedInput, array/null/empty answers, foreign answers field).
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings — code looks good. Downgraded from Approve to Comment: CI failing: Test (ubuntu-latest, Node 22.x). Suggestion-level recommendations are in the Suggestion summary comment below.
— qwen3.7-max via Qwen Code /review
CodeMirror's async measure pass (scheduled via requestAnimationFrame) calls getClientRects()/getBoundingClientRect() on a text Range. jsdom implements these on Element but not on Range, so the call throws "textRange(...).getClientRects is not a function" from a rAF callback after the test completed. Vitest surfaces it as an unhandled error and fails the whole run with exit code 1 even though every assertion passed (seen intermittently in useComposerCore.dom.test.tsx). Polyfill both methods on Range.prototype in the shared test setup, mirroring the existing ResizeObserver/scrollIntoView stubs.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
|
@qwen-code /resolve |
Adopt origin/main's cleaner implementation style (globalWithDom.Range, ??= operator, shared helper functions) while preserving the PR branch's explanatory comment about why Range stubs are needed for CodeMirror's async measure pass in jsdom.
|
Qwen Code resolved the merge conflicts and pushed the branch update. Conflict Resolution Summary — PR #6655Conflicted file
What conflictedBoth the PR branch and origin/main added Range prototype stubs (
How it was resolvedKept origin/main's implementation style for consistency with the rest of the file:
Preserved the PR branch's explanatory comment, which documents the non-obvious why: jsdom implements these methods on Commit
|
Address review suggestions on QwenLM#6655: - buildAllowConfirmationPayload now gates answers-promotion on the ToolNames.ASK_USER_QUESTION constant instead of a bare string literal, so a future rename of the tool name is a compile-time break rather than a silent regression. - Add an it.each case for a non-object primitive updatedInput (string) to cover the `typeof updatedInput !== 'object'` guard branch. - Assert the leader path overrides toolCall.request.args with the host's sanitized updatedInput before confirming. - Add a teammate-path test for an allow response with no updatedInput, asserting respond is called with (ProceedOnce, undefined).
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
All 4 previous suggestions have been addressed: ToolNames.ASK_USER_QUESTION is now used, edge-case test coverage expanded (string updatedInput, args mutation assertion, teammate no-payload test), and the teammate path correctly routes through buildAllowConfirmationPayload.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
| : undefined; | ||
| return { | ||
| updatedInput: updatedInputObj, | ||
| ...(answers && typeof answers === 'object' && !Array.isArray(answers) |
There was a problem hiding this comment.
[Suggestion] answers values are not validated as strings at the trust boundary. The guard checks that answers is a non-array object, but individual values could be nested objects, numbers, or booleans. The as Record<string, string> cast is compile-time only. Downstream in askUserQuestion.ts, ${value} coerces gracefully via template literal, but the cast makes a false promise. The ACP bridge (packages/acp-bridge/src/bridge.ts) performs per-value typeof v === 'string' validation before forwarding answers — this path should match.
| ...(answers && typeof answers === 'object' && !Array.isArray(answers) | |
| ...(answers && typeof answers === 'object' && !Array.isArray(answers) | |
| ? { answers: Object.fromEntries( | |
| Object.entries(answers as Record<string, unknown>) | |
| .filter(([, v]) => typeof v === 'string'), | |
| ) as Record<string, string> } | |
| : {}), |
— qwen3.7-max via Qwen Code /review
| if (confirmationPayload) { | ||
| // Override the tool's args in-process with the host's | ||
| // sanitised input before confirming. | ||
| toolCall.request.args = confirmationPayload.updatedInput ?? {}; |
There was a problem hiding this comment.
[Suggestion] Two observations on this line:
-
The
?? {}fallback is unreachable —buildAllowConfirmationPayloadonly returns a defined payload whenupdatedInputis a valid non-null, non-array object, andupdatedInputis always set on the returned value. Theif (confirmationPayload)guard above ensures we only reach this line with a valid payload. -
answersends up inrequest.argsvia theupdatedInputassignment, which violates theask_user_questiontool schema (additionalProperties: false— onlyquestionsandmetadataare valid). The tool reads answers frompayload.answers, not from args, so this is dead data — but a future maintainer inspectingrequest.argswould see an unexpectedanswersfield. Consider strippinganswersfromupdatedInputbefore assigning:
| toolCall.request.args = confirmationPayload.updatedInput ?? {}; | |
| const { answers: _, ...argsOnly } = confirmationPayload.updatedInput; | |
| toolCall.request.args = argsOnly; |
— qwen3.7-max via Qwen Code /review
| ToolConfirmationOutcome.ProceedOnce, | ||
| undefined, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] The leader path's else branch (no updatedInput → bare onConfirm(ProceedOnce)) has no explicit test in this PR. The teammate path has one ("confirms a teammate approval with no payload when updatedInput is absent"), and the pre-existing timeout test exercises it implicitly, but a leader-specific assertion would pin the branch down and document the intent.
— qwen3.7-max via Qwen Code /review
✅ Local verification — PR #6655 (maintainer build)I checked this branch out, built it, and ran real tests locally. Verdict: verified — the fix works end-to-end and every gate is green. Full evidence below. Env: macOS · Node What the model actually receivesThe core of this PR is that the user's
All local gates
1. The PR's own unit tests11 new cases cover the behavior and its guards — the leader path, the teammate approval path, and the defensive rejections (array/primitive 2. Consumer tests still green3. End-to-end proof — does the answer actually reach the model?The unit tests verify each side against a mock of the other. To close that gap I wrote a throwaway harness that wires the real BEFORE (main) — answers dropped: AFTER (this PR) — answers delivered to the model:
4. Types & lintNotes for merge
Recommendation: LGTM — safe to merge. 🇨🇳 中文版本✅ 本地验证 — PR #6655（维护者构建）我拉取该分支、完成构建，并在本地跑了真实测试。结论：已验证——修复端到端生效，所有检查项全绿。 环境： macOS · Node 模型真正收到了什么本 PR 的核心是：用户对 1. PR 自带的单元测试新增 11 个用例覆盖了该行为及其防护：leader 路径、teammate 审批路径，以及各类防御性拒绝（ 2. 下游消费方测试仍然通过3. 端到端验证 —— 答案真的到达模型了吗？单元测试是各自对着对方的 mock 验证的。为了补上这一环，我写了一个临时 harness，把真实的 修复前（main）—— 答案丢失： 修复后（本 PR）—— 答案送达模型：
4. 类型与 Lint合并注意事项
建议：LGTM，可以合并。 |
|
@qwen-code /triage |
…wenLM#6655) * feat(cli): forward ask_user_question answers from SDK can_use_tool SDK-hosted agents could receive ask_user_question calls through the can_use_tool callback and approve them, but the user's answers never reached the tool: the CLI called onConfirm(ProceedOnce) with no payload, so the tool read an empty answers map and the model never got the decisions. Route updatedInput.answers from the SDK's allow response into the tool confirmation payload so the collected answers reach the tool. Reuses the existing updatedInput channel — no new SDK API or types. Document the pattern in the TypeScript and Python SDK READMEs. * fix(cli): forward ask_user_question answers on teammate approval path Address review feedback on QwenLM#6655: - handleTeammateApproval now mirrors the leader path and promotes the user's answers from updatedInput into the confirmation payload, so ask_user_question calls approved through a teammate no longer drop the user's choices (wenshao). - Extract a shared buildAllowConfirmationPayload helper used by both the leader and teammate paths, and only promote `answers` for ask_user_question so a same-named field on any other tool's input can't leak into the payload. - Add tests for the teammate path and the defensive guards (array updatedInput, array/null/empty answers, foreign answers field). * test(web-shell): stub Range client-rect methods to fix flaky CI CodeMirror's async measure pass (scheduled via requestAnimationFrame) calls getClientRects()/getBoundingClientRect() on a text Range. jsdom implements these on Element but not on Range, so the call throws "textRange(...).getClientRects is not a function" from a rAF callback after the test completed. Vitest surfaces it as an unhandled error and fails the whole run with exit code 1 even though every assertion passed (seen intermittently in useComposerCore.dom.test.tsx). Polyfill both methods on Range.prototype in the shared test setup, mirroring the existing ResizeObserver/scrollIntoView stubs. * refactor(cli): use ToolNames constant and broaden permission tests Address review suggestions on QwenLM#6655: - buildAllowConfirmationPayload now gates answers-promotion on the ToolNames.ASK_USER_QUESTION constant instead of a bare string literal, so a future rename of the tool name is a compile-time break rather than a silent regression. - Add an it.each case for a non-object primitive updatedInput (string) to cover the `typeof updatedInput !== 'object'` guard branch. - Assert the leader path overrides toolCall.request.args with the host's sanitized updatedInput before confirming. - Add a teammate-path test for an allow response with no updatedInput, asserting respond is called with (ProceedOnce, undefined). --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
![qwen-autofix[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
* feat(web-shell): add mobile welcome composer slots * refactor(web-shell): deduplicate MessageList JSX and remove dead CSS reference - Extract ~80 lines of duplicated MessageList rendering into shared variables with conditional props and wrapper - Remove dead chatPaneWithWelcomeMiddle className reference (CSS class never defined) - Document mobileWelcomeFooterMiddle dependency on renderWelcomeFooter in JSDoc * fix(web-shell): stabilize MessageList tree position and conditional customFooter wrapper - Use stable outer wrapper div for IIFE to prevent MessageList unmount/remount when showMobileWelcomeFooterMiddle toggles - Only wrap CustomFooter in styles.customFooter div when hasMobileComposerBottom is true, avoiding DOM depth change for non-mobile consumers * fix(release): raise package size budget to 85 MiB (QwenLM#6688) * fix(interactive): configure Docker sandbox networking for protocol tag retry test (QwenLM#6684) (QwenLM#6689) The protocol-tags-interactive.test.ts started the fake OpenAI server on 127.0.0.1 without Docker-aware host options, making it unreachable from inside the Docker sandbox container. The CLI running in the container tried to connect to 127.0.0.1 which resolved to the container's own loopback, not the host where the test server listens. Bind the fake server to 0.0.0.0 and advertise host.docker.internal as the base URL host when QWEN_SANDBOX is docker or podman, matching the established pattern in tool-control.test.ts. Also set NO_PROXY to include host.docker.internal so the CLI does not route sandbox model requests through an HTTP proxy. Co-authored-by: qwen-autofix[bot] <qwen-autofix[bot]@users.noreply.github.com> * fix(core): keep YOLO mode when the model calls enter_plan_mode (QwenLM#6630) * fix(core): keep YOLO mode when the model calls enter_plan_mode A model-initiated enter_plan_mode call from YOLO silently switched the session into the read-only Plan mode, surprising users who explicitly chose YOLO for low-friction execution and then blocking the reads/writes they expected to proceed. Genuine user-driven plan-mode entries (Shift+Tab, /plan) call setApprovalMode directly and never route through this tool, so guarding the tool only affects the model deciding to plan on its own. From YOLO the tool now keeps the current mode and returns a message telling the model to continue planning without switching. Fixes QwenLM#5970 * fix(core): gate the YOLO plan-mode guard on an explicit user request Addresses review feedback on QwenLM#6630. The previous guard suppressed every enter_plan_mode invocation while the session was in YOLO mode. That fixes the unsolicited switch reported in QwenLM#5970, but it also blocks the legitimate path: the tool description tells the model to call this tool only after the user explicitly asks, and /plan is interactive-only (supportedModes: ['interactive']) with no Shift+Tab equivalent. In a headless or ACP YOLO session the tool is the only door into plan mode, so a blanket guard made an explicit user request unreachable. Add an optional userRequested flag to the tool schema and only no-op when the entry is NOT user-requested. A user-requested entry still goes through setApprovalMode(PLAN, { enteredByModel: true }) so the Plan Approval Gate on exit continues to run for AUTO/YOLO sessions (QwenLM#5574). * fix(core): address review suggestions on the YOLO plan-mode guard - Log via debugLogger.info when the guard suppresses a model-initiated entry, so a "I asked for plan mode and nothing happened" report is diagnosable by grepping ENTER_PLAN_MODE (the other early-return paths already log). - Strengthen the userRequested:false test to assert on the returned llmContent/returnDisplay, matching the unsolicited-entry sibling test. - Add a defensive test pinning that userRequested is inert outside YOLO: DEFAULT with the flag set enters plan mode normally. --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> * feat(cli): forward ask_user_question answers from SDK can_use_tool (QwenLM#6655) * feat(cli): forward ask_user_question answers from SDK can_use_tool SDK-hosted agents could receive ask_user_question calls through the can_use_tool callback and approve them, but the user's answers never reached the tool: the CLI called onConfirm(ProceedOnce) with no payload, so the tool read an empty answers map and the model never got the decisions. Route updatedInput.answers from the SDK's allow response into the tool confirmation payload so the collected answers reach the tool. Reuses the existing updatedInput channel — no new SDK API or types. Document the pattern in the TypeScript and Python SDK READMEs. * fix(cli): forward ask_user_question answers on teammate approval path Address review feedback on QwenLM#6655: - handleTeammateApproval now mirrors the leader path and promotes the user's answers from updatedInput into the confirmation payload, so ask_user_question calls approved through a teammate no longer drop the user's choices (wenshao). - Extract a shared buildAllowConfirmationPayload helper used by both the leader and teammate paths, and only promote `answers` for ask_user_question so a same-named field on any other tool's input can't leak into the payload. - Add tests for the teammate path and the defensive guards (array updatedInput, array/null/empty answers, foreign answers field). * test(web-shell): stub Range client-rect methods to fix flaky CI CodeMirror's async measure pass (scheduled via requestAnimationFrame) calls getClientRects()/getBoundingClientRect() on a text Range. jsdom implements these on Element but not on Range, so the call throws "textRange(...).getClientRects is not a function" from a rAF callback after the test completed. Vitest surfaces it as an unhandled error and fails the whole run with exit code 1 even though every assertion passed (seen intermittently in useComposerCore.dom.test.tsx). Polyfill both methods on Range.prototype in the shared test setup, mirroring the existing ResizeObserver/scrollIntoView stubs. * refactor(cli): use ToolNames constant and broaden permission tests Address review suggestions on QwenLM#6655: - buildAllowConfirmationPayload now gates answers-promotion on the ToolNames.ASK_USER_QUESTION constant instead of a bare string literal, so a future rename of the tool name is a compile-time break rather than a silent regression. - Add an it.each case for a non-object primitive updatedInput (string) to cover the `typeof updatedInput !== 'object'` guard branch. - Assert the leader path overrides toolCall.request.args with the host's sanitized updatedInput before confirming. - Add a teammate-path test for an allow response with no updatedInput, asserting respond is called with (ProceedOnce, undefined). --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> * fix(cli): localize approval mode UI labels (QwenLM#6592) * fix(cli): localize approval mode UI labels * fix(cli): address approval mode i18n review * fix(cli): stabilize approval mode i18n key * test(cli): cover approval mode i18n follow-up * test(cli): cover localized auto indicator * test(cli): address approval i18n suggestions --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> * feat(dingtalk): mention response senders (QwenLM#6679) * docs: design DingTalk at-sender replies * docs: plan DingTalk at-sender replies * feat(channels): preserve session for response delivery * feat(dingtalk): optionally mention response sender * docs(dingtalk): explain response mentions * fix(dingtalk): retain queued mention targets * fix(dingtalk): bound mention target lifecycle * fix(dingtalk): clear synthetic command mention target * fix(dingtalk): clear buffered targets on session death * debug(dingtalk): log mention delivery result * fix(dingtalk): render response mentions * fix(dingtalk): send visible response mentions * feat(dingtalk): use text replies for mentions * fix(dingtalk): preserve mentioned text replies * feat(web-shell): add artifact right panel (QwenLM#6591) * feat(web-shell): add artifact right panel * fix(web-shell): address artifact panel review feedback * fix(web-shell): handle artifact panel review edge cases * fix(web-shell): tighten scheduled task parsing * fix(web-shell): address artifact panel review followups * fix(web-shell): guard large file diff stats * fix(web-shell): address review panel suggestions * test(webui): stabilize heartbeat prompt cleanup test * fix(web-shell): address artifact review refresh issues * test(web-shell): stabilize ChatPane artifact hook mock * fix(web-shell): clear stale session artifacts while loading * fix(web-shell): preserve artifact tabs during refresh * fix(web-shell): address artifact review followups * fix(web-shell): respect workspace cwd for artifact outputs * fix(web-shell): scope artifact panel actions to pane * fix(web-shell): resolve split pane merge conflict * fix(web-shell): clear stale artifact panel state * fix(web-shell): preserve leading turn outputs * fix(web-shell): tighten turn output selectors * fix(web-shell): harden artifact preview sanitizer * fix(web-shell): address artifact panel review regressions * fix(web-shell): reconcile split pane artifact snapshots * fix(web-shell): clear pane artifacts on session switch * fix(web-shell): clear stale right panel snapshots * fix(web-shell): repair scheduled task hint string --------- Co-authored-by: ytahdn <ytahdn@gmail.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> * feat(cli): workspace-qualified ACP transport (daemon multi-workspace phase 4) (QwenLM#6621) * docs(design): add daemon multi-workspace phase 4 (workspace-qualified ACP) design * feat(cli): add workspace-qualified ACP transport (issue QwenLM#6378 phase 4) Per-runtime ACP dispatcher at /workspaces/:workspace/acp (HTTP + WS) dispatched by URL path from the single upgrade listener; per-runtime device-flow + reverse client-MCP; owner-index via bridge lifecycle; untrusted/unknown rejected; legacy /acp unchanged; advertise workspace_qualified_acp for multi-workspace. * fix(cli): keep per-runtime device-flow registry out of serve fast-path bundle Phase 4 secondary-runtime device-flow statically imported createDeviceFlowRegistry into run-qwen-serve, pulling glob/@iarna/toml into the serve fast-path bundle and failing the closure check. Import it dynamically at the creation site; the check now passes and behavior is unchanged. * refactor(cli): drop per-runtime device-flow for secondary workspaces Follow-up to the fast-path fix: instead of dynamically importing createDeviceFlowRegistry for secondary runtimes, drop the per-runtime device-flow wiring entirely. Secondary ACP device-flow falls back to the dispatcher default, keeping the serve fast-path bundle closure clean without the dynamic-import indirection. WorkspaceRuntime.deviceFlowRegistry stays optional for a future per-runtime hook. * fix(cli): share daemon-global device-flow across ACP mounts; harden WS path parsing Secondary ACP mounts share the daemon-global device-flow registry (single instance per daemon) instead of a per-runtime one; the event sink fans out to every trusted runtime bridge so secondary ACP clients receive their own flow events, fixing the reviewer QwenLM#6621 Critical and the CI test failure. Drops WorkspaceRuntime.deviceFlowRegistry. WS upgrade path is parsed from the raw request-target instead of new URL().pathname, rejecting %2e%2e / backslash / dot-segment traversal. * refactor(cli): gate CDP claim on primary mount; return plural ACP POST promise Add a primary flag to RuntimeAcpMount so a secondary workspace's ACP connection cannot claim the CDP tunnel -- the claim is gated on activeMount.primary, matching the primary-only chrome-devtools MCP wiring. The plural /workspaces/:workspace/acp POST handler returns the dispatch promise instead of voiding it. * refactor(cli): centralize ACP-HTTP enablement in resolveAcpHttpEnabled Add resolveAcpHttpEnabled() as the single interpretation of the QWEN_SERVE_ACP_HTTP opt-out, replacing four independent env checks across mount, voice-WS advertisement, and CDP-MCP gating. Advertise workspace_qualified_acp only when the ACP HTTP surface is enabled AND multi-workspace sessions are active, so it is not announced when ACP HTTP is disabled. * feat(cli): ACP dispose 503 gate + aggregate connection snapshot across mounts After dispose() the shared ACP HTTP handlers (legacy /acp + workspace-qualified) return 503 server_disposed instead of racing torn-down registries during the shutdown drain. Add AcpHttpHandle.getSnapshot() aggregating connection and wsStream counts across the primary mount and every trusted secondary runtime, and switch the metrics sampler to it so daemon metrics report all workspaces' ACP connections rather than only the primary's. * test(cli): cover ACP dispose 503, aggregate snapshot, and raw dot-segment WS reject * docs(design): record Phase 4 ACP systematic rework (8-axis hardening) Correct the Summary (the device-flow registry stays daemon-global and shared, not per-runtime) and add a section documenting the final architecture: runtime mount factory, routing/trust isolation, raw request-target WS parsing, daemon-global device-flow with event-sink fan-out, primary-only CDP, disposed 503 gate, aggregate getSnapshot, and resolveAcpHttpEnabled-gated capability advertisement. * fix(cli): align /daemon/status ACP counts with the aggregate mount snapshot Code review found a drift: the metrics sampler switched to the aggregate AcpHttpHandle.getSnapshot() (all mounts) while /daemon/status still read the primary-only registry snapshot, so the two observability surfaces diverged under multi-workspace. Extend AcpHttpSnapshot to aggregate all transport counters (connection/session/sse/ws streams + pending client requests) and feed the /daemon/status transport summary from it; per-connection diagnostics and the connection cap stay primary-scoped. Also refresh the device-flow-registry doc comment to the daemon-global shared model. * test(cli): regression-test device-flow on a trusted secondary workspace Locks in the reviewer Critical fix: a trusted secondary workspace's ACP now shares the daemon-global device-flow registry, so device_flow/start reaches provider resolution (an unsupported-provider error here) instead of erroring 'Device flow not configured'. Wires a shared DeviceFlowRegistry into the test harness and drives initialize + device_flow/start over the secondary WebSocket. * docs(design): mark the superseded per-runtime device-flow section Address PR QwenLM#6621 review: the pre-rework 'Per-runtime device-flow registry' section contradicted Systematic rework axis 4 (daemon-global shared registry + fan-out). Flag it as superseded design-history so readers don't build the wrong mental model. * refactor(cli): mount ACP only for trusted secondary workspaces Address PR QwenLM#6621 review suggestions: (1) skip creating a dispatcher/registry/remember-lane for untrusted non-primary workspaces (they are 403-rejected before any mount lookup), so they no longer appear as always-zero entries in the aggregate getSnapshot(); (2) test that a secondary workspace cannot claim the process-wide CDP tunnel (primary-only guard); (3) test that a WS upgrade to an unknown selector is rejected 400. * test(cli): cover device-flow event fan-out across bridges Address PR QwenLM#6621 review: the resolveEventBridges fan-out (the reviewer Critical fix's core delivery path) had zero test coverage. Add unit tests that a device-flow event reaches every resolved bridge, that one bridge throwing does not block the others (best-effort), and that it falls back to the single bridge when no resolver is provided. * fix(cli): report ACP connection pressure across all mounts Address PR QwenLM#6621 review: the connection_capacity_high warning read the primary mount's snapshot only, so a saturated secondary workspace was invisible. Compute the busiest mount from the aggregate snapshot (per-mount cap is uniform, opts.maxConnections) so any mount nearing capacity triggers the warning. * test(cli): allow acp-http-enabled.ts in the serve process.env guard Fix CI failure on PR QwenLM#6621: the serve process.env guard flagged the new acp-http-enabled.ts as a direct process.env reader. It is the QWEN_SERVE_ACP_HTTP interpreter extracted from index.ts and serve-features.ts (both already allow-listed); QWEN_SERVE_ACP_HTTP is a daemon-level process-global toggle, so the file inherits their allow-list entry. * docs: harden workspace-qualified ACP design Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs: plan workspace-qualified ACP hardening Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): align workspace-qualified ACP routing Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): harden qualified ACP request errors Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): cover unmarked URIError fallback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): make ACP disposal terminal Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): aggregate ACP connection diagnostics Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * chore: remove review process artifact Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address workspace ACP review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): finish ACP review follow-ups Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qqqys <qys177@gmail.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: qwen-autofix[bot] <qwen-autofix[bot]@users.noreply.github.com> Co-authored-by: nas <156536069+Nas01010101@users.noreply.github.com> Co-authored-by: Tianyuan <2720711917@qq.com> Co-authored-by: han <2992336417@qq.com> Co-authored-by: ytahdn <1294726970@qq.com> Co-authored-by: ytahdn <ytahdn@gmail.com> Co-authored-by: jinye <djy1989418@126.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
4 participants
What this PR does
Delivers the answers a user picks for an
ask_user_questiontool call back to the model when the session is driven by the TypeScript or Python SDK. When the host approves the tool through thecan_use_toolcallback and returns the collected answers on the allow response, the CLI now forwards those answers to the tool so the model receives the user's decisions.Why it's needed
SDK-hosted agents already received
ask_user_questioncalls through thecan_use_toolcallback and could approve them, but the user's answers never reached the tool. On approval the CLI confirmed the tool with no answer payload, so the tool always saw an empty set of answers and the model never learned what the user chose. This forced SDK integrations to steer the model away from the tool entirely, degrading interactive question flows.The fix reuses the existing
updatedInputchannel that the callback already returns — the host returns the questions' answers asupdatedInput.answers, and the CLI routes them into the tool confirmation. No new SDK callback, API surface, or types are introduced. The TypeScript and Python SDK READMEs document the pattern.Reviewer Test Plan
How to verify
Run the permission controller unit tests:
npx vitest run packages/cli/src/nonInteractive/control/controllers/permissionController.test.ts. Two new cases cover the behavior — one asserts that answers returned on the allow response reach the tool confirmation forask_user_question, and one asserts a normal sanitized tool (no answers) still confirms with just its updated input. The existingask_user_questiontool tests (packages/core/src/tools/askUserQuestion.test.ts) continue to pass, confirming the tool consumes the forwarded answers.End to end from an SDK: register a
can_use_toolcallback, and whentool_nameisask_user_question, presenttool_input.questionsto the user, collect their choices into an index-keyed map, and return{ behavior: 'allow', updatedInput: { ...input, answers } }. The model then receives the selected answers instead of an empty result.Evidence (Before & After)
N/A — no user-visible TUI change; this is SDK/host wiring plus docs.
Tested on
Environment (optional)
Unit tests via vitest.
Risk & Scope
updatedInput; guarded to only attachanswerswhen it is a plain object, and responses withoutupdatedInputkeep the previous single-argument confirmation call, so non-ask_user_questiontools are unaffected.Linked Issues
Closes #6647
中文说明
这个 PR 做了什么
当会话由 TypeScript 或 Python SDK 驱动时，把用户对
ask_user_question工具调用所选择的答案回传给模型。当宿主通过can_use_tool回调批准该工具并在 allow 响应中返回收集到的答案时，CLI 现在会把这些答案转发给工具，使模型收到用户的决定。为什么需要它
SDK 托管的 agent 已经能通过
can_use_tool回调收到ask_user_question调用并批准它，但用户的答案从未到达工具。批准时 CLI 确认工具却没有携带答案 payload，因此工具始终看到空答案，模型永远不知道用户选了什么。这迫使 SDK 集成方彻底避免使用该工具，削弱了交互式提问能力。该修复复用了回调本就返回的
updatedInput通道——宿主把答案作为updatedInput.answers返回，CLI 将其接入工具确认。不引入新的 SDK 回调、API 或类型。TypeScript 与 Python SDK 的 README 已补充该用法。审阅测试计划
如何验证
运行权限控制器单测：
npx vitest run packages/cli/src/nonInteractive/control/controllers/permissionController.test.ts。新增两个用例——一个断言 allow 响应中返回的答案会到达ask_user_question的工具确认，另一个断言普通的被净化工具（无答案）仍只带其更新后的输入进行确认。现有的ask_user_question工具测试（packages/core/src/tools/askUserQuestion.test.ts）继续通过，确认工具会消费转发过来的答案。从 SDK 端到端：注册一个
can_use_tool回调，当tool_name为ask_user_question时，把tool_input.questions展示给用户，将其选择收集为以下标为键的映射，并返回{ behavior: 'allow', updatedInput: { ...input, answers } }。模型随后即可收到所选答案而非空结果。证据（前后对比）
N/A —— 无用户可见的 TUI 变化；这是 SDK/宿主的连线加文档。
测试平台
风险与范围
updatedInput，确认 payload 现在就会被填充；已加保护，仅当其为普通对象时才附加answers，且没有updatedInput的响应保持此前的单参数确认调用，因此非ask_user_question工具不受影响。关联 Issue
Closes #6647