feat(web-shell): add opt-in interactive browser terminal - #9984
Conversation
Adds a browser terminal to the web-shell right panel, backed by shared node-pty sessions exposed through an authenticated /terminal WebSocket route on the serve daemon. The feature is opt-in (QWEN_WEB_TERMINAL=1 plus bearer auth) and gated behind a web_terminal capability so default deployments never expose the entry point. Terminals are workspace-scoped: the daemon resolves the client workspace selector against the live registry, binds each PTY to that runtime's cwd and effective environment, and releases sessions when the workspace drains. Reconnects claim the same session id and replay bounded scrollback (including exit state) without duplicating output; controls resize/release ride text frames while PTY data stays binary. A session with no listeners is reclaimed after 15 minutes. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
# Conflicts: # packages/web-shell/client/App.tsx
|
Re-run over the hardening rounds since the last pass ( Problem: feature PR, so no bug reproduction applies. The gap it fills — the Web Shell had no interactive shell tied to the active workspace — is real and long-requested. Still no linked tracking issue; informational only. Direction: unchanged and still aligned. The shipped gating contract (host right-panel action + Size: now roughly 1.39k production-logic lines vs ~1.97k test lines (plus 21 doc lines, 52 NOTICES lines, lockfile churn). The core-path share is the new registry at 438 production lines. As a Approach: every file still serves the feature, and the four new commits are exactly the kind this gate wants to see: focused fixes with named regression tests — release semantics for exited sessions, keyboard-event isolation from app shortcuts, PTY process-tree kill, input-budget accounting. No unrelated changes, no simpler path for the feature itself. Risk: no elevated revert-risk paths matched. Branch is MERGEABLE. Moving on to code review of the hardening rounds. 🔍 中文说明针对上次审查之后的加固轮（ 问题：功能 PR，不需要 bug 复现。它填补的空白——Web Shell 此前没有绑定当前工作区的交互式终端——真实且长期有需求。仍无关联跟踪 issue，仅供参考。 方向：不变且仍然对齐。实际交付的门控契约（宿主右侧面板 action + 规模：现约 1.39k 生产逻辑行对 约 1.97k 测试行（另有 21 行文档、52 行 NOTICES、lockfile 变更）。核心路径份额为新注册表的 438 行生产代码。作为 方案：所有文件仍服务于该功能，四个新提交正是门禁希望看到的形态：带点名回归测试的聚焦修复——已退出会话的释放语义、与应用快捷键隔离的键盘事件处理、PTY 进程树终止、输入预算记账。无无关改动，功能本身没有更简路径。 风险：未命中高回滚风险路径。分支可合并。 进入加固轮的代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code review — hardening rounds at
|
| File | What changed |
|---|---|
docs/design/web-terminal-lifecycle.md |
Design doc on gating, ownership, reconnect, reload and idle reclaim |
docs/developers/qwen-serve-protocol.md |
Lists the web_terminal capability |
package-lock.json |
xterm dependency entries |
packages/cli/src/serve/acp-http/index.ts |
bypassPrimaryDrain on ExtraWsRoute so self-managed routes skip the primary drain gate |
packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts |
Drain-gate exemption and primary-scoped fallback tests |
packages/cli/src/serve/capabilities.ts |
Registers web_terminal, conditional on the ACP HTTP surface |
packages/cli/src/serve/routes/terminal.test.ts |
25 route-level WebSocket lifecycle test blocks incl. all hardening regressions |
packages/cli/src/serve/routes/terminal.ts |
The /terminal WS route, control framing, release handshake, backpressure guards, heartbeat |
packages/cli/src/serve/run-qwen-serve.ts |
Stops the registry on shutdown, releases terminals in the workspace containment phase |
packages/cli/src/serve/server.test.ts |
Pins web_terminal as advertised only with acpHttpEnabled |
packages/cli/src/serve/server.ts |
Creates the registry, registers the route with a trust-checked workspace resolver |
packages/core/src/index.ts |
Exports the new registry |
packages/core/src/services/web-terminal-registry.test.ts |
Registry lifecycle, race, budget and reclaim tests incl. stale-detach guard |
packages/core/src/services/web-terminal-registry.ts |
PTY session registry with caps, bounded scrollback, proportional input budget, process-tree kill, idle reclaim |
packages/vscode-ide-companion/NOTICES.txt |
Regenerated notices for the two new @XTerm packages |
packages/web-shell/client/App.test.tsx |
Terminal entry gating, release on tab close, keyboard-isolation tests |
packages/web-shell/client/App.tsx |
Opens terminal tabs, releases on close and eviction, data-web-terminal key guards |
packages/web-shell/client/components/artifacts/ArtifactPanel.module.css |
Terminal pane styles incl. display none when hidden |
packages/web-shell/client/components/artifacts/ArtifactPanel.test.tsx |
Add-menu and empty-state terminal entries |
packages/web-shell/client/components/artifacts/ArtifactPanel.tsx |
Terminal tab kind, add-menu item, empty-state action, active state |
packages/web-shell/client/components/terminal/TerminalPanel.test.tsx |
Client framing, reconnect, release-retry, origin and focus tests |
packages/web-shell/client/components/terminal/TerminalPanel.tsx |
xterm panel with reconnect backoff, release handshake, localized notices |
packages/web-shell/client/customization.tsx |
Adds terminal to the right-panel item type |
packages/web-shell/client/i18n.tsx |
Terminal strings in EN and ZH |
packages/web-shell/client/main.tsx |
Terminal in the standalone app default right panel |
packages/web-shell/package.json |
Adds @xterm/xterm and @xterm/addon-fit |
packages/web-shell/vite.config.ts |
Dev proxy forwards the /terminal WebSocket to the daemon |
Test evidence
Unattended CI run — no PR code was built or executed here; the evidence below is the PR's own CI signal for the reviewed commit, fetched through the GitHub API. All five pull_request-event workflow runs on 7dca447c completed successfully, including the full Test (ubuntu-latest, Node 22.x) suite and Serve A/B. Test (macos-latest) and Test (windows-latest) show skipped — that is routing, not failure: those lanes were taken off PR events by #10059, which this branch already carries. Integration Tests (CLI, No Sandbox) is also skipped on this head — its qwen-serve coverage does not include this route, and the terminal paths are pinned by the 25 route test blocks and 19 registry tests instead; naming it so the gap is explicit.
| Check | Conclusion |
|---|---|
Capture web-shell visuals (ubuntu-latest, Node 22.x) |
✅ success |
Classify PR |
✅ success |
Dependency CVE audit |
✅ success |
Desktop Shell (ubuntu-22.04) |
✅ success |
Desktop Shell (windows-2022) |
✅ success |
Post Coverage Comment (ubuntu-latest, 22.x) |
✅ success |
Real daemon E2E / Java 11 |
✅ success |
Secret scan (TruffleHog) |
✅ success |
Serve A/B (ubuntu-latest, Node 22.x) |
✅ success |
Test (ubuntu-latest, Node 22.x) |
✅ success |
macos-latest / Java 21 |
✅ success |
ubuntu-latest / Java 11 |
✅ success |
ubuntu-latest / Java 17 |
✅ success |
ubuntu-latest / Java 21 |
✅ success |
web-shell E2E Smoke (ubuntu-latest, Node 22.x) |
✅ success |
windows-latest / Java 21 |
✅ success |
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。
Not verified here (unattended run): the live browser flow at this head — open, type, tab-switch, resize, reconnect replay, release handshake, and the new replay-then-release exit path. Each is unit-pinned at this head; @wenshao's full real-daemon + real-browser verification covered the entire reviewer test plan on Linux but predates all four hardening commits, and the author's self-reported live re-tests since are their claim, not evidence in this thread.
Sandboxed verification would settle this: @qwen-code /tmux — the terminal lifecycle (open → type → switch → resize → reconnect replay → close releases the PTY) is the PR's central behavioural claim and is only unit-pinned at this head; the author has write access, so the lane is available. @qwen-code /verify would add the A/B load-bearing proof for the daemon-side budget and release semantics.
中文说明
代码审查 —— 7dca447c 上的加固轮
本轮审查上次三查头（fdb8fbe）之后落地的全部改动：cc54771a、main 合并、f41be7be、7dca447c，并在审查头上完整重读了两个守护进程侧文件。独立方案先行：对遗留项（已退出会话清理、键盘事件隔离、进程树终止、输入预算健全性），我会做的正是落地的这些——退出先重放再释放、以 data-web-terminal 守护应用全局按键处理、ps 遍历后代进程后先杀子进程再回退进程组杀、按字节比例抵扣而非全额清零。PR 与之吻合，没有遗漏更简形态。
第一轮 4 个 Critical 在该头上仍然成立——我逐一对代码复核，@chiga0 在同一头上的独立核对结论一致：输出扇出背压（每次发送前检查 bufferedAmount，现为 8 MiB——自 1 MiB 上调，使 4 MiB 满滚动重放与实时输出可共存，超限清理并以可重试 1013 关闭）；错误监听窗口（ws.on('error') 是 onConnection 首条语句，先于任何 await）；输入预算（三态返回、含上限边界原子检查，背压以 1013 关闭且不释放会话，客户端可重连）；过期 detach/回收（detach 闭包与回收定时器均先校验会话身份）。
新提交的内容（已核实）：已退出会话先重放再释放（7dca447c——实时 PTY 退出不再立即释放，快照保留，重连重放输出与退出帧后才释放；收口 round-5 的 R3-4，且与问题 1 的修复相容；已退出会话不再占用 8 个名额，15 分钟回收兜底）；输入预算按比例抵扣（7dca447c——输出按字节数抵扣未确认输入而非清零，重新挂载监听不再重置预算，断连重连不能绕过 256 KiB 上限；残留权衡：完全无输出的前台程序在 256 KiB stdin 后仍会断开客户端——可重试、会话保留，但超预算输入不会送达；属设计确认而非阻断）；进程树终止（7dca447c——先按 PPID 后代与 PTY 会话成员枚举、先子后父 SIGKILL，再回退进程组杀；固定参数无 shell 注入；作者报告的真实 zsh 后台进程验证是其陈述，机制本身读来正确）；瞬时工作区不可用保留会话（f41be7be——resize/输入时发现工作区暂不可用以 4002 关闭但不释放；退出通知改为遍历副本，消除通知中释放的竞态）；释放移至工作区收容阶段（cc54771a——与 git-state、计划任务清理同阶段执行，守护进程关停由注册表的资源释放兜底；与 bypassPrimaryDrain: true 组合遗留一个有界窗口——收容之后创建的终端在 15 分钟回收前保活，且后续每条消息都重新校验工作区注册表；chiga0 判断"可接受、需确认意图"，作为开放设计确认带入结论）；键盘隔离补全（cc54771a + f41be7be——全部全局按键处理器对 [data-web-terminal] 目标（含 Shadow DOM composed 事件）放行，App 套件覆盖；客户端按配置的 daemon origin/基础路径构造 WS URL；隐藏面板改为 display:none）。
残留范围披露：本轮未逐行复审 ArtifactPanel.tsx 内部与 App.tsx 的终端页签/键盘生命周期之外的部分（有测试覆盖且早前轮已审）；客户端 TerminalPanel.tsx 由其 326 行套件固化。约 60 个未解决线程全部为 Suggestion 级（变异测试扩展、契约固化测试、辅助函数去重），作者已按五轮审查后的 PR 收敛策略明确延期——线程内均有记录，符合审查轮次指引，无静默丢弃。
测试证据
无人值守 CI 运行——此处未构建或执行任何 PR 代码；以上是审查提交上、通过 GitHub API 获取的 PR 自身 CI 信号。7dca447c 上全部 5 个 pull_request 事件工作流运行成功，含完整 Test (ubuntu-latest, Node 22.x) 套件与 Serve A/B。Test (macos-latest) 与 Test (windows-latest) 显示 skipped——属路由而非失败：该两通道已由 #10059 移出 PR 事件，本分支已携带该变更。Integration Tests (CLI, No Sandbox) 在该头上亦为 skipped——其 qwen-serve 覆盖不含本路由，终端路径由 25 个路由测试块与 19 个注册表测试固化；如实列出以明确该缺口。此处未验证（无人值守运行）：该头上的真实浏览器流程——打开、输入、切换页签、调整尺寸、重连重放、释放握手与新的"重放后释放"退出路径。各项均有单测固化；@wenshao 的完整真实 daemon + 真实浏览器验证（Linux，覆盖全部验证计划）早于四个加固提交，作者此后自报的实测是其陈述，不是本线程的证据。沙箱验证可以定论：@qwen-code /tmux——终端生命周期（打开 → 输入 → 切换 → 调整尺寸 → 重连重放 → 关闭释放 PTY）是本 PR 的核心行为主张且在该头上仅有单测固化；作者有写权限，该通道可用。@qwen-code /verify 可补充守护进程侧预算与释放语义的 A/B 承重证明。
— Qwen Code · qwen3.8-max
Reviewed at 7dca447c5bf9f5a32878ccb8e2d1dc1d8f14613f · re-run with @qwen-code /triage
|
Confidence: 3/5 — cleanest this PR has ever looked: every Critical from five review rounds is fixed and test-pinned at this head, an independent human cross-check found no blockers, and CI is fully green — but the standing maintainer-awareness escalation for a ~1.4k-production-line feature touching core caps the verdict, and two design confirmations stay open. The cap is policy, not doubt. Stepping back: since the last defer this PR went strictly in the right direction. The four fix commits are each a named defect closed by a named regression test — replay-then-release for exited sessions, key-event isolation across every global handler including Shadow DOM, a descendants-first process-tree kill, proportional input-budget credit that a detach/reconnect can no longer bypass. @chiga0's cross-check on this exact head refuted all four round-1 Criticals independently of my read, and the two questions he left (the drain-bypass creation window, the silent-stdin-consumer UX at the 256 KiB budget) are design confirmations with bounded worst cases, not defects. The author also held the line on scope: the ~60 surviving threads are Suggestion-level and were deferred under the review-round convergence policy with the rationale recorded in-thread — that is the documented posture after five rounds, not silent drop. If the question were "is this code ready to merge?", the answer is yes. Three things keep this out of an auto-approve. First, policy: a ⏸️ Deferring to @wenshao and @qqqys — three human actions, all small: (1) sign off on (or lift) the standing maintainer-awareness escalation for this core-touching feature; (2) dismiss or re-review the stale round-5 中文说明置信度：3/5 —— 本 PR 迄今为止最干净的状态：五轮审查中的每个 Critical 均已修复并在该头上以测试固化，独立的人工交叉核对未发现阻断项，CI 全绿——但对触及核心、约 1.4k 生产行功能的既有维护者关注升级给结论封顶，且仍有两项设计确认开放。封顶是政策，不是疑虑。 退一步看：自上次转交以来，本 PR 严格向好。四个修复提交各自以点名的回归测试关闭一个点名的缺陷——已退出会话的重放后释放、覆盖全部全局按键处理器（含 Shadow DOM）的键盘事件隔离、先子后父的进程树终止、断连重连无法再绕过的按比例输入预算抵扣。@chiga0 在同一头上的交叉核对独立于我的阅读推翻了全部 4 个第一轮 Critical，他留下的两个问题（排空旁路下的创建窗口、256 KiB 预算下静默消费 stdin 的交互模型）是坏情形有界的设计确认，不是缺陷。作者也在范围上守住了边界：约 60 个存活线程均为 Suggestion 级，按五轮后的审查收敛策略延期，且线程内记录了理由——这是有文档依据的姿态，不是静默丢弃。若问题是"这份代码可以合并了吗"，答案是肯定的。 三件事使其不走自动批准。其一，政策：触及 ⏸️ 转交 @wenshao 与 @qqqys——三项人工动作，都很小：（1）对该触及核心的功能给出（或解除）维护者关注签核；（2）dismiss 或重审过时的第 5 轮 — Qwen Code · qwen3.8-max Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
整体评估 / Overall assessment
在 head c0114c08 上完整审了 29 个文件（+2483/−16）：daemon 侧 PTY 注册表 + /terminal WS 路由 + xterm 面板 + 双重门控。静态审查（未运行测试），CI 全绿。整体设计扎实——升级门控复用、帧协议、竞态处理（创建中释放/排空/重放）和测试密度都很好。但有 1 个 🔴：pre-attach 窗口缺 WS error 监听，一个协议违规帧可以击垮整个 daemon。另有 5 个 🟡 值得处理。
先交代 bot 审查（针对更早的 head）的核心关切：描述承诺 QWEN_WEB_TERMINAL=1 环境门控而代码未实现。在当前 head 上，PR 描述与设计文档均已重写、不再承诺环境变量门控（全库无 QWEN_WEB_TERMINAL 引用），实际门控为"宿主配置项 + capability + bearer 认证 + 工作区信任"，与代码一致；残留物见下文第 11 条。两处纯格式化 drive-by（geminiChat.ts、config-session-env.test.ts）已逐行核实确无语义变化，建议若再触碰分支顺手去掉。
Reviewed all 29 files (+2483/−16) at head c0114c08: daemon-side PTY registry + /terminal WS route + xterm panel + dual gating. Static review (no tests run); CI fully green. The design is solid — upgrade-gate reuse, framing, race handling (release-during-create / drain / replay) and test density are all good. But there is 1 🔴: the pre-attach window lacks a WS error listener, so a single protocol-violation frame can take down the entire daemon, plus 5 🟡 items worth addressing.
Context on the earlier bot review (against an older head): its main concern was that the description promised a QWEN_WEB_TERMINAL=1 env gate the code never implemented. At the current head both the PR description and the design doc have been rewritten and no longer promise an env gate (zero QWEN_WEB_TERMINAL references repo-wide); the shipped gating is host-config item + capability + bearer auth + workspace trust, matching the code. Residue is covered in item 11 below. The two formatting-only drive-bys (geminiChat.ts, config-session-env.test.ts) were verified line-by-line as purely cosmetic — worth dropping if the branch is re-touched.
🔴 1. pre-attach 窗口没有 WS error 监听——一个坏帧击垮整个 daemon
packages/cli/src/serve/routes/terminal.ts:128-129（pre-attach 只注册 close/message）；:258（首个 error 监听在 attach 之后）；packages/cli/src/serve/acp-http/index.ts:1858-1868（extra route 拿到的 socket 不带任何 error 监听）
从 ws.once('close')/ws.on('message') 注册（:128-129）到 attach 段注册 ws.on('error', cleanup)（:258）之间，横着 await registry.create(...)（动态 import PTY + spawn，可达数百毫秒）。这个窗口内的协议违规（未掩码帧、非法 UTF-8 文本帧、超过 maxPayload 的帧）会让 ws（8.21.3）在 WebSocket 实例上 emit error；没有监听器时 EventEmitter 抛 ERR_UNHANDLED_ERROR → daemon 的未捕获异常处理器只豁免良性 PTY 竞态（uncaught-exception-handler.ts:62-73），其余一律 process.exit(1)——所有工作区的所有会话一起陪葬。对比：ACP 分支自己注册 error 监听（acp-http/index.ts:1901），voice 路由在 onConnection 开头同步注册三件套（voice-ws.ts:516-520）——本路由偏离了既有模式。默认定位的 loopback 无 token 姿态下任何本地进程都能升级成功（Origin 检查只在带 Origin 头时生效），触发成本极低。
修复：在 onConnection 开头（:128 附近）就注册 ws.on('error', markClosed)（或触发 cleanup），与 voice-ws 一致；另建议在分发点对 extraRoute.onConnection 的返回 promise 加防御性 catch（目前是悬浮 promise，acp-http/index.ts:1866）。
EN: Between the pre-attach close/message registration (:128-129) and the first error listener (:258) sits await registry.create(...) (dynamic PTY import + spawn, up to hundreds of ms). A protocol violation in that window (unmasked frame, invalid UTF-8 text frame, frame over maxPayload) makes ws 8.21.3 emit error on the WebSocket instance; with no listener, EventEmitter throws and the daemon's uncaught-exception handler — which suppresses only benign PTY races — calls process.exit(1), taking down every session in every workspace. The dispatch site hands extra routes a bare socket (no error listener), while the ACP branch and the voice route both register one immediately. In the default loopback-no-token posture any local process can reach this. Fix: register ws.on('error', markClosed) at the top of onConnection (mirroring voice-ws), ideally plus a defensive catch on the extraRoute.onConnection promise at the dispatch site.
🟡 2. 陈旧 detach 闭包可给被回收的 id 上定时器，误杀后来的新会话
packages/core/src/services/web-terminal-registry.ts:204-208（detach 闭包）与 :314-320（reclaim 回调只查 listener 数量，不查身份）
addOutputListener 返回的闭包捕获 session 对象但按键 terminalId 行事。时序：会话 S1（id=X）挂载中 → 工作区排空触发 release(X)（S1 从 sessions 移除、PTY 被杀，但路由此刻若尚未感知到 ws close，detach1 还没被调用）→ 客户端 ws 随后关闭 → 路由 cleanup 调 detach1() → 陈旧 S1 的 listeners 归零 → scheduleReclaim(X, S1) 给已删除的 S1 上 15 分钟定时器 → 客户端以同一 terminalId 重连（tab 未关、网络恢复——id 来自 URL 查询参数、同 tab 复用）→ 新会话 S2 建在 X 下 → 定时器到期，回调只见 S1.outputListeners.size === 0 → this.release('X') → 杀掉 S2。release() 清理的是"当前注册者"的 reclaimTimer（:277），清不掉挂在陈旧 S1 上的那个。
修复（一行级）：reclaim 回调与 detach 闭包都加身份校验 this.sessions.get(terminalId) === session（并检查 disposed），并补一条"release 后 detach + id 复用"的回归测试。
EN: The detach closure captures the session object but acts by terminalId, and the reclaim callback only re-checks the listener count, not identity. Sequence: S1 attached (id=X) → drain releases X (S1 removed, PTY killed) before the route sees the ws close → ws closes later → route cleanup calls the stale detach → zero listeners → scheduleReclaim(X, S1) arms a 15-minute timer on the removed session → client reconnects with the same terminalId (URL-param id, reused by the same tab) → new session S2 under X → timer fires and release('X') kills S2. release() clears the registered session's timer, not the stale one. Fix: identity-check this.sessions.get(terminalId) === session in both the detach closure and the reclaim callback (plus a disposed check), with a release-then-detach-then-reuse regression test.
🟡 3. 裸 spawned.kill()——正是仓库文档记载不够用的那种 kill
packages/core/src/services/web-terminal-registry.ts:152（kill: () => spawned.kill()）与 :281；对照 packages/core/src/services/shellExecutionService.ts:525-545（ConPTY 下 pty.kill() 只拆掉伪控制台宿主、不杀 shell 本体，曾导致 Windows 空闲 shell 堆积至 OOM；故用 taskkill 树杀）与 :637-640（POSIX 进程组杀 process.kill(-pid, 'SIGKILL')）
Web 终端是这个已知缺陷的最坏租户：长驻交互 shell + 15 分钟回收反复"杀"分离会话。Windows 上被回收的 shell 进程会按既有 issue 的模式堆积；POSIX 上 shell 的后台子进程会作为孤儿存活。另外 onData/onExit 注册（:171/:186）丢弃了 node-pty 返回的 disposable——若 kill 未落地，已释放会话的闭包还会继续缓冲至多 4 MB。
修复：把 shellExecutionService 里现成的平台清理策略（Windows taskkill 树杀 + ConPTY 宿主杀；POSIX 进程组杀）抽出来复用，并在 release 时 dispose onData/onExit。
EN: Bare spawned.kill() is exactly the kill the codebase documents as insufficient: under ConPTY it tears down only the pseudo-console host, not the shell itself (the Windows idle-shell OOM pattern), which is why shellExecutionService uses tree taskkill on Windows and process-group SIGKILL on POSIX. The web terminal is the worst-case tenant: long-lived interactive shells plus repeated reclaim kills. Also, the onData/onExit disposables are discarded, so a released session whose kill didn't land keeps buffering up to 4 MB in a detached closure. Fix: extract and reuse the existing platform cleanup strategies; dispose the pty listeners on release.
🟡 4. web_terminal 无条件通告——在端点不存在或 PTY 不可用的环境里也显示入口
packages/cli/src/serve/capabilities.ts:449（基线条目，不在 CONDITIONAL_SERVE_FEATURES）；packages/cli/src/serve/acp-http/index.ts:642-643（QWEN_SERVE_ACP_HTTP=0 时根本不挂 WS upgrade 监听）；packages/core/src/utils/getPty.ts:22-24（Bun 下返回 null）
/terminal 完全依赖 ACP-HTTP 的 upgrade 监听器，QWEN_SERVE_ACP_HTTP=0 姿态下端点不存在，capability 却照常通告，Web Shell 显示入口、每次连接都失败；Bun 运行时下 PTY 恒不可用同样如此。同一传输上的兄弟能力是正确做法：voice_transcribe 以 voiceWsAvailable !== false 为条件、realtime_voice 以 acpHttpEnabled === true 为条件（capabilities.ts:667-674）。
修复：给 web_terminal 加同类条件谓词（至少 acpHttpEnabled），并按兄弟条目的样式补上契约注释；顺带把 server.test.ts 里"for frontend version compatibility"的测试标题改成与实际断言（无条件通告 → 条件通告）匹配的措辞。
EN: /terminal lives exclusively on the ACP-HTTP upgrade listener, yet web_terminal is a baseline capability: with QWEN_SERVE_ACP_HTTP=0 the endpoint does not exist, and under Bun getPty() is null, but the entry still shows and every connect fails. The siblings on the same transport do it right (voice_transcribe gated on voiceWsAvailable, realtime_voice on acpHttpEnabled). Fix: make web_terminal conditional (at least on acpHttpEnabled), add a contract comment like its siblings, and adjust the test title accordingly.
🟡 5. 初始 resize 竞态：PTY 可能被钉在 80×24 直到下次面板缩放
packages/web-shell/client/components/terminal/TerminalPanel.tsx:107-114（首个 fit() 推迟到 rAF）、:192-201（onopen 立即发 term.cols/term.rows）、:247-269（ResizeObserver 观察的是容器）
首次 fit() 在 rAF 里（约 16ms 后），而 WebSocket 同一 effect 内同步创建；loopback 下 onopen 握手可以先于首帧完成，此时发出的 resize 是 xterm 默认 80×24。之后 rAF 的 fit() 改变的是 canvas 而非容器尺寸，ResizeObserver 不触发，没有补发——结果界面 137×32、PTY 80×24（折行/光标错位），直到用户手动缩放面板或重连才纠正。
修复：把 resize 控制帧改由 term.onResize(({cols, rows}) => ...) 驱动（fit() 引发的尺寸变化也会触发它），observer 只负责触发 fit()；或在首次 fit() 后若 ws.readyState === OPEN 补发一帧。
EN: The first fit() is deferred to requestAnimationFrame while the WebSocket is created synchronously in the same effect; on loopback the onopen handshake can complete first, sending the xterm defaults (80×24). The later fit() changes the canvas, not the observed container, so no corrective resize frame is ever sent — the PTY stays 80×24 (wrapped output, misplaced cursor) until the panel is resized or a reconnect happens. Fix: drive the resize control frame from term.onResize (fires for fit() too), or send a resize frame right after the initial fit() when the socket is OPEN.
🟡 6. 信任撤销/运行时替换后，空闲挂载的终端不会被释放
packages/cli/src/serve/run-qwen-serve.ts:5683-5701（释放只接在 beginDrain）；packages/cli/src/serve/workspace-trust-reconciler.ts:113,305（信任变更走 beginReplacement）；packages/cli/src/serve/routes/terminal.ts:207-232（再验证只发生在客户端发消息时）
releaseWebTerminalsForWorkspace 只在 beginDrain 钩子里被调用，而信任重协调通过 beginReplacement 替换运行时、不走 drain；路由侧的工作区再验证又只在收到 resize/输入时执行。于是一个挂载中但空闲（无按键）的终端，在其运行时被替换或降为不受信后，输出监听会压住 15 分钟空闲回收，PTY 持续流式输出——直到下一次按键才被 4002 关掉。这违背设计文档"每个终端由一个活的、受信任的工作区运行时拥有"的不变量。
修复：推式失效——在替换/撤销时通知注册表或路由，对受影响 cwd 关闭挂载 socket（4002）并释放；至少在设计文档中明确"空闲窗口内信任变更延迟生效"。
EN: Terminal release is wired only into beginDrain, but trust reconciliation replaces runtimes via beginReplacement, and the route re-validates the workspace only when the client sends a message. An attached-but-idle terminal therefore survives a trust revocation / runtime replacement indefinitely (its output listener suppresses idle reclaim), streaming until the next keystroke triggers the 4002 path — violating the design doc's "owned by one active, trusted workspace runtime" invariant. Fix: push-based invalidation on replacement/revocation (close attached sockets with 4002 and release for the affected cwd), or document the delayed-enforcement window explicitly.
🟢 7. release() 漏掉 CLOSING 状态
packages/web-shell/client/components/terminal/TerminalPanel.tsx:224-235
OPEN（发送 release + close）、CONNECTING（onopen 补发）、CLOSED/不存在（新建连接再释放）都有分支，唯独 readyState === CLOSING（例如服务端发起关闭的握手中用户关了页签）两个分支都不命中，release 帧永远发不出去，PTY 要等 15 分钟回收——这是唯一违背"关闭页签即杀 PTY"文档承诺的路径。影响有界；修复：CLOSING 分支走与 CLOSED 相同的 connect-then-release。
EN: release() handles OPEN, CONNECTING and CLOSED/absent, but when the socket is mid-close-handshake (CLOSING) neither branch runs and no release frame is ever sent — the PTY survives until the 15-minute reclaim, the one tab-close path that violates the doc's "closing a terminal tab kills the PTY". Bounded impact; fix by treating CLOSING like CLOSED (connect-then-release).
🟢 8. 已退出会话占满 15 分钟的并发槽与滚动缓冲
packages/core/src/services/web-terminal-registry.ts:186-190（onExit 不（重）排回收）与 :100-105（上限 8 计入一切在册会话）
只有干净关闭页签会发 release；异常断连后 shell 退出的会话会在 sessions 里躺满 15 分钟、各占一个并发槽和至多 4 MB。8 个这样的会话就能让新终端得到"Web terminal limit reached"长达 15 分钟。便宜修法：onExit 时无监听者（或从已退出会话 detach 时）改用短得多的回收 TTL——进程已不在，只剩重放价值。
EN: Only a clean tab close sends release; an exited session left by an abnormal disconnect sits in sessions for the full 15 minutes, holding a concurrency slot (cap 8) and up to 4 MB each — eight of them block new terminals for up to 15 minutes. Cheap fix: when onExit fires with no listeners (or on detach from an already-exited session), schedule a much shorter TTL — the process is gone; only replay value remains.
🟢 9. 单个超 4 MB 输出块会被整块丢弃
packages/core/src/services/web-terminal-registry.ts:171-184（驱逐循环把超大新块自己 shift 掉）；web-terminal-registry.test.ts:204-214（测试固化了 4.5 MB 块后 output === ''）
一次大粘贴/大输出会抹掉全部滚动缓冲而不是保留末尾 4 MB。建议：当最新块单独超过 MAX_BUFFER_BYTES 时保留其尾部切片。
EN: A single chunk larger than the 4 MB cap is evicted wholesale — one large paste erases all scrollback instead of keeping the trailing 4 MB. Retain the tail slice when the newest chunk alone exceeds MAX_BUFFER_BYTES.
🟢 10. 退出 signal 被丢弃
packages/core/src/services/web-terminal-registry.ts:186-190（只存 exitCode）与 :222-231（快照同样只暴露 exitCode）
node-pty 在信号终止时可能报 exitCode: 0 + 非零 signal（shellExecutionService 有文档记载）；被信号杀掉的 shell 会被重放成干净的 exit 0。建议把 signal 记入会话并加入 WebTerminalSnapshot。
EN: Only exitCode is stored and snapshotted; node-pty can report exitCode: 0 with a non-zero signal on signal termination (documented in shellExecutionService), so a signal-killed shell replays as a clean exit. Record signal on the session and include it in the snapshot.
🟢 11. 一致性残留：parseControl 注释、文档措辞、4003、术语
packages/cli/src/serve/routes/terminal.ts:60-62catch 注释说非法控制帧"保留为终端输入"，实际处理器将其丢弃（:198-199，测试也固化了丢弃）——行为更安全，注释过时。docs/design/web-terminal-lifecycle.md说文本帧"保留"给控制消息，但服务端实际接受任意非控制文本帧作为终端输入；要么收紧服务端、要么软化文档。- 客户端
NON_RETRYABLE_CLOSE_CODES仍含 4003（TerminalPanel.tsx:24），而服务端从不以 4003 关闭（全库唯一的 4003 发射方是无关的 Live Host 协议）——上一轮设计迭代的残留，删除或加注释。 - 术语碰撞：
docs/developers/daemon-client-adapters/web-ui.md描述过一个"不是 raw PTY 代理"的 web terminal；本 PR 交付的恰是同名 raw PTY 代理，建议在新设计文档加一行消歧。
EN: Stale comment — parseControl's catch says malformed frames are "preserved as terminal input" but the handler drops them (:198-199, tests pin the drop). Doc overstatement — the design doc says text frames are "reserved" for control messages, but the server accepts any non-control text frame as PTY input; tighten the server or soften the doc. Dead code — the client's non-retryable set still contains 4003 (TerminalPanel.tsx:24) though the server never closes with it (the only 4003 emitter repo-wide is the unrelated Live Host protocol); drop or comment it. Terminology — the adapter docs describe a "not a raw PTY proxy" web terminal; a one-line disambiguation in the new design doc would prevent confusion.
🟢 12. 终端内提示语绕过 i18n + bearer 编码器复制
TerminalPanel.tsx:139,145,214把[Process exited with code …]、[Error: …]、[Connection lost — reconnecting…]硬编码英文写进终端缓冲；其余新文案都做了中英双语。可经现有 i18n hook 本地化后写入。TerminalPanel.tsx:37-48与voice/useVoiceCapture.ts:89-100的bearerSubprotocol()逐字节相同，而 daemon 侧的同步注释（acp-http/index.ts:257-259）只点名 voice 是规范副本——抽共享 helper，或至少更新同步注释。
EN: Hardcoded English notices are written into the terminal buffer (TerminalPanel.tsx:139,145,214) while every other new string is localized EN/ZH — route them through the i18n hook. The bearerSubprotocol() encoder is a byte-for-byte copy of the voice client's, and the daemon's sync comment still names only the voice copy as canonical — extract a shared helper or update the comment.
🟢 13. 客户端小健壮性项
TerminalPanel.tsx:110-113：首次fit()有 try/catch，catch 里安排的 rAF 重试没有——页签在第二帧前关闭会对已 dispose 的终端抛未捕获异常。补同样的保护或检查 disposed。TerminalPanel.tsx:160-166：cleanup 里term.dispose()与 socket 真正关闭之间，缓冲消息仍会到达并调用已 dispose 终端的write（xterm 只告警不崩）；加一行if (disposed) return;即闭环。TerminalPanel.tsx:98：fontFamily: 'var(--font-mono, ...)'大概率不作用于 canvas 字形渲染——CanvasRenderingContext2D.font不解析 CSS 变量，非法值被静默忽略，而 DOM 侧度量却解析该变量，可能字体/度量不匹配。用字面字体栈或getComputedStyle解析后传入。
EN: The fit() retry scheduled inside the catch branch is itself unguarded and can throw after unmount. handleMessage lacks a disposed guard, so buffered messages between term.dispose() and socket close still hit the disposed terminal. fontFamily: 'var(--font-mono, ...)' almost certainly never applies to canvas glyph rendering — CanvasRenderingContext2D.font does not resolve CSS variables and silently ignores invalid values, while DOM measurement does resolve it, risking a font/metrics mismatch; pass a literal stack or resolve via getComputedStyle.
💡 建议 / Suggestions
- 每次按键都重跑工作区解析器（terminal.ts:222），非标 selector 会逐键付出一次同步
realpathSync.native；可缓存已解析运行时、按廉价条件复核。/ The workspace resolver re-runs on every keystroke (terminal.ts:222); a non-canonical selector pays a synchronousrealpathSync.nativeper key. Cache the resolved runtime and re-validate cheaply. - 连接从未成功（如 1006 循环）时无限退避重试；可对"从未 OPEN"的连续失败设上限或 N 次后在终端内报错。/ Reconnect retries forever when the connection never opens (e.g. repeated 1006); cap never-opened attempts or surface an error after N failures.
- vite dev 代理
/terminal是裸前缀（vite.config.ts:121-123），也匹配/terminals等并可能遮蔽未来同名顶层路径——voice 的注释专门警告过这个陷阱；改'^/terminal/?$'。/ The dev proxy/terminalis a bare prefix (also matches/terminals, may shadow future top-level paths) — exactly the trap the voice comment warns about; use'^/terminal/?$'. - 其他小项：挂载后的输入帧在 10 MB maxPayload 下没有更小的单帧上限（terminal.ts:233 直写 PTY）；terminalId 由客户端提供，同租户内可抢注（受害者得到不可重试的 4001，影响低）；ArtifactPanel 的终端入口只查
onOpenTerminal、未像兄弟入口那样同时查items.includes('terminal')（App 层门控使其当前不可达，纵深防御可补）；注册表测试可补：spawn 失败后槽位释放、getPty reject 分支、FIFO 驱逐顺序、未挂载即回收、dispose 已建立会话。
EN (grouped): no per-message cap below the 10 MB maxPayload on post-attach input (terminal.ts:233 writes straight to the PTY); client-supplied terminalId is squat-able within the shared-tenant daemon (victim gets a non-retryable 4001; low impact); the ArtifactPanel terminal entry checks only onOpenTerminal, not items.includes('terminal') like its siblings (unreachable today via the App-level gate; defense in depth); registry tests worth adding: cap-slot freeing after spawn failure, the getPty-rejection branch, FIFO eviction order, never-attached reclaim, and dispose of established sessions.
🎉 值得点名 / Highlights
- 共享升级门控（路径白名单、Host/Origin/CSWSH、bearer 哈希比较）在 socket 到达路由前全部执行，
/terminal无法绕过；bearer 子协议从不被回显，客户端只递交非秘密标记（TerminalPanel.tsx:50-53）。 - 客户端
cwd确实只是 selector：PTY 的 cwd/env 全部来自解析出的受信活体运行时（server.ts:2946-2961 → terminal.ts:144-148），无注入面；terminalId↔工作区绑定在挂载与释放两侧都防跨工作区劫持。 cancelledCreations+ 上限计入创建中会话 +finishCreating覆盖全部退出路径——创建/释放竞态干净无泄漏；排空/停机释放接线完整，bypassPrimaryDrain做成通用 ExtraWsRoute 属性且双向有测试。- 帧协议防碰撞：键盘输入恒为二进制帧、控制恒为 NUL 前缀文本帧，粘贴的
\x00{"type":"release"}无法释放终端（有测试证明）。 - 环境归一化作用域正确：copy-then-delete，nvm 的 npm_config_prefix 修复不触碰运行时环境本体（web-terminal-registry.ts:127-131，有测试）。
- 客户端双门控（宿主项 + capability）合取、capability 晚到时 fail-closed（
=== true），重连重放前先term.reset()防重复，关闭码集合与服务端契约精确镜像。
EN: Shared upgrade gates (path allowlist, Host/Origin/CSWSH, bearer hash-compare) all run before the socket reaches the route — /terminal cannot bypass them, and the bearer subprotocol is never echoed back. Client cwd is truly only a selector: PTY cwd/env come exclusively from the resolved trusted live runtime, with terminalId↔workspace binding enforced on both attach and release. cancelledCreations + cap counting in-flight creations + finishCreating on every exit path — the create/release race is clean; drain/shutdown release wiring is complete, and bypassPrimaryDrain is a general ExtraWsRoute property tested in both directions. Collision-proof framing (binary input vs NUL-prefixed control; a pasted release frame cannot release — tested). Env normalization is copy-then-delete, so the nvm fix never touches the runtime env. Client dual gating is conjunctive and fail-closed on late capabilities, replay is preceded by term.reset(), and the close-code set mirrors the server contract exactly.
Local verification on Linux — real daemon + real browserSince the PR table marks 🐧 Linux as untested, I built a full two-arm environment on Linux and ran the reviewer test plan end to end against a real Setup — Debian 13 / kernel 6.12, x86_64, Node 22.22.2, PTY backend
Everything below is browser → vite proxy → real daemon → real PTY; no mocks, no Reviewer test plan — all 7 items pass on Linux
Protocol and lifecycle probes (raw WebSocket client against the live daemon)
Tests and static checks in the PR worktree: Finding 1 (worth fixing before merge) — closing a tab whose shell already exited never releases the sessionReproduced through the real UI on a verified-fresh daemon: open a terminal, type
Proof at the daemon level: after the UI closes an exited tab, re-attaching to that same terminal id still replays Root cause is the ordering in Two shapes of fix that both look small:
Finding 2 (worth a note in the PR description) — a page reload orphans the PTY
Finding 3 (design question, not a blocker) — no operator opt-outOn this default daemon, Smaller observations
Not coveredWindows, non-UTF-8 input from non-browser clients, and third-party TUIs that do not redraw on Overall the daemon side is in good shape — auth, CSRF, trust, workspace binding, drain, reclaim and shutdown all behave on Linux. I would like finding 1 fixed before merge, since it turns an ordinary usage pattern into a 15-minute dead end. 中文说明Linux 本地验证 —— 真实 daemon + 真实浏览器PR 的测试矩阵里 🐧 Linux 标注为未测试，所以我在 Linux 上搭了完整的双臂环境，用真实 环境：Debian 13 / 内核 6.12，x86_64，Node 22.22.2，PTY 后端
下面所有结果都是 浏览器 → vite 代理 → 真实 daemon → 真实 PTY 的链路，没有任何 mock，也没有 评审验证计划 7 项在 Linux 上全部通过
（图见上方英文部分。） 协议与生命周期探针（裸 WebSocket 客户端直连真实 daemon）
PR worktree 内的测试与静态检查： 问题 1（建议合并前修复）—— shell 已退出后再关闭页签，会话永远不会被释放在验证过确实是全新启动的 daemon 上通过真实 UI 复现：打开终端 → 输入 daemon 层的证据：UI 关闭已退出的页签后，用同一个 terminal id 重新挂载，仍然会重放 根因在 两种看起来都很小的修法：
问题 2（建议在 PR 描述里说明）—— 整页刷新会遗留孤儿 PTY
问题 3（设计问题，不阻塞）—— 缺少运维侧关闭开关在这台默认配置的 daemon 上， 其它小观察
未覆盖Windows、非浏览器客户端的非 UTF-8 输入，以及不响应 整体上 daemon 侧状态不错 —— 认证、CSRF、信任、工作区绑定、排空、回收和关闭在 Linux 上都符合预期。我希望问题 1 能在合并前修掉，因为它会把一个很常见的使用方式变成 15 分钟的死路。 🤖 Generated with Claude Code — Claude Opus 5 (1M context) |
| FitAddon: vi.fn(() => ({ fit: vi.fn() })), | ||
| })); | ||
| vi.mock('../../themeContext', () => ({ useTheme: () => 'light' })); | ||
| vi.mock('../../config/daemon', () => ({ getDaemonToken: () => '' })); |
There was a problem hiding this comment.
[Suggestion] Pattern (5 locations): the client suite leaves the terminal's contract paths unpinned. At this location: the suite stubs the daemon token to '' for every test and never asserts that TerminalPanel offers the ['qwen-ws', 'qwen-bearer.<base64url(token)>'] subprotocols to the WebSocket constructor — the only auth channel a browser WebSocket has to the token-gated daemon (browsers cannot set Authorization on a WS; the daemon's extractUpgradeBearer rejects the upgrade with 401 without it). FakeWebSocket already captures the constructor's protocols argument, and the sibling voice feature pins exactly this (useVoiceCapture.test.tsx asserts the qwen-ws marker and the qwen-bearer. prefix). A refactor of wsProtocols() that drops, empties, or reorders the offered subprotocols ships with all tests green — and on any token-gated daemon (LAN pairing, non-loopback access, Local Control credentials) every /terminal upgrade is rejected 401 → retryable 1006 → the panel loops '[Connection lost — reconnecting…]' forever while REST/SSE/voice keep working. Fix: stub getDaemonToken to a non-empty token and assert FakeWebSocket.instances[0].protocols equals ['qwen-ws', 'qwen-bearer.' + base64url(token)].
中文说明
模式（5 处）：客户端套件没有固定终端的契约路径。此处的情况：套件在所有测试中把 daemon token stub 成 ''，且从未断言 TerminalPanel 向 WebSocket 构造器提供了 ['qwen-ws', 'qwen-bearer.<base64url(token)>'] 子协议——这是浏览器 WebSocket 对带 token 门禁的 daemon 唯一的认证通道（浏览器无法在 WS 上设置 Authorization；daemon 的 extractUpgradeBearer 在缺少它时以 401 拒绝升级）。FakeWebSocket 已经会捕获构造器的 protocols 参数，而兄弟 voice 功能恰好固定了这一点（useVoiceCapture.test.tsx 断言了 qwen-ws 标记和 qwen-bearer. 前缀）。如果重构 wsProtocols() 时删除、置空或调换了提供的子协议，所有测试仍会绿色出厂——而在任何带 token 门禁的 daemon 上（LAN 配对、非 loopback 访问、Local Control 凭据），每次 /terminal 升级都会被 401 拒绝 → 可重试的 1006 → 面板永远循环“连接丢失——重连中…”，而 REST/SSE/voice 照常工作。修复：把 getDaemonToken stub 成非空 token，并断言 FakeWebSocket.instances[0].protocols 等于 ['qwen-ws', 'qwen-bearer.' + base64url(token)]。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred as test-only follow-up. Authenticated terminal WebSocket behavior was verified end-to-end through the Vite proxy, while the dedicated subprotocol constructor assertion is not added in this late round.
| function render(): FakeWebSocket { | ||
| act(() => { | ||
| root.render(<TerminalPanel terminalId="terminal:one" cwd="/workspace" />); |
There was a problem hiding this comment.
[Suggestion] Pattern (5 locations): the client suite leaves the terminal's contract paths unpinned. At this location — the connect contract itself: the workspaceCwd → cwd prop → ?cwd= URL chain that gets a /terminal connect past the route's mandatory-workspace check is asserted by no test. Both panel-level mocks capture only terminalId (App.test.tsx and ArtifactPanel.test.tsx), so dropping or misspelling cwd={tab.workspaceCwd ?? workspaceCwd} in ArtifactPanel ships green; and FakeWebSocket records url but no test ever inspects it (zero hits), while the daemon route independently reads the names back (url.searchParams.get('terminalId') / .get('cwd')) and no integration test opens /terminal. A rename at either end ships with the web-shell, cli-route, and integration suites all green — and at runtime every terminal connect fails the route's mandatory-field check → close 4002 (non-retryable) → every Terminal open becomes a permanently dead panel with zero red tests. Fix: capture the cwd prop in the panel mocks and assert the override and fallback; here, assert new URL(ws.url) has pathname === '/terminal', terminalId and cwd=/workspace.
中文说明
模式（5 处）：客户端套件没有固定终端的契约路径。此处是连接契约本身：让 /terminal 连接通过路由“必须带工作区”检查的 workspaceCwd → cwd prop → ?cwd= URL 链条没有任何测试断言。两个面板级 mock 都只捕获 terminalId（App.test.tsx 和 ArtifactPanel.test.tsx），因此在 ArtifactPanel 中删除或拼错 cwd={tab.workspaceCwd ?? workspaceCwd} 可以绿色出厂；FakeWebSocket 记录了 url 但没有任何测试检查它（零命中），而 daemon 路由独立地把这些参数名读回（url.searchParams.get('terminalId') / .get('cwd')），也没有集成测试打开过 /terminal。任何一端的改名都会在 web-shell、cli 路由、集成三个套件全绿的情况下出厂——运行时每次终端连接都会因路由的必填字段检查失败 → 关闭 4002（不可重试）→ 每次打开终端都变成永久死亡的面板，且没有任何测试变红。修复：在面板 mock 中捕获 cwd prop 并断言覆盖与回退两种取值；在此处断言 new URL(ws.url) 的 pathname === '/terminal'、terminalId 与 cwd=/workspace。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Partially addressed in 50a15d0: TerminalPanel now pins pathname, terminalId, and cwd in its connection contract test. ArtifactPanel override/fallback prop assertions are deferred.
|
@wenshao @ytahdn Thanks for the detailed reviews. Commit
Workspace trust replacement already flows through The Vite proxy remains Remaining mutation-test expansions, signal metadata, reconnect-policy tuning, and helper deduplication are non-critical suggestions after several review rounds; I replied on those threads and left them open for follow-up instead of widening this PR further. Verified with the focused registry (18), route (23), and client (13) tests, a full install/build, and a real Vite-to-daemon terminal session (prompt, command I/O, release, reconnect). |
# Conflicts: # packages/web-shell/client/App.tsx
|
E2E verification for 4b1f855:
|
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.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped after round 4 of 5 without two consecutive dry rounds; every round reported findings.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/core/src/services/web-terminal-registry.test.ts:424 — [review] shared single PTY mock leaves multi-session kill attribution unpinned (wrong-session kill mutant ships 19/19 green)packages/cli/src/serve/routes/terminal.test.ts:401 — [review] create-failure releaseRequested precedence branch exercised by no test (probe-confirmed, mutant ships green)packages/cli/src/serve/routes/terminal.test.ts:498 — [review] cleanup() in the write-failure branch unpinned (mutant ships 26/26 green; stale listener stays on live session)packages/web-shell/client/components/artifacts/ArtifactPanel.test.tsx:392 — [review] inactive terminal panes' terminalPaneHidden class and aria-hidden are unasserted anywhere in the packagepackages/cli/src/serve/routes/terminal.test.ts:443 — [review] zero-byte frames accumulate unbounded pending entries during the create-pending window (probe: 5000 empty frames -> 5000 flushes, no cap)packages/cli/src/serve/routes/terminal.test.ts:496 — [review] no test pins that the 64 KiB pending-input cap stops applying post-attach (ws.off(bufferMessage) switch unguarded)
Convergence: round 4 posted 6 inline comment(s), 6 of them reported for the first time; the previous round posted 24 (6 new). Findings keep coming back to the same files: packages/web-shell/client/App.tsx (findings in rounds 1, 3; 2 more now); packages/cli/src/serve/routes/terminal.ts (findings in rounds 2, 3; 1 more now); packages/cli/src/serve/routes/terminal.test.ts (findings in round 2; 1 more now), and 2 more file(s). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：reverse audit — stopped after round 4 of 5 without two consecutive dry rounds; every round reported findings。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 6 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 4 轮发布了 6 条行内评论，其中 6 条是首次提出；上一轮发布了 24 条（其中 6 条首次提出）。发现反复回到同一批文件：packages/web-shell/client/App.tsx（第 1、3 轮已出过发现，本轮又有 2 条）；packages/cli/src/serve/routes/terminal.ts（第 2、3 轮已出过发现，本轮又有 1 条）；packages/cli/src/serve/routes/terminal.test.ts（第 2 轮已出过发现，本轮又有 1 条），另有 2 个文件。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.2)
| ws.emit('pong'); | ||
| await vi.advanceTimersByTimeAsync(15_000); | ||
| expect(ws.terminate).not.toHaveBeenCalled(); |
There was a problem hiding this comment.
[Suggestion] The heartbeat test feeds exactly one pong, so the route's persistent ws.on('pong', onPong) subscription (terminal.ts:122) is indistinguishable from a one-shot ws.once('pong', onPong) — with one pong the on/once timelines are identical (ping@15, pong rearms, ping@30, terminate@45). A ws.on → ws.once mutant ships the whole suite green (verified: mutant 26/26 passed), while in production every browser auto-pongs each ping and only the first pong would rearm alive, so every web-terminal socket would be terminated ~45 s after connecting — a mass disconnect on a fixed cycle. Emit a second pong after the next advance and assert terminate is still not called at t=45 s, then called only after a further 15 s without a pong.
| ws.emit('pong'); | |
| await vi.advanceTimersByTimeAsync(15_000); | |
| expect(ws.terminate).not.toHaveBeenCalled(); | |
| ws.emit('pong'); | |
| await vi.advanceTimersByTimeAsync(15_000); | |
| expect(ws.terminate).not.toHaveBeenCalled(); | |
| ws.emit('pong'); | |
| await vi.advanceTimersByTimeAsync(15_000); | |
| expect(ws.terminate).not.toHaveBeenCalled(); |
Fix witness: the added t=45 s not.toHaveBeenCalled() assertion goes red under the ws.once mutant, which today ships green.
中文说明
[Suggestion] 心跳测试只喂了一个 pong，因此路由里持久的 ws.on('pong', onPong) 订阅（terminal.ts:122）与一次性的 ws.once('pong', onPong) 无法区分——只有一个 pong 时 on/once 的时间线完全相同（ping@15、pong 重置、ping@30、terminate@45）。把 ws.on 突变为 ws.once 后整个测试套件依然全绿（已验证：突变体 26/26 通过），而生产中每个浏览器都会对每个 ping 自动回 pong，只有第一个 pong 会重置 alive，于是每个终端 socket 都会在连接约 45 秒后被终止——按固定周期大面积断连。修复：在下一次推进后再发一个 pong，断言 t=45s 时 terminate 仍未被调用、且只在再无 pong 的下一个 15 秒后才被调用。
修复见证：新增的 t=45s not.toHaveBeenCalled() 断言在 ws.once 突变体下会变红（该突变体目前能全绿通过）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
There was a problem hiding this comment.
Deferred as a non-blocking test-only Suggestion under the PR convergence policy after repeated review rounds. The persistent pong listener remains covered functionally; a second-pong mutation pin can follow separately.
| expect(registry.write('terminal:io', 'x'.repeat(256 * 1024))).toBe( | ||
| 'written', | ||
| ); | ||
| expect(registry.write('terminal:io', 'x')).toBe('backpressure'); |
There was a problem hiding this comment.
[Suggestion] The input-backpressure test never produces the exact-cap state (unacknowledgedInputBytes === 262144) at check time — it checks at 262145, then at 1, then at 262145 — so a >= → > mutant of write()'s cap check (web-terminal-registry.ts:302) ships all 19 tests green (verified: mutant passed; an exact-cap probe fails with expected 'written' to be 'backpressure'). Under the mutant, a write arriving exactly at the cap is accepted, letting one frame of any size pass the 256 KiB steady-state unacknowledged-input cap that the R3-1 fix introduced as counter >= MAX; the exact-cap state is reachable in production (a single 256 KiB write from a zero counter). From a zero counter, assert the boundary exactly: a 256 KiB write returns 'written' and the very next 1-byte write returns 'backpressure'.
Fix witness: the added exact-cap assertion goes red under the >= → > mutant, which today ships green.
中文说明
[Suggestion] 输入背压测试在检查时从未产生“恰好等于上限”的状态（unacknowledgedInputBytes === 262144）——它分别在 262145、1、262145 处检查——因此把 write() 的上限判断（web-terminal-registry.ts:302）从 >= 突变为 > 后全部 19 个测试依然全绿（已验证：突变体通过；一个“恰好上限”探针以 expected 'written' to be 'backpressure' 失败）。在该突变体下，恰好在上限时到达的写入会被接受，从而让任意大小的单帧越过 R3-1 修复以 counter >= MAX 引入的 256 KiB 稳态未确认输入上限；该“恰好上限”状态在生产中可达（从零计数器做一次 256 KiB 写入）。修复：从零计数器出发精确断言边界——一次 256 KiB 写入返回 'written'，紧随其后的 1 字节写入返回 'backpressure'。
修复见证：新增的“恰好上限”断言在 >= → > 突变体下会变红（该突变体目前能全绿通过）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
There was a problem hiding this comment.
Deferred as a non-blocking boundary-test Suggestion under the PR convergence policy after repeated review rounds. Current backpressure behavior and adjacent boundaries remain covered; the exact-cap assertion can follow separately.
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.
23 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- eviction release loop dead code (App.tsx:3071) — already reported (open thread R1-25, comment 3865555110)
- release-retry bound unpinned (TerminalPanel.test.tsx:231) — already reported (open thread R2-9, comment 3865555133)
- release dropped while a CONNECTING socket never opens (TerminalPanel.tsx:283) — already reported (open thread R1-32, comment 3865555119)
- /terminal resolver closure (trust gate + env fallback) untested (server.ts:3017) — already reported (open thread R1-9, comment 3865555104)
- drain/shutdown terminal wiring untested (run-qwen-serve.ts:6846) — already reported (open thread R1-8, comment 3865555093)
- killPtyTree group-kill branch unexercised, pid:1 mocks (registry.test.ts:39) — already reported (open thread R2-15, comment 3865555169)
- dispose() established-session teardown unpinned (registry.test.ts:335) — already reported (open thread R2-19, comment 3865555201)
- terminal bearer-subprotocol encoder unpinned (TerminalPanel.test.tsx:35) — already reported (open thread R2-8, comment 3865555126)
- limit-reached 1013 without error frame (terminal.ts:207) — already reported (open thread R2-10, comment 3865555138)
- established-session cwd guard in release() untested (registry.test.ts:413) — already reported (open thread R2-20, comment 3865555208)
- terminal tab titles reuse ordinals (App.tsx:3170) — already recorded in the round-3 review body deferral list
- reconnect backoff doubling/10 s cap unpinned (TerminalPanel.test.tsx:200) — already recorded in the round-3 review body deferral list
- 4001 dynamic error frame unpinned (terminal.test.ts:710) — already recorded in the round-3 review body deferral list
- replay-cap tests pin size only, not recency/bytes (registry.test.ts:202) — already reported (open thread R2-17, comment 3865555183, and comment 3852506632)
- 'Defaults to all actions' JSDoc falsified by the terminal opt-in (customization.tsx:132) — already recorded in the round-2 review body deferral list
- release-during-pending × create-failure branch untested (terminal.test.ts:361) — already recorded in the round-4 review body deferral list
- create-armed reclaim timer unpinned (registry.test.ts:464) — already reported (open thread, comment 3852506621; author deferred) and the round-3 deferral list
- Unix SHELL passthrough untested (registry.test.ts:94) — already reported (comment 3852506627; author deferred)
- pane visibility (terminalPaneHidden/aria-hidden) unpinned (ArtifactPanel.test.tsx:392) — already recorded in the round-4 review body deferral list
- ResizeObserver resize path unexercised (TerminalPanel.test.tsx:103) — already recorded in the round-3 review body deferral list
- …and 3 more (see the run report)
Not explored to full depth (tool budget reached): chunk 9: could not execute packages/web-shell/client/components/artifacts/ArtifactPanel.test.tsx — the shared worktree has no node_modules and a full monorepo instal….
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/terminal.ts:180 — [review] client-supplied low-entropy terminal ids enable same-workspace attach by guess/collisionpackages/web-shell/client/components/terminal/TerminalPanel.test.tsx:180 — [review] cleanup guard keeping a CONNECTING socket alive for the banked release is unpinnedpackages/web-shell/client/components/terminal/TerminalPanel.test.tsx:168 — [review] 1013 retryability is never asserted — only the non-retryable side is pinnedpackages/web-shell/client/components/artifacts/ArtifactPanel.tsx:613 — [probe] active/cwd props handed to TerminalPanel are unpinned; inverted-active mutant ships green (input silently dead)packages/web-shell/client/components/terminal/TerminalPanel.test.tsx:249 — [probe] connection-contract test never asserts the release param is absent from the main URL
Convergence: round 5 posted 4 inline comment(s), 4 of them reported for the first time; the previous round posted 6 (6 new). Findings keep coming back to the same files: packages/core/src/services/web-terminal-registry.ts (findings in round 4; 3 more now); packages/cli/src/serve/routes/terminal.ts (findings in round 4; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 23 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）：chunk 9：could not execute packages/web-shell/client/components/artifacts/ArtifactPanel.test.tsx — the shared worktree has no node_modules and a full monorepo instal…。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 5 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 5 轮发布了 4 条行内评论，其中 4 条是首次提出；上一轮发布了 6 条（其中 6 条首次提出）。发现反复回到同一批文件：packages/core/src/services/web-terminal-registry.ts（第 4 轮已出过发现，本轮又有 3 条）；packages/cli/src/serve/routes/terminal.ts（第 4 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.2)
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Scope: read terminal.ts (348 lines), web-terminal-registry.ts (437 lines), TerminalPanel.tsx (390 lines), server.ts wiring, acp-http/index.ts drain bypass, and all new test files in full. NOT reviewed: ArtifactPanel.tsx refactor depth, App.tsx beyond terminal tab lifecycle, design docs.
CI disclosure: Test (ubuntu-latest, Node 22.x) — in_progress; Test (macos-latest, Node 22.x), Test (windows-latest, Node 22.x), Integration Tests (CLI, No Sandbox) — SKIPPED. The macOS and Windows matrix jobs did not run against this head. PR author explicitly marks Linux/Windows as untested (
Cross-check against the bot's four Critical findings
C1 — "no backpressure guard on live output fan-out" → refuted
sendOutput (terminal.ts line 31) already checks ws.bufferedAmount > MAX_SOCKET_BUFFERED_BYTES before every ws.send, and on overflow the attached listener calls cleanup() and closes with 1013 — the same pattern the bot's suggested fix prescribes, implemented at 8 MiB rather than the bot's suggested 1 MiB. The bot's probe result (45 MiB accumulated) appears inconsistent with the current code; my best read is the probe ran against an earlier version.
C2 — "error listener attached only at end of happy path" → refuted
ws.on('error', markClosed) is the first statement in onConnection (terminal.ts line 120), before any await and before all early-return branches. There is no window where the socket has zero error listeners, so Receiver errors (maxPayload exceeded, RSV violations) cannot reach uncaughtException.
C3 — "post-attach input has no byte cap" → partially refuted; one question
registry.write() returns 'backpressure' once unacknowledgedInputBytes + incoming >= 256 KB, and the route handler closes with 1013 on that result. The mechanism works by treating PTY output bytes as implicit acknowledgment of consumed input. That means a foreground program that produces no output (e.g., sleep 3600) will disconnect the client after ~256 KB of stdin — the ack never arrives so the budget never replenishes. Is that the intended UX for programs that silently consume stdin? If not, a wall-clock drain of the unacknowledged counter, or a separate per-message size cap, would be more predictable. Not a crash risk; the question is whether 256 KB is the right interaction model.
C4 — "stale reclaim timer can kill a future session reusing the same terminalId" → refuted
The detach closure returned by addOutputListener opens with if (this.sessions.get(terminalId) !== session) return (web-terminal-registry.ts line 301). After release() removes a session from the map, every subsequent detach call exits at this guard and never reaches scheduleReclaim. The timer-kills-new-session sequence the bot describes requires that guard to be absent; it is present.
One open question — drain window
createTerminalWsHandler registers with bypassPrimaryDrain: true, which lets new WebSocket upgrades through even while the primary workspace is draining. releaseWebTerminalsForWorkspace(cwd) is called once at drain time and releases only sessions that exist at that moment. A terminal created after that call (new reconnect or new tab opened concurrently) is not covered. The PTY keeps running in the workspace cwd/env; the next client message re-validates trust via resolveWorkspace(selector), so once the workspace is fully gone the connection closes with 4002. The 15-minute idle reclaim is the final backstop. This is probably acceptable for the reconnect use case the bypass was designed for, but it is worth confirming that "bypass = reconnect only, never new creation" is the intent and that the intent matches the implementation.
No confirmed blockers. Withholding approval pending CI green on the major test jobs (ubuntu in-progress, macOS/Windows/Integration SKIPPED).
Reviewed with AI assistance.
| function sendOutput(ws: WebSocket, text: string): boolean { | ||
| if ( | ||
| ws.readyState !== ws.OPEN || | ||
| ws.bufferedAmount > MAX_SOCKET_BUFFERED_BYTES |
There was a problem hiding this comment.
C1 refutation: backpressure guard is present here. sendOutput returns false when ws.bufferedAmount > MAX_SOCKET_BUFFERED_BYTES (8 MiB); the attached output listener calls cleanup() and closes with 1013 on that result. The bot's finding and probe appear to target an earlier version of the code.
| closed = true; | ||
| stopHeartbeat(); | ||
| }; | ||
| ws.on('error', markClosed); |
There was a problem hiding this comment.
C2 refutation: this is the first statement in onConnection, before any await and before all early-return paths. The socket has an error listener from the start; Receiver errors cannot reach uncaughtException.
| session.outputListeners.add(listener); | ||
| this.clearReclaim(session); | ||
| return () => { | ||
| if (this.sessions.get(terminalId) !== session) return; |
There was a problem hiding this comment.
C4 refutation: the identity check this.sessions.get(terminalId) !== session is already here. After release() removes the session from the map, any detach call exits at this guard and never reaches scheduleReclaim — the stale-timer-kills-new-session sequence the bot describes cannot occur.
|
@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: 72 passed · 3 failed · 75 total Flakiness gate: ✅ 7 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：72 通过 · 3 失败 · 75 总计 抖动门：✅ 7 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9984 — feat(web-shell): add opt-in interactive browser terminalVerdict: 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：72 通过 · 3 失败 · 75 总计（3 条失败均为沿用发现 F-2，详见下表与 Findings）
Previous-finding status table (round 1 → new head)
Scope selection
A/B load-bearing proof (11/11,
|
| Cell | Build | Oracle | Result |
|---|---|---|---|
| BASE-TERM-CTRL | base | WS upgrade /acp |
opens (upgrade stack alive) |
| BASE-CAP | base | GET /capabilities |
no web_terminal (121 features), as predicted |
| BASE-CAP-CTRL | base | same envelope | sane (session_create present) |
| BASE-TERM | base | WS upgrade /terminal |
rejected (unknown path), as predicted |
| HEAD-TERM / -IO / -REL / -KILL | head | open + PTY echo + release | opens; real PTY echoes (echo $$ oracle); close 4004; shell pid dead |
| HEAD-CAP | head | GET /capabilities |
web_terminal present (122 features = base 121 + 1) |
| OFF-CAP | head + QWEN_SERVE_ACP_HTTP=0 |
GET /capabilities |
no web_terminal (116 features) |
| OFF-TERM | head + off | WS /terminal |
not upgraded (HTTP 404 — no upgrade listener mounted) |
Note: an early HEAD-CAP probe fired inside the bootstrap window and observed the feature absent (116 features) while /terminal already upgraded seconds later — a live capture of finding F-2; the cells above gate capability queries on the deferred runtime being mounted (WS-upgrade readiness / envelope stabilization), and the bootstrap window is measured by its own probe below.
E2E on the real daemon (head dist; 34/34 + 10/10 + 9/9 + 5/5)
One real qwen serve (head dist, SHELL=/bin/bash) + real ws/raw-socket clients + real node-pty sessions; plus an in-process real-registry cell set and a two-workspace daemon probe. Raw logs in logs/.
Lifecycle (02-e2e-terminal-lifecycle.png, 34/34):
| Cells | Asserted behavior | Result |
|---|---|---|
| A1–A5 | no token → 401; wrong token → 401; cross-Origin → 403; forged Host → 403; good raw upgrade → 101 | PASS |
| B1–B2 | missing terminalId / unregistered cwd → error control + close 4002 |
PASS |
| C1–C4 | open; PTY cwd = registered workspace; TERM=xterm-256color set, NO_COLOR/npm_config_prefix stripped; resize control reaches PTY (stty size 24 80 → 40 132) |
PASS |
| D1–D3 | abrupt disconnect keeps PTY; reconnect replays scrollback exactly once (replay count == original count); still interactive; same shell pid across reconnect | PASS |
| E1–E4 | exit 3 → exit control {exitCode:3} + close 4000; reconnect to exited session replays exit state and releases it (new round-2 behavior); id reusable afterwards |
PASS |
| F1–F6 | F-1 re-measure: bg child in its own pgid + nested grandchild spawned; release → 4004; shell, bg child, grandchild all dead | PASS |
| G1–G4 | detached session keeps PTY; ?release=1 releases (4004, pid dead); id reusable |
PASS |
| H1–H3 | 8 concurrent terminals; 9th → close 1013 retryable; slot reusable after release | PASS |
| I1 | malformed + out-of-range controls dropped silently; session alive; size unchanged | PASS |
| J1–J2 | release frame buffered during creation → 4004; id reusable | PASS |
Round-2 new semantics (03-e2e-new-behavior.png, 10/10):
| Cells | Asserted behavior | Result |
|---|---|---|
| K1–K3 | >256 KB unacknowledged input → error control + close 1013; session not released (shell pid alive — changed from round 1); ?release=1 then cleans it up (4004 + pid dead) |
PASS |
| M1–M3 | 8 sessions exit 0 (close 4000, retained) → 8 new sessions open simultaneously (exited sessions don't count against the cap); a retained exited session still replays its exit state |
PASS |
| K6–K7 | live heartbeat: server ping within 17 s; session interactive afterwards | PASS |
| K8–K9 | boundary: setsid escape (new session, reparented) survives release — same semantics as closing a real terminal; documented, not a defect |
PASS (as encoded) |
Observation (not a finding): after a backpressure close the session sits exactly at the 256 KB unacknowledged cap, so reconnect input is rejected with 1013 until the shell's own output (e.g. processing the buffered backlog) drains the counter. Self-healing; requires a >256 KB paste with no output to reach.
Workspace drain / dispose, real registry in-process (9/9): releaseWorkspace is cwd-scoped, removes only that workspace's session, exit listeners get {exitCode:143, signal:15}, drained PTY dead / survivor alive; dispose() kills the remainder; post-dispose create refused.
Cross-workspace live probe (04-m4-cross-workspace.png, 5/5): daemon booted with two registered workspaces (A primary, B secondary). Terminal created in A: attach from B → 4002 Terminal workspace mismatch; ?release=1 from B → PTY survives (guard holds); same-workspace reattach interactive; same-workspace release kills (4004 + pid dead). See finding 2 below for the close-code honesty wrinkle observed here.
Reviewer Test Plan walk
- Entry visibility — protocol/unit level:
web_terminalin/capabilities(HEAD-CAP), client gating covered by App/ArtifactPanel/TerminalPanel suites (630 tests). Real-browser rendering not exercised (Not covered). - Two independent tabs — H1–H3 (8 concurrent, independent sessions).
- Tab-switch survival — D1–D3 (detach/reattach keeps process and screen; same pid).
- Resize — C4 (
stty sizeoracle). - Reconnect replay once — D3 (count-equal oracle) + E3 (exit state replayed exactly once).
- Close releases PTY, new PTY afterwards — F3–F6, G2–G4, E4, J2.
- Embedded/hidden entry — OFF-CAP (capability absent when ACP HTTP off) + gating unit tests; action-omission path via customization type + App tests.
Findings
1. (Suggestion, low — carried over, stands) F-2: bootstrap /capabilities under-reports web_terminal for ~1.2 s after "listening"
Re-measured at the new head (05-f2-bootstrap-window.png, 3 runs): 36/38/37 bootstrap responses lack web_terminal (from +6…+1165 ms after "listening", 116 features); first present at +1178/+1184/+1196 ms (122 features). Root cause unchanged: currentServeFeaturesForRunQwenServe() passes clientMcpOverWsEnabled/cdpTunnelOverWsEnabled/browserAutomationMcpAvailable but still omits acpHttpEnabled, while the runtime path (server/serve-features.ts) passes acpHttpEnabled: currentAcpHttpEnabled — despite the bootstrap function's own comment promising parity "so the bootstrap window doesn't briefly under-report them". A Web Shell fetching capabilities in that window hides the Terminal entry until its next fetch. The function was even extended since round 1 (for currentSessionSchedulingAvailable, via the base merge) without closing this gap.
Minimal suggested fix (measured against the same probe)
In currentServeFeaturesForRunQwenServe (packages/cli/src/serve/run-qwen-serve.ts), add to the toggles object:
acpHttpEnabled: resolveAcpHttpEnabled(env),(resolveAcpHttpEnabled is already imported; env is the function's existing parameter.)
Measured, applied in a scratch copy + rebuilt dist, three results:
- Hostile fixture clean: f2-probe 3 runs → 0 under-reporting responses on every run; capability present from the first response (+8/+10/+33 ms; bootstrap envelope 117 features).
- Benign fixture identical: with
QWEN_SERVE_ACP_HTTP=0the bootstrap envelope still never advertisesweb_terminal(7 samples over ~1.3 s, 116→120 features, absent throughout) — the toggle-off semantics are preserved. - Affected suite unchanged:
run-qwen-serve.test.ts329/329 both with and without the patch (typecheck also passes with it). Note the suite is green on both sides — it pins nothing along the bootstrap-feature axis — so the fix should land together with a fixture asserting the bootstrap envelope containsweb_terminalwhenever ACP HTTP is on.
2. (Suggestion, nit — new) ?release=1 reports 4004 Terminal released even when the release is refused
The release handshake ignores registry.release()'s boolean: with the terminal owned by workspace A, a ?terminalId=…&cwd=B&release=1 request from workspace B closes 4004 Terminal released while the PTY (correctly) survives — measured live in probe cell P3 (close=4004, pid alive afterwards). The protective outcome is right (the ownership guard holds; P3/P5), and only the close code misreports to a client that is either buggy or probing another workspace's terminals on a multi-workspace daemon; the in-connection release branch has the same shape. Repro: m4-probe.mjs cells P1–P5. No test pins the refused-release close code (unpinned axis); suggested direction: honor the boolean and close a distinct code (e.g. 4002) when release() returns false — not applied; a fixture asserting the refused-release code would pin it.
3. (Completeness — carried over, not merge conditions) M4 and M8 survivors
Mutation matrix below: M4 (established-session cross-workspace release guard) and M8 (resize ≤1000 cap) still survive their suites. Both behaviors are empirically correct (M4 proven live above; M8 exercised in cell I1) — these are tests to write, recorded so they are not silently dropped:
- M4 test:
release('t', otherWorkspaceCwd)on an established session →false, session survives. - M8 test:
resizecontrol withcols:1001→registry.resizenot called (and size unchanged).
Mutation matrix (06-mutation-matrix.png, logs/mutation-matrix.txt)
Baselines green: registry 21/21, route 26/26. Positive controls killed in the same file as each mutant set (C1: exit-code 143→144 killed 21/21→20/21; C2: close 4004→4005 killed 26/26→23/26).
| Mutation | Guard | Suite | Result | Classification |
|---|---|---|---|---|
| M4 (carried) | release() cross-workspace check, established session |
registry 21 | SURVIVED 21/21 | coverage gap — guard proven correct live (probe P1–P5); test to write |
| N1 | killPtyTree ps descendant/session walk (F-1 fix) |
registry | KILLED (1 red) | pinned |
| N2 | cap counts only LIVE sessions | registry | KILLED | pinned |
| N3 | exit listeners notified from a snapshot copy | registry | KILLED | pinned |
| N4 | unack-input decrement by output bytes | registry | KILLED | pinned |
| C1 | CONTROL: drain exit code 143→144 | registry | KILLED | control |
| M8 (carried) | resize ≤1000 dimension cap | route 26 | SURVIVED 26/26 | coverage gap — behavior live-exercised (I1); unit pin missing |
| N5 | release after exit-state replay | route | KILLED (2 red) | pinned |
| N6 | 15 s WS heartbeat | route | KILLED | pinned |
| N7 | input-path workspace re-check | route | KILLED | pinned |
| C2 | CONTROL: close 4004→4005 | route | KILLED (3 red) | control |
Every guard the round-2 commits introduced is pinned by a test; the two survivors are both carried-over completeness items, and neither survivor's behavior is defective.
Targeted gates (project's own tests, all green)
| Suite | Result |
|---|---|
packages/core web-terminal-registry.test.ts |
21/21 |
packages/cli routes/terminal.test.ts |
26/26 |
packages/cli server.test.ts |
1102/1102 |
packages/cli acp-http/workspace-qualified-acp.test.ts |
50/50 (incl. the 2 drain-bypass tests) |
packages/web-shell App.test.tsx |
555/555 |
packages/web-shell ArtifactPanel.test.tsx |
60/60 |
packages/web-shell TerminalPanel.test.tsx |
15/15 |
Total 1829/1829 across the 7 test files touched by the PR's effective diff. Additionally run-qwen-serve.test.ts 329/329 (not in the PR diff — it arrived via the base merge — but run as the affected suite for the measured F-2 fix, with and without the patch). Flakiness (5 identical rounds) is run by the workflow's own lane; each file above ran once in this gate.
Not covered
- Per-commit attribution: depth-2 checkout; metadata lists 17 commits,
git rev-list HEAD^1..HEAD^2returns 1 across the shallow graft. The previous headfdb8fbe7is present in the object store, so the aggregate delta since round 1 was diffed (and the round-2 semantics were each exercised), but commits 13/14/16/17 cannot be separated individually. Verified aggregateHEAD^1..HEAD. - Real-browser client: xterm rendering, FitAddon refit-on-activate, inactive-tab key gating, and the new
buildWsUrl(baseUrl)derivation were not exercised in a browser; client behavior is covered by the 630 unit tests and by protocol-level E2E of the exact wire contract the client implements. (Shape note, untested topology: abaseUrlcarrying a path prefix would produce a WS path the upgrade matcher — exact raw/terminal— rejects; standalone origin-only deployments are unaffected.) - Windows/macOS: Linux container only; the Windows
taskkill /f /tbranch ofkillPtyTreeremains untested here (relevant to F-1's platform contrast). - 15-minute idle reclaim constant: not observed live; scheduling logic pinned by registry unit tests (fake timers).
- Untrusted-workspace negative path: trust check verified by reading (
runtime.primary ? isPrimaryWorkspaceTrusted() : runtime.trusted); both live workspaces in the M4 probe were trusted, so the untrusted rejection was not driven live. - Repo-wide gates: not run; targeted gates only.
- Capture
06-mutation-matrix.pngis rendered from the run's saved log (logs/mutation-matrix.txt, same artifact dir) rather than a re-execution; all other captures are live re-runs of their harnesses.
Methodology
Environment: CI verify container (node:22-bookworm), merge-ref checkout (HEAD = merge, HEAD^1 = base tip 7357136dd1, HEAD^2 = verified head 7dca447c5b). Harnesses live in this artifact dir (lib.mjs, ab-harness.mjs, terminal-e2e.mjs, e2e2.mjs, registry-proc.mjs, m4-probe.mjs, f2-probe.mjs, mutation-matrix.mjs) and boot the real built daemon (packages/cli/dist/index.js serve --port 0 --token … --require-auth --workspace <scratch> [--workspace B] [--no-web], SHELL=/bin/bash) driving it over loopback with the ws client and raw sockets (Host/Origin forgery); PTY liveness asserted via kill(pid, 0) on pids the shells themselves report (echo $$, echo $!, inner-bash $$). Base control: HEAD^1 worktree, packages built from base source with generated files reproduced, @qwen-code links realpath-asserted into the base tree (the assertion caught a broken-link fallback to head code before any base cell ran), base dist grep-verified free of the feature; daemon dep closure identical because the PR's lockfile delta is browser-only @xterm/*. The measured F-2 fix was applied to the working tree, validated (probe ×3, toggle-off probe, run-qwen-serve.test.ts both ways), then reverted (git checkout + rebuild); the tree was confirmed clean and the pristine A/B re-run green afterwards. Mutations were applied to source, run against the file's own vitest suite, and restored per-mutation via git checkout (final git status clean). Raw per-harness logs in logs/; captures in evidence/. Assertion accounting: fail counts only unexpected outcomes — the 3 fails are the standing finding F-2 (its probe asserts the reviewer-natural property "advertised from listening"); base-arm predicted absences/rejections and the setsid-boundary survival are encoded expectations and count as passes.
Flakiness gate log
rounds=5 files=7 skipped=0
file packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/acp-http/workspace-qualified-acp.test.ts
file packages/cli/src/serve/routes/terminal.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/routes/terminal.test.ts
file packages/cli/src/serve/server.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server.test.ts
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/App.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/App.test.tsx
file packages/web-shell/client/components/artifacts/ArtifactPanel.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/artifacts/ArtifactPanel.test.tsx
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
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts: PPPPP
packages/cli/src/serve/routes/terminal.test.ts: PPPPP
packages/cli/src/serve/server.test.ts: PPPPP
packages/core/src/services/web-terminal-registry.test.ts: PPPPP
packages/web-shell/client/App.test.tsx: PPPPP
packages/web-shell/client/components/artifacts/ArtifactPanel.test.tsx: PPPPP
packages/web-shell/client/components/terminal/TerminalPanel.test.tsx: PPPPP
verdict: pass
summary: 7 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/routes/terminal.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 1 · packages/core/src/services/web-terminal-registry.test.ts: P (exit 0)
round 1 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/artifacts/ArtifactPanel.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/terminal/TerminalPanel.test.tsx: P (exit 0)
round 2 · packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/routes/terminal.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/server.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/App.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/components/artifacts/ArtifactPanel.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/components/terminal/TerminalPanel.test.tsx: P (exit 0)
round 3 · packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/routes/terminal.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/server.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/App.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/components/artifacts/ArtifactPanel.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/components/terminal/TerminalPanel.test.tsx: P (exit 0)
round 4 · packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/routes/terminal.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/server.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/App.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/components/artifacts/ArtifactPanel.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/components/terminal/TerminalPanel.test.tsx: P (exit 0)
round 5 · packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts: P (exit 0)
round 5 · packages/cli/src/serve/routes/terminal.test.ts: P (exit 0)
round 5 · packages/cli/src/serve/server.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/App.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/components/artifacts/ArtifactPanel.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/components/terminal/TerminalPanel.test.tsx: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
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.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- create-pending input overflow (>64 KiB) kills the connection and the just-created session for a single large paste racing the create window (packages/cli/src/serve/routes/terminal.ts:166) — already reported (open thread R2-13,…
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/core/src/services/web-terminal-registry.test.ts:293 — [probe] killPtyTree session-membership branch unpinned by the kill-tree test fixture (D6-1)packages/core/src/services/web-terminal-registry.ts:208 — [probe] output credit bounds the accounting, not the bytes actually queued in the PTY (D6-2)packages/core/src/services/web-terminal-registry.test.ts:308 — [probe] kill fallback chain and if (killed) return; gate unpinned by any test (D6-3)packages/cli/src/serve/run-qwen-serve.ts:2162 — [review] bootstrap /capabilities omits acpHttpEnabled, under-reporting web_terminal for ~1.4 s after boot (D6-4)
Convergence: round 6 posted 4 inline comment(s), 4 of them reported for the first time; the previous round posted 4 (4 new). Findings keep coming back to the same files: packages/core/src/services/web-terminal-registry.ts (findings in round 5; 4 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 6 轮发布了 4 条行内评论，其中 4 条是首次提出；上一轮发布了 4 条（其中 4 条首次提出）。发现反复回到同一批文件：packages/core/src/services/web-terminal-registry.ts（第 5 轮已出过发现，本轮又有 4 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.2)
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Independent pass at 7dca447, focused on new ground (not repeating the ~55 open bot threads). Local verification on this head: registry suite 21 pass, terminal route suite 26 pass, tsc --noEmit clean in core and cli. The WS auth surface looks solid — Host allowlist, Origin rejection, and bearer/loopback auth all run before the route handoff, and cross-workspace attach/release is blocked by the workspaceCwd checks. Four new findings inline, two of them worth addressing before merge: (1) the unacknowledged-input budget can wedge a healthy session into a permanent 1013 close/reconnect loop; (2) /terminal bypasses the workspace-drain gate, so new PTYs can be spawned into a draining workspace after the one-shot release has already run.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Re-reviewed at 4960873. All four of my earlier findings are closed out, verified in source and by local test runs on this head:
- Input-budget wedges (c36212e): zero-debt first-frame admission + recovery-control pass-through — both lockout shapes are gone; the bounded-admission accounting is the right call given node-pty has no write-completion callback.
- Drain-bypass concern: withdrawn —
activeRuntime(workspace-registry.ts:318) only resolvesactiveentries, andbeginDrainflips the state todraining, so the terminal resolver returns undefined from drain start;bypassPrimaryDraincannot create a PTY in a draining runtime. - Terminal id collisions (34a5b0a):
crypto.randomUUID()now, counter ref removed. - Post-revocation output (34a5b0a): the shared
ensureWorkspaceAvailableguard covers input, resize, live output, and snapshot replay.
Local verification: registry suite 23/23, terminal route suite 27/27, tsc --noEmit clean in packages/cli. My pass is clear; the open bot-round threads are outside this approval.
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Re-reviewed at new commits c36212ef and 34a5b0a1. Prior findings from round 1 (7dca447c) resolved:
Prior findings — status at new head:
- C1 (bot: no buffered-amount check): Still refuted —
sendOutputgate onws.bufferedAmount > 8 MBis unchanged. - C2 (bot: no error handler): Still refuted —
ws.on('error', markClosed)remains the first statement in the connection handler. - C3 (bot: write drops input under backpressure) + UX question: Fixed by
c36212ef—\x03\x04\x1a\x1cnow bypass the budget unconditionally and don't accrue bytes; andunacknowledgedInputBytes !== 0guard allows a first oversized frame through on an idle session (test:accepts one complete oversized input frame on idle). Both lockout shapes gone. - C4 (bot: reclaim timer race): Still refuted — identity check at registry line 301 is unchanged.
- Drain window question: Resolved —
activeRuntime(workspace-registry.ts:318) only resolvesactiveentries;beginDrainflips the workspace state todrainingbefore any new PTY can be created, soresolveWorkspacereturnsundefinedat that point. ThebypassPrimaryDrainpath cannot race a draining workspace into existence.
New code checked (c36212ef, 34a5b0a1):
killPtyTreeportability (web-terminal-registry.ts:78): Linuxsid=, macOStdev=, fallback topid=,ppid=only (plus unconditionalSIGKILL(-pty.pid)) on other platforms and onpserror.maxBuffer: 8 MBandtimeout: 2_000guard thespawnSynccall.rootScopeguards (undefined/'0'/'??') are correct — prevents over-broad scope-based kills when the PTY process has no controlling terminal or is in kernel session 0. New test covers both the scope-based kill path (process 45, same session, orphaned from parent chain) and theps-failure fallback (ENOBUFS →-SIGKILLonly). ✅ensureWorkspaceAvailable()in output listener (terminal.ts:300): closes the post-revocation output leak — data from workspace A's PTY can no longer reach a WebSocket that has since become bound to a different workspace. Thecleanup()+ Set-delete path from within the iterator is safe per ES6 Set iteration semantics. ✅ensureWorkspaceAvailable()at connection setup (terminal.ts:329): closes the TOCTOU window betweenws.on('error', cleanup)being registered and the snapshot being sent. ✅crypto.randomUUID()for terminal IDs (App.tsx): removes theDate.now():countersequential ID, making terminal IDs unpredictable.crypto.randomUUID()is available in all current browser targets. ✅
CI (head 72d611b5): ubuntu unit tests in-progress at time of review · Desktop Shell ubuntu + windows ✅ · macOS unit tests SKIPPED · Windows unit tests SKIPPED · Integration Tests (CLI) SKIPPED.
No blocking findings in round 2.
Reviewed with AI assistance.
|
Released in v0.22.3. |
6 participants
What this PR does
Adds a manually managed interactive terminal to the Web Shell right panel. The standalone Web Shell opts into the Terminal action, while the client also requires the daemon's
web_terminalcapability before showing it; this keeps separately released frontend and backend versions compatible, and embedded hosts that do not add the action see no new entry.Each terminal tab owns an independent workspace-scoped PTY session. Switching tabs, hiding the panel, or reconnecting keeps the session alive and replays bounded scrollback plus exit state; explicitly closing the terminal tab releases and kills that PTY. Resize and release use control frames, while terminal input and output remain binary, preserving raw terminal data.
The daemon resolves every terminal against the live trusted workspace runtime and uses that runtime's cwd and effective environment. The WebSocket route shares the daemon's Host, Origin, authentication, and workspace trust checks. Sessions are capped globally, detached sessions are reclaimed after 15 minutes, workspace drain releases its terminals, and terminal startup removes the inherited npm prefix that otherwise makes
nvmprint a warning in development launches.Why it's needed
The Web Shell previously had no manually managed interactive shell tied to the active workspace. Users need to run long-lived commands, interactive CLIs, and development processes without opening an external terminal, while retaining output and process state when they switch panels or briefly disconnect.
Reviewer Test Plan
How to verify
web_terminalcapability.Evidence (Before & After)
Before: Web Shell had no interactive terminal surface. After: the right panel can host multiple independent terminal tabs backed by workspace PTYs.
Tested on
Environment (optional)
Verified with the development daemon and Vite Web Shell on macOS. Targeted registry, WebSocket route, capability, panel, and client lifecycle tests pass; full workspace type checking and build also pass.
Risk & Scope
Linked Issues
None.
中文说明
本 PR 做什么
在 Web Shell 右侧面板增加一个由用户手动管理的交互式终端。独立运行的 Web Shell 会显式加入 Terminal action，同时客户端还要求 daemon 返回
web_terminalcapability 后才展示入口；这样前后端分开发布时仍可保持兼容，未加入该 action 的嵌入式宿主不会出现新入口。每个终端页签拥有独立、按工作区隔离的 PTY 会话。切换页签、隐藏面板或重新连接时会话继续存活，并重放有界滚动内容及退出状态；用户明确关闭终端页签时则释放并终止对应 PTY。resize 和 release 使用控制帧，终端输入输出保持二进制传输，从而保留原始终端数据。
daemon 会把每个终端解析到当前存活且受信任的工作区运行时，并使用该运行时的 cwd 和生效环境。WebSocket 路由复用 daemon 的 Host、Origin、认证和工作区信任校验。终端会话有全局并发上限，脱离连接 15 分钟后自动回收，工作区排空时释放其全部终端；启动终端时还会移除继承的 npm prefix，避免开发启动链下
nvm输出冲突警告。为什么需要
Web Shell 此前没有绑定到当前工作区、由用户手动管理的交互式 Shell。用户需要在不打开外部终端的情况下运行长任务、交互式 CLI 和开发进程，同时在切换面板或短暂断连后保留输出与进程状态。
评审者验证计划
如何验证
web_terminalcapability 的 daemon，确认两种情况下入口都不会展示。前后对比证据
改动前：Web Shell 没有交互式终端界面。改动后：右侧面板可承载多个由工作区 PTY 支撑的独立终端页签。截图见上方。
测试环境
环境（可选）
已在 macOS 上使用开发 daemon 和 Vite Web Shell 验证。针对 registry、WebSocket 路由、capability、面板和客户端生命周期的测试均通过；全仓类型检查和构建也通过。
风险与范围
关联 Issue
无。