fix(integration-tests): make the project typecheckable and fix what that found - #8693
Conversation
…hat found `tsc -p integration-tests/tsconfig.json` could not run at all. The config carried a `"//"` documentation key inside `compilerOptions.paths`, and every value there must be an array, so tsc aborted with TS5063 before checking a single file. Nothing in CI runs it either, so the directory has been unchecked for its whole life -- which is how PR QwenLM#8620 shipped an `integration-tests/cli/qwen-serve-streaming.test.ts` that referenced an undeclared `REPO_ROOT`, swallowed the ReferenceError in a bare catch, and reported a green skip for a security regression test. Moving that note out of `paths` exposed 404 errors. Three more config defects accounted for 353 of them: - `composite: true` is inherited from the root config for the packages that are actually referenced. Composite requires every file in the program to appear in `include`, and these tests import package sources by relative path, so it produced 324 TS6307. Nothing references this project and it emits nothing, so it is now `composite: false`. - The root `lib` is ES2023 only. The suite drives browser-side code in `terminal-capture/` and pulls SDK sources that name `WebSocket` and `HeadersInit`, so 21 identifiers resolved to nothing. Now DOM + DOM.Iterable + ES2023, matching packages/cli. - Workspace packages resolved through `packages/core/dist` via a project reference, so with core unbuilt the checker reported a dozen members as missing from `Storage` that are right there in the source. They now resolve from source through `paths`, mirroring packages/cli, and the reference is gone. node-pty declares `types` at the top level but its `exports` map is a bare string with no `types` condition, so nodenext never reached the declarations and every pty handle degraded to `any` -- which is what silently untyped the `data` and `exitCode` callbacks in test-helper.ts. It now resolves through `paths` as well. `@types/jsdom` is added for the one file that uses it; DefinitelyTyped has no release matching jsdom 26 (it jumps 21 -> 27), so this pins the current 28.x. Two real defects fell out of the remaining 51: - write_file.test.ts built a detailed tool-call failure message and passed it to `toBeTruthy()`, which takes no arguments. It was discarded on every failure, leaving only a bare literal. - Two terminal-capture scenarios set `gif: true` inside `streaming`, where the runner never reads it. It is a scenario-level switch. The rest was making an existing `undefined` visible. `readToolLogs()` promised `name: string` for fields copied straight out of telemetry attributes that nothing validates; the stdout fallback can promise them, the telemetry branch cannot, and claiming otherwise just moved the `undefined` past the type checker into the assertions. This is type resolution only. `integration-tests/vitest.config.ts` keeps its own hardcoded aliases onto the built SDK bundle, so the suite still exercises the published-bundle shape at runtime. Not wired into CI here, but not for cost reasons: a cold run of `tsc -p integration-tests/tsconfig.json` takes about 106s on an idle developer box. The program is 2679 files, of which 103 are integration tests and roughly 1100 are package sources their own projects already check, so there is duplicated work available to reclaim by resolving the packages from their built declarations -- but at ~106s it is already cheap enough to gate on as-is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Re-running the gate on the current head after the autofix rounds — nothing below changed my read of the PR, but here is the pass against Template: complete ✓ Problem: real and evidenced, not theoretical. The base Direction: aligned. Restoring a typecheck that has been silently dead is unambiguously good, and it demonstrably caught real defects rather than just churning config. Size: no core-module paths touched — the diff is integration-test files, tsconfigs, and manifests. 0 production-logic lines, so the two-tier core gate is not applicable. Approach: sound, with one cost worth naming. The fix removes the invalid entry, relaxes Risk: no high-risk-path matches; no elevated risk signals. Moving on to code review. 🔍 中文说明在 autofix 轮次之后对当前 head 重新执行门禁——以下结论没有改变我对这个 PR 的判断，但这是针对 模板： 完整 ✓ 问题： 真实存在且有证据，不是理论性的。基线的 方向： 对齐。恢复一个已经静默失效的类型检查显然有益，而且它确实捕获了真实缺陷，而不只是改动配置。 规模： 未触及核心模块路径——diff 全部是集成测试文件、tsconfig 和依赖清单。生产逻辑行数为 0，因此两级核心门禁不适用。 方案： 合理，但有一个需要指明的代价。修复移除了无效项，为这个不产物的检查工程放宽了 风险： 无高风险路径命中；无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review first, then the CI signal. The blocker from the previous round is fixed, and the head is leaner than v1. Code reviewThe earlier blocker — root The head also improved on v1: the four Spot checks against the base sources, all verified:
Two non-blocking notes:
Testing evidenceUnattended CI run — no PR code executed locally; the evidence below is the PR's own CI results for The skips are by design, not casualties:
Not verified: Sandboxed verification would settle the first gap: 中文说明先说代码审查，再说 CI 信号。上一轮的阻塞项已修复，且当前 head 比 v1 更精简。 代码审查：先前阻塞项——根级 本 head 还优于 v1： 对基底源码的抽查全部核实通过：两个缺陷修复仍然属实（ 两条不构成阻塞的提示： 测试证据：无人值守 CI 运行——未在本地执行任何 PR 代码；以下为经 API 获取的 未验证： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean re-review: last round's blocker is fixed and CI-verified, and the head is simpler than v1; the remaining reservations are verification gaps, not code defects. Stepping back: this is still the change I would have proposed. The problem was demonstrated before any code was written (a Honest reservations, none blocking:
With this pass I'm also dismissing my own three earlier change-request reviews: the jsdom blocker is fixed and CI-verified, and the other two reviews objected to the integration suite not running on PR CI — that is this repository's merge-queue design, not a defect this PR introduced, and the PR discloses the gap honestly instead of hiding it. Approving, pinned to the commit under review. The merge queue remains the gate for the suite itself. 中文说明置信度：4/5 —— 复审干净：上一轮的阻塞项已修复并经 CI 验证，当前 head 比 v1 更精简；剩余的保留意见是验证缺口，不是代码缺陷。 退一步看：这仍然是我会提出的方案。问题在写任何代码之前就已证实（ 诚实的保留意见，均不构成阻塞：核心声明—— 本次同时撤销我自己此前的三条请求修改评审：jsdom 阻塞项已修复并经 CI 验证；另外两条针对的是集成套件不在 PR CI 上运行——那是本仓库的合并队列设计，并非本 PR 引入的缺陷，且 PR 诚实地披露了这一缺口而非掩盖。 批准，锚定在被审查的提交上。合并队列仍是套件本身的闸门。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
One blocker: the root @types/jsdom addition leaks into integrations/external-context's compilation and breaks its build (http-client.ts(117,29): error TS2504), failing three CI jobs that were green on the same base minutes earlier. Suggested fix: an explicit "types" allowlist (e.g. ["node"]) in that project's tsconfig, or equivalent isolation — full details in my review comment above. Everything else in the diff looks ready.
一个阻塞项：新增的根级 @types/jsdom 渗入 integrations/external-context 的编译并破坏其构建（http-client.ts(117,29): error TS2504），导致三个 CI job 失败——同一基底几分钟前还是绿的。建议修法：在该项目 tsconfig 中加显式 "types" 白名单（如 ["node"]）或等效隔离——详见上方审查评论。diff 其余部分已就绪。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
@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. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
github-actions
Bot
added
the
review/self-reported
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x), web-shell E2E Smoke (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), web-shell E2E Smoke (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (only cli/qwen-serve-streaming.test.ts was run locally: 5/5 pass).
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (only cli/qwen-serve-streaming.test.ts was run locally: 5/5 pass)。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
— qwen3.8-max via Qwen Code /review (v0.21.7)
…ution paths Address review round 1: - external-context: override `types` to ["node"]. The root @types/jsdom entered its program through vitest's optional jsdom types and injected lib dom, flipping @types/node's fetch globals to DOM variants whose ReadableStream is not async-iterable (TS2504 in http-client.ts), which failed every CI job during the npm ci prepare build. - integration-tests tsconfig: explicit nodenext paths entries for every workspace subpath the program imports (sdk/daemon, 19 acp-bridge subpaths, core goalWire/memoryScopes/userPromptSubmitContext, webui daemon-react-sdk, channel-base); drop the dead `*` wildcards; include **/*.tsx. Typechecks green with the source packages' dists removed. - Relax noPropertyAccessFromIndexSignature in integration-tests and revert the six bracket-access rewrites it forced in SDK sources. - channel-plugin: import channels/base from src and map @qwen-code/channel-base to source so both declarations agree. - qwen-serve-streaming: asAccepted delegates to the SDK's exported isNonBlockingAccepted type predicate instead of a drifted copy. - sleep-interception: tighten blocked predicates to success === false and fix the comment describing them. - Declare jsdom at the root next to @types/jsdom.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed — PR #8693 (round 2)All ten inline findings and the review-level blocker are resolved in code. No conflict work was needed ( Blocker: root
|
| Finding | Disposition | Change |
|---|---|---|
Critical tsconfig.json:47 — @qwen-code/sdk/daemon fell back to dist |
Resolved | Explicit paths entries for @qwen-code/sdk/daemon (+ /daemon/transcript, /daemon/transports); verified with SDK dist removed |
Critical tsconfig.json:49 — acp-bridge subpaths + core goalWire fell back to dist |
Resolved | Explicit entries for every acp-bridge subpath the program imports (19, incl. the divergent-name transcriptReplay/daemonMemoryBudget the diff had missed) and for goalWire, memoryScopes, userPromptSubmitContext; verified with acp-bridge/core dists removed |
Critical tsconfig.json:55 — @qwen-code/webui/daemon-react-sdk had no entry |
Resolved | Entry added; verified with webui dist removed |
Critical channel-plugin.test.ts:45 — value import still pointed at channels/base/dist |
Resolved | Switched to ../packages/channels/base/src/index.js and added the @qwen-code/channel-base paths entry (both halves, as prescribed); verified via typecheck with dist removed and a live single-turn E2E run |
Suggestion qwen-serve-streaming.test.ts:145 — asAccepted duplicated the SDK guard |
Resolved | Now delegates to isNonBlockingAccepted from @qwen-code/sdk/daemon (a real type predicate checking promptId and lastEventId); the drifted copy and its stale justification comment are gone |
Suggestion tsconfig.json:58 — **/*.ts misses .tsx |
Resolved | include now also has **/*.tsx, pulling terminal-capture/skill-review-harness/text-capture.tsx into the program; typecheck stays green |
Suggestion package.json:121 — @types/jsdom declared without jsdom |
Resolved | Root now declares "jsdom": "^26.1.0" (matching the runtime already hoisted from packages/cli) plus the matching lockfile root entry |
Suggestion transcript.ts:139 — SDK production source rewritten for noPropertyAccessFromIndexSignature |
Resolved | Took the reviewer's alternative: integration-tests sets the flag to false (precedent: packages/desktop), and all six bracket-access edits in four SDK source files are reverted to main |
Suggestion sleep-interception.test.ts:25 — comment misdescribed the predicates |
Resolved | Took the tightening option: the three blocked predicates now use call.success === false (symmetric with === true, no longer matching a malformed record with absent success), and the comment matches the shipped predicates |
Suggestion tsconfig.json:18 — webui paths entry surfaces three TS4111 |
Resolved | Discharged by the flag relaxation above; the webui sources need no edits and the program typechecks with webui dist removed |
Additional shrink this round enabled: the three nodenext paths wildcards (@qwen-code/{qwen-code-core,sdk,acp-bridge}/*) were probe-proven dead under nodenext (no extension/index probing on substituted paths — every subpath fell through to the exports map) and were removed along with the flag-only bracket-access churn this PR had introduced in test-helper.ts, acp-cron.test.ts, hooks-advanced.test.ts, and vitest.terminal-bench.config.ts.
Verification
Commands actually run this round, in the checked-out branch state (the runner's node_modules predates the branch's @types/jsdom, so verification used a minimal stand-in @types/jsdom whose two load-bearing behaviors — providing JSDOM types and the lib="dom" reference — match the real package; it will be the real package once CI runs npm ci from the branch lockfile):
npm run build— passed. Full ordered workspace build with@types/jsdompresent; this is the exact sequence CI'sprepareruns duringnpm ci, and external-context'stsc --build(the step that failed CI) now succeeds.npm run typecheck— passed (all workspaces, exit 0).npm run lint— passed (exit 0).npx tsc -p integration-tests— exit 0 with dists present, and exit 0 with the dists of core, sdk-typescript, acp-bridge, webui, and channels/base removed (the source-resolution promise, probe-verified both ways).- Focused Vitest, touched packages:
@qwen-code/sdk— 1464 passed (32 files);@qwen-code/external-context— 166 passed (9 files). - Integration tests after
npm run bundle(all four touched files, run against the bundle):channel-plugin.test.ts— 3 passed (single-turn, session state, second sender; exercises the switchedchannels/base/srcimport path live);cli/qwen-serve-streaming.test.ts— 5 passed with model keys cleared, mirroring the CI no-AK gate (covers theisNonBlockingAccepteddelegation);cli/sleep-interception.test.ts— 4 passed (covers the tightenedsuccess === falsepredicates);cli/qwen-serve-webui-live-journal-recovery.test.ts— 1 passed (the jsdom consumer). npm run generate:settings-schema— not applicable (no settings source changed).
中文说明
已处理的审查反馈 — PR #8693（第 2 轮）
十条行内发现与一条审查级阻塞项均已在代码中解决。无需冲突处理（--conflict false）。在现有分支上提交一次。
阻塞项：根级 @types/jsdom 破坏 integrations/external-context（3 个 CI job 失败）
已在本地复现，根因完整追溯，并采用审查者建议的隔离方式修复。
机制（逐步验证）：
vitest/optional-types.d.ts无条件执行export type * as jsdomTypes from 'jsdom'，而vitest/dist/index.d.ts会 import 它。因此任何types含vitest/globals（根 tsconfig 的默认值）的 TS 工程，只要安装了@types/jsdom就会加载它——types白名单拦不住，因为这个 import 正是经由白名单内的 vitest 类型进来的。@types/jsdom/index.d.ts带有/// <reference lib="dom" />，把 DOM lib 注入工程。- DOM lib 一旦存在，
@types/node的条件式 fetch 全局类型（typeof globalThis extends { onmessage: any } ? ...）翻转为 DOM 变体，其ReadableStream没有[Symbol.asyncIterator]（那在lib.dom.asynciterable里，未被包含）。 integrations/external-context/src/http-client.ts中的for await (const chunk of response.body)于是报出与 CI 完全一致的error TS2504（第 29 列）——已用@types/jsdom替身逐字符复现。
三个失败 job 共享同一条路径：CI 的 npm ci 会执行根的 prepare 脚本，该脚本按序对全部 workspace 执行 npm run build；integrations/external-context 最后构建，其 tsc --build 命中 TS2504。
修复：integrations/external-context/tsconfig.json 现在把 types 覆盖为 ["node"]。它编译的源码不使用任何 vitest 全局（测试文件已被排除），因此斩断了 jsdom 类型进入该工程的唯一路径。验证：修复前精确复现 TS2504；修复后在 @types/jsdom 存在的情况下 npm run build 完整通过（含 external-context）。其他 workspace 无需同样处理——CI 上所有其他构建在 DOM lib 注入的情况下均编译通过（external-context 在构建顺序中最后，其前面全部成功），core/acp-bridge 也在本地复核通过。
行内发现
| 发现 | 处置 | 变更 |
|---|---|---|
Critical tsconfig.json:47 — @qwen-code/sdk/daemon 回退到 dist |
已解决 | 为 @qwen-code/sdk/daemon（及 /daemon/transcript、/daemon/transports）添加显式 paths 条目；已在移除 SDK dist 后验证 |
Critical tsconfig.json:49 — acp-bridge 子路径 + core goalWire 回退到 dist |
已解决 | 为程序实际 import 的全部 acp-bridge 子路径添加显式条目（19 个，含 diff 遗漏的异名条目 transcriptReplay/daemonMemoryBudget），以及 goalWire、memoryScopes、userPromptSubmitContext；已在移除 acp-bridge/core dist 后验证 |
Critical tsconfig.json:55 — @qwen-code/webui/daemon-react-sdk 无条目 |
已解决 | 添加条目；已在移除 webui dist 后验证 |
Critical channel-plugin.test.ts:45 — 值 import 仍指向 channels/base/dist |
已解决 | 改为 ../packages/channels/base/src/index.js 并添加 @qwen-code/channel-base paths 条目（按规定两半都做）；通过移除 dist 后的 typecheck 和一次真实单轮 E2E 运行验证 |
Suggestion qwen-serve-streaming.test.ts:145 — asAccepted 复制了 SDK 守卫 |
已解决 | 改为委托 @qwen-code/sdk/daemon 的 isNonBlockingAccepted（真正的类型谓词，检查 promptId 与 lastEventId）；漂移的副本及其过时注释已删除 |
Suggestion tsconfig.json:58 — **/*.ts 漏掉 .tsx |
已解决 | include 增加 **/*.tsx，把 terminal-capture/skill-review-harness/text-capture.tsx 纳入程序；typecheck 保持绿 |
Suggestion package.json:121 — 声明了 @types/jsdom 却没有 jsdom |
已解决 | 根 package.json 增加 "jsdom": "^26.1.0"（与已从 packages/cli 提升的运行时版本一致），lockfile 根条目同步 |
Suggestion transcript.ts:139 — SDK 生产源码为 noPropertyAccessFromIndexSignature 改写 |
已解决 | 采用审查者的备选方案：integration-tests 将该标志设为 false（先例：packages/desktop），四个 SDK 源文件中全部六处方括号访问改动回退到 main |
Suggestion sleep-interception.test.ts:25 — 注释对谓词描述有误 |
已解决 | 采用收紧方案：三个"被阻塞"谓词改为 call.success === false（与 === true 对称，不再把缺少 success 的畸形记录当作阻塞），注释与谓词一致 |
Suggestion tsconfig.json:18 — webui paths 条目暴露三个 TS4111 |
已解决 | 由上面的标志放宽一并免除；webui 源码无需改动，移除 webui dist 后程序 typecheck 通过 |
本轮还顺势收缩：三条 nodenext paths 通配符（@qwen-code/{qwen-code-core,sdk,acp-bridge}/*）经探针证实是死配置（nodenext 对替换后的路径不做扩展名/索引探测——所有子路径都回退到 exports 映射），连同本 PR 为标志而在 test-helper.ts、acp-cron.test.ts、hooks-advanced.test.ts、vitest.terminal-bench.config.ts 引入的方括号访问改动一并移除。
验证
本轮在当前检出分支状态下实际执行的命令（runner 的 node_modules 早于分支的 @types/jsdom，验证使用了一个最小替身 @types/jsdom，其两个关键行为——提供 JSDOM 类型与 lib="dom" 引用——与真实包一致；CI 按分支 lockfile 执行 npm ci 时安装的是真实包）：
npm run build— 通过。@types/jsdom存在下的全量按序 workspace 构建；这正是 CInpm ci期间prepare执行的序列，其中 external-context 的tsc --build（CI 上失败的步骤）现已成功。npm run typecheck— 通过（全部 workspace，exit 0）。npm run lint— 通过（exit 0）。npx tsc -p integration-tests— dist 齐全时 exit 0；移除 core、sdk-typescript、acp-bridge、webui、channels/base 的 dist 后同样 exit 0（"从源码解析"的承诺已双向探针验证）。- 触及包的聚焦 Vitest：
@qwen-code/sdk— 1464 通过（32 个文件）；@qwen-code/external-context— 166 通过（9 个文件）。 npm run bundle之后的集成测试（全部四个触及文件，针对 bundle 运行）：channel-plugin.test.ts— 3 通过（单轮、会话状态、第二发送者；真实运行了切换后的channels/base/srcimport 路径）；cli/qwen-serve-streaming.test.ts— 清空模型 key 后 5 通过，与 CI no-AK 闸门一致（覆盖isNonBlockingAccepted委托）；cli/sleep-interception.test.ts— 4 通过（覆盖收紧后的success === false谓词）；cli/qwen-serve-webui-live-journal-recovery.test.ts— 1 通过（jsdom 使用方）。npm run generate:settings-schema— 不适用（未改动 settings 源）。
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/模型 qwen3.8-max
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round result: no code changes required (round-2 findings verified / answered)This round triaged review round 2 (one Critical, one Suggestion) plus the review-level "not reviewed" notes. No code change was needed: the Critical had already been fixed by the current head commit ( R2-1 (Critical): missing
|
|
@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: 45 passed · 0 failed · 45 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：45 通过 · 0 失败 · 45 总计 Verification reportPR 8693 verification — fix(integration-tests): make the project typecheckable and fix what that foundVerdict: 中文摘要
Central claim + A/BCentral claim:
Cell C reconciliation: the PR's base measured 404 errors; my cell C measured 385. The 21 missing Secondary A/B — the round-1
The override is load-bearing and the mechanism matches the comment exactly ( Runtime (no-impact claim)
The two real defects, proven:
CorrectionsNone required — the description's claims (TS5063 abort, ~404 exposed errors with the 349/21/4 split, the node-pty Findings (low severity)
Not covered
MethodologyEnvironment: the CI verify container ( Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
Superseded by re-review of 1f66c98: the @types/jsdom leak is fixed via the "types": ["node"] isolation in integrations/external-context/tsconfig.json, and the two jobs that failed on it (Test ubuntu-latest, web-shell E2E Smoke) build green on the new head.
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. ✅
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— deepseek-v4-flash via Qwen Code /review (v0.21.7)
|
@qwen-code /triage |
|
Sandboxed verification: The verification run did not complete, so the phases below may be partial or missing entirely. Advisory evidence for human reviewers — not a review, an approval, or a CI check. 中文 — 判定：
|
| # | Previous finding | Severity | Status at new head a8a6000 |
|---|---|---|---|
| 1 | The new tsc gate is not wired into CI (author's explicit deferral) | Low | Stands. Re-measured: no workflow step runs tsc -p integration-tests/tsconfig.json (grep of .github/workflows shows only vitest runs touching integration-tests/). I still agree with splitting the wiring decision out, but the residual risk — the 0-error state rotting silently, exactly the history this PR fixes — remains until a follow-up wires it. |
| 2 | sleep-interception predicate tightening (!call.success → call.success === false) could quiet-skip when telemetry omits success |
Info | Stands, re-measured, did not materialize. Live run at the new head: 4/4 tests pass (SUITE_REF), i.e. real telemetry still carries explicit booleans. |
| 3 | @types/jsdom 28.x against jsdom 26 (no matching DT line) |
Info | Stands, accepted tradeoff. Lock re-measured: @types/jsdom@28.0.3 + jsdom@26.1.0; sole consumer (qwen-serve-webui-live-journal-recovery.test.ts, the only from 'jsdom' import) ran green in the live suite. |
Central claim + A/B (re-measured at new head)
Central claim: tsc -p integration-tests/tsconfig.json runs at all (base aborts with TS5063 because compilerOptions.paths carried a "//" string key) and completes with 0 errors on head.
| Cell | Tree/config | Observable result | Assertion |
|---|---|---|---|
| A (control) | base 6ddb0307 |
rc=2, TS5063: Substitutions for pattern '//' should be an array, 0 files checked |
expected red → pass (01-ab-base-ts5063-abort.png) |
| B (gate) | head a8a6000 |
rc=0, 0 errors cold (tsbuildinfo deleted), 15.8 s; program = 3189 files, 106 under integration-tests/ | pass (02-ab-head-gate-clean.png) |
| C (intermediate) | base config with only // removed, on head sources |
rc=2, 362 errors: 327×TS6307, 21×TS4111, 6×TS7006, 4×TS7031, 4×TS7016 | config fix alone insufficient → source fixes load-bearing |
| D1 (live gate) | head minus the write_file fix | rc=2, exactly one error: write_file.test.ts(32,7): TS2554 Expected 0 arguments, but got 1 |
gate catches regression of a fixed shape |
| D2 (live gate) | head + planted undeclared identifier | rc=2, TS2304 Cannot find name 'UNDECLARED_IDENTIFIER_PR8693_PROBE' |
the #8692 bug class is now caught |
Cell C reconciliation with the previous round's 385: the TS6307 count moved 349 → 327 and the lone TS2322 disappeared because both the base tip and the PR sources advanced between rounds; the invariant (config fix alone leaves hundreds of errors; every non-TS6307 family identical: 21/6/4/4) is unchanged. The 21 window/WebSocket TS2304s the PR's own base saw remain masked here by the root @types/jsdom lib.dom leak through the inherited types chain — the same mechanism the external-context override plugs (below).
Delta this round: build-independence for the channel packages
Round-3 commit claim: the eight builtin channel adapters (imported by cli's channel-registry.ts) and web-templates (imported by cli's html.ts) still resolved through their exports maps to dist, so on a tree without built dists the gate failed with 9×TS2307. The commit maps all nine to source.
A/B in a dist-free state (the nine packages/channels/*/dist + packages/web-templates/dist directories removed from the tree, so resolution cannot fall back to build output):
| Cell | Config | Result |
|---|---|---|
| DELTA-control | head config minus exactly the 9 round-3 entries (channel-base entry kept) |
rc=2, exactly 9× TS2307 — one per adapter/web-templates import, matching the maintainer measurement the commit cites (03-delta-distfree-control-9x-ts2307.png) |
| DELTA-head | full head config | rc=0, 0 errors, dist-free (04-delta-head-distfree.png) |
Program census at head (independent corroborating instrument, --listFilesOnly): 0 workspace dist files in the 3189-file program — the only /dist/ hits are third-party declarations under node_modules. The typecheck now consumes no workspace build output at all.
Delta this round: single channel-base runtime instance
Round-3 claim: channel-plugin.test.ts importing @qwen-code/channel-base by bare specifier (instead of a relative src path) makes the test and plugin-example resolve the same dist/index.js through the exports map — one ChannelBase/SessionRouter instance where round 2's relative-src import silently split two.
| Probe | Result |
|---|---|
Static resolution (import.meta.resolve from both import sites) |
both resolve to file:///…/packages/channels/base/dist/index.js, same realpath (logs/resolve-identity.txt) |
| Runtime identity under vitest (probe test, deleted after) | 2/2: Object.getPrototypeOf(MockPluginChannel.prototype) === ChannelBase.prototype via the bare import — and the old relative-src import resolves to a distinct prototype, proving the round-2 split was real and the fix is what removed it (05-channel-identity-probe.png) |
Live channel-plugin.test.ts |
passed in the runtime suite (SUITE_REF) |
integration-tests/vitest.config.ts |
byte-identical across the PR (0-line diff) — no runtime alias added or removed; resolution flows through the package exports map as claimed |
Also surfaced by the now-live gate in this file: dmPolicy is a required ChannelConfig field (packages/channels/base/src/types.ts:71); the old fixture omitted it — invisible while the directory went unchecked. The PR adds dmPolicy: 'open', which matches the documented runtime default, so runtime behavior is unchanged. The MockPluginConfig retyping restores visibility of the required serverWsUrl.
Corrections
None required this round — the round-3 commit's claims (the 9×TS2307 shape, the exports-map fallback for unmapped subpaths, the instance split) all matched measurement.
Findings
FINDINGS
Not covered
NOT_COVERED
Methodology
METHODOLOGY
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Review-thread cleanup for current head
The PR description was also refreshed to match the current 19-file head, direct jsdom dependency, actual verification evidence, and remaining out-of-scope coverage. No new commit was needed in this cleanup pass. |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round result: no code changes required — PR #8693Feedback triaged this roundEverything newer than the last evaluation (2026-08-08T11:31:27Z) was reviewed against the raw PR payloads and the current head
Advisory findings from the verification report — dispositions
ResultNo code changes were made this round and no verification commands were needed (nothing was modified). Head remains 中文说明Autofix 本轮结果：无需代码改动 — PR #8693本轮分类处理的反馈对照原始 PR 数据与当前 head
验证报告中的建议性 findings —— 处置
结果本轮未做任何代码改动，因此也无需运行验证命令（没有任何修改）。Head 保持为 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. Verified at head a8a6000: the invalid // paths entry is gone and every remaining paths value is a proper array mapping subpaths to source declarations; the two exposed defects are genuine fixes — the GIF switch now sits at scenario level where the runner actually reads config.gif !== false (the old placement on a streaming step was inert), and the failure diagnostic now reaches vitest through the matcher message. The telemetry-shape edits preserve well-formed behavior (call.success === false refuses malformed records rather than false-matching), and nothing under packages/*/src is touched — integration-tests/, one integration tsconfig, and devDependencies only. CI green on this head.
One non-blocking follow-up worth an issue so it doesn't get lost: the restored typecheck isn't wired into any workflow yet, so the same silent rot (invalid paths entry, missing subpath mapping) could recur unnoticed — I see this is deliberately deferred, just make sure it's tracked. Nothing blocks merge.
|
Released in v0.21.8. |
Cherry-picks the tsconfig guard from 3037744 (#8693), which postdates this branch's merge base. With @types/jsdom installed, the inherited vitest/globals types drag the DOM lib into this program and break the for-await over response.body in http-client.ts, failing npm run build and npm run typecheck.
…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.
…await (QwenLM#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: QwenLM#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" />. QwenLM#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 QwenLM#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 (QwenLM#8614 reached attempt 13 that way; QwenLM#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 (QwenLM#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>
…enLM#8614) * feat(web-shell): add fullscreen view for the right artifact panel The right panel (artifacts / subagents / review) is narrow, so long previews and transcripts are hard to read. Add a fullscreen toggle to the panel header that expands the panel to cover the viewport; pressing Escape or the toggle again shrinks it back to its dock or drawer. * test(web-shell): capture fullscreen artifact panel in visual previews The fullscreen toggle and surface are only reachable with the artifact panel open, so no visual scenario rendered them and the before/after preview could not see this feature. Extend the code review artifact scenario to expand the panel fullscreen, capture it, and assert the Escape restore path. * fix(web-shell): keep artifact panel mounted across fullscreen toggle (QwenLM#8614) * fix(web-shell): harden artifact panel fullscreen per review feedback (QwenLM#8614) * fix(web-shell): guard artifact drawer Escape for IME composition (QwenLM#8614) * fix(web-shell): scope toast z-index, reset dock animation flag, guard IME keyCode (QwenLM#8614) Round-4 review fixes for the artifact panel fullscreen: - Elevate ToastHost above dialog-backdrop-tier surfaces only while the fullscreen surface is up (new `elevated` prop); otherwise it stays at its original z-index 30 so DialogShell modals keep painting above toasts. The old comment rested on DialogShell.module.css's dead `.backdrop` class — the live modal path resolves the same host variable with fallback 50. - Reset suppressArtifactDockOpenAnimation when the dock unmounts (panel close or floating drawer taking over) and only set it while docked, so a floating<->docked flip after a fullscreen round-trip no longer mounts the dock without its slide-in animation. - Extend the window Escape handler's IME guard to keyCode 229, matching the paired guard used everywhere else in the package: a WebKit-shaped IME Escape (isComposing false, keyCode 229) previously shrank a fullscreen panel and swallowed the native IME cancel. Adds the suggested coverage: keyCode-229 drawer variant, docked-fullscreen IME Escape, ask-user shrink keyboardActive history, streaming+fullscreen Escape ordering, and the floating-interlude dock animation regression. * fix(web-shell): portal docked fullscreen surface, gate chat shortcuts, clamp z-index (QwenLM#8614) Round-5 review fixes for the artifact panel fullscreen: - Clamp the fullscreen surface z-index floor to max(1, backdrop - 10): a host setting --web-shell-dialog-backdrop-z-index below 10 gave the surface a negative z-index, painting the opaque panel behind the app background and blanking the shell. - Add artifactPanelFullscreen to interactionBlocked: chat-only global shortcuts (Ctrl+L/O/Y, Shift+Tab, the btw hotkey) kept mutating the hidden chat behind the surface, and the btw capture-phase Escape handler dismissed hidden content and swallowed the Escape that shrinks the panel. - Pad the floating drawer fullscreen content with env(safe-area-inset-*): the portaled drawer sits outside the padded app root, so on viewport-fit=cover devices the toolbar and Exit control could sit under the notch/status bar and bottom content behind the home indicator. - Move the docked fullscreen surface into the top-level portal root and give it document-level modal semantics: a transformed, paint-contained, or lower-stacking host ancestor could bound the fixed panel or paint over it, and Tab could escape into covered host controls. The panel wrapper portals into a display:contents slot that the fullscreen effect parks in the portal root, so the SAME node survives the move (panel state preserved, React event delegation intact); FocusScope provides the Tab containment and the effect hides every outside tree from AT and captures stray focus — matching what the floating variant gets from vaul's Radix dialog. Declares @radix-ui/react-focus-scope (already in the tree via radix-ui) as a direct dependency. - Portal the elevated ToastHost into the portal root: in shadow-DOM portal mode the fullscreen drawer surface is sealed inside the portal host's stacking context, so an in-tree toast painted beneath it for its whole auto-dismiss lifetime. - Keep the dock animation suppression flag across dock<->floating hand-overs performed mid-fullscreen in both directions, so shrinking back to the dock never replays the slide-in on the already-open panel. Tests: pin both hand-over suppression directions (mutation-verified), the portal placement, the interaction gate, and toast elevation in the main fullscreen test; add a colocated ToastHost.test.tsx for the elevated class; fix the dead 0-measurement step in the docked-width round-trip test (the 0 is now seeded before entering fullscreen, where the clamp effect can actually observe it). Adds a drawer-fullscreen visual scenario at a narrow viewport in both themes, with Escape restoring the drawer. * fix(web-shell): repair docked fullscreen panel modal semantics (QwenLM#8614) * fix(web-shell): reset fullscreen state in the panel close commit (QwenLM#8614) Closing the artifact panel while fullscreen reset the fullscreen flag only in the passive management effect, so one render committed with the panel unmounted while the covered shells stayed display:none — one painted frame of an empty shell before the chat reappeared. Batch the fullscreen and dock-animation-flag resets into closeArtifactPanel and the last-tab close so the recovery happens in the same commit. Adds a regression test asserting the shells are revealed in the close's committed frame (fails without the batched reset), and pairing coverage for the drawer Escape pass-through: a plain Escape still closes the floating drawer when the panel was never fullscreen. * fix(web-shell): pull escaped focus back into the docked fullscreen surface (QwenLM#8614) * build(external-context): restore the node-only types override from main Cherry-picks the tsconfig guard from 3037744 (QwenLM#8693), which postdates this branch's merge base. With @types/jsdom installed, the inherited vitest/globals types drag the DOM lib into this program and break the for-await over response.body in http-client.ts, failing npm run build and npm run typecheck. * fix(web-shell): keep docked fullscreen usable in shadow DOM (QwenLM#8614) Resolve two Critical review findings on the docked fullscreen surface: - focusin is composed, so in shadow-DOM portal mode the document-level pull handler and the Tab-wrap handler saw the shadow host retargeted as the target and snapped every focus change back onto the surface. Resolve the real node via composedPath() and read the active element from the surface's own root. - The aria-hidden sweep recorded values still owned by a live hideOthers lock when the floating drawer unmounted in the same commit the docked surface mounted; the deferred restore then re-applied the lock's value and permanently hid the shell from assistive tech. Record null for lock-owned siblings and let the lock's unlock restore the original value. Adds regression tests for both paths; both fail without their fix. --------- Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
…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.
…eak (QwenLM#8816) * feat(ci): A/B deterministic gate rejections against the pre-round ref A deterministic rejection in the autofix verification gate is only chargeable to the round if the same check passes without the round's commit. The gate charged every red to the fix unconditionally, and run 31276008548 measured what that costs when the premise is false: PR 8614's branch predated QwenLM#8693's tsconfig guard while node_modules came from the post-QwenLM#8693 trusted base, so `npm run build` was equally red at origin/<branch> — 63 minutes of accepted agent work discarded, an 18-minute repair burned on a failure the repair agent is forbidden to touch (it may only amend the round's own fix), thirteen rounds in a row, and the same again on the QwenLM#8616 leg. On rejection the gate now re-runs the failing check at origin/<branch> (the branch as pushed, before the round) in the same environment: - baseline green: today's path exactly — outcome=failed, retryable=true, the repair pass gets its chance. - baseline red too: outcome=failed with preexisting=true and NO retryable. The repair step keys on retryable and is skipped — it cannot reach a failure outside the round's diff by construction — and gate-rejection.md says outright that the branch needs a base update (merge main), which flows into the failure comment as-is. Fail-closed toward today's semantics: any A/B infrastructure problem (missing ref, checkout failure) charges the fix as before, and a restore failure after the baseline run rejects outright since the tree can no longer be trusted. The round's work is still not pushed — this changes the verdict's honesty and cost, not the push policy. Tested by executing the real script in a real two-remote git repo with an npm stub whose failures are keyed by commit SHA: round-caused red (baseline green), pre-existing red (both red), and the untouched green path. Mutation-tested, 3 of 3 caught: skipping the A/B, claiming pre-existing without measuring, and dropping the tree restore. * Address review: bound the A/B to checks it can honestly compare All seven findings verified before fixing; the three Criticals were each a way the A/B compared something other than the check that failed. R1-1 — the contracts check feeds on stdin, which its first run drains; the baseline leg re-ran against EOF and checked an empty file list. R1-3 — the schema check's verdict rides on packages/core/dist, which the core-rebuild guard built from ROUND sources and which, being gitignored, survives the detach. Both checks are now A/B-exempt (run_check_no_ab): their baseline verdicts prove nothing, and their rejections stay where the repair agent can actually act on them. R1-2 — a workspace the round ADDS does not exist at the baseline, and npm exits 1 there with "No workspaces found" (measured; --if-present forgives a missing script, not a missing workspace) — a round-caused failure misread as pre-existing, skipping the one repair that can fix the round's own package. The per-package loop now A/Bs only when the workspace exists at origin/<branch>. R1-4 — a chatty PASSING baseline used to flood the tail -c 3000 evidence window and push the actual failure text out of gate-rejection.md, the sole carrier into the repair feedback, the PR comment, and the next round's LAST_REJECTION. The baseline transcript now goes to a side log and only a FAILING tail is merged back, where it is the evidence. R1-5 — the pre-existing paragraph pushed gate-rejection.md past the report's head -c 3500 cap, truncating the closing fence for branch names past 44 characters. Cap raised to 3900, invariant comment updated with the new arithmetic. R1-6 — preexisting=true had no read site. It now flows verify → Finalize verification → the failure report, whose headline swaps the generic gate clause for "PRE-EXISTING failure … needs a base update (merge main)". R1-7 — the no-round-commit guard was unpinned (deleting it kept all tests green). Now exercised through the core-rebuild path, the one A/B-eligible check that runs before the commit gate. Four new behavioral scenarios (chatty baseline, no-commit round, A/B-exempt checks, round-added workspace) plus workflow pins for the forwarding, the clause, and the cap. Mutation-tested, 4 of 4 caught: schema back to A/B (3 tests), guard dropped, side log reverted, no-commit guard dropped. * Address review round 2: A/B only what it can prove, prove what it claims Ten findings across two rounds, each verified before fixing. The three deepest share one lesson: the A/B is only sound for a check whose inputs travel entirely with the git ref, and whose failure it can IDENTIFY, not merely observe. R2-1 — rc=1 at both legs does not make them the same failure: the branch can fail for reason A while the round fails for reason B, and a baseline infrastructure hiccup is a nonzero exit too. Pre-existing now requires a MATCHING failure identity — tsc diagnostics normalized to file + error code (positions shift with the round's edits), compared via comm(1) on a per-check transcript. No diagnostics on either side means identity cannot be established and the round stays charged. R2-2 / R2-7 — gitignored dist survives the detach carrying the ROUND's build, so any dist-consuming check A/Bs reverted sources against round-built artifacts: package tests (channel-base resolved through dist exports) and typecheck (sdk-typescript resolves core's d.ts — probe-verified three-arm flip). Both are now A/B-exempt, as is lint, leaving `npm run build` — the incident class, and the one check that rebuilds its own inputs from the checked-out sources — as the sole A/B candidate. The workspace-existence guard dissolves with it. R2-3 — the fixture inherited the caller's global git config; a failing global pre-commit hook broke all seven cases. The harness now isolates GIT_CONFIG_GLOBAL/SYSTEM for every git child, and the suite is proven green under a deliberately hostile hooksPath. R2-4 — Finalize verification now selects preexisting from the same attempt whose outcome it selects (repair verification included). R2-5 / R2-8 — the "merge main" advice is now conditional at both layers: the script paragraph states the measured fact and hedges the remedy; the report headline uses the compare the step already ran — behind/diverged gets the base-update clause, an up-to-date branch is told its own pre-round code needs attention. R2-6 — the rejection document now sizes its evidence tail against its preamble (floor 500 bytes, total under the 3900-byte render cap), so the closing fence can no longer be truncated off by a long branch name. R2-9 — dissolved by R2-2: package tests no longer A/B, the guard and its uncovered positive branch are gone. R2-10 — the baseline-evidence merge is now pinned: the pre-existing scenario asserts the baseline leg's own failure line (keyed by its SHA) reaches gate-rejection.md. Eight behavioral scenarios; mutation-tested 5 of 5: identity dropped, typecheck re-enrolled, package tests re-enrolled, evidence merge dropped, fixed tail restored. * Address review round 4: sharpen identity, stage the git failures, sync prose Nine findings, all refinements — the design held, the edges did not. Identity now keeps the diagnostic MESSAGE (file + code collide: two unrelated TS2339s in one file compared equal, skipping a repair that could have shipped — probe-reproduced by the review), and the fixture emits a SHIFTED position on the baseline leg so the position strip is load-bearing instead of decorative (deleting the sed survived every test before; it fails one now). vite/esbuild failures still yield an empty signature by design — documented as the fail-closed limit rather than half-widened. The fail_signature assignments take `|| true`: grep exits 1 on the normal no-match case and survives errexit today only because the caller sits in an if-condition — a future unconditional call site would crash the gate verdict-less. The restore-failure branch is now stageable and staged: the baseline leg recreates (untracked) a file the branch tracks, the checkout back refuses, and the test pins retryable-not-preexisting with the 'could not restore' label. Relaxing the branch to `|| true` fails it. Prose synced to the mechanisms that replaced it: the render-cap invariant restates against the dynamic tail budget (the old 3000-based arithmetic would misguide the next retune), the no-round-commit guard comment names the core rebuild (schema/contracts left the A/B last round), the describe wording counts both A/B-eligible builds, and the pre-existing clauses no longer claim "the repair pass was skipped" — with REPAIR_PREEXISTING forwarded, repair may have RUN; they now state the invariant that is true either way: repair may only amend the round's own fix, so it cannot reach this failure. Mutation-tested, 3 of 3 caught: position strip dropped, message dropped from the identity, restore rejection relaxed. * fix(ci): watchdog silent sandbox hangs and reap the containers they leak Four autofix rounds have died the same way (QwenLM#8663 twice, QwenLM#8761 r3, QwenLM#8763 r4): the agent's last output is the sandbox wrapper's "ContainerName (regular): …" line at docker container entry, then nothing — not one event — until the 2-hour absolute budget kills the round. Four different runners, two image versions: systemic, not a bad machine. Where exactly the container wedges is still unknown (that needs docker state on the runner); what is certain from the logs is the shape — a wedged sandbox produces NOTHING, and a legitimate run is never silent for long (the fleet's longest tolerated quiet is the review pipeline's 10-minute stream-idle window for thinking phases). Two mitigations, each aimed at a measured half of the damage: - run-agent.mjs gains an idle watchdog (QWEN_IDLE_TIMEOUT_MS, default 20 minutes = 2x that longest legitimate silence): zero output for the window kills the agent with a distinct "idle-timeout … the sandbox likely hung at startup" detail, so the failure comment names the right knob and a hung round costs 20 minutes instead of 120. Polled, not reset-per-chunk — a busy stream should not spend its time re-arming timers. - Both sandboxed jobs reap stale qwen-code-* containers at job start: a budget kill reaps the HOST-side docker client, not the container, so every killed sandbox keeps running on the persistent runner — observed directly when a later leg's container-name counter found qwen-code-0.21.8-0 already occupied and picked -1. One job per runner at a time makes any container alive at job start stale by definition. Tested by executing the real run-agent.mjs end to end with stub agents: the hang shape (one line, then silence) dies at the idle window naming the idle limit, and a slow-but-talking agent that outputs every 400ms across a 1500ms window survives to a clean exit — the test that distinguishes a watchdog from a disguised absolute timer. Mutation- tested, 3 of 3 caught: watchdog disabled, last-output tracking dropped (the disguised-timer regression), cleanup dropped from a job. * Address review round 5: the gate's verdict defects and the reaper's live kill Budget-warning round — the five Criticals from both reviewers, no suggestions (each deferred with a recorded reply). fail_signature: `[^\n]*` in an ERE bracket expression does not mean "rest of line" — in POSIX bracket expressions `\` is literal, so it matched "neither backslash nor the letter n" and truncated every tsc message at its first n. Nearly every real message has an early n ("Cannot find name", "is not assignable"), so distinct same-file failures collapsed into identical signatures and a round-caused failure could be labeled pre-existing, skipping the repair. grep is line-oriented: `.*` is exactly the rest of the line. New fixture: two messages differing only after their first n. Pre-existing verdict: the intersection test mislabeled in both directions. A round that ADDS a diagnostic sharing one normalized line with the baseline was called pre-existing (repair skipped for a round-caused, repairable failure); and `comm -12 | grep -q` under `set -eo pipefail` SIGPIPEs comm (exit 141) once the shared output outruns the pipe buffer, charging true pre-existing failures to the round — the exact 18-minute repair waste the gate exists to kill. Pre-existing now means the round's failing set is a SUBSET of the baseline's, and the difference is captured before testing. New fixture: a round adding a second diagnostic to a failing baseline. Restore failure after the baseline leg: was retryable=true with HEAD still detached at the baseline commit — the repair agent works in that very checkout and does no git recovery, so its commit would land on the baseline and be orphaned. Now rejected non-retryable (reject_fix grows a third arg); the next round starts clean from the trusted checkout. The restoreClash test pins the new semantics. Stale-container reap: the premise "a runner runs one job at a time, so any live qwen-code-* container is stale" holds per runner registration, but the filter queries the docker daemon, which is per host — and this pool runs several registrations on one OS. With per-issue/PR serialization only, a concurrent job's sandbox is a substring match away from `docker rm -f`. The reap now takes only provably-dead containers (--filter status=exited/dead, both jobs) and the comment says why a running one is left alone. Preamble printf: the `\`` escapes sat inside a single-quoted format where backslash is literal, so every pre-existing rejection rendered raw backticks instead of code spans (shellcheck SC2016). Backticks need no escaping there. Also syncs the side-log comment to the dynamic tail_budget it actually renders. Verified: scripts suite 140/140 (was 138; the two new fixtures and the rewritten restoreClash test all fail against the pre-fix script), npm run build / typecheck / lint pass, bash -n clean. * Address review round 6: reap the kill's own orphan, tolerate the reaper * Address review: hang-bound the reaper, unblock the kill path, pin the unpinned arms - Wrap every docker call in the stale-container reap with timeout 30: an alive-but-wedged daemon blocks docker ps indefinitely, and the existing || guards only catch nonzero exits, not hangs (R3-1). - Make the kill-path container removal async in run-agent.mjs: the spawnSync blocked the event loop between SIGTERM and the 10s SIGKILL backstop for up to its 30s timeout — in exactly the wedged-daemon scenario the watchdog exists for. The main flow awaits the removal so the leak warning stays deterministic (R3-6). - Split the pre-existing gate clause for an empty CMP_R: a transient compare-API failure is "never measured", not "measured not-behind", and must not assert the branch's own code is at fault (R3-7). - Swap the timeout breaker's closing remedy to the sandbox investigation when every counted timeout was idle, mirroring the round-level split (R3-11). - Tests: pin the budget kill path separately from the idle kill path (R3-3), parameterize the idle-window parse guard over -1/0/NaN (R3-5), add a stderr-only liveness case (R3-12), pin the strict-subset A/B arm via a baseline-superset fixture knob (R3-15), and pin the breaker's current-round idle increment (R3-18). --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
…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
Fixes #8692. This PR makes
tsc -p integration-tests/tsconfig.jsonexecute instead of aborting before checking any files, and brings the integration-test project to 0 type errors. It removes the invalid documentation entry fromcompilerOptions.paths, configures the no-emit cross-repository program without composite constraints, keeps package source files under their own property-access rules, adds the browser libraries used by the tests, maps imported workspace subpaths to source declarations so the result does not depend on stale build output, and declares both the jsdom runtime and its available type package directly.The typecheck also exposed two real test defects: a detailed failure diagnostic was passed to a matcher that ignores arguments, and two terminal-capture scenarios placed the GIF switch where the runner never reads it. The remaining test edits make existing optional telemetry shapes explicit or improve type narrowing; they are intended to preserve behavior for well-formed telemetry. Production runtime behavior is unchanged.
Why it's needed
The integration-test project had an invalid
pathsentry whose string value caused TypeScript to stop withTS5063before checking a single source file. The regular repository typecheck does not cover this directory because it is not an npm workspace, while TypeScript-oriented ESLint rules do not detect undeclared runtime identifiers.This gap already allowed a real failure to ship: #8620 introduced an undeclared
REPO_ROOTin an integration test. A developer run swallowed the resultingReferenceErrorand reported a security regression test as a green skip, while CI crashed the module and four pre-existing tests with it. Restoring a runnable typecheck catches this class of failure before runtime.Reviewer Test Plan
How to verify
From a clean checkout with dependencies installed and package build artifacts absent, run
npx tsc -p integration-tests/tsconfig.json --pretty false; the PR head should exit 0 with no errors, while the base branch stops immediately withTS5063. Then build and bundle the repository and run the touched integration-test files; the corrected diagnostic, streaming, web UI journal, and channel-plugin paths should pass without relying on stale declarations.Maintainer verification on head
a8a6000aadused two isolated worktrees with independentnpm ciinstalls and recorded 17/17 scripted assertions passing, including the base/head typecheck comparison, clean-tree source resolution, build, bundle, and 10/10 tests across four touched integration-test files: #8693 (comment)Evidence (Before & After)
N/A — this is test infrastructure and type-safety work with no user-visible UI change.
Tested on
Environment (optional)
Maintainer verification used Node 22.23.2 and TypeScript 5.8.3 in isolated worktrees with independent dependency installations. The PR's GitHub checks are green; platform-specific checks that are merge-queue-gated remain skipped by design on the pull-request event.
Risk & Scope
Linked Issues
Fixes #8692.
中文说明
本 PR 做了什么
修复 #8692。本 PR 让
tsc -p integration-tests/tsconfig.json能够真正执行，而不是在检查任何文件之前中止，并将集成测试工程的类型错误降为 0。改动移除了compilerOptions.paths中无效的文档项，为这个不产物、跨仓库源码的检查工程关闭 composite 约束，保留各 package 自己的属性访问规则，加入测试所需的浏览器库，把实际导入的 workspace 子路径映射到源码声明以消除对陈旧构建产物的依赖，并直接声明 jsdom 运行时及当前可用的类型包。类型检查还发现了两个真实的测试缺陷：一处详细失败诊断被传给了会忽略参数的 matcher，另有两个 terminal-capture 场景把 GIF 开关放在 runner 不会读取的位置。其余测试改动用于显式表达已有的可选 telemetry 形状或加强类型收窄；对结构正常的 telemetry 预期保持行为不变。生产运行时行为不变。
为什么需要
集成测试工程的
paths中存在一个无效项，其字符串值导致 TypeScript 在检查任何源码前就以TS5063退出。仓库常规类型检查覆盖不到该目录，因为它不是 npm workspace；面向 TypeScript 的 ESLint 规则也无法发现运行时未声明标识符。这个盲区已经放出过真实故障：#8620 在集成测试中引入了未声明的
REPO_ROOT。开发者运行时产生的ReferenceError被吞掉，使安全回归测试显示为绿色跳过；CI 则在模块加载阶段崩溃，并连带中断同文件已有的四个测试。恢复可运行的类型检查后，可以在运行前捕获这类问题。审查者测试计划
如何验证
在干净 checkout 中安装依赖并确保没有 package 构建产物，然后运行
npx tsc -p integration-tests/tsconfig.json --pretty false；PR head 应以 0 退出且无错误，而基分支会立即以TS5063中止。随后构建并打包仓库，再运行被触及的集成测试文件；修正后的诊断、streaming、Web UI journal 和 channel-plugin 路径应通过，且不依赖陈旧声明。维护者已在 head
a8a6000aad上使用两个隔离 worktree 和各自独立的npm ci安装完成验证，17/17 条脚本断言通过，包括 base/head 类型检查对比、无构建产物时的源码解析、构建、打包，以及四个被触及集成测试文件的 10/10 测试：#8693 (comment)证据（前后对比）
N/A——这是测试基础设施与类型安全改动，没有用户可见的 UI 变化。
测试平台
环境（可选）
维护者验证使用 Node 22.23.2 和 TypeScript 5.8.3，并在隔离 worktree 中分别独立安装依赖。PR 的 GitHub 检查为绿色；仅在合并队列触发的平台检查按设计在 pull-request 事件中保持跳过。
风险与范围
关联 Issue
修复 #8692。