fix(logging): keep bounded log text UTF-16 safe by lsr911 · Pull Request #102560 · openclaw/openclaw

lsr911 · 2026-07-09T08:30:27Z

What Problem This Solves

Bounded logger text used raw UTF-16 code-unit slices. A boundary inside an emoji could leave an unpaired surrogate in diagnostic events or JSONL file-log messages.

Why This Change Was Made

The diagnostic and file-log clamps in src/logging/logger.ts now share the existing UTF-16-safe truncation helper. Review expanded the original one-sided diagnostic fix to the sibling file-log path in the same owner module.

User Impact

Long diagnostic messages, diagnostic attributes, file-log messages, and promoted file-log context remain valid Unicode at their existing size limits. Markers, limits, redaction order, and short-message behavior are unchanged.

Evidence

Added public transport-level regressions for an emoji straddling the 4,096-code-unit diagnostic and file-log boundaries.
vitest.logging.config.ts focused run — 2 files passed; 2 tests passed, 15 skipped.
The first attempt exposed and preserved the existing two-stage diagnostic clamp marker behavior; the exact regression reflects the production output.
Fresh autoreview — clean, 0.98 confidence.

🤖 Generated with Claude Code

Replace naive .slice(0, maxChars) with truncateUtf16Safe() in clampDiagnosticLogText() — the core helper used by all diagnostic log output (sanitizeDiagnosticLogText, formatDiagnosticAttributes, etc.). This prevents surrogate pair splitting across the entire diagnostic logging subsystem. Co-Authored-By: Claude <[email protected]>

lsr911 · 2026-07-09T08:49:57Z

@steipete this one fixes the core diagnostic logging helper clampDiagnosticLogText() — one function that protects ALL diagnostic log output from surrogate splitting. Same pattern as the merged UTF-16 PRs. Would appreciate a look!

clawsweeper · 2026-07-09T08:53:12Z

Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 5:05 AM ET / 09:05 UTC.

Summary
The PR changes src/logging/logger.ts to use the existing UTF-16-safe truncation helper for bounded diagnostic and JSONL file-log text, with regression tests for both paths.

PR surface: Source +1, Tests +29. Total +30 across 3 files.

Reproducibility: yes. from source inspection, though not from an executed local run: current main's diagnostic and file-log clamps use raw UTF-16 code-unit slicing at the bound. A 4,095-character prefix followed by an emoji crosses the 4,096 limit and can leave a dangling surrogate before the truncation marker.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/logging/logger-redaction-behavior.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

[P1] Add redacted terminal/live output showing a real logger call at an emoji boundary producing a valid diagnostic event or JSONL file-log line.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body reports focused Vitest coverage and autoreview, but no redacted live logger output, terminal transcript, log excerpt, recording, or linked artifact showing the after-fix diagnostic/file-log behavior in a real setup. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] Real behavior proof is still test-only; before merge, a redacted live or terminal run should show an actual logger call producing valid diagnostic event or JSONL file-log text across an emoji boundary.
[P1] The PR was mergeable but behind base, and status checks still had in-progress CI jobs when inspected, so merge readiness should wait for exact-head green checks.

Maintainer options:

Decide the mitigation before merge
Land this focused logger clamp change after redacted real-behavior proof is added and exact-head checks are green, keeping the shared helper and both diagnostic/file-log regression tests.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] No automated repair is needed; the remaining blocker is contributor-side real behavior proof plus normal exact-head merge validation.

Security
Cleared: No concrete security or supply-chain concern found; the diff reuses an existing internal helper in logging and adds tests without package, workflow, lockfile, permission, or secret-handling expansion.

Review details

Best possible solution:

Land this focused logger clamp change after redacted real-behavior proof is added and exact-head checks are green, keeping the shared helper and both diagnostic/file-log regression tests.

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

Yes from source inspection, though not from an executed local run: current main's diagnostic and file-log clamps use raw UTF-16 code-unit slicing at the bound. A 4,095-character prefix followed by an emoji crosses the 4,096 limit and can leave a dangling surrogate before the truncation marker.

Is this the best way to solve the issue?

Yes for the code shape: reusing truncateUtf16Safe in both owner-module clamps is the narrowest maintainable fix, and the PR covers the diagnostic and file-log sibling surfaces. Merge readiness still needs real behavior proof.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P2: This is a normal, limited-blast-radius logging correctness bug affecting long diagnostic/file-log text rather than core runtime availability or user setup.
add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports focused Vitest coverage and autoreview, but no redacted live logger output, terminal transcript, log excerpt, recording, or linked artifact showing the after-fix diagnostic/file-log behavior in a real setup. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

P2: This is a normal, limited-blast-radius logging correctness bug affecting long diagnostic/file-log text rather than core runtime availability or user setup.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports focused Vitest coverage and autoreview, but no redacted live logger output, terminal transcript, log excerpt, recording, or linked artifact showing the after-fix diagnostic/file-log behavior in a real setup. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +1, Tests +29. Total +30 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	3	2	+1
Tests	2	29	0	+29
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	32	2	+30

What I checked:

Current main behavior: Current main still uses raw code-unit .slice(0, maxChars) in both logging clamps, so an emoji split at the bound can produce an unpaired surrogate before the truncation marker. (src/logging/logger.ts:93, e975d4923808)
PR implementation: The PR head imports truncateUtf16Safe and uses it in both clampDiagnosticLogText and clampFileLogText, keeping the fix in the owner module rather than adding a new helper path. (src/logging/logger.ts:93, 1a4cba3933d1)
Diagnostic regression coverage: The PR adds a diagnostic event regression for an emoji straddling the 4,096-code-unit message boundary and preserves the existing two-stage marker behavior. (src/logging/diagnostic-log-events.test.ts:146, 1a4cba3933d1)
File-log regression coverage: The PR adds a JSONL file-log regression for the same emoji boundary and parses the emitted record message after a real logger write inside the test harness. (src/logging/logger-redaction-behavior.test.ts:195, 1a4cba3933d1)
Helper contract: truncateUtf16Safe delegates to sliceUtf16Safe, which backs up the end index when it would leave a high surrogate dangling before a following low surrogate. (packages/normalization-core/src/utf16-slice.ts:43, e975d4923808)
Exported helper subpath: The @openclaw/normalization-core/utf16-slice subpath is already exported and widely imported in current source, so this PR reuses an established internal package contract. (packages/normalization-core/package.json:47, e975d4923808)

Likely related people:

steipete: Blame for the current logging module and both affected clamp lines points to 7698783037447f8e2661c5a0bcd4181e0edaddb9, and this PR's second commit broadens the fix to the sibling file-log path. (role: recent area contributor and PR committer; confidence: high; commits: 769878303744, 1a4cba3933d1, a9582a1bb62a; files: src/logging/logger.ts, src/logging/diagnostic-log-events.test.ts, src/logging/logger-redaction-behavior.test.ts)
lsr911: Beyond opening this PR, this contributor recently authored merged UTF-16 truncation work in src/agents/tool-policy-audit.ts, which is adjacent to the same helper-adoption pattern but not direct ownership of src/logging/logger.ts. (role: adjacent UTF-16 truncation contributor; confidence: medium; commits: 951f09451372, 7b33e2e0a685, a8929df7790a; files: src/logging/logger.ts, src/agents/tool-policy-audit.ts, src/agents/tool-policy-pipeline.test.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.

steipete · 2026-07-09T09:11:15Z

Land-ready review completed at exact head 1a4cba3933d14a9f21e48a484c88a20786c8db9f.

What changed during maintainer review:

kept the original diagnostic UTF-16 fix;
applied the same invariant to the sibling bounded JSONL file-log path;
added public transport-level regressions for both outputs, preserving their existing marker behavior.

Evidence:

node ../../node_modules/vitest/vitest.mjs run --config test/vitest/vitest.logging.config.ts src/logging/diagnostic-log-events.test.ts src/logging/logger-redaction-behavior.test.ts -t 'keeps bounded (diagnostic|file-log) messages UTF-16 safe' — 2 files passed; 2 tests passed, 15 skipped.
fresh autoreview — clean, 0.98 confidence.
native scripts/pr review-validate-artifacts 102560 — passed.
native OPENCLAW_TESTBOX=1 scripts/pr prepare-run 102560 — exact-head hosted CI/Testbox gates passed.

Known proof gaps: none for this internal logging boundary change.

steipete · 2026-07-09T09:11:32Z

Merged via squash.

Prepared head SHA: 1a4cba3933d14a9f21e48a484c88a20786c8db9f
Landed commit: 0de5d37f92fc1721596a7d1a0197cb23d699329a

* fix(logging): use truncateUtf16Safe in diagnostic log text clamp Replace naive .slice(0, maxChars) with truncateUtf16Safe() in clampDiagnosticLogText() — the core helper used by all diagnostic log output (sanitizeDiagnosticLogText, formatDiagnosticAttributes, etc.). This prevents surrogate pair splitting across the entire diagnostic logging subsystem. Co-Authored-By: Claude <[email protected]> * fix(logging): preserve UTF-16 in bounded log text --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 9, 2026

fix(logging): preserve UTF-16 in bounded log text

1a4cba3

steipete changed the title ~~fix(logging): use truncateUtf16Safe in diagnostic log text clamp~~ fix(logging): keep bounded log text UTF-16 safe Jul 9, 2026

openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 9, 2026

steipete merged commit 0de5d37 into openclaw:main Jul 9, 2026
127 of 134 checks passed

wangmiao0668000666 mentioned this pull request Jul 9, 2026

fix(text): keep five src-side text truncations UTF-16 safe #102630

Closed

9 tasks

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

📡 Upstream Digest — 2026-07-09 10:56 UTC curtismercier/openclaw-mods#1242

Open

wangmiao0668000666 mentioned this pull request Jul 9, 2026

fix(chat,cli): keep session snippet and CLI output digest truncation UTF-16 safe #102747

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(logging): keep bounded log text UTF-16 safe#102560

fix(logging): keep bounded log text UTF-16 safe#102560
steipete merged 2 commits into
openclaw:mainfrom
lsr911:fix/utf16-logger

lsr911 commented Jul 9, 2026 •

edited by steipete

Loading

Uh oh!

lsr911 commented Jul 9, 2026

Uh oh!

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

lsr911 commented Jul 9, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

lsr911 commented Jul 9, 2026

Uh oh!

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

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lsr911 commented Jul 9, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading