feat(dingtalk): show dynamic lifecycle tags - #10504
Conversation
Co-authored-by: Codex Using gpt-5.6-sol
github-actions
Bot
added
the
review/self-reported
- Keep lifecycle cards phase-only and localized - Coalesce stale reactions and prioritize terminal cleanup - Resolve the effective language before channel construction Co-authored-by: Codex Using gpt-5.6-sol
…-lifecycle-tags # Conflicts: # packages/channels/dingtalk/src/interaction-presenter.test.ts
- Preserve terminal and latest phase updates across rejected reaction calls - Keep source labels and current phases stable in truncated status cards - Await bounded reaction cleanup before channel shutdown Co-authored-by: Codex Using gpt-5.6-sol
Reschedule lifecycle reaction draining when a phase arrives during drain teardown. Cover the no-op drain microtask race and remove an unrelated formatting diff from the PR. Co-authored-by: Codex Using gpt-5.6-sol <noreply@openai.com>
…lar-kind-phases-delivery
…lar-kind-phases-delivery
…lar-kind-phases-delivery
…lar-kind-phases-delivery
…lar-kind-phases-delivery
|
Thanks for the PR! Template looks good ✓
Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
仅完成部分审查，审查缺口已披露。
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
— qwen3.8-max via Qwen Code /review (v0.22.3)
| let sessionKinds = this.toolCallKindsBySession.get(sessionId); | ||
| const kind = explicitKind || sessionKinds?.get(toolCallId); | ||
| if (!kind) break; |
There was a problem hiding this comment.
[Critical] R1-1: [certifies-falsely] [new-surface] Daemon-mode DingTalk channels never receive the kind-restoration this PR adds: DaemonChannelBridge.handleSessionUpdate (unchanged here) rejects every kindless tool_call_update as malformed, while the restoration map was added only to AcpBridge, which serves standalone qwen channel start. Under qwen serve the qwen agent's terminal tool frames carry no kind by construction (createTranscriptToolCallResultUpdate in transcript-replay.ts never sets one), so every tool completion is dropped and ChannelBase.dispatchToolCall never emits the completed/failed lifecycle event: after 🖥️ Running is attached at tool start, the inbound message keeps showing it for the entire model-thinking window until the first text chunk flips it to ✍️ Replying, and a failing tool never surfaces ⚠️ Retrying. Standalone mode behaves correctly, so the headline feature diverges between the two shipped deployment modes.
Witness:
[probe] identical kindful tool_call + kindless terminal update fed to both bridges: `ACP toolCall events: [{"kind":"execute","status":"in_progress"},{"kind":"execute","status":"completed"}]` (kind restored, terminal delivered) vs `DAEMON toolCall events: [{"kind":"execute","status":"in_progress"}]` + `DAEMON errors: ["Malformed daemon tool_call_update event"]` (terminal dropped).
Suggested fix: Mirror the AcpBridge restoration in DaemonChannelBridge.handleSessionUpdate: keep a per-session toolCallId → kind map seeded by kindful frames, resolve kindless updates against it, and emit the protocol error only when neither source yields a kind; clear entries on terminal status and session drop exactly as AcpBridge does.
Note: The existing guard at DaemonChannelBridge.ts:1031-1045 drops kindless in_progress frames carrying _meta.shellProgress heartbeats before any further processing — the restoration must not start re-emitting those frames.
Fix witness: A DaemonChannelBridge test mirroring AcpBridge.test.ts 'restores the initial kind on a kindless terminal tool update': feed a kindful initial tool_call then a kindless completed tool_call_update, assert a toolCall event with the restored kind and no error event — please confirm the test goes red when the restoration is removed.
中文说明
[Critical] 守护进程模式下的钉钉频道无法获得本 PR 新增的 kind 恢复逻辑：DaemonChannelBridge.handleSessionUpdate（本 PR 未改动）会将所有不带 kind 的 tool_call_update 判为格式错误并拒绝，而 kind 恢复映射只添加到了服务于独立 qwen channel start 的 AcpBridge。在 qwen serve 下，qwen agent 的终态工具帧按构造就不带 kind（transcript-replay.ts 中的 createTranscriptToolCallResultUpdate 从不设置该字段），因此每次工具完成都会被丢弃，ChannelBase.dispatchToolCall 永远不会发出 completed/failed 生命周期事件：工具开始时挂上 🖥️ Running 之后，入站消息会在整个模型思考期间一直显示该标签，直到第一个文本块把它切换为 ✍️ Replying；失败的工具也永远不会显示 ⚠️ Retrying。独立模式行为正确，因此该特性在两种部署模式之间出现了分叉。
修复建议：在 DaemonChannelBridge.handleSessionUpdate 中镜像 AcpBridge 的恢复逻辑：维护按会话的 toolCallId → kind 映射（由带 kind 的帧写入），用它解析不带 kind 的更新，仅当两个来源都无法给出 kind 时才发出协议错误；并像 AcpBridge 一样在终态状态与会话移除时清理条目。
注意：注意 DaemonChannelBridge.ts:1031-1045 处的现有守卫会在任何进一步处理之前丢弃携带 _meta.shellProgress 心跳的无 kind in_progress 帧——恢复逻辑不应重新发出这些帧。
修复验收：请为 DaemonChannelBridge 增加一个镜像 AcpBridge.test.ts 中 'restores the initial kind on a kindless terminal tool update' 的测试：先喂一个带 kind 的初始 tool_call，再喂一个不带 kind 的 completed tool_call_update，断言发出带恢复 kind 的 toolCall 事件且没有错误事件——并请确认移除恢复逻辑后该测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
|
||
| - Start with two tags: `👀` and `🤔 Thinking`. | ||
| - Keep `👀` fixed while replacing only the status tag. | ||
| - Map tool events to `📖 Reading`, `🔎 Searching`, `🖥️ Running`, `🛠️ Editing`, `🛠️ Working`, or `⚠️ Retrying`. |
There was a problem hiding this comment.
[Suggestion] R1-11: The Lifecycle summary in this design doc enumerates only 6 tool phases while the same document's ACP projection contract table defines 10 — omitting 🗑️ Deleting, 📦 Moving, 🤔 Thinking, 🌐 Fetching, and 🔄 Switching mode. Both plan docs designate this file 'the final contract', so the stale summary is not inert: a maintainer working from the Lifecycle section — writing tag-cleanup coverage, auditing which status tags can appear on an inbound message, or projecting the lifecycle to another channel — will enumerate the 6 listed phases and conclude delete/move/think/fetch/switch_mode events collapse into 🛠️ Working, while the shipped toolPresentationPhase attaches 5 distinct tags: wrong expectations about observable UI state and incomplete cleanup/test lists.
Witness:
not run — textual self-contradiction settled by direct read of both sections at HEAD; no runnable oracle exists.
Suggested fix: Make the bullet match the contract table: '- Map tool events to the phases in the projection contract table below (📖 Reading, 🔎 Searching, 🖥️ Running, 🛠️ Editing, 🗑️ Deleting, 📦 Moving, 🤔 Thinking, 🌐 Fetching, 🔄 Switching mode, 🛠️ Working, or ⚠️ Retrying).'
中文说明
[Suggestion] 该设计文档的 Lifecycle 摘要只列出了 6 个工具阶段，而同一文档的 ACP 投影契约表定义了 10 个——遗漏了 🗑️ Deleting、📦 Moving、🤔 Thinking、🌐 Fetching 与 🔄 Switching mode。两份计划文档都将该文件指定为'最终契约'，因此这份过时的摘要并非无害：从 Lifecycle 一节开展工作的维护者——编写标签清理覆盖、审计入站消息可能出现哪些状态标签、或将生命周期投影到其他频道——会枚举这 6 个阶段并得出 delete/move/think/fetch/switch_mode 事件归并为 🛠️ Working 的结论，而已交付的 toolPresentationPhase 会挂上 5 个不同的标签：对可观察 UI 状态的错误预期，以及不完整的清理/测试清单。
修复建议：使该条目与契约表一致：'- 将工具事件映射到下方投影契约表中的阶段（📖 Reading、🔎 Searching、🖥️ Running、🛠️ Editing、🗑️ Deleting、📦 Moving、🤔 Thinking、🌐 Fetching、🔄 Switching mode、🛠️ Working 或 ⚠️ Retrying）。'
— qwen3.8-max via Qwen Code /review (v0.22.3)
| @@ -0,0 +1,414 @@ | |||
| # 钉钉工具类型精细阶段实施计划 | |||
|
|
|||
| > 状态说明：后续产品评审批准了在活动卡片阶段行展示受限的工具 title，并要求最小化修复 `AcpBridge` partial heartbeat 回归。最终契约以 `docs/design/dingtalk-dynamic-lifecycle-tags.md` 为准；下文保留最初的分步计划和当时的范围判断，不能作为当前实现边界。 | |||
There was a problem hiding this comment.
[Suggestion] R1-12: Both committed plan documents' status notes claim product review approved displaying restricted tool titles in the active card's phase line, while the final contract they cite in the same sentence — docs/design/dingtalk-dynamic-lifecycle-tags.md — states the opposite ('The running card displays only the allowlisted phase label. ACP tool titles are not projected because built-in tools may derive them from commands, paths, or parameters'), and the shipped code agrees with the design doc (grep-verified: no title reaches any card). The status notes are the only parts of these committed docs that present themselves as current — everything below is explicitly disclaimed as historical. A maintainer (or agentic worker, which these plans explicitly address) scoping a follow-up from them concludes the approved scope includes tool-title display; at worst they implement it, violating the privacy contract the design doc exists to enforce. The sibling plan file carries the same claim.
Witness:
not run — contradiction between committed texts settled by direct read at the reviewed commit plus a grep of the shipped presentation path (no title reaches any card); no runnable oracle exists.
Suggested fix: Reword the status note to match the final contract: the product outcome kept the card phase line phase-only (ACP tool titles are not projected), while the scope did grow to include the minimal AcpBridge partial-update fix — which the code corroborates.
Note: The fix must not re-introduce title projection to reconcile the note — the contract forbids it: docs/design/dingtalk-dynamic-lifecycle-tags.md — 'The running card displays only the allowlisted phase label. ACP tool titles are not projected'.
中文说明
[Suggestion] 两份已提交的计划文档的状态说明都声称产品评审批准了在活动卡片阶段行展示受限的工具 title，而同一句话中引用的最终契约——docs/design/dingtalk-dynamic-lifecycle-tags.md——表述相反（'运行中的卡片只显示白名单内的阶段标签。不投影 ACP 工具 title，因为内置工具的 title 可能来自命令、路径或参数'），且已交付的代码与设计文档一致（已用 grep 验证：没有任何 title 到达卡片）。状态说明是这两份已提交文档中唯一以当前状态自居的部分——其下内容均被明确声明为历史记录。维护者（或这些计划明确面向的执行 Agent）据此规划后续工作时，会得出批准范围包含工具 title 展示的结论；最坏情况是其实现 title 投影，违反设计文档赖以存在的隐私契约。另一份孪生计划文档也有相同表述。
修复建议：将状态说明改写为与最终契约一致：产品结果使卡片阶段行仅展示阶段标签（不投影 ACP 工具 title），同时范围确实扩展为包含最小的 AcpBridge 部分更新修复——代码佐证了这一点。
注意：修复不应为了自洽而重新引入 title 投影——契约禁止它：docs/design/dingtalk-dynamic-lifecycle-tags.md——'运行中的卡片只显示白名单内的阶段标签。不投影 ACP 工具 title'。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| @@ -0,0 +1,280 @@ | |||
| # DingTalk Lifecycle Delivery Convergence Implementation Plan | |||
|
|
|||
| > Historical plan: later product review approved a bounded tool-title summary in the active card and a minimal `AcpBridge` partial-update fix. The final contract is `docs/design/dingtalk-dynamic-lifecycle-tags.md`; the phase-only steps below preserve the original implementation sequence rather than the final scope. | |||
There was a problem hiding this comment.
[Suggestion] R1-12: Both committed plan documents' status notes claim product review approved displaying restricted tool titles in the active card's phase line, while the final contract they cite in the same sentence — docs/design/dingtalk-dynamic-lifecycle-tags.md — states the opposite ('The running card displays only the allowlisted phase label. ACP tool titles are not projected because built-in tools may derive them from commands, paths, or parameters'), and the shipped code agrees with the design doc (grep-verified: no title reaches any card). The status notes are the only parts of these committed docs that present themselves as current — everything below is explicitly disclaimed as historical. A maintainer (or agentic worker, which these plans explicitly address) scoping a follow-up from them concludes the approved scope includes tool-title display; at worst they implement it, violating the privacy contract the design doc exists to enforce. The sibling plan file carries the same claim.
Witness:
not run — contradiction between committed texts settled by direct read at the reviewed commit plus a grep of the shipped presentation path (no title reaches any card); no runnable oracle exists.
Suggested fix: Reword the status note to match the final contract: the product outcome kept the card phase line phase-only (ACP tool titles are not projected), while the scope did grow to include the minimal AcpBridge partial-update fix — which the code corroborates.
Note: The fix must not re-introduce title projection to reconcile the note — the contract forbids it: docs/design/dingtalk-dynamic-lifecycle-tags.md — 'The running card displays only the allowlisted phase label. ACP tool titles are not projected'.
中文说明
[Suggestion] 两份已提交的计划文档的状态说明都声称产品评审批准了在活动卡片阶段行展示受限的工具 title，而同一句话中引用的最终契约——docs/design/dingtalk-dynamic-lifecycle-tags.md——表述相反（'运行中的卡片只显示白名单内的阶段标签。不投影 ACP 工具 title，因为内置工具的 title 可能来自命令、路径或参数'），且已交付的代码与设计文档一致（已用 grep 验证：没有任何 title 到达卡片）。状态说明是这两份已提交文档中唯一以当前状态自居的部分——其下内容均被明确声明为历史记录。维护者（或这些计划明确面向的执行 Agent）据此规划后续工作时，会得出批准范围包含工具 title 展示的结论；最坏情况是其实现 title 投影，违反设计文档赖以存在的隐私契约。另一份孪生计划文档也有相同表述。
修复建议：将状态说明改写为与最终契约一致：产品结果使卡片阶段行仅展示阶段标签（不投影 ACP 工具 title），同时范围确实扩展为包含最小的 AcpBridge 部分更新修复——代码佐证了这一点。
注意：修复不应为了自洽而重新引入 title 投影——契约禁止它：docs/design/dingtalk-dynamic-lifecycle-tags.md——'运行中的卡片只显示白名单内的阶段标签。不投影 ACP 工具 title'。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| const kind = explicitKind || sessionKinds?.get(toolCallId); | ||
| if (!kind) break; |
There was a problem hiding this comment.
[Suggestion] R1-2: Initial tool_call events carrying no kind are now silently dropped at the bridge; before this PR they were emitted with kind: '' and also fired emitResponseBoundary. The ACP schema makes kind optional and ChannelBase.dispatchToolCall explicitly documents third-party bridges omitting fields, so a third-party ACP agent that omits kind on initial tool calls loses both the toolCall lifecycle event (QQChannel.onToolCall's immediate flush at QQChannel.ts:1349) and the response boundary (QQChannel.ts:1362; FeishuAdapter.ts:1597 card accumulatedText block cutting) — the card body accumulates as one growing block. The bundled qwen CLI always sets kind, so only non-qwen agents regress.
Witness:
[probe] kindless initial tool_call → `R1-2 toolCalls: []` / `R1-2 boundaries: []`; control with kind:'other' emits the toolCall event + 1 boundary.
Suggested fix: Default a kindless initial tool_call to the design's fallback instead of dropping it, e.g. const kind = explicitKind || sessionKinds?.get(toolCallId) || (type === 'tool_call' ? 'other' : undefined); so the event and its response boundary keep flowing.
Note: Kindless tool_call_updates must remain dropped — that is the design's explicit deferred scope, pinned by this diff's own tests 'restores the initial kind on a kindless terminal tool update' and 'ignores meta-only shell progress heartbeats'.
Fix witness: A new AcpBridge.test.ts case feeding an initial tool_call with no kind and asserting the toolCall event is emitted with kind 'other' and responseBoundary fires once — removing the fallback should make it red.
中文说明
[Suggestion] 不带 kind 的初始 tool_call 事件现在会在 bridge 处被静默丢弃；在本 PR 之前它们会以 kind: '' 发出并触发 emitResponseBoundary。ACP schema 中 kind 是可选的，ChannelBase.dispatchToolCall 也明确记录了第三方 bridge 可能省略字段，因此在初始 tool call 中省略 kind 的第三方 ACP agent 会同时失去 toolCall 生命周期事件（QQChannel.ts:1349 处 QQChannel.onToolCall 的即时 flush）与响应边界（QQChannel.ts:1362；FeishuAdapter.ts:1597 的卡片 accumulatedText 分块）——卡片正文会累积为一整块不断增长的文本。内置 qwen CLI 总是设置 kind，因此只有非 qwen agent 会回归。
修复建议：不要丢弃不带 kind 的初始 tool_call，而是默认使用设计中的兜底值，例如 const kind = explicitKind || sessionKinds?.get(toolCallId) || (type === 'tool_call' ? 'other' : undefined);，让事件与响应边界继续发出。
注意：不带 kind 的 tool_call_update 必须继续被丢弃——这是设计文档明确延后的范围，且由本 diff 自带的测试 'restores the initial kind on a kindless terminal tool update' 与 'ignores meta-only shell progress heartbeats' 固定。
修复验收：请在 AcpBridge.test.ts 新增用例：喂一个不带 kind 的初始 tool_call，断言发出 kind 为 'other' 的 toolCall 事件且 responseBoundary 触发一次——移除兜底逻辑应使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| try { | ||
| channel.disconnect(); | ||
| await channel.disconnect(); | ||
| } catch { |
There was a problem hiding this comment.
[Suggestion] R1-28: The connect-failure cleanup now awaits channel.disconnect() with no deadline inside the daemon-worker startup loop, and this PR's widened contract (disconnect(): void | Promise<void>) admits a non-settling disconnect. A qwen serve worker selecting e.g. ['broken-ext', 'telegram'] where the extension channel's connect() rejects and disconnect() never settles blocks at this await: telegram never connects, reportStartup/sendReady never fire, the supervisor's 30s startup timer kills the worker, scheduleRestart() relaunches into the identical hang, and the whole channel group — including healthy channels — stays down through the restart policy. Pre-PR the fire-and-forget disconnect let the loop continue to the next channel.
Witness:
[probe] INTACT: telegram.connect rejects + never-settling disconnect → after 500ms feishuConnect NOT called, worker promise still pending / REVERT to pre-PR fire-and-forget: startup proceeds to the next channel.
Suggested fix: Bound the await inside the existing try/catch (best-effort), e.g. race Promise.resolve(channel.disconnect()) against a setTimeout deadline, mirroring WORKER_SHUTDOWN_DRAIN_MS (10s, daemon-worker.ts:107).
Note: Any deadline must stay inside the supervisor's CHANNEL_WORKER_STARTUP_TIMEOUT_MS = 30_000 (packages/acp-bridge/src/channel-control-timeouts.ts:8), otherwise the supervisor kills the worker mid-cleanup before the loop can continue.
Fix witness: A new daemon-worker.test.ts case: first channel's connect rejects and its disconnect never settles, second channel connects — assert the second still connects (advance fake timers past the deadline); removing the deadline makes the test hang/fail.
中文说明
[Suggestion] 连接失败的清理逻辑现在会在 daemon-worker 的启动循环中无截止时限地 await channel.disconnect()，而本 PR 放宽后的契约（disconnect(): void | Promise<void>）允许永不落定的 disconnect。例如选择 ['broken-ext', 'telegram'] 的 qwen serve worker，当扩展频道 connect() 拒绝且 disconnect() 永不落定时，会阻塞在该 await：telegram 永远不会连接，reportStartup/sendReady 永不触发，监管者 30 秒启动超时杀掉 worker，scheduleRestart() 重启后又陷入同样的挂起，整个频道组（包括健康频道）会在重启策略下持续不可用。本 PR 之前发后即忘的 disconnect 允许循环继续处理下一个频道。
修复建议：在现有 try/catch 内（尽力而为）为该 await 设置时限，例如让 Promise.resolve(channel.disconnect()) 与 setTimeout 截止时限竞态，参照 WORKER_SHUTDOWN_DRAIN_MS（10 秒，daemon-worker.ts:107）。
注意：任何截止时限必须小于监管者的 CHANNEL_WORKER_STARTUP_TIMEOUT_MS = 30_000（packages/acp-bridge/src/channel-control-timeouts.ts:8），否则监管者会在循环继续之前于清理中途杀掉 worker。
修复验收：请在 daemon-worker.test.ts 新增用例：第一个频道 connect 拒绝且其 disconnect 永不落定，第二个频道连接——断言第二个仍能连接（将假定时器推进超过时限）；移除时限会使测试挂起/失败。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| await cleanupStartedChannels([channel], bridge, router); | ||
| removeServiceInfo(); | ||
| process.exit(0); |
There was a problem hiding this comment.
[Suggestion] R1-18: Graceful shutdown now awaits the disconnect drain before exiting, but bridge crash recovery still checks isShuttingDown() only at the do…while tail (start.ts:276). A crash landing during shutdown can therefore restart the bridge underneath the dying service: the bridge crashes, recovery is inside its RESTART_DELAY_MS wait (3s) or bridge.start()/session restore (up to 60s) when Ctrl+C arrives, the new awaited drain keeps the event loop alive, and recovery builds a replacement AcpBridge — spawning a fresh qwen CLI subprocess — re-registers relays and restores sessions before exit(0) kills the just-spawned subprocess; a recovery error in that window races the .catch path's process.exit(1) against exit(0). Pre-PR the fully synchronous shutdown exited inside the signal tick, so this window did not exist.
Witness:
[probe] real handler driven with the suite's mocks, bridge crashed, SIGINT during a hung drain, timers advanced: `{"bridgeConstructions":2,"bridgeStarts":2,"sessionRestores":1,"exitCalled":0,"disconnectCalls":1}` — replacement bridge constructed and sessions restored after shutdown began, before exit.
Suggested fix: In createBridgeRecovery, re-check shutdown after the delay and after the bridge restart: if (isShuttingDown()) return; before re-registering relays and restoring sessions — return rather than throw so the .finally still releases bridgeReadiness.
Fix witness: A new case in start.test.ts beside the recovery suite: fire the disconnect listener to start recovery, invoke the registered SIGTERM handler while the restart delay is pending, and assert the bridge was constructed exactly once / setBridge was not called a second time — removing the re-check should make it red.
中文说明
[Suggestion] 优雅退出现在会先等待断开连接排空再退出，但 bridge 崩溃恢复仍然只在 do…while 循环尾部（start.ts:276）检查 isShuttingDown()。因此落在退出期间的崩溃仍可能在即将退出的服务之下重启 bridge：bridge 崩溃后，恢复流程正处于 RESTART_DELAY_MS 等待（3 秒）或 bridge.start()/会话恢复（最长 60 秒）期间时收到 Ctrl+C，新增的 await 排空使事件循环继续存活，恢复流程会构建替代的 AcpBridge（派生新的 qwen CLI 子进程）、重新注册转发并恢复会话，随后 exit(0) 又杀掉了刚派生的子进程；该窗口内的恢复错误还会使 .catch 分支的 process.exit(1) 与 exit(0) 竞态。本 PR 之前退出逻辑完全同步，在信号处理回合内即退出，因此该窗口原本不存在。
修复建议：在 createBridgeRecovery 中，于延迟结束之后以及 bridge 重启之后再次检查退出状态：在重新注册转发与恢复会话之前执行 if (isShuttingDown()) return;——使用 return 而非 throw，以便 .finally 仍能释放 bridgeReadiness。
修复验收：请在 start.test.ts 的恢复测试套件旁新增用例：触发 disconnect 监听器开始恢复，在重启延迟挂起期间调用已注册的 SIGTERM 处理器，断言 bridge 只被构造一次/setBridge 未被第二次调用——移除该复查应使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| await createChannel(name, config, bridge, { | ||
| router, | ||
| proxy, | ||
| ...(displayLanguage ? { displayLanguage } : {}), |
There was a problem hiding this comment.
[Suggestion] R1-19: The startAll displayLanguage wiring is pinned by no test — the suite's only displayLanguage: 'zh' assertion sits in a test that invokes startSingle (invokeStartHandler({ name: 'telegram' })). Deleting the spread here ships green (measured: start.test.ts 34 passed): default all-channels qwen channel start then silently falls back to English lifecycle tags (🤔 Thinking instead of 🤔 思考中) for zh users while single-channel mode stays localized — an invisible divergence between two textually identical spreads.
Witness:
[probe] mutant deleting the startAll spread: full start.test.ts `34 passed (34)` — ships green; the same deletion applied to startSingle's spread fails the existing test (harness discriminates).
Suggested fix: Extend a startAll-mode test (or 'starts all channels with one shared bridge and router'): set general: { language: 'zh' } in mockLoadSettings and assert mockCreateChannel receives expect.objectContaining({ displayLanguage: 'zh' }) for each configured channel.
Fix witness: The new assertion is the witness — removing this spread must turn it red (the startSingle twin at start.test.ts:414 pins only the other call site).
中文说明
[Suggestion] startAll 的 displayLanguage 接线没有任何测试固定——套件中唯一的 displayLanguage: 'zh' 断言位于调用 startSingle 的测试中（invokeStartHandler({ name: 'telegram' })）。删除此处展开仍全绿（已实测：start.test.ts 34 通过）：默认全频道的 qwen channel start 会静默地为中文用户回退到英文生命周期标签（🤔 Thinking 而非 🤔 思考中），而单频道模式仍显示本地化标签——两个文本完全相同的展开之间出现不可见的分叉。
修复建议：扩展 startAll 模式测试（或 'starts all channels with one shared bridge and router'）：在 mockLoadSettings 中设置 general: { language: 'zh' }，断言 mockCreateChannel 对每个已配置频道收到 expect.objectContaining({ displayLanguage: 'zh' })。
修复验收：新增断言即验收标准——移除此处展开必须使其变红（start.test.ts:414 的 startSingle 孪生断言只固定了另一个调用点）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| disconnections.push( | ||
| Promise.resolve(channel.disconnect()).then(() => { | ||
| writeStdoutLine(`[Channel] "${name}" disconnected.`); | ||
| }), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R1-29: startAll's awaited disconnect drain discards every rejection — .then() logs only success and the Promise.allSettled(disconnections) result is dropped — so a failing channel disconnect exits silently with code 0 and no error line. During Ctrl+C of multi-channel qwen channel start, a channel whose disconnect() rejects (DingtalkAdapter's connectionManager.stop()/client.disconnect() can throw) produces no output at all, and work lost in that teardown — e.g. lifecycle-tag recalls never sent, leaving stale DingTalk reactions — becomes undiagnosable while the process reports success.
Witness:
[probe] INTACT: startAll shutdown with disconnect rejecting 'teardown boom' → process.exit(0) called, stderr 0 lines, no error / FIXED arm (rejection handler logging to stderr): stderr received `[Channel] "telegram" disconnect failed: teardown boom`.
Suggested fix: Add a rejection handler: Promise.resolve(channel.disconnect()).then(() => { ... }, (err) => { writeStderrLine([Channel] "${name}" disconnect failed: ${err instanceof Error ? err.message : String(err)}); }) — cleanupStartedChannels' drain has the same silent allSettled shape if symmetry is wanted.
Fix witness: A startAll shutdown test driving a stub channel whose disconnect() rejects must assert the failure is reported on stderr; no startAll shutdown test exists today, so the new test goes red if the rejection handler is removed.
中文说明
[Suggestion] startAll 的断开等待排空会丢弃所有拒绝——.then() 只记录成功，Promise.allSettled(disconnections) 的结果被丢弃——因此失败的频道断开会以退出码 0 静默退出且没有任何错误行。在多频道 qwen channel start 的 Ctrl+C 期间，disconnect() 拒绝的频道（DingtalkAdapter 的 connectionManager.stop()/client.disconnect() 可能抛错）不会产生任何输出，该清理中丢失的工作（例如生命周期标签撤回从未发出、钉钉上残留过期表情）将无从诊断，而进程却报告成功。
修复建议：增加拒绝处理：Promise.resolve(channel.disconnect()).then(() => { ... }, (err) => { writeStderrLine([Channel] "${name}" disconnect failed: ${err instanceof Error ? err.message : String(err)}); })——若需要对称，cleanupStartedChannels 的排空也存在同样的静默 allSettled 形态。
修复验收：请新增一个 startAll 退出测试，驱动 disconnect() 拒绝的桩频道，并断言失败被报告到 stderr；目前不存在 startAll 退出测试，因此移除拒绝处理器时新测试应变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| await Promise.allSettled(disconnections); | ||
| process.exit(0); |
There was a problem hiding this comment.
[Suggestion] R1-22: startAll's new shutdown semantics — awaited disconnections before process.exit(0) plus the if (shuttingDown) return; reentrancy guard — are pinned by no test: the only shutdown test ('waits for asynchronous channel cleanup before standalone exit') drives startSingle via invokeStartHandler({ name: 'telegram' }), and no invokeStartHandler({}) test captures or invokes a signal handler (sweep of all 34 tests: SIGTERM capture count 1, SIGINT 0, startAll shutdown coverage 0 of 8 call sites). A future edit reverting multi-channel Ctrl+C to exit-before-disconnect ships green, cutting off the DingTalk tag recalls this PR exists to perform on every multi-channel shutdown.
Witness:
sweep over start.test.ts — 34 tests; SIGTERM capture count: 1 (in the startSingle test); SIGINT capture count: 0; startAll (invokeStartHandler({})) shutdown coverage: 0 of 8 call sites.
Suggested fix: Add a startAll variant of the shutdown test: run invokeStartHandler({}) with disconnect returning a pending promise, pull the SIGTERM listener from the process.on spy, invoke it, assert process.exit has NOT been called yet, resolve the disconnect, then assert process.exit(0); also assert a second invocation of the handler is a no-op.
Fix witness: The new test is the witness — removing the await at start.ts:605 or the guard at start.ts:586 must make it red.
中文说明
[Suggestion] startAll 的新退出语义——process.exit(0) 前等待断开连接、加上 if (shuttingDown) return; 重入守卫——没有任何测试固定：唯一的退出测试（'waits for asynchronous channel cleanup before standalone exit'）通过 invokeStartHandler({ name: 'telegram' }) 驱动 startSingle，且没有任何 invokeStartHandler({}) 测试捕获或调用信号处理器（对全部 34 个测试的扫描：SIGTERM 捕获 1 次、SIGINT 0 次、startAll 退出覆盖 8 个调用点中的 0 个）。未来将多频道 Ctrl+C 还原为退出先于断开的改动会全绿通过，从而在每次多频道退出时切断本 PR 要执行的钉钉标签撤回。
修复建议：新增 startAll 版本的退出测试：以返回挂起 promise 的 disconnect 运行 invokeStartHandler({})，从 process.on spy 取出 SIGTERM 监听器并调用，断言此时 process.exit 尚未被调用，解析 disconnect 后断言 process.exit(0)；并断言第二次调用该处理器是无操作。
修复验收：新测试即验收标准——移除 start.ts:605 的 await 或 start.ts:586 的守卫必须使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
2 participants
What this PR does
This PR adds localized lifecycle feedback to DingTalk without exposing raw tool input, output, or reasoning. While a turn is active, the originating message keeps
👀plus one replaceable status reaction such as Thinking, Reading, Searching, Running, Editing, Retrying, or Replying. A known completion, failure, or cancellation removes transient reactions and leaves one terminal result; session death or adapter disconnect only cleans up transient state and does not fabricate an outcome.The same lifecycle phase is projected into interactive status cards, so normal replies, interactive status cards, and block-streaming cards present a consistent state. Active cards show the current phase above their content and remove it when the run becomes terminal. Labels follow the resolved Qwen UI language and currently provide English and Chinese presentation.
The channel bridge now consumes both initial tool calls and subsequent tool-call updates, preserves the tool kind across updates, ignores kind-less shell progress heartbeats, and clears cached tool state on terminal or session cleanup. DingTalk reaction operations are serialized per inbound message, coalesce duplicate or superseded states, and avoid stacking contradictory statuses when a recall fails. Channel shutdown paths also await adapters that need asynchronous cleanup.
Why it's needed
The existing eye acknowledgement confirms receipt but gives no indication whether a long-running turn is thinking, reading, searching, running a command, editing, retrying, or preparing its response. Compact lifecycle feedback on the original message and active status card makes progress visible without adding intermediate chat messages or exposing chain-of-thought, and it remains consistent across DingTalk delivery modes.
Reviewer Test Plan
How to verify
Send a DingTalk prompt that exercises reading, searching, command execution, editing, and response generation. Confirm the original inbound message keeps
👀while exactly one transient status reaction changes with activity, and confirm an interactive status card shows the matching localized phase. On success, verify transient state is removed and only✅ Doneremains; repeat the terminal checks for❌ Failedand⏹️ Stopped. Session death or adapter disconnect should remove transient state without adding a terminal result. Raw tool titles, inputs, outputs, commands, and secret-like strings must never appear in reaction labels or phase headings.At the current head, the complete DingTalk package suite passes 518 tests. The focused channel-base suites pass 719 tests, including tool-call update normalization and cache cleanup, and the focused CLI channel startup/worker suites pass 135 tests, including language propagation and asynchronous disconnect handling.
Evidence (Before & After)
Before: the original message kept only
👀for the entire turn and active status cards did not identify the current lifecycle phase.After: a real DingTalk E2E run verified the success path from
👀plus🤔 Thinkingto✅ Done. Four inbound requests were accepted and their interactive status-card runs completed in the real client. This E2E run did not exhaustively exercise every mapped tool phase or the failure, cancellation, session-death, and disconnect paths; those cases are covered locally. No persistent real-client screenshot is attached to this PR yet.Tested on
Environment (optional)
Local Node.js workspace tests and a real DingTalk E2E run on macOS. Credentials and callback payloads are not included in the PR.
Risk & Scope
Linked Issues
Closes #10366
中文说明
本 PR 做了什么
本 PR 在不暴露原始工具输入、输出或推理内容的前提下，为钉钉增加本地化的生命周期反馈。任务执行期间，原始消息保留
👀，并额外显示一个可替换的状态表情，例如思考、读取、搜索、执行、编辑、重试或回复。任务明确完成、失败或取消时会移除临时表情，仅保留一个终态结果；会话死亡或适配器断开时只清理临时状态，不虚构任务结果。相同的生命周期阶段也会投影到互动状态卡片，因此普通回复、互动状态卡片和块流式卡片能够展示一致的状态。运行中的卡片会在正文上方显示当前阶段，进入终态后移除阶段文案。标签遵循 Qwen UI 最终解析出的语言，目前提供英文和中文展示。
频道桥接层现在同时消费初始工具调用及后续工具调用更新，跨更新保留工具类型，忽略缺少工具类型的 shell 进度心跳，并在工具终态或会话清理时清除缓存状态。每条入站消息的钉钉表情操作会串行执行，合并重复或已被取代的状态，并在撤回失败时避免叠加相互矛盾的状态。频道关闭流程也会等待需要异步清理的适配器。
为什么需要它
现有的眼睛确认标签只能表示消息已收到，无法告诉用户长时间运行的任务当前是在思考、读取、搜索、执行命令、编辑、重试还是准备回复。在原始消息和活动状态卡片上提供紧凑的生命周期反馈，可以在不增加中间聊天消息、也不暴露思维链的情况下展示进度，并在不同钉钉投递模式之间保持一致。
Reviewer Test Plan
如何验证
发送一条会触发读取、搜索、命令执行、编辑和回复生成的钉钉请求。确认原始入站消息始终保留
👀，同时只有一个临时状态表情随活动变化，并确认互动状态卡片展示匹配的本地化阶段。成功后应移除临时状态且只留下✅ Done；失败和取消分别只留下❌ Failed与⏹️ Stopped。会话死亡或适配器断开应移除临时状态且不添加终态结果。原始工具标题、输入、输出、命令及类似密钥的字符串均不得出现在表情标签或阶段标题中。当前提交下，完整钉钉包测试共 518 项通过；聚焦的 channel-base 测试共 719 项通过，覆盖工具调用更新归一化和缓存清理；聚焦的 CLI 频道启动及 worker 测试共 135 项通过，覆盖语言传递和异步断开处理。
证据（变更前后）
变更前：原始消息在整个任务期间只有
👀，活动状态卡片也不会显示当前生命周期阶段。变更后：真实钉钉 E2E 验证了成功主链路，从
👀加🤔 思考中进入✅ 已完成。4 次入站请求均被正常接收，对应的互动状态卡片任务也在真实客户端完成。本次 E2E 未穷举全部工具阶段，也未覆盖失败、取消、会话死亡和断开路径；这些场景由本地测试覆盖。当前 PR 尚未附上可长期保留的真实客户端截图。测试平台
环境（可选）
在 macOS 上执行本地 Node.js 工作区测试和真实钉钉 E2E。PR 中不包含凭证或回调载荷。
风险与范围
关联 Issue
Closes #10366