fix(external-context): read the response body with a reader, not for-await - #8764
Conversation
…await Async-iterating a ReadableStream needs [Symbol.asyncIterator] on the TYPE, and whether it is there depends on which lib set the program resolves — @types/node's stream has it, the DOM lib's needs lib.dom.asynciterable. That resolution flipped underneath this file on 2026-08-08: #8693 installed @types/jsdom at the root, vitest's types pull the jsdom types in wherever they exist, and jsdom's carry /// <reference lib="dom" />. #8693 shipped the tsconfig `types` guard in the same commit, so main stayed green — but the guard travels with the BRANCH while node_modules travel with the TRUSTED BASE in the autofix verification build, so every managed branch behind #8693 failed that build with TS2504 on this line. Two legs measured on run 31276008548: 63 minutes of accepted agent work discarded per round, 18 more minutes burned by a repair step that cannot fix a failure outside the PR's diff (#8614 reached attempt 13 that way; #8616 died identically). Reproduced locally in both directions before changing anything: @types/jsdom installed + guard removed = the gate's exact error, character for character; with the reader loop the same poisoned setup builds clean. The guard stays — belt and suspenders — but the build no longer depends on it, or on which lib set any future environment resolves. Behavior is unchanged and now pinned by tests the file never had: multi-chunk assembly, the exact MAX_RESPONSE_BYTES boundary (bound is strictly-greater), invalid-UTF-8 rejection, and the easy one to drop in this rewrite — cancelling the stream on early exit, which `for await` did implicitly via iterator return(). Mutation-tested: removing the cancel fails exactly that test against an endless producer. The package's other for-awaits iterate process.stdin (a Node stream, async-iterable in every lib set) and are untouched.
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
|
Re-run on head Template looks good ✓ Problem: observed, not theoretical — the TS2504 failure in the autofix verification build is documented with a real run and real discarded agent work, and earlier passes re-verified the poisoned setup on Direction: aligned — internal build reliability for the autofix loop, nothing user-visible, nothing near auth/sandbox/model selection/public contract. Size: not applicable — no core paths ( Approach: scope remains right, and the new commit completes the fix rather than bloating it. The guard existed only to protect the Risk: no high-risk paths matched; no elevated risk signals. Moving on to code review. 🔍 中文说明在新 head 模板完整 ✓ 问题：已观测到，不是理论问题——autofix 验证构建的 TS2504 失败有真实 run 与被丢弃的 agent 工时为证，此前几轮也已在 方向：对齐——autofix 循环的内部构建可靠性，无用户可见行为，不涉及 auth/sandbox/模型选择/公共契约。 规模：不适用——未触及核心路径（ 方案：范围仍然合理，新 commit 是收尾而非膨胀。防护只为保护被改写删掉的 风险：未命中高风险路径；无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-run on
No blockers. Test evidence — the PR's own CI (unattended run; no PR code executed here)CI is fully green on
中文说明在
无阻塞项。 测试证据来自 PR 自己的 CI（无人值守运行；此处未执行任何 PR 代码）。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — minimal fix, proven on every axis: behavior pinned by six tests and a mutation-tested A/B that already passed, and the build-independence claim now demonstrated by this head's own green CI rather than argued. Stepping back on the re-run: this landed where I'd want it. The independent baseline for this problem — make the file compile regardless of which lib set resolves 中文说明重跑后回顾：这个 PR 落到了我希望的位置。此问题的独立基线方案——让文件无论哪套 lib 解析 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
中文说明
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
| if (!finished) { | ||
| void reader.cancel().catch(() => undefined); | ||
| } |
There was a problem hiding this comment.
[Critical] R1-1: The replacement starts early-exit cancellation but does not wait for it before releasing the reader lock and returning the request error. A flipping probe confirmed that, with an oversized stream whose cancel() returns a pending promise, postJson() has already rejected while cancellation remains unresolved; changing this to await reader.cancel().catch(() => undefined) keeps the request pending until teardown settles. — Failure scenario: an immediate retry can overlap with the previous response transport's unfinished asynchronous cancellation. Please await cancellation before releaseLock() and add a deferred-cancel regression test.
| if (!finished) { | |
| void reader.cancel().catch(() => undefined); | |
| } | |
| if (!finished) { | |
| await reader.cancel().catch(() => undefined); | |
| } |
中文说明
R1-1： 当前实现启动提前退出时的取消操作后，并未等待其完成，就释放 reader lock 并返回请求错误。可翻转 probe 已确认：当超限流的 cancel() 返回一个尚未完成的 Promise 时，postJson() 已经拒绝，而取消仍处于 pending；改为 await reader.cancel().catch(() => undefined) 后，请求会等待清理完成。失败场景是立即重试会与上一响应传输尚未完成的异步取消重叠。请在 releaseLock() 前等待取消，并补充 deferred-cancel 回归测试。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
| const { done, value } = await reader.read(); | ||
| if (done) { | ||
| finished = true; |
There was a problem hiding this comment.
[Suggestion] R1-2: The new behavioral suite does not cover reader.read() rejecting after a partial chunk has already been received. — Failure scenario: if a provider disconnects mid-response after sending partial JSON, a future regression could treat the error as EOF, parse partial data, map the wrong error, or skip cleanup while all four added tests remain green. Add a stream that enqueues a partial JSON chunk and then errors, and assert the request-did-not-complete mapping plus cleanup behavior.
中文说明
R1-2： 新增的行为测试没有覆盖已经收到部分 chunk 后 reader.read() 拒绝的路径。失败场景是 provider 在发送部分 JSON 后中断连接，未来回归可能把错误当作 EOF、解析部分数据、映射错误的异常，或漏掉清理，而现有 4 条测试仍全部通过。建议增加一个先 enqueue 部分 JSON、随后报错的流，并断言 request-did-not-complete 错误映射及清理行为。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
… request On early exit from the reader loop (the oversize throw) cancellation was started fire-and-forget, so postJson() rejected while the stream's teardown was still settling — `for await` had awaited its implicit iterator return() before propagating. An immediate retry could overlap the previous response transport's unfinished cancellation. Await reader.cancel() before releaseLock(), and pin the sequencing with a deferred-cancel regression test that fails against the fire-and-forget form. Also cover read() rejecting after a partial chunk was received: the error maps to the request-did-not-complete transport error rather than EOF-then-parse of the partial JSON, and the reader lock is still released.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Round summaryAddressed both inline findings from the round-1 review with one commit ( Feedback points
Checks noteThe review body referenced a failing ConflictNone ( Verification
中文说明本轮总结通过一次提交（ 反馈点
检查项说明评审正文提到 冲突无（ 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // (#8693) dragged lib.dom into any program without this package's | ||
| // tsconfig `types` guard, and every branch behind that guard failed the | ||
| // autofix verification build with TS2504 on this exact line. The reader |
There was a problem hiding this comment.
[Suggestion] R2-1: This diff deletes the for await over response.body that the sibling integrations/external-context/tsconfig.json comment cites as the reason the "types": ["node"] guard exists ("breaking the for await over response.body in http-client.ts"), while the comment added here asserts the opposite ("the build no longer depends on that resolution at all"). An A/B build at this commit confirms the guard is no longer load-bearing: with the guard removed, this getReader() code still compiles cleanly even with lib.dom in the program — Concrete cost: a maintainer auditing build config now faces two contradicting stories, and either cleanup decision (keep or drop the guard) gets made against false documentation.
Suggested fix — refresh the tsconfig comment to the post-rewrite reality (the guard keeps this program's typings on the node variants now that http-client.ts no longer depends on stream async-iterability), or drop the guard together with its comment in this PR — the A/B arm proves this code builds cleanly without it.
中文说明
R2-1： 本 diff 删除了对 response.body 的 for await，而 integrations/external-context/tsconfig.json 的注释恰恰把它引用为 "types": ["node"] 防护存在的理由（"会破坏 http-client.ts 中对 response.body 的 for await"）；这里新增的注释却断言了相反的事实（"构建不再依赖那套解析"）。在本提交上做的 A/B 构建确认该防护已不再是承重项：去掉防护后，即使程序里有 lib.dom，这份 getReader() 代码依然编译干净——具体代价是：审计构建配置的维护者现在面对两种互相矛盾的说法，无论保留还是删除防护，清理决定都是依据错误文档做出的。
建议修复——把 tsconfig 注释更新为改写后的现状（既然 http-client.ts 不再依赖流的异步可迭代性，防护的作用是让本程序的类型保持在 node 变体上），或者在本 PR 里连同注释一起删掉防护——A/B 分支已证明去掉防护后本代码构建干净。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // oversize rejection, and — the easy one to drop — cancelling the stream on | ||
| // early exit, which `for await` used to do implicitly via iterator return(). | ||
|
|
||
| const MAX_RESPONSE_BYTES = 1024 * 1024; |
There was a problem hiding this comment.
[Suggestion] R2-2: This re-declares the byte cap locally because MAX_RESPONSE_BYTES in http-client.ts is not exported, silently decoupling the boundary tests from the real constant they exist to pin. Verified by probe at this commit: with the source cap raised to 2 MiB, accepts a body of exactly MAX_RESPONSE_BYTES still passes while sending only 1 MiB — the total > MAX vs >= guarantee it was written to pin is silently gone (the sibling oversize test then fails on a misdirected enqueued <= 4 chunk count, not the boundary) — Concrete cost: a future cap change silently invalidates the guarantee this file exists to provide.
Suggested fix — export the constant and import it here:
// in http-client.ts
export const MAX_RESPONSE_BYTES = 1024 * 1024;
// in this file: replace the local re-declaration with
import { postJson, MAX_RESPONSE_BYTES } from './http-client.js';中文说明
R2-2： 由于 http-client.ts 中的 MAX_RESPONSE_BYTES 未导出，这里在本地重新声明了字节上限，使边界测试与它们本要钉住的真实常量悄然脱钩。已在本提交上用 probe 验证：把源码侧上限提高到 2 MiB 后，accepts a body of exactly MAX_RESPONSE_BYTES 仍然通过，但实际只发送 1 MiB——它本要钉住的 total > MAX 与 >= 之分的保证悄然消失（相邻的超限测试届时会以 enqueued <= 4 的 chunk 计数错误地失败，而不是败在边界上）——具体代价是：未来一次上限调整就会让本文件存在所要提供的保证悄然失效。
建议修复——导出该常量并在这里导入（见上方英文代码示例）：在 http-client.ts 中加 export，本文件改为 import { postJson, MAX_RESPONSE_BYTES } from './http-client.js'，删除本地的重复声明。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
@qwen-code /triage |
|
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: 224 passed · 0 failed · 224 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：224 通过 · 0 失败 · 224 总计 Verification reportPR 8764 — verification reportVerdict: 中文摘要
Central claim + A/BCentral claim: rewriting All four cells compile against the same root
4/4 cells as expected. Witness: Behavioral A/B — the PR's new
The tests pin behavior, not implementation — which is exactly what a behavior-preserving rewrite needs. Wire oracle —
15/15 each arm, stderr 0 bytes on both. Witness: Mutation matrixSuite = the PR's own
M6 proves the suite can be made red, so the kill counts are trustworthy. M2 (the intermediate variant of the two-commit bundle — commit 1 is unreachable at this checkout's depth, so it is reconstructed by mutation per its commit message) fails exactly the deferred-cancel test with
The rewrite (commit 1) preserved cancellation but dropped the await; commit 2 restored it. Either half alone is incomplete; the pair is behaviorally equivalent to base — the conclusion a two-cell A/B alone cannot reach. Corrections
Findings (non-blocking)
Not covered
MethodologyEnvironment: Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
…bsolete The `"types": ["node"]` override existed solely to keep @types/jsdom's lib.dom out of this program while http-client.ts read the response body with `for await` — the DOM lib's ReadableStream is not async-iterable, and the flip broke the build with TS2504 (#8693). The reader loop that replaced the `for await` types identically in every lib set, so the guard is no longer load-bearing: with it removed, lib.dom re-enters the program and the package still builds cleanly. Drop it with its stale comment instead of leaving maintainers two contradicting stories about whether it is needed. Also export MAX_RESPONSE_BYTES and import it in the boundary tests instead of re-declaring it locally, so the tests pin the real constant rather than a copy that can silently drift.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed — PR #8764Round summary: both automated-reviewer suggestions were resolved in code with one commit ( Feedback points and dispositions[rc:3742256354] R2-1 — tsconfig
|
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action needed this roundThe feedback collected for this round contains no actionable items:
No changes were made to the PR branch this round. It remains at 中文说明本轮无需处理本轮收集的反馈中没有需要处理的事项：
本轮未对 PR 分支做任何更改。分支仍停留在 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: reverse audit — stopped before round 2 by the review time budget.
中文说明
已审查。 建议见行内评论。 未审查：反向审计——评审时间预算不足，未能开始第 2 轮。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| "composite": true, | ||
| // Override the root's `vitest/globals` entry: vitest's types import the | ||
| // optional `jsdom` peer types, and once `@types/jsdom` is installed that | ||
| // drags `/// <reference lib="dom" />` into this program. The DOM lib | ||
| // flips @types/node's conditional fetch globals to their DOM variants, | ||
| // whose ReadableStream is not async-iterable (needs lib.dom.asynciterable), | ||
| // breaking the `for await` over `response.body` in http-client.ts. The | ||
| // sources compiled here use no vitest globals (tests are excluded), so | ||
| // `node` alone is enough. | ||
| "types": ["node"], | ||
| "outDir": "dist", |
There was a problem hiding this comment.
[Suggestion] R3-1: The PR description is stale against the final head: it says "The guard stays — belt and suspenders" (and 中文 "防护保留——belt and suspenders"), but head commit 4c7bad3 removed the "types": ["node"] guard from this tsconfig, and the Reviewer Test Plan says "Expected: build/typecheck clean, 170/170 (10 files; 4 new in http-client.test.ts)" while the suite now has 6 tests and the package reports 172 (measured at this commit: 10 files / 172 tests). — Failure scenario: a maintainer merging on the description's narrative believes this package retains the types: ["node"] defense; post-merge it does not — a future lib.dom typing regression bisected here sends the investigation looking for a guard that no longer exists. A reviewer following the Test Plan sees counts that don't match (170/4 vs 172/6) and a local-repro step (# remove the "types": ["node"] line ...) that names a line deleted in round 2, so the Before/After evidence recipe silently fails to reproduce. The guard removal itself is fully justified (per R2-1's A/B) — only the PR body was never updated to match.
Suggested fix — update the PR body: note the round-2 commit dropped the now-dead guard (per R2-1, with the A/B re-verification), and correct the Test Plan's expected counts to 172 tests / 6 in http-client.test.ts (and the repro to compare main against this branch).
中文说明
R3-1： PR 描述相对最终 head 已过时：描述声称 "The guard stays — belt and suspenders"（中文 "防护保留——belt and suspenders"），但 head commit 4c7bad3 已从本 tsconfig 删除 "types": ["node"] 防护；Reviewer Test Plan 声称 "Expected: build/typecheck clean, 170/170 (10 files; 4 new in http-client.test.ts)"，而当前套件实际有 6 个测试、包内共 172 个（已在本提交上实测：10 个文件 / 172 个测试）。失败场景：维护者依据描述合并后会以为本包仍保留 types: ["node"] 防护，而合并后实际并无此防护——未来一旦有 lib.dom 类型回归被二分定位到本包，排查会去寻找一个已不存在的防护。按 Test Plan 操作的评审者会看到不匹配的数字（170/4 对 172/6），以及一条引用已在第 2 轮删除的配置行的本地复现步骤（# remove the "types": ["node"] line ...），Before/After 证据配方因此悄悄失效。防护删除本身完全合理（见 R2-1 的 A/B）——只是 PR 正文一直没有同步更新。
建议修复——更新 PR 正文：说明第 2 轮 commit 已删除这个不再承重的防护（依据 R2-1 及其 A/B 复核），并把 Test Plan 的预期数字更正为 172 个测试 / http-client.test.ts 中 6 个（复现步骤改为对比 main 与本分支）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Disposition: deferred — the fix is a PR-body edit, which needs a maintainer.
The finding is correct and was re-verified at this commit: the package suite reports 10 files / 172 tests (6 in http-client.test.ts), and 4c7bad3 removed the "types": ["node"] line the description still claims is kept. This autofix environment operates only inside the checkout with no GitHub write access, so it cannot edit the PR body; the thread stays open. Suggested replacements:
- Replace "The guard stays — belt and suspenders" (and 中文 "防护保留——belt and suspenders") with: "The round-2 commit dropped the
"types": ["node"]guard fromintegrations/external-context/tsconfig.json: the reader-loop rewrite made it loadless (per the R2-1 A/B), and build/typecheck were re-verified with it removed." - Replace the Test Plan's "Expected: build/typecheck clean, 170/170 (10 files; 4 new in
http-client.test.ts)" with "Expected: build/typecheck clean, 172/172 (10 files; 6 inhttp-client.test.ts)". - Replace the repro step that removes the
"types": ["node"]line with amain-vs-branch comparison — the line no longer exists on this branch.
中文说明
处理：暂缓——修复方式是编辑 PR 正文，需要维护者操作。
该发现正确，并已在本提交上复核：包测试套件为 10 个文件 / 172 个测试（http-client.test.ts 中 6 个），且 4c7bad3 已删除描述中仍声称保留的 "types": ["node"] 行。本 autofix 环境只在 checkout 内运行、没有任何 GitHub 写权限，因此无法编辑 PR 正文；该 thread 保持打开。建议替换文案：
- 将 "The guard stays — belt and suspenders"（及中文 "防护保留——belt and suspenders"）替换为："第 2 轮 commit 删除了
integrations/external-context/tsconfig.json中的"types": ["node"]防护：reader 循环重写后该防护不再承重（见 R2-1 的 A/B），并已在删除后重新验证 build/typecheck。" - 将 Test Plan 中 "Expected: build/typecheck clean, 170/170 (10 files; 4 new in
http-client.test.ts)" 替换为 "Expected: build/typecheck clean, 172/172 (10 files; 6 inhttp-client.test.ts)"。 - 将复现步骤中"删除
"types": ["node"]行"的写法改为对比main与本分支——该行在本分支上已不存在。
| vi.stubGlobal( | ||
| 'fetch', | ||
| vi.fn(async () => | ||
| streamingResponse([new Uint8Array([0xff, 0xfe, 0xfd])], cancelled), |
There was a problem hiding this comment.
[Suggestion] R3-3: This invalid-UTF-8 test cannot discriminate its target behavior — the one-line mutation deleting { fatal: true } from the TextDecoder in readBoundedBody survives the whole suite. — Failure scenario: verified by probe at this commit: with lax decoding, the fixture bytes [0xff, 0xfe, 0xfd] decode to three U+FFFD chars, JSON.parse throws on them, and postJson maps that to the same 'External context provider returned an invalid response.' the test asserts — 6/6 pass against the mutant. The mutation is not equivalent: with bytes forming otherwise-valid JSON around an invalid byte, fatal decoding rejects while lax decoding resolves with { a: '\uFFFD' } — silently corrupted provider content accepted. So the test pins "invalid bytes reject" but not which layer guarantees it, and the decoding guarantee it was written for can regress undetected. (The fatal: true decode block is pre-existing untouched code, hence Suggestion, not Critical.)
| streamingResponse([new Uint8Array([0xff, 0xfe, 0xfd])], cancelled), | |
| streamingResponse( | |
| [new Uint8Array([0x7b, 0x22, 0x61, 0x22, 0x3a, 0x22, 0xff, 0x22, 0x7d])], | |
| cancelled, | |
| ), |
The distinguishing fixture makes only fatal decoding pass: lax decoding resolves { a: '\uFFFD' } instead of rejecting and fails the test (verified: this fixture kills the mutant and passes against the correct code).
中文说明
R3-3： 这个非法 UTF-8 测试无法区分它要钉住的行为——把 readBoundedBody 中 TextDecoder 的 { fatal: true } 删掉这一行变异，整个测试套件依然全绿。失败场景：已在本提交上用 probe 验证：宽松解码下，测试数据 [0xff, 0xfe, 0xfd] 会解码成三个 U+FFFD 字符，JSON.parse 对其抛错，postJson 把该错误映射为测试所断言的同一条 'External context provider returned an invalid response.'——变异体下 6/6 全部通过。该变异并不等价：当字节序列在非法字节周围构成合法 JSON 时，fatal 解码会拒绝，而宽松解码会以 { a: '\uFFFD' } 成功解析——被破坏的 provider 内容被静默接受。因此该测试钉住的是"非法字节会被拒绝"，而不是"由哪一层保证拒绝"，它本要固定的解码保证可以在无人察觉的情况下回归。（fatal: true 解码块是本次 diff 未触碰的既有代码，因此定级为 Suggestion 而非 Critical。）
上方的 suggestion 给出可区分两者的测试数据：宽松解码届时会解析出 { a: '\uFFFD' } 而不是拒绝，从而让测试失败（已验证：该数据能杀死变异体，且在正确代码下通过）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
@qwen-code /triage |
|
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: 260 passed · 0 failed · 260 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：260 通过 · 0 失败 · 260 总计 Verification reportPR 8764 — verification report (follow-up round)Verdict: 中文 — 结论:✅ 通过 · 可合入(agent 判定)
Previous-finding status at the new head
Every measurement from the previous round was re-run at the new head — the test file changed in commit 4 (constant import), the base tip moved ( Central claim + A/BCentral claim of the delta (commit 4): the Build matrix — all cells against the same root
8/8 cells as expected. Cell 2 matches the PR body's quoted gate error byte for byte; cells 5/6 prove the poison is active in this environment, so cell 3's clean build is the rewrite doing the work, not an absent hazard. Note both dependency layouts pass: worktree cells resolve the root Per-commit attribution — all four snapshot commits were object-reachable this round (previous round's depth-2 limitation is lifted;
(Base/commit-1/commit-3 arms use one non-behavioral adaptation: the tests import Commit-4's import wiring is load-bearing — M7 pair in the mutation matrix: halving the source constant leaves the head tests green (they construct bodies from the imported value, 6/6) while the commit-3 test file with its local Wire oracle —
18/18 each arm, stderr 0 bytes on both. Behavioral parity at the socket level is preserved by the whole PR, so the guard removal and export are confirmed structural-only. Mutation matrix (re-run at the new head)Suite = the PR's
9/9 cells as encoded. M6 proves the suite can be made red, so the kill counts and the M4/M7a survivors are trustworthy; M4 is classified dead code (a non- Targeted gates
Corrections
Findings (non-blocking)
Not covered
MethodologyEnvironment: the verify lane's own Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. The reader-loop rewrite preserves everything for-await did, and the tests pin it: bounded accumulation with the exact-MAX boundary, oversize rejection that cancels the stream and holds the reject until a deferred cancellation settles (parity with the awaited implicit iterator return()), mid-stream read failure mapped to the transport error with the lock released, and invalid-UTF-8 rejection. The getReader() API types identically in every lib set, so the build no longer depends on whether lib.dom.asynciterable resolves — and dropping the now-unneeded types:["node"] guard also removes the branch-vs-node_modules asymmetry that was discarding accepted autofix work behind #8693. CI green on this head. Nothing blocks merge.
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #8764Feedback triageR3-3 — Verified the finding against this branch's head before acting: with R3-1 — The finding is correct and was re-verified at this commit: the package suite reports 10 files / 172 tests with 6 in
Changes this round
Verification
中文说明Autofix 本轮总结 — PR #8764反馈分类处理R3-3 — 在动手修复前，先在本分支 head 上验证了该发现：将 R3-1 — 该发现正确，并已在本提交上复核：包测试套件为 10 个文件 / 172 个测试，其中
本轮改动
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
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: 268 passed · 0 failed · 268 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：268 通过 · 0 失败 · 268 总计 Verification reportPR 8764 — verification report (follow-up round, head
|
| # | finding (previous round) | severity | status at 2cae67b |
|---|---|---|---|
| 1 | pre-existing cancelResponseBody fire-and-forget cancel on the 3xx / non-ok / declared-oversize paths |
non-blocking | stands — re-measured (scripted, remesure-findings.console): one void response.body?.cancel() inside the helper, 3 call sites, 0 awaited body cancels |
| 2 | description drift: body says "170/170 (10 files; 4 new)" and "The guard stays" | nit | stands, unchanged — body untouched since the previous round; suite still measures 172/172 with 6 tests in http-client.test.ts, and the guard is still absent from tsconfig.json (drift check: all three claims PRESENT in body, all three contradicted by measurement) |
| 3 | M4 survivor: value === undefined guard runtime-dead |
nit | stands — M4 re-run at the new head: 6/6 green (mutation matrix below); dead code per the PR's own disclosure |
| — | correction: DaemonClient.ts's fetch-body consumer already uses getReader() via sse.ts |
correction | stands — re-verified at new head: sse.ts:74 reads the body via getReader(); npm run typecheck -w packages/sdk-typescript exits 0 |
Every measurement was re-run at the new head — the test file changed in the delta commit, the base tip moved (4a79517 → afcc937), and no input closure was identical, so nothing was carried forward on the shortcut.
Central claim of the delta + A/B
Delta claim (commit 5): the invalid-UTF-8 test now pins fatal decoding — it fails if TextDecoder('utf-8', { fatal: true }) is relaxed — instead of passing for the wrong reason. The test's fixture was rebuilt so the invalid byte sits inside otherwise-valid JSON ({"a":"<0xff>"}), where lax decoding yields parseable {"a":"\uFFFD"} and would silently accept corrupted provider content.
Proof, three layers:
- Mutation kill (the load-bearing check) — matrix rows M8/M8b below: relaxing the decoder (option removed, or
fatal: false) turns the suite 5/6 with exactly rejects a body that is not valid UTF-8 red. The delta test kills the mutation it was written to kill. - Fixture claim verified byte-for-byte —
utf8-siblings.mjsS1: lax-decoding the test's exact bytes produces{"a":"\uFFFD"}, andJSON.parseaccepts it (lax text="{\"a\":\"…\"}" parse={"a":"…"}inutf8-head.log). So fatal decoding is the only thing rejecting this shape — the test cannot pass for any other reason. - Sibling sweep through the production path — five invalid-UTF-8 shapes (in-string 0xff, truncated 2-byte sequence, overlong
0xC0 0xAF, lone continuation byte, out-of-string invalid byte) driven through the compiledpostJsonof both arms: all 5 rejected with the invalid-response error on head and base (10/10 per arm). Classification shows S1–S4 would have been silently accepted under lax decoding (only fatal protects them); S5 is caught by lax too, but only accidentally (parse fails on the replacement char). No sibling escapes the fatal decoder; parity holds across the rewrite.
Build matrix at the new base tip — re-run because the base moved; all cells against the shared root node_modules (already contains @types/jsdom@28.0.3 from #8693 — the trusted-base state). Guard swap = copying the other tree's tsconfig, which differs by nothing else (9-line guard block, verified by diff). Witness: 01-poisoned-build-ab-matrix.png.
| cell | tree | tsconfig | result | expectation |
|---|---|---|---|---|
| C1 | base afcc937 |
as-is (guard present) | exit 0, clean | main is green today |
| C2 | base | guard removed | exit 1, src/http-client.ts(124,29): error TS2504: Type 'ReadableStream<Uint8Array<ArrayBufferLike>>' must have a '[Symbol.asyncIterator]()' method that returns an async iterator. |
the incident, character for character |
| C3 | head 2cae67b |
as-is — no guard, the shape that merges | exit 0, clean | the fix stands without the guard |
| C4 | head | guard re-added | exit 0, clean | re-adding is harmless |
| C5 | head, + probe exporting document: Document |
as-is | exit 0, clean | lib.dom IS in the guardless program — C3 is not vacuous |
| C6 | base, + same probe | as-is (guard) | exit 1, src/libdom-probe.ts(1,38): error TS2584: Cannot find name 'document'. |
the guard excluded lib.dom — probe validity control |
6/6 cells as expected. C2 matches the PR body's quoted gate error byte for byte; C5/C6 prove the poison is active in this environment, so C3's clean build is the rewrite doing the work, not an absent hazard. (Previous-round cells 7/8 — commits c88d3ab/ba5dbade with guard removed — could not be re-run: those objects are unreachable in this round's depth-2 checkout; see Not covered.)
Wire oracle — wire-harness.mjs: real node:http loopback server, real global fetch over a real socket, each arm's dist/ rebuilt from clean state. Asserts both sides of the wire. Witness: 02-wire-harness-head-vs-base.png.
| scenario | head dist | base dist |
|---|---|---|
W0 404 → ProviderHttpStatusError(404) (control) |
ok | ok |
| W1 3-chunk assembly; server saw POST/Bearer/content-type/exact body | ok | ok |
| W2 exactly-MAX body accepted (strictly-greater bound) | ok | ok |
| W3 oversize endless stream rejected; server observed teardown ≤2 s (head: 2 ms, base: 1 ms); producer stopped at 3 chunks; immediate retry succeeds | ok | ok |
| W4 declared content-length > MAX rejected pre-stream (1–2 ms); connection closed after 64 KiB of the 1 MiB+1 body | ok | ok |
| W5 mid-stream socket drop → transport error, not parse error | ok | ok |
| W6 invalid UTF-8 rejected | ok | ok |
W7 export surface: head exports MAX_RESPONSE_BYTES === 1048576; base does not |
ok | ok |
20/20 each arm, stderr 0 bytes on both (wire-{head,base}.err). Behavioral parity at the socket level is preserved, confirming the delta (test-only) changed no wire behavior.
Behavioral A/B — the PR's 6-test suite run against the base for await implementation (one non-behavioral adaptation: export added to the base constant so the test's import resolves). Witness: 04-behavior-ab-base-arm.png.
| arm | result |
|---|---|
| head reader loop (CONTROL, within matrix) | 6/6 |
base for await |
6/6 |
Parity: for await's implicit iterator return() provides the same cancel-and-await semantics the reader loop now does explicitly, so the rewrite preserves behavior on every pinned axis.
Mutation matrix (re-run at the new head)
Suite = the PR's http-client.test.ts (6 tests), one mutant per row, restored after each. Witness: 03-mutation-matrix-head.png.
| mutant | result | killed by / classification |
|---|---|---|
| CONTROL unmutated head | GREEN 6/6 | — |
| M1 remove cancel entirely | RED 4/6 | over-budget-cancels + deferred-cancel |
| M2 fire-and-forget cancel (commit-1 shape) | RED 5/6 | deferred-cancel only — re-represents commit c88d3ab's shape, whose object is unreachable this round |
M3 > → >= |
RED 5/6 | exactly-MAX accepted |
M4 remove value === undefined guard |
GREEN 6/6 | survivor — dead code (Finding 4) |
M5 remove releaseLock() |
RED 5/6 | mid-stream locked assertion |
| M6 positive control: corrupt error message | RED 3/6 | the three message-asserting tests — the suite can be made red, so kill counts are trustworthy |
| M7a halve source constant, head tests (import) | GREEN 6/6 | tests track the imported constant |
M8 fatal: true removed (lax decoding) |
RED 5/6 | exactly rejects a body that is not valid UTF-8 — the delta test pins fatal decoding |
M8b fatal: false explicit |
RED 5/6 | same single test |
| M7b-sim halved constant vs reconstructed pre-commit-4 local-copy test | RED 5/6 | exactly-MAX — the commit-4 import wiring stays load-bearing |
11/11 cells as encoded. M6 is the positive control for the killers; M8/M8b are the delta's own claim, measured. M7b-sim reconstructs the pre-commit-4 test shape (local const MAX_RESPONSE_BYTES = 1024 * 1024 copy instead of the import), since commit 3's actual test file is unreachable this round — the reconstruction carries the same property the original pinned.
Targeted gates
| gate | result |
|---|---|
npm test -w integrations/external-context full package suite |
172/172, 10 files (vitest-head-full.log; witness 05-full-suite-and-gates.png) |
npm -w integrations/external-context run build (main tree, CI-shape layout with package-local @types/node@22.20.1) |
exit 0 |
npm -w integrations/external-context run typecheck |
exit 0 |
npm run typecheck -w packages/sdk-typescript |
exit 0 |
eslint src (head tree) |
exit 0 clean; liveness: planted unused variable caught at http-client.ts:174:7 (@typescript-eslint/no-unused-vars, eslint-plant.log), restored re-run clean |
Corrections
The previous round's correction stands (see status table): DaemonClient's fetch-body consumer already uses the explicit getReader() pattern via sse.ts:74, re-confirmed by grep and by the sdk typecheck gate above. No new corrections this round.
Findings (non-blocking)
- Description drift, unchanged (previous finding 2). The body still says "170/170 (10 files; 4 new in
http-client.test.ts)" while the suite measures 172/172 with 6 tests, and still says "The guard stays — belt and suspenders" (zh: "防护保留") while the guard is removed fromtsconfig.json. Scripted drift check: all three claims present in the body, all three contradicted by measurement (remesure-findings.console). Harmless to the code; one line each would fix it if the author touches the branch again. - Reviewer Test Plan step 3 does not work as written (previous finding, stands).
npx vitest run --config integrations/external-context/vitest.config.tsfrom the repo root exits 1 with "No test files found" — theinclude: ['src/**/*.test.ts']glob resolves against cwd, not the config dir. From the package directory it passes 172/172. Scripted: TP3/TP3b inremesure-findings.console. The plan needs a cwd note. - Pre-existing sibling, untouched (previous finding 1, stands):
cancelResponseBody()remains fire-and-forget on the 3xx, non-ok, and declared-oversize paths — the same hazard class this PR's rationale fixes forreadBoundedBody. Pre-existing cause; this PR contributes nothing to it. Report-only. - M4 survivor — dead code (previous finding 3, stands): removing the
value === undefinedguard leaves all 6 tests green because a non-doneread always carries a value at runtime. Classified dead code (not a coverage gap) per the PR's own disclosure; completeness reporting, not a merge condition.
Not covered
- Delta attribution is the aggregate diff. Commits
c88d3ab,dc1f1d4,ba5dbade,4c7bad3are unreachable in this round's depth-2 checkout (git rev-list --count HEAD^1..HEAD^2reports 1 vs the snapshot's 5 — the shallow-graft under-report noted previously). The delta diff4c7bad3..2cae67btherefore could not be computed directly; commit 5's "test-only" attribution is its commit message. What was verified instead: the aggregateHEAD^1..HEADdiff touches only the three known files; the production file's content matches the state the previous round verified at4c7bad3(export present, awaited cancel present, guard absent); and the delta's subject — the invalid-UTF-8 test — was exercised in depth (M8/M8b/S1). Per-commit attribution for commits 1–4 was executed in the previous round; its two behaviorally distinct shapes are re-represented this round by M2 (fire-and-forget) and M7b-sim (local-copy test). - Wire-level awaited-cancel ordering: the harness asserts server-observed teardown (≤2 s) and immediate-retry success but cannot order client-reject vs server-observation across a socket; strict sequencing is pinned by the in-process barrier test (and M2 red). Same position as previous rounds, re-measured.
- Base-side test coverage: only
http-client.test.ts(the changed surface) ran against the base tree, not the base workspace's full suite. - Repo-wide gates not run; targeted gates only.
- The structural half the description defers (verification gate charging base-skew failures to PRs) remains out of scope by the PR's own statement.
Methodology
Environment: the verify lane's own node:22-bookworm container (live sample: @types/jsdom@28.0.3 already in root node_modules — the trusted-base poison state needed no install; TS 5.8.3, vitest 3.2.4, node v22.23.2). Two scratch git worktrees under tmp/ (base HEAD^1 = afcc937, head HEAD^2 = 2cae67b), both nested under the repo root so every cell resolves the shared root node_modules — realpath control asserted typescript and @types/node from inside the base worktree resolve to /__w/qwen-code/qwen-code/node_modules/… (no workspace-link confound; the package has zero @qwen-code/* dependencies and http-client.ts imports nothing). Worktree cells use root @types/node@20.19.1; the CI-shape layout (main tree, package-local @types/node@22.20.1) was additionally gated for build/typecheck. The base arm received one non-behavioral adaptation (export added to the constant declaration so the test's import resolves). Harnesses: build-matrix.sh, wire-harness.mjs, mutation-matrix.mjs, utf8-siblings.mjs, behavior-ab.sh, remesure-findings.sh; raw logs in logs/ (build-matrix.log/.console, wire-{head,base}.log/.err, utf8-{head,base}.log, mutation-matrix-head.console, per-mutant vitest JSON under logs/mutants/, vitest-base-arm.json, vitest-head-full.log, tp3-{root,pkgdir}.log, drift.log, eslint-{head,plant,restored}.log, {build,typecheck,sdk-typecheck}-head.log, remesure-findings.console). Evidence images rendered with scripts/verify-capture.mjs. Worktrees removed after capture; git status --porcelain empty. Assertion tally: 6 (build cells) + 42 (wire, 20×2 arms + 2 stderr-clean) + 11 (mutation expectations) + 6 (behavioral base arm) + 20 (UTF-8 siblings, 10×2 arms) + 172 (head full suite) + 3 (build/typecheck/sdk gates) + 3 (lint liveness) + 5 (finding/test-plan re-measurements) = 268, all passing.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Re-approving after the test tightening (my previous approval was dismissed by the push): the invalid-UTF-8 case now embeds the bad byte inside otherwise-valid JSON, which is the shape that actually distinguishes fatal decoding from lax decoding — lax would resolve it to U+FFFD and accept corrupted provider content, so the test now pins the fatal flag rather than passing either way. The reader-loop behavior itself is unchanged from the reviewed revision. CI green on this head. Nothing blocks merge.
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no action taken (PR #8764)This round found no actionable feedback, so no code changes were made.
The branch remains at its current head ( 中文说明Autofix 审查轮次 —— 未采取任何操作（PR #8764）本轮未发现任何需要处理的反馈，因此未做任何代码改动。
分支仍停留在当前 head（ Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /takeover stop |
qwen-code-dev-bot
removed
the
autofix/takeover
|
👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply 中文说明👋 已释放：autofix 循环不再介入此 PR（在飞的一轮如有，将完成其有界工作）。重新打上 |
…await (QwenLM#8525) Port of QwenLM#8764 (10621b3) to this branch. Async-iterating a ReadableStream needs [Symbol.asyncIterator] on the TYPE, and whether it is there depends on which lib set the program resolves — @types/node's stream has it, the DOM lib's needs lib.dom.asynciterable. This branch predates QwenLM#8693, whose tsconfig "types" guard keeps @types/jsdom's lib.dom out of the package program; the autofix verification build resolves node_modules from the trusted base (which has @types/jsdom), so the guardless branch fails the build with TS2504 on the `for await`. The reader loop types identically in every lib set, so the build no longer depends on that resolution. Behavior is unchanged and pinned by the regression tests ported from the same commit: multi-chunk assembly, the exact MAX_RESPONSE_BYTES boundary, oversize rejection with stream cancellation, deferred-cancel sequencing, mid-stream read failure, and invalid-UTF-8 rejection. The Mem0-related changes that share main's http-client.ts (QwenLM#8507) are intentionally not ported.
|
Released in v0.21.9. |
…await (#8764) * fix(external-context): read the response body with a reader, not for-await Async-iterating a ReadableStream needs [Symbol.asyncIterator] on the TYPE, and whether it is there depends on which lib set the program resolves — @types/node's stream has it, the DOM lib's needs lib.dom.asynciterable. That resolution flipped underneath this file on 2026-08-08: #8693 installed @types/jsdom at the root, vitest's types pull the jsdom types in wherever they exist, and jsdom's carry /// <reference lib="dom" />. #8693 shipped the tsconfig `types` guard in the same commit, so main stayed green — but the guard travels with the BRANCH while node_modules travel with the TRUSTED BASE in the autofix verification build, so every managed branch behind #8693 failed that build with TS2504 on this line. Two legs measured on run 31276008548: 63 minutes of accepted agent work discarded per round, 18 more minutes burned by a repair step that cannot fix a failure outside the PR's diff (#8614 reached attempt 13 that way; #8616 died identically). Reproduced locally in both directions before changing anything: @types/jsdom installed + guard removed = the gate's exact error, character for character; with the reader loop the same poisoned setup builds clean. The guard stays — belt and suspenders — but the build no longer depends on it, or on which lib set any future environment resolves. Behavior is unchanged and now pinned by tests the file never had: multi-chunk assembly, the exact MAX_RESPONSE_BYTES boundary (bound is strictly-greater), invalid-UTF-8 rejection, and the easy one to drop in this rewrite — cancelling the stream on early exit, which `for await` did implicitly via iterator return(). Mutation-tested: removing the cancel fails exactly that test against an endless producer. The package's other for-awaits iterate process.stdin (a Node stream, async-iterable in every lib set) and are untouched. * fix(external-context): await stream cancellation before rejecting the request On early exit from the reader loop (the oversize throw) cancellation was started fire-and-forget, so postJson() rejected while the stream's teardown was still settling — `for await` had awaited its implicit iterator return() before propagating. An immediate retry could overlap the previous response transport's unfinished cancellation. Await reader.cancel() before releaseLock(), and pin the sequencing with a deferred-cancel regression test that fails against the fire-and-forget form. Also cover read() rejecting after a partial chunk was received: the error maps to the request-did-not-complete transport error rather than EOF-then-parse of the partial JSON, and the reader lock is still released. * fix(external-context): drop the types guard the reader rewrite made obsolete The `"types": ["node"]` override existed solely to keep @types/jsdom's lib.dom out of this program while http-client.ts read the response body with `for await` — the DOM lib's ReadableStream is not async-iterable, and the flip broke the build with TS2504 (#8693). The reader loop that replaced the `for await` types identically in every lib set, so the guard is no longer load-bearing: with it removed, lib.dom re-enters the program and the package still builds cleanly. Drop it with its stale comment instead of leaving maintainers two contradicting stories about whether it is needed. Also export MAX_RESPONSE_BYTES and import it in the boundary tests instead of re-declaring it locally, so the tests pin the real constant rather than a copy that can silently drift. * test(external-context): make the invalid-UTF-8 test pin fatal decoding --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
* fix(core): resolve Qwen 3.8 reasoning budget conflicts * fix(core): cover unconfigured Qwen 3.8 conflicts * chore: preserve latest main formatting * fix(core): harden DashScope thinking precedence * fix: honor DashScope thinking knob precedence * test(core): assert same-layer thinking knob drop warning for request pairs (QwenLM#8525) * fix: align effort override reporting with wire resolution * fix: resolve thinking knob review findings * fix: sort Python SDK test imports * fix(core): ignore null thinking knobs * fix(core): register enable_thinking true in thinking knob selection (QwenLM#8525) selectFromLayer only registered enable_thinking === false, so a higher-priority enable_thinking: true was invisible to cross-layer resolution: a lower-priority samplingParams disable won selection and rewrote the shipping tier to reasoning_effort 'none', inverting the documented extra_body > samplingParams precedence. Register the on-switch as the weakest knob in its own layer (an off-switch rewrites the tier, an on-switch never does) and make the drop branch value-aware: true keeps the shipping tier and drops only the redundant knobs, false keeps the canonical 'none' disable. getReasoningEffortOverride no longer reports an on-switch as shadowing the tier (the wire drops the switch and ships the tier), except for a request-level effort override that still shadows from under it. Also corrects the dropConflictingThinkingKnobs contract comment (only effort tiers ship alone; the 'none' disable and a winning budget keep a co-present enable_thinking) and the model-providers.md precedence callout, which overstated samplingParams precedence for older qwen hybrids where the reasoning-derived enable_thinking: true overrides it. * fix(core): preserve budget beneath thinking on-switch * fix(core): canonicalize disabled thinking knobs * fix: resolve round-6 thinking knob review findings (QwenLM#8525) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(external-context): read the response body with a reader, not for-await (QwenLM#8525) Port of QwenLM#8764 (10621b3) to this branch. Async-iterating a ReadableStream needs [Symbol.asyncIterator] on the TYPE, and whether it is there depends on which lib set the program resolves — @types/node's stream has it, the DOM lib's needs lib.dom.asynciterable. This branch predates QwenLM#8693, whose tsconfig "types" guard keeps @types/jsdom's lib.dom out of the package program; the autofix verification build resolves node_modules from the trusted base (which has @types/jsdom), so the guardless branch fails the build with TS2504 on the `for await`. The reader loop types identically in every lib set, so the build no longer depends on that resolution. Behavior is unchanged and pinned by the regression tests ported from the same commit: multi-chunk assembly, the exact MAX_RESPONSE_BYTES boundary, oversize rejection with stream cancellation, deferred-cancel sequencing, mid-stream read failure, and invalid-UTF-8 rejection. The Mem0-related changes that share main's http-client.ts (QwenLM#8507) are intentionally not ported. * fix(sdk-python): expose effort status reason from CLI (QwenLM#8525) The CLI emits a human-readable reason on effort_status and the TypeScript SDK surfaces it, but the Python EffortStatus TypedDict and _parse_effort_status dropped it, leaving Python callers to reconstruct the reason from override. Add reason as an optional field and pass it through, mirroring the TypeScript parser. * test(core): add direct unit tests for selectDashScopeThinkingKnob (QwenLM#8525) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
5 participants
What this PR does
Rewrites
readBoundedBody's loop fromfor await (const chunk of response.body)to an explicitgetReader()loop, and adds the behavioral tests the file never had. Behavior is unchanged.Why it's needed
Async-iterating a
ReadableStreamneeds[Symbol.asyncIterator]on the type, and whether it is there depends on which lib set the program resolves —@types/node's stream has it, the DOM lib's needslib.dom.asynciterable. That resolution flipped underneath this file on 2026-08-08:@types/jsdomat the root. vitest's types pull the jsdom types in wherever they exist, and jsdom's carry/// <reference lib="dom" />.types: ["node"]guard in the same commit, somainstayed green.node_modulestravel with the trusted base — so every managed branch behind fix(integration-tests): make the project typecheckable and fix what that found #8693 built branch sources against post-fix(integration-tests): make the project typecheckable and fix what that found #8693 dependencies and failed:Measured cost on one run (31276008548): the #8614 leg had 63 minutes of accepted agent work discarded, plus 18 minutes burned by a repair step that cannot fix a failure outside the PR's diff — that PR reached attempt 13 this way, and the #8616 leg died identically. Every autofix round on every stale branch pays this until the branch merges main.
The reader API types identically in every lib set, so after this change the build depends on neither the guard nor the environment's lib resolution. The guard stays — belt and suspenders.
Reviewer Test Plan
How to verify
Expected: build/typecheck clean, 170/170 (10 files; 4 new in
http-client.test.ts).To reproduce the incident locally (both directions):
Evidence (Before & After)
Before is the gate error above, reproduced locally character for character with the poisoned setup (jsdom installed + guard removed). After is the same setup building clean — the fix is provably independent of the guard.
New tests pin what the rewrite must preserve:
MAX_RESPONSE_BYTESboundary (bound is strictly-greater)for awaitdid this implicitly via iteratorreturn(); the reader loop must do it explicitly, and the test drives an endless producer so a dropped cancel cannot hide. Mutation-tested: removing the cancel fails exactly that test.Tested on
Risk & Scope
value === undefinedguard inside the loop is defensive against older non-discriminatedReadableStreamReadResulttypings; at runtime a non-done read always carries a value.for awaits iterateprocess.stdin(a Node stream, async-iterable in every lib set) and are untouched.packages/sdk-typescript/src/daemon/DaemonClient.tshas the same pattern over a fetch body in a different program; it builds today and is left alone, noted here so the class is on record. The structural half — the verification gate charging pre-existing/base-skew failures to the PR and burning an 18-minute repair on them — is the follow-up, not this PR.Linked Issues
Diagnosed from the discarded autofix round on #8614 (comment).
中文说明
What this PR does
把
readBoundedBody的循环从for await (const chunk of response.body)改写为显式的getReader()循环,并补上该文件此前从未有过的行为测试。行为不变。Why it's needed
对
ReadableStream做 async 迭代需要类型上存在[Symbol.asyncIterator],而它是否存在取决于程序解析到哪套 lib——@types/node的流类型有,DOM lib 的需要lib.dom.asynciterable。2026-08-08 这个解析在此文件脚下翻转了:@types/jsdom。vitest 的类型只要发现 jsdom 类型就会引入,而后者携带/// <reference lib="dom" />。types: ["node"]防护,所以main保持绿色。node_modules却跟着可信 base 走——于是所有落后于 fix(integration-tests): make the project typecheckable and fix what that found #8693 的受管分支都在用 fix(integration-tests): make the project typecheckable and fix what that found #8693 之后的依赖构建分支源码,必然失败:单个 run 的实测代价(31276008548):#8614 那条 leg 63 分钟已通过的 agent 工作被整体丢弃,外加修复步白烧 18 分钟去修一个不在 PR diff 内的失败——该 PR 就这样走到了第 13 轮,#8616 的 leg 死法一模一样。每个落后分支的每一轮 autofix 都在付这笔账,直到它合并 main 为止。
reader API 在任何 lib 集合下类型都一致,因此本改动之后,构建既不依赖那个防护,也不依赖环境的 lib 解析。防护保留——belt and suspenders。
Reviewer Test Plan
How to verify
预期:build/typecheck 干净,170/170(10 个文件;
http-client.test.ts新增 4 条)。本地双向复现事故:
Evidence (Before & After)
Before 即上面 gate 的报错,已用中毒组合(装 jsdom + 去防护)在本地逐字符复现。After 是同一组合下构建干净——证明修复不依赖防护而独立成立。
新测试钉住改写必须保持的行为:
MAX_RESPONSE_BYTES的精确边界(判定为严格大于)for await经由迭代器return()隐式做到;reader 循环必须显式做,测试用无限生产者驱动,漏掉 cancel 无处可藏。已做变异验证:删去 cancel 恰好挂掉这一条。Tested on
Risk & Scope
value === undefined防御针对旧版非区分联合的ReadableStreamReadResult类型;运行时非 done 的读取必有 value。for await迭代的是process.stdin(Node 流,任何 lib 集合下都可异步迭代),未改动。packages/sdk-typescript/src/daemon/DaemonClient.ts在另一个程序里有同模式的 fetch body 迭代;它目前构建正常,暂不改动,在此记录以备同类问题。结构性的另一半——验证门把预先存在/base 偏斜的失败记在 PR 头上并为其烧 18 分钟修复——是后续工作,不在本 PR。Linked Issues
从 #8614 被丢弃的 autofix 轮次(评论)诊断而来。