fix(web-shell): cap React dev performance.measure accumulation to stop renderer OOM - #9770
Conversation
React 19 dev builds emit ~16k performance.measure entries per second, and the browser's user-timing buffer retains them unboundedly on the Blink side (invisible to the JS heap). Long-lived vite dev tabs grew to ~65GiB of PartitionAlloc mappings and died with SIGABRT (Aw, Snap!) — 12 renderer crashes over 5 days, all on the dev server origin; the production build never reproduced. The existing guard only stripped the structured-cloned detail payload. Extend it to every React devtools track and clear the measure timeline every 16384 React measures so entries cannot accumulate without bound. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Re-run after three test-only commits from the autofix loop ( Template looks good ✓ Problem: observed bug with strong evidence — 12 renderer crashes ( Direction: aligned — this bounds a dev-only leak that was crashing maintainer browser tabs, and the guard stays inert in production builds (production React emits no devtools measures). No direct CHANGELOG reference, but web-shell is exactly where the crash reproduces. Size: not applicable — both files live in Approach: scope still feels right. The production change extends the guard already in the file (strip Risk: no elevated risk signals — neither changed file matches the high-risk revert-correlated paths. Moving on to code review. 🔍 中文说明这是 autofix 循环三个纯测试提交（ 模板完整 ✓ 问题：已观测到的 bug，证据充分——8/19–8/23 期间维护者机器记录了 12 次渲染进程崩溃（ 方向：对齐——限定一个仅影响 dev 模式、曾导致维护者浏览器标签页崩溃的泄漏，且 guard 在生产构建中保持惰性（生产版 React 不产生 devtools measure）。CHANGELOG 无直接引用，但崩溃正是在 web-shell 中复现。 规模：不适用——两个文件均位于 方案：范围依然合理。生产改动扩展的是文件中已有的 guard（对所有 React devtools track 剥离 风险：无升级风险信号——两个改动文件均未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review (re-run at The production guard is byte-identical to the commit the previous pass reviewed — the compare between the two heads shows only What's new since then is the test hardening from the three autofix rounds, and it's substance rather than padding — the harness extracts the inline
No blockers, no convention violations (inline script keeps the existing ES5 style of the guard it extends). One honest caveat on the suite's surrounding claims: the mutation-kill counts (33/33) in the autofix reports are that bot's own probe results — plausible and consistent with the tests above, but not something this review re-ran. What CI runs is the tests themselves, and they pin the contract directly. CI evidence (fetched via API for Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Everything that has landed is green — notably the web-shell visuals capture, which failed to render on the intermediate head Real-scenario testing: Sandboxed verification is settling the remaining behavioural claim right now: the 中文说明代码审查（在 此后新增的是三个 autofix 轮的测试加固，是实质内容而非凑数——harness 从 无阻塞项、无规范违规（内联脚本沿用所扩展 guard 的既有 ES5 风格）。对套件周边声明的一个诚实提示：autofix 报告里的变异杀死计数（33/33）是该 bot 自己的探针结果——合理且与上述测试一致，但本审查没有重跑。CI 实际运行的是测试本身，而它们直接钉住了契约。 CI 证据：审查时通过 API 一次性获取（不轮询；无人值守运行，此处未执行 PR 代码）。已出结果全部为绿——值得注意的是 web-shell 视觉采集，它在中间 head 真实场景测试： 沙箱验证正在坐实剩余的行为声明：本工作流运行中的 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — everything statically checkable is clean, including the test hardening; the one standing nit is unchanged from the last pass: the renderer RSS plateau itself is still the author's Playwright measurement, now being independently exercised by the Stepping back: the picture got strictly better since the last pass. The production fix — strip every devtools payload, clear the timeline on a 16,384-measure budget — was already reviewed at the earlier head and is byte-identical here, so this re-run's effort went where the new work went: three rounds of mutation-driven test hardening. That work earns its keep: the tests execute the shipped inline script rather than a copy of it, the boundary assertions are exact (16,383 / 16,384), and the final round closed a genuinely subtle self-comparison hole in the non-React pins instead of churning. Six months from now the guard's comment explains why it exists and the suite explains what it must keep doing. No drive-bys, no scope creep — the diff is one small production change plus the tests that lock it in. The honest remainder: the soak numbers (603→1501MB vs 593→971MB) stay the author's claim until the sandboxed verification lands — the unit suite pins the guard's mechanism, not the renderer memory curve. That verification is running now as the Verdict: approve — deferred until CI lands green on 中文说明置信度：4/5 —— 所有可静态核查的项均干净，包括测试加固；唯一保留的非阻塞提示与上一轮相同：渲染进程 RSS 平台期本身仍是作者的 Playwright 测量，目前正由本工作流中运行的 退一步看：自上一轮以来局面严格变好。生产修复——剥离所有 devtools 载荷、每 16,384 条 measure 按预算清空时间线——已在更早的 head 上审过且此处逐字节一致，因此本轮复审把精力放在新增工作所在处：三轮变异驱动的测试加固。这些工作物有所值：测试执行的是真实上线的内联脚本而非其副本，边界断言精确（16,383 / 16,384），最后一轮封闭了非 React 钉桩中一个真正隐蔽的自比较漏洞，而不是无效折腾。六个月后维护时，guard 的注释解释了它为何存在，测试套件解释了它必须保持的行为。没有顺手改动、没有范围蔓延——diff 就是一处小的生产改动加上锁定它的测试。 诚实的保留项：在沙箱验证落地之前，浸泡数字（603→1501MB 对比 593→971MB）仍是作者的声明——单元测试钉住的是 guard 机制，而非渲染进程内存曲线。该验证正作为 结论：通过——推迟到 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
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. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Test Plan (not a blocker): 4 tests pass — this review observed 4127 passed.
中文说明
已审查——无阻断问题。 建议见行内评论。
Test Plan（非阻断）：4 tests pass — this review observed 4127 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
Harden the budget test per review feedback: drive the counted measures on a non-Components track, assert the timeline is cleared without a name filter and with the performance object as receiver, assert every measure is still forwarded detail-stripped (including the clear- triggering one), and drive a second window to prove the clear is not latched. Each of the five one-line mutants these assertions target was verified to flip the suite red.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: reverse audit — stopped before round 10 by the review time budget.
Test Plan (not a blocker): 4 tests pass — this review observed 4127 passed.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：反向审计——评审时间预算不足，未能开始第 10 轮。
Test Plan（非阻断）：4 tests pass — this review observed 4127 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // Non-React measures do not count toward the budget. | ||
| for (let i = 0; i < 16384 - 1; i += 1) { | ||
| fakePerformance.measure('custom-measure', { |
There was a problem hiding this comment.
[Suggestion] The hardened budget test's mutant surface is not closable entrance by entrance — this round alone verified twelve one-line mutants of the inline guard in index.html that pass the entire suite. The assertions were authored against a handful of named mutants instead of systematically exhausting the guard's mutation space, so each review round keeps surfacing a new survivor. Close it structurally: run one systematic mutation pass over the guard script (Stryker on the extracted inline script, or the manual mutant-injection harness you already used for the round-1 five mutants, iterated until no mutant survives), and add the killing assertions in a single commit.
The production guard at HEAD is correct — this is about the suite's stated purpose (pinning the contract against mutations) silently failing: any one of these mutants shipping in a future edit of the inline guard either re-opens the unbounded user-timing growth this PR exists to prevent or breaks React's dev-mode instrumentation, with this hardened suite green.
Demonstrated witness (probe-verified in a scratch tree at this commit): a count-all mutant — hoisting reactMeasures += 1 and the >= CLEAR_EVERY check out of the if (devtools) branch — passes every assertion, because after the second window's clear resets the counter to 0, the final section emits only 16,383 custom measures (one short of the threshold), so no third clear fires in either implementation:
count-all mutant + current test → Tests 4 passed (4) (mutant survives)
count-all mutant + full-threshold loop → AssertionError: expected "spy" to be called 2 times, but got 3 times
correct guard + full-threshold loop → Tests 4 passed (4)
The eleven further verified survivors (each probed the same way — mutant survives the suite, the named assertion kills it, the fix stays green on the correct guard):
- Threshold off-by-one:
CLEAR_EVERY = 16383survives — nothing asserts no clear has fired after only 16,383 React measures. - Homogeneous flood: one track (
'Blocking') and one name ('⏱ track') for all 32,768 measures, so track-gated or name-gated counters (reactMeasures += devtools.track === 'Blocking' ? 1 : 0) pass the whole suite. - Null-check drop: removing
clear &&from the budget branch survives — no test drives the budget withclearMeasuresabsent; the 16,384th measure would then throwTypeError: clear is not a function. - Post-clear forwarding drop: a mutant that stops forwarding React measures after the first clear survives — window 2 never re-asserts
measurecall counts. - Post-clear strip drop: a mutant that stops stripping
detailafter the first clear survives — nothing assertscalls[32767]detail is null. - Non-React return drop: dropping
returnfrom the non-React branch survives — the return value is only pinned on the React path. - Predicate-conjunct drop: deleting
options.detail &&(oroptions &&) survives — every suite call shape carries a truthydetail; the guard would then throw on standard detail-lessperformance.measureshapes. - Reset-on-non-React: resetting
reactMeasureson every non-React measure survives — customs only run when the counter is already 0, so mixed app+React traffic would never reach the budget. - Name drop on the strip path: corrupting
args[0]in the strip branch survives — no assertion checks the forwarded measure name on the React path. - Post-clear non-React pass-through: mutants dropping all customs after the first clear, or stripping their
detail, survive — the custom section asserts only the clear count. - Install-time
measurebind: deleting.bind(performance)from the captured original survives — the suite never invokes the wrapper with a detached receiver.
中文说明
[Suggestion] 加固后的预算测试的变异面无法逐条封堵——仅本轮就验证了 index.html 内联 guard 的十二个单行变异体能通过整套测试。这些断言是针对少数几个已命名的变异体编写的,而不是系统性地穷尽 guard 的变异空间,因此每一轮审查都会冒出新的幸存者。请从结构上收口:对 guard 脚本运行一次系统性变异测试(对提取出的内联脚本跑 Stryker,或沿用你在第一轮响应中已用过的手动变异注入框架,迭代到没有变异体幸存),并在一次提交中补上所有能杀死变异体的断言。
HEAD 上的生产 guard 是正确的——问题在于测试套件声称的目的(钉住契约以防变异)会悄悄失效:这些变异体中任何一个随未来对 guard 的编辑上线,都会重新打开本 PR 要防止的无界 user-timing 增长,或破坏 React 的 dev 模式打点,而这套加固后的测试仍为绿色。
已验证示例(在该提交的 scratch tree 中探针验证):计数全部变异体——把 reactMeasures += 1 和 >= CLEAR_EVERY 检查提出 if (devtools) 分支——能通过所有断言:第二个窗口的清空把计数器归零后,末段只发出 16,383 条自定义 measure(差一条到阈值),两种实现都不会触发第三次清空。探针结果:计数全部变异体 + 现有测试 → Tests 4 passed (4)(变异体幸存);该变异体 + 完整阈值循环 → AssertionError: expected "spy" to be called 2 times, but got 3 times(被杀死);正确 guard + 完整阈值循环 → Tests 4 passed (4)。
其余十一个已验证幸存者(均以同样方式探针验证——变异体通过整套测试,指名断言可将其杀死,修复在正确 guard 上保持绿色):
- 阈值差一:
CLEAR_EVERY = 16383幸存——没有断言仅 16,383 条 React measure 时尚未清空。 - 同质洪流:全部 32,768 条 measure 只用一个 track(
'Blocking')和一个名字('⏱ track'),因此按 track/名字门控的计数器能通过整套测试。 - 空检查删除:去掉预算分支的
clear &&幸存——没有测试在缺少clearMeasures时驱动预算路径;否则第 16,384 条会抛TypeError: clear is not a function。 - 清空后停止转发:首次清空后不再转发 React measure 的变异体幸存——窗口 2 没有重新断言
measure调用次数。 - 清空后停止剥离:首次清空后不再剥离
detail的变异体幸存——没有断言calls[32767]的 detail 为 null。 - 非 React 返回值丢失:去掉非 React 分支的
return幸存——返回值只在 React 路径上被钉住。 - 谓词连词删除:删除
options.detail &&(或options &&)幸存——套件所有调用形状都带真值detail;guard 随后会在标准无 detail 的performance.measure形状上抛错。 - 非 React 重置:每条非 React measure 都重置
reactMeasures的变异体幸存——自定义 measure 只在计数器已为 0 时运行,混合流量永远到不了预算。 - 剥离路径丢名字:破坏剥离分支中
args[0]的变异体幸存——没有断言检查 React 路径上转发的 measure 名字。 - 清空后非 React 透传:首次清空后丢弃全部自定义 measure 或剥离其
detail的变异体幸存——自定义段只断言清空次数。 - 安装期
measure绑定:删除捕获原始函数时的.bind(performance)幸存——套件从未以脱离 receiver 的方式调用包装器。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
@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. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
The round-1 hardened budget test pinned a handful of named mutants; a systematic injection pass found twelve further one-line mutants of the inline guard that passed the whole suite. Drive a mixed-track/name flood, assert the exact budget boundary, post-clear forwarding/stripping/passthrough, interleaved traffic, detached receivers, standard measure shapes, and the install bailout, so every guard line now has a witness. All 33 probed mutants are killed; the guard itself is unchanged and correct.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #9770Commit: Feedback triage
What changed (rc:3838285649)Reproduced first: each claimed survivor was injected into The budget test was then rebuilt so every guard line has a witness, plus three small tests:
Mutation closure (manual injection harness, iterated as the finding requested): 33/33 mutants killed by the committed suite — the 12 reviewer-named survivor classes (15 probe variants), the 5 round-1 mutants (regression check), and 13 additional one-line mutants enumerated from the guard's full mutation space (threshold ±1, The round-1 pins (rc:3837883444) were re-verified at HEAD: all five hardening points are present, and all five round-1 mutants are killed by the committed suite. Visual preview failure (ic:5384400830)Investigated; found no code-level link to this PR. Evidence: the only render-relevant change in the head is the inline guard, which merely wraps Checks
VerificationCommands actually run this round:
中文说明Autofix 本轮总结 — PR #9770提交： 反馈分诊
改动内容（rc:3838285649）先复现：将每个被点名的幸存变异体以文本变异方式注入 随后重构预算测试，使 guard 的每一行都有见证，并新增三个小测试：
变异收口（按发现要求,用手动注入框架迭代执行）：33/33 个变异体被提交后的套件杀死——审查者点名的 12 类幸存者的 15 个探针变体、第 1 轮的 5 个变异体（回归检查），以及从 guard 完整变异空间枚举出的 13 个额外单行变异体（阈值 ±1、 第 1 轮的钉住项（rc:3837883444）已在 HEAD 重新验证：五个加固点全部存在，且五个第 1 轮变异体均被提交后的套件杀死。 视觉预览失败（ic:5384400830）已调查，未发现与本 PR 存在代码层面的关联。证据：该 head 中唯一与渲染相关的改动是内联 guard，它仅仅包装了 检查
验证本轮实际执行的命令：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 1a": none — but one disclosure: I could not run the actual vitest suite (no node_modules in the review worktree and a full npm ci + prepare build was dispropor….
Test Plan (not a blocker): 4 tests pass — this review observed 4130 passed.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/web-shell/client/index-html.test.ts:118 — [review] comment parenthetical '(React never names its measures)' contradicts the name-rotating flood the same test drives
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 1a"：none — but one disclosure: I could not run the actual vitest suite (no node_modules in the review worktree and a full npm ci + prepare build was dispropor…。
Test Plan（非阻断）：4 tests pass — this review observed 4130 passed。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // Identity: the non-React options object is forwarded as-is. | ||
| expect(measure.mock.calls[49151]?.[0]).toBe('custom-measure'); | ||
| expect(measure.mock.calls[49151]?.[1]).toBe(customOptions); |
There was a problem hiding this comment.
[Suggestion] R2-1: The measure-guard test's mutant surface is still not closed — this round's probe verified another survivor under the committed suite, so the round-3 commit's closure claim ("33/33 mutants killed; remaining mutants are genuinely equivalent") has not reached its own stated fixpoint ("iterated until no mutant survives"). The demonstrated entrance: a mutant that strips a non-React measure's detail by mutating the caller's options object in place (e.g. options.detail = null before forwarding — the allocation-free variant of the existing strip, plausible under the HTML comment's own "~16k measures/s" framing) passes the entire suite, because the "not stripped" half of this contract is pinned only by identity comparisons — toBe(customOptions) still passes on the same object once mutated, and the earlier toHaveBeenCalledWith('custom-measure', options) compares the recorded argument against that same mutated reference (self-equality). A future guard edit shaped like that would silently corrupt a caller-owned options object (web-shell code reusing one options object across measures loses its detail) while this comment claims the case is pinned. The React path IS protected against the same mutant shape (the first test reads options.detail.devtools.properties after the call); the non-React path has no equivalent post-call value read. Witness (probe in a scratch tree at this commit): the mutant survives with the suite green (7/7); adding the value-survival assertion below flips it red (expected null to deeply equal { source: 'web-shell' }); the same in-place strip on the React branch fails the existing post-call read (Cannot read properties of null (reading 'devtools')); and the correct guard with the proposed assertion stays green (7/7). Fix: fold this mutant into the mutation pass, re-run it to fixpoint, and keep the killing assertion below, which closes the demonstrated entrance.
| // Identity: the non-React options object is forwarded as-is. | |
| expect(measure.mock.calls[49151]?.[0]).toBe('custom-measure'); | |
| expect(measure.mock.calls[49151]?.[1]).toBe(customOptions); | |
| // Identity: the non-React options object is forwarded as-is. | |
| expect(measure.mock.calls[49151]?.[0]).toBe('custom-measure'); | |
| expect(measure.mock.calls[49151]?.[1]).toBe(customOptions); | |
| // Value survival, not just reference: an in-place strip of the caller's | |
| // options object must fail here. | |
| expect( | |
| (measure.mock.calls[49151]?.[1] as PerformanceMeasureOptions).detail, | |
| ).toEqual({ source: 'web-shell' }); |
中文说明
[Suggestion] measure-guard 测试的变异面仍未收口——本轮探针在已提交的套件下又验证了一个幸存变异体,因此第 3 轮提交的收口声明("33/33 变异体全部被杀死;其余变异体均为等价变异")并未达到其自身设定的收敛点("迭代到没有变异体幸存")。已验证的入口:在转发非 React measure 之前,通过原地修改调用方 options 对象来剥离 detail 的变异体(例如转发前 options.detail = null——现有剥离的零分配变体,在 HTML 注释自身 "~16k measures/s" 的语境下相当合理)能通过整套测试,因为该契约中"未被剥离"这半边仅由引用相等断言钉住——对象被原地修改后 toBe(customOptions) 仍然通过,而前面的 toHaveBeenCalledWith('custom-measure', options) 是 recorded 参数对同一被修改引用的自比较。未来若 guard 按此形态修改,将悄悄破坏调用方自有的 options 对象(复用同一 options 对象的 web-shell 代码会丢失其 detail),而此处注释却声称该情形已被钉住。React 路径对同形变异体有防护(第一个测试在调用后读取 options.detail.devtools.properties);非 React 路径缺少等价的调用后取值断言。见证(在本提交的 scratch tree 中探针验证):变异体幸存、套件保持绿色(7/7);补充下方的取值存活断言后由绿变红(expected null to deeply equal { source: 'web-shell' });同形变异体作用于 React 分支时会被现有的调用后读取杀死(Cannot read properties of null (reading 'devtools'));正确 guard 加上该断言保持绿色(7/7)。修复:将该变异体并入变异测试通道,重新迭代至无幸存者,并保留下方断言——它可杀死已验证的这个入口。
— qwen3.8-max via Qwen Code /review (v0.22.0)
The "not stripped" half of the non-React contract was asserted only by object identity, so a guard that stripped detail by mutating the caller's options object in place passed the whole suite (verified by mutation probe). Assert the forwarded detail's value so an in-place strip fails.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix address-review summary — PR #9770, round 4Commit: Feedback dispositions
Change
No conflict ( Mutation probes (witness verification)
Verification
中文说明Autofix address-review 摘要 — PR #9770，第 4 轮提交： 反馈处理
变更
无冲突（ 变异探针（见证验证）
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): 4 tests pass — this review observed 4130 passed.
中文说明
Test Plan（非阻断）：4 tests pass — this review observed 4130 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 4230 passed · 0 failed · 4230 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：4230 通过 · 0 失败 · 4230 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9770 Deep Verification — fix(web-shell): cap React dev performance.measure accumulation to stop renderer OOMVerdict: 中文摘要
Central claim and A/B proofCentral claim: with the fix, React dev-mode Secondary claims: (1) non-React measures are neither stripped nor counted; (2) production builds are unaffected (guard inert when no devtools measures occur). The change is entirely an inline Cell table
Witnesses: The head sawtooth's 409-entry floor is fully explained: the page's own natural React measures (~26–52/s in this sandbox) pre-load the counter, so each clear fires ~408 measures before a 4096-spaced sample — the mechanism is deterministic, not noisy. Corrections (to the PR description, not code-change requests)
FindingsNone blocking. Ordered by severity (all low): F1 — nit (description precision): the budget bounds React measures, not the whole timeline. The clear wipes everything but only React measures count toward the 16,384 budget, so interleaved non-React measures add to the peak (measured: 18,205 at 1 non-React per 9 React; bound ≈ F2 — informational (tradeoff scope): the accepted cost is slightly broader than described. The body names one cost: clearing "truncates an in-progress DevTools Performance recording of React tracks". The unfiltered F3 — note: fallback when No injection attempts were found in the PR text (title/body/commits read as untrusted input; no steering instructions present). Not covered
MethodologyEnvironment: CI verify container ( Flakiness gate logEvidence 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. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Review without approval, per request. Verified the production guard at head 66864a8:
- The guard itself is correct: detail-stripping now covers every React devtools track, the budget counts every React measure regardless of track/name,
clearMeasures()runs BEFORE the triggering entry is recorded so the buffer oscillates under the cap instead of losing the current entry, the bound-receiver captures survive detached calls, and the no-clearMeasuresfallback degrades to the pre-PR behavior without throwing. Production stays inert (no devtools measures, wrapper falls through). The acknowledged tradeoff —clearMeasures()with no name wipes non-React entries and in-progress Performance recordings too — is dev-only and disclosed. - The tests pin the contract well, including the value-survival assertion whose comment names the exact in-place-strip mutant.
Two open items, both on the test side, which is why this stays unapproved for now:
- The R2-1 thread appears stale against the round-3 suite: it claims an in-place mutant that strips a NON-React measure's detail survives, but the committed "value survival, not just reference" assertion (
detailmust still equal{ source: 'web-shell' }after 16,384 reuses of the same options object) kills exactly that mutant shape — an in-placeoptions.detail = nullon the shared object fails it. Either the automated review re-verifies at head and the thread resolves, or it names a genuinely different survivor; the current text reads as pre-round-3. - The other thread is the meta-question worth a maintainer call rather than another manual round: whether a dev-only inline guard warrants an exhaustive mutation pass (Stryker over the extracted script, iterated to fixpoint) to close the suite's stated purpose. The bot itself states the production guard at HEAD is correct; the gap is the suite's pinning completeness, not behavior. My read: the four committed tests already pin every observable contract (strip scope, budget arithmetic, non-React identity+value survival, fallback, standard shapes), so an exhaustive hunt is optional hardening — but that is the author's/maintainer's call to make explicitly.
CI at review time: 12 checks passing; Test (ubuntu) still pending on head.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Approving after the maintainer call on the two open test-side items.
The production guard was verified correct at head 66864a8 (see my earlier comment): strip scope across all devtools tracks, budget arithmetic with clear-before-record ordering, non-React identity + value survival, safe no-clearMeasures fallback, and an inert production path. The remaining threads are test-suite completeness questions, not behavior defects: R2-1 reads stale against the round-3 value-survival assertion (which kills the exact in-place mutant shape it names), and the exhaustive-mutation-pass question is settled as optional hardening for a dev-only inline guard whose observable contract the committed suite already pins.
CI at approval time: 12 checks passing, none failing; Test (ubuntu) is still pending on this head and should land green before merge.
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. No new findings beyond the 4 prior rounds at the same commit.
The PR's inline script correctly strips detail from all React devtools tracks (not just Components ⚛) and clears the measure timeline on a 16,384-measure budget. All 11 review dimensions found no new defects:
- Line-by-line correctness (1a): No findings — every hunk is correct.
- Removed-behavior audit (1b): The removed
track === 'Components ⚛'gate is correctly generalized to any devtools track. - Security (2): No findings — the guard operates on
performance.measureonly, no XSS or injection vector. - Reuse & duplication (3a): No findings.
- Altitude & abstraction (3b): No findings — the change is appropriately scoped.
- Consistency & clarity (3c): No findings.
- Performance & efficiency (4): No findings — the 16,384 budget prevents the addressed OOM with negligible overhead.
- Test coverage (5): No findings — 7 tests covering detail-stripping on all tracks, budget-based clear, standard shapes, and
clearMeasures-absent fallback. - Adversarial personas (6a/6b/6c): No new valid findings.
Verification: packages/web-shell build and all 197 test files (4130 tests) pass.
CI status: Test (ubuntu-latest, Node 22.x) is still failing; appears to be a pre-existing infrastructure issue unrelated to this PR's changes.
Existing threads: Comment 3838560605 (R2-1 value survival assertion) is at the current HEAD and was addressed by the autofix at this commit.
中文说明
已审查。在同一提交的 4 轮审查基础上未发现新问题。
PR 的内联脚本正确地从所有 React devtools track（不限于 Components ⚛）剥离 detail，并在达到 16,384 条 measure 预算时清空时间线。全部 11 个审查维度均未发现新缺陷：
- 逐行正确性 (1a)：无发现——每个 hunk 均正确。
- 删除行为审计 (1b)：移除的
track === 'Components ⚛'门控已正确泛化为任意 devtools track。 - 安全 (2)：无发现——guard 仅作用于
performance.measure，无 XSS 或注入向量。 - 复用与重复 (3a)：无发现。
- 层次与抽象 (3b)：无发现——变更范围适当。
- 一致性与清晰度 (3c)：无发现。
- 性能与效率 (4)：无发现——16,384 条预算以极低开销解决了 OOM 问题。
- 测试覆盖 (5)：无发现——7 个测试覆盖了所有 track 的 detail 剥离、预算清理、标准调用形状及缺少
clearMeasures的降级路径。 - 对抗性审查 (6a/6b/6c)：无有效新发现。
验证： packages/web-shell 构建成功，全部 197 个测试文件（4130 个测试）通过。
CI 状态： Test (ubuntu-latest, Node 22.x) 仍在失败，似乎是与本 PR 无关的预存基础设施问题。
现有线程： 评论 3838560605（R2-1 取值存活断言）位于当前 HEAD，已在该提交中由自动修复处理。
— deepseek-v4-flash via Qwen Code /review (v0.21.10)
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. ✅
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed at HEAD 66864a88. Approving — CI is fully green and the one live finding is fixed. Note the bot's Approve→Comment downgrade on this head was a false alarm: its round-4 review posted zero findings and downgraded solely on "CI failing: Test (ubuntu-latest)", but no job in that run had failed — the check was still in_progress, and it has since passed. I waited for it rather than assume: Test (ubuntu-latest) pass, and web-shell E2E Smoke (the lane that would catch a broken inline script here) pass. 14 pass / 0 fail.
The fix reads correctly. Two changes to the guard, and the interesting part is that they compose safely:
- The strip widens from
devtools.track === 'Components ⚛'to anydevtools-bearing measure, so Blocking/Transition/Suspense details are nulled too. Timing is preserved — onlydetailgoes — which is the stated intent. - The new budget counts React devtools measures and calls
clearMeasures()every 16384, because the stripped entries still accumulate in the unbounded user-timing buffer.
Gating the counter on devtools rather than on all measures is what makes this safe in production: React only attaches devtools detail in dev, so reactMeasures never advances in a production build and the guard stays a passthrough — no clearing where nobody asked for it. Worth stating explicitly since it is load-bearing and not obvious from the diff.
Object.assign({}, options, { detail: null }) keeps the caller's object intact rather than mutating it, which is exactly the property the round-2 finding was about.
The live Suggestion (R2-1) is fixed. The non-React survival contract was pinned only by identity (toBe(customOptions)), which an in-place options.detail = null mutant survives by self-equality. HEAD now asserts the value alongside the identity:
expect(measure.mock.calls[49151]?.[1]).toBe(customOptions);
expect((measure.mock.calls[49151]?.[1] as PerformanceMeasureOptions).detail)
.toEqual({ source: 'web-shell' });That kills the demonstrated mutant. The React path already had the equivalent protection via its post-call read of options.detail.devtools.properties.
The rest of the suite is unusually well-targeted for a 17-line change, and each assertion earns its place: clearThis pins that clear carries the right this (a real hazard given the bind/apply plumbing); the entries at 16383 and 32767 pin that crossing a clear boundary does not skip the strip; and the passthrough cases cover plain calls, null options, a string start-mark, and detail-less options forwarded by identity.
One non-blocking observation. clearMeasures() with no argument clears the whole measure buffer, so in dev it also drops non-React entries any other code recorded. That is the right tradeoff — React's measure names vary (⏱ lane, ⏱ render, ⏱ commit), so per-name clearing could not bound the flood, and losing dev user-timing beats the renderer dying with SIGABRT. I'd just add a clause to the HTML comment saying the clear is buffer-wide, since the comment currently says only that "the timeline is also cleared on a budget" and a future reader debugging missing measures would land there first. Not worth another round on its own.
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
静态/diff 审查，head 66864a8（PR 已合并，此为合并后复核；未本地运行测试）。
中文：
结论：💬 无阻塞、无需后续跟进。 这是一个证据充分、边界干净的 dev-only 修复，独立复核未发现任何值得报告的问题。以下为核验记录。
核验过的机制
- 预算语义正确：计数只统计带
detail.devtools形状的 measure（即所有 React devtools track，不再限于Components ⚛）；clear()在第 16,384 条时触发且发生在触发条目本身写入之前，因此时间线条目在 1..16,384 之间振荡；计数器先归零再清理，不锁死，后续窗口照常清理。 - 误伤面已核实为零：全仓 grep 确认 web-shell client 与 webui 没有任何自发的
performance.measure/performance.mark调用，也没有读取 measure 条目的路径，所以clearMeasures()的连带清理只影响 DevTools 录制本身——这正是 PR 描述中声明的 dev-only 权衡。 - 生产构建保持惰性：生产版 React 不产生 devtools measure，
devtools分支永不触发，包装器的每调用开销与改动前一致。 - 接收者安全：
m与clear都绑定到performance，wrapper 被摘出来单独调用也不会触发 Illegal invocation；clearMeasures不存在时降级为只剥离 detail、不清理、不抛错。 - 非 React measure 按引用原样转发，不做剥离也不计数；
Object.assign({}, options, { detail: null })只拷贝不改动调用方的 options 对象。 - 测试测的是真实制品：用例从 index.html 中提取实际内联脚本文本并在伪造的 performance 对象上执行，覆盖了预算边界精确触发、非锁死、混合流量不复位计数、身份与值双重存活、接收者品牌检查等路径。
🎉 值得肯定的点
- 证据驱动：12 次 renderer 崩溃记录、PartitionAlloc crash key、78MB transcript 的 RSS soak 前后对比（单调上涨 → 平台期），修复针对的是实际观察到的失败模式。
- 内联注释把"为什么剥了 detail 还不够"（user-timing 缓冲区在 Blink 侧无界增长）讲清楚了，后人不需要重新推导。
- 测试对边界条件（恰好在预算处触发、非 React 流量穿插、降级路径）的覆盖堪称这类内联守卫的范本。
English:
Static/diff review at head 66864a8 (post-merge verification; tests not run locally).
Verdict: 💬 no blockers, no follow-ups needed. This is an evidence-backed, tightly scoped dev-only fix; the independent pass found nothing worth reporting. Verification record below.
Mechanisms verified
- Budget semantics: only measures carrying the
detail.devtoolsshape are counted (all React devtools tracks, no longer justComponents ⚛);clear()fires at exactly the 16,384th entry and runs before the triggering entry is recorded, so the timeline oscillates within 1..16,384; the counter resets prior to clearing, so the clear is not latched and later windows clear as well. - Collateral surface verified empty: a repo-wide grep confirms neither the web-shell client nor webui issues its own
performance.measure/performance.markcalls or reads measure entries back, soclearMeasures()sweeping non-React entries can only truncate DevTools recordings — precisely the dev-only tradeoff the PR description declares. - Production stays inert: production React emits no devtools measures, the
devtoolsbranch never fires, and per-call wrapper overhead is unchanged from before. - Receiver safety: both
mandclearare bound toperformance, so detached wrapper calls cannot hit Illegal invocation; whenclearMeasuresis unavailable the guard degrades to stripping only, without throwing. - Non-React measures are forwarded by reference untouched — neither stripped nor counted;
Object.assign({}, options, { detail: null })copies rather than mutating the caller's options object. - Tests exercise the real artifact: they extract the actual inline script text from index.html and run it against a fake performance object, covering exact-boundary firing, non-latching, interleaved traffic not resetting the counter, identity and value survival, and receiver brand checks.
🎉 Highlights
- Evidence-driven: 12 recorded renderer crashes, the PartitionAlloc crash key, and a before/after RSS soak over a 78MB transcript (monotonic growth → plateau) — the fix targets the observed failure mode.
- The inline comment explains why stripping detail alone was insufficient (the user-timing buffer grows without bound on the Blink side), sparing the next reader the re-derivation.
- Boundary-condition coverage (fires exactly at budget, interleaved non-React traffic, degradation path) is exemplary for an inline guard of this kind.
|
Released in v0.22.2. |
6 participants
What this PR does
Bounds a React 19 dev-mode memory leak in the Web Shell shell page. In dev builds React records component timings via
performance.measure(); the browser's user-timing buffer retains those entries without limit (Blink-side, invisible to the JS heap), and an idle Web Shell dev page emits ~16k of them per second. The existing inline guard already stripped the structured-cloneddetailpayload but let the stripped entries accumulate forever. This PR extends the guard to stripdetailon every React devtools track (not justComponents ⚛) and clears the measure timeline every 16,384 React measures so entries can no longer accumulate without bound. Production builds are unaffected: production React never emits devtools measures, so the guard stays inert.Why it's needed
Long-lived vite dev tabs were crashing the Chrome renderer. Over 8/19–8/23 a maintainer machine recorded 12 renderer crashes (
SIGABRT, "Aw, Snap!"), all on the vite dev server origin (localhost:5173), each with the PartitionAlloc crash keypage-allocator-mapped-size ≈ 65GiB— the renderer's address-space pool exhausted. The production build served by the daemon never reproduced. Reproduction against the real 78MB session transcript showed the renderer RSS ratcheting 603→1501MB with no plateau during a 60s scroll soak on the dev build; with this fix the same soak plateaus (593→971MB, then flat), and measure entries oscillate under the 16,384 cap instead of growing at 16k/s.Reviewer Test Plan
How to verify
cd packages/web-shell && npx vitest run client/index-html.test.ts— 4 tests pass, covering detail-stripping on a non-Components track, the periodic clear, and that non-React measures neither get stripped nor count toward the budget.npx viteinpackages/web-shell): open any page, then in DevTools console runperformance.getEntriesByType('measure').lengthtwice ~15s apart. Before this PR the count grows ~16k/s unboundedly; after, it oscillates and never exceeds 16,384.Evidence (Before & After)
Before (dev build, 60s scroll soak over a 78MB session): renderer RSS 603→620→…→1453→1501MB, monotonic. After: 593→…→971→972→968MB, plateau. Measure entries before: 132,766 at t=6s → 296,280 at t=16s; after: oscillates in 1,632–15,449 over 70s on the session view.
Tested on
Environment (optional)
Reproduced and verified with real Chrome (headless, Playwright + CDP metrics) against a vite dev server proxying the live daemon, using a real 78MB session transcript.
Risk & Scope
Linked Issues
N/A — investigated from local crash reports; no tracking issue filed.
中文说明
本 PR 做了什么
为 Web Shell 页面修复一个 React 19 dev 模式的内存泄漏上界。dev 构建中 React 通过
performance.measure()记录组件耗时,而浏览器的 user-timing 缓冲区会无界保留这些条目(在 Blink 侧,JS 堆不可见),空闲的 Web Shell dev 页面每秒产生约 1.6 万条。现有的内联 guard 只剥离了被结构化克隆的detail载荷,条目本身却永久累积。本 PR 将 guard 扩展为:对所有 React devtools track(不再限于Components ⚛)剥离detail,并且每累积 16,384 条 React measure 就调用performance.clearMeasures()清空时间线,使条目不再无界增长。生产构建不受影响:生产版 React 不会发送 devtools measure,guard 保持惰性。为什么需要
长时间存活的 vite dev 标签页会导致 Chrome 渲染进程崩溃。8/19–8/23 期间一台维护者机器记录了 12 次 renderer 崩溃(
SIGABRT,即 "Aw, Snap!"),全部发生在 vite dev server 源(localhost:5173),每次崩溃的 PartitionAlloc crash keypage-allocator-mapped-size ≈ 65GiB——渲染进程地址空间池耗尽。daemon 提供的生产构建从未复现。用真实 78MB 会话 transcript 复现:dev 构建下滚动 60 秒 renderer RSS 从 603MB 单调涨到 1501MB 无平台期;修复后同样操作 RSS 在 593→971MB 后进入平台期,measure 条目数在 16,384 上限内振荡,而不是以每秒 1.6 万条无限增长。审查者验证方式
cd packages/web-shell && npx vitest run client/index-html.test.ts——4 个测试全部通过,覆盖非 Components track 的 detail 剥离、周期性清理,以及非 React measure 既不被剥离也不计入预算。performance.getEntriesByType('measure').length。修复前条目以每秒约 1.6 万无界增长;修复后振荡且永不超过 16,384。