fix(web-shell): report each connection error once to stop the inline onError re-render loop - #10454
Conversation
… re-render loop While a connection error persists (e.g. the daemon is unreachable), the error-notification effect re-fires whenever the onError callback identity changes. Hosts such as the VS Code embedded app pass an inline onError and update their own state when it fires, so every notification triggers a host re-render that hands the effect a fresh callback identity — re-notifying the same persistent error forever (#10406). Track the last reported connection.error value in a ref and notify only when the value changes, resetting the tracker once the connection recovers. This guards every inline-callback consumer, not just memoized hosts. Fixes #10406 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Thanks for the PR — re-run after the review-round fixes landed. Template looks good ✓ — all sections present, including the bilingual translation. Problem: observed bug, well evidenced. #10406 is still open and documents the loop with concrete code references (the persistent Direction: aligned. The issue itself recommends the app-side guard because it covers every inline-callback host, not just memoized ones; this implements that recommendation. The reference CHANGELOG has no direct entry for this loop, but the area (embedded IDE error notification) is clearly relevant. Size: not applicable — Approach: scope is exactly right, and stayed right through the six follow-up commits — those are precisely the round-1/round-2 review fixes (the stamp-ordering critical, comment/JSDoc wording, the EmbeddedApp mock brought in line with the new dedup semantics) plus one merge of the base branch. No scope creep. The two round-3 suggestions the author declined are legitimately out of scope: one points at a dependency entry introduced by the base merge itself, the other is test-mirror maintainability. Note this still targets the unmerged #9811 branch following the #10431/#10419 stacking pattern, so its merge path rides on #9811. Risk: no elevated risk signals (no high-risk-path matches). Moving on to code review. 🔍 中文说明感谢贡献!—— 本轮为评审意见修复落盘后的重新运行。 模板完整 ✓ —— 各节齐全,含中文对照。 问题:已观测到的 bug,证据充分。#10406 仍处于打开状态,给出了具体代码引用(持久的 方向:对齐。issue 本身建议取 App 侧防护,因为它覆盖所有内联回调宿主,而不只是已记忆化的宿主;本 PR 正是落实该建议。参考 CHANGELOG 没有与此循环直接对应的条目,但该领域(内嵌 IDE 错误通知)显然相关。 规模:不适用 —— 方案:范围恰好,且在后续六个提交中保持恰好——这些提交正是第 1/2 轮评审要求的修复(stamp 顺序的 Critical、注释/JSDoc 措辞、EmbeddedApp mock 与新的去重语义对齐),外加一次 base 分支合并。无范围蔓延。作者婉拒的两条第 3 轮建议确属范围外:一条指向 base 合并本身引入的依赖项,另一条是测试镜像的可维护性问题。注意本 PR 目标仍是未合并的 #9811 分支,沿用 #10431/#10419 的堆叠模式,合并路径依附于 #9811。 风险:无升级风险信号(未命中高风险路径)。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-run: re-reviewed the full diff at the new head (six commits past my previous pass). Independent baseline unchanged: the loop can only be broken by decoupling delivery from callback identity — either dedupe on the reported value (a ref tracking the last reported What I verified at the reviewed commit on top of the earlier pass:
No blockers, no convention violations, no drive-by changes. Testing evidence
Sandboxed verification would settle this: 中文说明代码审查重新运行:在新的头部提交上(比我上一轮多六个提交)重新审查了完整 diff。独立基线不变:打破循环只能把通知与回调标识解耦——要么按上报值去重(ref 记录上次上报的 在上一轮基础上,于 reviewed commit 上额外核实:
无阻塞项、无规范违例、无顺手改动。 测试证据
沙箱验证可以补齐这一点: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — minimal fix that matches my independent proposal; the round-1 critical (stamp ordering) is correctly fixed and pinned by a dedicated regression test; the one reservation remains evidentiary, not in the code. Stepping back on the re-run: every one of the six commits since my last pass earns its place — four implement exactly what review rounds 1–2 asked for (the late-attach stamp-ordering fix, the comment/JSDoc wording, the EmbeddedApp mock brought in line with the new semantics, and its follow-up bail-out), one is the requested docs wording, one keeps the branch current with its base. No scope creep; the only artifacts beyond the fix (the MessageList one-liner and the package-lock delta) are both verified merge byproducts. The fix itself still reads as the obvious fix: a ref-dedupe in the same "notify once" style this component already uses ( The honest caveat is unchanged: repo CI structurally never runs on this base branch, so the new tests — the loop reproduction is reported red before the fix — have only run on the author's machine. I'm approving anyway, for the same reasons as last time: the author is a collaborator with write access, the guard logic is statically verified across every error transition at the reviewed commit, the blast radius of being wrong is bounded (the loop persists as today, or at worst a missed notice in an edge transition — no data path involved), and this branch only reaches users through #9811, where the combined tree meets full CI. Approving pinned to the reviewed commit. That approval supersedes my earlier one on the original head and the stale changes-requested the test-plan run left behind, whose critical finding is fixed on this head. Merge-path reminder unchanged: this is a stacked PR — it lands after #9811 (or gets retargeted if that branch moves), same pattern as #10431/#10419. The base has advanced another ~20 commits since this branch merged it in at 08:11 UTC today, so expect one more sync before merge; nothing here is mergeable today regardless while #9811 is still under review. 中文说明重新运行后回头看:上一轮之后的六个提交每一个都有存在理由——四个恰好落实第 1–2 轮评审的要求(迟到挂载的 stamp 顺序修复、注释/JSDoc 措辞、EmbeddedApp mock 与新语义对齐、及其后续的提前退出修正),一个是所要求的文档措辞,一个让分支跟上 base。无范围蔓延;修复之外仅有的两处产物(MessageList 的一行与 package-lock 差异)都已核实为合并副产品。修复本身读起来依然是显而易见的修复:与组件中已有的"只通知一次"风格( 诚实的保留意见不变:仓库 CI 结构性地不会在此 base 分支上运行,因此新测试——循环复现用例据称修复前为红——只在作者机器上跑过。我仍然批准,理由与上次相同:作者是有写权限的 collaborator,防护逻辑在 reviewed commit 上对每种错误转换都经过静态核实,出错的影响半径有限(循环照旧持续,或至多在某个边界转换中漏掉一次通知——不涉及数据路径),且该分支只能经由 #9811 到达用户,届时合并后的树会接受完整 CI。想在那之前获得独立证据的维护者,仍可用 将批准固定到本次审查的提交。该批准取代我早前在原始头部上的批准,以及 test-plan 运行留下的过时 changes-requested——其 Critical 发现已在本头部修复。合并路径提醒不变:这是堆叠 PR——在 #9811 之后合入(若该分支移动则改 target),与 #10431/#10419 同一模式。自今天 08:11 UTC 本分支合入 base 以来,base 又前进了约 20 个提交,合并前预计还需一次同步;无论如何,#9811 仍在评审中,本 PR 目前不可合入。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): src/webview/EmbeddedApp.test.tsx — no such file or directory; Tests 553 passed — this review observed 4942, 480 passed; Tests 11 passed — this review observed 4942, 480 passed.
中文说明
Test Plan(非阻断):src/webview/EmbeddedApp.test.tsx — no such file or directory; Tests 553 passed — this review observed 4942, 480 passed; Tests 11 passed — this review observed 4942, 480 passed。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Stamping lastReportedConnectionErrorRef before delivery meant a host that attaches onError after a persistent connection error appeared never received it: the no-op delivery already marked the error as reported. Guard on the handler first and add a regression test covering the late-attach case (red when the guard is removed). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…wording Describe the reported-once-per-distinct-error semantics, the reset on recovery, and that replacing the handler mid-error does not re-deliver. Reword the effect and test comments to describe the host class instead of naming the VS Code embedded app, which passes a useCallback handler. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The WebShellWithProviders mock re-notified on every onError identity change, mirroring the loop App.tsx can no longer produce. Rewrite it to report each distinct error value once (resetting on recovery), keep the loop guard as a regression tripwire, exercise it with a changing callback identity plus a post-delivery effect re-run, and refresh the handleShellError comment that still cited the old loop as the memoization reason. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
CapturedProps is an unknown index signature, so the destructured onError needs the same cast the previous mock applied inline to stay callable under tsc. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Test Plan (not a blocker): src/webview/EmbeddedApp.test.tsx — no such file or directory; Tests 553 passed — this review observed 480, 4943 passed; Tests 11 passed — this review observed 480, 4943 passed.
中文说明
已审查——无阻断问题。 建议见行内评论。
Test Plan(非阻断):src/webview/EmbeddedApp.test.tsx — no such file or directory; Tests 553 passed — this review observed 480, 4943 passed; Tests 11 passed — this review observed 480, 4943 passed。
— qwen3.8-max via Qwen Code /review (v0.22.3)
…or exists The mirrored dedup effect stamped lastReportedError and counted a notification even when no handler was attached, while App.tsx returns before stamping on that path. Add the same early return so a handler attached mid-error still receives the persistent error, and pin the no-handler no-stamp behavior with a test that fails if the guard is removed. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…nnection-error-loop Keep the base branch's sessionSwitchTimerRef unmount cleanup while adopting this PR's updated handleShellError comment: Web Shell now reports each distinct connection error once, so callback identity no longer gates re-notification of a persisted error. Also declare hasOlderHistory in the renderItem dependency array to satisfy exhaustive-deps after the merge. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 6b": run the new App.test.tsx and EmbeddedApp.test.tsx suites to confirm green — the worktree has no node_modules and no built dist outputs, and install+build exceed….
Test Plan (not a blocker): src/webview/EmbeddedApp.test.tsx — no such file or directory.
Convergence: round 3 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: packages/vscode-ide-companion/src/webview/EmbeddedApp.test.tsx (findings in round 1; 1 more now). 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. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"agent 6b":run the new App.test.tsx and EmbeddedApp.test.tsx suites to confirm green — the worktree has no node_modules and no built dist outputs, and install+build exceed…。
Test Plan(非阻断):src/webview/EmbeddedApp.test.tsx — no such file or directory。
收敛情况:第 3 轮发布了 2 条行内评论,其中 2 条是首次提出;上一轮发布了 1 条(其中 1 条首次提出)。发现反复回到同一批文件:packages/vscode-ide-companion/src/webview/EmbeddedApp.test.tsx(第 1 轮已出过发现,本轮又有 1 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。本轮没有未决的 Critical,因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
Closeout: reviewed the two remaining suggestions and intentionally left them out of this PR because they would expand beyond the #10406 connection-error loop fix. No code changes. Current exact-head checks had no red failures at scan time. |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 1503 passed · 0 failed · 1503 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:1503 通过 · 0 失败 · 1503 总计 抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10454 verification — fix(web-shell): report each connection error once to stop the inline onError re-render loopVerdict: 中文摘要
Central claim and A/BCentral claim: while
Witness: Secondary claims:
Boundary probes (own harness, head) — 4/4 green: A→B→A alternation without recovery re-reports on every value flip (3 deliveries); empty-string Mutation matrixWitness:
No surviving mutation, so no coverage-gap/dead-code/redundant-defence adjudication is needed. M2 doubles as the per-commit verification of commits Reviewer Test Plan walkthrough
FindingsF1 (nit, description accuracy) — "one notice per distinct error per outage" overstates the implemented semantics. The ref holds only the last reported value, so within one uninterrupted outage an error flapping A→B→A is reported twice for A (probe F2 (nit, diff hygiene) — F3 (observation, not a defect) — the merge commit's Not covered
MethodologyCI merge-ref checkout ( Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
ca67964
into
codex/vscode-web-shell-cutover
…nLM#9811) * feat(vscode-ide-companion): reuse WebShell transcript UI behind experimental flag Bridge ACP session/update notifications into the shared SDK daemon transcript reducer and render the result with the WebShell transcript component, gated on qwen-code.experimental.webShellTranscript (default off). The WebShell renderer and its heavy transitive dependencies (echarts, mermaid, shiki, codemirror, katex) are lazily loaded via esbuild code splitting, so the default configuration keeps the ~700KB webview bundle unchanged. * fix(vscode-ide-companion): grant wasm-unsafe-eval for shiki WASM when WebShell transcript enabled * feat(vscode-ide-companion): adopt WebShell transcript as default timeline Drop the experimental flag and the legacy MessageList renderer. The companion timeline now always renders through the shared WebShell transcript component, fed by ACP session/update notifications via the SDK daemon transcript reducer (lazy loaded through esbuild code splitting). The flag-gated wiring is removed: the qwen-code.experimental.webShellTranscript setting, the conditional CSP/body attribute in WebViewContent, and the legacy MessageList path in App.tsx (~850 lines). The webview CSP now grants wasm-unsafe-eval unconditionally for Shiki's Oniguruma WASM. * fix(vscode-ide-companion): reset WebShell transcript state on session switch The experimental useAcpTranscript hook only consumed transcriptUpdate messages, so its reducer state survived session boundaries. When the extension switched sessions it kept the webview mounted and replayed the newly-selected session through ACP, causing the previous session's blocks to merge with the new replay (e.g. user text "alpha" from session A leaked into session B as "alphabeta"). Reset both the reducer state and the rendered blocks on the same boundaries the legacy message flow uses: qwenSessionSwitched (sent before the ACP replay of the selected session) and conversationCleared (new session). Adds a regression test that replays two sessions with a switch between them. * fix(vscode-ide-companion): harden WebShell transcript session boundaries - reset the transcript on `conversationLoaded` too, closing the same cross-session leak the previous commit fixed for `qwenSessionSwitched` and `conversationCleared` (agent reconnect posts only this boundary) - track the active session id and drop late `transcriptUpdate` frames whose `sessionId` no longer matches, so a previous session's trailing frames cannot contaminate the next session's timeline - seed the transcript from cached messages carried by `qwenSessionSwitched` so offline restores and load-failure fallbacks render their history instead of a blank timeline - dispatch `assistant.done` on `streamEnd`/`sessionLoadComplete` so the final assistant/thought block of a turn (or history replay) does not stay `streaming: true` forever * fix(vscode-ide-companion): adopt live ACP session id after load-failure fallback * fix(vscode-ide-companion): echo user prompt into WebShell transcript * fix(vscode-ide-companion): keep WebShell transcript expanded and clear of the composer * fix(vscode-ide-companion): surface local error and interrupt notices in the transcript area * fix(vscode-ide-companion): restore file-link opening from the WebShell transcript * fix(vscode-ide-companion): restore contributed copy commands for the WebShell transcript * fix(vscode-ide-companion): add localOnly marker to TextMessage state type * fix(vscode-ide-companion): restore /insight progress card and report link in the transcript UI * fix(vscode-ide-companion): finalize in-flight tools on timeout and pin session-switch seeding guard Map streamEnd reasons timeout/session_expired onto the reducer's error reason so abandoned mid-tool turns no longer spin forever (ceuI). Add qwenSessionSwitched cases with no messages field and an empty cache array; the no-messages case fails when the seeding guard is forced true, pinning its false side (ceuN). * fix(vscode-ide-companion): remove unreachable editMessage backend and dead submit options The user-message edit/rewind UI was dropped in the WebShell-transcript migration, leaving editTargetTurnIndex/onSubmitted options in useMessageSubmit and the full editMessage/rewind flow in SessionMessageHandler unreachable. Remove the dead options, the editMessage dispatch case, the rewind/snapshot flow with its recovery branches, and their tests (R1-8 direction b). * fix(vscode-ide-companion): drop write-only loadingMessage bookkeeping The waiting-message renderer was removed with the WebShell transcript migration and the user prompt is echoed into the timeline at send time (bd09e19), so the loadingMessage string was write-only dead state. Keep the isWaitingForResponse flag (submit gating / cancel) and pin its API surface (R1-19 direction b). * fix(vscode-ide-companion): align waiting-flag pin test with the argument-less setter * fix(vscode-ide-companion): echo attached images into the transcript timeline The prompt carries pasted/attached images as ACP resource_link blocks, which the transcript reducer cannot render (no inline data), so user images vanished from the timeline while the attach path stayed alive. Read each saved prompt image back from disk and echo it alongside the text echo as an inline user_message_chunk image part (the daemon-echo content shape), which the shared reducer folds into the user block and the WebShell renderer already displays. Unreadable images are skipped without breaking the send. * fix(vscode-ide-companion): track live VS Code theme for the transcript webShellTheme was snapshotted once at mount via useMemo with an empty dependency array, so switching the VS Code color theme left the timeline on the stale theme (VS Code updates data-vscode-theme-kind on <body> in place without reloading the webview). Hold the theme in state and refresh it with a MutationObserver on the body theme attributes. * fix(vscode-ide-companion): copy every transcript block kind and map ambiguous row keys - Copy All Messages now includes tool, shell, user_shell, and status blocks via getBlockCopyText, matching the pre-PR copyAllMessages handler which included formatted tool calls (review 5001842059 S-1). - findBlockByRowKey prefers an exact id match and otherwise the longest matching block id, so one block id that dash-prefixes a sibling (e.g. `a` vs `a-1`) can no longer capture the sibling's row key (S-4). * fix(vscode-ide-companion): drop whitespace-only cached transcript rows cachedMessageToNotification rejected empty strings but admitted whitespace-only content, which the reducer turns into an empty block when seeding history from cached rows. Reject content that trims to nothing (review 5001842059 S-2). * fix(vscode-ide-companion): ship missing third-party notices in NOTICES.txt Extend generate-notices.js so the regenerated NOTICES.txt carries the attribution texts it previously only pointed at or dropped: - Append license files from a package's licenses/ directory (echarts' Apache LICENSE references licenses/LICENSE-d3 for its embedded d3-derived files; the BSD-3-Clause text is now shipped). - Append a package's NOTICE file when present (Apache-2.0 §4(d)), covering echarts' Apache Software Foundation attribution. - Accept string-form package.json repository values (full URLs and GitHub shorthand) instead of emitting "(No repository found)". - Fall back to the standard MIT text (copyright holder from package.json metadata) for MIT-declared packages that ship no license file. * fix(vscode-ide-companion): show a recoverable error state when the transcript chunk fails to load * test(vscode-ide-companion): gate the transcript blocks wiring into the WebShell renderer * test(vscode-ide-companion): gate the transcriptUpdate forwarding from agent to webview * docs(vscode): plan complete Web Shell cutover * refactor(web-shell): own daemon React bindings * fix(webui): preserve package entry filenames * refactor(vscode): complete WebShell UI cutover * chore(vscode): refresh third-party notices * fix(vscode): fill embedded chat viewport * test(web-shell): disambiguate workspace visual locator * fix(vscode): match embedded chat layout to host * fix(vscode): compact embedded chat styling * fix(vscode): align embedded chat density with VS Code * fix(vscode): complete embedded composer integration * fix(vscode): restore user message editing after cutover * fix(vscode): complete WebShell feature parity * test(vscode-ide-companion): repair host-wiring tests for the WebShell cutover * refactor(vscode-ide-companion): replace webui build scanner with an ESLint boundary rule The bespoke recursive source scanner reimplemented a dependency-boundary check on every extension build. A scoped no-restricted-imports rule enforces the same boundary on every lint run with less custom code; the manifest dependency entry was already removed by the cutover. * fix(web-shell): keep ChatEditor commands prop referentially stable (QwenLM#9811) The `additionalSlashCommands = []` destructure default allocated a fresh array on every App render, invalidating the `commands` useMemo and breaking ChatEditor memoization on every transcript-only re-render. Default to a module-level constant instead, matching the existing EMPTY_* convention. Also align the /skills completion expectation with the autoSubmit field the completion source intentionally emits for leaf skill items. * fix(vscode): distinguish the VS Code channel and localize its chrome The companion now drives Web Shell against a shared `qwen serve` daemon, so the CLI, the browser Web Shell, and this extension all create sessions in the same workspace catalog. Web Shell recorded `'default'` for every surface, leaving VS Code conversations indistinguishable from terminal and browser ones — the panel's history listed sessions the user never opened here, and nothing attributed a session back to the editor. Give Web Shell a `sessionSourceType` prop (defaulting to today's `'default'`) and have the companion stamp `'vscode'` on the sessions it creates, then scope the history dropdown to that source. The host also supplies a stable daemon `clientId`, which the bootstrap previously declared but never sent. Web Shell localizes its own surface from the `language` signal while the companion's chrome was hardcoded English, so a zh-CN panel rendered a Chinese transcript under an English header, history dropdown, onboarding screen, and account dialog. Route that chrome through a small string table driven by the same signal, including the host-only slash entries. Also fix accessibility defects in the history dropdown: rename and delete were revealed on hover alone and unreachable by keyboard, date headers sat inside `role="listbox"` as invalid non-option children, arrow-key roving stopped at group boundaries, `aria-modal` had no focus trap, and a primed "Delete?" survived both search changes and the pointer leaving the row. Formatting: `FileMessageHandler` and `SessionMessageHandler` were left unformatted earlier in this branch and failed the Prettier gate. * refactor(vscode): drop code orphaned by the WebShell cutover The webview entry now renders EmbeddedApp against the daemon, which left the ACP-era hook layer unreachable: nothing imports acpTranscriptAdapter, useWebViewMessages, useAcpTranscript, useToolCalls, useSessionManagement, useMessageHandling, useFileContext, useImage, or the permissionTypes added by this branch. A reachability walk from webview/index.tsx reaches eight modules; every reference to the rest comes from inside the orphaned set itself, so it deletes as a closed unit. EmbeddedWebShell goes with them. It was the host-driven entry point from the earlier stage of this branch, superseded when EmbeddedApp moved to WebShellWithProviders, and has had no consumer since — only its own DOM test and a barrel export. Also harden the daemon process lifecycle. `start()` returned the cached runtime without comparing the workspace, so in a multi-root window the second folder's chat silently reused a daemon bound to the first and scoped every session, history page, and prompt to the wrong root. Bind the daemon to its workspace and respawn on a change, keep a superseded child's late exit from tearing down its successor, and report a post-startup exit to the webview instead of leaving it fetching against a dead port. * docs(vscode): describe the daemon architecture the cutover actually ships The design doc still recorded the plan this branch started from: keep ACP as the runtime boundary, add no daemon server or loopback port, and treat "replacing ACP with daemon HTTP/SSE" as a non-goal. The final stage did exactly that, so the document argued against the code beneath it. Record the decision and its consequences instead — two processes per workspace, a daemon shared with the CLI and browser Web Shell, the vscode source type that keeps the panel's history its own, workspace rebinding in multi-root windows, and the turn-driven host features that stopped firing. * fix(vscode): repair round-2 review findings on the web-shell cutover (QwenLM#9811) - closeDiff now resolves workspace-relative paths the same way showDiff does, so permission-cycle diffs opened from daemon-relative paths can actually be matched and closed - a superseded or disposed daemon child no longer reports its exit as a crash of the live daemon - authCancelled no longer hides an already-authenticated session behind onboarding; only an unknown auth state settles to unauthenticated - selection-only activeEditorChanged events no longer undo an explicit active-file exclusion - prepareSubmit dedupes mentions in both path spaces and matches typed references on a whole-reference boundary - permission diffs open only from the SDK's authoritative file_diff preview (writes included, model-controlled toolCall mining removed) - the webview HTML carries VS Code's locale so chrome strings localize - discontinued qwen-oauth models are no longer re-applied through the new-session initial-model route * fix(vscode): repair round-3 critical findings on the web-shell cutover (QwenLM#9811) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): import daemon-react-sdk from web-shell instead of webui The cutover branch dropped the ./daemon-react-sdk export from @qwen-code/webui, but the TerminalPanel merged in from main still imports it, breaking the web-shell vite build (Missing "./daemon-react-sdk" specifier). Point the import and its test mock at @qwen-code/web-shell/daemon-react-sdk, which re-exports the same useWorkspace hook and matches every other web-shell call site. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(vscode): close WebShell UI regression gaps Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(vscode): initialize WebShell refs explicitly Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(vscode): narrow queued prompt edits Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(release): enumerate actual npm workspaces Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): include hasOlderHistory in the render-item callback deps The renderItem useCallback reads hasOlderHistory to gate the edit action but omitted it from its dependency array, failing CI's react-hooks/exhaustive-deps gate. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): report each connection error once to stop the inline onError re-render loop (QwenLM#10454) * fix(web-shell): report each connection error once to stop the onError re-render loop While a connection error persists (e.g. the daemon is unreachable), the error-notification effect re-fires whenever the onError callback identity changes. Hosts such as the VS Code embedded app pass an inline onError and update their own state when it fires, so every notification triggers a host re-render that hands the effect a fresh callback identity — re-notifying the same persistent error forever (QwenLM#10406). Track the last reported connection.error value in a ref and notify only when the value changes, resetting the tracker once the connection recovers. This guards every inline-callback consumer, not just memoized hosts. Fixes QwenLM#10406 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): only stamp the dedup ref once an onError handler exists Stamping lastReportedConnectionErrorRef before delivery meant a host that attaches onError after a persistent connection error appeared never received it: the no-op delivery already marked the error as reported. Guard on the handler first and add a regression test covering the late-attach case (red when the guard is removed). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(web-shell): document the onError dedup contract and fix comment wording Describe the reported-once-per-distinct-error semantics, the reset on recovery, and that replacing the handler mid-error does not re-deliver. Reword the effect and test comments to describe the host class instead of naming the VS Code embedded app, which passes a useCallback handler. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(vscode): mirror the web-shell value-dedup in the EmbeddedApp mock The WebShellWithProviders mock re-notified on every onError identity change, mirroring the loop App.tsx can no longer produce. Rewrite it to report each distinct error value once (resetting on recovery), keep the loop guard as a regression tripwire, exercise it with a changing callback identity plus a post-delivery effect re-run, and refresh the handleShellError comment that still cited the old loop as the memoization reason. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(vscode): cast the captured onError prop for the mock wrapper CapturedProps is an unknown index signature, so the destructured onError needs the same cast the previous mock applied inline to stay callable under tsc. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(vscode): bail the EmbeddedApp mock before stamping when no onError exists The mirrored dedup effect stamped lastReportedError and counted a notification even when no handler was attached, while App.tsx returns before stamping on that path. Add the same early return so a handler attached mid-error still receives the persistent error, and pin the no-handler no-stamp behavior with a test that fails if the guard is removed. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): remove duplicate history dependency Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(vscode): close remaining WebShell cutover regressions Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(vscode): keep permission diff handling host-scoped Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(vscode): remove orphaned completion trigger test Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: yiliang114 <jinjing.zzj@gmail.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>



What this PR does
Makes the web-shell error-notification effect report each distinct
connection.errorvalue only once. It tracks the last reported value in a ref, so a changed callback identity no longer re-fires the notification while the same error persists, and the tracker resets once the error clears.Why it's needed
Fixes #10406. While the daemon is unreachable,
connection.errorpersists, and the notification effect (deps[connection.error, onError]) re-fires whenever either changes. The VS Code embedded host (EmbeddedApp) passes an inlineonErrorthat updates host state (setHostNotice) when it fires, so the resulting re-render produces a fresh callback identity — notify → host state update → re-render → freshonError→ notify again: an infinite re-render loop for as long as the error persists. The app-side guard is the direction recommended in the issue because it covers all inline-callback consumers, not just memoized hosts.Reviewer Test Plan
How to verify
Component-level tests in
packages/web-shell/client/App.test.tsx(red before the fix, green after):reports a persistent connection error once even while host re-renders pass a fresh inline onError— mountsAppwith a persistentconnection.error(daemon unreachable) and a host wrapper mirroringEmbeddedApp: every reported error is stored in host state, re-rendering the host with a fresh inlineonError. Before the fix the same error was reported 6 times (1 initial + 1 per host re-render, stopped only by the test's cap — unbounded without it). After the fix it is reported exactly once.still reports when the connection error changes to a different value— regression guard: dedupe must not swallow real errors.reports a recurring error again after the connection recovers— regression guard for the tracker reset: error → recovery → same error again is still reported the second time.Evidence (Before & After)
Verified at component-test level (the defect scenario is daemon-unreachable, so no live daemon; same approach as #10405/#10385). Before: the repro test fails — the same error is reported 6 times while the host re-renders. After: 3/3 new tests pass, full App.test.tsx suite 553/553, EmbeddedApp.test.tsx 11/11, both packages'
tsc --noEmitclean.Tested on
Environment (optional)
Unit/component tests only (vitest, jsdom) against
codex/vscode-web-shell-cutoverhead4f3737bb46; no live daemon involved.Risk & Scope
onErrormemoization (already added in refactor(vscode-ide-companion): migrate chat to WebShell and qwen serve #9811 round 3) untouched. No behavior change whenconnection.erroris absent. No UI-screenshot verification (daemon-unreachable scenario).Linked Issues
Fixes #10406
Targets the unmerged PR #9811 branch (
codex/vscode-web-shell-cutoveras base, following #10431/#10419), since the defect exists only on that branch, not onmain.中文说明
这个 PR 做了什么
让 web-shell 的错误通知 effect 对每个不同的
connection.error值只上报一次。用 ref 记录上次已上报的值:同一个错误持续存在时,回调标识变化不再重复触发通知;错误清除时重置记录。为什么需要
修复 #10406。daemon 不可达时
connection.error持续存在,而通知 effect(依赖[connection.error, onError])在任一依赖变化时都会重跑。VS Code 内嵌宿主(EmbeddedApp)传入内联onError,触发时更新宿主状态(setHostNotice),重渲染又产生新的回调标识——上报 → 宿主状态更新 → 重渲染 → 新onError→ 再次上报:错误持续多久,无限重渲染就持续多久。issue 建议取 App 侧防护,因为它覆盖所有内联回调消费方,而不只是已记忆化的宿主。审阅者测试方案
如何验证
packages/web-shell/client/App.test.tsx中的组件级测试(修复前红、修复后绿):reports a persistent connection error once even while host re-renders pass a fresh inline onError——挂载App,connection.error持久为'daemon unreachable'(daemon 不可达),宿主包装组件模拟EmbeddedApp:每次上报的错误写入宿主状态,触发宿主重渲染并传入新的内联onError。修复前同一错误被上报 6 次(首次 1 次 + 每次宿主重渲染各 1 次,仅被测试上限截断,无上限即无限)。修复后恰好上报 1 次。still reports when the connection error changes to a different value——回归保护:去重不能吞掉真错误。reports a recurring error again after the connection recovers——tracker 重置的回归保护:错误 → 恢复 → 同一错误再次出现,第二次仍会上报。命令与输出见英文部分(App.test.tsx 553/553 通过、EmbeddedApp.test.tsx 11/11 通过,两个包
tsc --noEmit均干净)。证据(修复前后)
以组件级测试验证(缺陷场景是 daemon 不可达,不起真实 daemon,与 #10405/#10385 做法一致)。修复前:复现用例失败——宿主重渲染期间同一错误被上报 6 次。修复后:新增用例 3/3 通过,App.test.tsx 全量 553/553,EmbeddedApp.test.tsx 11/11,两个包
tsc --noEmit干净。测试环境
环境(可选)
仅单元/组件测试(vitest、jsdom),基于
codex/vscode-web-shell-cutoverhead4f3737bb46;不涉及真实 daemon。风险与范围
onError记忆化(refactor(vscode-ide-companion): migrate chat to WebShell and qwen serve #9811 round 3 已加)不动。connection.error为空时行为不变。不做 UI 截图验证(daemon 不可达场景)。关联 Issue
Fixes #10406
目标为未合并的 PR #9811 分支(以
codex/vscode-web-shell-cutover为 base,沿用 #10431/#10419 的做法),因为该缺陷只存在于该分支,main上不可触发。