fix(channels): exclude discrete ACP messages from replies - #7223
Merged
Conversation
Collaborator
Author
Verification reportReproduction before the fixThe two deterministic bridge regressions failed with the same observed output: This reproduces the production ordering where the discrete background completion frame lands after the final assistant chunk and before Verification after the fixcd packages/channels/base
npx vitest run src/AcpBridge.test.ts src/DaemonChannelBridge.test.ts
Test Files 2 passed (2)
Tests 82 passed (82)Additional checks:
An independent test-engineer pass reran both bridge suites and the package build, reviewed the event flow, and found no blocking issue. The original Scope noteFirst-class proactive delivery of discrete background-task updates to Channel users remains out of scope. This PR fixes the user-visible reply contamination without expanding the Channel event contract. |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qqqys
marked this pull request as ready for review
July 19, 2026 15:58
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with a linked issue (#7222) and a clear deterministic reproduction — emit a normal assistant chunk, then a discrete background-notification chunk ( Direction: aligned — this fixes a real user-facing bug where Channel clients receive an internal status line appended to the final answer. The Size: not applicable — Approach: the scope is exactly right. The fix extends the existing Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，有关联 issue（#7222）和明确的确定性复现——发送一个普通助手 chunk，然后发送一个带有 方向：对齐——修复了 Channel 客户端收到内部状态行拼接到最终答案的真实用户 bug。 规模：不适用—— 方案：范围恰好。修复在 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
Code ReviewIndependent proposal: given the problem (discrete ACP messages leaking into Channel replies), I would extend the existing Comparison with the diff: the PR does exactly this. Both production changes are a single The No blockers. Clean, minimal fix. Unit TestsAll 82 tests pass (30 AcpBridge + 52 DaemonChannelBridge), including the two new regression tests: Real-Scenario TestingThis bug lives in the Channel bridge layer — it only manifests when a daemon-backed or legacy ACP Channel client (e.g. DingTalk) receives a background agent completion frame during the prompt-drain window. The regular CLI TUI does not go through the Channel bridge path, so the bug cannot be reproduced via The PR itself notes: "This is timing-sensitive in production but deterministic in a bridge unit test." The unit tests above ARE the deterministic reproduction — they emit the exact frame sequence (normal chunk → discrete chunk → turn_complete) and verify the fix. Smoke test confirming the dev build starts and responds: (Build starts, model responds, no crashes.) 中文说明代码审查独立方案： 针对离散 ACP 消息泄漏到 Channel 回复的问题，我会在 与 diff 对比： PR 完全这样做了。两处生产代码改动都是在现有守卫上增加一个
无阻塞项。干净、最小的修复。 单元测试82 个测试全部通过（30 个 AcpBridge + 52 个 DaemonChannelBridge），包括两个新的回归测试。 真实场景测试此 bug 存在于 Channel bridge 层——仅在 daemon-backed 或旧 ACP Channel 客户端（如钉钉）在 prompt-drain 窗口期间收到后台 Agent 完成帧时才会出现。常规 CLI TUI 不经过 Channel bridge 路径，因此无法通过 PR 本身指出："这在生产环境中对时序敏感，但在 bridge 单元测试中是确定性的。"上述单元测试就是确定性复现——它们发送精确的帧序列（正常 chunk → 离散 chunk → turn_complete）并验证修复。 冒烟测试确认 dev 构建正常启动和响应。 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — clean, minimal bugfix with a linked issue, deterministic reproduction, and a two-line fix that follows the exact pattern already used across four other consumers of this flag. This is what a good bugfix looks like: observed bug (#7222), clear root cause (Channel bridges missing the The independent proposal I wrote before reading the diff was identical to what the PR does — extend the existing guard in both 82/82 tests pass. Dev build starts and responds. The Channel bridge path can't be exercised via the CLI TUI (it requires a daemon-backed Channel client), but the unit tests are the deterministic reproduction and they cover the exact frame sequence from the issue. 中文说明置信度：5/5 — 干净、最小的 bugfix，有关联 issue、确定性复现，以及一个遵循此标志在其他四个消费者中已使用的精确模式的两行修复。 这是一个好的 bugfix 应有的样子：观测到的 bug（#7222），清晰的根因（Channel bridge 缺少其他消费者已有的 我在阅读 diff 之前写的独立方案与 PR 完全一致——在两个 bridge 中扩展现有守卫。不存在更简路径；修复已经是最小的。 82/82 测试通过。Dev 构建正常启动和响应。Channel bridge 路径无法通过 CLI TUI 验证（需要 daemon-backed Channel 客户端），但单元测试是确定性复现，覆盖了 issue 中的精确帧序列。 — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
approved these changes
Jul 19, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
approved these changes
Jul 19, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.8-max-preview via Qwen Code /review
Collaborator
Local verification reportI built and ran this PR locally, including two real-transport end-to-end harnesses (not just the unit tests shipped in the diff), and toggled only the 6 changed source lines between the base commit ( Verdict: the fix is correct, minimal, symmetric across both bridge paths, and safe to merge. One non-blocking observation and one pre-existing adjacent gap are noted at the end.
1. The reported bug reproduces, and the fix resolves it
The pre-fix assertion failure is byte-identical to the string in #7222: 2. About the E2E harnessesThe unit tests in the diff inject a mocked
Both harnesses also replay the frames actually emitted by Useful confirmation from the ACP run: the bridge still saw 5 raw ACP frames while emitting only 1 3. Regression surface
The single Environment: Linux x86_64, Node.js v22.22.2, npm 10.9.7. macOS and Windows not tested locally (CI 4. Observation — the trade this makes (non-blocking, please confirm intent)The filter is keyed on
I confirmed Minor related note: the three consumers now define "discrete" slightly differently — web-shell requires 5. Pre-existing adjacent gap in the same timing window (NOT a regression — recommend a follow-up issue)While auditing I found a second failure mode in the identical drain window that this PR does not close. A background-notification turn is a full agent turn: Result: the user's already-accumulated final answer is wiped, and the Channel reply is empty. I verified this on both transports and — importantly — it fails identically before and after this PR: So this PR neither causes nor worsens it, and it should not block the merge. But it lives in the same window as #7222 and is arguably worse (silence instead of a stray status line). The durable fix is probably to stamp a turn-level origin on all frames belonging to a notification turn — The verification harnesses (fake ACP agent + stub daemon + runner script) are throwaway scratch code living outside the tree; I can clean them up into 中文说明本地验证报告我在本地构建并运行了这个 PR，除了 diff 里自带的单元测试之外，还额外搭建了两套真实传输层的端到端测试。整个过程中我只在基线 commit（ 结论：修复正确、改动最小、两条 bridge 路径对称，可以合并。 文末有一条非阻塞的确认项和一个既存的相邻问题。 1. 缺陷可复现，修复有效
修复前的断言失败信息与 #7222 中描述的字符串完全一致： 2. 关于这两套 E2Ediff 中的单元测试注入的是 mock 过的
两套 E2E 回放的都是 ACP 那次运行还给出一个有用的确认：bridge 一共看到 5 个原始 ACP 帧，但只发出了 1 次 3. 回归面
环境：Linux x86_64、Node.js v22.22.2、npm 10.9.7。本地没有测试 macOS 和 Windows（CI 的 4. 一点观察 —— 这个修复实际做的取舍（非阻塞，想确认一下是否符合预期）过滤条件是
我确认了 一个相关的小提醒：目前三个消费方对"discrete"的定义已经略有分歧 —— web-shell 要求 5. 同一时间窗内的既存相邻问题（不是本 PR 引入的回归 —— 建议单开 issue）在审查过程中我发现在完全相同的 drain 时间窗内还存在第二种失败模式，本 PR 没有覆盖到。后台通知的处理本身是一个完整的 agent turn：当模型在消费该通知时决定调用工具， 结果是：用户已经累积好的最终答复被清空，Channel 收到的是空回复。我在两条传输链路上都验证了这一点，并且——关键在于——它在本 PR 前后的表现完全一致： 所以本 PR 既没有引入也没有加重它，不应该阻塞合并。但它和 #7222 处在同一个时间窗内，而且后果可以说更严重（用户收到的是空消息，而不是多一行状态文本）。更彻底的修法可能是给属于通知 turn 的所有帧（包括 本次验证用的 harness（假 ACP agent + stub daemon + 运行脚本）是放在仓库之外的一次性脚本；如果团队觉得有价值，我可以把它整理进 Verified locally with Claude Code (Opus 4.8). |
wenshao
approved these changes
Jul 20, 2026
Collaborator
Verification report — LGTM ✅I verified this PR on Linux with a real end-to-end reproduction, not just the unit tests: a real Result: the bug is real, reproduces deterministically end-to-end, and the PR fixes it on both paths with no collateral damage. 1. The bug reproduces with a real background agent — on both bridgesI drove two channel messages: message 1 launches a background agent; message 2 asks a normal question while that agent finishes. The string below is exactly what
The daemon-path harness is wired exactly like production 2. Why the window is not actually narrow — it is deterministicThe PR body calls this "the narrow prompt-drain window". On the raw ACP wire it is not a coin flip.
Practically: any background agent that completes while a channel turn is in flight corrupts that turn's reply on base. That makes the fix more valuable than the PR body suggests, not less. 3. Unit-test A/B
4. Scope review — the guard is complete and correctly placed
5. On the stated tradeoffThe "Risk & Scope" note is accurate, and I want to confirm it does not lose working behaviour. One consequence to be aware of: a turn whose only assistant output is discrete now yields an empty reply string, and Reprogit worktree add /tmp/pr7223 pr-7223 && cd /tmp/pr7223 && npm ci
# mock OpenAI on :8791; isolated HOME with tools.approvalMode=yolo
# msg 1 -> model returns tool_call agent{description:"Explore", run_in_background default}
# the sub-agent's model reply is withheld until msg 2 is in flight, so the agent
# completes while pendingPrompt is set; msg 2's reply is then stalled 2.5s.
QWEN_DEFAULT_AUTH_TYPE=openai OPENAI_BASE_URL=http://127.0.0.1:8791/v1 \
node acp-bridge-e2e.mjs # AcpBridge -> real cli-entry.js --acp
node daemon-bridge-e2e.mjs # DaemonChannelBridge -> real qwen serve
# A/B: git checkout bc0e2cd18 -- src/AcpBridge.ts src/DaemonChannelBridge.ts
# && (cd packages/channels/base && npm run build)Note for anyone re-running a bundle-level A/B: esbuild inlines each channel package's built Tested on
No blocking issues. Suggest only a PR-body addendum noting the 中文版（合并参考）验证报告 — 建议合并 ✅我在 Linux 上做了真实端到端复现，而不只是跑单测：真实的 结论：Bug 真实存在、端到端可确定性复现，本 PR 在两条路径上都修好了，且没有副作用。 1. 真实后台 Agent 下的复现 — 两条 bridge 都中招我发送了两条 channel 消息：消息 1 启动后台 Agent；消息 2 在该 Agent 即将完成时问一个普通问题。下面的字符串就是
daemon 路径的 harness 完全按生产代码 2. 这个时间窗其实不"狭窄"，而是确定性的PR 描述称之为"狭窄的 prompt drain 时间窗"。但在 ACP 原始协议流上，这并不是一个概率事件。 实际含义：在 base 上，任何在 channel turn 进行中完成的后台 Agent 都会污染该轮回复。这说明本修复的价值比 PR 描述所说的更高，而不是更低。 3. 单测 A/B
4. 范围审查 — guard 完整且位置正确
5. 关于所述的权衡"Risk & Scope" 部分的说法是准确的，我想确认它没有丢掉原本可用的行为。 需要留意的一个后果：如果某一轮的助手输出全部是离散消息，回复字符串会为空，而 复现方式git worktree add /tmp/pr7223 pr-7223 && cd /tmp/pr7223 && npm ci
# 在 :8791 起 mock OpenAI；隔离 HOME，设置 tools.approvalMode=yolo
# 消息 1 -> 模型返回 tool_call agent{description:"Explore"，默认后台执行}
# 子 Agent 的模型回复被扣住，直到消息 2 进入处理中，从而让 Agent 在
# pendingPrompt 仍然置位时完成；随后消息 2 的回复被拖延 2.5 秒。
QWEN_DEFAULT_AUTH_TYPE=openai OPENAI_BASE_URL=http://127.0.0.1:8791/v1 \
node acp-bridge-e2e.mjs # AcpBridge -> 真实 cli-entry.js --acp
node daemon-bridge-e2e.mjs # DaemonChannelBridge -> 真实 qwen serve
# A/B：git checkout bc0e2cd18 -- src/AcpBridge.ts src/DaemonChannelBridge.ts
# && (cd packages/channels/base && npm run build)给后续做 bundle 级 A/B 的同学提个醒：esbuild 内联的是各 channel 包已构建的 测试平台
没有阻塞性问题。唯一建议是在 PR 描述里补一句： |
This was referenced Jul 20, 2026
qwen-code-dev-bot
added a commit
that referenced
this pull request
Jul 20, 2026
pull Bot
pushed a commit
to bit-cook/qwen-code
that referenced
this pull request
Jul 20, 2026
…LM#7246) * fix(channels): exclude discrete messages from replies * feat(serve): make ACP initialize handshake timeout configurable Add --initialize-timeout-ms CLI flag to qwen serve, wiring it through to BridgeOptions.initializeTimeoutMs. The ACP initialize handshake defaults to 10 s (DEFAULT_INIT_TIMEOUT_MS); containerized deployments where the child process needs longer can now raise the ceiling without patching the source. Fixes QwenLM#7244 * fix(serve): wire initializeTimeoutMs to fast-path parser and embed bridge Add the missing NUMBER_OPTIONS entry in fast-path.ts and forward initializeTimeoutMs in the server.ts inline createAcpSessionBridge call so the direct-embed / test path also respects the flag. * fix(serve): address review — fast-path test, timer upper bound, revert QwenLM#7223, docs (QwenLM#7246) * test(cli): add happy-path propagation test for initializeTimeoutMs (QwenLM#7246) * refactor(cli): reuse isPositiveIntegerMs for initializeTimeoutMs validation (QwenLM#7246) --------- 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: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Collaborator
|
Released in v0.20.1. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This PR keeps ACP messages marked as discrete out of Channel reply accumulation on both the daemon-backed and legacy bridge paths. It also adds regression coverage for the timing window where a background agent completion lands after the final assistant text but before the turn-complete signal.
Why it's needed
Background task completion displays are emitted as assistant chunks with
qwenDiscreteMessage: truebecause UI clients must render them as separate events. Channel bridges previously discarded that metadata and treated the display as ordinary assistant text. If the completion arrived during the narrow prompt-drain window, users received a correct final answer with an internalBackground agent ... completed.status line appended to it.Reviewer Test Plan
How to verify
Run the focused Channel bridge tests and confirm that both implementations return only the final assistant answer when a discrete background completion frame arrives immediately before turn completion. Also confirm that ordinary assistant chunks, slash-command output, nested-agent suppression, tool-call boundaries, and permission boundaries remain green in the same suites.
Evidence (Before & After)
Before: the deterministic reproduction returned
Final answer.Background agent "Explore" completed.on both bridge paths.After: both bridge paths return only
Final answer.and all 82 focused tests pass.Tested on
Environment (optional)
Linux x86_64, Node.js 22.23.0, npm 10.9.8. Verified with the complete Channel base bridge test files, package TypeScript build, targeted ESLint, targeted Prettier, and
git diff --check.Risk & Scope
Linked Issues
Closes #7222
中文说明
本 PR 做了什么
本 PR 在 daemon-backed 和旧 ACP 两条桥接路径中，都不再把标记为离散消息的 ACP 内容累积进 Channel 回复。同时增加回归测试，覆盖后台 Agent 完成通知恰好在最终助手文本之后、turn-complete 信号之前到达的时间窗口。
为什么需要
后台任务完成提示以带有
qwenDiscreteMessage: true的助手 chunk 发出，因为 UI 客户端应将它渲染为独立事件。Channel bridge 之前丢弃了该元数据，并把提示当作普通助手文本。如果完成通知在 prompt drain 的狭窄时间窗口内到达，用户就会收到正确的最终答案，但末尾会被拼上内部的Background agent ... completed.状态行。Reviewer 测试计划
如何验证
运行聚焦的 Channel bridge 测试，确认离散的后台完成帧在 turn completion 之前立即到达时，两种实现都只返回最终助手答案。同时确认同一测试集中普通助手 chunk、斜杠命令输出、嵌套 Agent 输出抑制、工具调用边界和权限边界仍然通过。
证据（Before & After）
修复前：确定性复现在两条桥接路径上都会返回
Final answer.Background agent "Explore" completed.。修复后：两条桥接路径都只返回
Final answer.，并且 82 个聚焦测试全部通过。测试平台
环境（可选）
Linux x86_64、Node.js 22.23.0、npm 10.9.8。已通过完整的 Channel base bridge 测试文件、package TypeScript 构建、定向 ESLint、定向 Prettier 和
git diff --check验证。风险和范围
关联 Issue
关闭 #7222