fix(diagnostics-prometheus): use truncateUtf16Safe for error message truncation by Pandah97 · Pull Request #102591 · openclaw/openclaw

Pandah97 · 2026-07-09T09:04:54Z

What Problem This Solves

The Prometheus diagnostics service bounded sanitized event-handler errors with slice(0, 500). When an error contained an astral character across that boundary, the log could end with an unpaired UTF-16 surrogate.

Why This Change Was Made

Use the existing truncateUtf16Safe utility at the final rendering boundary. The established redaction, null-byte removal, control-character normalization, and 500-code-unit limit remain unchanged.

The follow-up test exercises the public service lifecycle: it captures the registered diagnostics listener, makes the real model.usage recorder throw a boundary-straddling error through a property getter, and checks the complete logger output. This proves the changed call site rather than only the shared helper.

User Impact

Prometheus exporter event-handler failures remain bounded and sanitized, but their logs no longer contain malformed UTF-16 when an emoji or another surrogate pair crosses the limit. No config, API, metric, or dependency behavior changes.

Evidence

node scripts/run-vitest.mjs extensions/diagnostics-prometheus/src/service.test.ts — 17 tests passed.
oxfmt completed for the changed test.
git diff --check passed.
The regression asserts the exact public logger message after the subscribed diagnostics callback catches a real recorder exception at the 500-code-unit boundary.

Risk

Low. One rendering boundary now uses the shared truncation invariant; ASCII and BMP output within the existing limit is unchanged.

AI-assisted

This PR was generated with Claude Code and improved/reviewed by a maintainer agent.

…truncation The safeErrorMessage function uses naive .slice(0, 500) on error messages which can split surrogate pairs. Replace with truncateUtf16Safe().

clawsweeper · 2026-07-09T09:26:17Z

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

Summary
The branch replaces the Prometheus diagnostics exporter error-message .slice(0, 500) with truncateUtf16Safe(..., 500) and imports the SDK text helper.

PR surface: Source +3. Total +3 across 1 file.

Reproducibility: yes. Source inspection shows current main truncates safeErrorMessage with raw .slice(0, 500), and the diagnostics service logs that result on event-handler failures; a message with a surrogate pair crossing that boundary is a high-confidence reproduction path.

Review metrics: 1 noteworthy metric.

Duplicate open fixes: 2 open PRs. Two open PRs propose the same one-call-site fix, so maintainers should avoid reviewing or landing both.

Root-cause cluster
Relationship: canonical
Canonical: #102591
Summary: This PR and #102613 target the same diagnostics-prometheus safeErrorMessage truncation site; this earlier PR has fuller context and should remain the canonical candidate unless maintainers choose otherwise.

Members:

duplicate: fix(diagnostics-prometheus): use truncateUtf16Safe for error message truncation #102613 - The later PR changes the same file and same .slice(0, 500) call to the same helper, with no distinct behavior or stronger proof.

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

Merge readiness
Overall: 🦐 gold shrimp
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 live output, logs, or a Prometheus scrape/error-path transcript showing emoji-containing error text crossing the 500-code-unit boundary after the patch.
Update the PR body with that proof so ClawSweeper re-reviews automatically; if it does not, ask a maintainer to comment @clawsweeper re-review.

Proof guidance:

[P1] Needs real behavior proof before merge: Only source/compile/test reasoning is provided; before merge the contributor should add a live Prometheus/exporter error-path run, terminal output, or redacted logs showing a surrogate-boundary error message after the patch. 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 source/compile/test reasoning only; no live Prometheus scrape or diagnostics exporter error-path log shows the after-fix surrogate-boundary output.
[P1] A later duplicate open PR touches the same call site, so maintainers should land only one version of the fix.

Maintainer options:

Decide the mitigation before merge
Land one narrow PR for this call site after redacted runtime proof is added, then close the duplicate open PR rather than carrying both.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] This is not an automated repair candidate because the code change is already narrow and reviewable; the remaining blocker is contributor-provided real behavior proof before merge.

Security
Cleared: The diff only imports an existing SDK text helper and keeps redaction before truncation; it does not touch dependencies, workflows, secrets, package resolution, or permission surfaces.

Review details

Best possible solution:

Land one narrow PR for this call site after redacted runtime proof is added, then close the duplicate open PR rather than carrying both.

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

Yes. Source inspection shows current main truncates safeErrorMessage with raw .slice(0, 500), and the diagnostics service logs that result on event-handler failures; a message with a surrogate pair crossing that boundary is a high-confidence reproduction path.

Is this the best way to solve the issue?

Yes. Replacing only the final truncation with the exported SDK helper preserves redaction and the 500-code-unit cap while matching the sibling diagnostics-otel pattern; the remaining gap is proof, not fix shape.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P2: This is a normal limited-blast-radius plugin diagnostics bug fix, not an urgent core runtime or release-blocking regression.
add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; 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: Only source/compile/test reasoning is provided; before merge the contributor should add a live Prometheus/exporter error-path run, terminal output, or redacted logs showing a surrogate-boundary error message after the patch. 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 plugin diagnostics bug fix, not an urgent core runtime or release-blocking regression.
rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; 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: Only source/compile/test reasoning is provided; before merge the contributor should add a live Prometheus/exporter error-path run, terminal output, or redacted logs showing a surrogate-boundary error message after the patch. 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 +3. Total +3 across 1 file.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	7	4	+3
Tests	0	0	0	0
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	1	7	4	+3

What I checked:

Current main still has the bug pattern: safeErrorMessage sanitizes the message and then calls .slice(0, 500), which can split a surrogate pair at the truncation boundary. (extensions/diagnostics-prometheus/src/service.ts:231, 4ad94febcb31)
Runtime call path uses the unsafe result in logs: The diagnostics service catches event-handler errors and includes safeErrorMessage(err) in the exported logger error text. (extensions/diagnostics-prometheus/src/service.ts:1034, 4ad94febcb31)
PR patch is the targeted replacement: The PR wraps the existing redaction/null/control-character cleanup expression in truncateUtf16Safe(..., 500) without changing the cap or earlier sanitization. (extensions/diagnostics-prometheus/src/service.ts:229, 79efa8059361)
Helper contract supports the fix: truncateUtf16Safe floors and clamps the limit, returns unchanged short strings, and delegates truncation to sliceUtf16Safe to avoid dangling surrogate halves; tests cover normal truncation and surrogate-boundary behavior. (packages/normalization-core/src/utf16-slice.ts:44, 4ad94febcb31)
Sibling diagnostics surface already uses the helper: The OpenTelemetry diagnostics service truncates bounded log text with truncateUtf16Safe, supporting this as the shared observability pattern. (extensions/diagnostics-otel/src/service.ts:723, 4ad94febcb31)
SDK boundary allows the import: The scoped extensions guide requires extension production code to import shared helpers from openclaw/plugin-sdk/*, and text-utility-runtime exports truncateUtf16Safe. (extensions/AGENTS.md:23, 4ad94febcb31)

Likely related people:

zhangguiping-xydt: PR metadata maps the merged implementation stack for the current diagnostics/prometheus service and UTF-16 helper to this author, and blame attributes the current unsafe call site to the resulting merge commit. (role: introduced current implementation; confidence: high; commits: b23fdcfac953; files: extensions/diagnostics-prometheus/src/service.ts, packages/normalization-core/src/utf16-slice.ts, src/plugin-sdk/text-utility-runtime.ts)
steipete: GitHub PR metadata shows this person merged the stack that introduced the current diagnostics-prometheus service and also authored one commit in that stack. (role: merger and adjacent reviewer; confidence: medium; commits: b23fdcfac953; files: extensions/diagnostics-prometheus/src/service.ts, extensions/diagnostics-prometheus/src/service.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-09T10:03:35Z

Land-ready verification at exact head b73f9fd3d97c0841aeea8c35b455bdee239cbff3:

Improved the patch with a public-path regression through createDiagnosticsPrometheusExporter: the registered listener receives a model-usage event whose recorder throws an emoji boundary error, and the test asserts the complete logger message.
node scripts/run-vitest.mjs extensions/diagnostics-prometheus/src/service.test.ts — 17 tests passed.
oxfmt and git diff --check passed.
Fresh autoreview: clean, 0.98 confidence; no accepted/actionable findings.
Native OPENCLAW_TESTBOX=1 scripts/pr prepare-run 102591 passed exact-head hosted CI/Testbox gates.
Whole-surface review confirmed redaction and control-character normalization still precede the existing 500-code-unit limit; successful metric recording and untrusted-event filtering remain unchanged.

No docs, config, metrics, API, or dependency changes. Ready to merge.

steipete · 2026-07-09T10:04:53Z

Merged via squash.

Prepared head SHA: b73f9fd3d97c0841aeea8c35b455bdee239cbff3
Landed commit: fae5421f8116005a829d8c24811b52e889796770

…truncation (openclaw#102591) * fix(diagnostics-prometheus): use truncateUtf16Safe for error message truncation The safeErrorMessage function uses naive .slice(0, 500) on error messages which can split surrogate pairs. Replace with truncateUtf16Safe(). * test(diagnostics-prometheus): cover UTF-16-safe errors --------- Co-authored-by: Peter Steinberger <[email protected]>

fix(diagnostics-prometheus): use truncateUtf16Safe for error message …

79efa80

…truncation The safeErrorMessage function uses naive .slice(0, 500) on error messages which can split surrogate pairs. Replace with truncateUtf16Safe().

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

clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jul 9, 2026

wangmiao0668000666 mentioned this pull request Jul 9, 2026

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

Closed

9 tasks

clawsweeper Bot mentioned this pull request Jul 9, 2026

fix(diagnostics-prometheus): use truncateUtf16Safe for error message truncation #102613

Closed

test(diagnostics-prometheus): cover UTF-16-safe errors

b73f9fd

steipete merged commit fae5421 into openclaw:main Jul 9, 2026
95 checks passed

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

OpenClawKobian99 mentioned this pull request Jul 14, 2026

[Bug]: memory-core dreaming narrative fails with "no low surrogate in string" API 400 — clampDiaryContextEntry truncates mid-surrogate-pair (fix on main, missing from v2026.7.1) #107815

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(diagnostics-prometheus): use truncateUtf16Safe for error message truncation#102591

fix(diagnostics-prometheus): use truncateUtf16Safe for error message truncation#102591
steipete merged 2 commits into
openclaw:mainfrom
Pandah97:fix/truncateUtf16Safe-prometheus

Pandah97 commented Jul 9, 2026 •

edited by steipete

Loading

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

Pandah97 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

Risk

AI-assisted

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

Pandah97 commented Jul 9, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading