feat(channels): add BTW side questions to Channel conversations - #10713
Conversation
|
|
E2E validation reportStatus: automated validation complete; temporary live environment retired.
Live external messages were not sent by the implementation pass. A future manual validation can run a normal long task, send |
|
@qwen-code /takeover from 0 |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 |
|
Thanks for the PR! Template looks good ✓ Problem: real, verified gap. Direction: aligned. This is surface parity with shipped infrastructure rather than new machinery: sender authorization reuses the existing channel gates, and nothing touches auth/sandbox/model-selection/telemetry contracts. Size: 229 production lines (+227/−2 in Approach: the scope matches the delivery plan accepted on #10711 — parsing, authorization, one side question per session, immediate acknowledgement, background delivery, stale-delivery suppression, fail-closed unsupported. The diff carries nothing beyond that. One upfront note: this half only becomes functional once the bridge half (#10712) exposes the capability; until then Risk: no high-risk path matches — no elevated risk signals. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:真实且已核实的缺口。 方向:对齐。这是对已发布基础设施的能力面补齐而非新机制:发送者授权复用现有频道门禁,不触及 auth/沙箱/模型选择/遥测契约。 规模:生产逻辑 229 行( 方案:范围与 #10711 认可的交付计划一致——解析、授权、每会话单个旁路问题、即时确认、后台投递、过期投递抑制、不支持时失败即关闭。diff 中没有超出该范围的内容。提前说明一点:这一半只有在桥接侧(#10712)暴露能力后才可用;在此之前 风险:未命中高风险路径——无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewBefore reading the diff, my independent proposal was: parse What stood out reading the diff:
No blockers. Two non-blocking notes:
sequenceDiagram
participant P1 as Sender
participant P2 as ChannelBase
participant P3 as Bridge btw
participant P4 as Session router
P1->>P2: send /btw question
P2->>P2: validate text, attachments, authorization
P2->>P4: resolve current session and target
P2-->>P1: ack BTW id, main task continues
P2->>P3: btw sessionId, question, abort signal
P3-->>P2: answer
P2->>P2: re-check session, target, task, bridge identity
alt still current
P2-->>P1: deliver BTW id answer
else cleared, closed, rerouted, died, replaced
P2->>P2: drop stale answer
end
Test evidence — the PR's own CIAt review time the ubuntu unit-test job is still running on the reviewed commit; the macOS/Windows test jobs were skipped in this run, and the security and precheck gates passed. The finalize job updates the table below once CI settles. Not verified here: Windows/Linux behavior (test jobs skipped in this run; the author tested macOS only). The author reports all 649 Channel base tests passing locally, including 16 focused Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Sandboxed verification would settle part of this: 中文说明代码审查读 diff 之前我的独立方案是:在 prompt 路由之前解析 要点:
无阻塞项。两点非阻塞提醒:
测试证据审查时 ubuntu 单测 job 仍在运行;本次 macOS/Windows 测试 job 为 skipped,安全与预检已通过,表格将由 finalize 任务在 CI 落定后更新。此处未验证:Windows/Linux 行为(本次运行中相应 job 被跳过,作者仅在 macOS 上测试)。作者自述 649 个 Channel base 测试全部通过(含 16 个 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, focused implementation that does exactly what the accepted issue plan asked for, with genuinely strong lifecycle test coverage; the only reservations are non-blocking (functional dependency on the companion bridge PR, and no timeout on the per-session slot). Stepping back: my independent proposal and the PR's approach are essentially the same design, and I could not find a simpler path that still meets #10711's lifecycle requirements — every event the issue lists (clear, close, reroute, session loss, bridge replacement) has both a cancellation hook and a delivery-time re-check, which is precisely the watch item the issue triage called out, and it has dedicated tests pinning it. The code reads like the file around it — sanitized logging, thread messages, router/namedSessions reuse — and the 363 test lines are not filler: they pin the guards rather than restate the implementation. This is also one half of a coherent two-PR plan against a triaged issue, not volume. Approving with one honest caveat: this half only becomes functional once #10712 lands; until then CI is still running on the reviewed commit, so approval is deferred until it lands green on 中文说明置信度:4/5 —— 干净、聚焦的实现,恰好完成了已被接受的 issue 计划所要求的内容,生命周期测试覆盖扎实;仅有的保留意见均为非阻塞项(功能上依赖配套 bridge PR,以及每会话名额没有超时)。 整体来看:我的独立方案与 PR 的做法本质上是同一设计,且我没有找到在满足 #10711 生命周期要求前提下更简单的路径——issue 列出的每个事件(clear、close、reroute、会话丢失、bridge 替换)都同时具备取消钩子和投递前复查,这正是 issue 分诊时点名的关注点,并且有专门测试固化。代码风格与所在文件一致——净化日志、线程消息、复用 router/namedSessions——363 行测试也非充数:它们固化的是守卫逻辑而非复述实现。这也是针对已分诊 issue 的两 PR 连贯计划的一半,而非刷量。 予以批准,但带一个诚实的提醒:这一半要等 #10712 合入后才可用;在此之前 审查提交上的 CI 仍在运行,因此批准推迟至其在 — Qwen Code · qwen3.8-max Reviewed at |
Critical: - Deliver the /btw answer to the chat/thread captured from the asking envelope instead of the session's creation-time router target, which froze the destination for sessions shared across chats or threads - Keep an acknowledged /btw current when SessionRouter promotes the live target to a group mid-flight; the promotion changes neither the conversation nor the delivery destination, while the named-task branch keeps the stricter target comparison Suggestions: - Move the shared-session authorization gate ahead of the usage, length, and attachment validation so non-members no longer receive validation answers - Move the bridge capability check ahead of session resolution so an unsupported /btw no longer creates and persists a session - Declare btw on ChannelAgentBridge as an optional capability instead of a module-private intersection type - Describe /btw in the /help command list and the channel docs Tests: cross-chat delivery under sessionScope single, mid-flight group promotion, foreign-session response rejection, unauthorized bare and oversized /btw, no-session-creation on unsupported bridges, and the described /help entry.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Autofix address-review summary — PR #10713 (round 1)Addressed 8 of 17 findings this round (both Criticals first, per the Fixed this round
Deferred to the next round (per-thread replies posted)R1-6 (constant mirror/equality), R1-7 (channel-side bound / cancel path), Not actionable / informational
Verification
中文说明Autofix address-review 摘要 — PR #10713(第 1 轮)本轮处理了 17 条发现中的 8 条(按每轮约 8 条的上限,优先处理两个 Critical);其余 9 条顺延到下一轮,并已在各自线程回复。单次提交: 本轮已修复
顺延到下一轮(已在各线程回复)R1-6(常量镜像/相等性)、R1-7(频道侧时限 / 取消路径)、R1-8(生命周期 stderr 日志)、R1-11(共享所有权谓词抽取)、R1-14(确认失败清理测试)、R1-15(中止信号测试)、R1-16(注册表清空断言)、R1-17(过期投递 finally 守卫测试)、R1-21(循环超时退役路径的 不可处理 / 信息性
验证
🧭 Gate advisory — this round modified areas outside the PR footprint (machine-measured, not agent-authored):
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧵 Resolved all 8 selected review thread(s). · 已关闭全部选中的 8 条评审线程。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 5": running ChannelBase.test.ts /btw tests — worktree has no node_modules and a full monorepo install was not attempted within budget; mutation verdicts are reasone….
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/channels/base/src/ChannelBase.test.ts:16143 — [probe] length-limit test pins only the reject side (4097); the 4096-char acceptance case is missingpackages/channels/base/src/ChannelBase.ts:6134 — [probe] text-only validation test never sends imageBase64; deleting the check ships greenpackages/channels/base/src/ChannelBase.ts:6126 — [probe] length limit counts UTF-16 units while docs promise characterspackages/channels/base/src/ChannelBase.ts:6065 — [probe] redundant btw-only memory-intent carve-outs
中文说明
未探索到全部深度(达到工具调用预算):"agent 5":running ChannelBase.test.ts /btw tests — worktree has no node_modules and a full monorepo install was not attempted within budget; mutation verdicts are reasone…。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
|
🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 2/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间(timeout (7200000ms))(第 2/100 次尝试)—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33567333885 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: Round summaryRound 2 of review feedback on PR #10713. Per the workflow's budget warning (a previous round exhausted its time budget), this round implemented the smallest blocking subset only — the two Critical findings — and deferred every Suggestion to later rounds with per-thread replies. Implemented
Deferred to later rounds (budget warning; per-thread replies recorded in
|
|
@qwen-code /takeover stop |
|
👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply 中文说明👋 已释放:autofix 循环不再介入此 PR(在飞的一轮如有,将完成其有界工作)。重新打上 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-7 unbounded in-flight /btw await (no channel-side timeout, no user-reachable cancel) — already reported (round-1 inline thread, comment 3906696349; escalated for a maintainer decision in round 2)
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/channels/base/src/ChannelBase.ts:6122 — [probe] D3-1 /btw question bypasses sanitizePromptText — C0/bidi control chars and line-leading [SYSTEM]:-style tags reach the forked agent verbatimdocs/users/features/channels/overview.md:492 — [review] D3-2 docs section preamble 'handled locally (no agent round-trip)' is false for the /btw entry, which always round-trips through the agent capability
Convergence: round 3 posted 3 inline comment(s), 3 of them reported for the first time; the previous round posted 12 (3 new). Findings keep coming back to the same files: packages/channels/base/src/ChannelBase.ts (findings in rounds 1, 2; 3 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查——无阻断问题。 建议见行内评论。
本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 3 轮发布了 3 条行内评论,其中 3 条是首次提出;上一轮发布了 12 条(其中 3 条首次提出)。发现反复回到同一批文件:packages/channels/base/src/ChannelBase.ts(第 1、2 轮已出过发现,本轮又有 3 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.3)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
7 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-7 unbounded in-flight /btw await, no channel-side timeout and no user-reachable cancel — already reported (comment 3906696349, re-posted at comment 3908619804; escalated for a maintainer decision in round 2)
- R1-6 CHANNEL_BTW_MAX_INPUT_LENGTH mirrors core's BTW_MAX_INPUT_LENGTH with nothing pinning equality — already reported (comment 3906696343, re-posted at comment 3908619799)
- R1-8 no stderr trace on the btw lifecycle's silent and suppression paths — already reported (comment 3906696361, re-posted at comment 3908619809)
- R1-11 isBtwCurrent is a third copy of the named-session ownership-freshness predicate — already reported (comment 3906696379, re-posted at comment 3908619815)
- D3-1 /btw question bypasses sanitizePromptText and sender attribution — already reported (round-3 review body deferred list, ChannelBase.ts:6122)
- redundant btw-only channel-memory-intent carve-outs — already reported (round-2 review body deferred list, ChannelBase.ts:6065)
- length-limit test pins only the reject side (4097); the 4096-character acceptance case is missing — already reported (round-2 review body deferred list, ChannelBase.test.ts:16143)
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/channels/base/src/ChannelBase.ts:6298 — [probe] D4-1 /btw never claims the session's static context, so operator instructions arrive one turn after the answer they governedpackages/channels/base/src/ChannelBase.ts:3607 — [probe] D4-2 two of the four new cancelBtw sites (/session close, onSessionDied) have no test that can observe thempackages/channels/base/src/ChannelBase.ts:690 — [probe] D4-3 whitespace-only answer branch and the .trim() that guards it are unpinnedpackages/channels/base/src/ChannelBase.test.ts:365 — [probe] D4-4 shared createBridge() mock hardcodes the echoed sessionId, mis-modelling the echo contract this diff documents
Convergence: round 4 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 3 (3 new). Findings keep coming back to the same files: packages/channels/base/src/ChannelBase.ts (findings in round 3; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
本轮确认的 7 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 4 轮发布了 2 条行内评论,其中 2 条是首次提出;上一轮发布了 3 条(其中 3 条首次提出)。发现反复回到同一批文件:packages/channels/base/src/ChannelBase.ts(第 3 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
No ChannelAgentBridge in this tree implements the optional btw capability yet, so the unsupported-bridge gate was taken on every shipped connection and /btw refused. At the merge base /btw was not a locally handled command: it fell through to the agent, which serves it as its own slash command and answered. Landing the interception before any producer therefore replaced a working answer with a permanent refusal. Intercept only when the bridge can answer out of band. Without the capability /btw falls through unchanged, so this PR no longer depends on the bridge half landing first. The /help de-duplication needed the same treatment: `btw` is registered locally whether or not the bridge supports it, so the filter hid the agent's entry even when that entry was the only working one. Tests: the fail-closed case is replaced by one asserting the /btw text reaches bridge.prompt, plus a case keeping the agent entry visible in /help. Also adds the session-scoped de-duplication case — the existing one runs with no active session, so it never reached getAgentCommandsForSession and a filter narrowed to the non-session branch stayed green while a live conversation listed /btw twice.
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
Reviewed at head 248c78ef.
- The last round's Critical (R4-1:
/btwfailing closed on every shipped connection was a regression over the merge base) is fixed at this head exactly as the thread records: interception now happens only behind abridge.btwcapability probe, so unsupported connections let/btwreach the agent like before, and the/helpde-dup keeps the agent's entry listed when there is no local handler — both branches pinned by new tests (also closing R4-2's dedup assertion). - The handler's own guards read sound: router-target validation before dispatch, one running BTW per session, authorization gating, abortable lifecycle with named-session presentation, and sanitized/attributed delivery back to the chat.
- All 34 threads across four review rounds are resolved; the remaining items on the bot's list are its own explicitly non-blocking Suggestions (the unbounded-await one was escalated to and accepted by the maintainers in-thread). CI: the only failure on this head is the vscode
imageSupport.bundle.test5000ms timeout — a known contention flake in a package this diff does not touch. Per the channel convention the call is on the review itself.
yiliang114
left a comment
There was a problem hiding this comment.
Reviewed the full diff (bridge capability + ChannelBase handling + ~700 lines of tests).
What I checked
- Capability gating:
btw?is optional onChannelAgentBridge; when absent,/btwfalls through to the agent as before and/helpdedup keeps the agent's entry visible — no behavior change until a bridge implements it. - Concurrency: one active side question per session, per-session isolation across sessions, slot released on ack failure / answer delivery failure / route change (identity-checked via
activeBtw.get() === request, so stale acks can't cancel a successor). - Stale-delivery suppression:
/clear, session death, named-task close, bridge replacement (setBridge→cancelAllBtw), loop-timeout eviction all abort the controller;isBtwCurrentre-validates target/thread/task before delivery and tolerates group promotion. - Security: authorization checked before session resolution and validation; bridge failure details never leak to chat (verified by tests); response
sessionIdecho mismatch is rejected. - Input validation: empty/oversized/attachment cases rejected before any session or bridge call; memory-intent classification bypassed for
/btw. - CI ubuntu failure is unrelated to this PR:
shellAstParser.test.tsCPU-budget flake,imageSupport.bundle.test.ts5s timeout flake, web-shellMessageList.dom.test.tsx— all pre-existing on main.
LGTM, approving.
|
Released in v0.23.0. |
What this PR does
This PR adds
/btw <question>handling to Channel conversations through an optional bridge capability. It validates text input and shared-session authorization, resolves the current task, immediately acknowledges the request with a correlation identifier, and delivers the side answer in the background while the main task continues. It limits each session to one active side question while allowing different sessions to run independently, and suppresses stale delivery after clear, close, reroute, session death, or bridge replacement.Why it's needed
Channel users currently cannot use the existing context-aware side-question flow safely. A
/btwmessage can reach the normal prompt path, polluting the main conversation and potentially interacting with the active turn. The Channel command should preserve the same non-interrupting intent while respecting Channel routing, authorization, and asynchronous delivery lifecycles.Reviewer Test Plan
How to verify
With a bridge that exposes BTW, start a normal Channel task and send
/btwwhile the task remains active. Confirm that an acknowledgement appears immediately, the main task is not prompted or cancelled, and the correlated answer arrives separately. Confirm that empty, oversized, attachment-bearing, or unauthorized requests are rejected before bridge invocation; a second request for the same session is rejected while one is active; different sessions can ask concurrently; and clearing, closing, rerouting, losing, or replacing the session prevents a stale answer. With a bridge that does not expose BTW, confirm that the command fails closed with an unsupported message instead of falling through to the normal prompt.Evidence (Before & After)
Before: Channel command routing has no dedicated
/btwpath, so the message can enter the normal prompt flow. After:/btwis acknowledged and delivered through the optional side-question capability without changing the main turn; unsupported connections fail closed.Tested on
Environment (optional)
Node.js 22.17.0, local workspace build without a sandbox. All 649 Channel base tests passed on this independent branch, including 16 focused
/btwscenarios; workspace build and typecheck also passed. The user-visible combined branch was built, typechecked, and bundled for live Channel validation.Risk & Scope
Linked Issues
Part of #10711.
中文说明
本 PR 做了什么
本 PR 通过可选 bridge 能力为 Channel 会话增加
/btw <question>处理。它会校验纯文本输入和共享会话授权,解析当前任务,立即用关联标识确认请求,并在主任务继续运行时于后台投递旁路答案。每个会话最多同时运行一个旁路问题,不同会话仍可独立并发;在 clear、close、reroute、会话死亡或 bridge 替换后会抑制过期结果投递。为什么需要
Channel 用户目前无法安全使用已有的上下文感知旁路提问流程。
/btw消息可能进入普通 prompt 路径,污染主会话并可能影响活跃 turn。Channel 命令需要保留“不打断主任务”的语义,同时遵守 Channel 的路由、授权和异步投递生命周期。Reviewer 测试计划
如何验证
使用暴露 BTW 能力的 bridge 启动普通 Channel 任务,并在任务仍运行时发送
/btw。确认确认消息立即出现,主任务既未收到 prompt 也未被取消,带相同关联标识的答案随后独立到达。确认空问题、超长问题、带附件问题和未授权请求都会在调用 bridge 前被拒绝;同一会话已有请求时第二个请求会被拒绝;不同会话可以并发;clear、close、reroute、会话丢失或 bridge 替换会阻止过期答案。使用未暴露 BTW 的 bridge 时,确认命令明确报告不支持,而不会落入普通 prompt。证据(Before & After)
Before:Channel 命令路由没有专用
/btw路径,消息可能进入普通 prompt。After:/btw通过可选旁路能力被确认和投递,不改变主 turn;不支持的连接会安全失败。测试环境
环境(可选)
Node.js 22.17.0,本地无 sandbox workspace build。此独立分支的 649 个 Channel base 测试全部通过,其中包含 16 个聚焦
/btw场景;workspace build 与 typecheck 也通过。用于真实 Channel 验证的组合分支已经完成 build、typecheck 和 bundle。风险与范围
关联 Issue
#10711 的一部分。