Skip to content

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

Closed
lzyyzznl wants to merge 1 commit into
openclaw:mainfrom
lzyyzznl:fix/truncateUtf16Safe-prometheus-diag
Closed

fix(diagnostics-prometheus): use truncateUtf16Safe for error message truncation#102613
lzyyzznl wants to merge 1 commit into
openclaw:mainfrom
lzyyzznl:fix/truncateUtf16Safe-prometheus-diag

Conversation

@lzyyzznl

@lzyyzznl lzyyzznl commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

extensions/diagnostics-prometheus/src/service.ts uses raw .slice(0, N) for string truncation at error message redactSensitiveText(...).slice(0, 500). When the text contains multi-byte Unicode characters such as emoji, a code-unit slice can split a UTF-16 surrogate pair, producing invalid Unicode.

Why This Change Was Made

Replacing these .slice(0, N) calls with truncateUtf16Safe ensures the truncated output is always valid Unicode while keeping identical behavior for ASCII-only content.

User Impact

Truncated text in the affected code path remains valid Unicode at the existing size limits. No behavioral, API, or performance changes for ASCII-only input.

Evidence

  • Mechanical one-to-one replacement: .slice(0, N) -> truncateUtf16Safe(str, N)
  • truncateUtf16Safe is already used extensively in the codebase following the same pattern
  • No runtime behavior change for the common case (valid Unicode or ASCII input)

Generated with Claude Code

@lzyyzznl
fix(diagnostics-prometheus): use truncateUtf16Safe for error message …
f790f6a 
…truncation

One .slice(0, 500) truncation site in the Prometheus diagnostics
extension may cut UTF-16 surrogate pairs in half when the error
message contains multi-byte characters such as emoji. Replace it with
truncateUtf16Safe to keep the truncated output valid Unicode.
@openclaw-barnacle openclaw-barnacle Bot added extensions: diagnostics-prometheus size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 9, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 9, 2026
Merged
@clawsweeper

clawsweeper Bot commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I did a careful shell check against current main, and this is already implemented.

Close: current main already has the same Prometheus diagnostics UTF-16-safe truncation fix plus regression coverage from the merged #102591; the latest release v2026.6.11 predates the fix, so this is current-main-only.

Root-cause cluster
Relationship: duplicate
Canonical: #102591
Summary: This PR duplicates the same diagnostics-prometheus safeErrorMessage truncation fix that already merged in the canonical sibling PR.

Members:

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

So I’m closing this as already implemented rather than keeping a duplicate issue open.

Review details

Best possible solution:

Keep the merged #102591 implementation on main and close this duplicate branch rather than reviewing or landing it independently.

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

No failing current-main reproduction remains. Current main contains the shared-helper fix and a regression test that exercises the diagnostics listener with surrogate-pair text crossing the truncation boundary.

Is this the best way to solve the issue?

Yes, current main is now the best solution. It preserves redaction, whitespace normalization, and the 500-code-unit cap while adding focused regression coverage that this duplicate branch does not carry.

Security review:

Security review cleared: The diff only imports an existing OpenClaw SDK text helper and changes local string truncation; no security or supply-chain concern was found.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • Pandah97: Authored the merged sibling PR and the current main fix commit that replaced the Prometheus diagnostics error truncation with truncateUtf16Safe and added regression coverage. (role: merged fix author; confidence: high; commits: fae5421f8116; files: extensions/diagnostics-prometheus/src/service.ts, extensions/diagnostics-prometheus/src/service.test.ts)
  • Ayaan Zaidi: Blame and file history show this person introduced the diagnostics-prometheus service and test files that contained the original raw truncation path. (role: introduced original service path; confidence: medium; commits: 4cbe49ba881a; files: extensions/diagnostics-prometheus/src/service.ts, extensions/diagnostics-prometheus/src/service.test.ts)
  • steipete: GitHub PR metadata and comments show this person posted land-ready verification and merged the canonical sibling implementation. (role: merger and verification commenter; confidence: medium; commits: fae5421f8116; files: extensions/diagnostics-prometheus/src/service.ts, extensions/diagnostics-prometheus/src/service.test.ts)

Codex review notes: model internal, reasoning high; reviewed against 2f66c3c0c845; fix evidence: commit fae5421f8116, main fix timestamp 2026-07-09T11:04:47+01:00.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. 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
@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 9, 2026
@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

extensions: diagnostics-prometheus P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@lzyyzznl