fix(e2e): keep phase log tails UTF-8 safe by wangmiao0668000666 · Pull Request #109923 · openclaw/openclaw

wangmiao0668000666 · 2026-07-17T10:29:31Z

What Problem This Solves

PhaseRunner (scripts/e2e/parallels/phase-runner.ts) keeps a bounded in-memory log tail per phase (appendTextTail) and writes that tail to stderr when a phase fails. The bound is applied as a raw byte cut — Buffer.from(combined).subarray(-tailBytes).toString("utf8") — so when the retained window starts in the middle of a multi-byte UTF-8 character (emoji/CJK in test output), the diagnostics tail begins with U+FFFD replacement characters.

This is the same bug family as merged #109669 (scripts/e2e/lib/text-file-utils.mjs) and open sibling #109670 (scripts/e2e/parallels/host-server.ts); phase-runner.ts is the remaining byte-based tail decoder in the parallels harness. The three PRs touch separate files and are not stacked; any order merges cleanly.

Why This Change Was Made

#109669 introduced decodeUtf8Tail, which drops leading UTF-8 continuation bytes before decoding a truncated window. This PR exports that helper and reuses it in appendTextTail, so the byte cap stays unchanged while the emitted tail always starts on a character boundary. The change is three lines in prod code plus a focused regression test.

Root cause on main (scripts/e2e/parallels/phase-runner.ts:18): the byte cut from subarray(-tailBytes) can start on a UTF-8 continuation byte, and toString("utf8") replaces each orphaned continuation byte with U+FFFD.

Deliberately out of scope: scripts/e2e/parallels/filesystem.ts has a similar byte-window read, but its only consumer is a regex scan (extractLastOpenClawVersionFromLog) whose ASCII pattern cannot straddle a character boundary, so a partial leading character there has no observable effect.

User Impact

Maintainers reading failed E2E phase output get a clean log tail instead of leading mojibake whenever multibyte text (emoji in test names/output, CJK) crosses the 512 KiB tail cutoff.

Evidence

Real behavior proof against the production PhaseRunner on main (driver: construct with a 128-byte cap, append() 134 bytes containing an emoji positioned so the byte window starts one byte into it, fail the phase, capture stderr):

Before (origin/main):

[phase log tail truncated to last 128 bytes]
���yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
contains U+FFFD replacement char: true

After (this branch):

[phase log tail truncated to last 128 bytes]
yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
contains U+FFFD replacement char: false

Regression coverage: test/scripts/parallels-phase-runner.test.ts drives the real class through a failing phase and asserts the stderr tail (1) contains no U+FFFD when the byte cut splits a character, (2) keeps the full emoji tail when the cut lands on a character boundary, (3) leaves short logs untruncated.

node scripts/run-vitest.mjs test/scripts/parallels-phase-runner.test.ts — 3/3 pass on this branch; the split-character test fails on unpatched phase-runner.ts (control-red verified by reverting the prod file to origin/main).
node scripts/run-vitest.mjs test/scripts/e2e-text-file-utils.test.ts — pass (sibling helper tests unaffected by the new export).
oxfmt --check on all touched files — clean; oxlint on touched files — 0 findings; tsgo --noEmit -p tsconfig.scripts.json — no errors in touched files (pre-existing packages/net-policy errors come from a stale local [email protected], unrelated).
git diff --check — clean.

clawsweeper · 2026-07-17T10:39:16Z

Codex review: needs maintainer review before merge. Reviewed July 17, 2026, 2:08 PM ET / 18:08 UTC.

Summary
The PR routes bounded Parallels phase-log tails through the shared UTF-8-safe tailText helper and adds PhaseRunner regression coverage for split characters, aligned characters, and untruncated short logs.

PR surface: Tests +63, Other +1. Total +64 across 2 files.

Reproducibility: yes. A production PhaseRunner driver positions an emoji across the retained byte boundary, reproduces U+FFFD on main, and the regression test was control-red against the unpatched implementation.

Review metrics: none identified.

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 branch is behind current main and several exact-head checks remain in progress, so the final merge result should be refreshed and reviewed once those checks complete.

Maintainer options:

Decide the mitigation before merge
Land the current canonical-helper implementation and regression tests after refreshing against current main and confirming the exact head passes the required checks.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No ClawSweeper repair job is needed because the prior mechanical finding is fixed; this PR now needs ordinary exact-head validation and maintainer landing review.

Security
Cleared: The patch only reuses an internal text utility and adds tests, with no new dependency, downloaded code, permission, secret, workflow, or publishing surface.

Review details

Best possible solution:

Land the current canonical-helper implementation and regression tests after refreshing against current main and confirming the exact head passes the required checks.

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

Yes. A production PhaseRunner driver positions an emoji across the retained byte boundary, reproduces U+FFFD on main, and the regression test was control-red against the unpatched implementation.

Is this the best way to solve the issue?

Yes. Reusing the merged tailText owner is narrower and more maintainable than exporting a lower-level decoder or retaining a parallel truncation implementation.

AGENTS.md: found and applied where relevant.

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

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 includes after-fix live output from the production PhaseRunner path, corresponding before-fix output, and control-red regression evidence.
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:

P3: This is a focused, low-blast-radius improvement to maintainer-facing E2E failure diagnostics rather than a user runtime regression.
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 includes after-fix live output from the production PhaseRunner path, corresponding before-fix output, and control-red regression evidence.
proof: sufficient: Contributor real behavior proof is sufficient. The PR includes after-fix live output from the production PhaseRunner path, corresponding before-fix output, and control-red regression evidence.

Evidence reviewed

PR surface:

Tests +63, Other +1. Total +64 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	0	0	0	0
Tests	1	63	0	+63
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	1	3	2	+1
Total	2	66	2	+64

What I checked:

Current-main problem remains: The live PR diff still replaces the byte-naive Buffer.subarray(...).toString("utf8") path relative to current main, so the central diagnostic corruption has not already been implemented elsewhere. (scripts/e2e/parallels/phase-runner.ts:18, f104c51e8598)
Prior finding fixed: The latest head imports and calls the existing tailText wrapper rather than exposing decodeUtf8Tail or duplicating its low-level truncation path. (scripts/e2e/parallels/phase-runner.ts:6, 927deb9d56c0)
Regression coverage: The added tests exercise the real PhaseRunner failure path and cover a byte cut inside an emoji, a cut on a character boundary, and a short log that should remain untruncated. (test/scripts/parallels-phase-runner.test.ts:32, 927deb9d56c0)
Real behavior proof: The PR provides production-path before/after output showing U+FFFD on main and clean text on the branch, plus a control-red check against the unpatched implementation. (scripts/e2e/parallels/phase-runner.ts:18, 927deb9d56c0)
Shared helper provenance: Merged PR fix(e2e): keep text file tails UTF-8 safe #109669 introduced the UTF-8-safe text-tail helper that this branch now reuses, establishing the canonical implementation boundary. (scripts/e2e/lib/text-file-utils.mjs:1, ce4a16b0d32d)

Likely related people:

qingminlong: Authored the merged change that established the UTF-8-safe tailText implementation reused by this PR and also worked on the adjacent Parallels diagnostic-tail occurrence. (role: introduced shared helper behavior; confidence: high; commits: ce4a16b0d32d, 07943109da2f; files: scripts/e2e/lib/text-file-utils.mjs, scripts/e2e/parallels/host-server.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-17T10:35:57.658Z sha 3173bf1 :: needs maintainer review before merge. :: none
reviewed 2026-07-17T14:57:38.596Z sha a7624be :: needs changes before merge. :: [P1] Reuse the existing tailText wrapper

wangmiao0668000666 · 2026-07-17T14:49:35Z

Fixed the check-test-types failure in a7624be: phase-runner.ts imports decodeUtf8Tail from text-file-utils.mjs, but the adjacent declaration file text-file-utils.d.mts did not export it (TS2305). Added the missing declaration line.

Focused proof: node scripts/run-vitest.mjs test/scripts/parallels-phase-runner.test.ts — 3/3 passed locally.

wangmiao0668000666 · 2026-07-17T18:02:20Z

ClawSweeper P1 fixed in 927deb9 (pushed; new head).

phase-runner.ts now reuses the existing tailText(combined, tailBytes) wrapper from scripts/e2e/lib/text-file-utils.mjs instead of exporting decodeUtf8Tail and hand-rolling the Buffer.subarray cut.
decodeUtf8Tail is back to module-private (still used internally by tailText/readTextFileTail); its .d.mts declaration line added in a7624be is reverted.
Behavior unchanged: the U+FFFD before/after regression tests are untouched and still pass.

Validation (Node 24.15.0):

node scripts/run-vitest.mjs test/scripts/parallels-phase-runner.test.ts test/scripts/e2e-text-file-utils.test.ts
# Test Files 2 passed (2), Tests 6 passed (6)

git diff --check clean.

The phase runner keeps a bounded in-memory log tail per phase and writes it to stderr when a phase fails. The byte-based cut can land inside a multi-byte UTF-8 character, and decoding from that offset emits U+FFFD replacement characters at the start of the diagnostics tail. Reuse the decodeUtf8Tail helper from scripts/e2e/lib/text-file-utils.mjs (openclaw#109669) so the retained window starts on a character boundary. Co-Authored-By: Claude <[email protected]>

phase-runner.ts imports decodeUtf8Tail from text-file-utils.mjs; the adjacent declaration file must export it or check-test-types fails with TS2305. Co-Authored-By: Claude <[email protected]>

Co-Authored-By: Claude <[email protected]>

Co-authored-by: wangmiao0668000666 <[email protected]>

steipete · 2026-07-21T02:45:40Z

Merged via squash.

Prepared head SHA: 0eb4532341303c4de9d06a0d2e1e65cc004185bb
Landed commit: 998f0ed27d81caf391be22fd0833cd0c81419f3b

* fix(e2e): keep phase log tails UTF-8 safe The phase runner keeps a bounded in-memory log tail per phase and writes it to stderr when a phase fails. The byte-based cut can land inside a multi-byte UTF-8 character, and decoding from that offset emits U+FFFD replacement characters at the start of the diagnostics tail. Reuse the decodeUtf8Tail helper from scripts/e2e/lib/text-file-utils.mjs (openclaw#109669) so the retained window starts on a character boundary. Co-Authored-By: Claude <[email protected]> * fix(e2e): declare decodeUtf8Tail in text-file-utils types phase-runner.ts imports decodeUtf8Tail from text-file-utils.mjs; the adjacent declaration file must export it or check-test-types fails with TS2305. Co-Authored-By: Claude <[email protected]> * refactor(e2e): reuse tailText wrapper for phase log tails Co-Authored-By: Claude <[email protected]> * test(e2e): focus phase tail regression Co-authored-by: wangmiao0668000666 <[email protected]> --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added scripts Repository scripts size: S labels Jul 17, 2026

wangmiao0668000666 mentioned this pull request Jul 17, 2026

fix(e2e): keep telegram proof diagnostic tails Unicode safe #109940

Open

clawsweeper Bot mentioned this pull request Jul 17, 2026

fix(parallels): keep host server stderr tails UTF-8 safe #109670

Closed

steipete self-assigned this Jul 21, 2026

steipete force-pushed the fix/e2e-phase-log-tail-utf8-safe branch from 927deb9 to 3e19f99 Compare July 21, 2026 02:30

openclaw-barnacle Bot added size: XS and removed size: S labels Jul 21, 2026

wangmiao0668000666 and others added 4 commits July 20, 2026 19:31

fix(e2e): declare decodeUtf8Tail in text-file-utils types

ad8b5ab

phase-runner.ts imports decodeUtf8Tail from text-file-utils.mjs; the adjacent declaration file must export it or check-test-types fails with TS2305. Co-Authored-By: Claude <[email protected]>

refactor(e2e): reuse tailText wrapper for phase log tails

700abbe

Co-Authored-By: Claude <[email protected]>

test(e2e): focus phase tail regression

0eb4532

Co-authored-by: wangmiao0668000666 <[email protected]>

steipete force-pushed the fix/e2e-phase-log-tail-utf8-safe branch from 3e19f99 to 0eb4532 Compare July 21, 2026 02:31

steipete merged commit 998f0ed into openclaw:main Jul 21, 2026
98 checks passed

github-actions Bot mentioned this pull request Jul 21, 2026

📡 Upstream Digest — 2026-07-21 05:13 UTC curtismercier/openclaw-mods#1360

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(e2e): keep phase log tails UTF-8 safe#109923

fix(e2e): keep phase log tails UTF-8 safe#109923
steipete merged 4 commits into
openclaw:mainfrom
wangmiao0668000666:fix/e2e-phase-log-tail-utf8-safe

wangmiao0668000666 commented Jul 17, 2026

Uh oh!

clawsweeper Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

wangmiao0668000666 commented Jul 17, 2026

Uh oh!

wangmiao0668000666 commented Jul 17, 2026

Uh oh!

Uh oh!

steipete commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

wangmiao0668000666 commented Jul 17, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wangmiao0668000666 commented Jul 17, 2026

Uh oh!

wangmiao0668000666 commented Jul 17, 2026

Uh oh!

Uh oh!

steipete commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jul 17, 2026 •

edited

Loading