fix(cli): patch ink to clear staticNode on indirect subtree removal - #7816
Merged
Conversation
When a component containing <Static> is unmounted (e.g. transcriptFreeze toggling <App/> off), the reconciler removes the ancestor and freeRecursive() frees the static node Yoga WASM memory. The existing identity check (staticNode === removeNode) only catches direct removal of <Static> itself -- removing an ancestor leaves a dangling staticNode reference. The next render then calls getComputedWidth() on freed WASM memory, crashing with RuntimeError: memory access out of bounds. Add clearStaticNodeIfContained() which walks up the parent chain from staticNode to detect ancestor removal, and null out removeNode.yogaNode after cleanup so stale JS references short-circuit on ?.yogaNode. Fixes #6820
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: Observed bug with evidence — issue #6820 reports a Direction: Crash fix in the rendering layer — squarely within qwen-code's core mission. No direction concerns. Size: Not applicable — the only changed file is Approach: The scope feels right. The fix targets the exact gap in the upstream reconciler: the old identity check ( Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题： 已观测到的 bug，有证据——issue #6820 报告了 Yoga WASM 在 Ink 渲染周期中 方向： 渲染层崩溃修复，完全在 qwen-code 核心使命范围内。无方向性顾虑。 规模： 不适用——唯一变更文件为 方案： 范围合理。修复精确针对上游 reconciler 的缺陷：旧的身份检查（ 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Code review Independent proposal: given the bug (dangling The implementation is clean and correct:
No correctness, security, or convention issues found. CI test evidence CI is still running on the reviewed commit. The ubuntu test job is in progress; windows and macOS jobs were skipped by CI configuration. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Not verified: direct reproduction of the WSL2/Node v24 crash — the triggering conditions (transcript freeze toggling under WSL2) are environment-specific and impractical to exercise in CI. The fix is validated by logical analysis of the reconciler code path. A maintainer can trigger 中文说明代码审查 独立方案：针对此 bug（祖先卸载后 实现简洁正确： 未发现正确性、安全性或规范问题。 CI 测试证据 CI 仍在运行。ubuntu 测试进行中；windows 和 macOS 被 CI 配置跳过。未验证：WSL2/Node v24 崩溃的直接复现——触发条件（WSL2 下 transcript freeze 切换）是环境特有的，CI 中不实际。修复通过 reconciler 代码路径的逻辑分析验证。维护者可触发 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Confidence: 4/5 — clean, minimal fix for a real crash; the only reservation is that the triggering conditions (WSL2 / Node v24 transcript freeze) can't be exercised in CI, so verification rests on logical analysis of the reconciler code path rather than a live before/after. This is a well-targeted patch. The root cause analysis is correct — ink's upstream #904/#905 fix only handled direct removal of Approval deferred until CI lands green on 中文说明置信度：4/5 — 针对真实崩溃的简洁最小修复；唯一保留意见是触发条件（WSL2 / Node v24 transcript freeze）无法在 CI 中复现，验证依赖于 reconciler 代码路径的逻辑分析而非实际 before/after 测试。 这是一个精准的 patch。根因分析正确——ink 上游 #904/#905 修复只处理了 审批延迟至 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
Contributor
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
approved these changes
Jul 27, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
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
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
中文说明
已审查——无阻断问题。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
— qwen3.7-max via Qwen Code /review
Comment on lines
+468
to
+470
| +const clearStaticNodeIfContained = (removeNode) => { | ||
| + const staticNode = currentRootNode?.staticNode; | ||
| + if (!staticNode) { |
Collaborator
There was a problem hiding this comment.
[Suggestion] No test covers the new clearStaticNodeIfContained ancestor-walk logic — Concrete cost: a future ink patch version bump could break the walk (e.g., renamed parentNode field) and the WASM crash from #6820 would silently return with no test to catch it
Suggested fix: add an integration test that renders a <Static> component inside a conditionally-rendered parent, toggles the parent off, and triggers another render cycle to confirm no crash.
中文说明
[建议] 新增的 clearStaticNodeIfContained 祖先遍历逻辑没有测试覆盖 —— 具体代价：未来 ink patch 版本升级可能破坏该遍历（例如 parentNode 字段被重命名），#6820 的 WASM 崩溃会在没有测试告警的情况下悄然回归。
建议修复：添加一个集成测试，在条件渲染的父容器中渲染 <Static> 组件，切换关闭父容器，并触发下一次渲染周期以确认不会崩溃。
— qwen3.7-max via Qwen Code /review
Collaborator
Author
There was a problem hiding this comment.
Valid suggestion. The regression test for this exact scenario (unmounting an ancestor of and confirming the renderer survives) exists in the ink fork and has been proposed upstream in vadimdemedes/ink#979.
For qwen-code, adding an integration test that exercises the patched reconciler through our own component tree (e.g. toggling transcriptFreeze) would be a good follow-up. The test would need to render via ink actual reconciler (not mocked) to exercise the yoga WASM path.
Collaborator
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
Collaborator
|
🤝 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 冲突，直到移除标签或达到轮次上限。移除 |
Collaborator
Collaborator
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no code changeFeedback triageOne actionable finding this round, plus a maintainer reply on the same thread:
Decision: defer to the maintainer's follow-up — no change in this PRI investigated whether the suggested regression test can be added cleanly here, and confirmed the maintainer's read that it is genuinely non-trivial:
This matches the maintainer's framing that a proper test must exercise the real yoga WASM path and is best done as a follow-up. Both inline threads are left open with replies recording this rationale so the follow-up (tracking the ink-fork test / 中文说明Autofix 审查轮次 —— 无代码改动反馈分类本轮有一条可处理的发现，以及同一线程下维护者的一条回复：
决定：遵循维护者的 follow-up 界定 —— 本 PR 不改动我调查了能否在此处干净地添加该回归测试，并证实维护者的判断——这确实非平凡：
这与维护者的界定一致：一个合适的测试必须走到真实的 yoga WASM 路径，最好作为 follow-up 完成。两条行内线程都保持打开，并附有记录上述理由的回复，以免后续跟进项（跟踪 ink-fork 测试 / Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Collaborator
Local build & runtime verification (maintainer) — merge referenceVerified head 1. Build & patch integrity (PR test plan, steps 1–4)
Also confirmed the branch merges clean: it's behind 2. Root-cause A/B — dangling
|
| 检查项 | 结果 |
|---|---|
PR head 上 npm ci(postinstall → patch-package) |
✅ 干净,ink@7.0.3 ✔ |
PR patch 应用到原版 registry ink@7.0.3(独立 patch-package) |
✅ 干净应用 |
node_modules/ink/build/reconciler.js 中的 clearStaticNodeIfContained |
✅ 3 处 |
修复进入发布 bundle(dist/chunks/chunk-UWMLWVCA.js) |
✅(与 #6820 崩溃栈的 chunk-CNEIYWSA.js 同层) |
完整 npm run build / bundle |
✅ 无新错误 |
分支落后 main,但唯一改动的文件(patches/ink+7.0.3.patch)自 merge-base 起在 main 上无变动,合并无冲突。
2. 根因 A/B —— staticNode 悬挂真实存在,修复彻底消除
两套独立安装的原版 ink@7.0.3 + React 19,一套打 main 的 patch(BASE),一套打本 PR 的 patch。用真实 ink 渲染复刻仓库内触发路径(AppContainer.tsx:4551 的 {transcriptFreeze ? <TranscriptView/> : <App/>} 卸载 MainContent 中 <Static> 的祖先),并在 renderer 于 renderer.js:30-33 调用的那个 WASM 方法上安插探针(node.staticNode?.yogaNode → getComputedWidth(),即 #6820 崩溃栈顶帧):
- BASE: 祖先卸载后
rootNode.staticNode仍指向已释放节点,ink 自身渲染管线每帧都对已释放内存做 use-after-free 读取(观测到 5 次:窗口内 4 次 + 分配器搅动后 1 次)。 - PR:
staticNode被清除、被移除节点的yogaNode置空,对已释放内存零访问——renderer.js:30的守卫直接短路。
为什么 BASE 在本机读到垃圾值而不崩溃:yoga-layout 3.2.x 没有已删除对象防护,已释放节点的 wrapper 会静默读取该 WASM 内存当前的内容(同进程微实验:释放前读 42 → freeRecursive() 后读 0 → 分配器搅动改写内存后读 NaN)。每次渲染都是未定义行为;返回垃圾还是触发 RuntimeError: memory access out of bounds 取决于分配器状态——这既独立印证了 PR 所述"WSL2/Node 24 之外难以直接复现崩溃",也钉死了故障机制。
3. 回归套件 —— 旧守卫的行为全部保留
REG3/REG4 正是旧代码注释特别防护的 key 驱动重挂载场景:新注册的 staticNode 得以保留,因为包含性遍历从新节点出发,而新节点绝不会在被移除的子树内。对照组: 同样 5 个场景在 BASE 上也全部通过——bug 窗口之外本 PR 不改变任何行为。
4. PR head 构建的实机 CLI E2E —— 真实 #6820 触发路径
在 pty 中启动构建产物 dist/cli.js(隔离 $HOME),用 /about 向 <Static> 历史提交内容,然后 Ctrl+O 开关 transcript ×5——每次打开都卸载 <App/>(<Static> 的祖先),并在卸载窗口内强制滚动渲染(正是 BASE 持有悬挂引用的窗口):
结果:5/5 轮全部存活,pty 原始输出无 WASM 错误,UI 完好,/quit 退出码 0。(诚实说明:macOS/Node 22 上 BASE 构建同样能扛过此 E2E——如 §2 所示,释放后读取是静默失败;此 E2E 的价值是证明 PR 构建在真实触发路径上渲染正确,判别性证据在 §2。)
5. 备忘(不阻塞合并)
removeNode.yogaNode = undefined只置空被移除子树根的指针,子孙 DOM 节点仍持有过期 wrapper。对本 bug 足够——跨 commit 指向被移除子树的引用只有rootNode.staticNode,已被主修复清除;记录于此以免后来者高估这层防御的覆盖面。- 延后的回归测试(作者与审查者已商定为 follow-up)在仓库内确实不易落地;上方脚本可作为移植起点。
所有复现脚本已放在 assets 分支,可独立重跑(链接见英文版末尾)。
Collaborator
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No changes this roundThe only new feedback since the last evaluation is a maintainer verification
The PR remains a single-file change to 中文说明本轮不做任何改动自上次评估以来唯一的新反馈,是维护者 @wenshao 针对 head
本 PR 仍是对 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
wenshao
approved these changes
Jul 28, 2026
Collaborator
|
Released in v0.21.1. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Patches the ink reconciler to correctly clear the
staticNodereference when a<Static>component is removed as part of a larger subtree unmount, and nulls out the freed yoga node pointer to prevent stale WASM memory access.Why it's needed
Issue #6820 reports a
RuntimeError: memory access out of boundscrash in Yoga WASM during Ink's render cycle. The root cause is an incomplete fix in ink's upstream #904/#905: the reconciler'sremoveChild/removeChildFromContaineronly checksremoveNode.internal_staticon the node being directly removed. When an ancestor of<Static>is unmounted (e.g.transcriptFreezetoggling<App/>off inAppContainer.tsx),freeRecursive()frees the static node's Yoga WASM memory as part of the subtree, but the identity check skips it because the removed node is the ancestor, not the<Static>element itself. The danglingstaticNodereference then causesgetComputedWidth()to access freed WASM memory on the next render, crashing the process.The fix adds
clearStaticNodeIfContained()which walks up the parent chain fromstaticNodeto detect whether it is contained in the subtree being removed, and also nulls outremoveNode.yogaNodeafterfreeRecursive()so any remaining stale JS references short-circuit on?.yogaNodeoptional chaining instead of trapping into invalid WASM memory.The same fix has been implemented and tested in the ink fork (branch
fix/static-node-dangling-wasm) and will be proposed upstream.Reviewer Test Plan
How to verify
npm install(postinstall runs patch-package automatically)ink@7.0.3in postinstall outputnpm run build— should succeed with no new errorsclearStaticNodeIfContainedtonode_modules/ink/build/reconciler.js— grep to confirmThe crash itself requires a specific unmount sequence (transcript freeze toggling) under WSL2/Node v24 to reproduce, so direct reproduction is impractical in CI. The fix is verified by the ink fork's regression test which exercises the exact code path (unmounting an ancestor of
<Static>and confirming the renderer remains functional).Evidence (Before & After)
N/A — non-UI internal patch change. No user-visible behavior change except eliminating a crash.
Tested on
Environment (optional)
npm run build, patch-package apply verification
Risk & Scope
clearStaticNodeIfContainedparent-chain walk adds O(depth) work per node removal. Tree depth is small in practice (< 20 levels), so the cost is negligible.Linked Issues
Fixes #6820
中文说明
本 PR 做了什么
修补 ink 的 reconciler，在
<Static>组件作为更大子树的一部分被移除时正确清除staticNode引用，并置空已释放的 yoga 节点指针以防止残留的 WASM 内存访问。为什么需要
Issue #6820 报告了 Yoga WASM 在 Ink 渲染周期中因
RuntimeError: memory access out of bounds崩溃。根因是 ink 上游 #904/#905 修复不完整：reconciler 的removeChild/removeChildFromContainer只检查被直接移除节点的internal_static属性。当<Static>的祖先被卸载时（例如AppContainer.tsx中transcriptFreeze切换关闭<App/>），freeRecursive()作为子树的一部分释放了 static 节点的 Yoga WASM 内存，但 identity check 跳过了它，因为被移除的节点是祖先而非<Static>元素本身。残留的staticNode引用在下次渲染时导致getComputedWidth()访问已释放的 WASM 内存，进程崩溃。修复方案新增
clearStaticNodeIfContained()函数，从staticNode沿 parent chain 向上遍历以检测其是否包含在被移除的子树中，同时在freeRecursive()后置空removeNode.yogaNode，使残留的 JS 引用通过?.yogaNode可选链正确短路，而非触发 WASM trap。同一修复已在 ink fork 中实现并通过回归测试（分支
fix/static-node-dangling-wasm），将向上游提交。审阅者测试计划
如何验证
npm install（postinstall 自动运行 patch-package）ink@7.0.3npm run build— 应成功且无新错误证据（前后对比）
N/A — 非 UI 内部 patch 变更。除消除崩溃外无用户可见行为变化。
风险与范围
clearStaticNodeIfContained的 parent chain 遍历为每次节点移除增加 O(depth) 开销。实际树深度很小（< 20 层），开销可忽略。关联 Issue
Fixes #6820