Skip to content

fix(telegram): keep raw update log truncation UTF-16 safe#101300

Closed
Alix-007 wants to merge 1 commit into
openclaw:mainfrom
Alix-007:alix/utf16-telegram-raw-update-log
Closed

fix(telegram): keep raw update log truncation UTF-16 safe#101300
Alix-007 wants to merge 1 commit into
openclaw:mainfrom
Alix-007:alix/utf16-telegram-raw-update-log

Conversation

@Alix-007

@Alix-007 Alix-007 commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • AI-assisted with Codex; I inspected the changed production path and can explain the change.
  • Telegram raw-update diagnostic string truncation preserves UTF-16 boundaries and does not emit escaped surrogate halves in JSON logs.
  • Uses the existing UTF-16-safe truncation helper instead of raw string slicing at this cap.
  • Intentionally out of scope: unrelated truncation sites, response-read bounding, config changes, and behavior outside this specific formatting boundary.

Linked context

  • No linked issue.
  • Related to the existing OpenClaw UTF-16-safe truncation hardening pattern.

Real behavior proof (required for external PRs)

  • Behavior addressed: Telegram raw-update diagnostic string truncation preserves UTF-16 boundaries and does not emit escaped surrogate halves in JSON logs.
  • Real environment tested: local OpenClaw source checkout on Node v22.22.0; PR head 79027a3.
  • Exact steps or command run after this patch: node scripts/run-vitest.mjs extensions/telegram/src/bot-core.raw-update-log.test.ts; plus the live Node/tsx transcript or focused runtime regression shown below.
  • Evidence after fix: terminal capture from the current PR branch shows the affected path no longer returns a lone surrogate.
$ node --import tsx -e '<drive stringifyTelegramRawUpdateForLog on 499 chars + rocket + tail>'
{"diagnosticLength":502,"diagnosticTail":"aaa...","rawHasEscapedSurrogate":false}

$ node scripts/run-vitest.mjs extensions/telegram/src/bot-core.raw-update-log.test.ts
Test Files 1 passed
Tests 3 passed
  • Observed result after fix: the affected telegram truncation path keeps output well-formed at the emoji/surrogate-pair boundary and preserves the existing truncation marker/cap behavior.
  • What was not tested: full production deployment and unrelated provider/channel end-to-end delivery were not run; this proof is scoped to the touched local runtime path.
  • Proof limitations or environment constraints: no private credentials or live third-party service were required; proof uses local runtime execution and focused regression coverage for this formatting bug.
  • Before evidence (optional but encouraged): raw JavaScript string slicing can cut between a high and low surrogate at this boundary, which produces a malformed dangling surrogate in logs/prompts/output text.

Tests and validation

  • node scripts/run-vitest.mjs extensions/telegram/src/bot-core.raw-update-log.test.ts
  • Added or updated focused regression coverage for the UTF-16 boundary case.
  • No known local failures from this change.

Risk checklist

Did user-visible behavior change? (Yes/No)

Yes. Malformed truncated text is now avoided while preserving existing caps and truncation markers.

Did config, environment, or migration behavior change? (Yes/No)

No.

Did security, auth, secrets, network, or tool execution behavior change? (Yes/No)

No.

What is the highest-risk area?

telegram raw update log redaction/truncation formatting.

How is that risk mitigated?

The patch is limited to the existing truncation boundary and is covered by focused regression proof above.

Current review state

What is the next action?

ClawSweeper re-review and maintainer review after this proof/body refresh.

What is still waiting on author, maintainer, CI, or external proof?

Nothing is waiting on the author after this proof update; waiting on CI/ClawSweeper/maintainer review.

Which bot or reviewer comments were addressed?

Addressed ClawSweeper's needs-proof feedback by using exact Real behavior proof field labels and adding copied terminal output from the current PR head.

@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram size: XS labels Jul 7, 2026
@clawsweeper

clawsweeper Bot commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 11:50 PM ET / 03:50 UTC.

Summary
The PR imports the shared UTF-16-safe truncation helper into Telegram raw update formatting and adds a regression test for a surrogate-pair boundary in long string values.

PR surface: Source +2, Tests +15. Total +17 across 2 files.

Reproducibility: yes. source inspection gives a high-confidence reproduction path: current main uses raw UTF-16 slices at the per-field cap and at the final raw-update preview cap, both of which can split surrogate pairs when an emoji straddles the boundary. I did not run the focused test locally because this review is read-only.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: extensions/telegram/src/bot-core.raw-update-log.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
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] Apply UTF-16-safe truncation to the final raw update preview cap in extensions/telegram/src/bot-core.ts and add coverage for an oversized serialized update.
  • [P1] Add redacted terminal/live output or a raw-update log transcript showing the formatter/log path after the fix; redact private IDs, tokens, phone numbers, and non-public endpoints.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body reports a focused Vitest run, but external PR proof still needs redacted real formatter/log output or a terminal transcript that shows the changed path after the fix; tests alone are supplemental. 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] The same raw-update logging middleware can still emit a dangling surrogate when the serialized redacted update exceeds the 8000-code-unit preview cap.
  • [P1] The external PR currently offers focused test validation but not real behavior proof such as redacted terminal output or a raw-update log transcript from the changed path.

Maintainer options:

  1. Decide the mitigation before merge
    Use truncateUtf16Safe for every truncation in Telegram raw update logging, including the final preview cap, keep the existing redaction and bounds unchanged, and attach redacted real formatter/log proof.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] No ClawSweeper repair lane: this external PR needs contributor-visible revision and real behavior proof before maintainer review can land it.

Security
Cleared: No security or supply-chain regression is visible; the diff only reuses an existing SDK text helper and adds focused Telegram formatter coverage.

Review findings

  • [P2] Protect the final raw update preview cap too — extensions/telegram/src/raw-update-log.ts:82
Review details

Best possible solution:

Use truncateUtf16Safe for every truncation in Telegram raw update logging, including the final preview cap, keep the existing redaction and bounds unchanged, and attach redacted real formatter/log proof.

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

Yes, source inspection gives a high-confidence reproduction path: current main uses raw UTF-16 slices at the per-field cap and at the final raw-update preview cap, both of which can split surrogate pairs when an emoji straddles the boundary. I did not run the focused test locally because this review is read-only.

Is this the best way to solve the issue?

No, not yet. Replacing the per-field slice is the right helper choice, but the best fix should also apply the same helper to the final raw-update preview cap in bot-core so the logging path has one invariant.

Full review comments:

  • [P2] Protect the final raw update preview cap too — extensions/telegram/src/raw-update-log.ts:82
    This changes the per-field 500-character truncation to use the UTF-16-safe helper, but the same raw-update logging middleware still does raw.slice(0, MAX_RAW_UPDATE_CHARS) before writing the log. A serialized redacted update over that cap can still be cut between surrogate halves, so this does not fully fix raw update log truncation; please route that final preview cap through the same helper and cover an oversized serialized update.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.87

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: This is a normal-priority Telegram logging correctness bug with limited blast radius and a focused repair path.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • 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 a focused Vitest run, but external PR proof still needs redacted real formatter/log output or a terminal transcript that shows the changed path after the fix; tests alone are supplemental. 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-priority Telegram logging correctness bug with limited blast radius and a focused repair path.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • 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 a focused Vitest run, but external PR proof still needs redacted real formatter/log output or a terminal transcript that shows the changed path after the fix; tests alone are supplemental. 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 +2, Tests +15. Total +17 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 3 1 +2
Tests 1 15 0 +15
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 18 1 +17

What I checked:

Likely related people:

  • galiniliev: Authored the merged Telegram raw update redaction formatter that owns this raw-update logging surface. (role: introduced behavior; confidence: high; commits: 74949eda2f72; files: extensions/telegram/src/raw-update-log.ts, extensions/telegram/src/bot-core.raw-update-log.test.ts)
  • joshavant: Merged the raw update redaction PR and contributed follow-up commits in that same formatter/test area. (role: reviewer/merger and adjacent contributor; confidence: high; commits: 74949eda2f72; files: extensions/telegram/src/raw-update-log.ts, extensions/telegram/src/bot-core.raw-update-log.test.ts)
  • steipete: Introduced the original UTF-16 truncation hardening and recently carried the normalization-core extraction and slice behavior fixes. (role: shared helper owner; confidence: high; commits: 63f5fa47deb6, 062f88e3e3af, b22c36f1125c; files: packages/normalization-core/src/utf16-slice.ts, src/plugin-sdk/text-utility-runtime.ts, src/utils.ts)
  • obviyus: Recent main history shows Telegram bot-core and UTF-16-safe Telegram draft preview work in nearby runtime surfaces. (role: recent Telegram area contributor; confidence: medium; commits: b07fd6f1b4c0, 547cc0f10903, 52d1e42eb14e; files: extensions/telegram/src/bot-core.ts, extensions/telegram/src/draft-stream.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.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. 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 7, 2026
@Alix-007
Alix-007 force-pushed the alix/utf16-telegram-raw-update-log branch from a8c39b4 to 7f5c6ca Compare July 7, 2026 04:19
@vincentkoc vincentkoc mentioned this pull request Jul 7, 2026
Merged
@Alix-007
Alix-007 force-pushed the alix/utf16-telegram-raw-update-log branch from 7f5c6ca to 79027a3 Compare July 7, 2026 04:23
@vincentkoc

vincentkoc commented Jul 7, 2026

Copy link
Copy Markdown
Member

Closing as superseded by the canonical UTF-16 boundary consolidation:

#101355
84e5327

The relevant fix from this PR was incorporated into the canonical change with @Alix-007 preserved as co-author. The landed patch consolidates the equivalent owner-local truncation boundaries, with exact changed gates, 142 focused tests, and clean exact-head CI.

@vincentkoc vincentkoc closed this Jul 7, 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

channel: telegram Channel integration: telegram P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. 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.

2 participants

@Alix-007 @vincentkoc