feat(web-shell): enrich browser notifications and open target sessions - #11447
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Browser notification E2E reportBaseline: merged #11398 displays a generic Workspace browser flow: an isolated Chrome page submitted a real request and consumed the resulting SSE terminal event. A Notification stub captured one attempt with a The browser test uses Notification stubbing and simulated loss of focus. It validates application content, icon-resource delivery, and the click/navigation chain, not OS banner or icon appearance. The reused runtime's standalone recent-session list showed a load error; this browser scenario covered workspace sessions only. Standalone and Live target handling, malformed/conflicting context rejection, same-session no-reload behavior, split-view exit, and focus failure are covered by unit tests. Build/typecheck/bundle and focused unit results are listed in the PR body. Temporary browser instrumentation, notification preferences, and the isolated daemon are cleaned up after verification. No other local service was started or stopped. 中文说明基线：已合并的 #11398 使用通用 **工作区浏览器流程：**独立 Chrome 页面提交真实请求并消费 SSE 终态。Notification stub 捕获一次通知，包含 浏览器测试使用 Notification stub 和模拟失焦，验证应用文案、图标资源交付及点击导航链路，不代表 OS 横幅或图标实际显示。复用 runtime 的无工作区最近会话列表出现加载错误，因此本次浏览器场景只覆盖工作区会话。无工作区与 Live 目标处理、畸形/冲突上下文拒绝、同会话不重新加载、退出分屏和聚焦失败均由单元测试覆盖。 构建、类型检查、打包和定向单测结果见 PR 正文。验证结束后清理临时浏览器注入、通知偏好和独立 daemon，没有启动或停止其他本地服务。 |
|
Re-ran the gate on the current head. Nothing about the code changed since the last pass — same commit — but the one thing that stopped me then has been answered by a human, so the gate reaches a different place this time. Template: complete ✓ — all nine required headings present, the three Risk & Scope bullets filled in, both languages, and the four design docs properly paired with reciprocal links. The base design record opens with a status banner marking it the historical #11398 baseline superseded by the two new designs, which is the right way to retire a decision instead of quietly editing it. Problem: observed, not theoretical. #11398 landed the notification itself; this fixes what makes it hard to use — with several sessions running you cannot tell which one just finished, and clicking only focused the window. The body also carries a real before/after reproduction of a regression that appeared mid-development ( Direction: aligned, and the part I escalated last round is no longer open. I deferred then on two public-contract/product items: the new exported prop and type, and Size: not applicable — no core paths are touched. Checked rather than assumed: zero files match Approach: scope feels right and the reuse instincts are good — it exports the transcript renderer's existing Risk: no elevated-risk paths — I ran the revert-history patterns against the production file list and nothing matched. CI on this head is green: Moving on to code review. 🔍 中文说明已在当前 head 上重跑准入检查。代码自上一轮以来没有变化 —— 仍是同一个 commit —— 但当时挡住我的那件事已由人给出裁定，因此这一次准入结论不同。 模板： 完整 ✓ —— 九个必需标题齐备，Risk & Scope 三条均已填写，中英文对照，四份设计文档正确成对并互设语言链接。基线设计文档开头带有状态横幅，标注其为 #11398 的历史基线并已被两份新设计取代 —— 这是退役一个决策的正确做法，而不是悄悄改掉它。 问题： 是已观测到的，不是理论性的。#11398 合入了通知本身；本次修的是它在实际使用中站不住的地方 —— 多个会话并行时分不清刚结束的是哪一个，且点击只会聚焦窗口。正文还给出了开发过程中引入的一个回归的 before/after 复现（ 方向： 对齐，而且我上一轮转交的那部分已不再是未决项。当时我就两处公共契约/产品项转交：新增的导出属性与类型，以及 规模： 不适用 —— 未触及核心路径。是核对过的，不是假设：没有任何文件匹配 方案： 范围合理，复用取向也好 —— 导出 transcript 渲染器已有的 风险： 无升级风险信号 —— 我把回滚历史的模式匹配跑在了生产文件列表上，没有任何命中。该 head 上的 CI 是绿的： 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewStatic review only — I did not build, run, or test any code from this PR, per the gate's no-execute rule on fork heads. Everything below was read at My independent proposal first. Before reading the diff, from the title and the "Why it's needed" section alone, I would have: reused the existing That is very close to what shipped, and the implementation is better than mine in four places I had not thought through: unreadable storage fails closed ( The two open Criticals: I checked both against the code as it stands, and both still stand. I am not approving on the basis that they were fixed. They are unresolved, and here is my adjudication of each — a maintainer who disagrees should say so, because my approval supersedes the automated review's
Deferred, recorded so nothing is silently dropped. This PR is past five review rounds, so by the repo's own guidance the remaining Suggestions belong in the Conventions. Clean on the things AGENTS.md actually enforces: ESM only, no sequenceDiagram
participant P1 as DaemonSessionProvider
participant P2 as turn notification observer
participant P3 as notification text cleaner
participant P4 as BrowserTurnNotifications
participant P5 as OS Notification
participant P6 as App navigation listener
P1->>P2: observe with turn_complete and lazy content closure
P2->>P2: session match, dedup, consume pending promptId
P2->>P3: project title, prompt and reply from transcript blocks
P3-->>P2: bounded plain text excerpts
P2->>P4: notify with outcome, content and captured target
P4->>P4: canShow - mounted, active, enabled, granted, unfocused
P4->>P5: construct with title, body, icon and tag
P5->>P6: onclick dispatches open-session on the owner EventTarget
P6->>P6: validate context, reject locked-workspace mismatch
P6-->>P6: reveal healthy current session or load the target
Files changed — the 9 production files (14 test/doc files omitted)
Test evidence — the PR's own CI, read via the APII ran nothing from this PR. Below is the real check state on
Two things about that red row, both verified rather than assumed. Zero The behavioural claim is already substantiated, twice, by harnesses stronger than anything I could add. The sandboxed lane ran against this head in an isolated token-free container: the same driver produced different observables at the Sandboxed lanes would not settle what is left, so I am not asking for one. Not verified: the native OS banner and lock-screen rendering itself — headless Chromium has no notification presenter, so a fired 中文说明代码审查仅做静态审查 —— 按准入关卡对 fork head 的「不执行」规则，我没有构建、运行或测试本 PR 的任何代码。以下内容均读自 先说我独立的方案。 在读 diff 之前，仅凭标题与「Why it's needed」，我会这样做：复用已有的 这与实际实现非常接近，而实现有四处比我想得更周到：存储不可读时 fail-closed（用 两条未决 Critical：我都对照当前代码核过，两条都仍然成立。 我不是以「它们已修复」为依据批准的。它们未解决，以下是我对每一条的裁定 —— 若维护者不同意，请指出，因为我的批准会在本账号上覆盖自动化审查的
已推迟处理，并记录在案以免被悄悄丢掉。 本 PR 已超过五轮 review，按仓库自身的约定，剩余建议应交给 约定。 在 AGENTS.md 真正强制的项上是干净的：仅 ESM、无 （时序图与文件表见英文部分，不重复。） 测试证据我没有运行本 PR 的任何代码。以下是 关于那一行红色，有两点都是核实过的、不是假设的。 行为性主张已经被定案，而且是被两个比我能补充的任何东西都更强的 harness 定案的。 沙箱化通道在同一 head 上、于隔离无凭证容器中运行：同一份驱动对照 base 树，在 剩下的部分沙箱化通道无法定案，因此我不要求再跑一次。 未核实：原生系统横幅与锁屏渲染本身 —— headless Chromium 没有通知呈现器，因此触发 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 4/5 — solid, and now better evidenced than most PRs this size; the reason it is not 5/5 is that I am approving with two automated Criticals still unresolved in the code, and while I have adjudicated both as non-blocking, that is a judgement a maintainer should be able to see and overrule. Stepping back. Last round I deferred on one thing and said plainly it was a decision rather than a defect: should the shipped Going back to my independent proposal: the shipped design is materially the same one I sketched before reading the diff, and it is better than mine in four fail-closed choices I had not thought through — unreadable storage degrading to off while still allowing a temporary opt-in, the bounded pending Map with injected-message cleanup, the locked-workspace rejection for stale notifications, and click ownership on a per-instance EventTarget instead of a window-wide broadcast. If I inherited this in six months I would thank whoever wrote it. The reuse instincts are right too: the only change to On the question I have to answer honestly — am I approving because it is good, or because I ran out of reasons to say no. It is the first. The feature is load-bearing rather than a suite that passes identically with and without the diff, and that is proven twice by harnesses stronger than anything static review could give me: 26/26 scenarios differing at the What keeps it at 4/5, named so it is on the record and not buried. Two Criticals from the automated review at this commit are still unresolved in the code, and my approval supersedes that review's Two smaller things worth a maintainer's eye, neither blocking. The exported Approving, pinned to 中文说明信心度：4/5 —— 扎实，而且现在的证据比大多数同规模 PR 都充分；之所以不是 5/5，是因为我批准时代码里仍有两条自动化 Critical 未解决，虽然我已将两条都裁定为不阻塞，但这个判断应当让维护者看得见、并能推翻。 退一步看。上一轮我只为一件事转交，并明确说过它是决策而非缺陷：既然该功能现在会把会话标题加提问与回复摘录送到系统通知层，发布出去的 回到我独立的方案：实际实现与我读 diff 前勾勒的基本一致，而且在我没想到四处 fail-closed 取舍上比我的更好 —— 存储不可读时降级为关闭但仍允许临时开启、带上限并在注入消息时清理的 pending Map、对旧通知的锁定工作区拒绝、以及把点击归属放在每实例 EventTarget 而不是 window 全局广播上。如果六个月后接手这份代码，我会感谢写它的人。复用取向也是对的： 关于我必须诚实回答的问题 —— 我是因为它好才批准，还是因为我说不出不批准的理由了。是前者。这个功能是「承重」的，而不是一个在有无 diff 时同样通过的套件，而这一点由两个比静态审查所能给我的任何东西都更强的 harness 证明了两次：沙箱化通道中对照 base 树在 让它停在 4/5 的原因，明确记录在此而不是埋起来。该 commit 上自动化审查的两条 Critical 在代码里仍未解决，而我的批准会在本账号上覆盖那次审查的 另有两件较小的事值得维护者过目，均不阻塞。导出的 批准，钉在 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 6 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
What this PR does / 本 PR 做了什么
Follow-up to PR 11398's browser task notifications. It enriches the opt-in notification with a QwenCode · <session title> header, a bounded plain-text excerpt of the current turn's reply (120 code points; title 60), and a bundled 128×128 icon; and it makes clicking a notification open the captured session in its captured product context (workspace / standalone / live), exiting settings or split-view on the way, with a reveal-without-reload guard so re-opening the already-healthy current session does not stomp the next turn. Everything sits under packages/web-shell/client/** plus design docs — no daemon route, no public SDK field, no core path, and it reuses the existing toggle and the existing qwen:open-session entry point.
这是 PR 11398 浏览器任务通知的后续增强。它给这条可选开启的通知加上 QwenCode · <会话标题> 标题、本轮回复的有限长度纯文本摘录（120 码点，标题 60）和随包的 128×128 图标；并让点击通知时打开捕获到的会话与捕获到的产品上下文（workspace / standalone / live），途中退出设置页或分屏，并带一个"已在目标会话则只显示不重载"的保护，避免重复加载打断下一轮。改动全部位于 packages/web-shell/client/** 与设计文档 —— 不新增 daemon 路由、不新增公共 SDK 字段、不触碰核心路径，复用已有开关和已有的 qwen:open-session 入口。
Verified correct (read against the head tree, not just the diff) / 已核对无误（对照 head tree 而非仅看 diff）
The code is clean and the privacy-sensitive edges all fail closed. I confirmed each independently:
代码很干净，涉及隐私的边界都是 fail-closed。我逐条独立核对过：
- Exact-turn extraction, no borrowing.
getTurnNotificationContentmatches the terminalpromptIdexactly; a blank/invalidpromptIdyields title-only with noresponseText, andturn_errorreturns before reply extraction, so failures never expose a partial reply or error detail. - 精确取本轮、不借用旧回复。
getTurnNotificationContent严格按终态promptId匹配；promptId为空/非法时只给标题、不给responseText，turn_error在提取回复前就返回，所以失败通知不会泄露部分回复或错误详情。 - Insight payload is fail-closed. The
breaksits outside theif (!/"insight_(?:progress|ready|error)"\s*:/.test(...)), so a payload-shaped last assistant block produces no excerpt rather than falling back to an earlier block. Silence over leaking internal JSON — the right way round. - insight 载荷 fail-closed。
break位于 insight 正则判断之外，因此最后一条 assistant block 若形如内部载荷，结果是不给摘录而不是回退到更早的 block。宁可静默也不泄露内部 JSON，方向正确。 - Code-point-safe truncation, URLs stripped first.
notificationExcerptdoesArray.from(plain)before slicing (astral chars can't be split), and link/image destinations are removed before truncation, so a token-bearing markdown URL can't reach the lock screen. - 按码点安全截断、先剥链接地址。
notificationExcerpt先Array.from(plain)再切片（不会拆开代理对字符），且链接/图片的目标地址在截断前就被剥掉，带 token 的 markdown URL 不会上锁屏。 - Navigation validation rejects instead of falling back. The new early-
returnvalidation runs only when'sessionContext' in detail, so legacyqwen-session://markdown links (nosessionContext) keep their behavior. A malformed context (non-object / nokind) or a conflicting one (standalone/live carrying aworkspaceCwd, or a workspacecwdmismatch) is rejected outright rather than silently routing to the current/primary workspace. - 导航校验直接拒绝而非回落。 新增的提前
return校验只在'sessionContext' in detail时执行，所以旧的qwen-session://markdown 链接（不带sessionContext）行为不变。上下文畸形（非对象/无kind）或冲突（standalone/live 却带workspaceCwd，或 workspace 的cwd不一致）会被直接拒绝，而不是悄悄回落到当前/主工作区。 - Reveal-without-reload fails through to a real load. It requires
connected+ not loading transcript + not missing + no standalone creation-recovery + no pending draft context + matching session id + matching context kind + (non-workspace OR matchingcwd); any uncertainty falls through to a genuine load, which is the safe direction.loadSidebarSessionturns the explicit workspace context'scwdinto the actual load target, consistent with the guard'scwdcompare. - "只显示不重载"在不确定时回落到真实加载。 它要求
connected、非 transcript 加载中、非 missing session、无 standalone 创建恢复、无待分配草稿上下文、sessionId 一致、context kind 一致，workspace 还要cwd一致；任何不确定都会落到真实加载，方向安全。loadSidebarSession会把显式 workspace 上下文的cwd变成实际加载目标，与该保护的cwd比较一致。
The one call that belongs to a maintainer / 唯一应由维护者拍板的点
🟡 This changes a privacy contract that PR 11398 stated explicitly, for users who already opted in. PR 11398 promised "generic status only, no chat body" (its README said notifications show 通用状态、不含聊天正文). This PR changes that to carry the session title plus a reply excerpt to the OS notification layer, where the lock screen decides what to show. The storage key qwen-code-web-shell-browser-notifications is unchanged and there is no version/migration gate, so a true preference stored under the old promise survives the upgrade — that user starts getting conversation content on their lock screen without re-consenting. It is still opt-in, it is disclosed in the setting string / README / both design docs, and the exposure is bounded and carefully filtered, so this may well be a deliberate tradeoff. I'm not calling it a defect; I'm flagging it because it is a product decision rather than an engineering one. Worth a conscious call on whether existing opt-ins want a re-consent or a one-time notice.
🟡 本 PR 改动了 PR 11398 明确声明过的隐私契约，且影响已经开启该开关的用户。 PR 11398 承诺"只显示通用状态、不含聊天正文"（其 README 也是这么写的）。本 PR 把契约改成携带会话标题与回复摘录送到系统通知层，锁屏如何展示由系统决定。存储键 qwen-code-web-shell-browser-notifications 未变、也没有版本/迁移门控，所以在旧承诺下存的 true 偏好会跨版本保留 —— 该用户会在没有重新确认的情况下开始在锁屏上看到会话内容。它仍是可选开启、已在设置文案/README/两份设计文档中告知、暴露范围有限且过滤细致，所以很可能是有意的取舍。我不认为这是缺陷；提出来是因为它是产品判断而非工程判断。是否要给已开启的老用户一个重新授权或一次性提示，值得有意识地拍一下。
Because this is an open product question rather than something I should settle unilaterally, I'm leaving a comment rather than approving.
因为这是一个待定的产品问题、不该由我单方面定案，所以我留评论而不直接批准。
Non-blocking notes / 非阻塞意见
- 🟢
notificationExcerptruns its whole regex chain andArray.fromover the full reply before slicing to 120 code points. It fires once per turn-end and only while unfocused (not a hot path), but on a long reply it does substantially more work than the output needs — slicing the source to a bounded prefix first would be cheap. - 🟢
notificationExcerpt会对完整回复跑完整条正则链和Array.from，之后才切到 120 码点。它每轮结束只触发一次、且仅在失焦时（非热路径），但回复很长时做的工作远超输出所需 —— 先把源文本切到有限前缀会更省。 - 🟢 The live observe call site prefers
connectionRef.current.displayNameon a session match and falls back togetSessionDisplayName(activeSession.state); the replay call site uses only the latter. Deliberate (connection state hasn't settled during catch-up) and the replay test pinsstate.displayName— but it's under-commented for how easy it is to "unify" the two into an epoch-reset regression. A one-line comment would protect the next reader. - 🟢 live 的 observe 调用点在会话匹配时优先用
connectionRef.current.displayName，否则回落到getSessionDisplayName(activeSession.state)；replay 调用点只用后者。这是有意的（追补期间 connection state 尚未稳定），replay 测试也锁定了state.displayName—— 但相对于"顺手统一两处就造成 epoch-reset 回归"的难度来说注释偏少，加一行注释能保护后来的读者。 - 🟢 Minor privacy note: the excerpt strips code-fence markers but keeps code-block content, so a code-heavy reply puts raw source into the excerpt. That's within the disclosed "reply excerpt", just noting it sits next to the lock-screen question above.
- 🟢 小的隐私提示：摘录会剥掉代码围栏标记但保留代码块内容，所以以代码为主的回复会把原始源码放进摘录。这在已告知的"回复摘录"范围内，只是提醒它和上面的锁屏问题相邻。
CI at time of writing / 发布时 CI 状态
Substantive checks are green on 2034a0a — Test (ubuntu, Node 22.x), Lint & Static, Capture web-shell visuals, Integration Tests (no-AK), Desktop Shell (windows + ubuntu). Still pending: web-shell E2E Smoke and the bot review-pr; nothing red. The author's "1,180 tests pass" is a local macOS claim, but the unit suite result on this commit is green in CI. Two behavioral claims remain browser-only and are not statically verifiable from the diff: that the icon URL resolves to a served 128×128 PNG in a production qwen serve bundle, and the real focus→dispatch→navigation handoff on click — both are asserted in the author's local Chrome trace and partly by the (still-running) E2E smoke.
发布时 2034a0a 的实质性检查均为绿 —— Test (ubuntu, Node 22.x)、Lint & Static、Capture web-shell visuals、Integration Tests (no-AK)、Desktop Shell (windows + ubuntu)。仍在运行：web-shell E2E Smoke 与机器人 review-pr；无红。作者"1,180 项测试通过"是 macOS 本地陈述，但本 commit 的单测结果在 CI 上是绿的。有两项行为性主张仅在浏览器中验证、无法从 diff 静态确认：图标 URL 在生产 qwen serve 打包下确实解析为可访问的 128×128 PNG，以及点击时真实的 聚焦→事件派发→导航 交接 —— 二者由作者本地 Chrome 轨迹及（仍在运行的）E2E smoke 部分覆盖。
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Current-prompt notification verificationCommit Build, typecheck, bundle, changed-file ESLint/Prettier, and 353 focused tests passed. Independent review found no issues. In Chrome, two successive real workspace requests each produced one captured notification containing its own prompt and reply; deduplication storage contained hashes only. Capture used a Notification stub and simulated loss of focus, so this does not establish OS banner layout. A separate fresh preview was checked to use the native Notification API with granted permission and the notification preference enabled. 中文说明提交 build、typecheck、bundle、修改文件的 ESLint/Prettier 和 353 项定向测试全部通过，独立审查未发现问题。Chrome 中连续两次真实工作区请求各捕获一条通知，包含自身回合的提问和回复，去重存储仅含哈希。参数采集使用 Notification stub 和模拟失焦，不代表 OS 横幅布局已验证。另一个新开的预览页面已确认使用原生 Notification API，权限允许且通知开关开启。 |
Allow hosts to configure notification names, icons, and the initial preference. Enable notifications by default in the built-in page while preserving saved user choices and browser permission requirements. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
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 explored to full depth (tool budget reached): "agent reverse-audit (round 2)": confirming whether WorkspaceSessionProvider 's targetWorkspace can resolve to a kind === 'live' workspace entry (the second, lock-plus-live variant of the …; "agent reverse-audit (round 2)": did not run packages/web-shell/client/daemon/session/turn-notification-context.test.ts to confirm the five new cases pass at this commit.; "agent reverse-audit (round 2)": did not confirm in the daemon/bridge event publisher whether an injected background_notification user message is stamped with the running turn's promptId (t…; "agent reverse-audit (round 3)": did not read the effectSessionContext?.kind === 'live' branches at DaemonSessionProvider.tsx:1741 and :2107 , so a {kind:'live'} context arriving from a …; "agent reverse-audit (round 3)": no mutation sweep over the six added browser-turn-notifications.test.tsx cases — each assertion was reasoned about individually and the file was run once gree…, and 2 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
仅完成部分审查，审查缺口已披露。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：confirming whether WorkspaceSessionProvider 's targetWorkspace can resolve to a kind === 'live' workspace entry (the second, lock-plus-live variant of the …；"agent reverse-audit (round 2)"：did not run packages/web-shell/client/daemon/session/turn-notification-context.test.ts to confirm the five new cases pass at this commit.；"agent reverse-audit (round 2)"：did not confirm in the daemon/bridge event publisher whether an injected background_notification user message is stamped with the running turn's promptId (t…；"agent reverse-audit (round 3)"：did not read the effectSessionContext?.kind === 'live' branches at DaemonSessionProvider.tsx:1741 and :2107 , so a {kind:'live'} context arriving from a …；"agent reverse-audit (round 3)"：no mutation sweep over the six added browser-turn-notifications.test.tsx cases — each assertion was reasoned about individually and the file was run once gree…，另有 2 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.23.1)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
doudouOUC
removed
the
autofix/takeover
|
⏳ Historical-head review — head moved to 中文说明⏳ 历史 head 评审 —— 本次评审进行中 head 移动到了 |
Resolve independent App test helper additions and main test imports while preserving both branches. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Processed this incremental review batch in 0cf3094 after merging current main in bb8ccdb. The two merge conflicts were independent test imports/helpers; both sides were retained. The branch is pushed and substantive CI is running again.
Validation on macOS / Node 22.22.3: root build and typecheck passed; focused ESLint and Prettier checks passed; 1,292 tests passed across App (879), notification/provider context (346), and notification text/browser/public/main entry (67). The link-wrapper defect was reproduced against 567912e and the revised helper was independently probed with code/link, control, dunder, and long-input samples. This batch did not rerun a native OS banner E2E; passing application tests do not certify OS presentation. Two clean diff-audit passes completed. The unconfirmed background-user-message question in the overall reviews was traced separately: live background announcements produce assistant chunks; user-kind notification records are produced by transcript replay without a same-turn promptId. No current producer was found generating the matching user block required by the hypothetical counterexample, so no speculative filter was added. This is a bounded code-trace conclusion, not a claim that every future producer is safe. All 10 new inline threads have individual replies; they are being resolved after this summary. Earlier resolved threads were not duplicated. Existing CHANGES_REQUESTED reviews are not dismissed by this operation, and the PR is not being merged automatically. Linux tests/integration/lint and visual capture are running on the new head; no failure was reported at this snapshot. 中文说明本轮增量审查已由 0cf3094 处理，并先通过 bb8ccdb 合入当前 main。两处冲突均为独立新增的测试导入或辅助函数，已保留双方内容。分支已推送，实质 CI 已重新运行。
macOS / Node 22.22.3 验证：根构建和类型检查通过，定向 ESLint 与 Prettier 检查通过，1,292 项测试通过：App 879 项、通知/provider context 346 项、通知文本/浏览器/公共入口/main 67 项。已在 567912e 复现链接问题，并独立验证新 helper 的代码、链接、控制字符、双下划线和长输入样本。本轮没有重跑原生 OS 横幅 E2E，应用测试通过不代表系统呈现已验证。完成两轮无新增问题的 diff 自查。 对于总体审查中的后台 user 消息疑点，已单独追踪：实时后台公告产生 assistant chunk；user-kind 通知记录来自 transcript replay，未携带同轮 promptId。当前未找到实际生产链路能生成反例所需的匹配 user block，因此没有加入推测性过滤。这只是本次代码追踪的有限结论，不代表未来所有 producer 都安全。 10 条新 inline 意见均已单独回复，接下来关闭相应线程；没有重复回复此前已处理线程。本操作不撤销现有 CHANGES_REQUESTED，也不会自动合并。新 head 的 Linux 测试、集成、lint 和视觉捕获正在运行，当前快照没有失败。 |
…ation-details # Conflicts: # packages/web-shell/client/main.test.tsx
|
Merged
Verification on the merge commit, under Node 22 to match CI: |
|
@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: 247 passed · 0 failed · 247 total Flakiness gate: ✅ 7 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：247 通过 · 0 失败 · 247 总计 抖动门：✅ 7 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 11447 deep verification —
|
| # | scenario | head observable | base observable |
|---|---|---|---|
| S01 | plain completed turn | QwenCode · Fix the login bug, 3 body lines, prompt + reply |
Qwen Code, 1 line, status only |
| S02 | Use Map<string, number> here, no title |
title falls back to the prompt line, generics intact | no prompt carried |
| S03 | Is 3 < 5 and 7 > 2 correct? |
comparisons intact | no prompt carried |
| S04 | reply ```html<br>``` |
Reply: <br> (literal, not parsed) |
no reply carried |
| S05 | turn_error with a partial reply + error text |
prompt shown; no reply, no ENOENT detail |
status only |
| S06 | trailing @attachment:///private.txt |
Prompt: check this — token removed |
status only |
| S07 | mid-line @attachment:/// |
kept (it is user content, not a transport tail) | status only |
| S08 | final block is insight payload only | no Reply: line, and the earlier real reply is not borrowed |
status only |
| S09 / S09b / S10 / S11 | background / vision-notice / thought / subagent block last | Reply: Main answer in all four; the excluded text never appears |
status only |
| S12 | two turns' blocks present | only Current request / Current reply; no borrowing |
status only |
| S13 | 200/200/300-char inputs | title 60, prompt 80, reply 120 code points, each ending … |
Qwen Code, 1 line |
| S14 | 100 astral emoji as the prompt | exactly 80 code points, no lone surrogate | no emoji carried |
| S15 | NUL, BEL, RLO, ZWSP, LRI | a b c d e f; zero control/bidi chars survive |
status only |
| S16 | appName: 'Acme Agent', CDN iconUrl |
Acme Agent · Branded, icon = the CDN URL |
Qwen Code, no icon passed |
| S17 | whitespace-only appName/iconUrl |
both fall back independently to QwenCode + bundled PNG |
Qwen Code |
| S19 | no title and no prompt | title is QwenCode with no separator; body is status only |
Qwen Code, status only |
| S20 | optimistic block without promptId |
prompt and title recovered from the admission label | no prompt carried |
| S21 | mid_turn_message_injected then terminal |
NO NOTIFICATION | notifies ⟵ flip |
| S22 | 1500 admissions, then two replayed terminals | 1 notification (evicted p1 suppressed, p1500 kept with its label) |
2 notifications ⟵ flip |
| S23 | click the notification | 1 event on the owning provider's EventTarget with {sessionId, sessionContext:{kind,cwd}}, 0 on window, notification closed |
0 / 0, closed (focus only) |
| S24 | active: false |
observer + settings contexts undefined, no delivery |
delivers (no such opt-out) ⟵ flip |
| S25 | shared storage after a delivery | claim store holds exactly one qwen-code-turn:<64 hex>; no title, prompt, reply, cwd or / anywhere |
same shape (pre-existing) |
| S26 | fresh site / saved false / saved true / unreadable storage | defaultEnabled on for a fresh site; saved false and true both win; unreadable storage starts off and non-persistent yet still accepts an explicit temporary enable; 0 permission requests at mount, 1 on explicit enable |
fresh site off (no defaultEnabled); the saved-choice and unreadable-storage rows are identical |
scenarios=26 observable_differences=26. Full table with the exact strings: logs/ab-table.txt, witness image 01-ab-notification-boundary-head-vs-base.png.
Harness validity control. S01 asserts a notification is delivered on both arms. This is not decorative: my first draft of this harness produced 0 notifications on both arms, because with an empty localStorage the stored preference is absent and readPreference() yields enabled: false on base and head. Every cell would have read "the PR changed nothing". Seeding the stored preference is what makes the 26 differences meaningful, and S01 is the assertion that proves the seed took.
Findings
1. (Suggestion) The excerpt cleaner still guesses Markdown emphasis on text that is code — the sibling of the regression a previous round already fixed
A previous review round caught this class: Use Map<string, number> here became Use Map here and Is 3 < 5 and 7 > 2 correct? became Is 3 2 correct?, because the cleaner stripped angle-bracket runs as HTML. The fix (preserve angle brackets literally, protect fenced code) is real and I verified it — 6/6 angle-bracket assertions and 6/6 fenced-code assertions pass, and reverting it (mutation M2 below) turns the PR's own test red.
The fix closed the angle-bracket door. The ** door is still open, and so is the indented-code-block door. Sweeping the adjacent shapes the same root cause admits:
| input (a realistic user prompt) | notificationExcerpt(input, 120) |
|
|---|---|---|
compute 2 ** 3 ** 4 |
compute 2 3 4 |
Python exponentiation eaten as bold |
why does ls **/*.ts **/*.js differ |
why does ls /*.ts /*.js differ |
shell glob corrupted — meaning changes |
const a = 1;<br> const b = **bold**; |
const a = 1; const b = bold; |
CommonMark indented code block, unprotected |
- 5 degrees below zero |
5 degrees below zero |
leading - read as a bullet |
* 2 * 3 = 6 |
2 * 3 = 6 |
leading * read as a bullet |
The sharpest form is the asymmetry with the fence protection the PR does ship:
notificationExcerpt('```py\nx = 2 ** 3 ** 4\n```', 120) -> 'x = 2 ** 3 ** 4' (protected)
notificationExcerpt('compute 2 ** 3 ** 4', 120) -> 'compute 2 3 4' (eaten)
notificationExcerpt(' const b = **bold**;', 120) -> 'const b = bold;' (eaten)
~~~/``` fences are protected; the four-space indented code block — equally a CommonMark code block — is not. That is precisely the "indented form an ^ {0,3}-anchored rule never matches" sibling.
Reproduce:
cd packages/web-shell
cp ../../tmp/pr11447-verify-20260910-102641/verify-cleaner.test.ts client/zz-probe.test.ts
npx vitest run --config vitest.config.ts zz-probe # then read logs/cleaner.json -> probes
rm client/zz-probe.test.tsFull 30-shape sweep: logs/cleaner.json (probes), witness image 02-cleaner-sibling-sweep.png.
What this is not. It is not a spec violation — the design doc says cleanup "removes common Markdown formatting", and **bold** is common Markdown formatting. It is not a security issue, not data loss and not a crash: the value is a transient notification excerpt, the transcript and the stored message are untouched, and shared storage holds only hashes (S25). It does not regress against base, because base shows no prompt text at all. The blast radius is "a notification may show compute 2 3 4 for a prompt that said compute 2 ** 3 ** 4".
Why report it anyway. The author already accepted one review round establishing that a plain-text notification should not guess whether punctuation is markup — that is the stated reason angle brackets are now preserved literally. The same argument applies verbatim to a ** pair in a programming prompt, and the shell-glob case changes meaning rather than only cosmetics.
Minimal suggested fix, preserving the commit's intent
Extend the existing fence protection to the indented code block, and require a bold run to be flanking rather than merely paired — the same shape CommonMark uses for emphasis delimiters. Sketch, not measured:
// notificationTextLines(): treat a >=4-space indented line as code, like a fence
if (!fence && /^ {4,}\S/.test(raw)) { /* push cleaned-of-controls raw, skip markup */ }
// and only strip `**` when it flanks a word, not when it is an operator/glob
.replace(/(\*\*|~~)(?=\S)(.*?\S)\1/g, '$2')I did not apply and drive this through the harnesses, so per this report's own standard it is unmeasured: I am not claiming it makes the hostile fixtures clean while leaving benign ones byte-identical. The fixture that would pin it is the five-row table above added to notification-text.test.ts; today that suite is green both with and without such a patch along this axis (see the mutation-run-in-reverse note under Not covered).
2. (Nice to have) defaultEnabled is inert for a host that supplies browserNotifications after first render
WebShellWithProviders always renders BrowserTurnNotifications, passing active={browserNotifications !== undefined}. The provider therefore mounts on the host's very first render, and defaultEnabled is captured then and never re-read:
const [defaultEnabled] = useState(options?.defaultEnabled ?? false);Measured (harness verify-lifecycle.test.tsx, case L4): mount with the prop omitted, then re-render with { defaultEnabled: true } on a fresh site with no stored choice →
enabledAfterLateDefaultEnabled: false
permissionRequests: 0
So a host that loads its own configuration asynchronously and passes browserNotifications on a later render — an ordinary React pattern — silently gets "off", with no permission prompt and no diagnostic. The rest of the knob works: appName and iconUrl are re-read on every render (S16, S17, L3), and defaultEnabled works when present at first render (S26: fresh site → enabled: true).
This is a documentation-precision issue rather than a code defect. The PR description is accurate ("Notification defaults are read at mount"). The design doc sentence "The initial default is read at mount even if integration is initially omitted" is the ambiguous one: on the natural reading, "even if integration is initially omitted" promises the default still applies when integration arrives later, which is not what happens. Suggest rewording to say plainly that defaultEnabled must be present on the first render, since the provider mounts whether or not the prop is passed.
Reproduce: logs/lifecycle.json → observed.L4.
Probed and did not hold
Bounding finding 1 matters more than widening it, so I drove the scarier readings of the same mechanism and most of them do not hold:
- No superlinear blowup on outsider-authored text. Prompt and reply text is written by users and by the model, so the cleaner runs over input an outsider authors — the shape that usually hides a ReDoS. I ran a 10-shape × 5-rung ladder (256 / 1024 / 2048 / 4096 / 20000 UTF-16 units) over the worst shapes I could construct: backtick runs,
[runs,<runs, bold-pair soup, link soup, an unterminated[+ 20k chars, an unterminated code span, word soup, multiline prose. The curve is linear up to the cap and flat past it —link-soup, the most expensive shape, goes 0.06 → 0.15 → 0.31 → 0.60 ms across the four sub-cap rungs (≈2× per doubling) and then 0.63 ms at 20000, i.e. the 20k input costs the same as the 4096 one. Slowest rung overall 0.63 ms; every rung's output stayed within the 120-code-point limit. The reason is structural and worth stating because it is the whole defence:text.slice(0, MAX_NOTIFICATION_SOURCE_LENGTH)runs before any regex, so no regex ever sees more than 4096 units regardless of what a user pastes. 21/21 ladder assertions pass. Witness image03-scaling-ladder-and-sections.png, rawlogs/cleaner.json→ladder. - The list-nested fence sibling is closed. The class this bug belongs to typically also fails "a fence nested in a list never enters the fenced state". Here it does:
- step/ two-space-indented```js/const f = <T,>(x: T) => x;yieldsstep const f = <T,>(x: T) => x;— the delimiter regex is^\s*-anchored, not^ {0,3}, so indentation does not defeat it, and the generics survive (P1.9). - Fence bookkeeping is CommonMark-correct on the mismatched cases.
cannot be closed by ``` ``` ``` but ``` ``` ``` can be closed by~~~fence is not closed by backticks and the stray backtick line is kept as content; an unclosed fence still protects everything after it. 6/6 assertions (A2). - Two shapes I expected to be eaten are not.
#include <stdio.h>and192.168.1.1 is the hostboth survive verbatim (P1.1, P1.6) — the list/heading rule requires\s+after the marker, so a#or.glued to text is not treated as markup. That is the guard doing its job, and it is the reason finding 1 is narrow (a**pair with surrounding spaces) rather than broad. - Attachment-tail stripping is layered correctly, not duplicated. The cleaner keeps a mid-line
@attachment:///token (P1.30) while the pipeline removes a trailing transport token (S06) and keeps a mid-line one (S07). The stripping lives innotificationPromptText, not innotification-text.ts, so the cleaner stays reusable for the reply — worth recording so nobody "fixes" P1.30 by pushing the rule down a layer. - No cross-instance navigation leak. A click dispatches on the owning provider's
EventTargetonly, never onwindow(S23: 1 event on the target, 0 on window), so two shells on one page cannot both navigate. Base broadcasts nowhere and navigates nowhere. - No content in shared storage. After a real delivery through the Web-Locks claim path,
localStorageholds exactly oneqwen-code-turn:<64 hex>tag and nothing else — no title, prompt, reply, cwd, or path separator (S25, 6 assertions including the positive control that the claim store really was written).
Mutation matrix and vacuity
15 single-clause reverts of guards this PR introduces, each run against the test file(s) that should catch it, then restored with a sha256 check. 15/15 killed, 0 survivors, 0 patch errors, 15/15 restores byte-exact. Witness image 04-mutation-matrix-15-of-15-killed.png; raw logs/mutations.log, structured logs/mutations.json.
| id | role | file | clause reverted | result | tests |
|---|---|---|---|---|---|
| PC1 | positive control | notification-text.ts |
slice(0, limit - 1) → slice(0, limit) |
killed | 1 failed / 4 passed (5) |
| M1 | guard | notification-text.ts |
fence state never opens | killed | 3 failed / 2 passed (5) |
| M2 | the reviewed regression re-introduced | notification-text.ts |
re-add .replace(/<[^>]*>/g, '') |
killed | 1 failed / 4 passed (5) |
| PC2 | positive control | browser-turn-notifications.tsx |
'QwenCode' → 'QwenCodeX' |
killed | 6 failed / 21 passed (27) |
| M3 | guard | browser-turn-notifications.tsx |
failed-turn reply suppression | killed | 2 failed / 25 passed (27) |
| PC3 | positive control | turn-notification-context.ts |
responseText gains ' MUTATED' |
killed | 5 failed / 19 passed (24) |
| M4 | guard | turn-notification-context.ts |
mid_turn_message_injected clearing |
killed | 1 failed / 23 passed (24) |
| M5 | guard | turn-notification-context.ts |
1024-entry bound on the admission path | killed | 1 failed / 23 passed (24) |
| M6 | guard | turn-notification-context.ts |
background / vision-notice exclusion | killed | 1 failed / 23 passed (24) |
| M10 | combination row | turn-notification-context.ts |
both bound sites on the pending Map | killed | 2 failed / 22 passed (24) |
| PC4 | positive control | App.tsx |
qwen:open-session handler disabled |
killed | 18 failed / 861 passed (879) |
| M7 | guard | App.tsx |
locked-workspace rejection | killed | 1 failed / 878 passed (879) |
| M8 | guard | App.tsx |
healthy-current-session fast path | killed | 2 failed / 877 passed (879) |
| PC5 | positive control | index.tsx |
active={false} |
killed | 2 failed / 19 passed (21) |
| M9 | guard | index.tsx |
active={browserNotifications !== undefined} → true |
killed | 2 failed / 19 passed (21) |
Notes on how to read this:
-
Every positive control is landed in the same file as the mutants it validates, so no survivor could have been explained by "the chosen command never collected anything that imports this file". PC4 in particular proves
App.test.tsxis collected and exercised: 879 tests, 18 of them red when the navigation handler is disabled. -
The kills are attributed to the intended assertion, not to an incidental one. For three of the guards I re-ran the mutation and captured the failing test name and message (
logs/mutation-M2-detail.log,-M5-,-M7-). Each landed on the test named for that exact guard, with a behavioural expected-versus-actual mismatch rather than a broken import or fixture:mutation test that went red failure message M2 (HTML-guessing re-added) notification text > keeps literal markup and code instead of guessing HTML tagsexpected '' to be '<br>'M5 (pending bound removed) turn notification observer > evicts oldest pending labels at the scope limitexpected "spy" to not be called at all, but actually been called 1 timesM7 (locked-workspace rejection removed) App session callbacks > rejects an old notification after the host changes its locked workspaceexpected "spy" to not be called at all, but actually been called 3 times -
M2 is the vacuity proof for the reviewed fix. Re-introducing the exact HTML-guessing the previous round rejected turns the PR's own new test red — and it is the test whose name states the fix's intent, failing on its first assertion. So
notification-text.test.tspins the fix deliberately, not by accident. -
M10 is the combination row. The pending-Map bound is defended at two sites (
admitand thepending_prompt_*branch). Reverting the admission site alone (M5) is caught, so this PR's guards are not layered-over-each-other in the way that hides single-hunk reverts; the combination row is included anyway and is caught by 2 tests rather than 1, i.e. the second site has its own coverage. -
M7 killed exactly one test — the locked-workspace rejection is pinned precisely, not incidentally.
Targeted gates
| gate | command | result |
|---|---|---|
packages/web-shell unit tests @ head |
vitest run --config vitest.config.ts |
303 files / 7213 tests, all passed, exit 0 (logs/gate-web-shell-head.log) |
packages/web-shell typecheck |
npx tsc -p tsconfig.json --noEmit |
exit 0, no diagnostics (logs/typecheck-web-shell.log) |
| design docs bilingual parity | heading-structure + line-count diff, reciprocal links | EN/zh-CN matched: 33/33 and 27/27 lines, identical heading counts, reciprocal language links in all four files; the superseded base design web-shell-browser-turn-notifications.md gained a status banner pointing at both new docs |
A typecheck false alarm worth recording. My first tsc run reported client/notification-text.ts(18,15): error TS18047: 'delimiter' is possibly 'null'. That was my own race, not a PR defect: I had started the typecheck while the background mutation runner had mutation M1 applied to that very file (if (delimiter && !fence) → if (false), which removes the narrowing that makes line 18 safe). Re-run on a verified-clean tree (git status --porcelain empty) it exits 0 with no diagnostics. Both runs are recorded so the discrepancy is auditable rather than quietly dropped.
Not covered
- Real browser and OS banner rendering. Everything here observes the arguments reaching the
window.Notificationconstructor and theCustomEventa click dispatches, inside jsdom. Native banner layout, icon placement, macOS/Chrome site attribution, lock-screen preview behaviour and notification-centre grouping are not exercised. The PR description says the same of its own evidence ("Notification API stubbing … establish application parameters and navigation, not native OS banner layout"), so this is a shared limit, not a gap against a claim. Chromium is available in this container (/__w/_temp/pw-browsers) and the workspace ships a Playwright config; I spent the budget on the content pipeline and the mutation matrix instead, because those are where the two findings live. - No end-to-end daemon turn. I did not drive
qwen servewith a real daemon, submit a real prompt, and unfocus a real page. The transcript blocks andDaemonEvents my harness feeds the pipeline are hand-built to the SDK's realDaemonTextTranscriptBlock/ event shapes. So this reproduces the wire shape of a terminal turn, not the model-side and daemon-side path that produces it. - Per-commit attribution is out of reach.
git rev-list HEAD^1..HEAD^2returns 1 commit locally while$QWEN_VERIFY_CONTEXTlists 8, andgit rev-parse --is-shallow-repositoryistrue. This is the shallow-boundary case: the checkout is depth 2, so only the merge commit, the base tip and the PR head exist. I verified the aggregateHEAD^1..HEADdiff only. In particular the description's9056cebreproduction of the angle-bracket regression is not locally reachable — I proved the fix holds at head and that reverting it (M2) turns the PR's own test red, which is the same conclusion by a different route. - Lint not run.
eslint/prettieroverpackages/web-shellis covered by the PR's own CI and I did not re-run it; I therefore make no claim about it, and I did not plant a violation to prove that gate live. - App-side navigation validation beyond M7/M8. The design doc lists several rejection rules for explicit contexts (malformed context,
workspaceCwdconflicting with astandalone/livecontext, stale notifications after a lock change). I mutated two of them (M7 locked-workspace, M8 fast path) and both are pinned; I did not independently drive the remaining validation branches, relying on the 879-testApp.test.tsxsuite plus PC4 as the evidence that this surface is genuinely exercised. - Mutation run in reverse for finding 1. I did not apply the suggested fix for finding 1 in a scratch copy and re-run the suite, so I cannot state whether
notification-text.test.tswould go red against it. As written above, the five-row fixture table is the thing that would pin that axis, and its absence is exactly why the**sibling survived review. - No trial merge against current
main. The base tip isHEAD^1of the merge ref, i.e. the merge GitHub already computed, and the checkout is shallow so I could not fetchmainto test a merge against today's tip.
Methodology
Environment: the CI verify job container (node:22-bookworm), Node v22.23.2, working tree at refs/pull/11447/merge (0239c4ee), npm ci and npm run build already completed at head before this round started. The base arm is a scratch worktree at tmp/base-tree pinned to HEAD^1 (0ef35351); no base npm ci was needed and none was run.
Ruling out the control confound. The base tree has no node_modules of its own, so Node resolves upwards into the head tree's root node_modules, where @qwen-code/* are symlinks pointing into the head tree — the trap that makes a naive base control quietly load changed head code. I asserted the realpaths rather than assuming: @qwen-code/sdk → /__w/qwen-code/qwen-code/packages/sdk-typescript, @qwen-code/web-shell → …/packages/web-shell (both head-tree paths, quoted in the header block of logs/ab-table.txt). This is safe here because the module graph my harness exercises reaches @qwen-code/sdk only through client/adapters/transcriptToMessages.ts, and the PR changes zero paths outside packages/web-shell/ and docs/ (git diff --name-only HEAD^1..HEAD filtered → 0), with packages/sdk-typescript/src/daemon/index.ts and .../ui/types.ts sha256-identical between the two trees. package.json and package-lock.json are untouched, so reusing the root node_modules is a clean control. Everything the PR does change is resolved from within each tree by relative import and by vitest.config.ts's own aliases. The two trees' browser-turn-notifications.tsx differ (sha256 5bf875a7… base vs 30aa9dbc… head), confirming the arms are not accidentally identical.
Harnesses are plain vitest/Node files kept in this directory so a maintainer can rerun them verbatim: verify-ab.test.tsx (arm-agnostic A/B, driven by run-ab.sh, which also prints the control-identity and realpath evidence), verify-cleaner.test.ts + print-cleaner.mjs (spec assertions, 30-shape sibling sweep, 10-shape × 5-rung scaling ladder), verify-lifecycle.test.tsx (prop-change and frozen-default behaviour), verify-assets.mjs (icon provenance and published-library inlining), mutate.mjs and mutation-detail.mjs. Each is copied into packages/web-shell/client/ as zz-verify-*.test.ts(x) for the run and deleted immediately after; git status --porcelain is empty at the end of every phase, and every mutation restore is sha256-verified before the next one starts.
Raw per-cell logs live in logs/: ab-head.log/ab-base.log (full vitest output per arm), obs-head.json/obs-base.json (per-scenario observables), ab-head.json/ab-base.json (assertion counts), cleaner.log/cleaner.json, lifecycle.log/lifecycle.json, assets.log/assets.json, mutations.log/mutations.json, mutation-M2-detail.log/mutation-M5-detail.log/mutation-M7-detail.log, gate-web-shell-head.log, typecheck-web-shell.log, base-realpath.log, ab-table.txt.
Assertion counts (assertions.json, 247 total, 0 fail): A/B head 83, A/B base control cells 55, cleaner spec + ladder 60, lifecycle 19, asset/library claims 15, mutation kills 15. Base-arm control cells are counted as passes because each asserts that base lacks the behaviour; fail counts only unexpected outcomes, and there were none.
Flakiness gate log
rounds=5 files=7 skipped=0
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/browser-turn-notifications.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/browser-turn-notifications.test.tsx
file packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/daemon/session/DaemonSessionProvider.test.tsx
file packages/web-shell/client/daemon/session/turn-notification-context.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/daemon/session/turn-notification-context.test.ts
file packages/web-shell/client/index.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/index.test.tsx
file packages/web-shell/client/main.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/main.test.tsx
file packages/web-shell/client/notification-text.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/notification-text.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/web-shell/client/App.test.tsx: PPPPP
packages/web-shell/client/browser-turn-notifications.test.tsx: PPPPP
packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: PPPPP
packages/web-shell/client/daemon/session/turn-notification-context.test.ts: PPPPP
packages/web-shell/client/index.test.tsx: PPPPP
packages/web-shell/client/main.test.tsx: PPPPP
packages/web-shell/client/notification-text.test.ts: 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/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/browser-turn-notifications.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/daemon/session/turn-notification-context.test.ts: P (exit 0)
round 1 · packages/web-shell/client/index.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/main.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/notification-text.test.ts: P (exit 0)
round 2 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/browser-turn-notifications.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/daemon/session/turn-notification-context.test.ts: P (exit 0)
round 2 · packages/web-shell/client/index.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/main.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/notification-text.test.ts: P (exit 0)
round 3 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/browser-turn-notifications.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/daemon/session/turn-notification-context.test.ts: P (exit 0)
round 3 · packages/web-shell/client/index.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/main.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/notification-text.test.ts: P (exit 0)
round 4 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/browser-turn-notifications.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/daemon/session/turn-notification-context.test.ts: P (exit 0)
round 4 · packages/web-shell/client/index.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/main.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/notification-text.test.ts: P (exit 0)
round 5 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/browser-turn-notifications.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/daemon/session/turn-notification-context.test.ts: P (exit 0)
round 5 · packages/web-shell/client/index.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/main.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/notification-text.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
@qwen-code /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: 229 passed · 0 failed · 229 total Flakiness gate: ✅ 7 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：229 通过 · 0 失败 · 229 总计 抖动门：✅ 7 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 11447 deep verification (round 2) —
|
| # | finding | severity (round 1) | status at this head |
|---|---|---|---|
| 1 | Excerpt cleaner guesses Markdown emphasis on text that is code (**, indented code block) |
Suggestion | stands, narrowed and partly corrected — re-measured live: 5/5 reported shapes reproduce byte-for-byte (logs/cleaner.json). Adjudicated against micromark: only 2 of the 5 are real divergences; 2 are CommonMark-correct and 1 is ambiguous markup. The suggested fix is now measured, not sketched. |
| 2 | defaultEnabled inert when browserNotifications arrives after first render |
Nice to have | stands — re-measured live (L4.enabled-after-late-defaultEnabled: false, L4.no-permission-request: 0). The L3 control re-confirms appName/iconUrl are re-read, so the knob is partly live. Documentation-precision issue, unchanged. |
Nothing worsened. No finding was declined by the author, so there is no declined-tradeoff row to re-price.
Central claim and A/B proof
Central claim. When a turn reaches a terminal state while the page is unfocused, the notification identifies the session (appName · session title), carries bounded plain-text excerpts of this turn's prompt (≤80 code points) and final main-assistant reply (≤120), omits partial replies and error details on failure, omits transport attachment tails and internal insight payloads, and a click dispatches navigation to the captured session on the owning provider's EventTarget only.
Oracle. The arguments actually reaching the window.Notification constructor (title, body, icon, tag) plus the qwen:open-session CustomEvent a click dispatches. The harness drives the real BrowserTurnNotifications provider, the real createTurnNotificationObserver, and the real getTurnNotificationContent/notification-text cleaner, rendered by React into jsdom. Nothing on the path under test is stubbed; only platform APIs jsdom lacks are provided, and Notification is the observation point.
Control. One byte-identical harness file (sha256 0e320fbf516aa3d4a7f68c86d66bc4d9e34db3bafdcd1dd76528851948b7ad31) is dropped into each tree and run with that tree's own vitest.config.ts. The arm is detected at runtime by whether the tree exports getTurnNotificationContent. Expectations are arm-specific and derived from the shipped design docs; base control cells assert the absence of session identity and excerpts, so base "failing" on the feature is encoded as a passing control assertion — which is why fail is 0 on both arms.
| arm | tree | assertions | notifications constructed |
|---|---|---|---|
| head (PR) | /__w/qwen-code/qwen-code @ 0239c4ee |
93 pass / 0 fail | 25 |
| base (control) | tmp/base-tree @ 0ef35351 |
61 pass / 0 fail | 27 |
observable_differences = 27/27. Every aligned position of the constructed-notification stream differs between the arms, and the stream lengths themselves differ (25 vs 27) because head suppresses two notifications base delivers. Witness: 01-ab-notification-boundary-head-vs-base.png; full per-assertion data logs/obs-head.json / logs/obs-base.json, raw vitest output logs/ab-head.log / logs/ab-base.log.
Representative cells (real strings, not paraphrases):
| # | head | base |
|---|---|---|
| 1 | QwenCode · Fix the login bug / status + Prompt: + Reply: / icon yes |
Qwen Code / status only / no icon |
| 2 | QwenCode · Use Map<string, number> here — generics intact |
Qwen Code, no prompt carried |
| 3 | Prompt: Is 3 < 5 and 7 > 2 correct? — comparisons intact |
no prompt carried |
| 5 | failed turn: Prompt: shown, no Reply:, no ENOENT |
status only |
| 14 | title 60 / prompt 80 / reply 120 code points, each ending … |
Qwen Code, 1 line |
| 17 | Acme Agent · Branded, icon = the CDN URL |
Qwen Code, no icon |
| 19 | QwenCode with no · separator |
Qwen Code |
The three flips (head suppresses what base delivers), each with its own passing assertion on both arms:
| flip | head | base |
|---|---|---|
mid_turn_message_injected then terminal |
S21.head.silent — 0 notifications |
S21.base.still-notifies — 1 |
| 1500 admissions, then two replayed terminals | S22.head.one-notification-only — evicted p1 suppressed, p1500 kept |
S22.base.both-notify — 2 |
active: false |
observer + settings + navigation contexts all undefined, no delivery |
observer attached, delivers (no such opt-out) |
Harness validity controls — two, both load-bearing.
S01.delivered-on-this-armrequires a notification on both arms. This is not decoration: my first draft produced 0 notifications on both arms, becausepermission()gates onwindow.isSecureContext, which jsdom leaves false. Every cell would have read "the PR changed nothing". The stub is now commented in the harness for exactly that reason.S21.control.sibling-still-deliversfires a second, never-injectedpromptIdafter asserting silence, and requires 1 notification. Without it,S21.head.silentandS24.head.*assert zero and would pass vacuously on a harness that can never deliver.
A third control is reported because it documents a measurement trap rather than a PR property: S26.diagnostic.instance-spy-does-not-intercept passes on both arms and proves that vi.spyOn(window.localStorage, 'getItem') does not intercept in jsdom (Storage is a Proxy). My first "unreadable storage" cell therefore silently measured a fresh site — where stored === null && defaultEnabled legitimately yields enabled: true — and reported it as a failure of the PR's documented behaviour. Making the storage access throw is what measures the real scenario; the assertion S26.unreadable-starts-off then passes.
Corrections to the previous round's finding 1
The previous round reported five shapes as the cleaner corrupting code. Adjudicating each against micromark, the CommonMark implementation this repo already ships (a reference oracle rather than my reading of the spec):
| input | cleaner output | CommonMark says | verdict |
|---|---|---|---|
compute 2 ** 3 ** 4 |
compute 2 3 4 |
literal compute 2 ** 3 ** 4, no <strong>/<em> |
diverges — real defect |
const a = 1; / const b = **bold**; |
const b = bold; |
<pre><code>…const b = **bold**;</code></pre> |
diverges — real defect |
why does ls **/*.ts **/*.js differ |
why does ls /*.ts /*.js differ |
**/<em>.ts **/</em>.js — CommonMark also parses emphasis here |
ambiguous markup, not a clean defect |
- 5 degrees below zero |
5 degrees below zero |
<ul><li>5 degrees below zero</li></ul> |
matches CommonMark — not a defect |
* 2 * 3 = 6 |
2 * 3 = 6 |
<ul><li>2 * 3 = 6</li></ul> |
matches CommonMark — not a defect |
So the finding is real but half the size round 1 reported. The two genuine divergences share one root cause: the cleaner strips a ** pair without requiring the delimiters to be flanking, and it protects ```/~~~ fences but not the equally-CommonMark four-space indented code block. That asymmetry is the sharpest form, and it is the same shape the previous round's own fence fix addressed one level up. The leading--/* rows should be dropped: stripping them is the correct plain-text rendering of a list item, and "fixing" them would make the cleaner diverge from CommonMark.
The suggested fix, measured
Round 1 explicitly declined to claim its sketch worked. I applied it to a scratch copy of client/notification-text.ts — flanking-only **/~~ stripping plus treating a ^ {4,}\S line as code — and drove it through the same harnesses:
| measurement | result |
|---|---|
| hostile fixtures | 3/3 fixed: compute 2 ** 3 ** 4, the shell glob, and the indented code block all come out literal |
| benign fixtures | 0/8 collateral — byte-identical, including The **bold** word… → The bold word… (genuine Markdown still stripped) and ```py\nx = 2 ** 3 ** 4\n``` → x = 2 ** 3 ** 4 |
notification-text.test.ts |
5/5 green unpatched AND 5/5 green patched |
| scaling ladder slowest rung | 0.76 ms → 1.22 ms, no rung over 1 s |
| restore | sha256 bd9cb629b041951bf8506b0ba361bcf158ffa827c9bb843ef320e76965af2bb6 verified identical after the run |
The suite being green on both sides is the finding, not reassurance: this axis is unpinned. The fixtures that would go red are exactly the three hostile rows above; they belong in notification-text.test.ts alongside any such fix. Witness 02-finding1-measured-then-fixed.png; raw logs/cleaner.json, logs/cleaner-patched.json, logs/suite-patched.log.
Severity stays Suggestion: the value is a transient notification excerpt, the transcript and stored messages are untouched, shared storage holds only hashes, and base shows no prompt text at all so this is not a regression.
Finding 2 (Nice to have, stands) — defaultEnabled is inert when the prop arrives after first render
WebShellWithProviders always renders BrowserTurnNotifications with active={browserNotifications !== undefined}, so the provider mounts on the host's first render and const [defaultEnabled] = useState(options?.defaultEnabled ?? false) freezes the value there. Re-measured live:
L4.enabled-before false
L4.enabled-after-late-defaultEnabled false <- host passes { defaultEnabled: true } later
L4.no-permission-request 0
A host that loads its own configuration asynchronously gets a silent "off", with no prompt and no diagnostic. The knob is not wholly dead — the L3 control in the same test re-renders with { appName: 'Late Brand', iconUrl: 'https://cdn/x.png' } and both are honoured (L3.late-appName-is-re-read, L3.late-iconUrl-is-re-read). This matches the PR description ("Notification defaults are read at mount"); the ambiguous sentence is the design doc's "The initial default is read at mount even if integration is initially omitted". Suggest rewording to say defaultEnabled must be present on the first render. Evidence: logs/obs-head.json → L4/L3.
Probed and did not hold
Bounding the findings mattered more than widening them, so I drove the scarier readings and most do not hold:
- The published package is not missing its icon.
npm pack --dry-runyields 388 files with 0 PNGs and 0dist/assets/entries, whilepackage.jsonfilesis["dist/*.js", "dist/types"]and the new asset lands indist/assets/qwen-code-notification-BRVagufF.png. That looks like a shipped-broken-icon defect. It is not: the library build inlines the icon as an 18,858-characterdata:image/png;base64URL insidedist/index.js, and decoding it gives 14,125 bytes, sha256ae2194d9e57bafc1…, byte-identical toclient/assets/qwen-code-notification.png.dist/transcript.jsanddist/daemon-react-sdk.jscarry no PNG data URL, i.e. the payload is not duplicated across entries. The emitted file serves the app build (index.html), which is not what npm consumers import. - A real browser accepts and decodes both delivery forms. In real Chromium,
fetch+createImageBitmapon the inlined data URL →image/png, 14,125 bytes, 128×128; on the dev-served/assets/qwen-code-notification.png→ 200,image/png, 14,125 bytes, 128×128. Same bytes, both decodable. Witness03-real-chromium-icon-and-env-control.png. - …but a status code alone would have lied. The wrong spelling
/client/assets/qwen-code-notification.pngreturns 200text/html, 15,949 bytes — vite's SPA fallback, becausevite.config.tssetsroot: 'client'. An asset check that asserts onlystatus === 200reports a working icon for a URL that serves the app shell. Both spellings are asserted in the harness so the trap is pinned, not just avoided. - No superlinear blowup on outsider-authored text. Re-ran the 10-shape × 5-rung ladder (256 / 1024 / 2048 / 4096 / 20000 UTF-16 units) at this head: slowest rung 0.76 ms, no rung over 1 s, and every rung's output stayed within the 120-code-point bound (50 assertions). The structural reason still holds —
text.slice(0, MAX_NOTIFICATION_SOURCE_LENGTH)runs before any regex, so no regex ever sees more than 4096 units.link-soupis the most expensive shape at 0.11 → 0.76 → 0.30 → 0.62 → 0.66 ms: flat past the cap. - The list-nested fence sibling is still closed, and fence bookkeeping is still CommonMark-correct.
```pyinside a list item protectsx = 2 ** 3 ** 4;#include <stdio.h>and192.168.1.1 is the hostsurvive verbatim because the list/heading rule requires\s+after the marker. - No cross-instance navigation leak.
S23: a click dispatches 1 event on the owning provider'sEventTargetand 0 onwindow, carrying{sessionId, sessionContext:{kind,cwd}}, and closes the notification. Base has no navigation target at all and dispatches nothing. Two shells on one page cannot both navigate. - No content in shared storage, through the real
navigator.lockspath.S25: after a delivery,localStorageholds exactly oneqwen-code-turn:<64 hex>tag — no title, prompt, reply, cwd or/repo/anywhere — with a positive control asserting the claim store really was written. - Astral and control-character handling is exact. 100 emoji → 80 code points / 159 UTF-16 units, i.e.
79 whole emoji + '…'with no lone surrogate. NUL, BEL, RLO, ZWSP and LRI collapse toa b c d e fwith zero control or bidi characters surviving in the body. - Bounds are enforced in code points, not UTF-16 units. 200/200/300-character inputs → title 60, prompt 80, reply 120 code points, each ending
….
Targeted gates
| gate | command | result |
|---|---|---|
packages/web-shell unit tests @ head |
npx vitest run --config vitest.config.ts |
303 files / 7213 tests passed, exit 0 (logs/gate-web-shell-head.log) — reproduces round 1's counts exactly |
| A/B harness, head arm | npx vitest run … zz-verify-ab |
93 pass / 0 fail |
| A/B harness, base arm | same file in tmp/base-tree |
61 pass / 0 fail |
| Real Chromium probes | npx playwright test --project=chromium |
3 tests passed across two specs (logs/browser.log, logs/browser2.log) |
notification-text.test.ts patched / unpatched |
npx vitest run … notification-text |
5/5 passed both ways |
Control cleanliness (asserted, not assumed). From inside the base worktree: readlink -f node_modules/@qwen-code/sdk → /__w/qwen-code/qwen-code/packages/sdk-typescript and @qwen-code/web-shell → /__w/qwen-code/qwen-code/packages/web-shell, i.e. the internal links point into the head tree — the trap that makes a naive base control quietly load changed head code. It is safe here because the PR changes zero paths outside packages/web-shell/ and docs/ (git diff --name-only HEAD^1..HEAD filtered → 0), package.json and package-lock.json are untouched, and everything the harness exercises resolves inside each tree by relative import and by vitest.config.ts's own aliases. The arms are not accidentally identical: browser-turn-notifications.tsx sha256 5bf875a7… base vs 30aa9dbc… head, and notification-text.ts is absent on base. All of this is printed in the header of logs/run-ab.log.
Not covered
- Live native notification delivery in a real browser. This is the round-1 gap I tried to close and could only half-close. Headless Chromium in this container reports
Notification.permission === "denied"— denied, not default — andcontext.grantPermissions(['notifications'], { origin })does not change it;Browser.setPermissionover CDP is rejected withInvalid parameters. Proved environmental, not a PR defect: a control page that imports no web-shell module and loads no built library measures the samedenied, whileisSecureContext,navigator.locksandcrypto.subtleare all present (logs/browser2.log→ENV_CONTROL). Since the spec requiresnew Notification(...)to throw unless permission is granted, native banner delivery, OS icon placement and click-to-focus cannot be observed here. What the browser round did establish: the icon resolves and decodes in a real browser in both delivery forms, and the delivery gate is reachable only with a genuine blur — headless Chromium reportsdocument.hasFocus() === truefor the front page, so the second-tab blur is load-bearing rather than decoration. The PR description says the same of its own evidence, so this remains a shared limit. - No end-to-end daemon turn. I drove the real app in Chromium against the repo's
mockDaemonHTTP/SSE peer far enough to measure the platform facts above, but not through a completed notification. The transcript blocks andDaemonEvents in the jsdom A/B are hand-built to the SDK's real shapes. So this reproduces the wire shape of a terminal turn, not the model-side and daemon-side path that produces it. Noqwen serverun. - Typecheck and lint not re-run.
tsc -p tsconfig.json --noEmitand eslint/prettier overpackages/web-shellwere carried forward under the identical-input-closure shortcut rather than re-executed: the tree hash proves the sources,tsconfig.jsonand the lockfile are unchanged, and round 1 recorded exit 0 on a verified-clean tree. I therefore make no new claim about them, and I did not plant a violation to prove either gate live this round. - Per-commit attribution is out of reach.
git rev-parse --is-shallow-repositoryistrueand the checkout is depth 2, so only the merge commit, the base tip and the PR head exist;$QWEN_VERIFY_CONTEXTlists 8 commits. I verified the aggregateHEAD^1..HEADdiff only. The description's9056cebreproduction of the angle-bracket regression is not locally reachable; I proved instead that the fix holds at head, thatMap<string, number>and3 < 5 … 7 > 2both survive (cells 2 and 3), and that reverting the HTML-guessing is caught — the same conclusion by a different route. - Mutation matrix not re-run. Round 1 reported 15/15 killed with 0 survivors and every positive control landed in the mutated file. Under the identical-tree shortcut that result is carried forward, not re-measured; this round's mutation work went to the reverse direction (the candidate fix), which round 1 named as its gap.
- App-side navigation validation beyond the two round-1 mutations. The design doc lists further rejection rules for explicit contexts (malformed context,
workspaceCwdconflicting with astandalone/livecontext). I exercised the locked-workspace and healthy-current-session paths only via round 1's carried-forward M7/M8 plus the 879-testApp.test.tsxsuite inside the 7213-test gate. - One behavioural question I raised and did not resolve.
handleOpenSessionFromOverviewnow callsnotifyControlledSplitClose()+clearSplitSessions()unconditionally whenmainView === 'split', and that handler is shared withqwen-session://markdown links, which previously went straight toloadSidebarSession. Whether this changes markdown-link behaviour in split view depends on whetherloadSidebarSessionalready tore the split down — I did not establish that, so I am flagging it as an open question for a reviewer rather than reporting it as a finding. - No trial merge against current
main. The checkout is shallow, somaincould not be fetched.
Methodology
Environment: the CI verify job container (node:22-bookworm), Node v22.23.2, working tree at refs/pull/11447/merge (0239c4ee), with npm ci and npm run build already completed at head before this round started; neither was repeated. The base arm was a scratch worktree at tmp/base-tree pinned to HEAD^1 (0ef35351), needing no install of its own, and was removed with git worktree remove --force after the A/B cells were captured; git status --porcelain is empty (excluding tmp/) at the end of the round.
Four harnesses drove the code, each kept in this directory so a maintainer can rerun it verbatim. verify-ab.test.tsx (run by run-ab.sh, which also prints the control-identity and realpath evidence) is arm-agnostic and drives the real provider, observer and content extractor into jsdom, observing the window.Notification constructor and the click CustomEvent. verify-cleaner.test.ts exercises the shipped cleaner directly for the sibling sweep and the scaling ladder and writes logs/cleaner.json; the same file was run a second time against a scratch copy of notification-text.ts carrying the candidate fix, producing logs/cleaner-patched.json, after which the original was restored under a sha256 check. Two Playwright specs (zz-verify-browser.spec.ts, zz-verify-browser2.spec.ts) drove real Chromium against the vite dev server and the repo's mockDaemon HTTP/SSE peer; the notification recorder there is a pass-through that delegates to the real constructor and records whether it threw, so a browser-side rejection would be visible rather than swallowed by a stub. The CommonMark adjudication used micromark, a dependency this repo already ships, as a reference oracle rather than hand-written expectations. The published-library icon claim was checked by decoding the base64 payload out of dist/index.js and comparing bytes to the source PNG, and by npm pack --dry-run.
Raw per-cell logs live in logs/: run-ab.log (control identity), ab-head.log / ab-base.log, obs-head.json / obs-base.json (per-assertion results and every constructed notification), cleaner.log / cleaner.json, cleaner-patched.log / cleaner-patched.json, suite-patched.log, browser.log, browser2.log, gate-web-shell-head.log, summary-{ab,cleaner,browser}.txt.
Assertion counts (assertions.json, 229 total, 0 fail): A/B head 93, A/B base control cells 61, cleaner spec + ladder 57, real-Chromium icon and environmental control 10, real-Chromium asset-path probe 4, mutation-in-reverse gates 3, unit-test gate 1. Base-arm control cells count as passes because each asserts that base lacks the behaviour; fail counts only unexpected outcomes, and there were none. The one harness that failed for environmental reasons (live native delivery) was converted into a passing environmental control and its subject is listed under Not covered rather than counted as a failure.
Flakiness gate log
rounds=5 files=7 skipped=0
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/browser-turn-notifications.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/browser-turn-notifications.test.tsx
file packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/daemon/session/DaemonSessionProvider.test.tsx
file packages/web-shell/client/daemon/session/turn-notification-context.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/daemon/session/turn-notification-context.test.ts
file packages/web-shell/client/index.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/index.test.tsx
file packages/web-shell/client/main.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/main.test.tsx
file packages/web-shell/client/notification-text.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/notification-text.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/web-shell/client/App.test.tsx: PPPPP
packages/web-shell/client/browser-turn-notifications.test.tsx: PPPPP
packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: PPPPP
packages/web-shell/client/daemon/session/turn-notification-context.test.ts: PPPPP
packages/web-shell/client/index.test.tsx: PPPPP
packages/web-shell/client/main.test.tsx: PPPPP
packages/web-shell/client/notification-text.test.ts: 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/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/browser-turn-notifications.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/daemon/session/turn-notification-context.test.ts: P (exit 0)
round 1 · packages/web-shell/client/index.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/main.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/notification-text.test.ts: P (exit 0)
round 2 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/browser-turn-notifications.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/daemon/session/turn-notification-context.test.ts: P (exit 0)
round 2 · packages/web-shell/client/index.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/main.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/notification-text.test.ts: P (exit 0)
round 3 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/browser-turn-notifications.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/daemon/session/turn-notification-context.test.ts: P (exit 0)
round 3 · packages/web-shell/client/index.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/main.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/notification-text.test.ts: P (exit 0)
round 4 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/browser-turn-notifications.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/daemon/session/turn-notification-context.test.ts: P (exit 0)
round 4 · packages/web-shell/client/index.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/main.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/notification-text.test.ts: P (exit 0)
round 5 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/browser-turn-notifications.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/daemon/session/turn-notification-context.test.ts: P (exit 0)
round 5 · packages/web-shell/client/index.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/main.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/notification-text.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
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.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- App-wide re-render from the unmemoized notification settings context — already reported as R1-11 (comment 3969541283)
- default 'QwenCode' app name versus DEFAULT_BRAND_NAME — already reported as R1-9 (comment 3969541341)
- code-point cap splits grapheme clusters — already reported as R1-32 (comment 3969541332)
- default icon inlined into the published SDK entry — already reported as R1-10 (comment 3969541324)
- new designs never link the base design — already reported as R1-4 (comment 3969541255)
Unresolved, please confirm:
- [Critical] R2-1 at packages/web-shell/client/main.tsx:308 — every code-and-document limb I could trace is closed: the description no longer says "opt-in" or "Breaking changes / migration notes: none" but discloses built-in default-on in both languages…
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": end-to-end confirmation (execution, not static reading) that a background_notification_response block can land inside a terminal turn's promptId window — th…; "agent reverse-audit (round 2)": did not trace how many microtask ticks loadSidebarSession needs before it reaches mockSessionActions.loadSession , so I could not rule out that the four rej…; "agent reverse-audit (round 1)": whether activeSession.workspaceCwd can ever be '' in the { kind: 'workspace', cwd: activeSession.workspaceCwd } fallback at DaemonSessionProvider.tsx:233…; "agent reverse-audit (round 1)": whether a workspace-scoped TurnNotificationTarget can ever carry an empty cwd ( remember reads owner.workspaceCwd , and App.tsx:13290-13299 drops the clic…; "agent reverse-audit (round 2)": confirming whether the SDK's live session.events() iterator can deliver session_metadata_updated and the following turn_complete inside one macrotask — th…, and 1 more.
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Not reviewed: "agent reverse-audit (round 2)" — pointed at diff lines it never opened: it made tool calls, but none of them read the diff.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/web-shell/client/index.tsx:22 — [probe] the new public browserNotifications prop is a complete silent no-op inside an iframe, and neither its JSDoc nor the new README section states the top-level-window preconditionpackages/web-shell/README.md:48 — [probe] the durability guarantee omits the deliberate, test-pinned exception that a same-origin storage deletion re-applies the mount-time default over the user's explicit opt-outpackages/web-shell/client/App.tsx:13207 — [probe] three of the reveal shortcut's nine conditions have no test that would go red if the condition were deletedpackages/web-shell/client/browser-turn-notifications.test.tsx:197 — [probe] the test named for the no-persist guarantee stages a prop transition that cannot discriminate it, and never reads storage afterwardspackages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx:19816 — [probe] the it.each matrix cannot distinguish the documented resolved workspace cwd from a pass-through of the declared prop
Convergence: round 3 posted 11 inline comment(s), 10 of them reported for the first time; the previous round posted 9 (7 new). Findings keep coming back to the same files: packages/web-shell/client/browser-turn-notifications.tsx (findings in rounds 1, 2; 2 more now); docs/design/web-shell/web-shell-browser-notification-branding.md (findings in round 2; 1 more now); packages/web-shell/client/browser-turn-notifications.test.tsx (findings in round 2; 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. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 5 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：end-to-end confirmation (execution, not static reading) that a background_notification_response block can land inside a terminal turn's promptId window — th…；"agent reverse-audit (round 2)"：did not trace how many microtask ticks loadSidebarSession needs before it reaches mockSessionActions.loadSession , so I could not rule out that the four rej…；"agent reverse-audit (round 1)"：whether activeSession.workspaceCwd can ever be '' in the { kind: 'workspace', cwd: activeSession.workspaceCwd } fallback at DaemonSessionProvider.tsx:233…；"agent reverse-audit (round 1)"：whether a workspace-scoped TurnNotificationTarget can ever carry an empty cwd ( remember reads owner.workspaceCwd , and App.tsx:13290-13299 drops the clic…；"agent reverse-audit (round 2)"：confirming whether the SDK's live session.events() iterator can deliver session_metadata_updated and the following turn_complete inside one macrotask — th…，另有 1 条。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
未审查："agent reverse-audit (round 2)"——启动 prompt 为它指定了 diff 中的行，但它从未打开：有工具调用，却没有一次读取 diff。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 5 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 11 条行内评论，其中 10 条是首次提出；上一轮发布了 9 条（其中 7 条首次提出）。发现反复回到同一批文件：packages/web-shell/client/browser-turn-notifications.tsx（第 1、2 轮已出过发现，本轮又有 2 条）；docs/design/web-shell/web-shell-browser-notification-branding.md（第 2 轮已出过发现，本轮又有 1 条）；packages/web-shell/client/browser-turn-notifications.test.tsx（第 2 轮已出过发现，本轮又有 1 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.23.2)
| (_match, label: string) => | ||
| label.replace(/(`+)(.*?)\1/g, '$2'), | ||
| ) | ||
| .replace(/(\*\*|~~)(.*?)\1/g, '$2'), |
There was a problem hiding this comment.
[Critical] R1-1: [certifies-falsely] [new-surface] Still standing — re-reported under its original id, as the class-level finding for this surface rather than as a list of corners.
First, what is closed, because it is most of the original comment: no tag rule remains at all, so angle brackets, generics and JSX survive; fenced content is kept literally, so a fenced reply no longer empties the Reply line; the 4096-code-unit slice bounds the input before any scan, so the quadratic cost is gone; and the emphasis rule is now (**|~~) only, so __init__.py and python -m __main__ survive. Every input the original comment named now round-trips.
What still stands is the class. The structural fix did not land, and the surface still deletes literal user text at a corner nobody enumerated: the emphasis stripper ignores CommonMark delimiter-flanking, so ** or ~~ surrounded by whitespace is treated as formatting and its characters are removed. compute 2 ** 32 ** 2 please reaches the OS banner as compute 2 32 2 please. In CommonMark that is not emphasis — whitespace on both sides of a delimiter means it is not left- or right-flanking — so the on-screen markdown keeps every character while the notification drops four of them. It reaches the most visible line too: for an untitled session turn-notification-context.ts:233 derives the title from the prompt's first cleaned line, so the banner reads QwenCode · compute 2 32 2. And 1 ** 2 ** 3 ** 4 → 1 2 3 ** 4 shows the strip is not even self-consistent. Your own suite pins the fenced variant at notification-text.test.ts:16-18 but nothing covers the unfenced one, so this ships green.
I checked whether the design doc's declared approximation already covers this, and it does not. web-shell-browser-notification-details.md:15 scopes the cleaner to "common Markdown formatting and simple inline link destinations" and declares exactly one preservation carve-out — angle brackets — "because a plain-text notification must not guess whether it is code or markup". 2 ** 32 ** 2 is not Markdown formatting, so stripping it is not an unhandled construct left visible; it is deletion of literal user text, which is the same harm direction as the Use Map<string, number> → Use Map incident this module exists to prevent. The doc's own "must not guess" rationale argues for this finding rather than against it.
Witness:
PR cleaner vs marked.parseInline (the authority), then the candidate fix re-run:
"compute 2 ** 32 ** 2 please" -> PR "compute 2 32 2 please" authority "compute 2 ** 32 ** 2 please"
"use a ~~ b ~~ c here" -> PR "use a b c here" authority "use a ~~ b ~~ c here"
"x = 2 ** 3 ** 4" (unfenced) -> PR "x = 2 3 4" authority "x = 2 ** 3 ** 4"
"1 ** 2 ** 3 ** 4" -> PR "1 2 3 ** 4" authority "1 ** 2 ** 3 ** 4"
"**bold** and ~~strike~~" -> PR "bold and strike" authority "<strong>bold</strong> and <del>strike</del>"
FIX .replace(/(\*\*|~~)(\S(?:.*?\S)?)\1/g, '$2') -> all four corrupted cases flip to the authority output;
"**bold** and ~~strike~~" -> "bold and strike" unchanged; notification-text.test.ts 5/5 still green.
Closed directions re-measured at this commit: no /<[^>]*>/ rule exists; fenced ```html<br>``` keeps <br> and the
Reply line survives; '<'x1000000 / '['x1000000 / '`'x1000000 -> 0.65 / 1.22 / 1.09 ms with outLen <= 120;
'why is my __init__.py not loading' and 'python -m __main__' unchanged.
| .replace(/(\*\*|~~)(.*?)\1/g, '$2'), | |
| .replace(/(\*\*|~~)(\S(?:.*?\S)?)\1/g, '$2'), |
The one-liner above closes this corner. It does not close the class, and I am not asking you to reverse the decision you already made about a real parser — your reason for declining it holds, since a parser route would re-introduce exactly the deletion the original incident was about for <OldType> and JSX placeholders, which the shipped code now preserves. What the class still costs is that each new corner is found by a reviewer rather than by a test, and that this package now carries two hand-rolled cleaners with different corner decisions (MessageList.tsx:853-902 strips __bold__ and *italic*; this module deliberately preserves __init__), so the same message previews one way in the timeline and another in the notification. notificationExcerpt('abcdef', 0) returning 'abcde…' where MessageList.tsx:865 guards if (maxLength <= 0) return ''; is that divergence in miniature, and was a fix constraint in the original comment.
Fix constraint: packages/web-shell/client/notification-text.test.ts:16-18 pins notificationExcerpt('```py\nx = 2 ** 3 ** 4\n```', 120) === 'x = 2 ** 3 ** 4', and fenced content takes the fence ? raw branch at notification-text.ts:32-33, so it must keep bypassing the inline regexes.
Fix witness: please add expect(notificationExcerpt('compute 2 ** 32 ** 2', 120)).toBe('compute 2 ** 32 ** 2') to notification-text.test.ts next to expect(notificationExcerpt('**bold** and ~~strike~~', 120)).toBe('bold and strike'), then remove the flanking guard and confirm the first one goes red.
中文说明
仍然存在 —— 以原 id 重新报告，并且是作为这一类问题的整体发现，而不是再列一个角落。
先说已经关闭的部分，因为原评论的大部分内容都已修复：标签规则已经完全不存在，所以尖括号、泛型和 JSX 都能保留；围栏内容按字面保留，所以围栏回复不再把 Reply 行清空；4096 码元的切片在任何扫描之前就限制了输入，所以平方级代价消失；强调规则现在只有 (**|~~)，所以 __init__.py 和 python -m __main__ 都能保留。原评论点名的每一个输入现在都能原样往返。
仍然存在的是这一类问题本身。结构性修复没有落地，而这个表面仍会在一个没有人枚举过的角落删除用户的字面文本：强调符号剥除规则忽略了 CommonMark 的 delimiter-flanking 规则，所以两侧带空白的 ** 或 ~~ 被当成格式处理，字符被删掉。compute 2 ** 32 ** 2 please 送到系统横幅时变成 compute 2 32 2 please。在 CommonMark 中这并不是强调——分隔符两侧都有空白意味着它既不是 left-flanking 也不是 right-flanking——所以页面上的 markdown 保留了每一个字符，而通知丢了四个。它也影响最显眼的那一行：会话没有标题时 turn-notification-context.ts:233 用提问的首个清洗行推导标题，于是横幅显示 QwenCode · compute 2 32 2。而 1 ** 2 ** 3 ** 4 → 1 2 3 ** 4 说明这条剥除规则连自身都不一致。你们自己的测试在 notification-text.test.ts:16-18 锁定了围栏内的变体，但没有任何用例覆盖非围栏的情况，所以这个角落是绿灯发布的。
我核对过设计文档声明的近似范围是否已经覆盖这一点，结论是没有。web-shell-browser-notification-details.md:15 把清洗范围限定为“常见 Markdown 格式和简单行内链接地址”，并且只声明了一处保留例外——尖括号——理由是“纯文本通知不能猜测它是代码还是标记”。2 ** 32 ** 2 不是 Markdown 格式，所以剥除它不是“未处理的构造被原样留下”，而是删除了用户的字面文本，与本模块存在的原因（Use Map<string, number> 被吃成 Use Map）是同一个损害方向。文档自己的“不能猜测”理由支持这条发现，而不是反驳它。
上方的一行修复能关闭这个角落，但关闭不了这一类问题。我不是在要求你们推翻已经做出的“不引入真实解析器”的决定——你们拒绝的理由是成立的，因为走解析器路线会为 <OldType> 和 JSX 这类占位符重新引入当初那次事故正是关于的删除行为，而现在的代码保留它们。这一类问题仍然存在的代价是：每一个新角落都由审查者而不是测试发现；并且本包现在有两套自行实现的清洗器、角落决策不同（MessageList.tsx:853-902 会剥掉 __bold__ 和 *italic*，本模块则有意保留 __init__），所以同一条消息在时间线里预览成一种样子、在通知里是另一种样子。notificationExcerpt('abcdef', 0) 返回 'abcde…'，而 MessageList.tsx:865 有 if (maxLength <= 0) return ''; 保护——这就是同一分歧的缩影，也是原评论中的修复约束之一。
修复约束：packages/web-shell/client/notification-text.test.ts:16-18 锁定了 notificationExcerpt('```py\nx = 2 ** 3 ** 4\n```', 120) === 'x = 2 ** 3 ** 4'，围栏内容走 notification-text.ts:32-33 的 fence ? raw 分支，必须继续绕过行内正则。
修复验收：请在 notification-text.test.ts 中补 expect(notificationExcerpt('compute 2 ** 32 ** 2', 120)).toBe('compute 2 ** 32 ** 2')，与 expect(notificationExcerpt('**bold** and ~~strike~~', 120)).toBe('bold and strike') 并列，然后移除 flanking 保护并确认第一条变红。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| ) | ||
| .join(''); | ||
| const plain = content | ||
| .replace(/[\p{Cc}\u200B\u200E\u200F\u202A-\u202E\u2066-\u2069]/gu, ' ') |
There was a problem hiding this comment.
[Critical] R3-1: [certifies-falsely] [new-surface] The new invisible-character filter hand-enumerates the bidi controls and omits U+061C ARABIC LETTER MARK, so one member of Unicode's closed Bidi_Control set survives into the OS notification while every one of its siblings is stripped.
\p{Bidi_Control} has exactly twelve members. This class covers eleven of them. The text reaching this filter is agent- and tool-influenced — a fetched page or an MCP tool result can carry it — and it is rendered by the OS banner, where an invisible strong-directional mark shifts bidi resolution of the adjacent neutrals (digits, /, .) so the displayed excerpt can read differently from the logical text. That is the trojan-source spoofing this filter exists to stop, and the sibling RLO case is stripped and is exactly what browser-turn-notifications.test.tsx:415/434 pins as "removing invisible controls" — so the module presents its output as bidi-clean while one directional control passes. U+2060 WORD JOINER and U+00AD SOFT HYPHEN survive as well, and a full BMP sweep found 30 \p{Cf} code points surviving, including the directional prefixes U+0600–U+0605, U+06DD, U+070F, U+0890/0891, U+08E2 and the interlinear annotations U+FFF9–U+FFFB.
This repository already closes the class everywhere else. All five other production bidi sanitizers in this tree include \u061c; this new module is the only one that does not.
Witness:
Authority set-difference sweep, oracle = V8's own Unicode tables, run against the shipped function:
AUTHORITY \p{Bidi_Control} size = 12 -> U+061C U+200E U+200F U+202A U+202B U+202C U+202D U+202E U+2066 U+2067 U+2068 U+2069
STRIPPED by the shipped filter (11): U+200E U+200F U+202A U+202B U+202C U+202D U+202E U+2066 U+2067 U+2068 U+2069
SURVIVE the shipped filter (1): U+061C
ALM U+061C: input=…U+0064 U+061C U+0020… -> output=…U+0064 U+061C U+0020… JSON: "Build passed"
RLO U+202E: input=…U+0064 U+202E U+0020… -> output=…U+0064 U+0020… JSON: "Build passed"
Also surviving: U+2060 -> "Build passed"; U+00AD -> "Build passed".
Repo-convention sweep — every other production bidi sanitizer here includes \u061c:
web-shell/client/hooks/useAtMentionSources.ts:135 cli/src/runtime/scheduled-task-run.ts:23
cli/src/serve/create-sub-session.ts:177 cli/src/serve/routes/scheduled-tasks.ts:155
core/src/ipc/peer-envelope.ts:42
| .replace(/[\p{Cc}\u200B\u200E\u200F\u202A-\u202E\u2066-\u2069]/gu, ' ') | |
| .replace(/[\p{Cc}\p{Bidi_Control}\u200B]/gu, ' ') |
Closing the class beats extending the list: \p{Bidi_Control} is supported under the u flag and was verified to match exactly those twelve code points, and to not match U+200B/200C/200D/00AD/2060. If you also want the non-directional invisibles gone, use \p{Cf} with U+200C and U+200D excluded; the minimum viable change is adding \u061C to the existing class.
Fix constraint: the current set deliberately leaves U+200C and U+200D alone so emoji ZWJ sequences pass through intact — verified, notificationExcerpt('\u{1F468}\u200D\u{1F469}\u200D\u{1F467} family', 60) returns '👨👩👧 family' — and packages/web-shell/client/hooks/useAtMentionSources.ts:135 enumerates for the same reason, so a blanket \p{Cf} would split those sequences and must exclude U+200C/U+200D.
Fix witness: please add expect(notificationExcerpt('Build\u061C passed', 60)).toBe('Build passed') to notification-text.test.ts, mirroring the shipped U+202E coverage — it is red against the current character class and green after; remove the fix and confirm it reds again.
中文说明
新增的不可见字符过滤器手工枚举了 bidi 控制字符，漏掉了 U+061C ARABIC LETTER MARK，因此 Unicode 封闭集合 Bidi_Control 中的一个成员会原样进入系统通知，而它的其余同类全部被剥除。
\p{Bidi_Control} 恰好有十二个成员，这个字符类覆盖了其中十一个。到达这个过滤器的文本受智能体和工具影响（抓取的网页或 MCP 工具结果都可能携带），并由系统横幅渲染；一个存活的不可见强方向标记会改变相邻中性字符（数字、/、.）的 bidi 解析，使显示的摘录与逻辑文本读起来不同。这正是该过滤器要阻止的 trojan-source 欺骗，而同类中的 RLO 已被剥除、也正是 browser-turn-notifications.test.tsx:415/434 以“移除不可见控制字符”为名锁定的用例——所以本模块对外呈现的输出是 bidi 干净的，而有一个方向控制符会通过。U+2060 WORD JOINER 与 U+00AD SOFT HYPHEN 同样存活；一次完整 BMP 扫描发现 30 个 \p{Cf} 码点存活，包括方向前缀 U+0600–U+0605、U+06DD、U+070F、U+0890/0891、U+08E2 以及行间注音 U+FFF9–U+FFFB。
本仓库在其他所有地方都已经关闭了这一类问题：树中另外五个生产环境的 bidi 清洗器都包含 \u061c，只有这个新模块没有。
上方修复建议是关闭整类问题而不是延长列表：\p{Bidi_Control} 在 u 标志下可用，经核实恰好匹配这十二个码点，且不匹配 U+200B/200C/200D/00AD/2060。如果还想清除非方向性的不可见字符，可用 \p{Cf} 但排除 U+200C 与 U+200D；最小改动是把 \u061C 加进现有字符类。
修复约束：当前集合有意保留 U+200C 与 U+200D，使 emoji ZWJ 序列完整通过——已核实 notificationExcerpt('\u{1F468}\u200D\u{1F469}\u200D\u{1F467} family', 60) 返回 '👨👩👧 family'——并且 packages/web-shell/client/hooks/useAtMentionSources.ts:135 出于同样原因采用枚举，因此无差别的 \p{Cf} 会拆开这些序列，必须排除 U+200C/U+200D。
修复验收：请在 notification-text.test.ts 中补 expect(notificationExcerpt('Build\u061C passed', 60)).toBe('Build passed')，与已有的 U+202E 覆盖对称——它对当前字符类是红的、修复后是绿的；移除修复后请确认它再次变红。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| expect(capture.settings!.persistent).toBe(false); | ||
| await act(() => capture.settings!.setEnabled(true)); |
There was a problem hiding this comment.
[Suggestion] R3-2: This new unreadable-storage test asserts only the preference state and never attaches or settles, so the one path that runs in every modern browser — claims read and write with navigator.locks present while storage access throws — stays untested.
beforeEach stubs navigator as { locks: undefined } (line 107), so the pre-existing test that blocks only setItem takes the deliver(false) branch and never reaches the claims block, while the Web Locks test has working storage. The cross product locks-present ∧ storage-throws is zero tests in this file. That matters because the claims read is a JSON.parse(window.localStorage.getItem(CLAIMS_STORAGE_KEY) ?? '[]') inside a try at browser-turn-notifications.tsx:229-245: if that read or the claims setItem were ever moved outside its try, the throw would escape deliver, be rethrown by the attempted-flag branch at :293-297, and land in void show().catch(() => setError(true)). A user with blocked storage who explicitly enabled notifications would then receive no turn-complete alert at all and would see a stuck error state — with this whole suite still green. This is the exact population main.tsx:308's defaultEnabled: true makes common.
Witness:
A/B probe in an isolated copy, same harness, both arms run with -t 'PROBE':
PRISTINE : PROBE-RESULT delivered=1 error=false
MUTANT : PROBE-RESULT delivered=0 error=true (claims getItem moved outside its try)
MUTANT vs the five suites that touch this component:
Test Files 5 passed (5)
Tests 414 passed (414) <- the mutant survives everything
Coverage sweep of the test file (oracle = the file itself):
vi.stubGlobal('navigator', …) at exactly 3 lines: 107 (beforeEach, {locks: undefined}), 356, 721
storage-throw sites: 214/217 (this test) and 742 (blocks only setItem, inherits locks:undefined -> deliver(false))
locks-present AND storage-throws = 0 tests
Extend this test after the enable step: stub navigator with a pass-through locks.request the way the Web Locks test does, call attach(capture), await settle(capture), and assert expect(notifications).toHaveLength(1) alongside the existing enabled and persistent assertions.
Fix constraint: beforeEach already stubs vi.stubGlobal('navigator', { locks: undefined }) at line 107 and afterEach calls vi.unstubAllGlobals(), so the locks stub has to be installed inside the test itself after beforeEach; delivery also needs the unfocused gate, which beforeEach already fixes via vi.spyOn(document, 'hasFocus').mockReturnValue(false).
Fix witness: with that delivery assertion added, remove the try { … } catch { } around the claims read and write at browser-turn-notifications.tsx:229-245 and confirm the assertion goes red.
中文说明
这个新增的“存储不可读”测试只断言了偏好状态，从未 attach 或 settle，因此在现代浏览器中一定会走的那条路径——navigator.locks 存在而存储访问抛错时的 claims 读写——仍然没有测试覆盖。
beforeEach 把 navigator 打桩为 { locks: undefined }（第 107 行），所以既有的“只阻塞 setItem”测试会走 deliver(false) 分支、根本到不了 claims 代码块；而 Web Locks 那个测试的存储是正常的。本文件中“locks 存在 ∧ 存储抛错”的交叉组合是 0 个测试。这一点重要，因为 claims 读取是 browser-turn-notifications.tsx:229-245 中位于 try 内的 JSON.parse(window.localStorage.getItem(CLAIMS_STORAGE_KEY) ?? '[]')：如果这个读取或 claims 的 setItem 被移到 try 之外，异常就会逃出 deliver，被 :293-297 的 attempted 分支重新抛出，最终落到 void show().catch(() => setError(true))。那样一来，存储被阻塞但已显式开启通知的用户将完全收不到回合完成提醒，并看到一个卡住的错误状态——而整个测试套件仍然是绿的。main.tsx:308 的 defaultEnabled: true 正是让这类人群变得常见的原因。
建议在这个测试的开启步骤之后扩展：像 Web Locks 测试那样给 navigator 打一个透传的 locks.request 桩，调用 attach(capture)、await settle(capture)，并在已有的 enabled 与 persistent 断言之外加上 expect(notifications).toHaveLength(1)。
修复约束：beforeEach 已在第 107 行执行 vi.stubGlobal('navigator', { locks: undefined })，afterEach 调用 vi.unstubAllGlobals()，所以 locks 桩必须在测试内部、beforeEach 之后安装；送达还需要失焦门禁，而 beforeEach 已通过 vi.spyOn(document, 'hasFocus').mockReturnValue(false) 固定。
修复验收：加上该送达断言后，移除 browser-turn-notifications.tsx:229-245 中包住 claims 读写的 try { … } catch { }，确认该断言变红。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| it('toggles browser notifications without restarting the session stream', async () => { | ||
| vi.stubGlobal('isSecureContext', true); | ||
| vi.stubGlobal('crypto', webcrypto); | ||
| vi.spyOn(document, 'hasFocus').mockReturnValue(false); |
There was a problem hiding this comment.
[Suggestion] R3-3: This new toggle test installs a document.hasFocus spy it never restores, and cleans the notification-preference key only as its final statement with no finally, so both leak into every test that runs after it in this file.
Two separate leaks, and they are not equally serious. The storage half is the one that actually fires: any failing assertion above the last statement — for example expect(notifications).not.toHaveBeenCalled() — leaves qwen-code-web-shell-browser-notifications = 'true' in jsdom storage for the rest of the run, because this file's beforeEach (649-654) only calls sdkMocks.reset(). The next test appended here that renders BrowserTurnNotifications then inherits an opted-in preference it never established, and passes or fails for reasons invisible in its own body. This PR appended a test at exactly that position, which is the natural home for provider-level notification tests, so the file will keep growing this way.
The spy half is hygiene only, and I want to correct the reason rather than let a wrong one stand: jsdom's hasFocus() is Boolean(this._lastFocusedElement) and measures false by default here, so the leaked spy does not change the value the canShow() gate reads. What it does leave behind is vi.isMockFunction(document.hasFocus) === true for the remainder of the file, so a later test's call-count assertion, or a vi.mocked(document.hasFocus) that should have thrown, is silently polluted.
Witness:
Canary probes in an isolated copy (CANARY2 appended immediately after the toggle test):
RUN A pristine : x CANARY2 AssertionError: expected true to be false (spy leaked)
RUN B + vi.restoreAllMocks() : ✓ 2 passed | 323 skipped (flip: the fix removes the leak)
RUN D fix (a), FULL file : Tests 1 failed | 324 passed (325) (only the canary; 0 regressions)
RUN C simulated failing assert above localStorage.removeItem, no fix:
x toggles browser notifications… expected 'SIMULATED EARLIER FAILURE' to be 'x'
x CANARY3 storage clean… AssertionError: expected 'true' to be null
jsdom authority: node_modules/jsdom/lib/jsdom/living/nodes/Document-impl.js:344
hasFocus() { return Boolean(this._lastFocusedElement); }
CONTROL (canary alone, toggle test skipped, no spy installed): expected false to be true
Add vi.restoreAllMocks(); to this file's existing afterEach next to vi.unstubAllGlobals(); at line 667, matching what browser-turn-notifications.test.tsx:116 already does. That fixes the spy half only — the storage half also needs the removeItem moved into a finally around the test body, or a localStorage.clear() added to the shared beforeEach.
Fix constraint: the shared afterEach at 657-668 serves 300 it/it.each sites with zero beforeAll, and all ten vi.spyOn calls in the file (1901, 3478, 3507, 3541, 3568, 5145, 12863, 13130, 15212, 19922) are test-local — none is installed in either beforeEach — so no cross-test spy dependency exists for a global restore to break. Measured: 324/324 other tests still pass with it added.
Fix witness: nothing pins this today, so the fix needs its own pin — a test appended after toggles browser notifications without restarting the session stream asserting expect(document.hasFocus()).toBe(true) and expect(localStorage.getItem('qwen-code-web-shell-browser-notifications')).toBeNull() goes red with the spy unrestored or the cleanup unguarded, and green with both fixed.
中文说明
这个新增的开关测试安装了一个 document.hasFocus spy 却从不恢复，并且只把通知偏好键的清理放在最后一句、没有 finally，因此两者都会泄漏到本文件中之后运行的每一个测试。
这是两个独立的泄漏，严重程度不同。存储那一半是真正会触发的：最后一句之上的任何断言失败——例如 expect(notifications).not.toHaveBeenCalled()——都会让 qwen-code-web-shell-browser-notifications = 'true' 在余下的运行中留在 jsdom 存储里，因为本文件的 beforeEach（649-654）只调用 sdkMocks.reset()。之后追加到这里、并渲染 BrowserTurnNotifications 的测试就会继承一个它自己从未建立的已开启偏好，并因自身代码中看不到的原因通过或失败。本 PR 正是在这个位置追加了测试，而这里也是 provider 级通知测试的自然归属，所以文件会继续这样增长。
spy 那一半只是卫生问题，并且我要纠正理由而不是让错误的理由留下：jsdom 的 hasFocus() 是 Boolean(this._lastFocusedElement)，在此环境默认测得 false，所以泄漏的 spy 并不会改变 canShow() 门禁读到的值。它真正留下的是文件余下部分中 vi.isMockFunction(document.hasFocus) === true，于是后续测试的调用次数断言、或本应抛错的 vi.mocked(document.hasFocus) 会被静默污染。
建议在本文件已有的 afterEach 中、第 667 行 vi.unstubAllGlobals(); 旁加上 vi.restoreAllMocks();，与 browser-turn-notifications.test.tsx:116 已有的做法一致。这只修复 spy 那一半——存储那一半还需要把 removeItem 移进包住测试体的 finally，或在共享的 beforeEach 中加 localStorage.clear()。
修复约束：657-668 行的共享 afterEach 服务于 300 个 it/it.each 位置、且没有任何 beforeAll，文件中全部十处 vi.spyOn（1901、3478、3507、3541、3568、5145、12863、13130、15212、19922）都是测试内局部的，没有一处安装在任一 beforeEach 中，所以不存在会被全局恢复破坏的跨测试 spy 依赖。实测：加上后其余 324/324 个测试仍然通过。
修复验收：目前没有任何用例锁定这一点，所以修复需要自己的锁定——在 toggles browser notifications without restarting the session stream 之后追加一个测试，断言 expect(document.hasFocus()).toBe(true) 与 expect(localStorage.getItem('qwen-code-web-shell-browser-notifications')).toBeNull()；spy 未恢复或清理未加保护时它是红的，两者都修复后是绿的。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| MAX_NOTIFICATION_SOURCE_LENGTH, | ||
| notificationTextLines, | ||
| } from '../../notification-text.js'; | ||
| import { splitInsightSegments } from '../../adapters/transcriptToMessages.js'; |
There was a problem hiding this comment.
[Suggestion] R3-4: This is the first production import from client/adapters/** anywhere under client/daemon/**, and it pulls the interactive-UI transcript adapter plus its transitive value imports into the published daemon-react-sdk entry — an entry that exists so hosts can use the daemon React bindings without the UI layer. The measured cost is +40,348 bytes minified / +9.06 kB gzip, and nothing bounds that entry.
package.json publishes "./daemon-react-sdk": { "import": "./dist/daemon-react-sdk.js" }, built from client/daemon-react-sdk.ts, which exports DaemonSessionProvider from ./daemon/index.js. DaemonSessionProvider.tsx now imports getTurnNotificationContent from ./turn-notification-context.js, which imports splitInsightSegments from the adapter. A consumer that only wants useActions/useConnection now downloads the adapter's insight machinery and client/utils/todos.ts behind it.
The cost is also structural rather than only bytes: the daemon layer now depends upward on the UI adapter layer, so any future edit to transcriptToMessages.ts can change or break the SDK-only bundle. Before this diff, client/daemon/** escaped its own directory in exactly two production places (actions.ts:65 → utils/sessionErrors.js; turn-navigation-store.ts:23 and DaemonSessionProvider.tsx:146 → constants/sessions.js) and nothing reached adapters/, i18n or components/. And unlike the transcript entry there is no size bound to catch a regression here — build-artifact.test.ts:352's expect(js.length).toBeLessThan(1_300_000) applies to readTranscriptBundle() only.
Two things I checked and want to record as not part of this finding, because a reviewer could reasonably worry about them: the icon cannot land in the size-bounded transcript entry (data:image/png;base64 scores 0 in the SDK closure in both arms), and the layering direction is not violated the other way (adapters/types.ts:20 is a type-only import back).
Witness:
Same-procedure A/B, `vite build --config vite.lib.config.ts` (Vite 7.3.6) in an isolated copy, then the
transitive static-import closure of dist/daemon-react-sdk.js summed:
PR arm (intact) SDK closure TOTAL BYTES = 335353
333578 toolNames-BNY2hSpt.js (gzip 79.28 kB) + 1775 daemon-react-sdk.js
MUTANT arm (adapters/transcriptToMessages import stubbed locally) TOTAL = 295005
293230 toolNames-DHebZpn3.js (gzip 70.22 kB) + 1775 daemon-react-sdk.js
DELTA = +40348 bytes minified, +9.06 kB gzip
arm proof — literal probes over the closure: insight_ready 5 -> 0 todos marker 24 -> 7 data:image/png;base64 0 -> 0
Source newly reachable, for scale only (not the download cost): transcriptToMessages.ts 61174 + todos.ts 27172 +
toolClassification.ts 4809 = 93155 B.
Move splitInsightSegments and its InsightSegment type into a small dependency-free module (e.g. client/adapters/insightSegments.ts) and import it from both transcriptToMessages.ts and turn-notification-context.ts. If the boundary matters, add a daemon-react-sdk assertion to build-artifact.test.ts in the style of the existing "does not pull the editor stack into the transcript entry" check.
Fix constraint: packages/web-shell/client/adapters/transcriptToMessages.ts:533 — const insightSegments = splitInsightSegments(textBlock.text); is the in-module caller that must be repointed in the same change, or the renderer's insight segmentation silently diverges from the notification's.
中文说明
这是 client/daemon/** 下任何位置第一处来自 client/adapters/** 的生产代码 import，它把交互式 UI 的 transcript 适配器及其传递的值 import 一并拉进了已发布的 daemon-react-sdk 入口——而这个入口存在的目的，正是让宿主在不引入 UI 层的情况下使用 daemon 的 React 绑定。实测代价是 +40,348 字节（压缩后）/ +9.06 kB gzip，并且没有任何约束限制这个入口的体积。
package.json 发布 "./daemon-react-sdk": { "import": "./dist/daemon-react-sdk.js" }，由 client/daemon-react-sdk.ts 构建，后者从 ./daemon/index.js 导出 DaemonSessionProvider。DaemonSessionProvider.tsx 现在从 ./turn-notification-context.js 导入 getTurnNotificationContent，而后者从该适配器导入 splitInsightSegments。于是一个只想要 useActions/useConnection 的使用者，现在会连带下载适配器的 insight 机制和 client/utils/todos.ts。
代价也不只是字节数，而是结构性的：daemon 层现在向上依赖 UI 适配层，因此将来对 transcriptToMessages.ts 的任何修改都可能改变或破坏仅含 SDK 的产物。在本次改动之前，client/daemon/** 在生产代码中只有两处跨出自身目录（actions.ts:65 → utils/sessionErrors.js；turn-navigation-store.ts:23 与 DaemonSessionProvider.tsx:146 → constants/sessions.js），没有任何一处触及 adapters/、i18n 或 components/。而且与 transcript 入口不同，这里没有体积约束能捕获回归——build-artifact.test.ts:352 的 expect(js.length).toBeLessThan(1_300_000) 只作用于 readTranscriptBundle()。
有两点我核对过并明确记为不属于本发现，因为审查者有理由担心：图标不会进入受体积约束的 transcript 入口（两个分支中 SDK 闭包内 data:image/png;base64 都为 0），并且反方向的分层也没有被破坏（adapters/types.ts:20 是反向的纯类型 import）。
建议把 splitInsightSegments 及其 InsightSegment 类型移到一个无依赖的小模块（例如 client/adapters/insightSegments.ts），由 transcriptToMessages.ts 与 turn-notification-context.ts 共同导入。如果这个边界重要，可在 build-artifact.test.ts 中按现有“does not pull the editor stack into the transcript entry”检查的风格，补一条针对 daemon-react-sdk 的断言。
修复约束：packages/web-shell/client/adapters/transcriptToMessages.ts:533 的 const insightSegments = splitInsightSegments(textBlock.text); 是模块内调用点，必须在同一次改动中一起改指向，否则渲染器的 insight 分段会与通知的分段静默分歧。
— qwen3.8-max via Qwen Code /review (v0.23.2)
|
|
||
| ## Configuration and integration | ||
|
|
||
| Expose `browserNotifications?: WebShellBrowserNotificationsOptions` on `WebShellWithProviders` and its `StandaloneWebShell` alias. The options contain optional `appName` and `iconUrl` strings, plus `defaultEnabled?: boolean` (false by default). Missing or whitespace-only branding values fall back independently to QwenCode and the bundled PNG. The title remains application name · session title, or only the application name when there is no session title. Pass the trimmed icon URL directly to Notification.icon; no proxy, preload, authentication headers, or automatic retry is added. The browser fetches the image under its security policies; a failed custom image does not guarantee the default icon is shown. |
There was a problem hiding this comment.
[Suggestion] R3-6: This title-format clause contradicts the companion design added by this same PR and the shipped code, for the untitled-session case. Both language versions carry the identical error, so it is not a translation gap — the two twins disagree with details.md and with the code, not with each other.
This line says the application name appears alone when there is no session title. In reality an untitled session that has a prompt shows appName · <first visible line of the prompt>: turn-notification-context.ts:232-233 sets content.sessionTitle = notificationTextLines(content.promptText)[0] when !sessionTitle?.trim() (again at :391-393), and browser-turn-notifications.tsx:257 composes title ? appName · title : appName. App-name-alone occurs only when neither a title nor a prompt exists — which is what the companion details.md:9 says in both languages ("if neither exists, display QwenCode").
The audience for this document is a host integrator, and the cost is that they read the branding design, conclude an untitled session's OS banner shows only their application name in the title, and repeat that in their own integration or privacy-facing copy — while conversation content is in fact in the title. Two designs added in one change assigning different titles to the same input is the concrete defect; it does not need a documentation rule to stand.
Witness:
All four texts verbatim at HEAD:
branding.md:13 "The title remains application name · session title, or only the application name when there is no session title."
branding.zh-CN.md:13 「标题保持“应用名称 · 会话标题”，没有会话标题时只显示应用名称。」
details.md:9 "…If a new session has no title yet or its title has been cleared, use the first visible line of this
turn's request after removing common Markdown formatting; if neither exists, display QwenCode."
details.zh-CN.md:9 「…新会话标题尚未生成或已清空时，使用移除常见 Markdown 格式后的本轮问题首个可见行；两者均缺失时显示 QwenCode。」
Code: turn-notification-context.ts:232-233 and :391-393; browser-turn-notifications.tsx:257.
Probe through the real component, stored preference 'true', permission granted, unfocused:
PROBE-P7 untitledWithPrompt title="QwenCode · Fix the login bug"
PROBE-P7 neitherTitleNorPrompt title="QwenCode"
PROBE-P7 withSessionTitle title="QwenCode · Build result"
Align this sentence with details.md in both languages, e.g. EN: "The title remains application name · session title; when the session has no title, the session-title part falls back to the first visible line of this turn's prompt, and the application name is shown alone only when neither exists." zh: 「标题保持“应用名称 · 会话标题”；会话没有标题时，会话标题部分回退到本轮提问的首个可见行，两者均缺失时才只显示应用名称。」
Fix constraint: the rewritten sentence must describe the implemented fallback rather than restate the app-name-alone rule — content.sessionTitle = notificationTextLines(content.promptText)[0]; at packages/web-shell/client/daemon/session/turn-notification-context.ts:233, consumed by packages/web-shell/client/browser-turn-notifications.tsx:257 — and docs/design/README.md requires both language versions to be updated in the same change, so the .md and .zh-CN.md edits must land together.
中文说明
这条标题格式条款与本次同一个 PR 新增的姊妹设计文档以及实际代码相矛盾，矛盾点在“会话没有标题”这一情形。两个语言版本带有完全相同的错误，所以这不是翻译差异——两个语言版本是与 details.md 和代码不一致，而不是彼此不一致。
这一行说：没有会话标题时只显示应用名称。实际上，一个没有标题但有提问的会话显示的是 appName · <提问的首个可见行>：turn-notification-context.ts:232-233 在 !sessionTitle?.trim() 时设置 content.sessionTitle = notificationTextLines(content.promptText)[0]（:391-393 重复一次），browser-turn-notifications.tsx:257 组装 title ? appName · title : appName。只有在标题和提问都不存在时才只显示应用名称——这正是姊妹文档 details.md:9 在两种语言中所说的（“两者均缺失时显示 QwenCode”）。
本文档的读者是宿主接入方，代价是：他们读了品牌设计文档后认为未命名会话的系统横幅标题只显示自己的应用名，并把这一点写进自己的接入说明或面向隐私的文案中——而实际上标题里含有对话内容。同一次改动中新增的两份设计对同一输入给出不同标题，这本身就是具体缺陷，不需要依赖任何文档规范就能成立。
建议把这句话在两种语言中都与 details.md 对齐，例如中文：「标题保持“应用名称 · 会话标题”；会话没有标题时，会话标题部分回退到本轮提问的首个可见行，两者均缺失时才只显示应用名称。」
修复约束：改写后的句子必须描述已实现的回退行为，而不是重述“只显示应用名称”的规则——见 packages/web-shell/client/daemon/session/turn-notification-context.ts:233 的 content.sessionTitle = notificationTextLines(content.promptText)[0];，由 packages/web-shell/client/browser-turn-notifications.tsx:257 消费；并且 docs/design/README.md 要求两个语言版本在同一次改动中一起更新，所以 .md 与 .zh-CN.md 的修改必须同时落地。
— qwen3.8-max via Qwen Code /review (v0.23.2)
|
|
||
| ## Content and icon | ||
|
|
||
| The default title is QwenCode · session title; hosts can override the application name and icon through the [branding options](web-shell-browser-notification-branding.md). If a new session has no title yet or its title has been cleared, use the first visible line of this turn's request after removing common Markdown formatting; if neither exists, display QwenCode. The body retains the turn status and includes a plain-text excerpt from the last main-assistant reply for a completed turn, without another model call to summarize it. Extraction matches the terminal promptId exactly and excludes tools, thoughts, background agents, and internal insight messages. Missing text does not fall back to an older turn, and failures do not expose partial replies or error details. |
There was a problem hiding this comment.
[Suggestion] R3-7: Both language versions of this new design scope the reply excerpt to a completed turn, but the shipped code attaches it to every non-failed outcome — including the ended status the base design defines for other turn_complete stop reasons, canonically the token limit, which is a definitionally partial reply. Neither new design mentions the ended body anywhere.
A long task hits the output token cap; the daemon emits turn_complete with stopReason: 'max_tokens', which turn-notification-context.ts:399-406 maps to outcome: 'ended'. getTurnNotificationContent early-returns before the reply scan only for turn_error (:235), so responseText is still populated from the last main-assistant block, and browser-turn-notifications.tsx:251-254 blanks the excerpt only when turn.outcome === 'failed'. The banner therefore reads This turn has ended. Return to check the result. / Prompt: … / Reply: <first 120 code points of a cut-off sentence> — a truncated fragment published under a Reply: label, on a surface this same document says may appear on the lock screen.
One correction in your favour, because the finder's framing overstated it: the base design's outcome table constrains the status wording, and the shipped status honours it — the banner says "Return to check the result", not "completed". So this is not a turn being presented as a finished result. What survives is the concrete, checkable divergence: the sentence this PR adds scopes the excerpt to a completed turn, and the code ships it for a definitionally partial one.
The path is also unpinned, so whichever way you resolve it a later change can flip it silently: every body assertion in browser-turn-notifications.test.tsx (lines 403, 444, 495) drives stopReason: 'end_turn' or 'cancelled', and cancelled never reaches this code because :216 returns first.
Witness:
Probe driving the real BrowserTurnNotifications with stopReason 'max_tokens' (jsdom, FakeNotification,
storage 'true', hasFocus false), responseText a 123-char mid-sentence fragment:
INTACT (PR code):
PROBE[en] body= "This turn has ended. Return to check the result.\nPrompt: Run all seven migrations\nReply: I have applied the first two of the seven migrations and the schema now has the users and sessions tables, next I will …"
PROBE[zh-CN] body= "本轮已结束，请返回查看结果。\n提问：Run all seven migrations\n回复：I have applied the first two of the seven migrations and the schema now has the users and sessions tables, next I will …"
CANDIDATE FIX (browser-turn-notifications.tsx:251-254 -> excerpt only when outcome === 'completed'):
PROBE[en] body= "This turn has ended. Return to check the result.\nPrompt: Run all seven migrations"
-> browser-turn-notifications.test.tsx (27) + turn-notification-context.test.ts (24) = 52 passed, 0 failed
(no existing test moves, confirming the path is unpinned)
Doc text: grep -n "completed turn|完成时" over both files -> the only hits are line 9 of each.
grep -n "ended|已结束|max_tokens|token 上限|token limit" across all four new design docs -> zero hits.
'ended' is real and pinned: turn-notification-context.test.ts:19-32
it.each([['end_turn','completed'],['cancelled','cancelled'],['max_tokens','ended']])('classifies %s without implying task-wide success').
Resolve in one direction and keep both languages in step. Either narrow the code — browser-turn-notifications.tsx:251-254 becomes turn.outcome === 'completed' ? notificationExcerpt(turn.responseText ?? '', 120) : '', which I ran and it needs no test changes — or restate this line and its zh twin as covering a completed or otherwise ended (non-failed) turn, and list the ended body in both ## Validation scope / ## 验证范围 sections.
Fix constraint: docs/design/web-shell/web-shell-browser-turn-notifications.md:78 — 「| 其他合法 turn_complete stopReason | “本轮已结束，请返回查看”，不把 token 上限等停止原因说成任务成功。 |」 — the ended status text must stay distinct from success (browserNotifications.ended, i18n.tsx:3584 EN / :7009 ZH); and docs/design/README.md requires both language versions in the same change, so a doc-side fix must land in .md and .zh-CN.md together.
Fix witness: add a case to browser-turn-notifications.test.tsx driving turn_complete with stopReason: 'max_tokens' plus an assistant block. Under the code fix it asserts the body has no Reply: line and goes red if the guard reverts to turn.outcome === 'failed' ? ''; under the doc fix the same test must assert the Reply: line is present, so the newly documented behaviour is pinned rather than left as prose.
中文说明
这份新设计的两个语言版本都把回复摘录限定为已完成的回合，但实际代码对除 failed 之外的所有结果都附上摘录——包括基线设计为其他 turn_complete 停止原因（典型是 token 上限）定义的 ended 状态，而那种回复按定义就是被截断的。两份新设计文档在任何地方都没有提到 ended 的正文形态。
一个长任务触到输出 token 上限；daemon 发出带 stopReason: 'max_tokens' 的 turn_complete，turn-notification-context.ts:399-406 把它映射为 outcome: 'ended'。getTurnNotificationContent 只在 turn_error 时提前返回（:235），所以 responseText 仍会从最后一段主助手 block 中取出，而 browser-turn-notifications.tsx:251-254 只在 turn.outcome === 'failed' 时清空摘录。于是横幅显示 本轮已结束，请返回查看结果。 / 提问：… / 回复：<被截断句子的前 120 个码点>——一段被截断的碎片以 回复： 标签发布，而这份文档自己说这个表面可能出现在锁屏上。
有一处需要替你们纠正，因为原始发现的表述夸大了：基线设计的结局表约束的是状态文案，而实际状态文案是遵守的——横幅说的是“请返回查看结果”，不是“已完成”。所以这不是把一个回合呈现为已完成的结果。仍然成立的是那个具体、可核对的分歧：本 PR 新增的这句话把摘录限定为已完成回合，而代码对一个按定义被截断的回合也发送摘录。
这条路径也没有测试锁定，所以无论你们选哪个方向，之后的改动都可能静默翻转它：browser-turn-notifications.test.tsx 中所有正文断言（403、444、495 行）都使用 stopReason: 'end_turn' 或 'cancelled'，而 cancelled 因为 :216 提前返回根本到不了这段代码。
请择一方向解决，并保持两种语言同步。要么收窄代码——browser-turn-notifications.tsx:251-254 改为 turn.outcome === 'completed' ? notificationExcerpt(turn.responseText ?? '', 120) : ''，我已实测，无需改动任何测试；要么把这一行及其中文版改写为覆盖“已完成或以其他原因结束（非失败）”的回合，并在两个 ## Validation scope / ## 验证范围 小节中列出 ended 的正文形态。
修复约束：docs/design/web-shell/web-shell-browser-turn-notifications.md:78 的 「| 其他合法 turn_complete stopReason | “本轮已结束，请返回查看”，不把 token 上限等停止原因说成任务成功。 |」——ended 的状态文案必须与成功保持区分（browserNotifications.ended，i18n.tsx:3584 英文 / :7009 中文）；并且 docs/design/README.md 要求两个语言版本在同一次改动中提交，所以文档侧修复必须 .md 与 .zh-CN.md 同时落地。
修复验收：在 browser-turn-notifications.test.tsx 中补一个用例，驱动带 stopReason: 'max_tokens' 的 turn_complete 并附一个 assistant block。走代码修复时它断言正文没有 回复： 行，且把守卫改回 turn.outcome === 'failed' ? '' 后变红；走文档修复时同一个测试必须断言 回复： 行存在，使新记录的行为被锁定而不是只停留在文字上。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| const request = version.current; | ||
| const canShow = () => | ||
| mounted.current && | ||
| activeRef.current && |
There was a problem hiding this comment.
[Suggestion] R3-8: The activeRef.current clause you added to canShow() is load-bearing and has no test. It is the only thing that suppresses an already-in-flight delivery, and every suite either keeps active true through delivery or blocks the notify upstream at the observer — so deleting the clause leaves all of them green.
The two existing "inactive" tests cannot reach it. In DaemonSessionProvider.test.tsx:19918-19993 and index.test.tsx:203-212, active=false makes TurnNotificationContext's value undefined, so useTurnNotificationBinding's observerRef.current?.observe(...) never calls notifyRef.current at all and delivery stops before canShow() runs. The clause only fires in the async gap inside show(): a backgrounded tab's turn completes while active is true, show() suspends at await crypto.subtle.digest(...) or at navigator.locks.request(CLAIMS_STORAGE_KEY, ...) — which blocks for as long as another same-origin tab holds the claims lock, so this is a real cross-tab window and not only a host-prop-change one — the host re-renders without browserNotifications, and deliver() then resumes. With the clause removed, mounted.current is still true, enabledRef.current still true and permission still granted, so a Notification is constructed and shown for an integration the host just turned off. And because TurnNotificationNavigationContext is now undefined, App has detached its qwen:open-session listener, so clicking that notification only closes it.
Witness:
Mutation, run in an isolated copy. Baseline = the six suites that touch turn-notification-context:
BASELINE (intact) : 6 files / Tests 1293 passed (1293) EXIT=0
MUTANT (`activeRef.current &&` deleted) : 6 files / Tests 1293 passed (1293) EXIT=0 <- nothing goes red
Probe opening the window deterministically — park delivery inside show() on the claims lock, flip active to
false on the SAME mounted root, then release the gate:
INTACT : PROBE r3f suspended: notifications= 0 / observer after flip= undefined / delivered: notifications= 0 []
MUTANT : PROBE r3f suspended: notifications= 0 / observer after flip= undefined / delivered: notifications= 1
[["QwenCode · Task","This turn has completed.\nReply: Done"]]
Reachability claim reproduced: `git grep "active={"` at HEAD -> the only hit is DaemonSessionProvider.test.tsx:19964.
Add a case that suspends delivery and flips active inside the suspension: stub navigator.locks.request (or crypto.subtle.digest) with a deferred, fire a turn_complete while active={true}, re-render the same root with active={false}, resolve the deferred, and assert the Notification spy was not called — then assert a subsequent turn with active={true} still delivers. The probe above is a working skeleton for it.
Fix constraint: the flip must happen on the already-mounted root, not by remounting — index.test.tsx:211 pins expect(notificationObservers.at(-1)).toBe(observer) and DaemonSessionProvider.test.tsx:19990-19992 pins expect(events).toHaveBeenCalledTimes(1) and expect(session.detach).not.toHaveBeenCalled(), so toggling active must preserve the same observer instance and must not restart the session stream.
Fix witness: the new test must go red when activeRef.current && is removed from canShow() — with the clause gone the deferred deliver() constructs the Notification and the not-called assertion fails.
中文说明
你们在 canShow() 中新增的 activeRef.current 这一项是承重的，但没有任何测试覆盖它。它是唯一能抑制“已经在途”的送达的东西，而所有测试套件要么在整个送达过程中保持 active 为 true，要么在 observer 层就把 notify 拦住了——所以删掉这一项，所有测试仍然是绿的。
既有的两个“未激活”测试都到不了这里。在 DaemonSessionProvider.test.tsx:19918-19993 和 index.test.tsx:203-212 中，active=false 会让 TurnNotificationContext 的值变成 undefined，于是 useTurnNotificationBinding 的 observerRef.current?.observe(...) 根本不会调用 notifyRef.current，送达在 canShow() 运行之前就停止了。这一项只在 show() 内部的异步间隙中生效：后台标签页的回合在 active 为 true 时完成，show() 在 await crypto.subtle.digest(...) 或 navigator.locks.request(CLAIMS_STORAGE_KEY, ...) 处挂起——后者会阻塞到另一个同源标签页释放 claims 锁为止，所以这是一个真实的跨标签页窗口，而不只是宿主改 prop 的场景——此时宿主在不带 browserNotifications 的情况下重渲染，随后 deliver() 恢复执行。删掉这一项后，mounted.current 仍为 true、enabledRef.current 仍为 true、权限仍为 granted，于是会为宿主刚刚关闭的集成构造并显示一个 Notification。而由于 TurnNotificationNavigationContext 此时已是 undefined，App 已经摘下了它的 qwen:open-session 监听器，所以点击那条通知只会把它关掉。
建议补一个用例：让送达挂起，并在挂起期间翻转 active——用一个 deferred 给 navigator.locks.request（或 crypto.subtle.digest）打桩，在 active={true} 时触发 turn_complete，在同一个已挂载的 root 上以 active={false} 重渲染，再 resolve 那个 deferred，并断言 Notification spy 未被调用；然后断言 active={true} 时的后续回合仍能送达。上面的探针就是可用的骨架。
修复约束：翻转必须发生在已挂载的 root 上，而不是通过重挂——index.test.tsx:211 锁定了 expect(notificationObservers.at(-1)).toBe(observer)，DaemonSessionProvider.test.tsx:19990-19992 锁定了 expect(events).toHaveBeenCalledTimes(1) 与 expect(session.detach).not.toHaveBeenCalled()，所以切换 active 必须保留同一个 observer 实例、且不得重启会话流。
修复验收：新测试必须在从 canShow() 中移除 activeRef.current && 时变红——去掉该项后，被延迟的 deliver() 会构造 Notification，未调用断言随之失败。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| const current = binding.current; | ||
| if (!current) return; | ||
| current.release(); | ||
| current.release = observer?.retain(current.scope) ?? (() => {}); |
There was a problem hiding this comment.
[Suggestion] R3-9: This new observer-rebind effect is the only code that establishes a retain when TurnNotificationContext goes from undefined to an observer after the binding was activated, and nothing in the suite exercises that direction — deleting the effect leaves 1293 tests green while the public prop's enable-after-load path silently stops delivering. The same two lines also lose in-flight turn memory on a deactivate, and one guard fixes both.
activate() stores release: observerRef.current?.retain(next.scope) ?? (() => {}). With the context undefined that is a no-op release and no retain, so the observer's scopes map never gets an entry for that scope, and activate() cannot recover it later because it only re-retains when the scope changes. When the prop arrives, only this effect calls observer.retain(current.scope). Remove it and every later observe() hits if (!state || state.references === 0) return;, so each turn_complete and turn_error for that session is dropped until the session is reloaded — the feature is dead with no error and no test red. Enable-after-load is documented public behaviour, not a hypothetical: index.tsx:147 gates on active={browserNotifications !== undefined} and branding.md:13 says "Omission disables notification contexts and delivery; returning to an object restores the same instance."
The second harm is the same two lines read the other way. Because the effect releases unconditionally, an active off-to-on flip drops references to 0 with no following retain, so the deferred queueMicrotask in retain's release finds references === 0 and deletes the scope together with its pending map; the later retain then builds a fresh, empty scope. A replayed catch-up terminal for a prompt admitted before the toggle is then dropped by the replay gate at :379, and the admitted label is gone for the live path too. I want to be accurate about the history here: an earlier reviewer read this as pre-existing and strictly narrowed by the new effect, and an A/B on the identical transition refutes that — the pre-PR [baseUrl, observer] memo recreated handlers, whose cleanup is generation-guarded, so the queued microtask returned without releasing and pending survived.
Witness:
Mutation plus two probes, isolated copy. Baseline: 6 files / 1293 tests passed.
INTACT (effect present) : PROBE r3h notifications= 1 ["QwenCode"] events invocations= 1 loads= 1
MUTANT (effect deleted) : PROBE r3h notifications= 0 [] events invocations= 1 loads= 1
MUTANT suite : 6 files / Tests 1293 passed (1293) EXIT=0 <- identical to baseline
A/B on the pending-registry half, identical probe in both arms (arm proof: PR tree has `}, [baseUrl]);` plus
`}, [observer]);`; base tree has `}, [baseUrl, observer]);` and no [observer] effect):
BASE: baseline replay-notify=1 | after off->on toggle replay-notify=1 | after same-observer re-render replay-notify=1
PR : baseline replay-notify=1 | after off->on toggle replay-notify=0 | after same-observer re-render replay-notify=1
[LABEL] no-toggle: notify=1 promptText="Refactor the auth module" [LABEL] off->on: notify=1 promptText=undefined
Hook-level probe with the one-line guard applied (release only when the observer is defined):
PR CODE (INTACT) : PROBE r3h/15 delivered= 0 payload= []
ONE-LINE FIX APPLIED : PROBE r3h/15 delivered= 1 payload= [{"sessionTitle":"Title",…"promptText":"My admitted question",…}]
FIX suite : 6 files / Tests 1293 passed (1293) EXIT=0 <- no existing test needed changing
Guard the release on the observer being defined, which fixes both harms in one edit:
useEffect(() => {
const current = binding.current;
if (!current || !observer) return;
current.release();
current.release = observer.retain(current.scope);
}, [observer]);With that guard the true-to-false transition retains nothing away, so pending survives; on false-to-true the release/retain pair runs against the same state object synchronously, so references goes 1→0→1 before the deferred delete microtask observes it and the map is kept. There is no leak: the observer instance is created once per mount and never changes identity, and teardown still releases through the [handlers] cleanup effect. A genuine observer replacement still releases and re-retains, because the guard only skips the undefined case.
Fix constraint: turn-notification-context.ts:299-301 — queueMicrotask(() => { if (state.references === 0 && scopes.get(scope) === state) scopes.delete(scope); }); — that reference count is the only thing that frees a scope when its provider unmounts or switches session, so the fix must still let references reach 0 on binding teardown (activate's binding.current?.release() and the [handlers] lifecycle cleanup). A new test must also preserve the sibling toggle test's no-restart guarantees at DaemonSessionProvider.test.tsx:19989-19991.
Fix witness: add a case to DaemonSessionProvider.test.tsx with the existing toggle test's shape but the first two show() calls reversed — mount with active={false}, let the session load, re-render with active={true}, and assert one delivered notification after a subsequent turn_complete. Delete turn-notification-context.ts:166-171 and confirm the new test goes red (no scopes entry, so observe returns at the references === 0 guard) while toggles browser notifications without restarting the session stream stays green.
中文说明
这个新增的 observer 重绑 effect，是唯一在绑定已激活之后、TurnNotificationContext 从 undefined 变为 observer 时建立 retain 的代码，而测试套件中没有任何用例覆盖这个方向——删掉这个 effect，1293 个测试仍然全绿，而公共 prop 的“加载后再启用”路径会静默停止送达。同样这两行还会在一次停用中丢失在途回合的记忆，而一个守卫可以同时修复两者。
activate() 存的是 release: observerRef.current?.retain(next.scope) ?? (() => {})。当 context 为 undefined 时，这是一个空操作的 release、并且没有 retain，所以 observer 的 scopes map 永远不会有该 scope 的条目；activate() 之后也无法补救，因为它只在 scope 变化时才重新 retain。当 prop 到达时，只有这个 effect 会调用 observer.retain(current.scope)。删掉它之后，后续每一次 observe() 都会命中 if (!state || state.references === 0) return;，于是该会话的每个 turn_complete 与 turn_error 都被丢弃，直到会话被重新加载——功能失效，却没有错误、也没有测试变红。“加载后再启用”是文档写明的公共行为，不是假想场景：index.tsx:147 以 active={browserNotifications !== undefined} 作为门禁，branding.md:13 写着“不传配置时停用通知 context 与送达；重新传入对象时恢复同一实例”。
第二个损害是同两行代码的另一种读法。由于该 effect 无条件 release，一次 active 由开到关的翻转会把 references 降到 0 且后面没有 retain，于是 retain 的 release 中那个延迟的 queueMicrotask 发现 references === 0，把 scope 连同它的 pending map 一起删除；随后的 retain 会新建一个空的 scope。这样一来，翻转之前已准入的提问，其重放补送终态就会被 :379 的重放门禁丢弃，已准入的标题在实时路径上也一并丢失。这里需要如实说明历史：早前有审查者认为这是既有问题、并被新 effect 严格收窄，而对同一转换做的 A/B 反驳了这一点——改动前 [baseUrl, observer] 的 memo 会重建 handlers，其 cleanup 受 generation 保护，所以排队的微任务会直接返回而不 release，pending 因此得以保留。
把 release 加上“observer 已定义”的守卫，一次修改即可同时修复两个损害（代码见上方英文部分）。有了这个守卫，由开到关的转换不会把 retain 释放掉，pending 得以保留；由关到开时 release/retain 成对地、同步地作用于同一个 state 对象，所以 references 走 1→0→1，在延迟删除的微任务观察到它之前就已恢复，map 被保住。不会有泄漏：observer 实例每次挂载只创建一次、身份不变，卸载仍然通过 [handlers] 清理 effect 释放；真正的 observer 替换仍会 release 并重新 retain，因为守卫只跳过 undefined 这一种情况。
修复约束：turn-notification-context.ts:299-301 的 queueMicrotask(() => { if (state.references === 0 && scopes.get(scope) === state) scopes.delete(scope); });——那个引用计数是 provider 卸载或切换会话时释放 scope 的唯一机制，所以修复仍必须允许绑定销毁时 references 归零（activate 的 binding.current?.release() 与 [handlers] 生命周期清理）。新测试还必须保留同类开关测试在 DaemonSessionProvider.test.tsx:19989-19991 的“不重启”保证。
修复验收：在 DaemonSessionProvider.test.tsx 中按既有开关测试的形态补一个用例，但把前两次 show() 调用顺序反过来——以 active={false} 挂载，让会话加载完成，再以 active={true} 重渲染，并断言随后的 turn_complete 送达一条通知。删除 turn-notification-context.ts:166-171 后确认新测试变红（没有 scopes 条目，observe 在 references === 0 守卫处返回），而 toggles browser notifications without restarting the session stream 保持绿色。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| block.meta?.source !== 'vision_bridge_notice' && | ||
| block.text.trim() | ||
| ) { | ||
| const segments = splitInsightSegments(block.text); |
There was a problem hiding this comment.
[Suggestion] R3-10: The reply-excerpt path assigns content.responseText verbatim, while the sibling prompt path a few lines above is bounded by MAX_NOTIFICATION_SOURCE_LENGTH through notificationPromptText. An unbounded string therefore crosses the observer → notify → async show() boundary for a value whose only read site truncates it to 120 characters.
A turn whose final main assistant block is a large dump leaves a multi-megabyte string retained on the TurnNotification object across await crypto.subtle.digest(...) and navigator.locks.request(...), so several can be in flight at once while the consumer reads 120 characters of it. The prompt side of the very same object never exceeds 4096. Blocks that large are within normal store operation (DEFAULT_MAX_RETAINED_BYTES = 128 MB) and the store does evict them — which is what turns the notification's reference into a lifetime extension.
I want to be precise about the cost, because the first version of this finding overstated it and measurement corrected it. The claimed O(braces × length) copying inside splitInsightSegments is false on V8: text.slice(braceIdx + 1) yields a SlicedString and neither .trimStart() nor .startsWith(p) flattens it, so the per-brace cost is O(1) and the timings are flat across an 8× input range rather than growing ~4×. The segmentation is also pre-existing on the render path (transcriptToMessages.ts:533, run repeatedly during streaming; the only diff hunk in that file is function → export function at :1757). So what this PR adds is the retention, not the scan — and the fix is still one line, because bounding the source costs 0.0 ms.
Witness:
Probe against the real modules, unmodified PR tree:
MAX_NOTIFICATION_SOURCE_LENGTH = 4096
reply 1600KB: 2.9 ms -> responseText.length=1646326 (source 1646326) promptText.length=20
prompt 1600KB source: 0.0 ms -> promptText.length=4096
json 1600KB len=1646326 braces=46764: 2.1 ms
soup 200KB braces=34134: 2.4 ms soup 400KB braces=68267: 2.5 ms soup 4MB braces=699051: 24.5 ms
-> FLAT where quadratic predicts ~4x, so the O(braces x length) half is REFUTED (V8 SlicedString)
fixed splitInsightSegments(text.slice(0,4096)) on 400KB and 1.6MB: 0.0 ms
pinned inputs at turn-notification-context.test.ts:405-425 (all <= 86 chars): unchangedByBound=true sameContent=true x4
straddling-bound marker: CURRENT responseText.len=4120 vs BOUNDED responseText.len=4096
Frequency: once per turn_complete per retained scope, not per SSE event — the thunk is invoked only after the
type/session/promptId gates and a successful consume().
Bound the reply source before scanning, mirroring the prompt path:
const source = block.text.slice(0, MAX_NOTIFICATION_SOURCE_LENGTH);
const segments = splitInsightSegments(source);
const visibleText = segments
? segments
.filter((segment) => segment.kind === 'text')
.map((segment) => segment.text)
.join('\n')
.trim()
: source;Fix constraint: MAX_NOTIFICATION_SOURCE_LENGTH = 4096 (packages/web-shell/client/notification-text.ts:7), and turn-notification-context.test.ts:405-425 pins that a report line plus a balanced insight_ready marker plus a truncated insight_error marker yields { sessionTitle: 'Title' } with no responseText. All four pinned inputs are ≤ 86 chars, so the bound leaves them unchanged (measured). One behaviour change worth knowing: a marker straddling char 4096 degrades to truncated prose rather than being stripped, and where the cut leaves a raw "insight_ready":{ prefix the existing regex guard at :255 omits responseText — so it fails closed.
Fix witness: please add a case to turn-notification-context.test.ts beside keeps visible insight prose while omitting internal segments feeding an assistant block longer than MAX_NOTIFICATION_SOURCE_LENGTH and asserting content.responseText.length <= MAX_NOTIFICATION_SOURCE_LENGTH while the first-visible-prose excerpt is unchanged; delete the slice and confirm it goes red.
中文说明
回复摘录这条路径把 content.responseText 原样赋值，而上方几行的提问路径通过 notificationPromptText 受 MAX_NOTIFICATION_SOURCE_LENGTH 约束。因此一个无界字符串会跨越 observer → notify → 异步 show() 的边界，而它唯一的读取点只截取 120 个字符。
当某回合最后一段主助手 block 是一个很大的转储时，一个数兆字节的字符串会在 await crypto.subtle.digest(...) 与 navigator.locks.request(...) 期间一直挂在 TurnNotification 对象上，因此可能同时有多个在途，而消费方只读其中 120 个字符。同一个对象的提问侧从不超过 4096。这么大的 block 属于 store 的正常运行范围（DEFAULT_MAX_RETAINED_BYTES = 128 MB），而 store 确实会淘汰它们——这正是通知持有的引用变成生命周期延长的原因。
需要准确说明代价，因为这条发现的第一版夸大了它，实测做了纠正：所谓 splitInsightSegments 内部 O(braces × length) 的复制在 V8 上是不成立的——text.slice(braceIdx + 1) 产生 SlicedString，.trimStart() 与 .startsWith(p) 都不会将其扁平化，所以每个花括号的代价是 O(1)，实测耗时在 8 倍输入范围内是平的，而不是按约 4 倍增长。该分段在渲染路径上也已存在（transcriptToMessages.ts:533，流式期间反复运行；该文件在本 PR 中唯一的 hunk 是 :1757 处 function → export function）。所以本 PR 新增的是保留，不是扫描——而修复仍然只是一行，因为限制源文本的代价是 0.0 ms。
建议在扫描前先限制回复源文本，与提问路径保持一致（代码见上方英文部分）。
修复约束：MAX_NOTIFICATION_SOURCE_LENGTH = 4096（packages/web-shell/client/notification-text.ts:7），并且 turn-notification-context.test.ts:405-425 锁定了“一行报告文本 + 一个完整的 insight_ready 标记 + 一个被截断的 insight_error 标记”必须得到 { sessionTitle: 'Title' } 且没有 responseText。四个被锁定的输入都 ≤ 86 字符，所以该边界不会改变它们（已实测）。有一处行为变化值得知道：跨越第 4096 字符的标记会退化为被截断的正文而不是被剥除；而当切口留下裸的 "insight_ready":{ 前缀时，:255 已有的正则守卫会省略 responseText——即失败时是关闭方向。
修复验收：请在 turn-notification-context.test.ts 中、keeps visible insight prose while omitting internal segments 旁边补一个用例，喂入一个长于 MAX_NOTIFICATION_SOURCE_LENGTH 的 assistant block，断言 content.responseText.length <= MAX_NOTIFICATION_SOURCE_LENGTH 且首个可见正文摘录不变；删除该 slice 后确认它变红。
— qwen3.8-max via Qwen Code /review (v0.23.2)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- notification-text.ts:45 hand-rolled Markdown stripping — already reported (comment 3980349067)
- notification-text.ts:49 invisible-character strip — already reported (comment 3980349103)
- browser-turn-notifications.tsx:211 activeRef clause in canShow() — already reported (comment 3980349219)
- turn-notification-context.ts:170 observer-rebind effect — already reported (comment 3980349229)
Not reviewed: mid-turn-injection attachment shape — an auditor traced that inline (non-trailing-line) attachment tokens are not stripped by notificationPromptText, but stopped before establishing whether such a block can reach the prompt scan carrying the turn's promptId — stopped at the agent tool budget.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": whether the create-path remember at DaemonSessionProvider.tsx:4423 (hardcoded { kind: 'workspace', cwd: owner.workspaceCwd } , pre-existing) is reachable whi…; "agent reverse-audit (round 2)": verifying each remaining item in the two new docs' "Tests cover …" enumerations (focus failure, Unicode truncation, icon parameters, captured targets, cross-wor…; "agent reverse-audit (round 3)": did not read scripts/copy_bundle_assets.js to confirm the installed qwen serve bundle carries the app-mode-emitted dist/assets/*.png (14,125 bytes exceeds…; "agent reverse-audit (round 1)": the handleOpenSessionFromOverview body behind web-shell-browser-notification-details.md §Clicking to open a session ("reuses the existing session-loading fl…; "agent reverse-audit (round 4)": none of my checks were cut short, but two verifications are reasoned rather than executed — I did not run the iconUrl -throws mutation against vitest (Finding …, and 7 more.
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Convergence: round 3 posted 12 inline comment(s), 12 of them reported for the first time; the previous round posted 9 (7 new). Findings keep coming back to the same files: packages/web-shell/client/App.tsx (findings in rounds 1, 2; 4 more now); packages/web-shell/client/browser-turn-notifications.test.tsx (findings in round 2; 2 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. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
本轮确认的 4 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：mid-turn-injection attachment shape — an auditor traced that inline (non-trailing-line) attachment tokens are not stripped by notificationPromptText, but stopped before establishing whether such a block can reach the prompt scan carrying the turn's promptId — stopped at the agent tool budget.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：whether the create-path remember at DaemonSessionProvider.tsx:4423 (hardcoded { kind: 'workspace', cwd: owner.workspaceCwd } , pre-existing) is reachable whi…；"agent reverse-audit (round 2)"：verifying each remaining item in the two new docs' "Tests cover …" enumerations (focus failure, Unicode truncation, icon parameters, captured targets, cross-wor…；"agent reverse-audit (round 3)"：did not read scripts/copy_bundle_assets.js to confirm the installed qwen serve bundle carries the app-mode-emitted dist/assets/*.png (14,125 bytes exceeds…；"agent reverse-audit (round 1)"：the handleOpenSessionFromOverview body behind web-shell-browser-notification-details.md §Clicking to open a session ("reuses the existing session-loading fl…；"agent reverse-audit (round 4)"：none of my checks were cut short, but two verifications are reasoned rather than executed — I did not run the iconUrl -throws mutation against vitest (Finding …，另有 7 条。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
收敛情况：第 3 轮发布了 12 条行内评论，其中 12 条是首次提出；上一轮发布了 9 条（其中 7 条首次提出）。发现反复回到同一批文件：packages/web-shell/client/App.tsx（第 1、2 轮已出过发现，本轮又有 4 条）；packages/web-shell/client/browser-turn-notifications.test.tsx（第 2 轮已出过发现，本轮又有 2 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.23.2)
| ) => { | ||
| splitClassificationGenerationRef.current += 1; | ||
| // Explicit navigation cancels any pending shrink-fold split restore. | ||
| if (mainView === 'split' || splitFoldedByShrinkRef.current) { |
There was a problem hiding this comment.
[Suggestion] R2-4: (fix-induced) The fix for this entry added notifyControlledSplitClose() and clearSplitSessions() under a two-disjunct condition, and the splitFoldedByShrinkRef.current disjunct has no test — narrowing the condition to mainView === 'split' alone leaves 879/879 App.test.tsx tests green.
The unpinned behaviour is real. The shrink handler deliberately keeps the stored split (App.tsx:8737-8739, 8778-8786), loadSplitSessions() re-opens it on a later load (:8729-8732), and clearSplitSessions() is sessionStorage.removeItem(SPLIT_STORAGE_KEY) (utils/splitUrl.ts:101-108). So a user who shrinks the window and then clicks a notification for another session keeps the per-tab split set persisted only because of this disjunct; without it a refresh restores the stale split view over the session they just opened. Two corrections to the original harm narrative: the stale state is that sessionStorage set, not the ?split= URL param (consumed and deleted once at bootstrap, App.tsx:8709-8716); and notifyControlledSplitClose() in the folded disjunct can never fire, because the fold flag is set only when the host is uncontrolled (:8785-8786) while the callee fires only when it is controlled (:8635-8639) — the two disjuncts need different bodies.
Witness:
baseline (unmodified) : Tests 879 passed (879)
mutant `|| splitFoldedByShrinkRef.current` removed : Tests 879 passed (879)
In the folded-split case (or a notification-target variant of it), also assert the persisted set was cleared — expect(loadSplitSessions()).toEqual([]) after onOpenSession — mirroring what opens a notification target in the main chat from split view already asserts for the unfolded case, and give the folded disjunct clearSplitSessions() only.
A shrink on its own must still not clear the stored split: App.tsx:8648-8650 records “(A shrink-fold is transient and deliberately doesn’t clear it.)” and App.test.tsx:30569 pins the shrink-only behaviour, so the new assertion has to be tied to explicit navigation.
Acceptance criterion: that folded-split case must go red when || splitFoldedByShrinkRef.current is dropped from App.tsx:13198 — please remove the disjunct, run the case, and confirm it reds.
中文说明
本条目的修复在双分支条件下加入了 notifyControlledSplitClose() 与 clearSplitSessions()，其中 splitFoldedByShrinkRef.current 这一分支没有测试——把条件收窄为仅 mainView === 'split'，App.test.tsx 的 879 个测试仍全部通过。
未被锁定的行为是真实存在的：shrink 处理刻意保留已存储的分屏集合（App.tsx:8737-8739、8778-8786），loadSplitSessions() 会在后续加载时重新打开它（:8729-8732），而 clearSplitSessions() 就是 sessionStorage.removeItem(SPLIT_STORAGE_KEY)（utils/splitUrl.ts:101-108）。因此用户先缩小窗口、再点击另一个会话的通知时，只是因为这一分支才没有留下持久化的分屏集合；否则刷新后会在刚打开的会话上恢复过期的分屏视图。对原描述的两点更正：过期状态是这个 sessionStorage 集合，而不是 ?split= URL 参数（它在启动时已被消费并删除，App.tsx:8709-8716）；并且折叠分支里的 notifyControlledSplitClose() 永远不会触发，因为折叠标志只在宿主非受控时设置（:8785-8786），而该回调只在受控时触发（:8635-8639）——两个分支需要不同的函数体。
建议在折叠分屏用例（或其通知目标变体）中同时断言持久化集合已被清空——在 onOpenSession 之后 expect(loadSplitSessions()).toEqual([])——与 opens a notification target in the main chat from split view 对非折叠场景已有的断言保持一致，并让折叠分支只调用 clearSplitSessions()。
修复约束：单独 shrink 仍不得清空已存储的分屏——App.tsx:8648-8650 记录了“(A shrink-fold is transient and deliberately doesn’t clear it.)”，App.test.tsx:30569 锁定了仅 shrink 的行为，因此新断言必须绑定在显式导航上。
验收标准：从 App.tsx:13198 删除 || splitFoldedByShrinkRef.current 后，该折叠分屏用例必须变红——请移除该分支、运行用例并确认其失败。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| event, | ||
| store.getSnapshot().blocks, | ||
| connectionRef.current.sessionId === activeSession.sessionId | ||
| ? connectionRef.current.displayName |
There was a problem hiding this comment.
[Suggestion] R4-1: The mismatched-session branch of this new title ternary is never exercised, so collapsing it to connectionRef.current.displayName alone keeps every test green while a notification can be titled with the previous session's name.
In all four captures $kind target and current metadata cases the terminal belongs to the connected session, and the epoch-reset case goes through the replay path (line 2841) which has no ternary — so connectionRef.current.sessionId === activeSession.sessionId is always true in tests. The window the guard covers is real: sessionRef.current is swapped synchronously during a load while connection.displayName is only refreshed through setConnection (:3117-3119), so a turn_complete drained right after a switch can still see connectionRef.current naming the previous session. With the false branch removed, the banner for the newly opened session is titled with the old session's name — QwenCode · Refactor auth over the new session's own prompt and reply excerpt — which is exactly the mislabelling this enhancement exists to remove.
Witness:
mutant: ternary -> connectionRef.current.displayName : Tests 324 passed (324) <- false branch unpinned
mutant: ternary -> getSessionDisplayName(activeSession.state) : Tests 4 failed | 320 passed (324)
FAIL 'captures workspace target and current metadata (Renamed session) after a live terminal'
FAIL '… ("") …' / '… standalone (Standalone title) …' / '… live (Live title) …'
Note only the connectionRef-only collapse is green; the other direction fails four tests, so the gap is one-sided.
Add a case where the observed session and connectionRef.current.sessionId differ at notify time — drain session A's terminal after the connection has moved to session B, or hold the connection state stale — and assert the title comes from the owning session's own state.
Acceptance criterion: that new case must go red when the ternary is flattened to connectionRef.current.displayName.
中文说明
这个新增标题三元表达式的“会话不匹配”分支从未被执行，因此把它坍缩为只取 connectionRef.current.displayName，全部测试仍然通过，而通知却可能使用上一个会话的名字作为标题。
四个 captures $kind target and current metadata 用例中，终态事件都属于当前已连接的会话；epoch-reset 用例走的是 replay 路径（2841 行），那里没有三元表达式——所以测试里 connectionRef.current.sessionId === activeSession.sessionId 恒为真。该分支要覆盖的窗口是真实存在的：加载过程中 sessionRef.current 是同步替换的，而 connection.displayName 只能通过 setConnection 刷新（:3117-3119），因此切换后立刻排空的 turn_complete 可能仍看到 connectionRef.current 指向上一个会话。去掉 false 分支后，新打开会话的横幅会带旧会话的名字——QwenCode · Refactor auth 却配着新会话自己的提问与回复摘录——这正是本增强要消除的错标。
注意只有 connectionRef 单侧坍缩是绿的；另一侧会让 4 个测试失败，所以缺口是单向的。
建议新增一个用例：在通知时刻让被观察会话与 connectionRef.current.sessionId 不一致（在连接已切到会话 B 后排空会话 A 的终态，或让连接状态保持过期），并断言标题取自所属会话自身的 state。
验收标准：把三元表达式压平为 connectionRef.current.displayName 时，该新用例必须变红。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| current.status === 'connected' && | ||
| !current.loadingTranscript && | ||
| !current.missingSession && | ||
| !current.standaloneSession?.creationRecovery && |
There was a problem hiding this comment.
[Suggestion] R4-2: Three of the eligibility guards on the new reveal-instead-of-reload short-circuit are untested — only loadingTranscript and missingSession are parameterized — so each can be deleted with the suite green.
it.each(['loadingTranscript', 'missingSession'])('reloads the current notification target when %s') is the only test driving the negative side. Deleting !current.standaloneSession?.creationRecovery, current.status === 'connected', or !pendingSessionContextRef.current each leaves 879/879 passing, while the positive control (deleting !current.loadingTranscript) reddens — so these are coverage gaps, not a dead comparator. For creation recovery concretely: a standalone session that reloaded into creationRecovery shows the recovery banner and depends on loadSidebarSession(sessionId, undefined, { kind: 'standalone' }) inside handleCheckStandaloneRecovery (:13058-13060) to attach; clicking that session's own notification takes the short-circuit, reveals the chat, closes panels and scrolls, and skips the load — the user is left on the unrecovered session with the banner still up, so the click visibly does nothing. The same shape applies to a matching sessionId on a non-connected connection.
Witness:
baseline (unmodified) : Tests 879 passed (879)
drop `!current.standaloneSession?.creationRecovery &&` : Tests 879 passed (879)
drop `current.status === 'connected' &&` : Tests 879 passed (879)
drop `!pendingSessionContextRef.current &&` : Tests 879 passed (879)
positive control - drop `!current.loadingTranscript &&` : Tests 1 failed | 878 passed (879)
FAIL App.test.tsx > reloads the current notification target when loadingTranscript
Widen the it.each to cover a non-connected mockConnection.status, a mockConnection.standaloneSession.creationRecovery, and a pending draft context, each asserting mockSessionActions.loadSession IS called.
The reveal path must still be taken for a healthy matching session: reveals the current notification target without reloading an active session asserts expect(mockSessionActions.loadSession).not.toHaveBeenCalled() together with notificationScrollToBottom called with 'auto'.
Acceptance criterion: the extended cases must go red if any one of the three guards is deleted from the condition.
中文说明
新的“只显示不重载”短路条件中，有三项资格判断没有测试——只有 loadingTranscript 与 missingSession 被参数化——因此删掉任意一项，测试套件依然全绿。
it.each(['loadingTranscript', 'missingSession'])('reloads the current notification target when %s') 是唯一驱动反向分支的测试。删除 !current.standaloneSession?.creationRecovery、current.status === 'connected' 或 !pendingSessionContextRef.current，879 个测试全部通过；而正向对照（删除 !current.loadingTranscript）会变红——说明这是覆盖缺口，不是比较条件失效。以 creation recovery 为例：一个重载后进入 creationRecovery 的 standalone 会话会显示恢复横幅，并依赖 handleCheckStandaloneRecovery 中的 loadSidebarSession(sessionId, undefined, { kind: 'standalone' })（:13058-13060）才能挂载；点击它自己的通知会走短路分支，显示聊天、关闭面板并滚动，却跳过加载——用户停在未恢复的会话上、横幅仍在，点击看起来毫无效果。sessionId 匹配但连接不是 connected 的情形同理。
建议把 it.each 扩展到：非 connected 的 mockConnection.status、mockConnection.standaloneSession.creationRecovery、以及待分配草稿上下文，每种都断言 mockSessionActions.loadSession 被调用。
修复约束：健康且匹配的会话仍必须走显示路径——reveals the current notification target without reloading an active session 断言 expect(mockSessionActions.loadSession).not.toHaveBeenCalled()，同时 notificationScrollToBottom 以 'auto' 被调用。
验收标准：从条件中删除三项判断中的任意一项时，扩展后的用例必须变红。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| # Web Shell 浏览器任务通知：独立实现设计 | ||
|
|
||
| 状态：独立浏览器通知已实现，构建和自动化验证通过。调研及实现日期：2026-09-08。 | ||
| 状态：2026-09-08 基础版历史设计。以下默认关闭、通用文案、点击仅聚焦、不增加公共 API 等条款描述 #11398 当时的基线；当前行为已由[通知内容与会话导航](web-shell-browser-notification-details.zh-CN.md)及[可配置品牌](web-shell-browser-notification-branding.zh-CN.md)取代，英文对应文档通过两份设计的语言链接提供。 |
There was a problem hiding this comment.
[Suggestion] R4-3: This PR updates an existing single-language design without adding the missing counterpart version that docs/design/README.md requires for exactly that case.
git ls-tree HEAD docs/design/web-shell shows only web-shell-browser-turn-notifications.md — there is no .zh-CN.md, and this .md slot (which docs/design/README.md:11-13 reserves for English, with relative links to both versions immediately below each title) contains Chinese-only prose with no language links under its title. README.md:40-42, “Requirements for subsequent changes”: “When updating an existing single-language design, add the missing version and align the complete pair.” The new status line acknowledges the gap instead of closing it, so the checklist items “Both files exist, and their language links resolve in both directions” and “Both versions reflect the same status” fail for the one design this PR edits, while the two designs it adds are correctly bilingual. An English-reading maintainer who lands on the superseded baseline — the document that still holds authority for permissions, background triggers and deduplication — cannot read the sentence that tells them which clauses are dead.
Witness:
$ git ls-tree -r HEAD docs/design/web-shell --name-only | grep zh-CN
docs/design/web-shell/web-shell-browser-notification-branding.zh-CN.md <- added by this PR
docs/design/web-shell/web-shell-browser-notification-details.zh-CN.md <- added by this PR
(no web-shell-browser-turn-notifications.zh-CN.md)
$ git ls-tree -r HEAD^2 docs/design/web-shell --name-only | grep -c zh-CN
0
The gap is pre-existing on main, but the exclusion for pre-existing issues covers unchanged code and this diff changes this file — which is precisely the trigger the README's subsequent-changes rule names. Per AGENTS.md a translation gap is a Suggestion, not a Critical.
Either add web-shell-browser-turn-notifications.zh-CN.md plus an English version carrying the same status line, with reciprocal [English] | [简体中文] links under both titles; or, if translating the 177-line historical baseline is deliberately out of scope, rename it to .zh-CN.md in this PR so the file occupies the slot matching its language, and update the two new designs' references in the same commit.
Acceptance criterion: N/A — documentation only, no guard or behaviour for a test to pin.
中文说明
本 PR 修改了一份既有的单语言设计文档，却没有补上 docs/design/README.md 针对这种情况明确要求的另一语言版本。
git ls-tree HEAD docs/design/web-shell 只有 web-shell-browser-turn-notifications.md——没有 .zh-CN.md；而这个 .md 位置（docs/design/README.md:11-13 把它保留给英文，并要求在每份标题下方给出两个版本的相对链接）里放的却是纯中文正文，标题下也没有语言链接。README.md:40-42“后续修改的要求”写道：“When updating an existing single-language design, add the missing version and align the complete pair.”。新的状态行只是承认了这个缺口而没有关闭它，因此本 PR 修改的这份设计不满足检查项“两个文件都存在，且语言链接双向可解析”和“两个版本反映相同状态”，而本 PR 新增的两份设计则是正确双语的。一位只读英文的维护者若落到这份被取代的基线文档——它仍然是权限、后台触发与去重规则的权威来源——将无法读到说明哪些条款已失效的那句话。
该缺口在 main 上确实已存在，但“既有问题”的排除项针对的是未改动的代码，而本次 diff 修改了这个文件——这正是 README“后续修改”规则所指的触发条件。按 AGENTS.md，翻译缺口属于 Suggestion 而非 Critical。
建议：要么补上 web-shell-browser-turn-notifications.zh-CN.md 以及带相同状态行的英文版，并在两个标题下加互链 [English] | [简体中文]；要么如果翻译这份 177 行的历史基线确实不在本 PR 范围内，就在本 PR 中把它改名为 .zh-CN.md，使文件占据与其语言相符的位置，并在同一次提交中更新两份新设计的引用。
验收标准：不适用——纯文档，没有可供测试锁定的判断或行为。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| getTurnNotificationContent( | ||
| terminal(), | ||
| [ | ||
| block('check\n\n@attachment:///private.txt', { kind: 'user' }), |
There was a problem hiding this comment.
[Suggestion] R4-4: This is the only attachment-tail fixture in the added suite and it carries a single token line, so the repeated pop in notificationPromptText is unpinned even though multi-token tails are the normal shape for a prompt with two or more attachments.
Changing while to if at turn-notification-context.ts:210 leaves all 7213 tests in the package green, including this case, whose single token line is removed by either form. In the app a prompt sent with two files is recorded as look\n\n@attachment:///a.log\n@attachment:///b.md (the shape promptContent.test.ts:121 and actions.test.ts already use), so only b.md is popped and the notification body renders Prompt: look @attachment:///a.log — the transport token the filter exists to omit leaks into a lock-screen-visible notification. A token-only tail is worse: it also becomes the derived sessionTitle via notificationTextLines(promptText)[0].
Witness:
mutant `while` -> `if` at turn-notification-context.ts:210, full package suite, pristine tests:
Test Files 303 passed (303) Tests 7213 passed (7213)
probe on getTurnNotificationContent, real two-attachment shape:
MUTANT (if) : promptText="look\n\n@attachment:///a.log" tokenOnly="@attachment:///a.log" tokenOnlyTitle="@attachment:///a.log"
INTACT (while): promptText="look" tokenOnly="" tokenOnlyTitle=undefined
Extend the case with a two-token tail — block('check\n\n@attachment:///a.log\n@attachment:///b.md', { kind: 'user' }) — asserting promptText and sessionTitle are both 'check'; optionally add a token-only tail asserting promptText '' and sessionTitle undefined. Keep the existing single-token and mid-prose assertions.
The producer builds the tail as attachmentUris.map((uri) => `@${uri}`).join('\n') appended after \n\n (promptContent.ts:21-24), so the fixture must be \n\n followed by one @attachment:///… token per line — a comma- or space-separated fixture would not match what reaches the transcript.
Acceptance criterion: the extended assertion must go red when while becomes if at turn-notification-context.ts:210; today it does not.
中文说明
这是新增测试套件中唯一的附件尾行夹具，且只带一行 token，因此 notificationPromptText 中的循环弹出没有被锁定——而带两个以上附件时，多行 token 才是常态形态。
把 turn-notification-context.ts:210 的 while 改成 if，整个 package 的 7213 个测试全部通过，包括本用例（它只有一行 token，两种写法都会移除）。在实际应用中，带两个文件的提问会被记录为 look\n\n@attachment:///a.log\n@attachment:///b.md（promptContent.test.ts:121 与 actions.test.ts 已使用这种形态），于是只有 b.md 被弹出，通知正文变成 Prompt: look @attachment:///a.log——这个过滤器本应省略的传输 token 泄露到了锁屏可见的通知里。只有 token 的尾部更糟：它还会通过 notificationTextLines(promptText)[0] 变成派生的 sessionTitle。
建议扩展该用例，加入两行 token 的尾部——block('check\n\n@attachment:///a.log\n@attachment:///b.md', { kind: 'user' })——断言 promptText 与 sessionTitle 都是 'check'；可选再加一个纯 token 尾部，断言 promptText 为 ''、sessionTitle 为 undefined。保留既有的单 token 与正文中出现 token 的断言。
修复约束：生产侧把尾部构造为 attachmentUris.map((uri) => `@${uri}`).join('\n') 并追加在 \n\n 之后（promptContent.ts:21-24），因此夹具必须是 \n\n 之后每行一个 @attachment:///… token——用逗号或空格分隔的夹具与真正进入 transcript 的文本不符。
验收标准：把 turn-notification-context.ts:210 的 while 改为 if 时，扩展后的断言必须变红；目前不会。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| focusComposerAfterSplitCloseRef.current = true; | ||
| closePanel(); | ||
| closeMobileDrawer(); | ||
| resumeChatBottomFollow('auto'); |
There was a problem hiding this comment.
[Suggestion] R4-7: The new reveal path enumerates the split view, the right-hand panel and the mobile drawer, but not artifactPanelFullscreen — the one overlay that puts the chat view itself behind display:none and aria-hidden — so a notification click that is supposed to reveal the conversation can reveal nothing.
With the artifact panel (Review / Side Task / Terminal) fullscreen, App.tsx:17043/17048 and 17261/17265 apply styles.chatViewHidden and aria-hidden to the chat view; the comment at :6597 states “Fullscreen hides the chat pane (display:none)”. The user switches tabs, the turn completes, the notification fires (canShow() requires the document hidden or unfocused), and they click it. The handler passes the lock check and satisfies the reveal guard. closePanel() only does setActivePanel(null) (:8317-8320) and never touches artifactPanelFullscreen, and the only reset a navigation would have caused is gated on an actual session change (:5849 → :5860), which by definition cannot fire here. The tab refocuses still showing the fullscreen artifact, the chat holding the new response stays display:none, and resumeChatBottomFollow('auto') scrolls a hidden node — a visible no-op. The approval overlay hit exactly this hole and had to add an explicit reset (:8890-8899), as did expandWorkflowGraph (:13605).
Witness:
real App rendered, monitor tab opened, toolbar Fullscreen clicked, then a qwen:open-session dispatch
carrying the current session and a matching workspace context:
PR BEFORE {"overlay":true,"contextHidden":true,"contextAria":"true","sidebarHidden":true}
PR AFTER {"overlay":true,"contextHidden":true,"contextAria":"true","sidebarHidden":true} loadSession=0
FIX AFTER {"overlay":false,"contextHidden":false,"contextAria":null,"sidebarHidden":false} loadSession=0
scrollToBottom=[["auto"]] (candidate fix leaves 880/880 App.test.tsx green)
In the reveal branch, shrink the artifact panel out of fullscreen alongside the other reveal calls, keeping artifactPanelOpen true so the user's panel and tab state survive:
if (artifactPanelFullscreen) {
setArtifactPanelFullscreen(false);
if (!useFloatingArtifactPanel) setSuppressArtifactDockOpenAnimation(true);
}App.tsx:8016-8018 records “Only the docked node replays its open animation on the fullscreen -> docked class swap; the floating drawer has no such replay”, and the management effect at :8021-8023 only resets the flag when !artifactPanelOpen — so a bare setArtifactPanelFullscreen(false) replays the dock slide-in unless the fix mirrors toggleArtifactPanelFullscreen.
Acceptance criterion: a case that mounts with the artifact panel open and fullscreen, dispatches a matching-context payload for the already-current session, and asserts the chat container no longer carries chatViewHidden / aria-hidden; deleting the added reset must redden it.
中文说明
新的显示路径处理了分屏视图、右侧面板和移动端抽屉，却没有处理 artifactPanelFullscreen——唯一会把聊天视图本身置于 display:none 与 aria-hidden 之后的遮罩——因此本应“显示对话”的通知点击可能什么也没显示。
当 artifact 面板（Review / Side Task / Terminal）处于全屏时，App.tsx:17043/17048 与 17261/17265 会给聊天视图加上 styles.chatViewHidden 和 aria-hidden；:6597 的注释写明“Fullscreen hides the chat pane (display:none)”。用户切到别的标签页，本轮结束，通知触发（canShow() 要求文档隐藏或失焦），用户点击。处理器通过锁定校验并满足显示条件。closePanel() 只做 setActivePanel(null)（:8317-8320），完全不涉及 artifactPanelFullscreen；而导航本来会触发的那次重置以会话真正变更为前提（:5849 → :5860），在此处按定义不可能触发。于是标签页重新获得焦点时仍显示全屏 artifact，承载新回复的聊天区仍是 display:none，resumeChatBottomFollow('auto') 滚动的是一个隐藏节点——点击表现为无效的可见空操作。审批遮罩曾遇到完全相同的缺口并不得不显式重置（:8890-8899），expandWorkflowGraph（:13605）也是如此。
建议在显示分支中，与其他显示调用一起把 artifact 面板退出全屏，同时保持 artifactPanelOpen 为真，以保留用户的面板与标签状态（代码见上方英文部分）。
修复约束：App.tsx:8016-8018 记录“Only the docked node replays its open animation on the fullscreen -> docked class swap; the floating drawer has no such replay”，而 :8021-8023 的管理 effect 只在 !artifactPanelOpen 时重置该标志——因此仅调用 setArtifactPanelFullscreen(false) 会重放 dock 滑入动画，除非修复方式与 toggleArtifactPanelFullscreen 保持一致。
验收标准：新增一个用例，在 artifact 面板打开且全屏时挂载，对当前会话派发上下文匹配的载荷，断言聊天容器不再带 chatViewHidden / aria-hidden；删除新增的重置必须使其变红。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| expect(capture.observer).toBe(observer); | ||
| expect(capture.settings!.enabled).toBe(true); |
There was a problem hiding this comment.
[Suggestion] R4-8: The “without resetting … the preference” half of this branding test is not discriminating: setEnabled(true) wrote 'true' into the stubbed localStorage before the branding re-render, so every storage-respecting re-derivation of the preference returns enabled: true, and no test in the file combines a non-persistent preference with a branding change.
Of the two mutant shapes, one is already caught and one is not. Re-deriving on an options change while reading options?.defaultEnabled live IS caught, by index.test.tsx:200. But re-deriving on an options change using the mount-time snapshot is caught by nothing — all 7211 shipped tests stay green, including this one. The uncovered regression is real: a blocked-storage user (Safari private mode or a third-party-storage block — the shape already built at lines 214-219) who opted in temporarily, so enabled: true with persistent: false and nothing written, loses notifications the moment a white-label host re-renders with a new appName or iconUrl, because readStoredPreference() returns undefined and stored === null && defaultEnabled is false. Delivery stops mid-session with no user action, contradicting the clause this test is named for.
Witness:
M1 mutant (re-derive reading options?.defaultEnabled live) : index.test.tsx:200 FAILS 'expected false to be true' <- caught
M2 mutant (re-derive using the mount-time snapshot) : all 7211 shipped tests green <- NOT caught
probe (blocked storage + branding re-render):
PR : MOUNT {enabled:false,persistent:false} -> setEnabled(true) -> 1 notification
-> AFTER BRANDING CHANGE {enabled:true,persistent:false} -> 2 notifications
M2 : AFTER BRANDING CHANGE {enabled:false,persistent:false} -> still 1 notification <- temporary opt-in revoked
Add one case reusing the blocked-storage shape around the branding change: make setItem throw before setEnabled(true), then re-render with a different appName/iconUrl and assert enabled is still true, persistent is still false, and the second notification still arrives with the new title.
web-shell-browser-notification-branding.md:15 requires “If storage is unreadable, start off rather than assuming a missing preference; the user may still explicitly enable notifications temporarily”, and the assertions at lines 229-230 must keep holding — so the new case may reach enabled: true only through the explicit setEnabled(true), never through the host's default.
Acceptance criterion: that new case must go red under the M2 mutant while today's line-327 assertion and the notify('second') delivery both still pass.
中文说明
这个品牌测试中“不重置偏好”的那一半没有区分能力：在品牌重渲染之前，setEnabled(true) 已把 'true' 写入被打桩的 localStorage，因此任何尊重存储的偏好重算都会得到 enabled: true；而文件中没有任何测试把“非持久偏好”与“品牌变更”组合在一起。
两种变异形态里，一种已被捕获，一种没有。在 options 变化时重算并实时读取 options?.defaultEnabled，会被 index.test.tsx:200 捕获；但在 options 变化时使用挂载时快照重算，没有任何测试能发现——全部 7211 个已发布测试保持绿色，包括本用例。未覆盖的回归是真实的：存储被阻止的用户（Safari 隐私模式或第三方存储拦截，即 214-219 行已搭建的形态）临时开启后（enabled: true、persistent: false、未写入任何值），一旦白标宿主以新的 appName 或 iconUrl 重渲染，就会失去通知，因为 readStoredPreference() 返回 undefined，而 stored === null && defaultEnabled 为假。投递会在会话中途停止且没有任何用户操作，与本测试名称所声称的条款相矛盾。
建议新增一个用例，在品牌变更周围复用“存储被阻止”的形态：在 setEnabled(true) 之前让 setItem 抛错，然后用不同的 appName/iconUrl 重渲染，断言 enabled 仍为 true、persistent 仍为 false，且第二条通知仍以新标题送达。
修复约束：web-shell-browser-notification-branding.md:15 要求“If storage is unreadable, start off rather than assuming a missing preference; the user may still explicitly enable notifications temporarily”，且 229-230 行的断言必须继续成立——因此新用例只能通过显式的 setEnabled(true) 达到 enabled: true，绝不能通过宿主的默认值。
验收标准：在 M2 变异下该新用例必须变红，而当前第 327 行的断言与 notify('second') 的投递仍应通过。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| ], | ||
| [ | ||
| 'failure', | ||
| 'turn_error', |
There was a problem hiding this comment.
[Suggestion] R4-9: The outcome table that pins notification body assembly covers only completed and failed; the third shipped outcome, ended, never reaches the component in any test, so the reply-excerpt branch is pinned on one side only.
Every stopReason in this file is 'end_turn' or 'cancelled' (8 occurrences), and neither 'This turn has ended.' nor its Chinese equivalent appears anywhere in it. Yet ended is live production code: turn-notification-context.ts:399-406 routes any stopReason that is neither 'cancelled' nor 'end_turn' there, and turn-notification-context.test.ts:22 pins the ['max_tokens', 'ended'] mapping at observer level only. Narrowing the body condition from turn.outcome === 'failed' to turn.outcome !== 'completed' leaves all 77 tests in the witness population green while the Reply: line disappears from every max_tokens or stop_sequence notification — precisely the long turn a user is most likely to have walked away from. The i18n key exists in both locales, so this is a coverage gap rather than a raw-key leak.
Witness:
MUTANT `turn.outcome === 'failed'` -> `turn.outcome !== 'completed'` at browser-turn-notifications.tsx:252:
notification-text 5 | turn-notification-context 24 | index.test.tsx 21 | browser-turn-notifications 27
-> Tests 77 passed (77)
probe (stopReason 'max_tokens', responseText 'partial but real reply'):
INTACT body: "This turn has ended. Return to check the result.\nPrompt: Question\nReply: partial but real reply"
MUTANT body: "This turn has ended. Return to check the result.\nPrompt: Question"
Add a fourth entry to that table carrying stopReason: 'max_tokens' and a real responseText, asserting the body is 'This turn has ended. Return to check the result.\nReply: partial but real reply' — note that is the actual English string at i18n.tsx:3584-3585, not 'Return to view the details.'.
Acceptance criterion: that new entry must go red when the condition at browser-turn-notifications.tsx:252 is narrowed to turn.outcome !== 'completed'; it also pins the browserNotifications.ended string, which no current test renders.
中文说明
用于锁定通知正文组装的 outcome 表只覆盖 completed 与 failed；第三种已发布的 outcome ended 在任何测试中都没有到达组件，因此回复摘录分支只有一侧被锁定。
本文件中所有 stopReason 都是 'end_turn' 或 'cancelled'（共 8 处），且 'This turn has ended.' 及其中文等价文案在文件中完全没有出现。但 ended 是实际生效的生产代码：turn-notification-context.ts:399-406 会把既不是 'cancelled' 也不是 'end_turn' 的任何 stopReason 归入其中，而 turn-notification-context.test.ts:22 只在 observer 层锁定了 ['max_tokens', 'ended'] 的映射。把正文条件从 turn.outcome === 'failed' 收窄为 turn.outcome !== 'completed'，见证范围内的 77 个测试仍全部通过，而每条 max_tokens 或 stop_sequence 通知的 Reply: 行都会消失——这恰恰是用户最可能已经离开的长回合。i18n key 在两种语言中都存在，所以这是覆盖缺口而非 key 泄漏。
建议在该表中新增第四项，带 stopReason: 'max_tokens' 与真实的 responseText，断言正文为 'This turn has ended. Return to check the result.\nReply: partial but real reply'——注意这才是 i18n.tsx:3584-3585 中真实的英文文案，而不是 'Return to view the details.'。
验收标准：把 browser-turn-notifications.tsx:252 的条件收窄为 turn.outcome !== 'completed' 时，该新条目必须变红；它同时也会锁定目前没有任何测试渲染过的 browserNotifications.ended 文案。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| : block.text; | ||
| if ( | ||
| visibleText && | ||
| !/\{\s*"insight_(?:progress|ready|error)"\s*:/.test(visibleText) |
There was a problem hiding this comment.
[Suggestion] R4-10: This insight-leak guard is coarser than its purpose: a reply that merely contains a brace-prefixed marker-shaped string the renderer could not parse loses its entire responseText, including the visible prose that precedes it, so the enriched notification degrades to status-only.
For this suite's own fixture at turn-notification-context.test.ts:422 — Report ready, a complete insight_ready marker, then an unbalanced insight_error fragment — splitInsightSegments cleanly isolates Report ready as a text segment, but the unbalanced tail makes extractJsonObject return null so it is pushed back as another text segment (transcriptToMessages.ts:1795-1798); the whole-text regex then matches and responseText is never assigned, so the user's visible line is thrown away with the payload. The commoner case is a reply quoting a near-miss marker, e.g. an insight_progress object whose progress member is not a number: parseInsightJson requires typeof prog['progress'] === 'number' (transcriptToMessages.ts:1694-1701), returns null, segmentation returns null, the same regex matches the whole text, and the entire reply excerpt disappears. Any turn whose answer discusses or documents this protocol gets a bodyless notification.
Witness:
A 'Report ready\n{insight_ready ok}\n{"insight_error":{"path":"/private'
segments [["text","Report ready"],["insight",…],["text","{\"insight_error\"…"]] -> {"sessionTitle":"Title"} <- prose discarded
B 'Build finished in 42s.\n{"insight_progress":{"stage":"build","progress":"high"}}'
segments null -> {"sessionTitle":"Title"} <- prose discarded
C 'Report ready\n{insight_ready ok}' -> {"sessionTitle":"Title","responseText":"Report ready"}
D 'the marker "insight_ready": is emitted' -> responseText preserved verbatim
E 'Here is the protocol:\n{"insight_error":{"error":"boom"}}' -> responseText "Here is the protocol:"
F '{"insight_ready":{"path":"/private' -> {"sessionTitle":"Title"} (correct, pinned at :429-435)
candidate segment-level fix: A -> responseText "Report ready"; B unchanged; F still suppressed; 23/24 suite green,
the single failure being the pinned assertion at :418-428
Two limits on the claim, both measured: the guard requires a leading brace, so a bare prose mention of "insight_ready": is preserved verbatim (case D); and this fails closed — the outcome is a less informative banner, never a leak.
Filter at segment level rather than discarding the whole string: drop only marker-shaped text segments and keep the whole-text guard for the segments === null path, then assign the remaining visible text. That restores A while keeping F suppressed. Case B needs a separate step (trimming only the marker-shaped tail from the raw text when segmentation returns null), which was not run and is a hypothesis rather than a measured fix.
turn-notification-context.test.ts:429-435 asserts a fragment-only block yields { sessionTitle: 'Title' }, and a slice(0, marker.index) fix keeps that ('' is falsy). Note also that fail-closed suppression of a malformed payload is a deliberate, already-ruled decision on this PR — the R1-7 thread and the design's “visible text is retained while internal payloads are omitted”. This finding is about the preceding visible prose, which that rationale does not cover, and a maintainer may reasonably decline it.
Acceptance criterion: updating :418-428 to assert responseText: 'Report ready' is itself the witness — red today, green only with the segment-level filter, while :429-435 must keep asserting { sessionTitle: 'Title' }.
中文说明
这个防止 insight 泄露的判断比其目的更粗糙：只要回复中包含一个带左花括号、但渲染器无法解析的类标记字符串，整段 responseText 就会被丢弃，连同它前面的可见正文一起，于是本应更丰富的通知退化为只有状态行。
以本套件自己的夹具（turn-notification-context.test.ts:422）为例——Report ready、一个完整的 insight_ready 标记、再加一个不闭合的 insight_error 片段——splitInsightSegments 已把 Report ready 干净地识别为 text 段，但不闭合的尾部使 extractJsonObject 返回 null，因此它又作为另一个 text 段被推回（transcriptToMessages.ts:1795-1798）；随后整段文本的正则命中，responseText 从不被赋值，用户可见的那一行连同载荷一起被丢掉。更常见的情况是回复中引用了一个“近似但不匹配”的标记，例如 progress 成员不是数字的 insight_progress 对象：parseInsightJson 要求 typeof prog['progress'] === 'number'（transcriptToMessages.ts:1694-1701），返回 null，分段返回 null，同一个正则命中整段文本，回复摘录整体消失。任何在回答中讨论或记录该协议的回合都会得到一条没有正文的通知。
对结论的两点限定（均已实测）：该判断要求以左花括号开头，因此正文中裸写 "insight_ready": 会被原样保留（用例 D）；并且这是 fail-closed 的——结果是信息更少的横幅，绝不会泄露。
建议在段级别过滤，而不是丢弃整段：只丢掉形态像标记的 text 段，并保留 segments === null 路径上的整段判断，然后赋值剩余的可见文本。这样可恢复 A，同时仍然抑制 F。用例 B 需要另一步（在分段返回 null 时只裁掉原始文本中形态像标记的尾部），该步骤未实际运行，属于设想而非已验证的修复。
修复约束：turn-notification-context.test.ts:429-435 断言仅含片段的 block 得到 { sessionTitle: 'Title' }，而 slice(0, marker.index) 的修法能保持这一点（'' 为假值）。另请注意：对畸形载荷采取 fail-closed 抑制是本 PR 上已有明确结论的决定——见 R1-7 讨论串以及设计文档中的“visible text is retained while internal payloads are omitted”。本条针对的是其前面的可见正文，该理由并不覆盖它，维护者完全可以合理地拒绝。
验收标准：把 :418-428 改为断言 responseText: 'Report ready' 本身就是见证——当前为红，只有加入段级过滤后才为绿，同时 :429-435 必须继续断言 { sessionTitle: 'Title' }。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| if ( | ||
| delimiter && | ||
| fence && | ||
| delimiter[1].startsWith(fence) && |
There was a problem hiding this comment.
[Suggestion] R4-11: Both clauses that discriminate a closing fence from an opening one — startsWith(fence), because a longer run of the same character may close, and !delimiter[2].trim(), because a closing fence may not carry an info string — are load-bearing and correct, but nothing in the package exercises either.
Collapsing lines 21-26 to if (delimiter && fence && delimiter[1] === fence) { leaves 56/56 green across notification-text.test.ts (5), browser-turn-notifications.test.tsx (27) and turn-notification-context.test.ts (24) — the complete witness population, since this module has exactly two non-test importers (browser-turn-notifications.tsx:16, daemon/session/turn-notification-context.ts:12). The same mutation changes real output: a prompt opening a ```ts fence and closing it with a longer run followed by prose with `**` emphasis gains a stray fence line and keeps raw `**`, and every line after the missed close silently loses fence protection, so the emphasis and link strippers start eating code. The info-string half diverges too: for a markdown outer fence containing a ```ts inner fence and then a ts line, the shipped code correctly keeps ts as fence content while the mutant treats it as the close. markdown-it, run as the CommonMark authority on the same inputs, agrees with the shipped code on both discriminators and disagrees with the mutant.
Witness:
INTACT: notification-text 5 | turn-notification-context 24 | browser-turn-notifications 27 -> Tests 56 passed (56)
MUTANT (`delimiter[1] === fence`): -> Tests 56 passed (56)
Fixture A = "```ts\nconst a = 1;\n````\nSome **bold** prose"
INTACT ["const a = 1;","Some bold prose"] MUTANT ["const a = 1;","````","Some **bold** prose"]
Fixture B = "````markdown\n```ts\ninner\n````ts\nstill content\n````\ntail prose"
INTACT ["```ts","inner","````ts","still content","tail prose"] MUTANT ["```ts","inner","still content","tail prose"]
markdown-it A: <pre><code class="language-ts">const a = 1;\n</code></pre><p>Some <strong>bold</strong> prose</p> == INTACT
markdown-it B: <pre><code class="language-markdown">```ts\ninner\n````ts\nstill content\n</code></pre> == INTACT
The cost is that these two clauses sit directly under an opening branch that looks like their mirror image, so “simplify the duplicate fence check” is the natural next edit here and no gate stops it. Add one fixture to notification-text.test.ts pinning both halves — a longer closing run followed by prose with emphasis, and an outer fence whose content includes an info-string-bearing line of the same character — written against notificationTextLines so it does not have to model the double clean reported separately at line 57.
The closer already depends on both captured groups (:24-25), so any widened info-string group must preserve “no trailing content on a closing fence”, or an unterminated block swallows the rest of the prompt as raw code.
Acceptance criterion: that fixture must go red when lines 21-26 are collapsed to delimiter[1] === fence; today the collapse passes 56/56.
中文说明
用于区分“闭合围栏”与“开启围栏”的两个条件——startsWith(fence)（因为同字符的更长串可以闭合）与 !delimiter[2].trim()（因为闭合围栏不能带 info string）——都是承重的且实现正确，但整个 package 中没有任何测试执行到它们。
把 21-26 行坍缩为 if (delimiter && fence && delimiter[1] === fence) {，notification-text.test.ts（5）、browser-turn-notifications.test.tsx（27）与 turn-notification-context.test.ts（24）共 56/56 全部通过——这已是完整的见证范围，因为该模块只有两个非测试引用方（browser-turn-notifications.tsx:16、daemon/session/turn-notification-context.ts:12）。同一变异会改变真实输出：一个以 ```ts 开启、用更长的 串闭合、后面跟着带 `**` 强调的正文的提问，会多出一行游离的围栏标记并保留原始 `**`，而且漏掉闭合之后的每一行都会静默失去围栏保护，于是强调与链接的剥离规则开始吃掉代码。info string 那一半同样分歧：对于外层 markdown 围栏内含 ```ts 内层围栏、随后又有一行 ts 的输入，现有代码正确地把 ts 当作围栏内容，而变异体把它当成闭合。以 markdown-it 作为 CommonMark 权威对同样输入运行，两个判别条件上都与现有代码一致、与变异体不一致。
代价在于：这两个条件正位于一个看起来像它们镜像的开启分支之下，因此“简化这个重复的围栏判断”是此处最自然的下一步改动，而没有任何门禁能拦住它。建议在 notification-text.test.ts 中补一个夹具，同时锁定两半——一个更长的闭合串后跟带强调的正文，以及一个外层围栏、其内容包含同字符且带 info string 的行——并直接针对 notificationTextLines 编写，这样就不必模拟第 57 行另行报告的二次清洗。
修复约束：闭合判断已经依赖两个捕获组（:24-25），因此任何放宽的 info string 组都必须保留“闭合围栏不得有尾部内容”，否则一个未终止的代码块会把提问的剩余部分整体当作原始代码吞掉。
验收标准：把 21-26 行坍缩为 delimiter[1] === fence 时，该夹具必须变红；目前这种坍缩能通过 56/56。
— qwen3.8-max via Qwen Code /review (v0.23.2)
Maintainer verification — real daemon + real browser notificationsI rebuilt this locally and drove it end to end rather than re-reading the diff. Head The gap I wanted to close is the one every previous round named itself: all earlier evidence — the author's Chrome runs and the sandboxed lane — observed a stubbed Harness (
The feature is load-bearing, and it works
On a fresh browser profile the base build delivers nothing (public default off) and the PR delivers one notification carrying this turn's session title, prompt and reply.
Clicking the real
One finding (Suggestion, not a blocker)The excerpt cleaner still strips
Adjudicated against Severity stays low on purpose: the value is a transient excerpt, the transcript and stored messages are untouched, shared storage holds only hashes, and the base build shows no prompt text at all — new surface, not a regression. I also re-measured the standing The one thing that is genuinely a maintainer call
The built-in page now starts with notifications on, and the content those notifications carry changed from a generic status line to the session title plus prompt and reply excerpts. I measured all four preference × permission cells: the page never calls That is disclosed in the PR body and the original design is marked historical, so it is a deliberate product decision rather than a defect — but it is the decision worth making explicitly before merge. Defaulting the built-in page off and letting the existing toggle carry the new content would cost the feature very little. Gates
Not covered: the native OS banner drawing itself (headless Chromium has no notification presenter — the Verdict: good to merge. The behaviour is real, the navigation chain works on a real daemon, and the privacy properties the PR claims (no partial replies or error details on failure, hashes only in shared storage, no automatic permission prompt) all hold under measurement. The 中文说明维护者验证 —— 真实 daemon + 真实浏览器通知本地重新构建并端到端跑通，而不是只读 diff。Head 我想补上的，正是此前每一轮自己都点名的缺口：作者的 Chrome 验证与沙箱验证观测的都是被 stub 的 测试环境（
功能承重，且确实可用全新浏览器 profile 下，base 构建一条都不发（公共默认关闭），PR 发出一条，包含本轮会话标题、提问与回复。 点击真实 一条发现（建议级，不阻塞）摘录清理仍然在不要求定界符 flanking 的情况下剥离 用仓库自带的 严重度刻意定低：只影响一条临时通知摘录，transcript 与已存消息不受影响，共享存储只有哈希，而 base 构建根本不展示提问文本 —— 属于新增面，不是回归。 我也独立复测了遗留的 真正需要维护者拍板的一点内置页面现在默认开启通知，而通知内容也从通用状态行变成了会话标题加提问、回复摘录。我实测了偏好 × 权限四个格子：页面从不主动调用 PR 描述已披露此事，原设计也标记为历史，因此这是有意的产品决定而非缺陷 —— 但它值得在合并前被明确地拍一次板。把内置页面默认改回关闭、让已有开关承载新内容，几乎不会损失什么。 门禁
未覆盖：原生系统横幅的绘制本身（headless Chromium 没有通知呈现器， 结论：可以合并。 行为是真实的，导航链路在真实 daemon 上成立，PR 声称的隐私性质（失败回合不带部分回复与错误详情、共享存储只有哈希、不自动申请权限）在实测中全部成立。 |
|
@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: 442 passed · 0 failed · 442 total Flakiness gate: ✅ 7 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：442 通过 · 0 失败 · 442 总计 抖动门：✅ 7 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 11447 deep verification (round 3) —
|
| # | finding | severity (round 2) | status at this head |
|---|---|---|---|
| 1 | Excerpt cleaner guesses Markdown emphasis on text that is code (**, indented code block) |
Suggestion | stands, refined — re-measured live on the new merged tree (logs/cleaner.json, 187 assertions). Under a text-equivalence oracle (micromark + GFM) there are 4 divergences from 2 root causes, not 2 divergences: the two round-2 rows reproduce byte-for-byte, and the sibling sweep adds indented-code-4-space / indented-code-8-space (same root cause). Round 2's adjudication of genuine-strike as "matches CommonMark" is confirmed, and I show below that a core-only oracle would have filed it as a false positive. |
| 2 | defaultEnabled inert when browserNotifications arrives after first render |
Nice to have | stands — re-measured live on a fresh mount with no explicit choice (S24b): L4.enabled-after-late-defaultEnabled: false, L4.no-permission-request: 0, L4.no-delivery: 0. The L5 control on the same arm confirms defaultEnabled present at first render does work, and S24's L3 control re-confirms appName/iconUrl are re-read. Unchanged: documentation-precision issue. |
| — | Round 2's open question: does handleOpenSessionFromOverview's new split teardown change qwen-session:// markdown-link behaviour? |
(unresolved) | RESOLVED — it does. Promoted to finding 3 below, with a 24-assertion A/B (logs/split-compare.txt). |
Nothing worsened. No finding was declined by the author, so there is no declined-tradeoff row to re-price.
Central claim and A/B proof
Central claim. When a turn reaches a terminal state while the page is unfocused, the notification identifies the session (appName · session title), carries bounded plain-text excerpts of this turn's prompt (≤80 code points) and final main-assistant reply (≤120), omits partial replies and error details on failure, omits transport attachment tails and internal insight payloads, and a click dispatches navigation to the captured session on the owning provider's EventTarget only.
Oracle. The arguments actually reaching the window.Notification constructor (title, body, icon, tag, renotify) plus the qwen:open-session CustomEvent a click dispatches. The harness drives the real BrowserTurnNotifications provider, the real createTurnNotificationObserver, the real useTurnNotificationBinding (cell S20b) and the real getTurnNotificationContent / notification-text cleaner, rendered by React into jsdom. Nothing on the path under test is stubbed; only platform APIs jsdom lacks are provided, and Notification is the observation point.
Control. One byte-identical harness file (sha256 c981a9fa6ebabdea74f69622adfa56d94e7e6d82e467929d7d2353497be743e8, asserted equal in both trees) is dropped into each tree and run with that tree's own vitest.config.ts. The arm is detected at runtime from whether the tree exports getTurnNotificationContent / TurnNotificationNavigationContext — never from a flag — so neither side can be told what to expect. Base control cells assert the absence the PR exists to remove, so base "failing" on the feature is encoded as a passing control assertion; that is why fail is 0 on both arms.
| arm | tree | assertions | notifications constructed |
|---|---|---|---|
| head (PR) | /__w/qwen-code/qwen-code @ 64a75383 |
132 pass / 0 fail | 29 |
| base (control) | tmp/base-tree @ af4dece3 |
98 pass / 0 fail | 31 |
observable_differences = 32/32. Every aligned position of the constructed-notification stream differs between the arms. 30 of 33 cells differ; the 3 that do not (S00, S24b, S30) are cells where neither arm constructs a notification at all, which is the correct identical result. Witness: 01-ab-notification-boundary-head-vs-base.png; per-assertion data logs/obs-head.json / logs/obs-base.json, raw vitest output logs/ab-head.log / logs/ab-base.log, the aligned cell table logs/ab-compare.txt.
Representative cells (real strings, not paraphrases):
| # | head | base |
|---|---|---|
| S02 | QwenCode · Fix the login bug / status + Prompt: + Reply: / icon present |
Qwen Code / This turn has completed. only / no icon |
| S03 | QwenCode · Use Map<string, number> here — generics intact |
Qwen Code, no prompt carried |
| S04 | Prompt: Is 3 < 5 and 7 > 2 correct? — comparisons intact |
no prompt carried |
| S05 | failed turn: Prompt: shown, no Reply:, no ENOENT, no partial-reply leak |
status only |
| S06 | title 60 / prompt 80 / reply 120 code points, each ending … |
Qwen Code, single line |
| S07 | 100 emoji → 80 code points / 159 UTF-16 units, no lone surrogate | no prompt carried |
| S09 | ```html fence → <div class="x">hi</div> kept literally |
no prompt carried |
| S10 | @attachment:///tmp/secret-payload.txt tail dropped, visible prompt kept |
no prompt carried |
| S18 | Acme Agent · Branded, icon = the CDN URL |
Qwen Code, no icon |
| S19 | QwenCode with no · separator when the title is empty |
Qwen Code |
| S25 | 本轮已完成。 + 提问：中文提问 + 回复：好的 |
本轮已完成。 only |
The four cells where the delivery count itself differs:
| flip | head | base |
|---|---|---|
S15 mid_turn_message_injected then terminal |
1 — injected prompt suppressed, never-injected sibling still delivers | 2 — base has no injection handling, both deliver |
| S16 1500 admissions, then two replayed terminals | 1 — evicted p1 suppressed, p1500 kept |
2 — base's pending is an uncapped Set |
S17 active: false |
0 — observer, settings and navigation contexts all undefined |
1 — base has no such opt-out, observer attached and delivers |
S20b real useTurnNotificationBinding |
1 — click dispatches exactly 1 event on the owning EventTarget, 0 on window, carrying {sessionId:'session-1', sessionContext:{kind:'workspace', cwd:'/repo/proj'}}, then closes |
0 — base exports no TurnNotificationNavigationContext, so there is no navigation target to dispatch to |
Harness validity controls — all run on BOTH arms.
S01.delivered-on-this-armrequires a notification on both arms. This is load-bearing, and it earned its keep this round: my first base-arm run produced 0 notifications in every cell and 34 spurious failures, becausedefaultEnabledis a head-only knob (base'sreadPreference()takes no argument and returnsstored === 'true'). WithoutS01that would have read as "the PR changed nothing observable". The fix isrenderEnabled(), which makes the explicit user choice on both arms so the A/B measures notification content and navigation rather than a preference default the base cannot express.S15.control-sibling-still-deliversfires a second, never-injectedpromptIdafter asserting silence. Without it, every "silent" assertion would pass vacuously on a harness that can never deliver.S23keeps the storage-throw trap pinned:vi.spyOn(localStorage,'getItem')does not intercept in jsdom (Storage is a Proxy), so the cell makes the access itself throw. It also moves the platform topermission === 'default'before the explicit choice, because with'granted'the provider skipsrequestPermissionand a "0 calls" reading would be an artefact of the stub rather than of the PR.S20(driven straight through the observer, so notargetis attached) asserts a targetless notification navigates nowhere but still closes — which is what makesS20b's single dispatch on the owning target a measured property rather than an assumption.
Findings
Finding 3 (Suggestion, NEW this round) — the shared navigation handler now tears down split state on two pre-existing link paths, and fires a host callback it never fired before
Round 2 raised this and explicitly did not resolve it. It is resolved now, by measurement.
handleOpenSessionFromOverview is registered as the window listener for qwen:open-session, and three dispatchers share it (grep -rn "new CustomEvent('qwen:open-session'" over client/, excluding tests):
| dispatcher | detail shape | new in this PR? |
|---|---|---|
browser-turn-notifications.tsx:280 |
{sessionId, sessionContext} object |
yes |
components/messages/Markdown.tsx:788 (qwen-session:// links) |
bare string | no — pre-existing |
components/messages/ToolGroup.tsx:1136 |
bare string | no — pre-existing |
The PR adds to that handler:
if (mainView === 'split' || splitFoldedByShrinkRef.current) {
notifyControlledSplitClose();
clearSplitSessions();
}loadSidebarSession does neither (read at App.tsx:12946 — it only sets splitFoldedByShrinkRef.current = false), so these are genuinely new side effects on the two pre-existing paths. Measured with a probe appended byte-identically (sha256 88c3602d5e45005aecd7c0176b33c02818437a0bc3d92d9d29194d5d2f48392d) to a copy of App.test.tsx in each tree, driving the real App and dispatching exactly what Markdown.tsx:788 dispatches:
| cell | head | base |
|---|---|---|
| ZZ0 control: split's own back button | ["s1","s2"] → [] |
["s1","s2"] → [] (identical — the probe is live on both arms) |
| ZZ1 markdown link, bare string, in split view | persisted ["s1","s2"] → **[]** |
persisted ["s1","s2"] → **["s1","s2"]** |
| ZZ2 same, to an unknown session id | persisted → [] |
persisted → ["s1","s2"] |
| ZZ3 notification-shaped object detail | persisted → [] |
persisted → ["s1","s2"] |
ZZ4 externally-controlled split (splitSessionIds + onSplitSessionIdsChange) |
host called with [[]], i.e. onSplitSessionIdsChange([]) |
host called 0 times |
| ZZ5 control: not in split view | nothing torn down, no host call | nothing torn down, no host call |
24 scripted assertions, 24 pass. Witness 02-split-teardown-markdown-link-head-vs-base.png; raw logs/split-head.log, logs/split-base.log, logs/split-compare.txt.
Is it a defect? Partly in the PR's favour, and the report should say both halves:
- In its favour: both arms leave the split view visually (
still-in-split-view: falseon both), so base left the persisted split behind after the user had visibly navigated away — a refresh would have restored a split the user just left. That contradictshandleSplitExit's own documented intent ("The user left the split of their own accord, so a refresh must not bring it back"). Head makes markdown-link navigation consistent with it. - Against: this is a behaviour change to a public prop contract (
onSplitSessionIdsChange) on a code path the PR description never mentions. A host that treats[]as "the user closed the split" now receives that signal from aqwen-session://click inside a pane. The PR description says the change is additive — "The public Web Shell options are additive; there are no new daemon routes or wire fields" — which is true ofbrowserNotificationsbut does not cover this.
Bounded, and what does NOT hold. No state desync: when the host re-asserts the same ['s1','s2'] after the callback, the split does not reopen on either arm (ZZ4.split-view-after-rerender: false on both), so the callback is informational. In the uncontrolled case no host call happens at all (ZZ1.host-callback-calls: []), so only hosts using splitSessionIds + onSplitSessionIdsChange observe the change. And the blast radius is exactly three dispatchers plus SessionOverviewPanel (App.tsx:17715); the scheduled-tasks and goals panels at App.tsx:17856 and App.tsx:18031 use their own inline onOpenSession calling loadSidebarSession directly and are unaffected.
What I could not demonstrate. The teardown runs before the fallible loadSidebarSession, so a rejected load destroys the persisted split with nothing gained — head ZZ2.persisted-after-failed-load: [] vs base ["s1","s2"]. But the harness's mock daemon accepted the unknown session id, so no rejection was actually produced (ZZ2.error-reported: false on both arms, asserted as ZZ2.no-load-rejection-was-actually-produced). The ordering hazard is a code read, not a demonstrated failure; I am not claiming it reproduces.
Suggested change (documentation-level, preserves the commit's intent)
The code is defensible as-is; what is missing is disclosure. Two options, cheapest first:
- Add one line to the PR description's Breaking changes / migration notes — the split teardown now also applies to
qwen-session://markdown links and ToolGroup session links, and hosts usingonSplitSessionIdsChangewill receive[]from those clicks in split view. - If the author intends the teardown to be notification-only, gate it on the caller rather than on
mainView, e.g. pass a flag from theqwen:open-sessionbranch that only the notification dispatcher sets. I did not measure this variant, so it ships as a sketch, not a measured fix.
Finding 1 (Suggestion, stands — refined by a stronger oracle)
Re-measured on the new merged tree. notification-text.ts is byte-identical to what round 2 measured (sha256 bd9cb629b041951bf8506b0ba361bcf158ffa827c9bb843ef320e76965af2bb6, and git diff --stat HEAD^2 HEAD over the three notification modules is empty), but the file's sibling App.tsx and the whole tree around it moved, so the harness was re-run rather than carried forward: 187 assertions, 0 fail, 75 recorded rows (logs/cleaner.json).
Oracle correction first, because it changes the answer. Round 2 adjudicated with micromark and asked "did the reference emit an emphasis tag". That predicate is incomplete in two directions, and I hit both:
micromarkcore does not implement GFM strikethrough. It rendersThe ~~struck~~ word mattersas<p>The ~~struck~~ word matters</p>, so a core-only oracle flags the cleaner's~~handling as a divergence. Undermicromark+micromark-extension-gfm(installed in this repo) the reference rendersThe struck word matters— byte-identical to the cleaner. My first pass filed this as a third sibling divergence; it is a false positive of the instrument, and I am reporting it as such rather than as a finding against the author.- Asking about tags also misses cases where the reference consumes a delimiter without emitting a tag. So the primary oracle here is plain-text equivalence: strip tags from the GFM render, decode entities, collapse whitespace, compare to the cleaner's output (
logs/gfm-reference.json).
Under that oracle:
| input | cleaner output | GFM reference text | verdict |
|---|---|---|---|
compute 2 ** 3 ** 4 |
compute 2 3 4 |
compute 2 ** 3 ** 4 |
diverges |
const a = 1; / const b = **bold**; |
const a = 1; | const b = bold; |
const a = 1; const b = **bold**; |
diverges (<pre><code>) |
const x = **y**; (4-space sibling) |
const x = y; |
const x = **y**; |
diverges (same root cause) |
const x = **y**; (8-space sibling) |
const x = y; |
const x = **y**; |
diverges (same root cause) |
why does ls **/*.ts **/*.js differ |
why does ls /*.ts /*.js differ |
why does ls **/.ts **/.js differ |
both corrupt it, differently — see below |
The ~~struck~~ word matters |
The struck word matters |
The struck word matters |
matches |
The **bold** word matters |
The bold word matters |
The bold word matters |
matches |
- 5 degrees below zero |
5 degrees below zero |
5 degrees below zero |
matches |
* 2 * 3 = 6 |
2 * 3 = 6 |
2 * 3 = 6 |
matches |
#include <stdio.h> is first |
unchanged | unchanged | matches |
Use Map<string, number> here |
unchanged | unchanged | matches |
So the finding is real and has two root causes, exactly as round 2 said, plus two more instances of the second one:
- a
**/~~pair is stripped without requiring the delimiters to be flanking, so2 ** 3 ** 4loses both pairs where GFM keeps them literal; ```/~~~fences are protected but the equally-valid four-space indented code block is not, soconst b = **bold**;is stripped where GFM says the content is code and must survive literally. The sibling sweep confirms this at 4 and 8 spaces and confirms the boundary is correct at 3 spaces (indented-code-3-spacedoes not diverge, matching CommonMark's own limit).
shell-glob deserves its own line because it is the one row where "match the reference" is not obviously the fix: GFM's own text is why does ls **/.ts **/.js differ — it eats a * inside emphasis — while the cleaner eats **. Neither preserves the user's literal glob, so round 2's "ambiguous markup" label was right in spirit and this row should not drive the fix.
29 further siblings came out clean, including the ones round 2 pinned: fence-backtick, fence-tilde, fence-crlf, fence-in-list, fence-indented-3, fence-indented-4, unclosed-fence, longer-fence, backtick-code-span-star, backtick-code-span-tilde, inline-html-raw, link, image, hr-dashes, trailing-lone-surrogate, astral-run. Every row was also asserted to stay within the 120-code-point bound, to carry no control or bidi character, and to carry no lone surrogate.
No superlinear blowup on outsider-authored text. 10 hostile shapes × 4 rungs (2 k / 3 k / 5 k / 20 k), 40 timed rungs: slowest 0.955 ms, and link-soup — the most expensive shape — goes 0.486 → 0.705 → 0.928 → 0.942 ms, i.e. flat past the cap. The structural reason is asserted rather than trusted: MAX_NOTIFICATION_SOURCE_LENGTH === 4096 and text.slice(0, …) runs before any regex, so no regex ever sees more than 4096 units.
Severity stays Suggestion, for the reasons round 2 gave and which still hold: the value is a transient notification excerpt, the transcript and stored messages are untouched, shared storage holds only hashes (re-asserted this round in S21), and base shows no prompt text at all so this is not a regression.
Not re-measured this round: round 2's candidate fix (flanking-only **/~~ plus treating ^ {4,}\S as code) and its measurements — 3/3 hostile shapes fixed, 0/8 benign collateral, suite green on both sides. That measurement's entire input closure is notification-text.ts plus the fixture strings, and I verified the module is byte-identical (sha256 above) with no caller or lockfile change feeding it — git diff --stat HEAD^2 HEAD over the notification modules is empty and the PR touches no package.json. I am carrying it forward under that closure argument rather than re-deriving it, and I make no new claim about it. The unpinned-axis conclusion stands: notification-text.test.ts is green with and without that patch, so the fixtures that would go red are the hostile rows in the table above.
Finding 2 (Nice to have, stands) — defaultEnabled is inert when the prop arrives after first render
WebShellWithProviders always renders BrowserTurnNotifications with active={browserNotifications !== undefined}, so the provider mounts on the host's first render and const [defaultEnabled] = useState(options?.defaultEnabled ?? false) freezes the value there. Re-measured on the new tree, on a fresh mount that never makes an explicit choice (S24b, which is the clean form of this measurement — round 2's cell could be confounded by an earlier setEnabled):
L1.starts-off false
L4.enabled-after-late-defaultEnabled false <- host passes { defaultEnabled: true } on a later render
L4.no-permission-request 0
L4.no-delivery 0
L5.control-early-defaultEnabled true <- same arm, defaultEnabled present at FIRST render
A host that loads its own configuration asynchronously gets a silent "off", with no prompt and no diagnostic. The knob is not wholly dead — S24's L3 control re-renders with {appName:'Late Brand', iconUrl:'https://cdn/x.png'} and both are honoured (L3.late-appName-is-re-read, L3.late-iconUrl-is-re-read). This matches the PR description ("Notification defaults are read at mount"); the ambiguous sentence is the design doc's "The initial default is read at mount even if integration is initially omitted". Suggest rewording to say defaultEnabled must be present on the first render. Evidence: logs/obs-head.json → S24/S24b.
Probed and did not hold
- The new fast path is unreachable from markdown links.
handleOpenSessionFromOverview's new early-return requiresexplicitContext, which is only built when the event detail is an object carrying a validsessionContext. Both pre-existing dispatchers send a bare string, sosessionContextstaysundefinedand the fast path cannot fire from aqwen-session://link or a ToolGroup link. Round 2's worry that markdown-link clicks might start skippingloadSidebarSessiondoes not hold; only the split teardown (finding 3) reaches them. - No cross-instance navigation leak.
S20b: a click dispatches 1 event on the owning provider'sEventTargetand 0 onwindow, carrying{sessionId, sessionContext:{kind:'workspace', cwd:'/repo/proj'}}, and closes the notification. Base has no navigation target at all. Two shells on one page cannot both navigate. - No content in shared storage, through the real
navigator.lockspath.S21: after a delivery the store holds exactly oneqwen-code-turn:<64 hex>tag and nothing else — asserted by enumerating every key and requiring each to be one of the two known keys. No title, prompt, reply, cwd or/repo/anywhere. (The preference key is legitimately absent underdefaultEnabled, becausesavePreferenceonly runs on an explicit choice;S22pins the written form as'true'/'false'.) - Astral, control and bidi handling is exact. 100 emoji → 80 code points / 159 UTF-16 units,
79 whole emoji + '…', no lone surrogate. NUL, BEL, RLO, ZWSP and LRI collapse toa b c d e fwith zero control or bidi characters surviving in body or title. A trailing lone surrogate in the source is stripped before splitting. - Bounds are code points, not UTF-16 units, and the boundary is exact: a 500-character input yields 80, an exactly-80-character input is returned untouched, and the empty string yields the empty string.
- Failure, cancellation and malformed events behave as documented.
turn_errorshows the prompt but noReply:line, no partial-reply text and noENOENT(S05);stopReason:'cancelled'is silent while'max_tokens'delivers an "ended" status (S27); aturn_completewith no stringstopReasonis ignored (S28); a foreignsessionIdin the envelope is ignored (S26); duplicate and replayed terminals are silent (S13/S14);remove()andrelease()both cancel (S29/S30) — each with a positive control on the same arm proving the harness can still deliver. - Transport and internal payloads do not leak.
@attachment:///tails are dropped (S10);{"insight_ready": …}produces noReply:line at all (S11);background_notificationandvision_bridge_noticeassistant blocks are skipped in favour of the last real main-assistant reply (S12). - Disabling really stops delivery, and unreadable storage really starts off.
S22: explicit off → 0 notifications, and the choice is persisted as'false'; re-enable → delivers.S23: throwing storage →enabled:false,persistent:false, 0 automatic permission requests, no delivery; an explicit choice is still honoured and does walkrequestPermissionexactly once. - zh-CN reaches the wire.
S25:本轮已完成。+提问：中文提问+回复：好的.
Targeted gates
| gate | command | result |
|---|---|---|
packages/web-shell unit tests @ the new merged tree |
npx vitest run --config vitest.config.ts |
305 files / 7289 tests passed, exit 0 (logs/gate-web-shell-head.log) |
| A/B harness, head arm | npx vitest run … zz-verify-ab |
132 pass / 0 fail |
| A/B harness, base arm | same bytes in tmp/base-tree |
98 pass / 0 fail |
| Split-navigation probe | … zz-verify-app-split -t 'ZZ VERIFY' |
6 tests ran (880 skipped) on each arm; 24 comparator assertions, 24 pass |
Cleaner vs micromark+GFM |
npx vitest run … zz-verify-cleaner |
187 pass / 0 fail, 75 rows |
Typecheck packages/web-shell |
npx tsc -p tsconfig.json --noEmit |
exit 0, no diagnostics (logs/typecheck-web-shell.log) |
Round 2 recorded 303 files / 7213 tests. The delta (+2 files, +76 tests, +0 failing) is main's advance, not the PR: linkify.test.ts, LinkifiedText.test.tsx and the UserMessage / useQueuedPrompts additions arrived with the new base. The gate ran on the merged tree, so it covers the PR and main's changes to the same package together — which is what actually lands.
The gate was run before any zz-verify-* file was created, so its counts are the PR's own: grep -c zz-verify logs/gate-web-shell-head.log is 0. The typecheck, by contrast, ran with the three zz-verify-* files present in client/; since adding files can only add diagnostics, exit 0 is a fortiori a clean result for the PR's own sources.
Control cleanliness (asserted, not assumed). The base worktree has no node_modules of its own; resolution walks up to the repo root, whose @qwen-code/* symlinks point into the head tree — the trap that makes a naive base control quietly load changed head code. It is safe here, and I proved the reason rather than assuming it: git diff --name-only HEAD^1..HEAD | grep -v -E '^(packages/web-shell/|docs/)' | wc -l is 0, so every other workspace, including packages/sdk-typescript which App.tsx imports, is byte-identical between the two arms by construction. The one gap this left was real and I hit it: @datafe-open/markdown-chart lives in a package-local packages/web-shell/node_modules, which the worktree lacked, so the base arm failed to collect with Failed to resolve import "@datafe-open/markdown-chart". Fixed by symlinking that one directory and re-running; the printed realpaths are in the transcript above. The arms are not accidentally identical: browser-turn-notifications.tsx sha256 5bf875a7… base vs 30aa9dbc… head, and notification-text.ts is absent on base.
Not covered
- Live native notification delivery in a real browser. Not re-attempted this round. Round 2 established headless Chromium here reports
Notification.permission === "denied"and proved it environmental with a control page importing no web-shell module. That is a property of the container, not of the tree, so it still holds; but I ran no new browser probe and make no new claim about icon decoding,dist/index.jsinlining, ornpm packcontents. Round 2's results on those stand on its own evidence. - Lint and formatting not run. eslint and prettier over
packages/web-shellwere not executed at all this round, and I did not plant a violation to prove either gate live. Round 2 carried these forward under a tree-hash argument that does not apply this round — the merged tree is new — so there is no verified lint/format claim for64a75383from this lane. The PR's own CI covers them. Typecheck was re-run and is reported in the gate table above. - No end-to-end daemon turn. The split-navigation probe drives the real
Appcomponent, but againstApp.test.tsx's mocked daemon; the transcript blocks andDaemonEvents in the notification A/B are hand-built to the SDK's real shapes. So this reproduces the wire shape of a terminal turn and the real component's handler, not the model-side and daemon-side path that produces them. Noqwen serverun. - A genuine
loadSidebarSessionrejection was never produced — see finding 3. The ordering hazard (teardown before the fallible load) is a code read, andZZ2measures the ordering, not a failure. - Per-commit attribution is out of reach.
git rev-parse --is-shallow-repositoryistrue, depth 2, so only the merge commit, the base tip and the PR head exist.git rev-list HEAD^1..HEAD^2returns a plausible small number at a shallow boundary rather than erroring, so I compared against the snapshot instead:$QWEN_VERIFY_CONTEXTlists 8 commits, of which only321f53b2is locally reachable. I verified the aggregateHEAD^1..HEADdiff only. The description's9056cebreproduction is not reachable; I proved instead that the fix holds at head (S03,S04,S09all intact). - No mutation matrix this round. Round 1 reported 15/15 killed with 0 survivors and every positive control landed in the mutated file. Those mutations targeted the PR's own sources, which are byte-identical at this head (the PR head OID did not move), so the result is probably unchanged — but the merged tree around them is new and I did not re-run it. Treat it as unverified at
64a75383. - No vacuity check on the PR's new tests this round.
notification-text.test.ts(5 tests) and the addedApp.test.tsx/browser-turn-notifications.test.tsxcases ran green inside the 7289-test gate, but I did not revert a source hunk to prove any of them can fail. Round 1 did this for the then-current tests; the merged tree adds main's tests to the same files, so the old result does not transfer cleanly. - App-side navigation validation beyond finding 3. The design doc lists further rejection rules for explicit contexts (malformed context,
workspaceCwdconflicting with astandalone/livecontext). I exercised the locked-workspace guard only by reading it, and the healthy-current-session fast path only viaS20b's detail shape plus the 880-testApp.test.tsxsuite inside the gate. - No trial merge beyond the merge ref itself. The checkout is shallow, so current
maincould not be fetched;HEAD^1(af4dece3) is the base GitHub used and is the newest base I can see. - Playwright / e2e specs not run (
vitest.config.tsexcludese2e/**, and I did not invoke the playwright config).
Methodology
Environment: the CI verify job container (node:22-bookworm), Node v22.23.2, npm 10.9.8, 64 cores, working tree at refs/pull/11447/merge (64a75383), with npm ci and npm run build already completed at head before this round started; neither was repeated. The base arm was a scratch worktree at tmp/base-tree pinned to HEAD^1 (af4dece3), needing no install of its own beyond one symlink for the package-local @datafe-open dependencies, and was removed with git worktree remove --force after the cells were captured; git status --porcelain is empty (excluding tmp/) at the end of the round.
Three harnesses drove the code, each kept in this directory so a maintainer can rerun it verbatim. verify-ab.test.tsx is arm-agnostic — the same bytes (sha256 c981a9fa…) in both trees — and drives the real provider, the real observer, the real useTurnNotificationBinding and the real content extractor into jsdom, observing the window.Notification constructor and the click CustomEvent; it writes per-assertion results and every constructed notification to logs/obs-{head,base}.json, and compare-ab.mjs aligns the two streams and computes observable_differences. verify-split-probe.append.tsx (sha256 88c3602d…) was appended to a copy of App.test.tsx in each tree — never to the tracked file — so it inherits that file's entire mock harness and drives the real App; it only measures and prints ZZRESULT <id> <json>, with every expectation asserted externally by compare-split.mjs, so a base-arm value that moves becomes a scripted failure rather than a re-read of two logs. verify-cleaner.test.ts exercises the shipped cleaner directly for the sibling sweep and the scaling ladder and adjudicates against micromark + micromark-extension-gfm, both dependencies this repo already ships, using plain-text equivalence rather than tag presence.
Raw per-cell logs live in logs/: ab-head.log, ab-base.log, ab-compare.txt, obs-head.json, obs-base.json, split-head.log, split-base.log, split-compare.txt, cleaner.log, cleaner.json, gfm-reference.txt, gfm-reference.json, gate-web-shell-head.log, typecheck-web-shell.log.
Assertion counts (assertions.json, 442 total, 0 fail): A/B head 132, A/B base control cells 98, cleaner + micromark/GFM + ladder 187, split-navigation comparator 24, unit-test gate 1. Base-arm control cells count as passes because each asserts that base lacks the behaviour; fail counts only unexpected outcomes, and there were none. Two harness faults found and fixed during the round are reported above rather than hidden: the base arm initially delivered 0 notifications everywhere (34 spurious failures, caught by the S01 control), and the split comparator initially dropped 16 of 24 keys because vitest indents grouped stdout under a header line.
Flakiness gate log
rounds=5 files=7 skipped=0
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/browser-turn-notifications.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/browser-turn-notifications.test.tsx
file packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/daemon/session/DaemonSessionProvider.test.tsx
file packages/web-shell/client/daemon/session/turn-notification-context.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/daemon/session/turn-notification-context.test.ts
file packages/web-shell/client/index.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/index.test.tsx
file packages/web-shell/client/main.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/main.test.tsx
file packages/web-shell/client/notification-text.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/notification-text.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/web-shell/client/App.test.tsx: PPPPP
packages/web-shell/client/browser-turn-notifications.test.tsx: PPPPP
packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: PPPPP
packages/web-shell/client/daemon/session/turn-notification-context.test.ts: PPPPP
packages/web-shell/client/index.test.tsx: PPPPP
packages/web-shell/client/main.test.tsx: PPPPP
packages/web-shell/client/notification-text.test.ts: 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/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/browser-turn-notifications.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/daemon/session/turn-notification-context.test.ts: P (exit 0)
round 1 · packages/web-shell/client/index.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/main.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/notification-text.test.ts: P (exit 0)
round 2 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/browser-turn-notifications.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/daemon/session/turn-notification-context.test.ts: P (exit 0)
round 2 · packages/web-shell/client/index.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/main.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/notification-text.test.ts: P (exit 0)
round 3 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/browser-turn-notifications.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/daemon/session/turn-notification-context.test.ts: P (exit 0)
round 3 · packages/web-shell/client/index.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/main.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/notification-text.test.ts: P (exit 0)
round 4 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/browser-turn-notifications.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/daemon/session/turn-notification-context.test.ts: P (exit 0)
round 4 · packages/web-shell/client/index.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/main.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/notification-text.test.ts: P (exit 0)
round 5 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/browser-turn-notifications.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/daemon/session/turn-notification-context.test.ts: P (exit 0)
round 5 · packages/web-shell/client/index.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/main.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/notification-text.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅ Approved at the commit I reviewed, after a maintainer independently verified the behaviour in a real browser against a real daemon. Two automated Criticals remain unresolved in the code; I checked both and adjudicated them non-blocking, with the reasoning and the deferral record in the stage 3 comment above.
6 participants
What this PR does
Browser notifications identify the session, show bounded excerpts of the current user prompt and final main-assistant reply, and open the captured session in the owning Web Shell when clicked. The default brand is QwenCode with the bundled icon. The public
browserNotificationsprop and exportedWebShellBrowserNotificationsOptionstype let hosts configureappName,iconUrl(including HTTPS CDN URLs), anddefaultEnabled. The public default is off; the built-in page explicitly starts on when storage confirms that no saved choice exists, while saved choices still win.Why it's needed
A generic completion banner is difficult to associate with one of several active sessions. The added context and navigation let users recognize the completed turn and return directly to it. Host applications can supply their own notification brand without changing the sidebar or browser-controlled site attribution.
Reviewer Test Plan
How to verify
With browser and OS permission granted, submit a request and move focus away from the page. Expect one notification with this turn's title, prompt, and reply. Clicking it should open its captured workspace, standalone, or Live session in the owning shell. From split view, clicking the healthy current session should reveal chat and focus the composer without reloading; missing or loading sessions should use the normal load path. A failed turn may show the prompt but must omit partial replies and error details.
Try prompts containing TypeScript generics, comparisons, fenced HTML, and invisible control characters. Code punctuation should survive, control and bidi-formatting characters should not, and long inputs should produce bounded excerpts. Attachment transport tails and internal insight payloads should not appear. Historical and duplicate terminal events should stay silent; mid-turn injected messages should be removed from pending notification state.
Check custom branding and a CDN icon URL through the public entry. Omit and restore the options without remounting or reconnecting the current session. The public preference defaults off; the built-in page explicitly defaults it on for a fresh site. Saved true/false choices take precedence. Unreadable storage starts off and still allows an explicit temporary choice; permission is never requested automatically. Disabling notifications must stop delivery.
Evidence (Before & After)
Before this PR, notifications had a generic title/status and clicking only focused the window. Existing Chrome verification captured real daemon turns, prompt/reply text, the PNG URL, and navigation back to the source session after switching sessions and opening Settings; detailed reports are in the PR comments. Notification API stubbing and simulated loss of focus establish application parameters and navigation, not native OS banner layout.
Review reproduction on
9056ceb:Use Map<string, number> herebecameUse Map here, andIs 3 < 5 and 7 > 2 correct?becameIs 3 2 correct?. The regression tests now preserve both, retain fenced HTML literally, remove controls, and restore split-view composer focus. Current validation results are recorded in the review-resolution comment.Tested on
Environment (optional)
macOS, Node.js v22.22.3, npm 10.9.8. Local workspace build, type checks, Vitest, and the existing Chrome/daemon verification described above.
Risk & Scope
Linked Issues
Follow-up to #11398.
中文说明
What this PR does
浏览器通知标识对应会话，展示本轮用户提问及最后一段主助手回复的有限摘录，点击后在所属 Web Shell 中打开已捕获的会话。默认品牌为 QwenCode 和随包图标。公开的
browserNotifications属性及导出的WebShellBrowserNotificationsOptions类型允许宿主配置appName、iconUrl（包括 HTTPS CDN 地址）和defaultEnabled。公共默认值为关闭，内置页面在存储确认没有保存选择时显式默认开启，已有选择仍优先。Why it's needed
多个会话并行时，通用完成通知难以对应到具体任务。增加上下文和导航后，用户能够辨认完成的回合并直接返回。宿主应用可以提供自己的通知品牌，无需改变侧栏或浏览器控制的站点来源。
Reviewer Test Plan
How to verify
授予浏览器和系统通知权限后提交请求，并让页面失焦。预期收到一条包含本轮标题、提问和回复的通知。点击应在所属 shell 中打开捕获的 workspace、standalone 或 Live 会话。处于分屏时，点击当前健康会话的通知应显示聊天并聚焦编辑器，不重新加载；会话缺失或仍在加载时应走正常加载路径。失败回合可以展示提问，但必须省略部分回复和错误详情。
尝试包含 TypeScript 泛型、比较式、HTML 代码围栏及不可见控制字符的提问。代码标点应保留，控制字符及双向格式字符应移除，长输入应生成有限摘录。附件传输尾行及内部 insight 载荷不应显示。历史及重复终态保持静默；轮中注入消息应从待通知状态中清理。
通过公开入口检查自定义品牌及 CDN 图标地址。省略并恢复配置时，当前会话不应重新挂载或重连。公共偏好默认关闭，内置页面对新站点显式默认开启。保存的 true/false 选择优先。存储不可读时默认关闭，仍允许用户显式临时开启；不会自动申请权限。关闭通知后必须停止发送。
Evidence (Before & After)
本 PR 之前，通知只有通用标题和状态，点击仅聚焦窗口。此前 Chrome 验证捕获了真实 daemon 回合、提问及回复文本、PNG 地址，以及切换会话并打开 Settings 后返回来源会话的导航；详细报告已在 PR 评论中。Notification API stub 和模拟失焦验证的是应用参数及导航，不代表原生 OS 横幅布局。
在
9056ceb上复现：Use Map<string, number> here被变成Use Map here，Is 3 < 5 and 7 > 2 correct?被变成Is 3 2 correct?。回归测试现已保留两种文本，按字面保留围栏中的 HTML，移除控制字符，并恢复分屏退出后的编辑器焦点。本轮验证结果记录在审查处理汇总评论中。Tested on
Environment (optional)
macOS、Node.js v22.22.3、npm 10.9.8。使用本地工作区构建、类型检查、Vitest，以及上述已有 Chrome/daemon 验证。
Risk & Scope
Linked Issues
#11398 的后续增强。