fix(web-shell): prevent terminal query freezes and clean up protocol rejection - #11748
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Focused verification on macOS / Node 22.22.0:
The xterm test exercises the real compiled module and parser under jsdom, not a full Vite build or real-browser E2E. Existing copied dependencies included a Vite-private esbuild 0.28.2 override, so the regression deliberately uses the repository's locked root compiler (0.25.6). Web Shell typecheck remains unverified because SDK daemon build declarations are missing in this worktree. Windows native behavior was not rerun. 中文macOS / Node 22.22.0：TerminalPanel 22 项、registry 43 项、package assets 38 项、真实 xterm 压缩回归 1 项通过，共 104 项。ES2020 负向对照在 xterm 测试覆盖真实编译模块及 jsdom 中的 parser，不是完整 Vite 构建或真实浏览器 E2E。依赖副本中的 Vite 私有 esbuild 曾升级到 0.28.2，因此回归明确使用仓库锁定的根版本 0.25.6。新工作树缺 SDK daemon 构建声明，Web Shell 类型检查尚未验证通过；Windows 原生行为未重跑。 |
|
Thanks for the PR! Template looks good ✓ — every required section is present with real content. (The opening heading reads Problem: two things are bundled here and they don't have the same evidentiary standing.
Direction: aligned. This is the tail of #11643, which landed as Size: 12 production lines — Approach: scope is right, with one honest question. The five items are a grab-bag, but they all trace back to #11643, so bundling them is defensible. The heavy part is the new 97-line Risk: no elevated risk signals — none of the changed non-test files match the revert-correlated path list. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 所有必需章节都有实质内容。（开头标题写的是 问题： 这里捆了两件事，证据强度并不相同。
方向： 对齐。这是 #11643 的收尾，该 PR 已作为 规模： 生产代码 12 行—— 方案： 范围合理，但有一个坦率的疑问。五项改动是个组合包，不过都能追溯到 #11643，捆在一起说得通。最重的是新增的 97 行 风险： 无升级风险信号——改动的非测试文件均未命中与 revert 相关的路径列表。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewStatic review only — this run does not execute any PR-derived code (see Testing evidence below for what that means for the claims).
Considered and dismissed — injecting a control frame into an old daemon's shell. This branch fires precisely when the daemon is too old to have sent a snapshot control frame, so I checked whether i18n — Registry test — package-assets test — Docs — the EN and zh-CN additions correspond one-to-one (the offline-query boundary, and release-before-reject with a localized notice), both updated in the same change. ✓ One finding worth your attention (non-blocking)The new build test doesn't exercise the compiler that builds the shipped bundle.
The new test sits in So the test proves None of this makes the fix wrong: Minor, and not worth a change on its own: Testing evidenceWhat this comment carries: static review of the diff plus the PR's own CI check results read through the API. This is an unattended Nothing has failed so far, but the four jobs that would actually exercise this diff were still in flight when I read them, so there is no green signal to report yet. I don't poll; the finalize workflow rewrites the table below once CI settles.
Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Skipped entries above are the platform matrix legs this PR didn't trigger plus bot-orchestration jobs ( Not verified, explicitly:
Sandboxed verification would settle this: Real-scenario testing (2c): N/A — unattended CI run, so no tmux capture was driven and none is claimed. 中文说明代码审查仅静态审查——本轮不执行任何来自 PR 的代码（这对各项论断意味着什么，见下方"测试证据"）。
已考虑并排除——把控制帧当成输入注入旧 daemon 的 shell。 这个分支恰好在 daemon 旧到没发快照控制帧时触发，所以我确认了 i18n—— registry 测试—— package-assets 测试—— 文档——英文与中文的新增内容一一对应（离线期间查询的边界、以及拒绝前释放并显示本地化提示），在同一次改动中同步更新。✓ 一个值得注意的问题（非阻塞）新增的构建测试并没有使用真正构建发布产物的那个编译器。
新测试位于 所以该测试证明的是： 这并不说明修复本身有问题： 次要问题，单独看不值得改： 测试证据本评论承载的证据： 对 diff 的静态审查，加上通过 API 读取的 PR 自身 CI 检查结果。这是一次无人值守的 目前没有任何失败，但真正会验证这个 diff 的四个 job 在我读取时仍在运行，所以还没有可报告的绿灯信号。我不做轮询；CI 结束后 finalize workflow 会就地重写下方表格。
上方表格中被 skip 的条目，是本 PR 未触发的平台矩阵分支，以及本次运行不适用的 bot 编排 job（ 明确未验证的部分：
沙箱验证可以解决这一点： 真实场景测试（2c）：N/A——无人值守 CI 运行，因此没有驱动 tmux 抓取，也不作此宣称。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 2 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
Confidence: 3/5 — the code is clean and safe; what I can't settle is whether the headline fix actually cures the observed freeze in the bundle we ship. Going back to what I'd have written myself: for the freeze I'd have raised the build target so the buggy lowering never runs, plus a regression test that compiles the real dependency rather than asserting a config string — which is what this does, and its behavioural oracle is better than the The reservation is that the PR's own verification stops short of the thing it claims to fix. Production builds with nested Vite 5.4.21 and its own esbuild 0.21.5; the new harness compiles with root Vite 7.3.6 and root esbuild 0.25.6. The freeze was observed pre-fix by the maintainer in a real browser against a real daemon, and nobody has observed the post-fix production bundle — the description is upfront about that ("does not repeat that full E2E run", "real-daemon browser E2E are not claimed"). So the causal chain is: a real bug was seen, a plausible mechanism was named, a fix was applied that addresses that mechanism in a different compiler than the one that produced the symptom, and the evidence offered is from that different compiler. Two reasons I'm not treating that as a blocker. The change is monotonic — lifting the floor above the Two reasons I'm not approving either. Green CI won't close the gap — Not volume fatigue and not vagueness — this is one PR and my hesitation is specific and nameable. What would move me to a 4: either ⏸️ Deferring to @BenGuanRan — needs a human call on whether the unverified production-toolchain link above is acceptable to merge as-is, or whether the harness should be repointed first. Two Nothing in this PR is broken and I'd merge it after the harness repoint. Flagging rather than blocking because the author is a maintainer and the call about verification depth on their own follow-up is theirs to make. 中文说明置信度：3/5 —— 代码本身干净且安全；我无法确定的是，这个核心修复是否真的治好了发布产物中所观测到的冻结。 回到我自己会怎么写：针对冻结问题，我会抬高构建 target 让有 bug 的降级根本不发生，再加一个编译真实依赖的回归测试，而不是断言一个配置字符串——这正是本 PR 的做法，而且它的行为级验证点比我很可能会将就的 我的保留意见是：PR 自身的验证止步于它所声称修复的对象之前。生产构建用的是嵌套的 Vite 5.4.21 及其自带的 esbuild 0.21.5；新增的验证工具用的是根 Vite 7.3.6 和根 esbuild 0.25.6。冻结是维护者在修复之前、在真实浏览器对真实 daemon 的场景下观测到的，而修复之后的生产产物无人观测过——描述对此很坦诚（"没有重跑完整 E2E"、"不宣称真实 daemon 浏览器 E2E"）。所以这条因果链是：确实看到了 bug，指出了合理的机制，应用了一个针对该机制的修复，但修复所作用的是与产生症状时不同的编译器，而提供的证据也来自那个不同的编译器。 我不把它当作阻塞项的两个理由。其一，这个改动是单调的——把下限提到 Vite 5 默认 target 中 我也不批准的两个理由。CI 变绿并不能弥合这个缺口—— 这不是数量疲劳，也不是含糊其辞——只有一个 PR，我的迟疑是具体且可指名道姓的。 什么能让我给到 4 分： 要么 ⏸️ 转交 @BenGuanRan —— 需要人来判断：上述"生产工具链未经验证"这一环是否可以就这样合入，还是应当先把验证工具重新指向正确的编译器。审查时还有两个 这个 PR 没有任何东西是坏的，把验证工具重新指向之后我就会合。之所以是提示而非阻塞，是因为作者本身就是维护者，对其自己后续 PR 的验证深度应当由他来定。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Maintainer verification —
|
| base | head | |
|---|---|---|
| DA1 control | 7 reply bytes | 7 reply bytes |
DECRQM ?2026 / 4 / ?2004 |
0 / 0 / 0; nothing after the first query renders | 11 / 7 / 11; everything renders |
| Page errors | 2 × ReferenceError: n is not defined |
0 |
| After a page reload | blank pane; new output never renders | history restored; new output renders |
Compiler matrix (real xterm 6.0.0, minified and parsed; bottom table of the §3 figure):
- esbuild 0.21.5 has the same bug as 0.25.6. Both break at
es2020and at Vite 5's defaultmodulestarget, and both are fine ates2021. - esbuild 0.28.2 is fine even at
es2020. - So on the shipped compiler,
es2021is what removes the broken lowering.
2 · New Web Shell → daemon that predates the replay protocol
Setup
- The daemon is a pre-fix(core): run web terminal PTYs on the bundled ConPTY backend #11643 build (
a651427f). It sends an unmarked binary snapshot, but it already honours{type:'release'}. - Frames are captured with Playwright, and shells are counted in
/procunder the daemon PID.
Base Web Shell
- It sends only
resize, then closes with 4002. - 1 bash is left stranded. It goes away only when the user closes the tab, which opens a
release=1socket.
Head Web Shell
- It sends
resizeand thenrelease, and 0 shells remain. - Closing the tab afterwards opens no socket, sends 0 frames and leaves 0 shells.
- After
/language ui zh-CN, the notice renders as终端协议已更新，请重启 daemon 并刷新页面。. The base bundle has noterminal.notice.protocolMismatchkey at all.
Observation, non-blocking: at head, the browser's close event is 4004 Terminal released, not 4002.
- The old daemon handles the release first and closes the socket itself, before the client's own 4002 close takes effect.
onclosereturns early onreleaseRequested, so nothing misbehaves.- Just don't expect a 4002 on the wire or in daemon-side logs for this path. The test plan's "closing with code 4002" describes what the client requests, not what arrives.
3 · Does scripts/tests/web-terminal-build.test.ts catch a revert? Not in CI (should-fix)
Why the test misses the revert
- The test imports bare
viteandesbuild, so it gets the root copies. Inpackage-lock.json(this head and currentmain) those are Vite 7.3.6 and esbuild 0.25.6. - A revert deletes the
targetline. Vite 7 then resolves the defaultbaseline-widely-availableto["chrome107","edge107","firefox104","safari16"]. - esbuild accepts that list and never lowers
||=, so the test passes with the fix reverted, even though the real build still ships the broken site. - It only fails where root
node_moduleshas drifted to Vite 5. That is how this box first "caught" it.
Correction to the triage stage-2 note: it said a removed target "would fail with a confusing esbuild error … the guard holds". It does not fail; resolveConfig expands the keyword into a browser list. The author's negative control set es2020 explicitly, which proves ES2020 breaks but is not the revert case.
Repointed test, verified in all four arms: it fails on the merge-base config under both toolchains and passes at head under both.
-import { resolveConfig } from 'vite';
import { JSDOM } from 'jsdom';
-// Use the repository's compiler; a locally overridden Vite dependency may
-// already fix the logical-assignment lowering bug and mask this regression.
-import { transform } from 'esbuild';
const require = createRequire(import.meta.url);
const webShellRoot = resolve(import.meta.dirname, '../../packages/web-shell');
+// Resolve the toolchain `packages/web-shell` builds with. Root Vite 7 resolves
+// a missing target to modern browsers and would pass with the fix reverted.
+const webShellRequire = createRequire(resolve(webShellRoot, 'package.json'));
+const { resolveConfig } = webShellRequire('vite') as typeof import('vite');
+const { transform } = webShellRequire('esbuild') as typeof import('esbuild');The existing comment about "a locally overridden Vite dependency" is also misleading: esbuild 0.21.5 is simply Vite 5.4.21's own dependency, i.e. the production compiler.
4 · The other new tests are load-bearing (mutation A/B)
web-terminal-registry:- Head passes 43/43.
- Removing
queryTerminal?.dispose()from the spawn-failurecatchfails exactlyreports a spawn failure and frees the id when both backends fail, attoHaveBeenCalledOnce. - The merge-base test file passes against that mutant (43/43).
package-assets:- Mutant M1 drops the win32-arm64 pin; mutant M2 adds a stale hard-coded linux-x64 pin.
- The new test fails on both mutants.
- The merge-base test file passes on both (36 passed, 1 skipped).
TerminalPanel: the PR's tests against the merge-base component give 21/22. The one failure is the new release assertion: the component sent onlyresize.
Gates at 538ae5cc
- PR's tests:
TerminalPanel22/22,web-terminal-registry43/43,package-assets37 passed / 1 skipped,web-terminal-build1/1. - Web Shell
tsc --noEmit: identical to a merge-base overlay of the three files (1 error on this box, the same one in both), so the PR adds no type errors. - Formatting:
prettier --checkis clean on every changed file. - ESLint: it crashes on this box with an ajv
missingRefserror, identically on an untouchedmaincheckout, so I did not run it locally. The CI Lint & Static job is green. - CI: all checks green at this head.
Test (ubuntu-latest)ran✓ scripts/tests/web-terminal-build.test.ts (1 test). - Not rerun: Windows. This PR changes no ConPTY or registry source.
Environment, arms and how to reproduce
Environment
- Debian 13 (kernel 6.12), Node 22.22.2, Chromium via Playwright 1.58.2,
@xterm/xterm6.0.0. - Web Shell build: Vite 5.4.21 + esbuild 0.21.5, the nested copies per the lockfile.
- The CI-lockfile toolchain arms use root Vite 7.3.6 + esbuild 0.25.6, installed separately and aliased into the scripts Vitest run.
Trees
- One worktree at
538ae5cc, withcore,acp-bridge,sdk-typescript,web-shell,web-templatesandclibuilt in it. - The base Web Shell arm is a second
vite buildof that tree aftergit show b5567bb7a9:of the three product files. The tree was restored afterwards and verified clean. - Each daemon serves one arm, linked in as
packages/web-shell/dist. Every bundle was checked by entry chunk and broken-site count.
Daemons
- New:
packages/cli/dist/index.js servefrom this head. - Old: the pre-fix(core): run web terminal PTYs on the bundled ConPTY backend #11643 build
a651427f, whose/terminalroute has no snapshot control frame. - Both run on loopback with an isolated
HOME, folder trust off, and a local fake OpenAI stub. Like round 2, the driver sends one prompt before opening the terminal.
Harness sources: pr11748/harness/ on my fork.
e2e11748.cjs+query-emit.cjs: the DECRQM and old-daemon scenarios.run-arms.sh+start-daemon.sh: arm orchestration.zh-probe2.cjs: the zh-CN notice.matrix.mjs: esbuild × target.target.mjs:resolveConfigunder Vite 5 and 7.web-terminal-build.ws-toolchain.test.ts: the repointed test.
中文版
维护者验证 —— 538ae5cc：真实栈上的发布产物，以及新增构建测试能否守住修复
本文是我在 #11643 第 2 轮 报告的后续。本 PR 关闭了那一轮留下的全部事项（§3.1–§3.4 与 §5）。triage 审查留下一个未决问题：es2021 能否在我们实际发布所用的编译器（嵌套的 Vite 5.4.21 → esbuild 0.21.5）上治好冻结？这次我直接实测，而不是推理。
结论：三处产品改动在 538ae5cc 上端到端成立。 有一处测试问题应当修：
- 冻结修复： 生产 Web Shell 产物在 merge-base 上遇到 DECRQM 会冻结，在本 head 上能应答并继续渲染（§1）。验证跑在真实
qwen serve、真实 bash PTY 和 Chromium 上。 - 协议拒绝： 连接 fix(core): run web terminal PTYs on the bundled ConPTY backend #11643 之前的 daemon 时，拒绝前会先释放 PTY，关闭页签不会重复释放，提示已本地化（§2）。
- registry 与 package-assets 测试： 两者都真正起作用，变异 A/B 显示它们各自能捕获 merge-base 测试集漏掉的回归（§4）。
- 应修 —— 新增的
web-terminal-build.test.ts在 CI 的 lockfile 工具链下发现不了回退。 我把修复回退后实际跑了，它依然通过（§3）。改为从packages/web-shell解析vite和esbuild即可修正，改法已验证，见下文。产品修复本身不依赖这个测试，所以先合入、再用后续 PR 修正测试也合理。
1 · 在构建发布产物的工具链上验证冻结修复
环境
- 两臂使用同一个由本 head 构建的 daemon。
- 每个 Web Shell 都是在
packages/web-shell内真实执行vite build得到的，按 lockfile 即 Vite 5.4.21 + esbuild 0.21.5。 - base 臂只有三个产品文件不同，取自
b5567bb7：vite.config.ts、TerminalPanel.tsx、i18n.tsx。 - 判定依据： 终端内的程序发送一个查询，并记录真正到达 PTY stdin 的应答字节。
构建产物
- base：
requestMode(e,t){(v=>(…))(void 0||(n={}));…，let n声明被丢掉，即 1 处错误编译。 - head：
requestMode(e,t){let n;(v=>(…))(n||={});…，0 处。 - 已发布的
@qwen-code/qwen-code@0.23.3Web Shell 产物含有同样的(void 0||(n={}))，所以当前发布版本受影响。
行为
| base | head | |
|---|---|---|
| DA1 对照 | 7 字节应答 | 7 字节应答 |
DECRQM ?2026 / 4 / ?2004 |
0 / 0 / 0；第一个查询之后不再渲染任何内容 | 11 / 7 / 11；全部正常渲染 |
| 页面错误 | 2 次 ReferenceError: n is not defined |
0 |
| 刷新页面后 | 空白面板，新输出永不渲染 | 历史恢复，新输出正常渲染 |
编译器矩阵（真实 xterm 6.0.0 压缩后解析；即 §3 图中底部表格）：
- esbuild 0.21.5 与 0.25.6 有同样的 bug。 两者在
es2020和 Vite 5 默认modulestarget 下都会出错，在es2021下都正常。 - esbuild 0.28.2 即便在
es2020下也正常。 - 所以在发布所用的编译器上，正是
es2021消除了错误降级。
2 · 新 Web Shell → 早于回放协议的 daemon
环境
- daemon 是 fix(core): run web terminal PTYs on the bundled ConPTY backend #11643 之前的构建（
a651427f）。它发送未标记的二进制快照，但本来就支持{type:'release'}。 - 帧由 Playwright 捕获，shell 数量在
/proc中按 daemon PID 统计。
base Web Shell
- 只发送
resize，随后以 4002 关闭。 - 残留 1 个 bash。 只有用户关闭页签（打开一个
release=1的 socket）后才会释放。
head Web Shell
- 先发
resize再发release，残留 0 个 shell。 - 之后关闭页签，不开新 socket、发送 0 帧、shell 仍为 0。
- 执行
/language ui zh-CN后，提示显示为终端协议已更新，请重启 daemon 并刷新页面。。base 产物里根本没有terminal.notice.protocolMismatch这个 key。
观察（非阻塞）： head 下浏览器收到的关闭事件是 4004 Terminal released，而不是 4002。
- 旧 daemon 先处理 release 并自己关闭 socket，早于客户端自己的 4002 关闭生效。
onclose因releaseRequested提前返回，行为没有问题。- 只是这条路径上线路和 daemon 日志里不会出现 4002。测试计划中"以 4002 关闭"描述的是客户端请求的关闭码，不是实际收到的。
3 · scripts/tests/web-terminal-build.test.ts 能发现回退吗？在 CI 上不能（应修）
为什么测试发现不了回退
- 测试 import 的是裸
vite和esbuild，得到的是根目录的副本。按package-lock.json（本 head 和当前main）它们是 Vite 7.3.6 和 esbuild 0.25.6。 - 回退就是删掉
target那一行。这时 Vite 7 把默认的baseline-widely-available解析为["chrome107","edge107","firefox104","safari16"]。 - esbuild 接受这个列表，并且不会降级
||=，于是修复被回退后测试依然通过，而真实构建仍在发布错误代码。 - 只有根
node_modules漂移成 Vite 5 的环境才会失败，本机最初"抓到"它就是这个原因。
更正 triage 第 2 阶段的说法： 它说去掉 target"会以难懂的 esbuild 报错失败……守护依然成立"。实际上不会失败，因为 resolveConfig 会把该关键字展开为浏览器列表。作者的负向对照是显式设置 es2020，这证明了 ES2020 会出错，但不是回退场景。
改为从 packages/web-shell 解析后的测试，四臂均已验证： merge-base 配置在两种工具链下都失败，head 在两种工具链下都通过。改法见英文部分的 diff。另外，现有注释说"本地覆盖的 Vite 依赖"也有误导：esbuild 0.21.5 就是 Vite 5.4.21 自己的依赖，也就是生产编译器。
4 · 其余新增测试确实起作用（变异 A/B）
web-terminal-registry：- head 43/43 通过。
- 删除 spawn 失败
catch中的queryTerminal?.dispose()后，恰好reports a spawn failure and frees the id when both backends fail在toHaveBeenCalledOnce处失败。 - merge-base 的测试文件对该变异体全部通过（43/43）。
package-assets：- 变异 M1 去掉 win32-arm64 的版本声明；变异 M2 加入一个过期的硬编码 linux-x64 版本声明。
- 新测试对两者都失败。
- merge-base 测试文件对两者都通过（36 通过、1 跳过）。
TerminalPanel： 用 PR 的测试跑 merge-base 组件，结果 21/22。唯一的失败是新的 release 断言：组件只发了resize。
538ae5cc 上的检查
- PR 的测试：
TerminalPanel22/22，web-terminal-registry43/43，package-assets37 通过 / 1 跳过，web-terminal-build1/1。 - Web Shell
tsc --noEmit： 与三个文件回到 merge-base 的对照完全一致（本机 1 个错误，两边是同一个），所以 PR 没有新增类型错误。 - 格式： 所有改动文件
prettier --check通过。 - ESLint： 本机因 ajv
missingRefs报错崩溃，在未改动的main上也完全一样，所以没有在本地跑。CI 的 Lint & Static 任务为绿色。 - CI： 本 head 全部检查为绿色。
Test (ubuntu-latest)实际跑了✓ scripts/tests/web-terminal-build.test.ts (1 test)。 - 未重跑： Windows。本 PR 没有改动 ConPTY 或 registry 源码。
复现环境与脚本见上方英文的折叠部分。脚本在我 fork 的 pr11748/harness/。
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 15 passed · 0 failed · 15 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：15 通过 · 0 失败 · 15 总计 抖动门：✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #11748 deep verification —
|
| # | artifact | built by | build.target |
requestMode enum-init |
verdict |
|---|---|---|---|---|---|
| 1 | dist-base/assets/index-C8Yx7BVy.js (4,715,209 B) |
vite 5.4.21 + esbuild 0.21.5 | none → ['es2020','edge88','firefox78','chrome87','safari14'] |
(void 0||(n={})) — declaration dropped |
DEFECT |
| 2 | dist-head/assets/index-CRYSdxJW.js (4,623,893 B) |
same | 'es2021' |
let n;(…)(n||={}) |
FIXED |
| 3 | packages/web-shell/dist/index.js (7,272,167 B, CI-built at HEAD) |
vite.lib.config.ts, vite 5.4.21 + esbuild 0.21.5 |
none | (void 0 || (n = {})) |
DEFECT |
| 4 | lib-head-asis/index.js (fresh rebuild at HEAD) |
same | none | (void 0 || (n = {})) |
DEFECT |
| 5 | lib-head-fixed/index.js (candidate fix) |
same | 'es2021' added |
let n; ((w) => …)(n ||= {}) |
FIXED |
Artifact 3 contains the PR's own new i18n key (terminal.notice.protocolMismatch ×3), which proves it was built at HEAD, not stale.
A/B result: 2/2 standalone-app cells flip DEFECT → FIXED; 0/2 library cells flip. 5/5 scripted assertions passed.
Table B — behavioural A/B on the production compiler
Harness matrix.mjs, log logs/01-matrix.log, witness 01-esbuild-compiler-target-matrix.png. Oracle: load the transformed real xterm.mjs in a JSDOM vm context, synchronously parse('\x1b[?2004$pafter-query'), assert the reply is exactly \x1b[?2004;2$y and line 0 reads after-query.
| compiler (realpath-asserted) | target | result |
|---|---|---|
prod packages/web-shell/node_modules/esbuild 0.21.5 — the one vite 5.4.21 uses to build the Web Shell |
['es2020','edge88','firefox78','chrome87','safari14'] (= vite 5 default) |
THROW ReferenceError: i is not defined |
| prod 0.21.5 | es2020 |
THROW ReferenceError: i is not defined |
| prod 0.21.5 | es2021 (= HEAD) |
OK replies=["ESC[?2004;2$y"] output="after-query" |
| prod 0.21.5 | esnext |
OK |
root node_modules/esbuild 0.25.6 — the one the PR's new test imports |
vite5-default / es2020 |
THROW ×2 |
| root 0.25.6 | es2021 / esnext |
OK ×2 |
vite7 node_modules/vite/node_modules/esbuild 0.28.2 |
es2020 / es2021 / esnext |
OK ×3 (bug already fixed upstream) |
| vite7 0.28.2 | vite5-default list | TRANSFORM-ERROR (cannot lower destructuring to that target set) |
7/12 cells OK. The central claim holds on the production compiler: base target → freeze, es2021 → correct answer plus continued output. 5/5 scripted assertions passed.
Strict-mode characterization (logs/10-strict-mode-semantics.log, 1/1 passed): evaluating the exact base-emitted expression void 0||(n={}) under "use strict" yields ReferenceError: n is not defined; in sloppy mode it silently creates a global instead. Both shipped artifacts are strict, so both fail loud at the query rather than leaking a global: the standalone bundle is loaded via <script type="module" crossorigin src="/assets/index-CRYSdxJW.js"> in the built index.html, and the library bundle is an ES module by construction (exports["."].import, formats: ['es']).
Corrections to the PR description
These are corrections of stated fact, not requests to change code.
- "esbuild 0.25.6 can miscompile xterm's logical assignments" — 0.25.6 does miscompile (measured, Table B), but it is not the compiler that builds the Web Shell.
packages/web-shelldeclaresvite: ^5.0.0and resolves vite 5.4.21, which resolves its own nested esbuild 0.21.5 atpackages/web-shell/node_modules/esbuild. Rootesbuild@^0.25.0→ 0.25.6 is a separate, unrelated dependency (used byesbuild.config.jsfor the CLI bundle attarget: 'node22'). The version that matters here is 0.21.5, and it reproduces the identical defect. - "The production target is now ES2021" — true for the standalone app only. The published library entry keeps vite 5's default target (Table A, rows 3–4).
- "The real xterm module minified with locked esbuild 0.25.6 passes at ES2021; the same test at ES2020 fails" — reproduced exactly (
ReferenceError: i is not definedinrequestMode). But this is a statement about 0.25.6, and the test's config read goes through root vite 7.3.6, not the vite 5.4.21 that builds the artifact. See Finding 2.
Findings
1. Major — the published @qwen-code/web-shell library still ships the same miscompile (pre-existing, not a regression)
packages/web-shell/vite.lib.config.ts sets no build.target (grep -n target packages/web-shell/vite.lib.config.ts → no match), so it builds dist/index.js with vite 5's default target list and esbuild 0.21.5 — the exact combination that drops the declaration. The published entry point is "." → ./dist/index.js, consumed by packages/web-templates/package.json and packages/vscode-ide-companion/package.json, and it does contain xterm (requestMode ×3, _inputHandler ×21, xterm-accessibility ×6, allowProposedApi ×4) and the PR's own TerminalPanel (terminal.notice.protocolMismatch ×3).
At HEAD, that bundle reads:
requestMode(e, t) {
((_) => (_[_.NOT_RECOGNIZED = 0] = "NOT_RECOGNIZED", …))(void 0 || (n = {}));Reproduce (≈15 s):
cd packages/web-shell && npx vite build --config vite.lib.config.ts --outDir /tmp/lib-asis
node -e "const s=require('fs').readFileSync('/tmp/lib-asis/index.js','utf8');const i=s.indexOf('requestMode(e, t) {');console.log(s.slice(i,i+330))"Blast radius and bounds. I proved the emitted defect is byte-identical in shape to the standalone base bundle, and that adding target: 'es2021' to vite.lib.config.ts clears it (Table A row 5 — a measured candidate fix, not an eyeballed one). What I did not settle behaviourally is the runtime consequence inside the 7 MB library bundle: it is an ES module, hence strict, so an undeclared write should throw ReferenceError — but the bundle does contain one module-scope function n, so I cannot rule out the quieter variant where (n = {}) clobbers that binding and the failure surfaces later as a TypeError in unrelated code. Per the observability ranking, the quiet variant would be the worse one. Either way the terminal is broken for library consumers on a DECRQM query.
Not a merge blocker for this PR: vite.lib.config.ts is untouched by the diff, so rows 3–4 are equally broken at base. This is an unfixed sibling of the same bug class, and the PR's title ("prevent terminal query freezes") and its new test's name ("Web Shell production terminal") both read as broader than what is fixed. The description's Risk section does say "The standalone Web Shell now targets ES2021", so the narrower scope was declared.
Measured candidate fix
--- a/packages/web-shell/vite.lib.config.ts
+++ b/packages/web-shell/vite.lib.config.ts
@@
build: {
+ target: 'es2021',
emptyOutDir: false,Verified: with this applied, lib-head-fixed/index.js contains let n; ((w) => …)(n ||= {}) and the DEFECT pattern is absent; index.js goes 7,272.17 kB → 7,167.20 kB and toolNames 364.18 kB → 346.18 kB (fewer lowering helpers emitted). Both library builds completed clean (6.9 s / 6.4 s). I did not run the library's consumers against the patched bundle.
2. Major — the new regression test cannot fail when the fix is removed
scripts/tests/web-terminal-build.test.ts is the only guard for the central claim. Removing target: 'es2021' from vite.config.ts (i.e. restoring base) leaves it green:
✓ scripts/tests/web-terminal-build.test.ts (1 test) 261ms
Test Files 1 passed (1)
Tests 1 passed (1)
Reproduce — witness 03-vacuity-test-green-with-fix-removed.png, log logs/05-vacuity-base-test-run.log:
# delete the `target: 'es2021',` line from packages/web-shell/vite.config.ts, then:
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/web-terminal-build.test.ts
git checkout -- packages/web-shell/vite.config.tsRoot cause — the test reads and compiles through a different toolchain than production. Measured with the fix removed (logs/04-vacuity-target-resolution.log):
resolveConfig(...).build.target |
|
|---|---|
vite 7.3.6 at node_modules/vite — what the test imports |
['chrome107','edge107','firefox104','safari16'] → no logical-assignment lowering → no bug |
vite 5.4.21 at packages/web-shell/node_modules/vite — what builds the Web Shell |
['es2020','edge88','firefox78','chrome87','safari14'] → lowers → bug |
So the test's control arm structurally cannot reproduce production's base behaviour. Independently, its transform import resolves to root esbuild 0.25.6, while production uses 0.21.5. Today both miscompile, so the test is not wrong — but it is coincidentally correct: esbuild 0.28.2 already has the fix (Table B), so any future bump of the root esbuild@^0.25.0 range past 0.27 would leave this test permanently green while the production compiler stays at 0.21.5.
Positive control (the harness is live). The assertion can fail, and fails with the intended behavioural mismatch — setting target: 'es2020' explicitly turns it red (logs/06-positive-control-es2020.log):
× Web Shell production terminal > answers DECRQM and keeps processing output after minification
→ i is not defined
ReferenceError: i is not defined
❯ ja.requestMode evalmachine.<anonymous>:44:103624
❯ queryAndPrint scripts/tests/web-terminal-build.test.ts:71:32
So the gap is specifically the removal path — which is the realistic regression, since a future cleanup would read target: 'es2021' as redundant against vite's default.
Suggested direction
Resolve the compiler and the config through the web-shell workspace rather than the repo root — e.g. createRequire(resolve(webShellRoot, 'x.js')).resolve('esbuild') and the same for vite — so the test exercises vite 5.4.21 / esbuild 0.21.5. Then assert the base condition directly as a second case: transform at vite 5's default target and assert it throws, which pins the mechanism rather than only the configured value. I did not implement or measure this variant.
3. Minor — coverage gap: releaseRequested = true is load-bearing but unpinned
Mutation matrix for the guards this PR adds/relies on. logs/20-vacuity-mutations.log, logs/21-fine-mutations.log, logs/22-m5-adjudication.log.
| # | mutation | suite | expected | observed | classification |
|---|---|---|---|---|---|
| M1 | revert the whole TerminalPanel.tsx mismatch hunk (no release, hardcoded English notice) |
TerminalPanel 22 | red | red 1/22 — expected "spy" to be called with [ '\u0000{"type":"release"}' ] |
pinned |
| M4 | keep the release send, move it after ws.close(4002) |
TerminalPanel 22 | red | red 1/22 — expected 204 to be less than 203 (ordering assertion) |
pinned |
| M5 | keep the release send, drop releaseRequested = true |
TerminalPanel 22 | red | green 22/22 | coverage gap |
| M2 | drop queryTerminal?.dispose() from the spawn-failure catch |
web-terminal-registry 43 | red | red 1/43 — expected "dispose" to be called once, but got 0 times |
pinned |
| M3 | stop spreading ...nodePtyPins into optionalDependencies |
package-assets (filtered) | red | red — expected 6 pins, received {} |
pinned |
| M6 | remove target: 'es2021' from vite.config.ts |
web-terminal-build 1 | red | green 1/1 | Finding 2 |
M5 is a genuine gap, not dead code and not redundant defence — adjudicated with a positive control:
| arm | probe | result |
|---|---|---|
| A | HEAD + expect(FakeWebSocket.instances.length).toBe(socketsBefore) |
22/22 green |
| B | M5 + same probe | red — expected 2 to be 1 |
| C | M5, PR's own test unmodified | 22/22 green |
Mechanism: FakeWebSocket.close() sets readyState = CLOSED, so with the flag unset release() skips the OPEN branch and falls into else if (!ws || ws.readyState !== WebSocket.CONNECTING) → connect(true), opening a brand-new release-only WebSocket (with up to 3 retries via releaseAttempts). The PR's assertion counts ws.send on the original socket, so it cannot observe the second one. The guard therefore prevents a spurious reconnect against an already-rejected old daemon — real behaviour, unasserted.
The fixture that would pin it, appended to the existing case:
const socketsBefore = FakeWebSocket.instances.length;
act(() => releaseWebTerminal('terminal:one'));
expect(FakeWebSocket.instances.length).toBe(socketsBefore);4. Minor — target: 'es2021' narrows browser support to exclude Firefox 78
'es2021' replaces vite 5's whole default list, dropping the browser-specific entries. Measured over every .js in each standalone bundle (logs/16-strictness-and-browser-floor.log):
| total JS bytes | un-lowered ||= / ??= / &&= |
private-field-ish | static blocks | |
|---|---|---|---|---|
dist-base |
18,086,783 | 8 | 65 | 0 |
dist-head |
17,970,719 | 62 | 65 | 0 |
| Δ | −116,064 | +54 | 0 | 0 |
The entire syntax delta is logical assignment (ES2021); ES2022 constructs are unchanged. Logical assignment shipped in Chrome/Edge 85, Safari 14.0, Firefox 79 — so of vite's default list (es2020, edge88, firefox78, chrome87, safari14) only Firefox 78 regresses, from working to a parse-time SyntaxError (blank page, not a broken terminal). Every residual is accounted for: the +54 sites are exactly the lowering the fix exists to suppress, and the −116 kB is the lowering helpers no longer emitted.
The trade is clearly worth it — a hard freeze on every browser beats a parse failure on a 2020 ESR release. If keeping the floor matters, the equivalent fix that preserves it is a list rather than a bare spec:
target: ['es2021', 'edge88', 'firefox79', 'chrome87', 'safari14'],I did not build or test that variant; it is a suggestion, not a measured fix.
Also verified (no finding)
- No control-byte leak into the shell. The client now sends
\x00{"type":"release"}to a daemon that by definition does not mark its snapshot.packages/cli/src/serve/routes/terminal.ts:276drops any NUL-prefixed frame that fails to parse (if (!isBinary && text.startsWith(CONTROL_FRAME_PREFIX) && !control) return;) instead of falling through toregistry.write(), so an old daemon cannot receive the frame as typed input. Current daemons handlecontrol?.type === 'release'explicitly. - The headless sibling is safe.
@xterm/headlessreaches production only through the CLI bundle (esbuild.config.js,target: 'node22'×3 sites). Node 22 supports logical assignment natively, so no lowering occurs and the bug class cannot reach it. @xterm/xterm@6.0.0really does contain the construct — 17||=, 2??=, 2&&=— so this is not a synthetic repro.- The 6-pin assertion matches reality.
packages/core/package.jsondeclares exactly 6@lydell/node-pty*optional dependencies;package-assets.test.jsfull file 38/38 pass at HEAD (logs/24-package-assets-full.log), matching the PR's claim. - Suite counts match the PR's claims: TerminalPanel 22/22 (
logs/11-terminalpanel-head.log), web-terminal-registry 43/43 (logs/12-registry-head.log), package-assets 38/38. - Localization is complete on both axes.
terminal.notice.protocolMismatchis present inEN(i18n.tsx:1472) andZH(i18n.tsx:5169), and consumed atTerminalPanel.tsx:310;tis in scope viauseI18n()(line 117). - Reviewer Test Plan, step by step. Step 1 (DECRQM answers and output continues) — verified at the parser level on real artifacts (Tables A/B); not verified in a real browser. Step 2 (release before 4002, localized notice, no re-release on tab close) — the ordering half is pinned (M4) and the notice is localized; the "closing the tab afterward should not send release again" half is asserted in a way that passes with the guard removed (Finding 3), so that step is not actually verified by the PR's own test. Step 3 (responder disposed, id reusable; six pins preserved) — both pinned (M2, M3).
Not covered
- No real browser or real daemon E2E. No tmux/Playwright run; the PR cites the maintainer's round-2 baseline for that and does not claim it. My DECRQM evidence is at the parser/artifact level. This reproduces the mechanism and the emitted defect, not a user watching a browser freeze.
- Windows ConPTY native paths not exercised — Linux container, no
conpty.dll/OpenConsole.exe. The double-spawn-failure path was verified only through the registry's mockedspawn. - The library bundle's runtime failure mode is not settled (Finding 1 bounds):
ReferenceErrorvs silent clobber of the module-scopefunction n. Settling it needs the 7 MB ESM bundle evaluated with its externals stubbed. - No repo-wide gates run. Not executed:
npm run build,npm run typecheck,npm run lint,prettier --check, the fullscripts/testssuite (onlypackage-assets.test.jsandweb-terminal-build.test.ts), and every other workspace suite. The PR itself states web-shell typecheck did not pass in the author's worktree; I did not attempt it. vite build --config vite.lib.config.ts --mode transcriptvariant not built — only the default (index+daemon-react-sdk) entry.- No trial merge into current
main. The checkout is depth 2 (git rev-parse --is-shallow-repository→ true), somainis not reachable and merge-conflict status against it is unknown. - The docs change is not behaviourally tested. The new claim that "queries emitted while no browser is attached remain unanswered on reconnect" is a prose boundary statement; I did not drive a reconnect to confirm replay suppresses replies, and I did not diff the EN/ZH design docs beyond reading the changed hunks (they are consistent with each other).
- esbuild versions outside this lockfile. I tested 0.21.5 / 0.25.6 / 0.28.2 only; I did not bisect which release fixed the lowering bug.
Methodology
Environment: the CI verify container (node:22-bookworm), Node v22.23.2, npm 10.9.8, tree at the merge ref c67f83165d with npm ci + npm run build already completed; no zstd binary present; $RUNNER_TEMP unset in my shell. I asserted every toolchain identity by realpath rather than assuming it (createRequire(...).resolve + realpathSync, printed at the top of logs/01-matrix.log), because the monorepo carries three different esbuilds and two different vites and a naive harness silently measures the wrong one. Four harnesses drove real code with no stubs: matrix.mjs transformed the genuine @xterm/xterm@6.0.0 lib/xterm.mjs with each compiler and loaded the output in a JSDOM vm context to parse a real DECRQM sequence; probe-dist.mjs and lib-sibling.mjs classified requestMode in five artifacts produced by real vite build invocations (two standalone arms from mutating only packages/web-shell/vite.config.ts, three library arms from vite.lib.config.ts as-is and with the candidate fix); the mutation runs drove the real vitest suites against single-point edits of production source, each reverted with git checkout -- and confirmed by git status --porcelain | wc -l = 0. All scratch output went to tmp/pr11748-verify-20260913-031940/; the four vite build output trees (dist-base, dist-head, lib-head-asis, lib-head-fixed, ≈50 MB of minified JS) were deleted after evidence extraction to keep the published artifact small — their decisive excerpts are quoted verbatim in Tables A/B and in logs/09, logs/15, logs/19, and each is regenerable with the commands named in Finding 1 and in Table A's built by column. No git worktree was needed, so none was left behind. Raw per-run logs are logs/01–logs/24; harness sources are matrix.mjs, probe-dist.mjs, lib-sibling.mjs, vacuity.sh, fine-mutations.sh, m5-adjudicate.sh, all rerunnable as-is. Images were produced with node scripts/verify-capture.mjs; captures 01 and 02 pipe the saved logs from the real executions, capture 03 is a live run.
Flakiness gate log
rounds=5 files=4 skipped=0
file packages/core/src/services/web-terminal-registry.test.ts: (cd packages/core) npx --no-install vitest run ./src/services/web-terminal-registry.test.ts
file packages/web-shell/client/components/terminal/TerminalPanel.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/terminal/TerminalPanel.test.tsx
file scripts/tests/package-assets.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/package-assets.test.js
file scripts/tests/web-terminal-build.test.ts: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/web-terminal-build.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/core/src/services/web-terminal-registry.test.ts: PPPPP
packages/web-shell/client/components/terminal/TerminalPanel.test.tsx: PPPPP
scripts/tests/package-assets.test.js: PPPPP
scripts/tests/web-terminal-build.test.ts: PPPPP
verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/core/src/services/web-terminal-registry.test.ts: P (exit 0)
round 1 · packages/web-shell/client/components/terminal/TerminalPanel.test.tsx: P (exit 0)
round 1 · scripts/tests/package-assets.test.js: P (exit 0)
round 1 · scripts/tests/web-terminal-build.test.ts: P (exit 0)
round 2 · packages/core/src/services/web-terminal-registry.test.ts: P (exit 0)
round 2 · packages/web-shell/client/components/terminal/TerminalPanel.test.tsx: P (exit 0)
round 2 · scripts/tests/package-assets.test.js: P (exit 0)
round 2 · scripts/tests/web-terminal-build.test.ts: P (exit 0)
round 3 · packages/core/src/services/web-terminal-registry.test.ts: P (exit 0)
round 3 · packages/web-shell/client/components/terminal/TerminalPanel.test.tsx: P (exit 0)
round 3 · scripts/tests/package-assets.test.js: P (exit 0)
round 3 · scripts/tests/web-terminal-build.test.ts: P (exit 0)
round 4 · packages/core/src/services/web-terminal-registry.test.ts: P (exit 0)
round 4 · packages/web-shell/client/components/terminal/TerminalPanel.test.tsx: P (exit 0)
round 4 · scripts/tests/package-assets.test.js: P (exit 0)
round 4 · scripts/tests/web-terminal-build.test.ts: P (exit 0)
round 5 · packages/core/src/services/web-terminal-registry.test.ts: P (exit 0)
round 5 · packages/web-shell/client/components/terminal/TerminalPanel.test.tsx: P (exit 0)
round 5 · scripts/tests/package-assets.test.js: P (exit 0)
round 5 · scripts/tests/web-terminal-build.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — the "Test (macos-latest, Node 22.x)" and "Test (windows-latest, Node 22.x)" CI legs were skipped at this commit, so the two new scripts tests were exercised only on Linux (locally by the build/test agent and on the ubuntu leg); their cross-platform behaviour is unmeasured.
Not explored to full depth (tool budget reached): "agent 6b": did not re-run vite build for packages/web-shell to confirm from a fresh build (rather than the tree's existing dist/ ) that the app chunk and lib bundle d…; "agent 3c": running vite build --config vite.lib.config.ts and executing the emitted bundle to confirm the lib-build breakage end to end (finding 1 rests on the resolved …; "agent 6c": I did not enumerate the .github/workflows jobs that invoke test:scripts / test:ci (which OSes, and whether any are merge_group - or schedule -gated), so…; "agent 6c": I did not check git log on packages/cli/src/serve/routes/terminal.ts to establish whether a pre-marking daemon parses the release control frame or would t…; "agent 3a": did not run packages/web-shell 's vite.lib.config.ts builds to confirm whether @xterm/xterm survives tree-shaking into dist/index.js (left as the stated …, and 1 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查（原文为英文）：build-and-test — the "Test (macos-latest, Node 22.x)" and "Test (windows-latest, Node 22.x)" CI legs were skipped at this commit, so the two new scripts tests were exercised only on Linux (locally by the build/test agent and on the ubuntu leg); their cross-platform behaviour is unmeasured.
未探索到全部深度（达到工具调用预算）："agent 6b"：did not re-run vite build for packages/web-shell to confirm from a fresh build (rather than the tree's existing dist/ ) that the app chunk and lib bundle d…；"agent 3c"：running vite build --config vite.lib.config.ts and executing the emitted bundle to confirm the lib-build breakage end to end (finding 1 rests on the resolved …；"agent 6c"：I did not enumerate the .github/workflows jobs that invoke test:scripts / test:ci (which OSes, and whether any are merge_group - or schedule -gated), so…；"agent 6c"：I did not check git log on packages/cli/src/serve/routes/terminal.ts to establish whether a pre-marking daemon parses the release control frame or would t…；"agent 3a"：did not run packages/web-shell 's vite.lib.config.ts builds to confirm whether @xterm/xterm survives tree-shaking into dist/index.js (left as the stated …，另有 1 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.23.3)
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
APPROVE
核对基线：head 538ae5cc1690b3640b18371c970fe9b78708308f。
历史阻塞问题：无
本 PR 只有一条 review（qwen-code-ci-bot 于 2026-09-13T03:46:40Z，针对当前 head，状态 COMMENTED），从未出现 CHANGES_REQUESTED。5 条线程全部未解决，但逐条读过，R1-1 至 R1-5 全是 [Suggestion] 级，Critical 级未解决数为 0。按本渠道策略 Suggestion 不作为合入门禁。
本轮独立扫描：未发现 Critical
生产改动只有三处共 +9/-3，我全部读完并核对了调用链：
TerminalPanel.tsx:305-310的协议不匹配分支是实质修复，不只是文案本地化。 改动前该分支只term.writeln(硬编码英文)然后ws.close(4002, ...)；改动后变成releaseRequested = true→ 发送CONTROL_FRAME_PREFIX + {type:'release'}→ 写本地化提示 → 关闭。两点都对：releaseRequested确实是承重状态，不是死代码。它在:200声明，被:329（ws.onopen：若已请求释放则立即发 release 并关闭，避免一个迟到的 socket 反而开起会话）、:370（onclose重连路径：if (disposed || releaseRequested) return;）、:388（release()的幂等门）、:431（卸载清理）四处读取。其中:370是关键：不设这个标志，协议不匹配关闭后客户端仍可能走重连，而这种不匹配永远不可能靠重连恢复。- 补发
release控制帧使该分支与:387-395的release()契约一致（标记已释放 → 通知服务端释放 PTY → 关闭）。改动前直接 close 而不发 release，服务端那一侧的 PTY 可能被继续持有。
i18n.tsx的两个字典同步添加。terminal.notice.protocolMismatch在 EN（:1472-1473）与 ZH（:5169-5170）两侧都加了，文案与原硬编码字符串语义一致，Messages类型由 TS 约束，缺一侧会编译不过；当前 head 的Lint & Static（含类型检查）为 pass。vite.config.ts:91-92的target: 'es2021'不会造成兼容性回归。 Vite 默认的build.target: 'modules'本身就以支持原生 ESM 的浏览器为底（含 safari14），而 ES2021 的逻辑赋值运算符正是 Safari 14 / Chrome 85 / Firefox 79 起支持，因此浏览器下限实际没有抬高；换来的是不再让 esbuild 去降级 xterm 里的逻辑赋值。- 其余 6 个文件是设计文档（中英双份同步）、
web-terminal-registry.test.ts、TerminalPanel.test.tsx、scripts/tests/package-assets.test.js、scripts/tests/web-terminal-build.test.ts，均为文档或测试，不含生产逻辑。
CI：当前 head 上 Lint & Static、Test (ubuntu-latest, Node 22.x)、Integration Tests (no-AK, No Sandbox)、web-shell E2E Smoke、review-pr 全部 pass，无 fail、无 pending，没有由本 PR 引入的失败。
两条值得后续处理的 Suggestion（不阻塞本次合入）
我复核后认为这两条观察成立，建议跟进，但都不是本 diff 的缺陷：
- R1-2 的范围问题确实存在。
packages/web-shell/package.json的build是vite build && vite build --config vite.lib.config.ts && vite build --config vite.lib.config.ts --mode transcript && tsc -p tsconfig.lib.json，本次的target只加在vite.config.ts，因此只覆盖第一个构建。而vite.lib.config.ts的build块没有target，其rollupOptions.external列表里也没有 xterm；同时 lib 入口client/index.tsx会引入App，App又会渲染TerminalPanel，所以 xterm 是被打进 lib 产物的，两个 lib 构建仍按默认 target 让 esbuild 降级逻辑赋值。也就是说宿主以库形式嵌入@qwen-code/web-shell时，可能仍带着这次要修的那个产物。这不是本 PR 引入的回归（lib 构建的行为在改动前后一致），本 PR 也没有让任何一侧变差，因此我按 Suggestion 处理；但既然根因是「esbuild 降级 xterm 的逻辑赋值会产出无效代码」，建议把同样的 target 加到vite.lib.config.ts，或抽成共享的 build 片段，让三个构建口径一致。 - R1-1 的守卫测试可能无法失败。 该条指出
scripts/tests/web-terminal-build.test.ts里import { resolveConfig } from 'vite'从scripts/tests/解析到根 hoist 的 vite，因此这条回归守卫对它要抓的回归不敏感。测试有效性问题按策略不构成 Critical，但如果守卫抓不到回归，上面那条 target 覆盖范围的问题将来更容易悄悄退回去，建议一并修。
R1-3（断言观察不到 releaseRequested）、R1-4（本地化文案缺测试见证）、R1-5（硬编码了另一个套件已拥有的计数）同样按 Suggestion 处理，不影响本次结论。
结论：无历史阻塞问题，本轮未发现可证明的 Critical，提交 APPROVE。
|
Post-merge review — findings are for follow-up tracking. ScopeSource files and all changed tests reviewed. No working tree available for execution rungs; PR provides its own evidence (ES2020 throws ReferenceError, unit suites pass). Cross-check against existing review (qwen-code-ci-bot R1)All five findings from the prior review confirmed independently. None refuted. No additional findings. R1-1 — ineffective regression guard ( R1-2 — ES2021 fix misses lib build ( R1-3 — releaseRequested mutation not caught ( R1-4 — protocolMismatch key unasserted ( R1-5 — hardcoded pin count ( Production code — no blocking defectThe three substantive production changes are correct:
Reviewed with AI assistance. |
|
Closeout for head 314b1ac (round-1 findings R1-1..R1-5). Changed
Verified
Pending: CI + automated re-review on the new head. |
|
Released in v0.23.4. |
6 participants
What this PR does
Fixes the remaining Web Shell terminal follow-ups from #11643: preserves mode-query handling in minified output, releases the PTY when rejecting an older daemon, and localizes the restart notice. Adds regression coverage for failed-spawn responder cleanup and the six published node-pty pins, and documents that reconnect does not answer queries emitted while the browser was offline.
Why it's needed
The maintainer's round-2 verification found that esbuild 0.25.6 can miscompile xterm's logical assignments when lowering to ES2020, freezing the terminal after a DECRQM query. The production target is now ES2021. Separately, protocol rejection used to leave the older daemon's shell alive until idle reclamation because the client closed without requesting release.
Reviewer Test Plan
How to verify
Evidence (Before & After)
Baseline real-daemon/browser evidence: maintainer round 2. This follow-up verifies parser behavior and control ordering with focused tests; it does not repeat that full E2E run.
Local: TerminalPanel 22/22, web-terminal-registry 43/43, package-assets 38/38; changed-file ESLint passed. The real xterm module minified with locked esbuild 0.25.6 passes at ES2021; the same test at ES2020 fails with
ReferenceError: i is not definedinrequestMode.Tested on
Environment (optional)
Node 22.22.0. Existing dependencies were copied into an isolated worktree. Registry tests use the changed source with existing core build artifacts for package-entry prerequisites.
Risk & Scope
Linked Issues
Follow-up to #11643. Refs #11734; its replay-suppression implementation already merged in #11643, and this PR documents the remaining offline-query boundary.
中文说明
改动内容
处理 #11643 剩余的 Web Shell 终端后续问题：避免压缩产物中的模式查询冻结，在拒绝旧 daemon 时释放 PTY，并本地化重启提示。补上 spawn 失败释放应答器、发布包六个 node-pty 版本声明的回归测试，并说明浏览器离线期间的查询不会在重连时补答。
修改原因
维护者第 2 轮验证发现 esbuild 0.25.6 向 ES2020 降级时可能错误编译 xterm 的逻辑赋值，导致 DECRQM 查询后终端冻结。生产构建目标现改为 ES2021。另外，旧的协议拒绝路径只断开连接而不请求释放，旧 daemon 的 shell 会残留至空闲回收。
验证计划
发送 DECRQM 查询及后续文字，应得到应答且继续显示输出。新客户端连接旧 daemon 时，应在以 4002 关闭之前发送 release 并显示本地化重启提示；随后关闭页签不应重复释放。Windows 两个后端都 spawn 失败时，应释放应答器并允许重用终端 ID；发布包应完整继承 core manifest 的六个 PTY 版本声明。
前后证据
基线真实 daemon/浏览器证据见上方维护者第 2 轮报告。本轮通过定向测试验证解析行为和控制帧顺序，没有重跑完整 E2E。TerminalPanel 22/22、registry 43/43、package-assets 38/38 通过；改动文件 ESLint 通过。真实 xterm 模块经锁定的 esbuild 0.25.6 压缩后，在 ES2021 下通过；同一测试改为 ES2020 时在
requestMode报ReferenceError: i is not defined。测试平台与环境
macOS 定向测试和 lint 通过；Windows 未重跑，Linux 未在本地测试。Node 22.22.0，在独立工作树中使用现有依赖副本。registry 测试加载改动源码，包入口前置条件使用已有 core 构建产物。
风险与范围
独立 Web Shell 的构建目标升为 ES2021；原生 ConPTY 行为和应答归属不变。工作树缺少 SDK daemon 构建声明，Web Shell 类型检查未通过；不宣称全仓 build/typecheck 或真实 daemon 浏览器 E2E 通过，由 CI 承担完整构建验证。本地 review-gate 和 pr-ui-verify skills 不可用，本轮证据是定向代码检查及解析器/组件测试。不新增协议迁移，仅在现有不匹配路径显式释放 PTY。英文和中文设计文档同步更新，链接见上方。
关联
#11643 的后续修复；关联 #11734，其回放抑制已由 #11643 合入，本 PR 补充离线查询边界说明。