Skip to content

fix(dingtalk): recover status cards after network failures - #10357

Merged
wenshao merged 17 commits into
QwenLM:mainfrom
qqqys:fix/dingtalk-card-network-recovery
Sep 1, 2026
Merged

wenshao merged 17 commits into
QwenLM:mainfrom
qqqys:fix/dingtalk-card-network-recovery

Conversation

@qqqys

@qqqys qqqys commented Aug 28, 2026 •

Copy link
Copy Markdown
Collaborator

What this PR does

This change makes DingTalk status cards recover from two network-interruption modes. Transient Card OpenAPI failures retry with capped exponential backoff while retaining the latest full running snapshot and terminal intent. Separately, the existing running instance heartbeat now republishes the bounded and sanitized full content every five seconds, giving a DingTalk client that missed streaming frames while offline a later authoritative instance update without increasing the number of OpenAPI requests.

Terminal state remains ordered after any in-flight running checkpoint, card creation and terminal writes remain retryable only for transient failures, and disconnecting the channel stops pending recovery work.

Why it's needed

A brief Qwen Code host outage can leave a delivered card permanently in Running when content or terminal writes fail. A real two-client reproduction also showed a different failure: one DingTalk client disconnected and remained on an older phase after reconnecting, while another continuously connected client displayed the newer content from the same card. In that case Qwen Code's OpenAPI requests succeeded, so error-driven retries could not repair the stale client view.

Reviewer Test Plan

How to verify

  • Make a running content write fail transiently, append newer output, and restore the Card OpenAPI connection. Confirm the same card receives only the latest full snapshot.
  • Make stream finalization and the first terminal instance update fail, then restore connectivity. Confirm the card eventually shows complete final content, a terminal status, and no Stop action.
  • Reject card creation once with a retryable response, then recover. Confirm creation resumes without blocking response-boundary fallback.
  • Let the five-second full-content checkpoint fail once. Confirm the next status update retries the complete content and only a successful checkpoint advances the interval.
  • Hold a full-content checkpoint in flight while completing the task. Confirm terminal state is the final instance write and no later Running checkpoint occurs.
  • For live acceptance, keep client B online while client A disconnects during streaming. Reconnect A and keep the task running for at least five to ten seconds. Confirm A converges to the newest complete content and B continues streaming without flicker, scroll jumps, or a reset.

Evidence (Before & After)

Before: two clients viewing the same card diverged after one client disconnected; the reconnected client remained on an older phase and Running state while the continuously connected client showed later content. After (automated): fault-injection tests establish periodic authoritative running checkpoints, checkpoint retry, and terminal ordering. A real-client after capture is still required because unit tests cannot assert DingTalk desktop cache behavior.

Tested on

OS Status
🍏 macOS ✅
🪟 Windows N/A
🐧 Linux N/A

Environment (optional)

Node.js 22; repository-wide build and typecheck; targeted lint and formatting; all 434 DingTalk package tests.

Risk & Scope

  • Main risk or tradeoff: every five seconds, an existing one-second instance heartbeat carries up to 20,000 characters of content. This adds payload but no request count. Live validation must confirm that updating the same content key through the instance API does not reset or flicker an active streaming card.
  • Not validated / out of scope: detecting an individual DingTalk client's connectivity; guaranteed refresh when the client remains offline through terminal completion; recovery across a Qwen Code process restart; production Card OpenAPI fault injection.
  • Breaking changes / migration notes: none; card templates and configuration are unchanged.

Linked Issues

Fixes #10354

中文说明

本 PR 做了什么

此变更让钉钉状态卡片能够从两类网络中断中恢复。Card OpenAPI 的瞬时故障会按有上限的指数退避重试,并保留最新完整运行快照和终态意图。除此之外,现有运行态实例心跳每五秒会重新发布一次经过长度限制和路径净化的完整正文,让断网期间错过流式帧的钉钉客户端有机会通过后续权威实例更新追上内容,同时不增加 OpenAPI 请求次数。

终态会排在任何执行中的运行态 checkpoint 之后;卡片创建和终态写入只对瞬时故障重试;channel 断开后会停止待执行的恢复工作。

为什么需要

Qwen Code 主机短暂断网时,内容或终态写入失败可能让已投递卡片永久停留在 Running。真实双客户端复现还暴露了另一类故障:一个钉钉客户端断网并重连后仍停在旧阶段,而持续在线的另一个客户端已经看到同一卡片的后续内容。该场景中 Qwen Code 的 OpenAPI 请求实际成功,因此基于请求错误的重试无法修复客户端陈旧视图。

Reviewer Test Plan

如何验证

  • 让一次运行态内容写入瞬时失败,继续追加输出并恢复 Card OpenAPI 连接,确认同一卡片只接收最新完整快照。
  • 让流结束请求和第一次终态实例更新失败,再恢复网络,确认卡片最终展示完整正文、终态状态且不再显示中止操作。
  • 让卡片创建先返回一次可重试错误再恢复,确认创建继续且不阻塞响应边界 fallback。
  • 让五秒完整正文 checkpoint 失败一次,确认下一次状态更新继续携带完整正文,且只有成功才推进 checkpoint 间隔。
  • 在完整正文 checkpoint 执行中完成任务,确认终态是最后一次实例写入,之后不再出现 Running checkpoint。
  • 真实验收时让客户端 B 持续在线,客户端 A 在流式输出期间断网;A 重连后让任务继续运行至少五到十秒。确认 A 追上最新完整内容,同时 B 不出现闪烁、滚动跳变或流式状态重置。

证据(Before & After)

Before:同一卡片在两个客户端上因为其中一个客户端断网而出现内容分叉;重连客户端停在旧阶段和 Running,持续在线客户端显示后续内容。After(自动化):故障注入测试建立了周期性权威运行快照、checkpoint 失败重试和终态顺序保证。仍需真实客户端 After 证据,因为单元测试无法断言钉钉桌面端缓存行为。

测试平台

OS 状态
🍏 macOS ✅
🪟 Windows N/A
🐧 Linux N/A

环境(可选)

Node.js 22;全仓 build 和 typecheck;定向 lint 和格式检查;钉钉包 434 个测试全部通过。

风险与范围

  • 主要风险或取舍:现有每秒实例心跳每五秒会携带最多 20,000 字符正文,增加的是 payload 而不是请求数。真实验证必须确认实例接口更新同一个正文 key 不会重置或闪烁活跃流式卡片。
  • 未验证 / 不在范围内:感知单个钉钉客户端连接状态;客户端跨越整个终态过程离线时的保证刷新;Qwen Code 进程重启恢复;生产 Card OpenAPI 故障注入。
  • 破坏性变更 / 迁移说明:无;卡片模板和配置不变。

关联 Issue

Fixes #10354

@github-actions github-actions Bot added the review/self-reported The linked issue was opened by the PR author (self-reported) label Aug 28, 2026
qqqys and others added 6 commits August 28, 2026 15:27
Address round-1 review findings on QwenLM#10357:

- gettoken business errors are classified by errcode: only known
  credential/app errors (40001, 40013, 40089, 90002, 90003) are permanent;
  transient codes such as -1 (system busy) and 88 (throttled) stay retryable
  so card recovery keeps going instead of latching the card in Running.
- A retryable createAndDeliver failure now retries with the same capped
  backoff. Boundary decisions do not wait through the backoff (they fall back
  to text and the retry continues), and finalize/dispose abandon it.
- flush() no longer writes to a card whose stream latched a permanent
  failure, closing the stray-write window opened by create() returning true.
- The terminal statusLine is computed once at finalize time so retries do not
  inflate the elapsed seconds by the outage duration.
- Drop the redundant streamRetryTimer check in flush()'s re-arm; scheduleFlush
  already refuses to arm behind a retry timer.
- Tests pin the terminal retry, exponential backoff and its 30s cap, the drain
  failure fallback, the retry-timer clear, ensure() after dispose, record
  untracking, the permanent-failure boundary fallback at presenter level, the
  non-retryable delivery failure, and transient gettoken errcodes.

Claude-Session: https://claude.ai/code/session_01GNcnzoA34LZr3rMhsHEcbs
@wenshao

wenshao commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Local verification against a real card-API stack

I built a local environment and ran this PR head against the merge base to check the recovery claims end to end, rather than only through the unit suite. Summary first, then the setup, the numbers, and two defects the PR introduces that main does not have.

Verdict: the outage-recovery behaviours (scenarios 2, 3, 5, 6 below) are real fixes for things that are genuinely broken on main today, and every one of them reproduces cleanly. One new defect (F1) shows up on the happy path and has a three-line fix I verified locally — I'd take that before merging. The reconnect/checkpoint half of the PR is the only part whose payoff cannot be settled here; it hinges on a DingTalk-client assumption that only a real-client capture can confirm.

Setup

head f1ce5f317f (the test(dingtalk) commit is test-only — src/*.ts is byte-identical to b7f629a7b5)
base 4b5396c69a (merge base with main)
node v24.18.1, macOS

The Qwen Code side of the wire is the real thing: the production DingtalkInteractionPresenter, StatusCardController and DingtalkInteractiveCardClient are loaded from each worktree and issue real HTTP requests on real timers; only the API host in the URL is rewritten to a local server that implements createAndDeliver, card/streaming and card/instances. Faults are injected as destroyed sockets (connection reset) or HTTP status codes, so the PR's own DingtalkCardRequestError classification runs unmodified.

Two simulated DingTalk clients subscribe to the same card. Streaming frames reach only the clients that are online at that moment and are not replayed on reconnect; instance updates (updateCardDataByKey) reach whoever is online. That is the client model implied by #10354 — see What this cannot prove.

Harness, raw per-scenario JSON and the mutation verdicts: https://github.com/wenshao/qwen-code/tree/assets-pr10357

Scenario results (10 scenarios, both trees, identical harness)

# Scenario main @ 4b5396c PR #10357
1 client drops off at 2.0s, reconnects at 5.3s, task keeps running body frozen at the moment it went offline — 1320 chars (~9.9s of output) behind at t=11.8s, never repaired while running repaired 4.7s after reconnect, then at most one checkpoint behind (212 chars)
2 3s host outage while streaming card latches: stuck at 208/1263 chars, status line frozen at 1s, boundary falls back to a plain-text message latest snapshot back on the card 14ms after connectivity returns; boundary delivered by the card, no fallback
3 outage spanning completion card stays flowStatus=2 (Running) with a live Stop button and a stale body — permanently terminal write retried; Completed at t=5.4s with the full answer, no Stop, status line shows the completion time (2s) not the outage
4 card creation transiently rejected creation abandoned, no card at all for the whole run creation retried, card delivered and terminal — but see F2
5 stream permanently rejected (HTTP 403) 1 attempt, no retry storm; run ends stuck in Running 1 attempt, no retry storm; run reaches Completed
6 channel disconnects mid-run 11 further Card OpenAPI requests in the next 12s (heartbeat keeps running for an abandoned card) 0
7 happy path, 15s, 2005-char answer 47 requests / 44.5 KB uploaded 47 requests / 46.6 KB
8 happy path, answer at the 20k cap, 25s 77 requests / 897.0 KB 77 requests / 968.7 KB
9 700ms round trip on createAndDeliver, no faults card body monotonic body rewinds 156 → 104 chars for ~500ms — see F1
10 700ms latency on the instance heartbeat no rewind no rewind

Repo checks on the head, and again after merging current origin/main (f63369e7d4) into it: 440/440 DingTalk tests, tsc --build clean, eslint 0, prettier clean.

1 — reconnect

Reconnected client A: frozen on main, repaired on the PR

Characters rendered on each client over time

3 — outage across completion

Card stuck in Running on main, Completed on the PR

F1 — the card body rewinds after a slow card creation (new in this PR, happy path)

Write-by-write trace of the rewind

No faults at all: a 700ms round trip for POST /v1.0/card/instances/createAndDeliver — an ordinary latency for that call — with output streaming every 300ms. 4/4 reproductions on the head, 0/4 on main.

flush() captures record.pendingSnapshot before await record.ready, and ready now covers the whole creation round trip (and, with the new backoff, entire retry cycles). Meanwhile this PR changed create() to read record.content after its own await, so create() publishes 156 chars and the flush queued behind ready then writes back the 104-char snapshot it captured earlier. On main create() used a hoisted initialContent, so the first stream frame could never be newer than the queued flush and the body only grew.

In the creation-retry path the same race is larger: the card appears with 364 chars and immediately drops to 104.

Fix, verified locally (rewind gone in 4/4 runs, 440/440 tests still green):

// status-card-controller.ts, flush()
const ready = await record.ready;
if (!ready || record.terminal || record.streamFailed) return;
// `record.content` can advance while `ready` is pending — the creation round
// trip, or a creation retry backoff. Never write back a snapshot older than
// what `create()` has already published.
await this.options.client.openOrUpdateStream({
  outTrackId: record.outTrackId,
  key: 'content',
  content: sanitizeStreamingImageMarkers(record.content),
  finalize: false,
});

That all 440 tests pass either way also means the current behaviour is unpinned; the fix wants its own test.

F2 — after a transient creation failure the same text is delivered twice

Card OpenAPI black-holed from 0 to 2.5s, response boundary at 2.6s. The boundary correctly does not wait through the creation backoff, so a 364-char text fallback goes out at t=2.60s — and then the retried creation succeeds at t=3.03s and delivers a card whose body is that same 364-char text. The overlap lasts until the next segment replaces the body — t=3.03s to t=6.01s here. On main the creation is abandoned, so the user gets exactly one delivery.

This is the documented trade-off from the round-1 fix ("boundary decisions do not wait through the backoff"), but double delivery is a user-visible consequence that is neither in the PR description nor covered by a test. Cheapest containment: abandon the pending creation once a boundary has already fallen back to text.

Cost of the 5s checkpoint

Request count is unchanged in every scenario — the claim holds. Upload grows: +4.6% on a 2 k-char answer, +8% (+71.7 KB over 25s) once the answer sits at the 20k cap, where each checkpoint carries ~20.2 KB, i.e. ~4 KB/s sustained while a long answer streams.

Worth noting what the new lastContentSyncSecond test is protecting: with that reset deleted, the same 25s run goes from 5 checkpoints to 21 (one per second) and upload rises to 1271 KB. Good that f1ce5f31 pinned it.

Test discriminating power

Ten single-hunk mutants on the head, DingTalk suite as the oracle: 9/10 caught. The checkpoint block, the 5s interval, the retryable-status classifier, the stream/terminal/creation retries, the 30s cap and the dispose() call in disconnect() are all pinned. The one survivor is record.streamRetryAttempt = 0 on the success path — the same one the review bot still has open. Consequence if it is ever dropped: after a burst of transient failures the stream backoff never decays, so a later isolated failure waits up to 30s instead of 1s.

One more note, not a finding: terminal and stream retries are capped in interval (30s) but not in duration. A card whose terminal write never succeeds keeps a timer and a record alive for the lifetime of the process; only dispose() clears it. Fine for channel teardown, but a ceiling would be cheap.

What this cannot prove

The reconnect result depends entirely on how a real DingTalk client behaves after it comes back:

  • m2 — the streaming subscription is not restored and only instance updates reach the client. This is what fix(dingtalk): recover status cards after transient network failures #10354's two-client divergence implies, and it is what the table above uses: main never repairs, the PR repairs in ≤5s.
  • m1 — the client re-attaches and the next isFull frame repairs it by itself. I ran this too: main already repairs in 235ms, and the checkpoint buys nothing.

So scenario 1 — and the payload it costs — is only worth it if the real client behaves like m2. The real-client "After" capture the PR itself lists as still required is exactly what settles this; nothing local can. Equally unverifiable here: whether writing content through the instance API resets or flickers an actively streaming card in the real DingTalk UI, which is this PR's own stated main risk.

Merge view

  • Scenarios 2, 3, 5 and 6 are unambiguous fixes for behaviour that is broken on main today, and none of them depend on the client model. Those alone justify the change.
  • F1 is small, reproducible on the happy path, and has a verified fix — I'd want it in before merging.
  • F2, the retry-duration ceiling and the surviving streamRetryAttempt mutant are non-blocking; F2 at least deserves a line in the PR description if it stays as designed.
  • Scenario 1 stays "pending real-client evidence", as the PR already says.
中文说明

在本地真实卡片 API 栈上的验证

我在本地搭了一套环境,把 PR head 与 merge base 对跑,端到端验证恢复行为,而不只依赖单测。

结论: 断网恢复类行为(下表场景 2、3、5、6)确实修复了 main 上今天真实存在的问题,且逐条可稳定复现。PR 新引入了一个发生在正常路径上的缺陷(F1),我已在本地验证了三行修复,建议合并前先带上。重连/checkpoint 这一半是唯一无法在本地定论的部分,它取决于一个只有真实客户端截图才能确认的钉钉客户端行为假设。

环境

head f1ce5f317f(test(dingtalk) 那个 commit 只改测试,src/*.ts 与 b7f629a7b5 逐字节相同)
base 4b5396c69a(与 main 的 merge base)
node v24.18.1,macOS

Qwen Code 这一侧是真的:生产代码里的 DingtalkInteractionPresenter、StatusCardController、DingtalkInteractiveCardClient 从两个 worktree 分别加载,用真实定时器发真实 HTTP 请求;只把 URL 里的 API 域名改写到本地服务(实现了 createAndDeliver、card/streaming、card/instances)。故障注入在 socket 层(连接被重置)或 HTTP 状态码层,因此 PR 自己的 DingtalkCardRequestError 分类逻辑原样运行。

两个模拟钉钉客户端订阅同一张卡片。流式帧只送达当时在线的客户端,重连后不补发;实例更新(updateCardDataByKey)送达当时在线的客户端。这正是 #10354 所暗含的客户端模型——见「本地无法证明的部分」。

工具、逐场景原始 JSON 与变异测试结论:https://github.com/wenshao/qwen-code/tree/assets-pr10357

场景结果(10 个场景,两棵树,同一套工具)

# 场景 main @ 4b5396c PR #10357
1 客户端 2.0s 断网、5.3s 重连,任务继续运行 正文冻结在断网那一刻——t=11.8s 时落后 1320 字符(约 9.9s 的输出),运行期间从未追上 重连后 4.7s 追上,之后最多落后一个 checkpoint(212 字符)
2 流式输出中主机断网 3s 卡片锁死:停在 208/1263 字符,状态行冻结在 1s,响应边界退化成纯文本消息 网络恢复后 14ms 最新快照回到卡片;边界由卡片投递,无 fallback
3 断网跨越任务完成 卡片永久停在 flowStatus=2(Running),中止按钮仍在,正文陈旧 终态写入重试;t=5.4s 变为 Completed,正文完整、无中止按钮,状态行显示完成时刻(2s)而不是断网时长
4 卡片创建被瞬时拒绝 创建被放弃,整轮没有卡片 创建重试成功,卡片投递并进入终态——但见 F2
5 流被永久拒绝(HTTP 403) 只试 1 次,无重试风暴;本轮最终停在 Running 只试 1 次,无重试风暴;本轮到达 Completed
6 运行中 channel 断开 之后 12s 内仍发出 11 次 Card OpenAPI 请求(心跳还在为已废弃卡片跑) 0
7 正常路径,15s,2005 字符答案 47 次请求 / 上行 44.5 KB 47 次请求 / 46.6 KB
8 正常路径,答案打满 20k 上限,25s 77 次请求 / 897.0 KB 77 次请求 / 968.7 KB
9 createAndDeliver 700ms 往返,无任何故障 卡片正文单调增长 正文回退 156 → 104 字符,持续约 500ms——见 F1
10 实例心跳接口 700ms 延迟 无回退 无回退

head 上以及把当前 origin/main(f63369e7d4)合入后的仓库检查:440/440 钉钉测试通过,tsc --build 干净,eslint 0,prettier 干净。

F1 —— 卡片创建较慢时正文会回退(PR 新引入,正常路径)

完全没有故障注入:POST /v1.0/card/instances/createAndDeliver 往返 700ms(这对该接口是很普通的时延),输出每 300ms 一段。head 上 4/4 复现,main 上 0/4。

flush() 在 await record.ready 之前就取走了 record.pendingSnapshot,而 ready 现在覆盖整个创建往返(加上新的退避,还可能覆盖整轮重试)。同时本 PR 把 create() 改成在自己的 await 之后才读 record.content,于是 create() 发布了 156 字符,排在 ready 后面的 flush 又把此前抓取的 104 字符快照写了回去。main 上 create() 用的是提前取好的 initialContent,第一帧不可能比排队的 flush 更新,因此正文只会增长。

在创建重试路径上这个竞态更明显:卡片出现时是 364 字符,随即掉到 104。

已验证的修复(4/4 不再回退,440/440 测试仍通过):在 flush() 里 await record.ready 之后改用 sanitizeStreamingImageMarkers(record.content) 作为写入内容,而不是之前抓取的 content。

改与不改测试都全绿,也说明当前行为无人钉住;这个修复需要配一条自己的测试。

F2 —— 创建瞬时失败后同一段文本被投递两次

Card OpenAPI 从 0 到 2.5s 黑洞,2.6s 出现响应边界。边界确实没有等完创建退避,于是 t=2.60s 发出了 364 字符的文本兜底——随后重试的创建在 t=3.03s 成功,投递出的卡片正文就是同一段 364 字符文本。重叠一直持续到下一个 segment 覆盖正文——本例是 t=3.03s 到 t=6.01s。main 上创建被放弃,用户只会收到一份。

这是第一轮修复中写明的取舍(「边界决策不等待退避」),但「重复投递」是用户可见的后果,PR 描述里没有写,也没有测试覆盖。最省事的收口方式:边界一旦退化成文本,就放弃仍在等待的创建。

5s checkpoint 的成本

所有场景请求次数都没有增加——这个结论成立。上行流量增加:2k 字符答案 +4.6%;答案打满 20k 上限时 +8%(25s 内 +71.7 KB),此时每次 checkpoint 约 20.2 KB,即长答案流式期间持续约 4 KB/s。

顺带说明新增的 lastContentSyncSecond 测试在保护什么:删掉那行 reset 后,同一个 25s 场景的 checkpoint 从 5 次变成 21 次(每秒一次),上行涨到 1271 KB。f1ce5f31 把它钉住是对的。

测试的甄别力

在 head 上做了 10 个单 hunk 变异,以钉钉测试套件为判据:捕获 9/10。checkpoint 代码块、5s 间隔、可重试状态码分类、流/终态/创建三处重试、30s 上限、disconnect() 里的 dispose() 调用都被钉住。唯一存活的是成功路径上的 record.streamRetryAttempt = 0——正是评审机器人仍挂着的那一条。一旦丢失:一连串瞬时失败之后退避不再回落,后续一次孤立失败要等最多 30s 而不是 1s。

另有一条不算缺陷的说明:终态与流式重试只限制了间隔(30s),没有限制总时长。终态写入始终失败的卡片会让定时器和 record 活到进程结束,只有 dispose() 能清掉。对 channel 拆除来说够用,但加个上限成本很低。

本地无法证明的部分

重连这一条完全取决于真实钉钉客户端重连后的行为:

  • m2 —— 流订阅未恢复,只有实例更新能到达客户端。这是 fix(dingtalk): recover status cards after transient network failures #10354 双客户端分叉所暗含的模型,也是上表所用:main 永远追不上,PR 在 ≤5s 内追上。
  • m1 —— 客户端重新挂上流,下一帧 isFull 自行修复。我也跑了:main 本身 235ms 就恢复,checkpoint 毫无收益。

所以场景 1 及其付出的流量,只有在真实客户端表现为 m2 时才值得。PR 自己列为「仍需补充」的真实客户端 After 证据,正是能定论这一点的东西,本地任何手段都替代不了。同样无法在本地验证的还有:通过实例接口写 content 会不会重置或闪烁真实钉钉 UI 上正在流式输出的卡片——这也是本 PR 自述的主要风险。

合并意见

  • 场景 2、3、5、6 修的是 main 今天确实坏掉的行为,且都不依赖客户端模型,仅凭这几条本 PR 就值得合。
  • F1 影响正常路径、可稳定复现、修复已验证,建议合并前先带上。
  • F2、重试总时长上限、仍存活的 streamRetryAttempt 变异都不阻断;如果 F2 保持现在的设计,至少值得在 PR 描述里写一句。
  • 场景 1 维持「等待真实客户端证据」的状态,与 PR 现有表述一致。

@qqqys

qqqys commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator Author

已修复 F1(a2211707e2):延迟创建期间仍会清空 pendingSnapshot 以保持队列语义,但 ready 后发送最新 record.content,不再把旧快照写回。新增慢创建回归用例在修复前稳定得到 latest → queued,修复后为 latest → latest;DingTalk 包 12 files / 441 tests、根级 build、typecheck、ESLint、Prettier 均通过。F2 按评审明确的 non-blocking 结论本轮不扩展处理。

@wenshao

wenshao commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Round 2 — re-verified on a real card-API stack at 90eb0a9a61

Following up on my round-1 verification: three commits landed since (a2211707e2, bc35af3fc4, 90eb0a9a61), so I rebuilt the environment and ran the whole matrix again against the new head.

Verdict: F1 is fixed, and it is now pinned by a test — which was my one blocking item. None of the three new commits regresses anything across 11 scenarios, and bc35af3fc4 is load-bearing (measured, not assumed). Two residual items remain, both non-blocking: F2 is unchanged as the author declared, and one streaming write per card is still redundant on the slow-creation path — I have a verified optional patch for it. The reconnect half of the PR still hinges on real-client evidence that nothing local can supply.

Setup

head 90eb0a9a61
base 4b5396c69a (merge base with main, unchanged since round 1)
trees dedicated worktrees pinned at those SHAs, git rev-parse + clean git status asserted before and after every measurement
node v24.18.1, macOS

Same harness as round 1: the production DingtalkInteractionPresenter, StatusCardController and DingtalkInteractiveCardClient are loaded from each tree and issue real HTTP requests on real timers; only the API host is rewritten to a local server implementing createAndDeliver, card/streaming and card/instances. Faults are destroyed sockets or HTTP status codes, so the PR's own DingtalkCardRequestError classification runs unmodified. Two simulated DingTalk clients subscribe to the same card.

Two additions this round: the harness now records every non-finalizing PUT /v1.0/card/streaming with its payload, flagging any write byte-identical to its predecessor (a call that changes nothing on the card); and a new scenario, boundary-slow-create, exercising the exact path bc35af3fc4 targets.

Harness, raw per-scenario JSON, mutation verdicts and the candidate patch: https://github.com/wenshao/qwen-code/tree/assets-pr10357/round2

1 — F1 is fixed

Round 1 rewound 156 to 104 chars; round 2 is monotonic

a2211707e2 is exactly the fix: flush() now reads record.content at send time instead of a snapshot captured before await record.ready. 4/4 clean reproductions of the scenario that failed 4/4 in round 1.

It is also no longer unpinned. I reverted just that hunk on the head (keeping everything else) and the suite caught it: does not rewind content after delayed card creation fails. That closes round 1's "all 440 tests pass either way, so the fix wants its own test".

2 — No regression across the matrix

# Scenario main @ 4b5396c6 PR head @ 90eb0a9a
1 client offline 2.0–5.3 s, task keeps running frozen at 208 chars, 1320 behind at t=11.8 s, never repaired repaired 4.7 s after reconnect, then ≤ 212 chars behind
2 3 s host outage while streaming stuck at 208/1263 chars, status frozen at 1s, boundary falls back to text latest snapshot back 20 ms after connectivity returns, boundary delivered by the card
3 outage spanning completion flowStatus=2 with a live Stop and a stale body — permanently Completed at t=5.4 s, full answer, no Stop, status line 2s
4 card creation transiently rejected no card at all for the whole run creation retried, card delivered and terminal — see F2
5 stream permanently rejected (403) 1 attempt, no retry storm; run stuck in Running 1 attempt, no retry storm; run reaches Completed
6 channel disconnects mid-run 11 further Card OpenAPI requests in the next 12 s 0
7 happy path, 2005-char answer 47 req / 44.5 KB 47 req / 46.6 KB
8 answer at the 20k cap, 25 s 77 req / 897.0 KB 77 req / 968.7 KB
9 700 ms createAndDeliver, no faults body monotonic body monotonic (round 1: rewound 156→104)
10 700 ms latency on the instance heartbeat no rewind no rewind
11 boundary right after a slow creation (new) 3 streaming writes, 0 redundant 2 streaming writes, 1 redundant

Reconnected client A: frozen on main, repaired on the PR

Characters rendered on each client over time

Card stuck in Running on main, Completed on the PR

Repo checks on the head: 443/443 DingTalk tests, tsc --build clean, eslint 0, prettier clean. Merges cleanly with origin/main (168a88c02e).

3 — bc35af3fc4 is load-bearing, and I can show by how much

This was the commit I most wanted to measure, since it rewrites the write-tracking state (pendingSnapshot → contentVersion + hasPendingWrite) for a benefit the PR does not quantify. Running the same scenario on four trees:

Streaming writes at a boundary after a slow creation, four trees

The chain is: a2211707e2 fixes the rewind correctly but, because both create() and flush() now read the freshest content, it makes two writes redundant instead of one being stale. bc35af3fc4 removes one of them via the version guard. So the commit earns its place — but note it is the only path where it shows: on the other 10 scenarios the head is byte-identical to a2211707e2-only in request count, upload bytes and streaming-write ledger.

4 — Residual: one streaming write per card is still redundant (non-blocking)

create() ends by publishing sanitizeStreamingImageMarkers(record.content), and the flush() queued behind record.ready then writes the very same bytes again. It costs exactly one extra PUT /v1.0/card/streaming per card, whenever output arrives while the creation round trip is in flight — the ordinary case for a fast-streaming answer.

This is not a cost regression: on scenario 11 the head makes 5 requests where main makes 6, and on scenario 9 both make 26. It is simply a call that changes nothing.

If you want it gone, this patch does it — track the version create() published and let flush() skip an unchanged one. I ran it through all 11 scenarios (redundancy 0, everything else unchanged) and the suite. One caveat that makes this your call rather than an obvious win: the current duplicate is asserted by does not rewind content after delayed card creation, which expects ['latest', 'latest']; with the patch it is ['latest']. So adopting it means deciding that the second write was never wanted.

5 — Test discriminating power: 13/15

Fifteen single-semantic mutants on the head, DingTalk suite as the oracle. The checkpoint block, the 5 s interval, the lastContentSyncSecond advance, the retryable-status classifier, the stream/terminal/creation retries, the 30 s cap, dispose() on disconnect, the contentVersion bump, capturing sentVersion before the write, the "already wrote the latest" clear, and the F1 fix itself are all pinned. Two survivors:

  • record.streamRetryAttempt = 0 on the success path — the same survivor as round 1, and the one the review bot still has open. Consequence if it is ever dropped: after a burst of transient failures the stream backoff never decays, so a later isolated failure waits up to 30 s instead of 1 s.
  • record.contentVersion !== sentVersion in the re-arm guard — this one is an equivalent mutant, not a coverage gap. I dropped the guard and re-ran nine scenarios on the real stack: request count, upload bytes and the streaming-write ledger are identical in all nine. It cannot differ, because either the first if has already cleared hasPendingWrite, or handleStreamFailure has already armed streamRetryTimer and scheduleFlush short-circuits on it. Harmless; if (record.hasPendingWrite) this.scheduleFlush(record) would read the same and be one condition shorter.

6 — F2 is unchanged, as declared

Card OpenAPI black-holed 0–2.5 s, boundary at 2.6 s: a 364-char text fallback goes out, then the retried creation delivers a card whose body is that same text — visible from t=3.0 s to t=6.0 s. Still 1 fallback + a duplicate card body on the head, 1 delivery on main. The author called this non-blocking for this round and I agree it does not block; it would be worth one line in the PR description under Risk & Scope, since it is user-visible and untested.

7 — Cost of the 5 s checkpoint

Unchanged from round 1 and confirmed on the new head: request count is identical in every scenario — the PR's central cost claim holds. Upload grows +4.6 % on a 2 k-char answer and +8 % (+71.7 KB over 25 s) once the answer sits at the 20k cap, where each checkpoint carries ~20.2 KB.

What this still cannot prove

Unchanged from round 1, and it remains the deciding question for half the PR:

  • m2 — the streaming subscription is not restored after a reconnect and only instance updates reach the client. This is what fix(dingtalk): recover status cards after transient network failures #10354's two-client divergence implies, and what the table above uses: main never repairs, the PR repairs in ≤ 5 s.
  • m1 — the client re-attaches and the next isFull frame repairs it by itself. I ran this too on both trees: main already repairs in 241 ms, the PR in 235 ms, and the checkpoint buys nothing.

Scenario 1 — and the payload it costs — is only worth it if the real client behaves like m2. Equally unverifiable here: whether writing content through the instance API resets or flickers an actively streaming card in the real DingTalk UI, which is this PR's own stated main risk.

Merge view

  • Scenarios 2, 3, 5 and 6 are unambiguous fixes for behaviour broken on main today, independent of the client model. They alone justify the change, and they all still hold at the new head.
  • F1 is resolved and pinned. My round-1 blocker is cleared.
  • Everything remaining is non-blocking: F2 (declared), the residual duplicate write (patch available, your call), the surviving streamRetryAttempt mutant, and the unbounded retry duration I noted last time (interval is capped at 30 s, total duration is not; only dispose() ever clears it).
  • The reconnect/checkpoint half stays "pending real-client evidence", as the PR already says.
中文说明

第二轮 —— 在真实卡片 API 栈上对 90eb0a9a61 复验

接续第一轮验证:此后新增了三个 commit(a2211707e2、bc35af3fc4、90eb0a9a61),我重建了环境,对新 head 重跑了整个矩阵。

结论: F1 已修复,并且现在有测试钉住了它 —— 这是我上一轮唯一的阻塞项。三个新 commit 在 11 个场景中均未引入回归,且 bc35af3fc4 确实承重(是实测出来的,不是假定的)。剩余两项均不阻塞:F2 与作者声明的一致保持原样;慢速创建路径上仍有一次冗余的流式写入,我为此准备了一个已验证的可选补丁。PR 中重连那一半仍然取决于本地无法提供的真实客户端证据。

环境

head 90eb0a9a61
base 4b5396c69a(与 main 的 merge base,自第一轮未变)
工作树 专用 worktree 钉死在上述 SHA,每次测量前后都断言 git rev-parse 与 git status 干净
node v24.18.1,macOS

harness 与第一轮相同:生产代码的 DingtalkInteractionPresenter、StatusCardController 和 DingtalkInteractiveCardClient 从各自的树加载,按真实定时器发出真实 HTTP 请求;只把 URL 中的 API host 改写到本地服务,该服务实现了 createAndDeliver、card/streaming 和 card/instances。故障以销毁 socket 或 HTTP 状态码注入,因此 PR 自己的 DingtalkCardRequestError 分类原样运行。两个模拟钉钉客户端订阅同一张卡片。

本轮新增两点:harness 现在记录每一次非 finalize 的 PUT /v1.0/card/streaming 及其 payload,并标记与前一次逐字节相同的写入(即对卡片毫无改变的调用);新增场景 boundary-slow-create,专门覆盖 bc35af3fc4 所针对的那条路径。

harness、逐场景原始 JSON、变异测试判定和候选补丁:https://github.com/wenshao/qwen-code/tree/assets-pr10357/round2

1 —— F1 已修复

a2211707e2 正是所需的修复:flush() 现在在发送时刻读取 record.content,而不是在 await record.ready 之前捕获的快照。第一轮 4/4 复现失败的场景,本轮 4/4 干净通过。

它也不再是"未钉住"的了。我在 head 上只回退这一处 hunk(其余保持不变),测试套件抓住了它:does not rewind content after delayed card creation 失败。这就解决了第一轮提出的"440 个测试改与不改都全绿,所以这个修复需要自己的测试"。

2 —— 矩阵无回归

# 场景 main @ 4b5396c6 PR head @ 90eb0a9a
1 客户端 2.0–5.3 s 离线,任务持续运行 冻结在 208 字符,t=11.8 s 时落后 1320 字符,运行期间永不修复 重连后 4.7 s 追上,之后最多落后 212 字符
2 流式输出期间主机断网 3 s 卡死在 208/1263 字符,状态行冻结在 1s,响应边界退化为纯文本 网络恢复后 20 ms 内最新快照回到卡片,边界由卡片投递
3 断网跨越任务完成 永久停留 flowStatus=2,Stop 按钮仍在,正文陈旧 t=5.4 s Completed,完整正文,无 Stop,状态行 2s
4 卡片创建被瞬时拒绝 整轮完全没有卡片 创建重试,卡片投递并终态 —— 见 F2
5 流被永久拒绝(403) 1 次尝试,无重试风暴;运行卡在 Running 1 次尝试,无重试风暴;运行到达 Completed
6 运行中 channel 断开 之后 12 s 内仍有 11 次 Card OpenAPI 请求 0
7 正常路径,2005 字符答案 47 请求 / 44.5 KB 47 请求 / 46.6 KB
8 答案达到 20k 上限,25 s 77 请求 / 897.0 KB 77 请求 / 968.7 KB
9 createAndDeliver 700 ms,无故障 正文单调 正文单调 (第一轮:回退 156→104)
10 实例心跳 700 ms 延迟 无回退 无回退
11 慢速创建后紧跟响应边界 (新增) 3 次流式写入,0 次冗余 2 次流式写入,1 次冗余

head 上的仓库检查:443/443 钉钉测试通过,tsc --build 干净,eslint 0,prettier 干净。与 origin/main(168a88c02e)可干净合并。

3 —— bc35af3fc4 确实承重,并且能量化

这是我最想实测的一个 commit,因为它重写了写入跟踪状态(pendingSnapshot → contentVersion + hasPendingWrite),而 PR 并未量化其收益。在四棵树上跑同一场景:a2211707e2 正确修复了回退,但由于 create() 和 flush() 现在都读取最新内容,它把"一次陈旧写入"变成了"两次冗余写入";bc35af3fc4 通过版本号守卫消掉其中一次。所以这个 commit 站得住 —— 但要注意这是它唯一显现作用的路径:在其余 10 个场景中,head 与仅含 a2211707e2 的树在请求数、上行字节和流式写入流水账上逐项相同。

4 —— 残留:每张卡片仍有一次冗余流式写入(不阻塞)

create() 结尾会发布 sanitizeStreamingImageMarkers(record.content),而排在 record.ready 之后的 flush() 随即又写入完全相同的字节。只要创建往返期间有输出到达(对流式较快的答案属于常态),每张卡片就正好多出 一次 PUT /v1.0/card/streaming。

这不是成本回归:场景 11 中 head 发出 5 个请求而 main 发出 6 个,场景 9 中两者都是 26 个。它只是一次什么都没改变的调用。

如果想消掉它,这个补丁可以做到 —— 记录 create() 已发布的版本号,让 flush() 跳过未变化的那次。我用它跑完了全部 11 个场景(冗余归零,其余不变)以及测试套件。有一个注意点使这成为你的决定而非显然的改进:当前这次重复写入被 does not rewind content after delayed card creation 断言为期望值 ['latest', 'latest'];打上补丁后是 ['latest']。所以采纳它意味着确认第二次写入本就不该存在。

5 —— 测试辨别力:13/15

在 head 上做了 15 个单语义变异体,以钉钉测试套件为判据。checkpoint 代码块、5 s 间隔、lastContentSyncSecond 推进、可重试状态分类器、流/终态/创建三类重试、30 s 上限、断开时的 dispose()、contentVersion 自增、在写入前捕获 sentVersion、"已写入最新"清除,以及 F1 修复本身,全部被钉住。两个存活体:

  • 成功路径上的 record.streamRetryAttempt = 0 —— 与第一轮相同的存活体,也是评审 bot 仍未关闭的那条。若被删除的后果:一串瞬时失败之后流式退避永不衰减,后续一次孤立失败要等最多 30 s 而不是 1 s。
  • re-arm 守卫中的 record.contentVersion !== sentVersion —— 这个是等价变异体,而非测试盲区。我去掉守卫后在真实栈上重跑了九个场景:请求数、上行字节和流式写入流水账在九个场景中完全一致。它不可能产生差异,因为要么前一个 if 已经把 hasPendingWrite 清掉了,要么 handleStreamFailure 已经armed 了 streamRetryTimer、scheduleFlush 会因此短路返回。无害;写成 if (record.hasPendingWrite) this.scheduleFlush(record) 语义相同且少一个条件。

6 —— F2 与声明一致,保持原样

Card OpenAPI 在 0–2.5 s 黑洞,边界在 2.6 s:先发出 364 字符的纯文本兜底,随后重试成功的创建投递了一张正文就是同一段文本的卡片 —— 从 t=3.0 s 持续到 t=6.0 s 可见。head 上仍是 1 次兜底 + 一份重复的卡片正文,main 上则只有一次投递。作者本轮判定为不阻塞,我同意它不阻塞;但值得在 PR 描述的 Risk & Scope 里补一行,因为它是用户可见且无测试覆盖的。

7 —— 5 s checkpoint 的成本

与第一轮一致,并在新 head 上确认:每个场景的请求数都完全相同 —— PR 的核心成本主张成立。上行字节增长:2k 字符答案 +4.6%;答案达到 20k 上限时 +8%(25 s 内 +71.7 KB),此时每次 checkpoint 携带约 20.2 KB。

本地仍无法证明的部分

与第一轮相同,并且它仍是决定 PR 另一半价值的关键问题:

  • m2 —— 重连后流式订阅不恢复,只有实例更新能到达客户端。这是 fix(dingtalk): recover status cards after transient network failures #10354 双客户端分叉所暗含的模型,也是上表所采用的:main 永不修复,PR 在 ≤5 s 内修复。
  • m1 —— 客户端重新挂上,下一个 isFull 帧自行修复它。我在两棵树上也跑了这个模型:main 已经在 241 ms 内修复,PR 为 235 ms,checkpoint 毫无收益。

场景 1 及其付出的 payload,只有在真实客户端表现为 m2 时才值得。同样无法在此验证的是:通过实例接口写入 content 是否会重置或闪烁一张正在流式输出的真实钉钉卡片 —— 那正是本 PR 自述的主要风险。

合并意见

  • 场景 2、3、5、6 是对 main 上今天确实损坏的行为的明确修复,且不依赖客户端模型假设。仅凭这几条就足以支撑这次变更,并且它们在新 head 上依然成立。
  • F1 已解决并被钉住。 我第一轮的阻塞项已清除。
  • 其余全部不阻塞:F2(已声明)、残留的冗余写入(补丁已备,由你决定)、存活的 streamRetryAttempt 变异体,以及我上轮提到的重试总时长无上限(间隔封顶 30 s,总时长没有;只有 dispose() 会清理)。
  • 重连 / checkpoint 那一半维持"待真实客户端证据",正如 PR 自己所写。

wenshao
wenshao previously approved these changes Aug 30, 2026
@wenshao

wenshao commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@wenshao

wenshao commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Round 2 addendum — the same head, verified through the full product stack

My round-2 report drove the channel classes in-process. This addendum re-runs the same head through the shipped product, because the in-process rig cannot exercise the adapter wiring, the Stream SDK handshake, the channel's own gettoken, or disconnect() in a real process.

Verdict: every conclusion in the round-2 comment holds when the whole product is in the loop, and three things it could not reach are now measured: b7f629a7b5's gettoken classification works end to end, the terminal-retry ceiling is one attempt every 30 s forever until dispose(), and F2 does not fire in an ordinary single-segment run — it needs a response boundary inside the creation backoff. My merge view is unchanged: take it.

Setup

head 90eb0a9a61, merged into origin/main → tree actually exercised is 1d5a1383e5
base 168a88c02e (origin/main), not the merge base — so this also re-checks the PR against current main
artifact under test dist/cli.js from npm ci && npm run build && npm run bundle in each tree
host / container macOS, Node v24.18.1 / node:22-bookworm

The rig runs qwen channel start dt inside a container started with --network none and --add-host api.dingtalk.com:127.0.0.1 --add-host oapi.dingtalk.com:127.0.0.1; a self-signed cert for both names is trusted through NODE_EXTRA_CA_CERTS. Everything the channel talks to is local: HTTPS 443 serves oapi…/gettoken, api…/v1.0/gateway/connections/open and the three Card OpenAPI endpoints; a WS gateway on 8899 accepts the SDK connection and pushes a real /v1.0/im/bot/messages/get CALLBACK frame; sessionWebhook points at a local port so plain-text fallbacks are captured; an OpenAI-compatible endpoint streams a 2604-character answer over ~17 s. Nothing under packages/ is patched or mocked — the SDK handshake, DingtalkChannel.getProactiveToken, the presenter, the status-card controller and the card client all run as shipped, over real TLS, on real timers. Faults are destroyed sockets, so the PR's own DingtalkCardRequestError classification runs unmodified.

Two simulated DingTalk clients subscribe to the card, with the same m2 assumption as before (a client that was offline is not re-sent the frames it missed). That assumption is still the one thing nothing local can settle — see the round-2 comment.

Rig, raw per-scenario JSON, mutation log: https://github.com/wenshao/qwen-code/tree/assets-pr10357/round2-fullstack

Scenario results (containerised, both trees, identical rig)

# Scenario main @ 168a88c02e PR head, merged (1d5a1383e5)
1 happy path, 2604-char answer, ~17 s 54 Card OpenAPI requests / 61.2 KB 54 requests / 65.8 KB (+7.5 % upload, same request count)
2 Card OpenAPI black-holed 3 s mid-stream 10 requests for the whole run, then 15.3 s of total silence; body frozen at 207/2604, status line frozen at 2s 52 requests, longest gap 1.0 s; resumes 58 ms after the last refused request; status line ticks 5s…17s
3 5 s outage spanning completion 2 refused attempts, then nothing. Card ends Running · fake-model · 16s, flowStatus=2, Stop still live, body 2433/2604 — permanently 8 refused attempts on backoff, succeeds at 26.85 s: Completed · fake-model · 17s, flowStatus=3, no Stop, full 2604-char body
4 4 s outage from the moment the message arrives (creation refused) creation abandoned; no card at all for the run, only a 2604-char text fallback creation retried, card delivered at 9.5 s already carrying 1044 chars, run ends Completed — and no text fallback (see F2 below)
5 client A offline 2.0 → 5.3 s, task keeps running A frozen at 207 chars; first newer content 12.0 s after reconnect (that is the terminal write); 2181 chars behind just before the run ends first newer content 4.7 s after reconnect; 154 chars behind at the same probe
6 every gettoken returns errcode 40001 (invalid AppKey) no creation retry (main has none) 1 attempt, then stop — classified permanent
7 every gettoken returns errcode -1 (system busy) no creation retry retries on the 1 / 2 / 4 / 8 s backoff

Repo checks on the merged tree 1d5a1383e5: 453/453 DingTalk tests, npm run typecheck clean, eslint 0 warnings, prettier clean. The suite also passed 15/15 consecutive runs.

3 — outage spanning completion

Card stuck in Running on main, Completed on the PR

2 — outage while the answer is streaming

Characters on a connected client; main goes flat, the PR recovers

The shape worth noticing on the left: one destroyed socket is enough. After it, main issues no Card OpenAPI request at all for 15.3 s — not a failing one, none — because streamFailed latches and takes the status-refresh timer with it.

5 — client drops off and reconnects

Client A frozen on main, repaired 4.7s after reconnect on the PR

New: b7f629a7b5 works end to end

gettoken attempts: the permanent code stops, the transient code backs off

This commit is only reachable through the adapter, so the in-process harness could not exercise it. Once the Stream connection is up, switching every gettoken reply to an error and starting a task gives: errcode -1 → creation retried at +1.0 / +2.0 / +4.0 / +8.0 s; errcode 40001 → one attempt, no retry. Without the classification an invalid AppKey would retry until the process ends, so the commit is load-bearing rather than cosmetic. (The isolated call ~17 s later in each lane is the plain-text fallback at the response boundary, not a retry.)

New: the retry ceiling, measured

Round 1 noted that retries are capped in interval but not in duration. Holding the outage open instead of restoring it puts a number on it: terminal-write attempts land at 19.3 / 19.7 / 20.7 / 22.8 / 26.8 / 34.8 / 50.9 / 80.9 s — gaps of 0.4 / 1 / 2 / 4 / 8 / 16 / 30 s, then one attempt every 30 s for as long as the process lives, with the record and its timer retained. Eight attempts in the 62 s the fault was held. dispose() is the only thing that stops it, and it does (0 requests after disconnect). Two writes per minute per stuck card is cheap; the unbounded retention is the part worth a deliberate decision.

F2 is narrower than round 1 implied

In the full stack, scenario 4 — creation refused for 4 s, then recovered — produced no duplicate at all: the card was delivered late and the run had no response boundary during the backoff, so nothing fell back to text. F2 needs a segment boundary to land inside the creation backoff window, which the in-process harness forces and an ordinary single-segment answer does not. Still worth the line in Risk & Scope, but the exposure is smaller than "every transient creation failure".

Confirmations of the round-2 comment

  • F1 — 4/4 clean reproductions of the 700 ms-creation scenario on the current head, against 4/4 rewinds on f1ce5f317f.
    Round 1 rewound 156 to 104 chars; the current head is monotonic
  • Mutation — an independent 13-mutant sweep (anchors re-pointed after the pendingSnapshot → hasPendingWrite rename, plus one mutant each for the three round-2 commits) agrees: everything is pinned except record.streamRetryAttempt = 0 on the success path. My sweep first reported that one as caught; the only failing test was an unrelated DingtalkAdapter quoted-media case, and re-running the mutant three times the suite passes every time. It survives — same conclusion as the round-2 comment, reached separately.
  • Cost — request count identical in every scenario, upload +7.5 % on this 2604-char answer.

One unrelated observation: DingtalkChannel quoted media > still delivers the text when an over-long quoted file name fails the store failed once in ~35 suite runs under concurrent load and was not reproducible in 21 further runs. The test exists on origin/main, so it is not this PR's, but it looks timing-sensitive.

Merge view

Unchanged and now confirmed at the product level: scenarios 2, 3, 4 and 6/7 fix behaviour that is broken on main today and none of them depend on the client model. F1 is fixed and pinned. F2 is narrower than it looked. The one thing I would still like written down before merge is the retry-retention behaviour (a stuck card holds a record and a 30 s timer until the channel disconnects) — a sentence in Risk & Scope would do; a duration ceiling is optional. Scenario 5 stays "pending real-client evidence", exactly as the PR says.

中文说明

第二轮补充 —— 同一个 head,改用完整产品链路验证

我的第二轮报告是在进程内直接驱动 channel 的几个类。这份补充把同一个 head 放到真正的产品链路上重跑,因为进程内的方式覆盖不到 adapter 的接线、Stream SDK 握手、channel 自己的 gettoken,以及真实进程里的 disconnect()。

结论: 第二轮报告的每一条结论在整条产品链路下都成立;另外三件它够不到的事现在有了实测数据:b7f629a7b5 的 gettoken 分类端到端有效;终态重试没有总时长上限,稳定后每 30 秒一次直到 dispose();F2 在普通的单 segment 运行里不会触发——它需要响应边界正好落在创建退避窗口里。合并意见不变:建议合。

环境

head 90eb0a9a61,合入 origin/main 后实际跑的是 1d5a1383e5
base 168a88c02e(origin/main),不是 merge base——所以这轮同时复核了 PR 对当前 main 的表现
被测产物 两棵树各自 npm ci && npm run build && npm run bundle 出来的 dist/cli.js
宿主 / 容器 macOS,Node v24.18.1 / node:22-bookworm

容器用 --network none 启动,并 --add-host api.dingtalk.com:127.0.0.1 --add-host oapi.dingtalk.com:127.0.0.1,两个域名的自签证书通过 NODE_EXTRA_CA_CERTS 信任,容器里跑 qwen channel start dt。channel 要访问的一切都在本地:HTTPS 443 提供 oapi…/gettoken、api…/v1.0/gateway/connections/open 和三个 Card OpenAPI 接口;8899 的 WS 网关接受 SDK 连接并下发真实的 /v1.0/im/bot/messages/get CALLBACK 帧;sessionWebhook 指向本地端口以捕获纯文本兜底;另有一个 OpenAI 兼容端点在约 17 秒内流式吐出 2604 字符的答案。packages/ 下没有任何打桩或改动——SDK 握手、DingtalkChannel.getProactiveToken、presenter、状态卡控制器、卡片客户端全部按发布形态运行,走真实 TLS、真实定时器。故障注入是直接断开 socket,因此 PR 自己的 DingtalkCardRequestError 分类原样生效。

两个模拟钉钉客户端订阅同一张卡片,仍沿用 m2 假设(断线期间错过的帧不会补发)。这个假设依然是本地无法定论的唯一一点,详见第二轮报告。

工具、逐场景原始 JSON、变异日志:https://github.com/wenshao/qwen-code/tree/assets-pr10357/round2-fullstack

场景结果(容器内,两棵树,同一套工具)

# 场景 main @ 168a88c02e PR head 合并后(1d5a1383e5)
1 正常路径,2604 字符答案,约 17 秒 54 次 Card OpenAPI 请求 / 61.2 KB 54 次 / 65.8 KB(上行 +7.5%,请求次数相同)
2 流式过程中 Card OpenAPI 黑洞 3 秒 整轮只发出 10 次请求,随后 15.3 秒完全静默;正文冻结在 207/2604,状态行冻结在 2s 52 次请求,最大间隔 1.0 秒;在最后一次被拒请求后 58 毫秒恢复;状态行继续从 5s 走到 17s
3 跨越任务完成的 5 秒断网 只试了 2 次就再无动作。卡片最终停在 Running · fake-model · 16s、flowStatus=2、中止按钮仍在、正文 2433/2604——永久 按退避重试 8 次,26.85 秒成功:Completed · fake-model · 17s、flowStatus=3、无中止按钮、正文完整 2604 字符
4 从消息到达起黑洞 4 秒(创建被拒) 创建被放弃;整轮根本没有卡片,只有一条 2604 字符的文本兜底 创建重试成功,9.5 秒投递出的卡片正文已有 1044 字符,最终 Completed——且没有文本兜底(见下文 F2)
5 客户端 A 在 2.0–5.3 秒断网,任务继续 A 冻结在 207 字符;重连后 12.0 秒才看到新内容(那已经是终态写入);运行结束前落后 2181 字符 重连后 4.7 秒看到新内容;同一时刻只落后 154 字符
6 gettoken 一直返回 errcode 40001(AppKey 无效) 无创建重试(main 本来就没有) 只试 1 次就停——判定为永久错误
7 gettoken 一直返回 errcode -1(系统繁忙) 无创建重试 按 1 / 2 / 4 / 8 秒退避重试

合并树 1d5a1383e5 上的仓库检查:钉钉包 453/453 测试通过,npm run typecheck 干净,eslint 0 警告,prettier 干净;套件另外连跑 15/15 全绿。

场景 2 左侧值得注意的形状:一次 socket 断开就够了。此后 main 在 15.3 秒里一次 Card OpenAPI 请求都没有发——不是发了失败,而是根本没发——因为 streamFailed 锁死并把状态刷新定时器一起带走了。

新增:b7f629a7b5 端到端有效

这个 commit 只能经由 adapter 触达,进程内工具覆盖不到。在 Stream 连接建立之后,把每次 gettoken 应答换成错误再发起任务:errcode -1 → 创建在 +1.0 / +2.0 / +4.0 / +8.0 秒重试;errcode 40001 → 只试一次,不重试。如果没有这层分类,AppKey 配错会一直重试到进程结束,所以这个 commit 是承重的,不是装饰。(两条泳道里约 17 秒处那次孤立调用是响应边界的纯文本兜底,不是重试。)

新增:重试上限的实测

第一轮提过重试只限制了间隔、没有限制总时长。这次不恢复网络、把故障一直保持住,就有了具体数字:终态写入尝试落在 19.3 / 19.7 / 20.7 / 22.8 / 26.8 / 34.8 / 50.9 / 80.9 秒——间隔依次 0.4 / 1 / 2 / 4 / 8 / 16 / 30 秒,此后只要进程还在,就每 30 秒一次,record 和定时器一直保留。故障持续的 62 秒内共 8 次尝试。只有 dispose() 能停下它,而它确实能停(断开后 0 请求)。每分钟两次写入成本很低;真正值得明确决策的是无上限的保留。

F2 比第一轮描述的更窄

在完整链路下,场景 4——创建被拒 4 秒后恢复——完全没有出现重复投递:卡片只是晚了一点投递,而这一轮在退避窗口内没有响应边界,所以没有任何东西退化成文本。F2 需要 segment 边界正好落在创建退避窗口内,进程内工具是强制构造出这个边界的,普通的单 segment 回答不会。仍然值得在 Risk & Scope 里写一句,但暴露面比「每次瞬时创建失败」小得多。

对第二轮报告的印证

  • F1 —— 在当前 head 上 700ms 创建场景 4/4 干净;在 f1ce5f317f 上 4/4 回退。
  • 变异 —— 独立做了一轮 13 个变异体(按 pendingSnapshot → hasPendingWrite 重命名重新定位锚点,并为三个第二轮 commit 各加一个),结论一致:除了成功路径上的 record.streamRetryAttempt = 0,其余全部被钉住。我这轮最初把它记成「被捕获」,但唯一失败的是一条无关的 DingtalkAdapter quoted-media 用例;把该变异体重跑三次,套件每次都通过。它确实存活——与第二轮报告独立得出同一结论。
  • 成本 —— 所有场景请求次数完全相同,本例 2604 字符答案上行 +7.5%。

另有一条无关观察:DingtalkChannel quoted media > still delivers the text when an over-long quoted file name fails the store 在约 35 次套件运行中、在并发负载下失败过一次,随后 21 次都没能复现。该用例在 origin/main 上就存在,不是本 PR 引入的,但看起来对时序敏感。

合并意见

不变,并且现在在产品层面得到印证:场景 2、3、4 和 6/7 修的是 main 今天确实坏掉的行为,且都不依赖客户端模型;F1 已修复并被测试钉住;F2 的暴露面比看上去更窄。合并前我唯一希望写明的是重试保留行为(一张卡死的卡片会持有一个 record 和一个 30 秒定时器,直到 channel 断开)——在 Risk & Scope 里加一句即可,总时长上限可加可不加。场景 5 维持「等待真实客户端证据」,与 PR 现有表述一致。

@qqqys

qqqys commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover from 8

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Aug 31, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 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). This window's round counter starts at 8 (the rounds this PR spent in review before takeover), so the Critical-only brake engages after 0 more change-producing round(s) instead of a full fresh 5. Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。本窗口轮次计数从 8 起算(即本 PR 托管前已进行的评审轮数),因此再经过 0 个产生改动的轮次即进入 Critical-only,而非重新计满 5 轮。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-dev-bot

qwen-code-dev-bot commented Aug 31, 2026 •

Copy link
Copy Markdown
Collaborator

✅ AutoFix round 9 finished — view run. See this round's report below.

中文说明

✅ AutoFix 第 9 轮已完成 —— 查看运行。本轮报告见下方。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 9/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 9/100 轮)。改动内容与我反驳保留之处如下:

Review round summary — PR #10357

Critical-only mode is active (round budget engaged). This round touched no
production code
: it strengthens two test witnesses for the abandon()
guards that the round-8 Critical fix (5ac200bf8f) introduced, because a
mutation probe showed the previously claimed pins did not actually hold.

What this round changed

One commit, 12bfb94086 (test(dingtalk): pin status card abandon guards),
17 added test lines in
packages/channels/dingtalk/src/status-card-controller.test.ts:

  • stops retrying after a failed boundary drain is abandoned now holds the
    status-refresh heartbeat in flight (via a deferred updateInstance) while the
    boundary drain hangs, so the heartbeat is still pending when abandon()
    detaches the record. This pins the record.terminal = true latch: deleting
    that latch now turns the test red
    (it re-arms a per-second heartbeat that
    re-publishes the fallen-back content). Before this change the latch-deletion
    mutant passed all 46 controller tests.
  • keeps a terminal retry when abandon races finalization now asserts the
    final updateInstance carries the terminal content with flowStatus: 3,
    not just a call count. This pins the terminal-record skip guard: deleting
    || record.terminal now turns the test red
    (a bare close-out would satisfy
    the old count-only assertion). Before this change the skip-guard-deletion
    mutant passed all 46 controller tests.

No production source changed, so no behavior moved; the two mutants that
previously survived with the suite green are now caught.

Finding dispositions

  • R8-1 (rc:3894465040, Critical) — Resolved in code by 5ac200bf8f
    (abandon() latches terminal, skips terminal records, best-effort no-content
    close-out, ownership check on in-flight creation). This round added the
    missing witness strength so the latch and skip guards are actually pinned.
    Verified: latch-deletion and skip-guard-deletion mutants both go red; close-out
    deletion goes red on two tests; restored code is green.
  • R8-2 (rc:3894465057, Critical) — Resolved in code by 5ac200bf8f
    (all three fallback branches abandon only after a successful sendFallback).
    Verified by mutation probe: moving the response_boundary abandon back before
    the awaited send turns retains card content when a response-boundary fallback fails red. No change needed this round.
  • R7-1 / R7-2 (Critical) and R1-1 … R1-14 (Suggestions) — All previously
    resolved in code (commits b2482e4762…042a784ba7). Re-verified against the
    current tree this round: gettoken errcode classification, flush streamFailed
    guards, creation retry, single-computation terminal status line, capped
    exponential backoff, 401 invalidate-and-retry-once, per-recipient
    non-retryable delivery, and the five unpinned-guard cleanups are all present
    and exercised by passing tests. Marked resolved.

Deferred non-Critical items listed in the feedback were left untouched, per
critical-only mode.

Conflict notes

--conflict false; no merge was performed and none was needed.

Verification

Commands actually run (not inferred from the diff):

  • npx vitest run src/status-card-controller.test.ts (dingtalk package) — 46 passed after the edits.
  • Mutation probes, each applied then reverted with git checkout --:
    • delete record.terminal = true in abandon() → 1 test failed (wanted red) → restored green.
    • delete || record.terminal in abandon() → 1 test failed (wanted red) → restored green.
    • delete the no-content close-out in abandon() → 2 tests failed (wanted red) → restored green.
    • move response_boundary abandon before await sendFallback → 1 test failed (wanted red) → restored green.
  • npx vitest run (full packages/channels/dingtalk) — 461 passed (12 files).
  • Scoped eslint + prettier --check on the changed test file — clean.
  • npm run build — exit 0.
  • npm run typecheck — exit 0.
  • npm run lint — exit 0.
中文说明

评审轮次总结 — PR #10357

当前处于仅处理 Critical 的模式(轮次预算已触发)。本轮未改动任何生产
代码
:仅加强了两个测试见证(witness),用于钉住第 8 轮 Critical 修复
(5ac200bf8f)引入的 abandon() 守卫——因为变异探针显示此前声称的钉扎
实际上并不成立。

本轮改动

单个提交 12bfb94086(test(dingtalk): pin status card abandon guards),
在 packages/channels/dingtalk/src/status-card-controller.test.ts 中新增
17 行测试代码:

  • stops retrying after a failed boundary drain is abandoned:现在用
    延迟(deferred)的 updateInstance 把状态刷新心跳挂在半空,使边界 drain
    悬而未决时该心跳仍处于在途状态,从而在 abandon() 摘除记录时心跳仍未落定。
    这钉住了 record.terminal = true 闩:删除该闩现在会让测试变红(它会
    重新挂起每秒心跳并再次发布已兜底的内容)。改动前,删除该闩的变异体在全部
    46 个控制器测试下仍为绿。
  • keeps a terminal retry when abandon races finalization:现在断言最后
    一次 updateInstance 携带终态 content 且 flowStatus: 3,而不仅是调用
    次数。这钉住了"跳过终态记录"守卫:删除 || record.terminal 现在会让测试
    变红
    (一次不带正文的收尾写入就能满足旧版的仅次数断言)。改动前,删除该
    守卫的变异体在全部 46 个控制器测试下仍为绿。

生产源码未改动,因此行为没有任何变化;此前在套件全绿下存活的两个变异体,
现在都会被捕获。

发现处理结论

  • R8-1(rc:3894465040,Critical)——代码已由 5ac200bf8f 修复
    (abandon() 闩住终态、跳过终态记录、尽力发出无正文收尾写入、对在途创建做
    所有权校验)。本轮补齐了缺失的见证强度,使闩守卫与跳过守卫真正被钉住。
    已验证:删除闩、删除跳过守卫的变异体均变红;删除收尾写入使两个测试变红;
    恢复后的代码为绿。
  • R8-2(rc:3894465057,Critical)——代码已由 5ac200bf8f 修复(三个
    fallback 分支都只在 sendFallback 成功后才 abandon)。已用变异探针验证:
    把 response_boundary 分支的 abandon 移回 await sendFallback 之前,会
    使 retains card content when a response-boundary fallback fails 变红。
    本轮无需改动。
  • R7-1 / R7-2(Critical)与 R1-1 … R1-14(Suggestion)——均已在先前的
    提交(b2482e4762…042a784ba7)中于代码层面修复。本轮在当前树上复核:
    gettoken 按 errcode 分类、flush 的 streamFailed 守卫、创建重试、终态状态
    行只计算一次、有上限的指数退避、401 失效缓存并重试一次、按收件人判定为不可
    重试的投递失败,以及五处未钉扎守卫的清理——全部存在,且由通过的测试覆盖。
    标记为已解决。

反馈中列出的、被延后的非 Critical 条目,按仅处理 Critical 的模式保持原样、
未做改动。

冲突说明

--conflict false;未执行、也无需执行任何合并。

验证

实际执行的命令(并非凭 diff 推断):

  • npx vitest run src/status-card-controller.test.ts(dingtalk 包)——改动后
    46 个通过。
  • 变异探针,均先用 git checkout -- 还原:
    • 删除 abandon() 中的 record.terminal = true → 1 个测试失败(符合
      预期的变红)→ 恢复后为绿。
    • 删除 abandon() 中的 || record.terminal → 1 个测试失败(符合预期
      的变红)→ 恢复后为绿。
    • 删除 abandon() 中无正文的收尾写入 → 2 个测试失败(符合预期的变红)
      → 恢复后为绿。
    • 把 response_boundary 的 abandon 移到 await sendFallback 之前 → 1 个
      测试失败
      (符合预期的变红)→ 恢复后为绿。
  • npx vitest run(整个 packages/channels/dingtalk)——461 个通过
    (12 个文件)
    。
  • 对改动的测试文件做定向 eslint 与 prettier --check——干净。
  • npm run build —— 退出码 0。
  • npm run typecheck —— 退出码 0。
  • npm run lint —— 退出码 0。

🦷 Gate advisory — this round resolves a Critical/Request-changes finding with test-only changes (machine-measured): the bite check cannot verify a test-side fix, so the resolution rests on the round summary alone. · 本轮以纯测试改动解决 Critical/Request-changes 反馈(门自动测量):bite 检查无法验证测试侧修复,该解决仅以轮次摘要为凭。

Deferred non-Critical feedback

Critical-only mode is active: the round counter reached 5 (this window was seeded at round 8 by @qwen-code /takeover from 8, plus 0 change-producing round(s) since). The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (@qwen-code /retry starts a fresh counting window.)

中文说明

已进入仅处理 Critical 的模式:轮次计数已达 5(本窗口由 @qwen-code /takeover from 8 从第 8 轮起算,此后又完成 0 个产生改动的轮次)。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 @qwen-code /retry 可开启新的计数窗口。)

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

🧵 Resolved all 18 selected review thread(s). · 已关闭全部选中的 18 条评审线程。

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/模型 qwen3.8-max

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 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 将重新运行。

@wenshao

wenshao commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@wenshao
wenshao enabled auto-merge August 31, 2026 23:42
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@qqqys
qqqys dismissed a stale review September 1, 2026 02:13

fixed

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM at 4382ce4. Reviewed the full diff:

  • Retryability classification is right: transient HTTP (408/425/429/5xx) and non-client errors retry; DingTalk business failures (success:false) stay non-retryable; 401 gets a single token-invalidate-and-retry, then errors.
  • The controller rework is coherent: creation retries with capped backoff settle via creationAttempt so mid-backoff boundary decisions fall back to text without losing the retry; terminal intent is computed once so retries don't inflate elapsed time; flush's version tracking correctly reschedules when content advances mid-write; dispose/abandon clear every timer and record path.
  • Content always goes through boundContent + sanitizeStreamingImageMarkers at every write point, including the 5s heartbeat republish; no new untrusted sinks.
  • Presenter calls abandon() on all three fallback-to-text paths, so pending card recovery stops when the fallback takes over.
  • ~1.1k new test lines cover the recovery paths.

CI note: Test (ubuntu) and review-pr were still running at approval time; the merge queue will gate on them. Approving.

@wenshao
wenshao added this pull request to the merge queue Sep 1, 2026
Merged via the queue into QwenLM:main with commit 4c9a2db Sep 1, 2026
39 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) review/self-reported The linked issue was opened by the PR author (self-reported)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(dingtalk): recover status cards after transient network failures

4 participants