Skip to content

fix(web-shell): polyfill Range layout APIs in tests - #6677

Merged
wenshao merged 2 commits into
QwenLM:mainfrom
yiliang114:codex/fix-web-shell-range-jsdom
Jul 10, 2026
Merged

fix(web-shell): polyfill Range layout APIs in tests#6677
wenshao merged 2 commits into
QwenLM:mainfrom
yiliang114:codex/fix-web-shell-range-jsdom

Conversation

@yiliang114

@yiliang114 yiliang114 commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Adds a web-shell test setup polyfill for the jsdom Range layout methods that CodeMirror calls during asynchronous editor measurement, and adds a focused setup test that locks this environment contract.

Why it's needed

The Ubuntu PR CI can finish all web-shell assertions successfully and still fail because Vitest catches an unhandled TypeError: textRange(...).getClientRects is not a function from CodeMirror after useComposerCore.dom.test.tsx schedules a jsdom animation-frame measurement. jsdom does not provide these Range layout methods, so the shared test setup should provide stable empty geometry for editor tests.

Reviewer Test Plan

How to verify

Confirm that jsdom document.createRange() exposes getClientRects and getBoundingClientRect, then run the web-shell DOM composer test and full web-shell CI test command. The expected result is that all tests pass without the CodeMirror getClientRects unhandled error.

Evidence (Before & After)

Before: npm -w packages/web-shell run test -- test/setup.test.ts failed with expected 'undefined' to be 'function' for range.getClientRects.

After: npm -w packages/web-shell run test -- test/setup.test.ts passed 1 test; npm -w packages/web-shell run test -- hooks/useComposerCore.dom.test.tsx passed 4 tests; npm -w packages/web-shell run test:ci passed 83 files and 1340 tests; npm -w packages/web-shell run typecheck passed; npm -w packages/web-shell run lint passed; ./node_modules/.bin/prettier --check packages/web-shell/client/test/setup.ts packages/web-shell/client/test/setup.test.ts passed.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

Node v22.22.0, package workspace installed with npm ci in an isolated git worktree.

Risk & Scope

  • Main risk or tradeoff: The polyfill returns empty layout geometry, which is appropriate for jsdom tests but does not simulate real browser layout.
  • Not validated / out of scope: I did not run the suite locally on Linux or Windows; GitHub Actions should validate those environments.
  • Breaking changes / migration notes: N/A.

Linked Issues

Related to #6578 and the failing PR CI observed on #6662.

中文说明

What this PR does

为 web-shell 的测试环境补上 jsdom Range layout 方法 polyfill,覆盖 CodeMirror 在异步编辑器测量里会调用的 getClientRectsgetBoundingClientRect,并新增一个聚焦的 setup 测试锁住这个测试环境契约。

Why it's needed

Ubuntu PR CI 里 web-shell 的断言本身可以全部通过,但 Vitest 随后会捕获 CodeMirror 在 useComposerCore.dom.test.tsx 相关异步 animation-frame 测量中抛出的 TypeError: textRange(...).getClientRects is not a function。jsdom 本身不提供这些 Range layout 方法,所以应该在共享测试 setup 里提供稳定的空布局几何,避免编辑器测试出现 unhandled error。

Reviewer Test Plan

How to verify

确认 jsdom 的 document.createRange() 暴露 getClientRectsgetBoundingClientRect,然后运行 web-shell 的 DOM composer 测试和 full web-shell CI 测试命令。预期结果是所有测试通过,并且不再出现 CodeMirror getClientRects unhandled error。

Evidence (Before & After)

Before:npm -w packages/web-shell run test -- test/setup.test.tsrange.getClientRects 上失败,错误为 expected 'undefined' to be 'function'

After:npm -w packages/web-shell run test -- test/setup.test.ts 通过 1 个测试;npm -w packages/web-shell run test -- hooks/useComposerCore.dom.test.tsx 通过 4 个测试;npm -w packages/web-shell run test:ci 通过 83 个文件和 1340 个测试;npm -w packages/web-shell run typecheck 通过;npm -w packages/web-shell run lint 通过;./node_modules/.bin/prettier --check packages/web-shell/client/test/setup.ts packages/web-shell/client/test/setup.test.ts 通过。

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

Node v22.22.0,在独立 git worktree 中通过 npm ci 安装 workspace 依赖。

Risk & Scope

  • Main risk or tradeoff:polyfill 返回空布局几何,适合 jsdom 测试,但不模拟真实浏览器布局。
  • Not validated / out of scope:未在本地 Linux 或 Windows 环境运行测试;这些环境交给 GitHub Actions 验证。
  • Breaking changes / migration notes:N/A。

Linked Issues

关联 #6578 以及 #6662 上观察到的 PR CI 失败。

CI Follow-up

After the first push, Ubuntu full CI no longer reproduced the CodeMirror getClientRects unhandled error, and packages/web-shell passed 83 files / 1340 tests in the GitHub Actions log. The same full CI run then exposed an unrelated timing-sensitive packages/webui heartbeat prompt cleanup test. This PR now also stabilizes that test by releasing the simulated terminal heartbeat failure only after submitPrompt has started.

CI follow-up 中文说明

第一次 push 后,Ubuntu full CI 已经没有再复现 CodeMirror getClientRects unhandled error,GitHub Actions 日志里 packages/web-shell 通过了 83 个文件 / 1340 个测试。随后同一次 full CI 暴露了一个无关的 packages/webui heartbeat prompt cleanup 计时型测试问题。这个 PR 现在也把该测试稳定化:只有在 submitPrompt 已经启动后,才释放模拟的 terminal heartbeat 失败。

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: Observed bug — Ubuntu PR CI fails with TypeError: textRange(...).getClientRects is not a function from CodeMirror after jsdom animation-frame measurement. Well-documented in #6578 and #6662, with before/after test output provided.

Direction: Aligned. Polyfilling jsdom gaps in the shared test setup is exactly the pattern setup.ts already uses (ResizeObserver, scrollIntoView, clipboard, matchMedia, mediaDevices). This extends it to Range layout methods.

Size: Not applicable — no core infrastructure paths touched. All changes are test files or test setup (packages/web-shell/client/test/ and a webui test file). 36 production lines (setup.ts polyfill), 23 test lines (setup.test.ts), 13/10 test lines (DaemonSessionProvider timing fix).

Approach: Scope is tight. Two focused files for the polyfill + test, plus a timing stabilization in the DaemonSessionProvider test discovered during CI validation. The ??= operator is the right defensive choice. Clean.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已观测到的 bug — Ubuntu PR CI 在 jsdom animation-frame 测量后抛出 TypeError: textRange(...).getClientRects is not a function。关联 #6578#6662,提供了 before/after 测试输出。

方向:对齐。在共享测试 setup 中补充 jsdom 缺失的 polyfill 正是 setup.ts 已有的模式,此 PR 延续了该模式覆盖 Range layout 方法。

规模:不适用 — 未触及核心基础设施路径。所有改动均为测试文件或测试 setup。36 行生产代码(setup.ts polyfill)、23 行测试代码、13/10 行测试修复。

方案:范围精简。两个文件专注做 polyfill + 测试,加上 CI 验证中发现的 DaemonSessionProvider 计时稳定化修复。使用 ??= 是正确的防御性做法。干净。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Code Review

Clean. The PR extends the existing setup.ts polyfill pattern with Range layout methods — getClientRects and getBoundingClientRect. Uses ??= to only polyfill when jsdom doesn't provide the methods natively. The createEmptyDOMRect fallback for environments where DOMRect isn't a constructor is a good defensive touch. The test locks the contract.

The DaemonSessionProvider timing fix uses createDeferred to ensure the heartbeat failure is only released after submitPrompt has started, eliminating the race condition. Straightforward and correct.

No issues found.

Testing

This is test infrastructure — no user-visible behavior change. Ran the key test suites locally on the PR branch:

setup.test.ts (new test)

$ cd packages/web-shell && npx vitest run test/setup.test.ts

 RUN  v3.2.4 packages/web-shell/client

 ✓ test/setup.test.ts (1 test) 3ms

 Test Files  1 passed (1)
      Tests  1 passed (1)
   Duration  466ms

useComposerCore.dom.test.tsx (the test that triggered the CI failure)

$ cd packages/web-shell && npx vitest run hooks/useComposerCore.dom.test.tsx

 RUN  v3.2.4 packages/web-shell/client

 ✓ hooks/useComposerCore.dom.test.tsx (4 tests) 141ms

 Test Files  1 passed (1)
      Tests  4 passed (4)
   Duration  1.16s

DaemonSessionProvider.test.tsx (timing stabilization)

$ cd packages/webui && npx vitest run src/daemon/session/DaemonSessionProvider.test.tsx

 ✓ src/daemon/session/DaemonSessionProvider.test.tsx (146 tests) 3197ms

 Test Files  1 passed (1)
      Tests  146 passed (146)
   Duration  4.31s

Full web-shell suite

$ cd packages/web-shell && npx vitest run

 Test Files  83 passed (83)
      Tests  1340 passed (1340)
   Duration  4.65s

Typecheck & Lint

$ cd packages/web-shell && npx tsc --noEmit
(clean)

$ npx eslint packages/web-shell/client/test/setup.ts packages/web-shell/client/test/setup.test.ts packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx
(clean)

All green — no TypeError: textRange(...).getClientRects is not a function unhandled error. The polyfill works.

中文说明

代码审查

干净。PR 延续了 setup.ts 已有的 polyfill 模式,补充了 Range layout 方法。使用 ??= 仅在 jsdom 未提供方法时生效。createEmptyDOMRectDOMRect 不作为构造函数可用的环境做了降级处理。测试锁住了契约。

DaemonSessionProvider 计时修复使用 createDeferred 确保 heartbeat 失败仅在 submitPrompt 启动后释放,消除了竞态条件。简洁正确。未发现问题。

测试

测试基础设施修复——无用户可见行为变化。本地在 PR 分支上运行了关键测试套件,全部通过:setup.test.ts 1 个测试、useComposerCore.dom.test.tsx 4 个测试、DaemonSessionProvider.test.tsx 146 个测试、web-shell 全量 83 文件 1340 个测试。Typecheck 和 lint 均干净。未出现 getClientRects 未处理错误。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

This is a tight, well-scoped test infrastructure fix. The problem is real and well-documented (CI failures in #6578, #6662), the solution follows the exact pattern already established in setup.ts for polyfilling jsdom gaps, and the tests confirm it works — 83 files, 1340 tests in web-shell all green, plus the DaemonSessionProvider timing fix verified at 146 tests. No over-engineering, no scope creep. LGTM. ✅

中文说明

这是一个范围明确、精简的测试基础设施修复。问题是真实且有据可查的(#6578#6662 中的 CI 失败),方案完全遵循 setup.ts 中已有的 jsdom polyfill 模式,测试确认有效——web-shell 83 个文件 1340 个测试全部通过,DaemonSessionProvider 计时修复 146 个测试也通过。没有过度工程,没有范围蔓延。LGTM。✅

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Downgraded from Approve to Comment: CI failing: Test (ubuntu-latest, Node 22.x). Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

No new Suggestion-level findings this round — all prior suggestions have been addressed or superseded.

— qwen3.7-max via Qwen Code /review

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

@yiliang114

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@wenshao
wenshao added this pull request to the merge queue Jul 10, 2026
Merged via the queue into QwenLM:main with commit f06e932 Jul 10, 2026
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants