fix(core): recognize OpenAI SDK APIUserAbortError as an abort

[harjothkhara]

What this PR does

Fixes #8398 — a user cancel on the OpenAI-compatible path is misclassified and reported as an API error.

1. isAbortError recognizes the OpenAI SDK's APIUserAbortError. That class sets no .name (it stays 'Error') and carries no ABORT_ERR code, so both existing checks missed it. It is matched by class name, which keeps this provider-agnostic util free of an SDK import.
2. safelyLogApiError skips the api_error event on a user cancel. It emitted ApiErrorEvent unconditionally, so cancelling produced a qwen-code.api_error with error_type=APIUserAbortError — the noise reported in Bug: after APIUserAbortError, subsequent turns are not written to the local session transcript #8356. The span still records the cancellation, so nothing is lost.

Why it's needed

• Telemetry — the event comes from LoggingContentGenerator.safelyLogApiError, which was not gated at all. Thanks to @wenshao and @yiliang114 for verifying the util fix and showing it did not close this path on its own.
• Retry classification — the classifier labelled the error kind:'unknown' instead of 'abort', missing the no-retry short-circuit.
• Debug logging — shouldSuppressErrorLogging gates on isAbortError, so the cancel was logged as an API error there too.

geminiChat and the MCP/artifact tools also route aborts through isAbortError, so the util fix corrects them as well.

Scope

Per @wenshao's review, this PR is only the bug fix. The cross-cutting invariant that grew here across earlier rounds — a shared user-cancel predicate plus tagging every internal deadline — is split into a separate PR so its polarity can be settled before it merges. That work is implemented and will be opened once this lands.

Known limitation, carried deliberately: the gate is abortSignal?.aborted && isAbortError(error), which cannot tell a user cancel from an internal deadline aborting the same request, so a timed-out internal side query stays suppressed. That is the pre-existing behaviour, not a regression introduced here, and it is what the follow-up PR fixes.

Reviewer Test Plan

npx vitest run --root packages/core

Result on this branch: 19575 passed, 3 failed, 11 skipped.

All three failures are unrelated to this PR, and each file is byte-identical to origin/main on this branch:

• two in src/tools (read-file, zoom-image) — local workspace-trust/path-permission expectations that fail the same way on unmodified main
• one in src/memory (extract) — flaky under parallel load; passes when run in isolation

Every suite this PR touches is green: the changed and reverted areas together are 43 files, all passing. tsc --noEmit is clean for both core and cli, and lint is clean.

Evidence

RED→GREEN on the fix itself: on unmodified source isAbortError returns false for the SDK's user-abort error and the retry classifier labels it kind:'unknown'; both flip with the fix applied.

The gate is covered for the shapes a cancel actually takes — the SDK error on the non-stream path, on stream setup, and mid-stream, plus the DOMException AbortError the Google GenAI SSE reader propagates — and for the case that must still be reported: a genuine failure that races a cancel.

Risk & Scope

• Class-name matching: recognition keys off the constructor name rather than instanceof, to keep the provider-agnostic util free of an SDK import. This relies on the CLI bundle's keepNames. Happy to switch to instanceof at the provider boundary — @wenshao suggested this and I agree; it is a small independent change that can land here or in the follow-up, your call.
• Breaking changes: none. isAbortError's signature is unchanged; it only starts returning true for a genuine user abort it previously missed.
• Out of scope: Bug: after APIUserAbortError, subsequent turns are not written to the local session transcript #8356's transcript-write blackout is a separate recorder/lifecycle matter and is not addressed here, hence Refs, not Closes.

Linked Issues

Refs #8398

[qwen-code-ci-bot]

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

✅ Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

[qwen-code-ci-bot]

Re-run on head aa6eca29 (unchanged since the last pass) — triggered by @wenshao's approval + /triage.

Template ✓ — all required sections present.

Problem — observed, not theoretical. The real error_type=APIUserAbortError noise from #8356 / #8398: the SDK's user-abort class keeps .name === 'Error' and carries no ABORT_ERR code, so both existing checks in isAbortError miss it. The premise was verified on openai by @yiliang114, and @wenshao has since reproduced it on the real stack — the base build emits the spurious api_error on ESC, this PR's build does not.

Direction — aligned; the prior escalation is resolved. A fork suppressing a telemetry event inside packages/core is the escalate-to-a-human case, so the last pass deferred to @wenshao instead of approving. He has now answered it directly: steered the split, ran an end-to-end local A/B verification (real bundled build, real ESC, telemetry outfile, wire ledger), and approved this head. Nothing left for the escalation to wait on.

Size — small and focused. Core paths: utils/errors.ts + loggingContentGenerator/loggingContentGenerator.ts. Production 53 lines (23 + 30); tests 323 lines; generated/schema 0. Well under every threshold.

Approach — right-sized, and the description now matches it. The one real issue from the last pass — the stale body still describing the split-out isUserCancel / producer-tagging scope — is fixed: the body now describes exactly the 2-file change, and the gate's known limitation is stated deliberately under Scope.

Risk — no elevated signals. Neither changed production file matches a revert-correlated path.

Moving on to code review. 🔍

中文说明

在 head aa6eca29 上复跑（与上次审查相比无变化）——由 @wenshao 的批准 + /triage 触发。

模板 ✓ —— 必填章节齐全。

问题 —— 已观测到，非理论性。 即 #8356 / #8398 报告的真实 error_type=APIUserAbortError 噪声：SDK 的用户中止类 .name 保持为 'Error' 且无 ABORT_ERR code，isAbortError 现有两处检查都会漏掉它。前提已由 @yiliang114 在 openai 上验证，@wenshao 随后在真实栈上复现——base 构建在 ESC 时发出虚假的 api_error，本 PR 构建不会。

方向 —— 对齐；此前的升级（escalation）已解决。 fork 在 packages/core 中抑制遥测事件，属于“交给人类裁决”的情形，所以上一轮暂缓并转交 @wenshao。他现已直接作答：主导拆分、在本地做了端到端 A/B 验证（真实打包产物、真实 ESC、遥测落盘、wire 账本），并批准了该 head。升级已无任何待决事项。

规模 —— 小而聚焦。 核心路径：utils/errors.ts + loggingContentGenerator/loggingContentGenerator.ts。生产代码 53 行（23 + 30）；测试 323 行；生成/schema 0。远低于所有阈值。

方案 —— 范围合理，且描述现已与之一致。 上一轮唯一真实的问题——正文仍在描述已拆出的 isUserCancel / 产生方打标范围——已修复：正文现在准确描述这 2 个文件的改动，门的已知限制被有意写入 Scope。

风险 —— 无升级信号。 两个改动的生产文件均不命中与 revert 相关的高风险路径。

进入代码审查。🔍

— Qwen Code · qwen3.8-max

_{Reviewed at aa6eca291fbffe0f0a226610ebfc1201c85cde2a · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code review (head aa6eca29, re-verified this pass)

Independent proposal first: broaden the shared isAbortError — matching the class name, the same trick getErrorType in the same file already uses for SDK errors, so the provider-agnostic util needs no SDK import — and separately gate the api_error emission on "the caller's signal fired AND the error is abort-shaped," mirroring the two-part gate OpenAIContentGenerator.shouldSuppressErrorLogging already uses. That is exactly what this PR does.

isAbortError (utils/errors.ts). The new branch — error instanceof Error && error.constructor?.name === 'APIUserAbortError' — sits before return false and can only flip a previous false to true for this one class. I re-confirmed the premise against base: the existing checks are name === 'AbortError' and code === 'ABORT_ERR', and the SDK class matches neither. Both openai and @anthropic-ai/sdk are Stainless-generated and share the class name; tests pin both positives plus an APIConnectionError negative so the match can't be silently broadened. The keepNames note holds — the CLI bundle sets keepNames: true in esbuild.config.js, and the vscode companion has no keepNames but also no SDK usage.

Telemetry gate (loggingContentGenerator.ts). safelyLogApiError gains an optional abortSignal parameter and skips the api_error event when abortSignal?.aborted && isAbortError(error); the span's aborted status still records the cancel. I re-checked all three call sites — the non-stream catch, the stream-setup catch, and the stream-iteration catch — and all three now pass the caller's signal (abortSignal is in scope at the third, used by its own finally). The 8 caller-level tests pin the full truth table: cancels suppressed (non-stream, stream setup, mid-stream, DOMException shape), while a real failure, an abort-shaped error the user didn't cause, a request with no signal, and a real failure racing a cancel are all still reported.

Consumers of the broadened util — walked again: geminiChat (compression / resolve / fallback rethrows), shouldSuppressErrorLogging (now catches SDK cancels on the openai path), artifact-tool, mcp-tool, retryErrorClassification (kind:'abort' short-circuits retries), fileUtils, readManyFiles. Treating a genuine provider user-cancel as an abort moves every one in the right direction; webui / web-shell / desktop / vscode / sdk keep their own local isAbortError and are untouched.

Non-blocking observations (carried, all known and deliberate):

• Wording: the body's "a timed-out internal side query stays suppressed — that is the pre-existing behaviour" is looser than the sandboxed measurement, which found the base build did record internal-deadline failures in the api_error / model-health counters and this gate newly suppresses them. The trade-off itself is the one @wenshao explicitly accepted at the split and is the follow-up PR's subject — but if the body is touched again, that clause is the one to tighten.
• Qwen debug-log path: QwenContentGenerator.shouldSuppressErrorLogging returns only isAuthError and doesn't call super, so a Qwen cancel still appears in the debug log even though the telemetry event is now suppressed. Status quo for Qwen, no regression.
• keepNames is load-bearing for the class-name match and has no CI gate — fine to leave for the follow-up, as accepted.

Test evidence (unattended re-run — no PR code executed here)

The sandboxed @qwen-code /verify round on this head has completed: 69/69 scripted assertions passed, and the A/B proof is the substantive part — base + this PR's tests is RED (7 failed, exactly the new assertions), head is GREEN (148/148); on the compiled bundle, a user cancel short-circuits the retry loop on attempt 1 where base retried 3×. Its findings — the "status quo" wording above and the then-stale body — are carried as non-blocking notes; the body has since been updated.

The gap that round listed ("real network-cancel E2E") was then closed by @wenshao's maintainer-run local verification posted in this thread: real bundled build, real ESC in a terminal, telemetry outfile + wire ledger — the api_error flip reproduces (1× on base → 0 on head), genuine HTTP 500s still report (4×, one per retry), mid-stream cancels emit nothing on either side, and no zombie retry appears post-cancel. That is the maintainer's own first-hand evidence, and it matches what the diff promises.

Check	Conclusion
Test (ubuntu-latest, Node 22.x)	✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x)	✅ success
Desktop Shell (ubuntu-22.04)	✅ success
Desktop Shell (windows-2022)	✅ success
precheck-pr / precheck	✅ success
Test (macos-latest, Node 22.x)	⏭️ skipped (merge_group-gated)
Test (windows-latest, Node 22.x)	⏭️ skipped (merge_group-gated)
Integration Tests (CLI, No Sandbox)	⏭️ skipped (merge_group-gated)

CI is fully settled and green on this head — nothing pending, nothing failing.

Not verified (by this run): live TUI driving — this is an unattended re-run, so no tmux pass was driven here; the real-stack behavioral evidence is the maintainer's local run above. The vitest numbers in the PR body are the author's self-reported local run and were not independently re-run here — CI's green suite is the authoritative signal.

中文说明

代码审查（head aa6eca29，本轮重新核验）

先给独立方案： 放宽共享的 isAbortError——用类名匹配，与同文件 getErrorType 对 SDK 错误已用的手法相同，使该与 provider 无关的工具函数无需引入 SDK——另外单独给 api_error 上门：“调用方 signal 已触发 且 错误为中止形态”，与 OpenAIContentGenerator.shouldSuppressErrorLogging 已在用的两段式门一致。这正是本 PR 所做的。

isAbortError（utils/errors.ts）。 新分支——error instanceof Error && error.constructor?.name === 'APIUserAbortError'——位于 return false 之前，只会把此前为 false 的结果在且仅在这一类上变为 true。已在 base 上重新确认前提：现有检查为 name === 'AbortError' 与 code === 'ABORT_ERR'，SDK 该类两者均不命中。openai 与 @anthropic-ai/sdk 均为 Stainless 生成、共享该类名；测试固定了两个正例，并有 APIConnectionError 负例防止匹配被悄悄放宽。keepNames 说明成立——CLI 产物在 esbuild.config.js 中设 keepNames: true；vscode companion 不带 keepNames，但也不使用 SDK。

遥测门（loggingContentGenerator.ts）。 safelyLogApiError 增加可选 abortSignal 参数，在 abortSignal?.aborted && isAbortError(error) 时跳过 api_error 事件；span 的中止状态仍会记录取消。我复核了全部三处调用点——非流式 catch、流建立 catch、流迭代 catch——三处现在都传入调用方的 signal（第三处 abortSignal 在作用域内，其自身 finally 也在用）。8 个调用方级测试固定了完整真值表：取消被抑制（非流、流建立、流中、DOMException 形态）；真实失败、非用户导致的中止形态错误、无 signal 的请求、与取消竞态的真实失败仍会上报。

被放宽工具函数的消费方——再次走查： geminiChat（压缩 / resolve / fallback 的重抛）、shouldSuppressErrorLogging（现在能在 openai 路径捕获 SDK 取消）、artifact-tool、mcp-tool、retryErrorClassification（kind:'abort' 短路重试）、fileUtils、readManyFiles。把真实的 provider 用户取消当作 abort 处理，对每一处都是正确方向；webui / web-shell / desktop / vscode / sdk 各自保留本地 isAbortError，不受影响。

非阻塞观察（延续项，均已知且为有意取舍）：

• 措辞： 正文中“内部超时侧查询保持被抑制——这是既有行为”比沙箱实测更宽松：实测发现 base 构建确实把内部超时失败记入 api_error / 模型健康计数，是该门新近将其抑制。取舍本身是 @wenshao 在拆分时明确接受、后续 PR 要解决的事项——但若再改正文，应收紧这一句。
• Qwen 调试日志路径： QwenContentGenerator.shouldSuppressErrorLogging 仅返回 isAuthError、不调 super，因此即便遥测事件已被抑制，Qwen 的取消仍会出现在调试日志中。对 Qwen 是现状、非回归。
• keepNames 对类名匹配是 load-bearing，且无 CI 门禁——按已接受的意见留给后续 PR 即可。

测试证据（无人值守复跑 —— 本轮未执行 PR 代码）

针对该 head 的沙箱 @qwen-code /verify 已完成：69/69 脚本断言通过，实质性部分是 A/B 证明——base + 本 PR 测试为 RED（7 失败，恰为新断言），head 为 GREEN（148/148）；编译产物层面，用户取消在第 1 次即短路重试循环，base 会重试 3 次。其发现——上述“现状”措辞与当时过时的正文——作为非阻塞项延续；正文现已更新。

那轮列出的缺口（“真实网络取消 E2E”）已由本主题下 @wenshao 的维护者本地验证补上：真实打包产物、真实终端按 ESC、遥测落盘 + wire 账本——api_error 翻转复现（base 1 次 → head 0 次），真实 HTTP 500 仍上报（4 次，每次重试一条），流中取消两侧都不产生事件，取消后 wire 上无僵尸重试。这是维护者本人的一手证据，与 diff 的承诺一致。

CI 表格见上（标记区内）：该 head 的 CI 已全部落定且为绿，无 pending、无失败检查；macOS / Windows 单测与集成测试由 merge_group 门控，在 PR 上跳过。

本轮未验证： 真实 TUI 驱动——无人值守复跑，不在本地跑 tmux；真实栈行为证据即上文维护者的本地验证。PR 正文中的 vitest 数字为作者自述的本地结果，本轮未独立复跑——以 CI 绿色套件为权威信号。

— Qwen Code · qwen3.8-max

_{Reviewed at aa6eca291fbffe0f0a226610ebfc1201c85cde2a · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 4/5 — clean across every stage, and the behavioural claim is now proven twice over (sandboxed A/B, then the maintainer's real-stack run); what keeps it short of 5 is carried follow-up debt everyone agreed to defer, not any defect in this diff.

Stepping back: the last pass deferred for two reasons, and both are now closed. The escalation — fork + core + a suppressed telemetry event — went to @wenshao, and he answered it with the strongest possible signal: a local end-to-end A/B on the real bundled build (ESC during setup flips api_error 1→0, HTTP 500 still reports 4×, mid-stream is clean on both sides, no zombie retry on the wire), then an approval on this head. The stale description was rewritten and now matches the 2-file diff, with the gate's known limitation stated deliberately under Scope.

My independent proposal for this bug was exactly what landed: broaden isAbortError by class name (the getErrorType pattern, no SDK import) and gate the telemetry emission on signal-fired + abort-shaped (the shouldSuppressErrorLogging pattern). All three call sites pass the signal, the gate's truth table is pinned in both directions, and every consumer of the broadened util moves the right way. If I had to maintain this in six months I'd thank the author — it's small enough to audit in one sitting, tested at the caller level, and honest about what it deliberately leaves out.

What remains is documented and deliberate, not blocking: the user-cancel vs internal-deadline ambiguity (the body's "pre-existing behaviour" clause is slightly looser than the sandboxed measurement — base did report those events — but the trade-off is reviewer-accepted and the follow-up inverts the polarity), the Qwen debug-log override that skips super, and keepNames having no CI gate. All three belong to the follow-up PR the author has already implemented.

Verdict: approve. CI is settled and green on this head, the fork-refactor guardrail doesn't apply (this is a fix), and no escalation remains open. Approving pinned to the reviewed commit — this supersedes my pre-split CHANGES_REQUESTED reviews, which were gating the PR's old, larger shape.

@harjothkhara — nice work seeing this through the rounds and the split.

中文说明

置信度：4/5 —— 各阶段全部干净，且行为论断已被双重证明（沙箱 A/B，随后是维护者的真实栈验证）；没到 5 分是因为延续到后续 PR 的既定债务，而非本 diff 有任何缺陷。

退一步看：上一轮暂缓有两个原因，现已全部关闭。升级事项——fork + 核心 + 被抑制的遥测事件——交给了 @wenshao，他以最有力的信号作答：在真实打包产物上做本地端到端 A/B（建立期 ESC 使 api_error 1→0，HTTP 500 仍上报 4 次，流中两侧都干净，wire 上无僵尸重试），随后在该 head 上批准。过时的描述已重写，与 2 文件 diff 一致，门的已知限制被有意写入 Scope。

我对这个 bug 的独立方案与最终落地的完全一致：按类名放宽 isAbortError（getErrorType 的手法，无需引入 SDK），并给遥测上“signal 已触发 + 中止形态”的门（shouldSuppressErrorLogging 的手法）。三处调用点都传入 signal，门的真值表双向固定，被放宽工具函数的每个消费方都朝正确方向移动。六个月后维护这段代码我会感谢作者——小到一次就能审完、在调用方层级测试、对有意不做的事诚实。

余下的是已记录、有意为之、非阻塞的事项：用户取消与内部超时的歧义（正文“既有行为”一句比沙箱实测略宽松——base 确实上报过那些事件——但该取舍为评审者接受，后续 PR 会反转极性）、Qwen 调试日志覆写未调 super、keepNames 无 CI 门禁。三者都属于作者已实现的后续 PR。

裁决：批准。 该 head 的 CI 已落定且全绿，fork-refactor 护栏不适用（这是 fix），无任何未决升级。按被审提交钉住批准——本次批准取代我拆分前的 CHANGES_REQUESTED 审查，那些审查针对的是旧的、更大的形态。

@harjothkhara —— 这么多轮评审加一次拆分，做得漂亮。

— Qwen Code · qwen3.8-max

_{Reviewed at aa6eca291fbffe0f0a226610ebfc1201c85cde2a · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

LGTM, looks ready to ship — CI landed green after the review. ✅

[qwen-code-ci-bot]

Reviewed — no blockers. Suggestions are inline. Test Plan (not a blocker): src/utils/errors.test.ts — no such file or directory; src/utils/retryErrorClassification.test.ts — no such file or directory.

中文说明

已审查——无阻断问题。 建议见行内评论。 Test Plan（非阻断）：src/utils/errors.test.ts — no such file or directory; src/utils/retryErrorClassification.test.ts — no such file or directory。

— qwen3.8-max-preview via Qwen Code /review (v0.21.3)

[qwen-code-ci-bot]

[Suggestion] A one-provider SDK detail — the OpenAI SDK class name APIUserAbortError — is hardcoded into the provider-agnostic isAbortError. Both the OpenAI and Anthropic SDKs are Stainless-generated and happen to name their abort class APIUserAbortError, so this check covers auth_type=anthropic only by coincidence of the shared name. The codebase's established depth for this is the provider boundary: openaiContentGenerator/pipeline.ts already converts its inactivity-timeout abort into new Error('Aborted') with name='AbortError' so the generic check catches it. — Failure scenario: a future provider SDK (or a Stainless rename) whose abort class differs would not match the hardcoded string; user cancels on that path would be classified 'unknown' in classifyRetryError (retries don't stop) and logged as api_errors — the exact regression this PR fixes. The same class of defect already exists unfixed on the Anthropic path this diff doesn't touch.

Suggested fix: normalize the SDK abort at the provider boundary (e.g. in the openai/anthropic pipeline catch, when abortSignal.aborted and the error is the SDK's APIUserAbortError via a real instanceof import, rethrow a standard Error named 'AbortError'), keeping isAbortError provider-agnostic; or generalize this check so it isn't pinned to one SDK's class name.

中文说明

这里把一个 provider 专属的 SDK 细节(OpenAI SDK 的类名 APIUserAbortError)硬编码进了与 provider 无关的 isAbortError。OpenAI 和 Anthropic 两个 SDK 都由 Stainless 生成,恰好都把中止类命名为 APIUserAbortError,所以这个判断只是因类名相同而碰巧覆盖了 auth_type=anthropic。代码库中既有的恰当层次是 provider 边界:openaiContentGenerator/pipeline.ts 已经把它的空闲超时中止转换成 name='AbortError' 的 new Error('Aborted'),从而被通用判断捕获。 — 失败场景:未来某个 provider SDK(或 Stainless 重命名)的中止类名不同时,将不会匹配这个硬编码字符串;该路径上的用户取消会在 classifyRetryError 中被标为 'unknown'(重试不会停止)并被记录为 api_error —— 正是本 PR 要修复的回归。同一类缺陷在本 PR 未触及的 Anthropic 路径上已经存在且未修复。

建议修复:在 provider 边界规范化该 SDK 中止(例如在 openai/anthropic 的 pipeline catch 中,当 abortSignal.aborted 且错误是 SDK 的 APIUserAbortError 时,通过真实的 instanceof 导入,重新抛出 name='AbortError' 的标准 Error),让 isAbortError 保持与 provider 无关;或者把这个判断泛化,使其不绑定到某一个 SDK 的类名。

— qwen3.8-max-preview via Qwen Code /review (v0.21.3)

[qwen-code-ci-bot]

[Suggestion] The new constructor-name match is pinned only on its positive side; no test asserts that sibling OpenAI SDK error classes (e.g. APIConnectionError, RateLimitError) still return false. — Failure scenario: a plausible future edit broadening the match — e.g. error.constructor?.name.startsWith('API') or .includes('API') — survives the entire suite: the new positive test stays green ('APIUserAbortError'.startsWith('API')), and every existing negative test uses Error/TimeoutError/ECONNREFUSED, none of whose constructor names start with API. Meanwhile new APIConnectionError(...) distinguishes the mutant: transient provider network/rate-limit errors on auth_type=openai would be classified as user aborts — classifyRetryError returns {kind:'abort'}, retries stop, and the error is logged as a cancellation instead of a retryable api_error.

Suggested fix:

it('should return false for other OpenAI SDK errors (e.g. APIConnectionError)', () => {
  const error = new APIConnectionError({ message: 'Connection error.' });
  expect(isAbortError(error)).toBe(false);
});

中文说明

新增的按类名匹配只固定了正向一侧;没有测试断言 OpenAI SDK 的兄弟错误类(如 APIConnectionError、RateLimitError)仍返回 false。 — 失败场景:未来一个合理的放宽匹配改动 —— 例如 error.constructor?.name.startsWith('API') 或 .includes('API') —— 会让整个测试套件仍然通过:新的正向测试仍为绿('APIUserAbortError'.startsWith('API')),而所有既有负向测试用的是 Error/TimeoutError/ECONNREFUSED,它们的类名都不以 API 开头。而 new APIConnectionError(...) 能区分这个变异:auth_type=openai 路径上瞬时的 provider 网络/限流错误会被当成用户中止 —— classifyRetryError 返回 {kind:'abort'},重试停止,该错误被记为取消而非可重试的 api_error。

建议修复:补充一个负向测试(见上方代码块)。

— qwen3.8-max-preview via Qwen Code /review (v0.21.3)

[wenshao]

Review

Verdict: the code change is correct and I'd take it — but the stated benefit is only partly real. The api_error telemetry entry the reporter of #8356 actually saw is not gated by isAbortError, so it will still be emitted after this PR. Details below.

What the PR does

Adds a third branch to isAbortError matching error.constructor.name === 'APIUserAbortError', plus two unit tests (one on isAbortError, one on classifyRetryError).

What I verified locally (openai 5.11.0, the pinned version in packages/core/package.json:86)

• The premise holds exactly as described: new APIUserAbortError({message}).name === 'Error', code === undefined, constructor.name === 'APIUserAbortError', prototype chain APIUserAbortError -> APIError -> OpenAIError -> Error. Pre-fix isAbortError returns false.
• constructor.name survives error redaction. This was my main worry, because pipeline.ts:254 explicitly warns that redaction prototype-clones errors. redactProxyErrorValue clones via Object.create(Object.getPrototypeOf(error)) (runtimeFetchOptions.ts:526), so the prototype — and therefore constructor.name — is preserved. Confirmed empirically. Good.
• openai is not in the esbuild external list, and the main CLI build sets keepNames: true and never sets minify, so the name is doubly safe there.

---

1. The telemetry claim doesn't hold — api_error is emitted on a different, ungated path

The PR body and #8398 both say shouldSuppressErrorLogging "logs/telemeters" the cancel as an api_error. It doesn't. shouldSuppressErrorLogging has exactly one consumer:

// errorHandler.ts:54
if (!this.shouldSuppressErrorLogging(redactedError, request)) {
  debugLogger.error('OpenAI API Error:', errorMessage, ...);
}

That gates a debug console log only. The qwen-code.api_error event with error_type=APIUserAbortError that #8356 reported comes from LoggingContentGenerator._logApiError → logApiError(...) → new ApiErrorEvent({ errorType: getErrorType(error) }), and getErrorType returns error.constructor.name (errors.ts:269-272) → "APIUserAbortError". Its three call sites (loggingContentGenerator.ts:405, :493, :799) are not gated on abort at all — they compute const aborted = req.config?.abortSignal?.aborted only to pick the span status message, then call safelyLogApiError unconditionally.

So after this PR the reporter of #8356 will still see the same telemetry entry. Two options, either is fine by me:

• Narrow the claim — reword the PR/issue to "suppresses the debug error log + fixes retry classification", drop the telemetry framing; or
• Finish the job — gate safelyLogApiError on isAbortError(error) && abortSignal.aborted (the span already distinguishes the aborted case, so the signal isn't lost). This is what would actually close the loop on what was reported.

I'd lean toward the second, since the isAbortError fix is a prerequisite for it and the whole point of the PR is the user-cancel-is-not-an-error story.

2. Mechanism: constructor.name vs instanceof vs boundary normalization

You asked for a call on the mechanism. I'm fine with constructor.name here, but two things to note:

• keepNames isn't a repo-wide guarantee. packages/vscode-ide-companion/esbuild.js:185 bundles with minify: production and no keepNames, and externals only vscode — so core's errors.ts gets minified there. The companion doesn't drive OpenAI requests today, so no live break, but "the build preserves class names" is true of the CLI bundle specifically, not of every bundle in this repo. Worth softening the code comment to say the CLI bundle (esbuild.config.js:241,267).
• This is the inverse of the existing convention. Everywhere else, the producer normalizes: pipeline.ts:254-256 builds a plain Error and sets name = 'AbortError' specifically so isAbortError matches, and concurrencyLimiter.ts:19-25 documents the same. This PR introduces a second convention (the shared util sniffs a foreign SDK's class name). Centralizing is defensible — the SDK error escapes from many pipeline call sites, so normalizing at the boundary would mean touching several catches — but it's a convention change worth an explicit maintainer ack, not a silent one.

Minor: constructor?.name matches the exact class only, not subclasses (verified: subclassing yields constructor.name === 'Sub'). Irrelevant in practice — nothing subclasses it.

3. JSDoc not updated

errors.ts:19-22 still reads "This handles both DOMException-style AbortError and Node.js abort errors." The new branch adds a third shape; the doc comment above the function should mention it (the inline comment inside the body is good, but the JSDoc is what shows in editor hovers).

4. Test nits

• expect(error.name).toBe('Error') pins an SDK internal. If OpenAI ever assigns .name = 'APIUserAbortError', this test goes red even though the production behavior is unchanged and correct. expect(error.name).not.toBe('AbortError') captures the actual requirement ("the existing branches can't match it") without the brittleness.
• Missing the caller-level regression test. Both new tests sit at the util layer. The bug being fixed is about caller behavior; a test in openaiContentGenerator.test.ts asserting shouldSuppressErrorLogging(new APIUserAbortError(...), {config:{abortSignal: abortedSignal}}) === true would lock in the thing users care about and would survive a future refactor of the detection mechanism. (And if you take suggestion pre-release: fix ci #1's second option, a test that no ApiErrorEvent is emitted on user cancel.)

5. Behavioral note on the retry side (not a blocker)

kind: 'abort' carries diagnosis: 'fail-fast' and hits the authoritative throw error at retry.ts:348. That's the intended effect for user cancels, but it also means any internally generated APIUserAbortError now fails fast where it previously fell through as unknown. The one internal aborter on this path is the idle watchdog (pipeline.ts:260 abortRequest()), and there the StreamInactivityTimeoutError wins the Promise.race while the orphaned next() rejection is explicitly swallowed (pipeline.ts:276-278) — so no practical regression. Just flagging that the blast radius of unknown → abort is retry control, not only labelling.

6. Pre-existing gaps, out of scope

• isAbortError doesn't walk .cause, so an abort wrapped in APIConnectionError still misses — note getTransportCode in the same classifier does walk causes (retryErrorClassification.ts:204-215).
• Axios-style CanceledError is recognized by isRetryAbortError but not by isAbortError, so shouldSuppressErrorLogging still misses it.

---

Summary

• Correctness: ✅ verified against the pinned SDK, including through the redaction clone path.
• Scope/claim accuracy: ⚠️ the telemetry benefit is not delivered — please either fix the logApiError gate or correct the PR/issue wording.
• Convention: ⚠️ consumer-side class-name sniffing inverts the existing producer-normalizes pattern; fine by me with an explicit ack, and please scope the keepNames comment to the CLI bundle.
• Tests: solid RED→GREEN at the util layer; would like one caller-level test at the layer the bug is actually about.
• Security/perf: no impact — pure function, one added string comparison on an error path.

[qwen-code-ci-bot]

⚠️ Downgraded from Approve to Comment: CI failing: Post Coverage Comment, Integration Tests (CLI, No Sandbox), Test (macos-latest, Node 22.x), Test (windows-latest, Node 22.x), review-pr, review-config. Reviewed. Test Plan (not a blocker): src/utils/errors.test.ts — no such file or directory; src/utils/retryErrorClassification.test.ts — no such file or directory; Tests 2 passed — this review observed 18994 passed.

中文说明

⚠️ 已从批准降级为评论：CI failing: Post Coverage Comment, Integration Tests (CLI, No Sandbox), Test (macos-latest, Node 22.x), Test (windows-latest, Node 22.x), review-pr, review-config。 已审查。 Test Plan（非阻断）：src/utils/errors.test.ts — no such file or directory; src/utils/retryErrorClassification.test.ts — no such file or directory; Tests 2 passed — this review observed 18994 passed。

— qwen3.8-max-preview via Qwen Code /review (v0.21.3)

[harjothkhara]

@qwen-code /review

CI is green on this commit — the checks flagged as "failing" were a first CI run cancelled by concurrency ~37s in, immediately superseded by a passing run (Test (ubuntu, Node 22) and web-shell E2E Smoke both succeeded; macOS/Windows/Integration are merge_group-gated and skip on the PR). Re-running the review against the current status.

[wenshao]

@qwen-code /review

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 33 passed · 0 failed · 33 total

中文 — 判定：✅ 通过 · 可合入（agent 判定）

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。

脚本断言：33 通过 · 0 失败 · 33 总计

Verification report

Verification report — PR #8399 fix(core): recognize OpenAI SDK APIUserAbortError as an abort

Verdict: merge-ready — 33/33 scripted assertions passed, 0 unexpected failures.
Verified head: 2fad2fde5525a268feed938ce1fac5bbd959bd62 (merge-ref base tip 2d2bdab2b25f1483858c9a3c3f118b1d007aa8ff).
Assertion totals: {"pass": 33, "fail": 0, "total": 33} (see assertions.json; inner harness comparisons are folded into their run's count check, see Methodology).

中文摘要

• 结论: merge-ready。33/33 脚本化断言通过,0 个意外失败。
• A/B 结论: 中心论断成立且 load-bearing。基线(2d2bdab)+ PR 测试 = 3 failed | 80 passed(失败恰为 PR 要修的行为:isAbortError 返回 false、分类 kind:'unknown');head(2fad2fd)= 83 passed。对编译产物的真实 SDK 错误矩阵:head 25/25(识别 openai 与 anthropic 的 APIUserAbortError、抑制日志、分类 abort),基线外科控制 25/25(全部呈现 PR 描述的误行为)。重试循环真实调用点:基线在宽松自定义 shouldRetryOnError 下对用户取消重试 3 次,head 第 1 次即短路;默认谓词与 503 重试控制两臂一致(无回归)。
• 机制证明: 生产 bundle(dist/cli.js + chunks)含 3 处 __name(this, "APIUserAbortError")(esbuild keepNames 标记);以仓库 esbuild 参数复刻的探针 bundle 在 keepNames 下(无论是否 minify)保留类名且识别正确;去掉 keepNames + minify 则类名被改写、识别静默失效(控制臂,预期失效)。
• Findings: 无阻塞项。三条低优先级注记:① keepNames 是 load-bearing 但无 CI 门禁覆盖 bundle 路径(单测跑源码),未来构建配置改动可能静默回归;② instanceof Error 守卫无测试钉住(变异存活,行为正确,属覆盖缺口);③ 任何同名类都会匹配(名称匹配机制的固有边界,与 getErrorType 既有机制一致)。另有一处描述修正:PR 正文 "After" 片段数字过期(写 2 passed | 79 skipped,实测 3 passed | 80 skipped)。
• 未覆盖: 真实网络中断流取消的 E2E(本验证复现的是 SDK 抛出的错误对象,即 wire shape,而非触发源);CLI 包套件与全仓套件未跑(仅 targeted);per-commit 归因不可达(shallow checkout,仅 HEAD^2 可达,与快照 commits 数组不符)。

Central claim and A/B

Central claim: isAbortError returns true for the OpenAI/Anthropic SDK APIUserAbortError (user cancel), flipping downstream behavior — retry classification 'unknown' → 'abort' (the no-retry short-circuit at retry.ts:348) and error-log suppression in shouldSuppressErrorLogging (openaiContentGenerator.ts:62) — while leaving every existing abort/non-abort path unchanged.

Cell	Environment	Oracle	Result
base 2d2bdab + PR test files	scratch worktree, vitest (source)	2 changed suites	3 failed | 80 passed — the two positive isAbortError tests and the retry-classification test fail with the intended behavioral mismatch (expected true to be false; expected {kind:'abort'} got kind:'unknown')
head 2fad2fd	vitest (source)	same	83 passed (01-ab-red-green.png)
base dist control (head dist with only the PR hunk reverted; diff -rq = exactly 1 file)	01-abort-matrix.mjs vs compiled dist, real SDK errors	25 scripted checks	25/25 as encoded: isAbortError(abort)=false, classify kind:'unknown', suppression false
head dist	same harness	25 checks	25/25: true / {kind:'abort',diagnosis:'fail-fast',reason:'aborted'} / suppressed; siblings (APIConnectionError, RateLimitError(429)), existing abort paths (DOMException AbortError, ABORT_ERR), and null/string/spoof boundary cells identical both arms (02-harness-ab-matrix.png)
retry loop, base	02-retry-loop.mjs, real APIUserAbortError, permissive custom shouldRetryOnError, no signal	attempt count	3 attempts — the base build retries a user cancel
retry loop, head	same	attempt count	1 attempt — abort-kind short-circuit fires (03-retry-loop-ab.png)
retry loop controls	both arms	default predicate → 1 attempt; 503 → 3 attempts	identical both arms (no regression)

The base-arm reds are encoded as expectations (control cells), so they count as passes in assertions.json.

Premise verified against real artifacts: new APIUserAbortError({message}) from locked openai@5.11.0 and @anthropic-ai/sdk@0.36.3 carries .name === 'Error', no .code, no .status — neither pre-existing check can match it; both SDKs share the constructor name, so one clause covers both providers (confirmed empirically, not from the description).

Corrections

• Stale "After" numbers in the PR description (correction to the description, not the code): the Reviewer Test Plan's "After" snippet shows 2 passed | 79 skipped (81); at the verified head the exact command yields 3 passed | 80 skipped (83) — the Anthropic-pinning test added in commit 2 also matches the -t "APIUserAbortError" filter. Behavior is confirmed; only the quoted counts predate the second commit.

Findings (no blockers)

1. keepNames: true is load-bearing and unguarded by any test (Suggestion). The recognition mechanism is error.constructor?.name === 'APIUserAbortError'; both SDKs are bundled (packages: 'bundle', not in the external list), so the name survives only via esbuild keepNames: true. Proofs: the real production bundle carries 3 __name(this, "APIUserAbortError") markers; a probe entry bundled with the repo's mainBuild flags passes 5/5 with keepNames (with and without --minify); the same bundle with --minify and without --keep-names renames the classes (re, me) and isAbortError returns false — silent breakage (04-bundle-keepnames-probes.png). Unit tests run from source, so a future build-config change (enabling minify or dropping keepNames) would regress abort recognition in the shipped bundle with the whole suite still green. The PR comment documents the dependency; a bundled-path smoke assertion (or a comment in esbuild.config.js) would pin it. Not a merge condition — the flag is present today and both current bundle modes work.
2. Coverage gap: the instanceof Error guard on the new clause is unpinned (Suggestion). Mutation M2 (guard removed) survives the PR suite (83/83) but is killed by the harness: a non-Error spoof {constructor:{name:'APIUserAbortError'}} flips false → true under the mutant (06-mutation-noguard-survivor.png). Behavior at head is correct; a one-line negative test would pin it. Completeness reporting, not a merge condition.
3. Name-match boundary: any Error subclass literally named APIUserAbortError matches (informational). A locally defined class of that name returns true at head. This is inherent to the constructor-name mechanism the codebase already uses in getErrorType (the PR comment cites it), and its blast radius is limited to log suppression and classification labels. Acceptable, documented tradeoff.

Mutation matrix (positive controls quoted beside survivors):

Mutant	Suite that should catch it	Result
M0 clause deleted (= base)	the 3 new tests	killed — 3 failed with intended assertions (01-ab-red-green.png)
M1 match broadened to startsWith('API')	the APIConnectionError negative test	killed — exactly 1 failed | 82 passed, failing test is the negative, on expected true to be false (05-mutation-broaden-killed.png)
M2 instanceof Error guard removed	(none in PR suite)	survives PR suite (83 passed); killed by harness spoof cells → coverage gap, not dead code

The anthropic-pinning test cannot be killed by any single-point production mutant without also killing the openai test (both SDKs traverse the one shared clause); its value is intent documentation, and M1 proves the suite can go red for the right reason.

Not covered

• Live end-to-end cancel: no run against a real OpenAI-compatible endpoint with a mid-stream user cancel. The harnesses reproduce the wire shape the SDK throws (APIUserAbortError constructed from the locked SDKs), not the network-side trigger that produces it; the suppression cell mirrors the 3-line shouldSuppressErrorLogging expression rather than driving the generator class.
• Full-repo and CLI-package suites: not run here (targeted core suites only — 28 files / 1709 tests green, incl. every isAbortError caller). The author's claim that CLI Session/FileCommandLoader suites are green was not independently re-run.
• Per-commit attribution: the checkout is shallow (grafted); only HEAD^2 is reachable while the metadata lists 2 commits, so commit 1's fix and commit 2's test pinning were verified as the aggregate HEAD^1..HEAD diff (commit 2's claim was additionally verified behaviorally via M1).
• Telemetry channel: the suppression boolean and classification kind are asserted at the function level; no api_error/api_retry telemetry event was captured end-to-end.
• Anthropic runtime path beyond class-name identity (no Anthropic API traffic).

Methodology

Environment: node:22-bookworm CI container, Node v22.23.2, locked openai@5.11.0 / @anthropic-ai/sdk@0.36.3 / vitest 3.2.4; head build pre-existing (dist/ compiled). A/B base arm 1 = scratch git worktree at HEAD^1 (2d2bdab) with the PR's two test files copied in (vitest runs source; openai resolves to the shared root node_modules — asserted, and packages/core has no @qwen-code/* deps, so no workspace-symlink confound); base arm 2 = byte-exact copy of head's compiled dist with only the PR hunk reverted (diff -rq = 1 file). Harnesses (01-abort-matrix.mjs, 02-retry-loop.mjs, bundle-probe-entry.ts, 90-final-tally.sh) are mock-free: real SDK error instances against compiled dist, real retryWithBackoff call site, and esbuild probes replicating esbuild.config.js mainBuild flags (packages:'bundle', keepNames, platform/target, esbuild-shims.js inject). Bundle corroboration: static grep of the real dist/ for __name markers + runtime probe. Raw logs in logs/ (01–13), captures in evidence/ (01–06), tallied by 90-final-tally.sh whose 33 checks each encode their expectation (expected base-arm reds count as passes; inner harness comparisons are folded into their run's inner N/N count check). Scratch worktrees removed after capture; repo git status clean.

Evidence images

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

— Qwen Code · sandboxed verification

[github-actions]

_Qwen Code review request accepted. Review is queued in [workflow run](https://github.com/QwenLM/qwen-code/actions/runs/31243170056)._

[qwen-code-ci-bot]

⏸️ Deferring to @wenshao — the re-run at 2fad2fde re-confirms the fix is correct, minimal, and green on CI, but your review's open items are unresolved: the telemetry benefit still isn't delivered by the diff (safelyLogApiError is ungated and the PR body's claim is unchanged), and the JSDoc / keepNames-scoping / caller-level-test asks are untouched. Needs your call: accept with a narrowed claim, or ask the author to gate the telemetry path.

⏸️ 交还 @wenshao —— 针对 2fad2fde 的复跑再次确认修复正确、最小且 CI 全绿，但你评审中的未决事项仍未解决：遥测收益仍未由 diff 交付（safelyLogApiError 未设门、PR 正文论断未变），JSDoc / keepNames 范围限定 / 调用方层测试也未处理。需要你的裁决：收窄论断后接收，或请作者给遥测路径设门。

— Qwen Code · qwen3.8-max

_{Reviewed at 2fad2fde5525a268feed938ce1fac5bbd959bd62 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Triage re-run completed without a new review.

⚠️ The bot's only review on 2fad2fde5525a268feed938ce1fac5bbd959bd62 is a COMMENTED one, which carries no vote — so it has no verdict of its own on this commit, and main needs two approving reviews: an approval left by another account is a separate vote and does not count as the bot's own. Two different things look like this, and the stage-3 comment above says which: the triage skill deferring on purpose at 3/5 — a fork refactor hitting the approval guardrail, or a core change escalated for maintainer awareness, both normal outcomes — or an earlier approval that a push dismissed, leaving only the comment behind, which needs a fresh review.

⚠️ 机器人在 2fad2fde5525a268feed938ce1fac5bbd959bd62 上唯一的评审是 COMMENTED，不带票 —— 因此它在该 commit 上没有自己的裁决，而 main 需要两个批准（其他账号的批准是另一张票）。有两种情况长这样，上方的 stage-3 评论会说明是哪一种：triage skill 在 3/5 时有意 defer（fork refactor 命中审批护栏，或核心改动被升级交由维护者把关，两者都是正常结果）；或者更早的批准被一次推送作废、只剩下这条评论，此时需要重新评审。

The stage comments above were updated with the latest result. View workflow run.

上方各阶段评论已更新为最新结果。查看工作流运行。

[qwen-code-ci-bot]

⚠️ Downgraded from Approve to Comment: CI failing: Post Coverage Comment, Integration Tests (CLI, No Sandbox), Test (macos-latest, Node 22.x), Test (windows-latest, Node 22.x), review-config. Reviewed. Test Plan (not a blocker): src/utils/errors.test.ts — no such file or directory; src/utils/retryErrorClassification.test.ts — no such file or directory; Tests 2 passed — this review observed 18994 passed.

中文说明

⚠️ 已从批准降级为评论：CI failing: Post Coverage Comment, Integration Tests (CLI, No Sandbox), Test (macos-latest, Node 22.x), Test (windows-latest, Node 22.x), review-config。 已审查。 Test Plan（非阻断）：src/utils/errors.test.ts — no such file or directory; src/utils/retryErrorClassification.test.ts — no such file or directory; Tests 2 passed — this review observed 18994 passed。

— qwen3.8-max via Qwen Code /review (v0.21.4)

[yiliang114]

Verified this locally against the current head, since the fork CI is still pending approval. The premise checks out: on openai 5.11.0, APIUserAbortError keeps .name === 'Error' and carries no code, and the bundled output preserves the class name (esbuild keepNames: true — __name(this, "APIUserAbortError") is present in the dist chunks), so the constructor-name match holds in the shipped build, not just under vitest. Ran the new tests plus the retry and openaiContentGenerator suites locally: 865 tests pass, tsc --noEmit clean.

One observation on the description, non-blocking: the stated goal is to stop cancels from being "logged / telemetered as api_error", but ApiErrorEvent is emitted unconditionally from loggingContentGenerator.safelyLogApiError (in the generateContent / generateContentStream catch blocks) and never routes through isAbortError — so that telemetry event still fires after this change. What this PR actually fixes is the OPENAI_ERROR debug-log line in EnhancedErrorHandler, the unknown → abort classification, and the fallback chain: before this change, a cancel landing during a fallback-model attempt would walk the remaining fallback models; now it throws immediately. All real improvements, just a narrower surface than the description implies — silencing the api_error event itself needs a separate gate in loggingContentGenerator, worth tracking as a follow-up.

Nice addition in the second commit pinning the negative case against APIConnectionError and the Anthropic path.

[harjothkhara]

Thanks both — the telemetry gap was the catch that mattered, and I've finished the job rather than narrowing the claim (ad700bf).

Telemetry (wenshao #1 / yiliang114's observation) — gated. You're right that the api_error event comes from safelyLogApiError, not shouldSuppressErrorLogging. Gated it there: when the caller's signal is aborted and the error is abort-shaped, it skips the event — the span's aborted status already records the cancellation, so the signal isn't lost. Threaded abortSignal through the three call sites in loggingContentGenerator. So #8356's reporter no longer gets the event. New caller-level regression test asserts no api_error fires on a user cancel; I verified it goes red with the gate removed (spy called once), and a contrast test confirms a genuine failure still reports.

Mechanism / convention (#2) — explicit ack. Agreed this inverts the producer-normalizes pattern (pipeline.ts:254 setting name = 'AbortError'). I kept the consumer-side class-name match for the reason you gave — the SDK error escapes from many pipeline catches, so normalizing at each boundary would touch several of them — but flagging it as a deliberate convention choice for you to accept, not a silent one. Scoped the keepNames comment to the CLI bundle and noted vscode-ide-companion minifies without it (no live break — it doesn't drive SDK requests).

JSDoc (#3) / test nits (#4) — done. JSDoc now names the third shape. Both expect(error.name).toBe('Error') → .not.toBe('AbortError'), so the tests pin the requirement rather than an SDK internal.

Retry blast radius (#5) / pre-existing gaps (#6) — noted, untouched. The unknown → abort fast-fail is the intended effect and the idle-watchdog path is safe as you traced. isAbortError not walking .cause, and Axios CanceledError missing from it, are real but separate — I'd rather do those as a focused follow-up than widen this PR. Happy to pick them up.

packages/core errors + loggingContentGenerator + retry suites green (158 tests); prettier/eslint/tsc clean.

[qwen-code-ci-bot]

Reviewed — no blockers. Suggestions are inline.

中文说明

已审查——无阻断问题。 建议见行内评论。

— qwen3.8-max via Qwen Code /review (v0.21.3)

[qwen-code-ci-bot]

[Suggestion] The gate's truth table is only half pinned — both single-conjunct mutants survive the whole suite (measured). — Failure scenario: the mutant if (isAbortError(error)) return; (drops the signal condition) passes the directory suite 75/75 — a network-induced abort the user did not cause would then silently lose its api_error entry; the mutant if (abortSignal?.aborted) return; (drops the error-shape condition) also passes 75/75 — a genuine 401/429 racing a user cancel would silently drop from telemetry. The sibling predicate shouldSuppressErrorLogging pins both boundary cases in openaiContentGenerator.test.ts (should return false for AbortError when signal is NOT aborted (network abort) / should return false for non-AbortError even when signal is aborted); this gate pins neither.

Suggested tests:

// abort-shaped error, signal NOT aborted -> still reported
// reject with new APIUserAbortError({ message: 'Request was aborted.' })
// and a request config whose abortSignal is NOT aborted
expect(logApiError).toHaveBeenCalledTimes(1);

// aborted signal, non-abort-shaped error -> still reported
// the pre-existing aborted-partial-stream test (~line 1731) already drives
// this combination; one added assertion there pins it
expect(logApiError).toHaveBeenCalledTimes(1);

中文说明

门控的真值表只固定了一半——两个"单条件"变异体都能在整个套件中存活（已实测）。 — 失败场景：变异体 if (isAbortError(error)) return;（去掉 signal 条件）在目录套件 75/75 全绿——用户未触发的网络中止会静默丢失 api_error 遥测条目；变异体 if (abortSignal?.aborted) return;（去掉错误形态条件）同样 75/75 全绿——与用户取消竞态发生的真实 401/429 会静默地从遥测中消失。姊妹判断 shouldSuppressErrorLogging 在 openaiContentGenerator.test.ts 中固定了这两个边界用例（"signal 未中止时 AbortError 不抑制"/"signal 已中止但非 AbortError 不抑制"）；本门控两者都未固定。

建议补充两个测试（见上方代码块）：中止形态错误 + 未中止的 signal → 仍应上报；已中止的 signal + 非中止形态错误 → 仍应上报（约 1731 行既有的 aborted-partial-stream 测试已驱动后一组合，补一条断言即可固定）。

— qwen3.8-max via Qwen Code /review (v0.21.3)

[qwen-code-ci-bot]

[Suggestion] The streaming call sites of the cancel gate are untested. — Failure scenario: the gate is only tested through the non-stream generateContent path; a mid-stream Escape cancel (the most common cancel UX) surfaces as APIUserAbortError inside the for await and hits the iteration call site (~line 825), which no test covers. If a future refactor drops req.config?.abortSignal at either streaming call site, the gate's first conjunct becomes undefined and the qwen-code.api_error noise this PR fixes silently returns for stream cancels while the suite stays green (this class has exactly one test consumer, verified by grep). Probe-verified: appended stream tests pass with the gate in place and flip to failing when it is removed.

Suggested test:

// generateContentStream whose wrapped stream throws
// new APIUserAbortError({ message: 'Request was aborted.' })
// with an aborted config.abortSignal
expect(logApiError).not.toHaveBeenCalled();

中文说明

取消门控的流式调用点没有测试覆盖。 — 失败场景：门控目前只通过非流式 generateContent 路径被测到；流式输出中途按 Escape 取消（最常见的取消交互）会以 APIUserAbortError 从 for await 中抛出，命中约 825 行的迭代调用点，而该点没有任何测试覆盖。若未来重构在任一流式调用点漏传 req.config?.abortSignal，门控的第一个条件变为 undefined，本 PR 修复的 qwen-code.api_error 噪声会在流式取消场景下静默回归，而测试套件仍然全绿（已用 grep 确认该类只有一个测试消费方）。已用探针验证：补充的流式测试在门控存在时通过、移除门控后变红。

建议补充一个测试（见上方代码块）：generateContentStream 的包装流抛出 APIUserAbortError 且 config.abortSignal 已中止，断言 logApiError 未被调用。

— qwen3.8-max via Qwen Code /review (v0.21.3)

[qwen-code-ci-bot]

Test Plan (not a blocker): src/tools/read-file.test.ts — no such file or directory; src/tools/zoom-image.test.ts — no such file or directory; 371 tests green — this review observed 18571, 19521, 1124, 1466, 481, 2941, 454 passed; Tests 19297 passed — this review observed 18571, 19521, 1124, 1466, 481, 2941, 454 passed.

中文说明

Test Plan（非阻断）：src/tools/read-file.test.ts — no such file or directory; src/tools/zoom-image.test.ts — no such file or directory; 371 tests green — this review observed 18571, 19521, 1124, 1466, 481, 2941, 454 passed; Tests 19297 passed — this review observed 18571, 19521, 1124, 1466, 481, 2941, 454 passed。

— qwen3.8-max via Qwen Code /review (v0.21.7)

[qwen-code-ci-bot]

[Critical] The seventh unconverted deadline producer: the sibling goal checkpoint verifier (packages/core/src/goals/goal-checkpoint-verifier.ts:156-160) still aborts its 30s deadline with a plain new Error(...), so isUserCancel reads its timeouts as user cancels and this diff's new gates suppress its api_error telemetry and debug log — the exact misclassification this hunk fixes for goal-verifier.ts. — Failure scenario: a goal checkpoint verification LLM call exceeds its 30s budget mid-request (wired in production at config.ts:7430, invoked from goal-runtime.ts:822; the signal composes via AbortSignal.any and routes through runSideQuery → baseLlmClient → LoggingContentGenerator) → timeoutController.abort(new Error(...)) fires with reason .name === 'Error' → the SDK rejects abort-shaped → isUserCancel returns true → qwen-code.api_error is skipped and the debug log is suppressed. Before this diff the same timeout was reported; the diff newly hides it behind a clean model-health chart. Probe-verified: plain-Error reason → isUserCancel true; with timeoutAbortReason(...) → false.

Fix: mirror this conversion in goal-checkpoint-verifier.ts (import timeoutAbortReason from ../utils/errors.js):

timeoutController.abort(
  timeoutAbortReason(`Goal checkpoint verifier timed out after ${timeoutMs}ms`),
);

plus a reason-shape test mirroring goal-verifier.test.ts.

中文说明

[Critical] 第七个未转换的超时产生方：孪生的 goal checkpoint verifier（packages/core/src/goals/goal-checkpoint-verifier.ts:156-160）仍以普通 new Error(...) 中止其 30 秒截止时限，isUserCancel 会把它的超时判为用户取消，本 diff 新增的门控因此抑制其 api_error 遥测与调试日志——正是此处为 goal-verifier.ts 修复的同一误判。——失败场景：goal checkpoint 校验的 LLM 调用在请求进行中超出 30 秒预算（生产环境经 config.ts:7430 装配、由 goal-runtime.ts:822 调用；signal 经 AbortSignal.any 合成并沿 runSideQuery → baseLlmClient → LoggingContentGenerator 传入）→ timeoutController.abort(new Error(...)) 以 .name === 'Error' 的 reason 触发 → SDK 以中止形态拒绝 → isUserCancel 返回 true → qwen-code.api_error 被跳过、调试日志被抑制。本 diff 之前同一超时是会上报的；合并后它会隐藏在干净的模型健康图表背后。已用探针验证：普通 Error reason → isUserCancel 为 true；改用 timeoutAbortReason(...) 后 → false。

修复：在 goal-checkpoint-verifier.ts 中镜像此转换（从 ../utils/errors.js 导入 timeoutAbortReason），并补充一个与 goal-verifier.test.ts 对应的 reason 形态测试（代码见上方英文代码块）。

— qwen3.8-max via Qwen Code /review (v0.21.7)

[harjothkhara]

@wenshao — when you have a moment, could you take a look at the direction here? This started as the small #8398 fix (recognize APIUserAbortError) but grew, across several automated-review rounds, into a broader invariant: internal deadlines that reach a model request must signal TimeoutError so a timed-out side-query is not misclassified as a user cancel and silently dropped from telemetry. That is now a shared isUserCancel predicate plus timeoutAbortReason applied to seven producers.

The automated reviews have been thorough on correctness, but no human has weighed in on the scope — whether this cross-cutting invariant belongs in one PR, or should be split (recognition first, the invariant second). I wrote it up in docs/design/2026-08-08-user-cancel-vs-internal-deadline.md, including the honest limits (the invariant is convention, not type-enforced) and the full isAbortError blast radius. That doc is the fastest way to judge it.

To be clear on linkage: this fixes #8398 and the api_error noise; it does not fix #8356's transcript-write blackout (a separate recorder matter), hence Refs #8398, not Closes. No rush — mainly want a maintainer's read on the invariant before it goes further.

_{@wenshao 有空时能否看下整体方向?这个 PR 由 #8398 的小修复(识别 APIUserAbortError)扩展为一个更大的约定:到达模型请求的内部超时必须以 TimeoutError 形态中止,以免被误判为用户取消而静默丢弃遥测。现为共享的 isUserCancel 谓词加 timeoutAbortReason,应用于七处产生方。设计说明见 docs/design/2026-08-08-user-cancel-vs-internal-deadline.md。它修复 #8398 与 api_error 噪声,但不修复 #8356 的转写丢失(另一问题),故为 Refs 而非 Closes。主要想在继续之前听听维护者对该约定与拆分与否的意见。}

[wenshao]

Thanks for writing the design doc — it made this reviewable, and the "Limits"
section is where I'd like to start, because I think it understates its own point.

My read: split it, but not for size reasons

There are two things in this PR:

1. The bug fix (isAbortError does not recognize the OpenAI SDK's APIUserAbortError — user cancels in auth_type=openai are misclassified #8398). isAbortError missing APIUserAbortError, and
   safelyLogApiError having no abort gate at all. No disagreement — that
   should land.
2. The cross-cutting invariant. isUserCancel + timeoutAbortReason and
   the seven producer conversions across goals / hooks / agent runtime / CLI
   voice.

I do want these split, but the reason isn't diff size. It's that the invariant's
polarity isn't settled yet, and once it merges it becomes very hard to flip.

The polarity is the decision that matters

isUserCancel reads: abort-shaped, and the reason is not TimeoutError
→ user cancel → suppress. So the default is suppress, and an internal
deadline has to opt in to being reported. The failure mode is silently
dropped telemetry, with nothing enforcing it at compile time.

Your doc says exactly this and calls it convention-not-type. I'd go further —
the evidence in this PR argues the default is on the wrong side:

• It took nine review rounds to find seven producers, and round 9's sweep still
  missed the checkpoint verifier, because it aborts with new Error(...) rather
  than a bare abort().
• The idiom is still being written fresh on main:
  packages/cli/src/ui/hooks/useGeminiStream.ts:2864 does
  timeout.abort(new Error(MID_TURN_AT_COMMAND_RESOLVE_TIMEOUT_MESSAGE)) —
  the exact anti-shape. It happens not to reach a model request
  (resolveAtCommandQuery only resolves files), so your audit claim holds. But
  it shows the shape keeps reappearing.
• packages/core/src/hooks/combinedAbortSignal.ts still exports
  createCombinedAbortSignal, which aborts bare on timeout and duplicates
  combineAbortSignals in utils/abortController.ts. Currently referenced only
  by its own test, so it's not a live producer — but it's a loaded gun for the
  next person who wraps a model request with it.

The opposite polarity — suppress only aborts explicitly tagged as a user
cancel — inverts both properties. The cancel-producer set is closed and stable
(TUI turn cancel, ACP/daemon, webui) rather than open and growing, and one of
them is already tagged: USER_CANCEL_ABORT_REASON = 'qwen:user-cancel' at
packages/cli/src/acp-integration/session/Session.ts:316. Fewer sites to
convert, and a missed tag costs one spurious api_error — i.e. the #8398 noise
— instead of a silent hole in telemetry.

Your stated reason for negative-only is to avoid enumerating cancel shapes. But
enumeration is what makes the invariant enforceable. Since it can't be a type,
the default needs to be the safe side.

Specific points

• error.constructor?.name === 'APIUserAbortError'. The comment leans on
  keepNames: true in esbuild.config.js (confirmed, lines 241 and 267). But
  packages/vscode-ide-companion/esbuild.js:185,213 sets minify: production
  with no keepNames. You're right that it doesn't drive provider SDK requests
  today — it's just an invisible dependency for whoever changes that later.
  Since openai is already a core dependency, I'd prefer instanceof at the
  provider layer (or normalizing at the pipeline boundary) and keeping
  utils/errors.ts provider-agnostic.
• voice-refine.ts:54 launders the reason. onExternalAbort = () => controller.abort() drops the parent's reason. The PR keeps it bare on the
  grounds that it forwards a genuine user cancel — but the function can't know
  what the parent signal is. createChildAbortController already does
  child.abort(parent.reason) correctly. Small on its own; a good sample of why
  the hand-rolled idiom keeps going wrong.
• isAbortError blast radius. The doc lists the eight consumers, which I
  appreciate. classifyRetryError is fine (an expired budget shouldn't retry).
  The one I'd want confirmed separately is geminiChat.ts:3671 and :3755 —
  if (isAbortError(...)) throw means a broader match sends more errors down
  the rethrow path instead of the fallback.
• Reason propagation itself checks out. createChildAbortController and
  combineAbortSignals both forward reason, and AbortSignal.any() adopts
  the firing source's reason per spec. The mechanism works; my concern is only
  the default direction.

Concretely

Land #8399 as recognition + the telemetry gate only — isAbortError fixed,
plus the "approximately correct" isAbortError(error) && signal.aborted gate.
That stops the #8398 noise immediately. Internal deadlines stay suppressed under
it, but that is today's behaviour, not a regression.

Then the invariant as its own PR, where we settle the polarity first. Happy to
review that one on its own terms.

中文

先谢谢你写了设计文档——它让这个 PR 可评审了。我想从 "Limits" 那一节谈起，因为我认为它低估了自己提出的问题。

我的意见：该拆，但不是因为体量

这个 PR 里其实是两件事：

1. Bug 修复（isAbortError does not recognize the OpenAI SDK's APIUserAbortError — user cancels in auth_type=openai are misclassified #8398）：isAbortError 漏认 APIUserAbortError，以及 safelyLogApiError 根本没有任何 abort 门控。这部分没有异议，应该合。
2. 跨切面约定：isUserCancel + timeoutAbortReason，以及散落在 goals / hooks / agent runtime / CLI voice 的七处 producer 改造。

我确实希望拆开，但理由不是 diff 大小，而是这个约定的判别极性还没定下来——一旦合入就很难再翻转。

真正需要决定的是极性

isUserCancel 的规则是：abort 形态，且 reason 不是 TimeoutError → 判为用户取消 → 抑制上报。也就是说默认抑制，内部超时必须主动声明才会被上报。失败方向是静默丢遥测，而且编译期没有任何东西能强制它。

你的文档正是这么写的，并称之为「是约定而非类型」。我想再往前推一步——这个 PR 自身的证据说明默认值站错了边：

• 找齐七处 producer 用了九轮评审，而且第 9 轮的扫描仍然漏了 checkpoint verifier，因为它用的是 abort(new Error(...)) 而不是裸 abort()。
• 这个习语在 main 上仍在被新写出来：packages/cli/src/ui/hooks/useGeminiStream.ts:2864 就是 timeout.abort(new Error(MID_TURN_AT_COMMAND_RESOLVE_TIMEOUT_MESSAGE))，正是那个反例形态。它恰好不打模型请求（resolveAtCommandQuery 只做文件解析），所以你的审计结论成立——但它说明这个形态会反复出现。
• packages/core/src/hooks/combinedAbortSignal.ts 里还导出着 createCombinedAbortSignal，超时时裸 abort()，并且与 utils/abortController.ts 的 combineAbortSignals 重复。目前只有它自己的测试引用，所以不是活的 producer——但下一个人拿它去包模型请求就会直接复现这个 bug。

相反的极性——只抑制被显式标记为用户取消的 abort——把这两点都反了过来。取消方的集合是封闭且稳定的（TUI 回合取消、ACP/daemon、webui），而不是开放增长的；而且其中一处已经有标记了：packages/cli/src/acp-integration/session/Session.ts:316 的 USER_CANCEL_ABORT_REASON = 'qwen:user-cancel'。要改的点更少，而漏标的代价只是一条多余的 api_error——也就是 #8398 那种噪声——而不是遥测里一个静默的窟窿。

你选择 negative-only 的理由是避免枚举取消形态。但枚举恰恰是让这个约定可执行的前提。既然它无法成为类型，那默认值就必须落在安全的那一侧。

几个具体点

• error.constructor?.name === 'APIUserAbortError'。 注释依赖 esbuild.config.js 的 keepNames: true（已确认，241 和 267 行）。但 packages/vscode-ide-companion/esbuild.js:185,213 是 minify: production 且没有 keepNames。你说得对，它今天不驱动 provider SDK 请求——问题只是这成了一个对后来改动者不可见的依赖。既然 openai 已经是 core 依赖，我更倾向在 provider 层用 instanceof（或在 pipeline 边界归一化），让 utils/errors.ts 保持与 provider 无关。
• voice-refine.ts:54 洗掉了 reason。 onExternalAbort = () => controller.abort() 丢弃了父 signal 的 reason。PR 以「它转发的是真实用户取消」为由保持裸 abort——但这个函数无从知道父 signal 是什么。createChildAbortController 已经正确地做了 child.abort(parent.reason)。这一处本身影响很小，但它是「手写这个习语迟早出错」的好样本。
• isAbortError 的辐射面。 文档列出了八个消费方，这点很好。classifyRetryError 那条没问题（超时预算不该重试）。我希望单独确认的是 geminiChat.ts:3671 和 :3755 的 if (isAbortError(...)) throw——匹配放宽后会有更多错误走 rethrow 而不是 fallback。
• reason 传播机制本身是对的。 createChildAbortController 与 combineAbortSignals 都正确转发 reason，AbortSignal.any() 按规范采纳触发源的 reason。机制通的；我的顾虑只在默认方向。

具体建议

#8399 只保留识别 + 遥测门控——修好 isAbortError，加上「近似正确」的 isAbortError(error) && signal.aborted 门控。这能立刻止住 #8398 的噪声。在这个版本下内部超时仍会被抑制，但那是现状，不是回退。

约定单独开一个 PR，先把极性定下来。那个 PR 我很乐意单独评审。

[qwen-code-ci-bot]

Test Plan (not a blocker): src/tools/read-file.test.ts — no such file or directory; src/tools/zoom-image.test.ts — no such file or directory.

中文说明

Test Plan（非阻断）：src/tools/read-file.test.ts — no such file or directory; src/tools/zoom-image.test.ts — no such file or directory。

— qwen3.8-max via Qwen Code /review (v0.21.8)

[qwen-code-ci-bot]

[Critical] R11-1: The completeness audit misses the daemon-boundary deadline producer. The daemon prompt deadline (PromptDeadlineExceededError, fired in packages/acp-bridge/src/bridge.ts onDeadline) and the live-call turn timeout reach model requests laundered as the string reason 'qwen:user-cancel': the forwarded ACP cancel (forwardRunningPromptCancel) carries { sessionId } only — the cause never crosses the boundary — and the agent side aborts with USER_CANCEL_ABORT_REASON (Session.cancelPendingPrompt → pendingPrompt.abort('qwen:user-cancel')). isUserCancel then reads a string reason (not TimeoutError) and returns true, so the new gates suppress the api_error event, apiActivityTracker.recordError() and the debug log for a genuine deadline failure. Before this diff no gate existed, so the event was emitted — the suppression is newly introduced, the same regression class the five prior Criticals on this PR fixed. The deliberate exclusions above (ACP recovered-parent wait, runBudget) do not cover this: it is a deadline aborting an in-flight model request, not a planned interruption. Probe-verified: isUserCancel(APIUserAbortError, signal aborted with 'qwen:user-cancel') → true. — Failure scenario: an operator runs qwen serve with a prompt deadline (or an SDK client sends deadlineMs) and the deadline fires while a model request is in flight → the deadline kill is laundered to 'qwen:user-cancel' → api_error, model-health entry and debug log are all suppressed → an unattended daemon's LLM work dies on deadlines behind a clean model-health chart.

Suggested fix (spans files outside this diff, so no suggestion block): propagate deadline attribution across the ACP cancel boundary — include the cause in the forwarded cancel (forwardRunningPromptCancel / PROMPT_CANCEL_METHOD), and have Session.cancelPendingPrompt abort with timeoutAbortReason(...) for deadline-caused cancels while genuine client cancels keep 'qwen:user-cancel'. Alternatively, explicitly add the daemon prompt deadline to this exclusions list with a justification (the harder case — unlike runBudget it is not a planned interruption of a healthy request).

中文说明

[Critical] R11-1：完整性审计遗漏了 daemon 边界上的超时产生方。daemon 提示词截止时限（PromptDeadlineExceededError，在 packages/acp-bridge/src/bridge.ts 的 onDeadline 中触发）与实时通话轮次超时会以字符串 reason 'qwen:user-cancel' 的形态到达模型请求：转发的 ACP 取消（forwardRunningPromptCancel）只携带 { sessionId }——原因没有跨越边界——agent 一侧以 USER_CANCEL_ABORT_REASON 中止（Session.cancelPendingPrompt → pendingPrompt.abort('qwen:user-cancel')）。isUserCancel 因此读到字符串 reason（而非 TimeoutError）并返回 true，新门控便对真实的截止时限失败抑制 api_error 事件、apiActivityTracker.recordError() 与调试日志。本 diff 之前不存在门控、该事件原本会发出——抑制是新引入的，与本 PR 此前五个 Critical 所修复的回归类型相同。上文的有意排除项（ACP recovered-parent 等待、runBudget）并不覆盖此情形：这是一个中止进行中模型请求的截止时限，而非计划性中断。已用探针验证：isUserCancel(APIUserAbortError, signal aborted with 'qwen:user-cancel') → true。——失败场景：运维以提示词截止时限运行 qwen serve（或 SDK 客户端发送 deadlineMs），截止时限在模型请求进行中触发 → 该超时中止被洗成 'qwen:user-cancel' → api_error、模型健康记录与调试日志全部被抑制 → 无人值守 daemon 的 LLM 工作在干净的模型健康图表背后死于超时。

建议修复（跨本 diff 之外的文件，故不提供 suggestion 代码块）：在 ACP 取消边界传播截止时限归因——在转发的取消中携带原因（forwardRunningPromptCancel / PROMPT_CANCEL_METHOD），并让 Session.cancelPendingPrompt 对超时导致的取消以 timeoutAbortReason(...) 中止，真实的客户端取消保持 'qwen:user-cancel'。或者，在本排除列表中显式加入 daemon 提示词截止时限并给出理由（较难成立——与 runBudget 不同，它不是对健康请求的计划性中断）。

— qwen3.8-max via Qwen Code /review (v0.21.8)

[harjothkhara]

@wenshao — a specific semantic call I need your judgment on, from the round-11 review (R11-1).

The bot flagged that a daemon prompt deadline (qwen serve with a deadline, or an SDK client sending deadlineMs) is now suppressed by this PR's cancel gate. I verified the mechanism: onDeadline in acp-bridge forwards a generic cancel carrying only { sessionId }, so the deadline cause is dropped at the ACP wire; the agent re-stamps the model-facing signal 'qwen:user-cancel' at the Session admission boundary, so isUserCancel reads it as a cancel and skips the api_error / llmApiErrors for that attempt.

Crucially it is not a blackout: the deadline is still published as the prompt_deadline_exceeded terminal and the LLM span still ends errored. Only the provider-health error count is affected. And llmApiErrors is documented as "the provider-side failures" — a caller-configured deadline is a local interruption, so suppressing it there is arguably correct, the same shape as the existing runBudget exclusion.

So the question is genuinely yours to decide:

1. Legitimate exclusion — a local deadline should not count as a provider llmApiError; the canonical terminal already carries it. → I document it as an exclusion (already drafted in docs/design/2026-08-08-user-cancel-vs-internal-deadline.md) and we're done.
2. Real regression — a deadline-killed attempt should still count. → then it needs a cross-boundary fix (carry attribution via CancelNotification._meta → map to timeoutAbortReason at the Session stamp), which per AGENTS.md I'd land in this PR despite the scope growth.

This is the same wire-boundary escalation as my earlier scope question — it's the one producer that can't be fixed in-process. No rush; I'd rather have your read than guess.

_{@wenshao 需要你就 R11-1 做一个语义判断。守护进程 prompt 截止(qwen serve 设 deadline,或 SDK 传 deadlineMs)现被本 PR 的取消门控抑制:截止原因在 ACP wire 处丢失(仅传 {sessionId}),agent 端在 Session admission 边界把模型信号重标为 'qwen:user-cancel',故 isUserCancel 视其为取消,跳过该次的 api_error/llmApiErrors。但并非全丢:截止仍以 prompt_deadline_exceeded terminal 发布、LLM span 仍以错误结束,只影响 provider 健康错误计数。而 llmApiErrors 文档定义为「provider 侧失败」,调用方配置的截止属本地中断,故抑制或许是正确的(同 runBudget)。请裁定:(1) 合理排除 → 我按设计说明记录即可;(2) 真实回归 → 需跨边界修复(经 CancelNotification._meta 传递归因,在 Session 处映射为 timeoutAbortReason),按 AGENTS.md 我会在本 PR 内修复。这是我之前提的 wire 边界范围问题的同一处——唯一无法在进程内修复的 producer。}

[harjothkhara]

Thanks — this is the right call, and I've done the split.

This PR is now just the #8398 fix: isAbortError recognizes APIUserAbortError, and safelyLogApiError skips the event on the approximately-correct abortSignal?.aborted && isAbortError(error) gate. Net diff is 5 files, +372/−4 (production is 2 files). Internal timeouts stay suppressed under this gate — status quo, not a regression, as you said. I removed isUserCancel, timeoutAbortReason, the seven producer conversions, the Qwen override, and the design note.

On your point about polarity — you've convinced me the default is on the wrong side. The invariant PR will open with positive-only (suppress only explicitly-tagged user cancels), starting from the closed cancel-producer set (TUI / ACP-daemon / webui) and the existing 'qwen:user-cancel' tag, rather than negative-only + a growing producer list. Your evidence is the argument: nine rounds to find seven, and the daemon deadline I flagged separately is the same failure mode — it launders through 'qwen:user-cancel' at the Session admission boundary, so under negative-only it reads as a cancel with nothing enforcing otherwise. Positive-only turns that into a recoverable spurious api_error instead of a silent hole. That daemon-deadline question is better answered by the polarity decision than in isolation, so I'll fold it into the follow-up rather than leave it dangling here.

Your other specifics, carried to the invariant PR:

• instanceof over constructor.name. Agreed the class-name match is an invisible dependency for a rebundler without keepNames. I'll do instanceof APIUserAbortError at the provider layer (or normalize at the pipeline boundary) and keep utils/errors.ts provider-agnostic, per your steer. Kept constructor.name in this PR only because it's the already-validated minimal isAbortError does not recognize the OpenAI SDK's APIUserAbortError — user cancels in auth_type=openai are misclassified #8398 fix; happy to switch here too if you'd rather.
• voice-refine.ts reason wash and combinedAbortSignal.ts / useGeminiStream.ts:2864 — good catches; those belong in the invariant PR as producer/adapter cleanup (child.abort(parent.reason) etc.), not here.
• geminiChat.ts:3671 / :3755 rethrow. Worth confirming — broadening isAbortError does route more errors to rethrow vs fallback there. I'll verify that path explicitly in the invariant PR.

I'll open the invariant PR separately and tag you. Thanks for the thorough read.

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 47 passed · 0 failed · 47 total

中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。

脚本断言：47 通过 · 0 失败 · 47 总计

Verification report

Sandboxed verification: ⚠️ findings — 47/47 assertions passed; the reduced PR's code is load-bearing at the new base tip; the body's "pre-existing behaviour" characterization of the deadline suppression remains falsified by measurement (agent verdict) — follow-up round at head aa6eca291fbffe0f0a226610ebfc1201c85cde2a (PR head unchanged since the previous round; merge-ref base tip moved e20601d → d96de59)

Ran the PR in an isolated, token-free container: A/B against the new base build, mock-free harness assertions against the compiled dist, mutation matrix, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 47 passed · 0 failed · 47 total

中文 — 判定：⚠️ 有需评审关注的发现（agent 判定）

• 结论：findings。47/47 脚本化断言通过，0 个意外失败。本轮为跟进轮：PR head（aa6eca2）与上轮逐字节相同，base tip 从 e20601d 前进到 d96de59，所有上轮测量均在新 base 上重跑（重测而非比对旧报告）。中心论断（isAbortError 识别 APIUserAbortError + 用户取消时抑制 api_error）在源码层与编译产物层均再次证实 load-bearing。
• A/B 结论：基线 d96de59 + PR 测试 = 7 failed | 141 passed（失败恰为 PR 要修的行为）；head = 148 passed。编译产物 harness：错误矩阵 12/12 vs 12/12、遥测门控 11/11 vs 11/11（含两个内部超时探针 1→0）、重试循环 head 1 次 vs base 3 次（503 对照两臂一致）。变异矩阵 6 杀 1 存活（instanceof Error 守卫，spoof 探针可杀 → 覆盖缺口）。
• Findings：① 正文"内部超时保持被抑制是现状、非回归"的说法在新 base 上仍被测量证伪——基线在 api_error/模型健康计数器上曾记录内部超时失败（探针 1→0），head 起被抑制；该取舍是评审者指示的有意拆分，但表述不准确，需评审者明确确认。② 延续项：instanceof Error 守卫无套件钉住；keepNames load-bearing 且无 CI 门禁。③ 正文全套件数字（19575/3/11）在本容器不复现（环境差异），已用 A/A 等价性替代验证。
• 未覆盖：真实网络取消 E2E（复现的是 wire shape）；CLI 套件；per-commit 归因（shallow）。

Verification report

Verification report — PR #8399 (follow-up round at unchanged head aa6eca2, new base tip d96de59)

Verdict: findings — 47/47 scripted assertions passed, 0 unexpected failures. No code defect failed any check. The findings are one falsified claim in the PR text (re-measured at the new base), three carried-over suggestions, and one environmental note.
Verified head: aa6eca291fbffe0f0a226610ebfc1201c85cde2a (merge-ref base tip d96de59acf901e7c91c05e5db03a033fcb9ee2ec).
Assertion totals: {"pass": 47, "fail": 0, "total": 47}.

This is a follow-up round. The previous substantive round verified the same head aa6eca2 against base tip e20601d; since then only main moved (d96de59 = "feat(serve): bound daemon ACP NDJSON buffers (#8911)" and intermediates). Per the follow-up contract, every carried-forward measurement was re-run at the new base rather than diffed off the old report — the PR's input closure is byte-identical (git diff HEAD^1..HEAD is the same 5 files, +372/−4, content-identical to the previous round's screen.diff), but the base side of every A/B cell is new.

Previous-finding status table

#	Finding (round at aa6eca2 vs e20601d)	Severity	Status at aa6eca2 vs d96de59
1	Internal deadline failures stop reaching api_error telemetry / the model-API-health chart; the reduction's "status quo, not a regression" claim is false	Suggestion	stands — re-measured: deadline probes (controller+timer and AbortSignal.timeout, side-query prompt ids) record 1 at base d96de59 and 0 at head (02-gate-ab-telemetry-destination.png, D1/D2 rows)
2	instanceof Error guard on the new clause unpinned by the suite	Suggestion	stands — M4 (guard removed) survives all 148 tests at the new head; killed by the spoof probe with the intended assertion (05-mutation-matrix-and-keepnames.png)
3	Any locally declared class named APIUserAbortError matches, and flips retry classification to kind:'abort'	Informational	stands — re-measured in the matrix INFO cell: local Error subclass with the same constructor name → isAbortError=true, kind=abort at head; false/unknown at base (03-abort-matrix-head-vs-base.png)
4	Correction: body described the pre-reduction scope (stale)	Correction	superseded — the body was rewritten for the reduced scope and now matches the tree (isUserCancel/timeoutAbortReason absent, verified by grep = 0). The rewrite's suite numbers still do not reproduce in this container (environmental; see Finding 5), and its "pre-existing behaviour" sentence is the falsified claim carried as Finding 1

No declined or deferred rows from the previous round; all were re-measured, none worsened.

Central claim and A/B

Central claim: (a) isAbortError returns true for the OpenAI and Anthropic SDKs' APIUserAbortError (constructor-name match), flipping retry classification 'unknown' → 'abort'; (b) LoggingContentGenerator.safelyLogApiError skips the api_error event exactly when abortSignal?.aborted && isAbortError(error) at all three call sites (non-stream, stream setup, mid-stream), stopping the #8398/#8356 noise at the telemetry layer — while every non-cancel error still reports.

Cell	Environment	Oracle	Result
Source RED→GREEN	base worktree d96de59 + the PR's 3 test files vs head, vitest	3 changed suites	base 7 failed | 141 passed (148) — exactly the 7 intended mismatches (2× isAbortError positive, 1× kind:'unknown' vs 'abort', 4× expect(logApiError).not.toHaveBeenCalled()); head 148 passed (01-ab-red-green-base-vs-head.png)
SDK error matrix, head dist	compiled packages/core/dist, real locked SDK errors (openai 5.11.0, @​anthropic-ai/sdk 0.36.3)	isAbortError + classifyRetryError, 12 cells	12/12: both SDK aborts true/abort; DOMException, ABORT_ERR, axios-shape unchanged; APIConnectionError/RateLimitError(429)/null/string/spoof negative; INFO cell shows the same-name boundary (03-abort-matrix-head-vs-base.png)
SDK error matrix, surgical base control	byte copy of head dist with exactly the two production hunks reverted (diff -rq = 2 files)	same	12/12 as encoded: SDK aborts false/unknown, everything else identical to head
Telemetry gate, head dist	real LoggingContentGenerator from dist, fake provider adapter as the seam, observed at two real destinations (apiActivityTracker.drain() and the uiTelemetryService.addEvent stream)	errors recorded per cell, 11 cells	11/11: 5 cancel shapes (non-stream, stream-setup, mid-stream DOMException, mid-stream SDK, daemon string reason) recorded 0; 4 report cells (real failure, race, abort-shaped-no-signal, DOMException-no-signal) recorded 1; both deadline probes recorded 0 (02-gate-ab-telemetry-destination.png)
Telemetry gate, base control	same	same	11/11 as encoded: every error reaching the catch recorded 1, including the five cancel cells and both deadline probes
Retry loop, head vs base	real retryWithBackoff from dist, maxAttempts:3, permissive predicate	attempt + onRetry counts	user cancel: head 1 attempt / 0 retries, base 3 attempts / 2 retries; 503 controls 3 attempts both arms; error identity preserved (04-retry-loop-ab.png)

Base-arm reds are encoded as expectations (control cells), so they count as passes.

Mutation matrix (each mutant killed by exactly its pinned tests; attribution verified per failed-test name — 05-mutation-matrix-and-keepnames.png):

Mutant	Result
M0 revert both hunks (= base)	killed — the 7 RED failures above
M2a drop abortSignal?.aborted &&	killed — 2 failed (the two "abort-shaped the user did not cause / no signal" tests)
M2b drop isAbortError(error) &&	killed — 1 failed ("real failure racing a cancel")
M2c drop signal arg at non-stream site	killed — 1 failed (non-stream cancel)
M2d drop signal arg at stream-setup site	killed — 1 failed (stream-setup cancel)
M2e drop signal arg at mid-stream wrapper	killed — 2 failed (both mid-stream tests)
M3 broaden to startsWith('API')	killed — 1 failed (the APIConnectionError negative)
M4 remove instanceof Error guard	survives all 148; killed by the spoof probe (isAbortError({constructor:{name:'APIUserAbortError'}}) → true) → coverage gap, not dead code; head's guard rejects the same spoof

Corrections

• The previous round's stale-body correction is resolved at this head. The body now describes exactly the reduced scope (two changes, the split, the known limitation); isUserCancel/timeoutAbortReason are absent from the tree (grep = 0). What remains inaccurate is only the "pre-existing behaviour, not a regression" sentence, carried as Finding 1 with its measurement.

Findings (no blockers)

1. Internal deadline failures stop reaching api_error telemetry and the model-API-health chart at this head, and the body's "that is the pre-existing behaviour, not a regression" is falsified at the new base (Suggestion — accepted trade-off, mischaracterized claim). Re-measured at d96de59: the two deadline probes that mirror real producer shapes reachable through BaseLlmClient (which threads the caller's composed signal into config.abortSignal; re-verified baseLlmClient.ts passes abortSignal into the request config) — a plain AbortController+timer deadline (goalHook/promptHook shape) and an AbortSignal.timeout budget (memory recall/forget shape), each rejecting the fake provider with the openai SDK's abort error — record 1 at base and 0 at head on both telemetry destinations (02-gate-ab-telemetry-destination.png, DEADLINE PROBE rows). So head changes those paths from reported to suppressed; the span still records the abort, but the event and the health-chart counter no longer see it until the follow-up PR lands. The deferral is reviewer-directed, which is why this is a finding about the claim's wording and the deferral, not a blocked verdict. Reproduce: node tmp/pr8399-verify-20260812-030112/harness/02-gate.mjs <dist> head|base. Reviewers should confirm the deferral is intentional at this reduced scope and that the follow-up is tracked; a one-line body edit ("becomes suppressed at this head; the follow-up restores reporting") would make the claim true.
2. instanceof Error guard unpinned (Suggestion, carried over). Reproduce: delete the error instanceof Error && conjunct in packages/core/src/utils/errors.ts, run cd packages/core && npx vitest run src/utils/errors.test.ts src/utils/retryErrorClassification.test.ts src/core/loggingContentGenerator/loggingContentGenerator.test.ts → 148/148 green; then isAbortError({constructor:{name:'APIUserAbortError'}}) returns true. A one-line negative test would pin it.
3. keepNames load-bearing, unguarded by any test or CI gate (Suggestion, carried over). Re-run at the new head: esbuild.config.js is unchanged by the PR (keepNames: true at lines 241/267), the shipped chunks carry 3 __name(this, "APIUserAbortError") markers, and the esbuild probe shows the shipped config (keepNames, no minify) and keepNames+minify both preserve recognition, while no-keepNames+minify renames the class to E and recognition silently goes false (05-mutation-matrix-and-keepnames.png, bottom rows).
4. Name-match boundary extends to retry classification (Informational, carried over): a locally declared Error subclass named APIUserAbortError is recognized and classified kind:'abort' at head (matrix INFO cell), so it would also short-circuit retries. Inherent to the constructor-name mechanism; limited blast radius; documented trade-off in the code comment.
5. Body/test-plan numbers do not reproduce in this container (Informational, environment). The body's npx vitest run --root packages/core claim (19575 passed, 3 failed, 11 skipped) matches neither arm here: head measures 72 failed | 19595 passed | 10 skipped (19677), base 72 failed | 19499 passed | 10 skipped (19581) (the 96-test delta decomposes exactly: 12 new PR tests + 84 preset tests that failed collection in the base worktree). The 72 are this container's HOME-content environmental failures (logger checkpoints, ide-client, memoryDiscovery, file-token-storage, skill/subagent managers, installationManager, rulesDiscovery — e.g. installationManager reads a real pre-existing install-id file from $HOME), and the author's three named failures (read-file, zoom-image, memory extract) do not fail here. The load-bearing invariant was verified instead by A/A: the failure sets are equal modulo 4 parallel-load flakes (2 head-only + 2 base-only in extensionSettings.test.ts/write-file.test.ts, all green in isolation on both arms — extensionSettings 3× green at head) and 5 base-only provider-preset collection errors that are a worktree artifact of this verification (base tree has no built dist/ for the @qwen-code/qwen-code-core self-import; the vite error message proves the cause).

Not covered

• Live end-to-end cancel against a real provider: harnesses reproduce the wire shapes the SDK throws (real APIUserAbortError/DOMException instances through the real compiled generator and retry loop), not the network-side trigger.
• CLI package suite not run (core only).
• Per-commit attribution: shallow checkout (git rev-list HEAD^1..HEAD^2 reaches 1 commit; the metadata lists 17). Verified the aggregate HEAD^1..HEAD diff; the reduction's removals verified behaviorally by grep.
• shouldSuppressErrorLogging debug-log path not re-probed live this round; code read shows the openai provider's gate (openaiContentGenerator.ts:62, unchanged by the PR) uses the identical isAbortError(error) && signal.aborted shape and inherits the util fix — the same known limitation applies there by construction.
• The 5 base-only provider-preset failures and the 4 load flakes are artifacts of this verification environment, not properties of the PR or of base.

Methodology

Environment: node:22-bookworm CI container, Node v22.23.2, locked openai@5.11.0 / @anthropic-ai/sdk@0.36.3, head build pre-existing (packages/core/dist + root dist/ bundle). A/B arms: (1) scratch git worktree at HEAD^1 (d96de59) with the PR's three test files copied in for the source-level RED cell (vitest runs source; the PR leaves the lockfile untouched — verified by git diff --name-only = exactly the 5 PR files — so the shared root node_modules is a clean control; a package-local node_modules symlink fixed an ajv@​8-vs-@​6 hoisting artifact, same as the previous round); (2) a byte copy of head's compiled dist with exactly the two production hunks reverted (diff -rq = 2 files, asserted by harness/00-make-base-dist.mjs) as the dist control. Harnesses (01-abort-matrix.mjs, 02-gate.mjs, 03-retry-loop.mjs, 06-keepnames-probe.mjs) are mock-free with respect to the code under test: real SDK error instances against compiled dist; the real LoggingContentGenerator observed at two real telemetry destinations; the real retryWithBackoff; esbuild probes replicating mainBuild knobs. Mutations (05-mutation-matrix.mjs) applied to scratch copies of the source, suites re-run, restored via git checkout (repo left clean; worktrees removed). One environment quirk discovered: Node's AbortSignal.timeout() timer does not keep a bare process's event loop alive, so the D2 probe needed a loop-keeper in the harness — a harness artifact, not a PR property (production processes always have other loop keepers). Raw logs in logs/ (01–17 plus per-mutant logs), captures in evidence/ (01–05), tallied by harness/90-final-tally.mjs whose 47 checks each encode their expectation (base-arm reds and mutant kills count as passes).

Evidence images

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

— Qwen Code · sandboxed verification

[qwen-code-ci-bot]

Triage re-run completed without a new review.

⚠️ The bot has neither a verdict nor a deferral on aa6eca291fbffe0f0a226610ebfc1201c85cde2a — no APPROVED, CHANGES_REQUESTED, or COMMENTED review of its own. A DISMISSED one does not count: dismiss_stale_reviews voids the bot's approval on every push, which is exactly when a fresh one is needed. If this re-run was meant to review or approve, it did not, and an approval left by another account is a separate vote that does not count as the bot's own.

⚠️ 机器人在 aa6eca291fbffe0f0a226610ebfc1201c85cde2a 上既没有裁决也没有 defer —— 没有属于它自己的 APPROVED、CHANGES_REQUESTED 或 COMMENTED 评审。DISMISSED 不算：dismiss_stale_reviews 会在每次推送时作废机器人的批准，而那恰恰是需要一次新批准的时刻。如果这次重跑本应评审或批准，那么它没有做到；而其他账号留下的批准是另一张票，不能算作机器人自己的。

The stage comments above were updated with the latest result. View workflow run.

上方各阶段评论已更新为最新结果。查看工作流运行。

[wenshao]

Test Plan (not a blocker): src/tools/read-file.test.ts — no such file or directory; src/tools/zoom-image.test.ts — no such file or directory; 371 tests green — this review observed 19524, 1124 passed; Tests 19297 passed — this review observed 19524, 1124 passed.

中文说明

Test Plan（非阻断）：src/tools/read-file.test.ts — no such file or directory; src/tools/zoom-image.test.ts — no such file or directory; 371 tests green — this review observed 19524, 1124 passed; Tests 19297 passed — this review observed 19524, 1124 passed。

— qwen3.7-max via Qwen Code /review (v0.21.9)

[wenshao]

[Critical] The gate keys only on the state of the caller's signal, never on who fired it. Several in-repo callers compose timeout-based abort signals into model requests — permissions/classifier.ts:170 composes AbortSignal.any([input.signal, AbortSignal.timeout(stage1TimeoutMs)]), memory/relevanceSelector.ts:113 uses AbortSignal.timeout(30_000), and memory/forget.ts:193 uses AbortSignal.timeout(8_000), all routing through runSideQuery → baseLlmClient → req.config.abortSignal. When the timeout fires, Node sets signal.reason to a DOMException named TimeoutError and AbortSignal.any propagates it — but this gate checks only abortSignal?.aborted (true) and isAbortError(error) (true for APIUserAbortError, which the SDK throws for any aborted signal). — Failure scenario: provider latency spike → classifier/memory side query blows its AbortSignal.timeout budget → SDK rejects abort-shaped → gate suppresses the api_error event → at 3 AM the incident shows zero api_errors and a clean model-health chart while background LLM work silently fails. Probe-verified: with a TimeoutError-reason signal and APIUserAbortError, logApiError is called 0 times; adding signal.reason.name !== 'TimeoutError' flips the probe to pass with all 8 existing cancel tests still green.

Suggested change

	if (abortSignal?.aborted && isAbortError(error)) {
	return;
	}
	const reason: unknown = abortSignal?.reason;
	const isTimeoutAbort = reason instanceof Error && reason.name === 'TimeoutError';
	if (abortSignal?.aborted && !isTimeoutAbort && isAbortError(error)) {
	return;
	}

Node sets signal.reason to a DOMException named TimeoutError for AbortSignal.timeout(), and AbortSignal.any propagates the firing source's reason; please also add a test pinning the timeout case.

中文说明

[Critical] 该门控只依赖调用方 signal 的状态，从不区分是谁触发了它。仓库中多个调用方会向模型请求传入基于超时的 abort signal——permissions/classifier.ts:170 组合 AbortSignal.any([input.signal, AbortSignal.timeout(stage1TimeoutMs)])，memory/relevanceSelector.ts:113 使用 AbortSignal.timeout(30_000)，memory/forget.ts:193 使用 AbortSignal.timeout(8_000)，均经 runSideQuery → baseLlmClient → req.config.abortSignal 传入。超时触发时，Node 会把 signal.reason 设为名为 TimeoutError 的 DOMException，AbortSignal.any 会传播该 reason——但此门控仅检查 abortSignal?.aborted（true）和 isAbortError(error)（对 APIUserAbortError 为 true，SDK 对任何已中止 signal 都会抛出）。——失败场景：provider 延迟尖峰 → classifier/memory 侧查询超出超时预算 → SDK 以中止形态拒绝 → 门控抑制 api_error 事件 → 凌晨 3 点的故障中 api_error 为零、模型健康图表干净，后台 LLM 任务悄无声息地失败。已用探针验证：TimeoutError reason 的 signal 加 APIUserAbortError 时，logApiError 被调用 0 次；增加 signal.reason.name !== 'TimeoutError' 后探针翻转为通过，全部 8 个已有取消测试保持绿色。

— qwen3.7-max via Qwen Code /review (v0.21.9)

[harjothkhara]

On R1-1 — I'm not applying this one, because it's the trade-off @wenshao explicitly accepted when he asked for the split, and the suggested fix is the polarity he argued against.

His review: "under this version internal timeouts are still suppressed, but that's the status quo, not a regression." That is exactly R1-1 — the gate can't tell a user cancel from an internal deadline aborting the same request. It's deliberate here, and the PR body now says so under Scope rather than leaving it implicit.

The suggestion is signal.reason.name !== 'TimeoutError' — suppress by default, deadlines opt out. That's the negative-only polarity his review made the case against: a deadline that forgets to tag itself is silently dropped from telemetry, and the deadline-producer set is open and growing. This PR previously carried that design and it took nine review rounds to find seven producers, still missing one — which is the evidence he cited.

The follow-up inverts it: report by default, user cancels opt in. Only the small closed set of cancel producers is tagged (the TUI turn cancel, which was a bare abort, and the ACP session, which already used 'qwen:user-cancel' and now shares the constant). Under that gate the classifier and memory side queries in R1-1 are reported without needing any per-deadline conversion — a missed tag costs one recoverable api_error instead of a silent hole.

That work is implemented and verified, waiting on this PR to land: 10 files, +254/−35, with a test pinning the exact R1-1 case (an internal deadline aborting the request is reported, not suppressed). It also documents two things it does not close — the daemon prompt deadline still launders through the cancel tag at the ACP boundary, and the cancel-producer set wants a wider audit for secondary aborts.

Also fixed here: the test-plan numbers in the description were stale from before the split. The body now matches this PR — 19575 passed, 3 failed, with each failing file byte-identical to origin/main (two local workspace-trust expectations, one flaky under parallel load that passes in isolation).

@wenshao — no code change on this PR from this round. If you'd rather the instanceof change for the SDK match land here instead of the follow-up, say so and I'll add it.

[wenshao]

Local real-stack verification (maintainer run)

I built this PR and its merge-base locally and drove the real TUI end-to-end — real bundled dist/cli.js, real OpenAI SDK, ESC pressed in a real terminal — against a local OpenAI-compatible SSE mock, with telemetry captured via QWEN_TELEMETRY_OUTFILE and a request ledger on the mock. The fix does exactly what it claims, and the gate does not over-suppress. This complements the sandboxed bot round, which listed "real network-cancel E2E" as its main uncovered item.

Setup. base = merge-base 3037744 · head = aa6eca2, both bundled with the standard npm run build && npm run bundle; TUI in tmux with isolated $HOME, security.auth.selectedType=openai, OPENAI_BASE_URL → local mock (300 ms/chunk slow SSE; scenarios for slow-header hold and HTTP 500). macOS, Node 24.

Results

1. The #8398/#8356 repro — ESC during request setup — flips as claimed. With the mock holding the response head and ESC pressed ~1.5 s in, the OpenAI SDK throws APIUserAbortError from the in-flight fetch:

• base: telemetry outfile contains 1 × qwen-code.api_error with error_type: "APIUserAbortError", error_message: "Request was aborted." — exactly the noise from Bug: after APIUserAbortError, subsequent turns are not written to the local session transcript #8356;
• PR: 0 × api_error in the same scenario. The api_cancel event (1×, from the UI hook) and the llm_request span (status ERROR "API call aborted", error_type=APIUserAbortError attribute) are present on both sides — the cancellation signal is not lost, only the bogus error event is gone.

2. Negative control — a genuine failure is still reported. HTTP 500 from the mock on the PR bundle: 4 × qwen-code.api_error with error_type: "InternalServerError" (one per retry attempt). The abortSignal?.aborted && isAbortError(error) gate does not swallow real API errors, including ones that race the retry loop.

3. Mid-stream ESC: no api_error on either side. Cancelling at ~chunk 14 of a live stream produced 0 × api_error on base and head — the pipeline already normalizes mid-stream aborts to name='AbortError'. This confirms the PR is surgically about the paths where the raw SDK error propagates (setup / non-stream), and introduces no mid-stream regression.

4. No zombie retry on the wire. Mock ledger over a 25 s post-ESC quiet window: request → client_aborted_during_setup → silence on both sides. (An earlier run of mine appeared to show a post-cancel retry; the ledger's lastUserTail exposed it as my own harness artifact — ESC restores the cancelled prompt into the input box, so a typed /quit was appended and submitted as a new message. Worth knowing when eyeballing cancel behaviour manually.)

5. Unit A/B is RED→GREEN. PR's new test files on unmodified base source: 3 files, 7 failed / 141 passed — the failures are precisely the seven new assertions (both SDK APIUserAbortError shapes, 'abort' classification, and the four api_error-gate cases). On head: 148/148.

6. Class-name matching holds in the production bundle. The E2E runs exercise the bundled dist/cli.js, so the constructor.name === 'APIUserAbortError' match was proven end-to-end under esbuild keepNames — not just in vitest. (The bot's carried-over note stands: nothing pins keepNames in CI; fine to leave for the follow-up.)

7. UX unchanged. Same "Request cancelled." flow, prompt restored, single ESC — identical on both builds:

Not covered here: the user-cancel vs internal-deadline ambiguity of the gate — that is the known limitation this PR carries deliberately and the follow-up PR's subject.

Verdict: behaviour verified on the real stack; from my side this is good to merge as the reduced bug fix.

_{Evidence images live on pr-assets/8399-verify under verify/pr8399-local-20260812/ (appended after the CI bot's run directories, fast-forward).}

中文版本（Chinese version）

本地真实栈验证（维护者执行）

本地构建了本 PR 与其 merge-base,用真实打包产物 dist/cli.js + 真实 OpenAI SDK 在 tmux 真终端里按 ESC,对接本地 OpenAI 兼容 SSE mock;遥测经 QWEN_TELEMETRY_OUTFILE 落盘,mock 侧记请求账本。**修复行为与描述一致,且门控没有过度抑制。**这补上了沙箱 bot 报告中"真实网络取消 E2E 未覆盖"一项。

环境: base = merge-base 3037744,head = aa6eca2,均以标准 npm run build && npm run bundle 打包;隔离 $HOME、openai 认证、OPENAI_BASE_URL 指向本地 mock(300 ms/chunk 慢速 SSE,含慢首字节与 HTTP 500 剧本);macOS,Node 24。

结果

1. isAbortError does not recognize the OpenAI SDK's APIUserAbortError — user cancels in auth_type=openai are misclassified #8398/Bug: after APIUserAbortError, subsequent turns are not written to the local session transcript #8356 的复现形态——请求建立期按 ESC——按预期翻转。 mock 压住响应头、ESC 在 ~1.5 s 时按下,SDK 从在途 fetch 抛出 APIUserAbortError:base 侧遥测出现 1 × qwen-code.api_error(error_type: "APIUserAbortError",即 Bug: after APIUserAbortError, subsequent turns are not written to the local session transcript #8356 的噪音);PR 侧同场景 0 × api_error。两侧都保留 api_cancel 事件(1×)与 llm_request span(status ERROR "API call aborted" + error_type 属性)——取消信号没有丢,只去掉了错误事件。(图 1)
2. 阴性对照:真错误仍然上报。 PR 产物上 mock 返回 HTTP 500:4 × api_error(InternalServerError,每次重试一条)。门控不会吞掉真实 API 错误。
3. 流中 ESC:两侧都是 0 条 api_error。 流到第 ~14 个 chunk 时取消,base 与 head 均无 api_error——pipeline 早已把流中 abort 归一为 name='AbortError'。说明本 PR 精确针对原始 SDK 错误上抛的路径(建立期/非流式),对流中路径无回归。
4. 取消后无僵尸重试。 ESC 后 25 s 静默窗内账本只有 request → client_aborted_during_setup → 静默,两侧一致。(我早先一轮看到的"取消后请求"经账本 lastUserTail 证实是测试手法伪影:ESC 会把被取消的提示词还原到输入框,后输入的 /quit 被拼接成新消息发出。手工目测取消行为时值得留意。)
5. 单测 A/B RED→GREEN。 PR 的新测试文件放到未改动的 base 源码上:3 个文件 7 failed / 141 passed,失败恰为七条新断言;head 上 148/148 全过。(图 3)
6. 类名匹配在生产 bundle 中成立。 E2E 跑的是打包产物,constructor.name === 'APIUserAbortError' 在 esbuild keepNames 下端到端生效,不止 vitest 层。(bot 延续项仍然成立:CI 没有钉住 keepNames,留给后续 PR 合理。)
7. 用户体验无变化。 两个构建的取消流程完全一致("Request cancelled."、提示词还原、单次 ESC)。(图 2)

未覆盖: 门控无法区分用户取消与内部超时——即本 PR 有意保留的已知限制,属后续 PR 范畴。

结论: 真实栈行为验证通过;就本人而言,该缩减后的 bug fix 可以合入。

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[qwen-code-ci-bot]

Released in v0.21.11.