Skip to content

fix(browser): keep Playwright truncation UTF-16 safe#101761

Merged
vincentkoc merged 2 commits into
openclaw:mainfrom
mushuiyu886:fix/followup-101591
Jul 8, 2026
Merged

fix(browser): keep Playwright truncation UTF-16 safe#101761
vincentkoc merged 2 commits into
openclaw:mainfrom
mushuiyu886:fix/followup-101591

Conversation

@mushuiyu886

@mushuiyu886 mushuiyu886 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Keep Playwright browser response-body truncation UTF-16 safe.
  • Keep Playwright AI snapshot truncation UTF-16 safe before appending the page-too-large marker.
  • Keep Chrome MCP AI snapshot truncation UTF-16 safe for the same maxChars route contract.
  • Add regression coverage for all three truncation paths.

What Problem This Solves

Browser tool output can be truncated at a raw JavaScript string index. When the limit lands between the two code units of an emoji or other surrogate pair, OpenClaw can return a dangling surrogate in the browser response body or AI snapshot text.

User Impact

Users inspecting browser response bodies or page snapshots can see malformed Unicode output when content is truncated near an emoji boundary. The fix preserves the existing size limits and truncation markers while avoiding invalid UTF-16 in returned browser tool text.

  • Why it matters / User impact: Browser response body and AI snapshot output are directly shown to the agent/user; malformed UTF-16 can make truncated browser evidence confusing or lossy when page text or response bodies contain emoji.
  • What did NOT change: Only the Playwright response-body truncation boundary and the two browser AI snapshot truncation boundaries changed. Routing, navigation, network matching, byte limits, provider behavior, channel behavior, config, and unrelated browser tool paths are unchanged and out of scope.
  • Architecture / source-of-truth check: The source-of-truth contract is that browser helper output must remain bounded and valid UTF-16. The canonical helper for that contract is truncateUtf16Safe from openclaw/plugin-sdk/text-utility-runtime, which the browser extension already imports, so this patch stays inside the existing plugin architecture boundary.

Origin / follow-up

Follows #101591, which fixed another user-visible UI truncation path by making emoji / surrogate-pair clipping UTF-16 safe.

Gap this fills: the browser Playwright response-body, Playwright AI snapshot, and Chrome MCP AI snapshot helpers still used direct .slice(0, maxChars) / .slice(0, limit) truncation in user-visible browser tool output.

Consistency: this patch reuses the existing truncateUtf16Safe helper already used in the browser extension instead of introducing a new truncation implementation.

Competition / linked PR analysis

  • Related open PR scan / competition scan: checked open PRs touching extensions/browser/src/browser/pw-tools-core.responses.ts, extensions/browser/src/browser/pw-tools-core.snapshot.ts, and extensions/browser/src/browser/chrome-mcp.snapshot.ts; no matching open PR found.
  • This is a focused follow-up, not a broad Unicode sweep. It only changes the Playwright response-body boundary plus the two browser AI snapshot output paths covered below.

Real behavior proof

  • Behavior or issue addressed: Browser Playwright response-body, Playwright AI snapshot, and Chrome MCP AI snapshot truncation no longer returns dangling surrogate halves when maxChars cuts through an emoji.
  • Canonical reachability path: user browser command/input supplies response or snapshot options including maxChars → browser route/type layer passes typed options into the Playwright or Chrome MCP runtime helper → helper normalizes the limit with Math.floor / bounded option handling → runtime object reads a Playwright response, calls page.ariaSnapshot({ mode: "ai" }), or converts a Chrome MCP structured snapshot into browser AI snapshot text → helper truncates returned body/snapshot → agent/user receives browser tool output.
  • Boundary crossed: local-runtime; the Playwright proof starts a real local Google Chrome process, connects over CDP, uses Playwright page APIs, serves a local HTTP response, and calls the production browser helpers. The Chrome MCP sibling path is covered by a focused regression test for the same public AI snapshot maxChars contract requested in review.
  • Shared helper / provider constraint check: Reused truncateUtf16Safe from openclaw/plugin-sdk/text-utility-runtime in both Playwright and Chrome MCP browser snapshot helpers. Provider constraints are N/A because this does not change provider-owned API contracts.
  • Real environment tested: Linux worktree /media/vdb/code/ai/aispace/openclaw-worktrees/pr-101761, Node 22.22.0, local /usr/bin/google-chrome headless CDP, local HTTP server on 127.0.0.1.
  • Exact steps or command run after this patch: cd /media/vdb/code/ai/aispace/openclaw-worktrees/pr-101761 && node --import tsx /media/vdb/code/ai/aispace/openclaw-followup-101591-evidence/browser-playwright-unicode-proof.mjs > /media/vdb/code/ai/aispace/openclaw-pr-101761-evidence/after-pr-review-fix-browser-proof.json
  • Evidence after fix: /media/vdb/code/ai/aispace/openclaw-pr-101761-evidence/after-pr-review-fix-browser-proof.json shows responseBodyViaPlaywright.body is "", responseBodyViaPlaywright.hasLoneSurrogate is false, snapshotAiViaPlaywright.hasLoneSurrogate is false, and both Playwright paths remain truncated: true. /media/vdb/code/ai/aispace/openclaw-pr-101761-evidence/vitest-browser-review-fix.txt shows the requested Chrome MCP snapshot regression passed with the two existing browser test files.
  • Observed result after fix: The same local Chrome/CDP path that produced dangling surrogates on origin/main now returns UTF-16-safe truncated text for Playwright response body and AI snapshot output. The Chrome MCP sibling AI snapshot test now verifies the same surrogate-boundary truncation case uses the shared safe helper.
  • What was not tested: No external website, remote browser, or third-party service was used; the bug is in local browser output truncation, so the proof uses a local Chrome instance and local HTTP response.
  • Fix classification: Root cause fix

Review findings addressed

  • RF-001 (status: ⏳ waiting on author): addressed by the code/test update below and ready for a fresh review after push.
  • Reviewer proof request / local evidence judgment: the reviewer accepted the original local Chrome/CDP proof for the Playwright paths as sufficient, and this update attaches current-PR-head local Chrome/CDP proof plus the requested Chrome MCP regression test. No third-party provider, channel, transport, account, or external-service live boundary is part of this browser truncation fix.
  • RF-002 / RF-005 (chrome-mcp.snapshot.ts sibling path): fixed by applying truncateUtf16Safe in buildAiSnapshotFromChromeMcpSnapshot.
  • RF-003 / RF-004 (same-route Chrome MCP existing-session gap): addressed by covering the Chrome MCP AI snapshot maxChars branch instead of leaving the Playwright-only fix.
  • RF-006 (requested test command): addressed with node scripts/run-vitest.mjs extensions/browser/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts extensions/browser/src/browser/pw-tools-core.snapshot.test.ts extensions/browser/src/browser/chrome-mcp.snapshot.test.ts.

Regression Test Plan

  • node scripts/run-vitest.mjs extensions/browser/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts extensions/browser/src/browser/pw-tools-core.snapshot.test.ts extensions/browser/src/browser/chrome-mcp.snapshot.test.ts
  • Evidence: /media/vdb/code/ai/aispace/openclaw-pr-101761-evidence/vitest-browser-review-fix.txt reports 3 test files passed and 27 tests passed.
  • Target test file: extensions/browser/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts, extensions/browser/src/browser/pw-tools-core.snapshot.test.ts, and extensions/browser/src/browser/chrome-mcp.snapshot.test.ts.
  • Scenario locked in: Each regression test places the truncation limit between the high and low surrogate of 🙂, then verifies the returned browser output does not include the dangling half.
  • Why this is the smallest reliable guardrail: The tests cover the exact three helper-level truncation boundaries changed by the patch, while the real Chrome/CDP proof covers the local-runtime Playwright browser path.

Merge risk

  • Risk labels considered: merge-risk: compatibility and merge-risk: availability considered; neither should be required because this patch does not change limits, protocol shape, routing, process lifecycle, or external service behavior.
  • Risk explanation: The only output difference occurs when a truncation boundary would split a surrogate pair; the safe helper returns a valid shorter prefix instead of malformed UTF-16.
  • Why acceptable: Existing truncation semantics are preserved for ordinary text, and the maximum returned text length remains bounded by the caller's limit plus the existing snapshot marker where applicable.

Patch quality notes

  • Maintainer-ready confidence: High. The diff is six files, reuses an existing helper, has before/after real Chrome/CDP proof for the Playwright paths, and adds focused regression tests for all three changed truncation boundaries.
  • Patch quality notes: No new helper, no broad Unicode sweep, no unrelated formatting, and no dependency or lockfile changes. The public-output contract stays compatible because returned strings remain bounded and valid.

Risk / Compatibility

Low. The response body byte cap, maxChars option, truncation flag, and snapshot truncation marker behavior stay the same across Playwright and Chrome MCP snapshot output. The only behavior change is that a truncation boundary may move back by one UTF-16 code unit when it would otherwise split a surrogate pair.

What was not changed

  • No browser routing, CDP connection, navigation, or network matching behavior changed.
  • No response byte limit changed.
  • No snapshot URL collection, Chrome MCP structured snapshot conversion, or role-ref storage behavior changed.
  • No provider/channel contracts changed.

Root Cause

  • Root cause: The source truncation invariant for browser helper output is bounded text that remains valid UTF-16, but the Playwright response-body, Playwright AI snapshot, and Chrome MCP AI snapshot helpers bypassed that contract with direct .slice(0, maxChars) / .slice(0, limit). That caused the runtime browser output path to split surrogate pairs when the caller's limit landed between the emoji code units.
  • Why this is root-cause fix: The fix moves all three proven truncation sites back onto the repository's existing UTF-16-safe text helper, so the source-of-truth contract is enforced at the boundary that returns browser tool output rather than masked downstream. Because the same Chrome/CDP response, page.ariaSnapshot({ mode: "ai" }), and Chrome MCP AI snapshot conversion paths now call truncateUtf16Safe before returning text, they no longer emit dangling surrogate halves while preserving the existing truncation semantics.

@clawsweeper

clawsweeper Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 7, 2026, 1:07 PM ET / 17:07 UTC.

Summary
The PR changes Playwright response-body, Playwright AI snapshot, and Chrome MCP AI snapshot truncation to use truncateUtf16Safe, with focused regression tests for each path.

PR surface: Source +2, Tests +63. Total +65 across 6 files.

Reproducibility: yes. source-level. Current main uses raw .slice(0, maxChars) / .slice(0, limit) at the affected browser boundaries, and the PR tests exercise a surrogate-pair split that would otherwise produce a dangling half.

Review metrics: none identified.

Root-cause cluster
Relationship: canonical
Canonical: #101761
Summary: This PR is the canonical open item for the browser response and snapshot truncation boundaries; related UTF-16 PRs are adjacent fixes on other surfaces.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] The live PR is behind current main, so final landing should refresh the branch and rely on exact-head CI before merge.

Maintainer options:

  1. Decide the mitigation before merge
    Land the helper-based browser truncation fix after refreshing against current main and passing the normal exact-head CI and maintainer review gates.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No ClawSweeper repair lane is needed; the prior Chrome MCP sibling gap is fixed and the remaining action is ordinary maintainer merge handling after refresh/CI.

Security
Cleared: The diff only changes TypeScript truncation calls and colocated tests; it does not add dependencies, workflows, package metadata, permissions, downloads, or secret-handling surface.

Review details

Best possible solution:

Land the helper-based browser truncation fix after refreshing against current main and passing the normal exact-head CI and maintainer review gates.

Do we have a high-confidence way to reproduce the issue?

Yes, source-level. Current main uses raw .slice(0, maxChars) / .slice(0, limit) at the affected browser boundaries, and the PR tests exercise a surrogate-pair split that would otherwise produce a dangling half.

Is this the best way to solve the issue?

Yes. Reusing the existing truncateUtf16Safe helper at the three owning browser output boundaries is the narrowest maintainable fix; broader UTF-16 sweeps are adjacent work, not required in this PR.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against fa84741f93b9.

Label changes

Label changes:

  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied after-fix local Chrome/CDP live output for the Playwright paths and copied focused regression output for the Chrome MCP path, which is sufficient non-visual proof for this local browser truncation fix.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This is a bounded browser-output correctness fix with limited blast radius and no security, data-loss, or release-blocking signal.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied after-fix local Chrome/CDP live output for the Playwright paths and copied focused regression output for the Chrome MCP path, which is sufficient non-visual proof for this local browser truncation fix.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied after-fix local Chrome/CDP live output for the Playwright paths and copied focused regression output for the Chrome MCP path, which is sufficient non-visual proof for this local browser truncation fix.
Evidence reviewed

PR surface:

Source +2, Tests +63. Total +65 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 3 5 3 +2
Tests 3 63 0 +63
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 68 3 +65

What I checked:

Likely related people:

  • steipete: Git history shows Peter Steinberger introduced the relevant browser plugin files in 8eeb7f0 and authored the current main helper/raw-boundary commit 84d0a71. (role: introduced browser plugin surface and shared helper owner; confidence: high; commits: 8eeb7f082975, 84d0a714062d; files: extensions/browser/src/browser/pw-tools-core.responses.ts, extensions/browser/src/browser/pw-tools-core.snapshot.ts, extensions/browser/src/browser/chrome-mcp.snapshot.ts)
  • vincentkoc: Recent history shows c16bb87 touched adjacent browser snapshot text paths with the same UTF-16-safe truncation invariant. (role: recent adjacent browser UTF-16 contributor; confidence: medium; commits: c16bb8725a8a; files: extensions/browser/src/browser/cdp.ts, extensions/browser/src/browser/pw-tools-core.snapshot.ts, extensions/browser/src/browser/routes/agent.snapshot.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (2 earlier review cycles)
  • reviewed 2026-07-07T15:47:37.688Z sha c772674 :: needs changes before merge. :: [P2] Cover the Chrome MCP snapshot truncation path
  • reviewed 2026-07-07T16:39:30.468Z sha 97c81af :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. labels Jul 7, 2026
@mushuiyu886

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 7, 2026
@vincentkoc vincentkoc self-assigned this Jul 8, 2026
@vincentkoc

Copy link
Copy Markdown
Member

Maintainer verification complete on exact head 97c81af0f442c8d23b854fe79fd6908a5fe1fd8b.

  • Reviewed all three changed browser output boundaries plus the shared truncateUtf16Safe contract and sibling implementations.
  • node scripts/run-vitest.mjs extensions/browser/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts extensions/browser/src/browser/pw-tools-core.snapshot.test.ts extensions/browser/src/browser/chrome-mcp.snapshot.test.ts --maxWorkers=1 --reporter=dot — 27 passed.
  • Targeted oxlint, oxfmt --check, and git diff --check passed for all six changed files.
  • AutoReview (gpt-5.5, high reasoning) completed cleanly with no accepted/actionable findings.
  • Native OPENCLAW_TESTBOX=1 scripts/pr prepare-run 101761 passed in hosted_exact_or_recent_rebase mode at 2026-07-08 02:55 UTC. Live exact-head checks have 66 successful jobs and no pending jobs; remaining cancelled entries are superseded routine runs.

No proof gaps remain for this local browser truncation change. Proceeding with the native squash-merge path.

@vincentkoc
vincentkoc merged commit 5bdea31 into openclaw:main Jul 8, 2026
113 of 122 checks passed
@vincentkoc

Copy link
Copy Markdown
Member

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 8, 2026
* fix(browser): keep Playwright truncation UTF-16 safe

* fix(browser): cover Chrome MCP snapshot truncation
giodl73-repo pushed a commit to giodl73-repo/openclaw that referenced this pull request Jul 8, 2026
* fix(browser): keep Playwright truncation UTF-16 safe

* fix(browser): cover Chrome MCP snapshot truncation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants