fix(native-hook-relay): use truncateUtf16Safe for hook display text truncation by lsr911 · Pull Request #102467 · openclaw/openclaw

lsr911 · 2026-07-09T06:33:04Z

What Problem This Solves

The native hook relay uses naive .slice(0, N) truncation in two places:

truncateText() helper (4 call sites: tool names, commands, descriptions)
Inline hook result display (...[truncated] suffix)

Both can split surrogate pairs mid-character in hook display output.

Why This Change Was Made

Replace .slice(0, N) with truncateUtf16Safe() which preserves surrogate pair boundaries.

Evidence

=== BEFORE: naive slice(0, 23) on emoji text ===
Result: "tool-audit-field-with-\ud83d"
Last char is high surrogate: true  ← DANGLING!

=== AFTER: truncateUtf16Safe respects pair boundaries ===
Result: "tool-audit-field-with-"
Clean (no broken pair): true  ← CORRECT

Files Changed

File	Change
`src/agents/harness/native-hook-relay.ts`	+1 import; `.slice(0,N)` → `truncateUtf16Safe()` in `truncateText()` + inline truncation

🤖 Generated with Claude Code

Co-Authored-By: Claude [email protected]

Replace naive .slice(0, MAX) with truncateUtf16Safe() to prevent surrogate pair splitting in tool policy audit log output. Co-Authored-By: Claude <[email protected]>

…runcation Replace naive .slice(0, N) truncation with truncateUtf16Safe() in: - truncateText() helper (covers 4 call sites for tool display text) - Inline hook result truncation (...[truncated] suffix) Prevents surrogate pair splitting in native hook relay display output (tool names, commands, descriptions shown to users). Co-Authored-By: Claude <[email protected]>

clawsweeper · 2026-07-09T06:44:25Z

Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 2:48 AM ET / 06:48 UTC.

Summary
The PR replaces direct string slicing in native hook relay display truncation with the repository's existing UTF-16-safe truncation helper.

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

Reproducibility: yes. from source inspection: current main slices user-visible hook text by UTF-16 code units, and the shared helper tests show a one-code-unit cut through an emoji surrogate pair is unsafe without boundary adjustment. I did not run a live native-hook scenario in this read-only review.

Review metrics: none identified.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof 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 after-fix proof, such as terminal output or logs showing native hook display text with emoji truncating cleanly.
[P1] Add focused regression coverage for surrogate-pair truncation in the native hook relay snapshot or approval-preview path if feasible.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body explains the bug but does not include after-fix real behavior proof; the contributor should add redacted terminal output, copied live output, or logs, then update the PR body to trigger re-review or ask a maintainer for @clawsweeper re-review.

Risk before merge

[P1] Contributor real-behavior proof is missing, so merge should wait for redacted terminal output, logs, or a validation transcript showing the after-fix native hook display truncation behavior.

Maintainer options:

Decide the mitigation before merge
Keep the narrow helper substitution, add real behavior proof, and preferably add focused coverage for surrogate-pair truncation in the native hook relay display paths before merge.
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 maintainer path is to wait for contributor real-behavior proof and then review the narrow patch on the exact current head.

Security
Cleared: The diff only swaps display-text slicing for an existing local text helper and does not touch execution, secrets, dependencies, CI, or package resolution.

Review details

Best possible solution:

Keep the narrow helper substitution, add real behavior proof, and preferably add focused coverage for surrogate-pair truncation in the native hook relay display paths before merge.

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

Yes, from source inspection: current main slices user-visible hook text by UTF-16 code units, and the shared helper tests show a one-code-unit cut through an emoji surrogate pair is unsafe without boundary adjustment. I did not run a live native-hook scenario in this read-only review.

Is this the best way to solve the issue?

Yes for the code shape: reusing the existing shared UTF-16-safe helper at the two native-hook display truncation sites is the narrowest maintainable fix. Local surrogate logic or a Codex hook protocol change would be broader than needed for display-only truncation.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P3: The PR is a very small display-text correctness fix with limited blast radius and no evidence of a broken core workflow.
add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab 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 explains the bug but does not include after-fix real behavior proof; the contributor should add redacted terminal output, copied live output, or logs, then update the PR body to trigger re-review or ask a maintainer for @clawsweeper re-review.

Label justifications:

P3: The PR is a very small display-text correctness fix with limited blast radius and no evidence of a broken core workflow.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab 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 explains the bug but does not include after-fix real behavior proof; the contributor should add redacted terminal output, copied live output, or logs, then update the PR body to trigger re-review or ask a maintainer for @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +1. Total +1 across 1 file.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	3	2	+1
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	3	2	+1

Acceptance criteria:

[P1] Contributor proof: redacted terminal output, copied live output, logs, or a reproducible validation transcript showing native hook display text with surrogate-pair content truncates without a dangling replacement character.
[P1] Suggested focused validation after proof: pnpm test src/agents/harness/native-hook-relay.test.ts packages/normalization-core/src/utf16-slice.test.ts in the repo-approved remote test environment.

What I checked:

Root policy applied: Root AGENTS.md and the scoped agents guide were read fully; their review rules required checking the native hook relay path, adjacent tests, history, and Codex hook contracts. (AGENTS.md:1, 5f91f6c95840)
Current main behavior: Current main records retained native hook payload history through snapshotNativeHookRelayPayload, whose snapshotString currently returns value.slice(0, limit) before adding the truncated marker. (src/agents/harness/native-hook-relay.ts:1932, 5f91f6c95840)
Current approval display behavior: Current main formats native permission approval titles/descriptions through truncateText, which currently slices by code units before appending ellipsis. (src/agents/harness/native-hook-relay.ts:2226, 5f91f6c95840)
Helper contract: truncateUtf16Safe clamps/floors the requested length and delegates to sliceUtf16Safe, which avoids returning a dangling surrogate half at the boundary. (packages/normalization-core/src/utf16-slice.ts:44, 5f91f6c95840)
Helper coverage: Existing tests cover truncating at a surrogate-pair boundary and expect the safe helper not to return the dangling half. (packages/normalization-core/src/utf16-slice.test.ts:84, 5f91f6c95840)
PR diff: The final PR diff imports truncateUtf16Safe into native-hook-relay.ts and applies it at the two display truncation sites; the earlier tool-policy-audit.ts commit is reverted in the final branch state. (src/agents/harness/native-hook-relay.ts:1930, cb083e53c9d5)

Likely related people:

pashpashpash: Authored the merged PR that bridged Codex native hooks into OpenClaw and subsequent merged native-hook relay updates touching the same file. (role: feature owner; confidence: high; commits: cec11954e474, 7f5bb0d978b4, 9fd9c1b9a459; files: src/agents/harness/native-hook-relay.ts, src/agents/harness/native-hook-relay.test.ts, src/gateway/server-methods/native-hook-relay.ts)
vincentkoc: Current-main blame and GitHub commit metadata tie the present native hook relay file and both current slicing sites to commit aad99747cd45b3845e027c2297b85835b0bef75f. (role: recent area contributor; confidence: medium; commits: aad99747cd45; files: src/agents/harness/native-hook-relay.ts, packages/normalization-core/src/utf16-slice.ts, src/utils.ts)
steipete: Merged the original Codex native hook relay PR and a later MCP hook support PR that both touched this surface. (role: merger; confidence: medium; commits: d49b63a95dcf, c7b440201fda; files: src/agents/harness/native-hook-relay.ts, src/agents/harness/native-hook-relay.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-09T07:03:07Z

Land-ready maintainer pass complete at exact head 7e4ace03e4b8884a38ccec7372915948b765ab9e.

I kept the useful two-boundary fix, switched it to the direct normalization-core import, corrected snapshot accounting to charge the actual retained prefix, and added exact production-path tests for retained hook payloads and permission approval previews.

Proof:

Focused fallback: node scripts/run-vitest.mjs src/agents/harness/native-hook-relay.test.ts — 82 tests passed.
Exact-head CI run https://github.com/openclaw/openclaw/actions/runs/28999878793 — success.
Targeted oxfmt and git diff --check — passed.
Fresh full-branch autoreview — clean, no accepted/actionable findings (0.98).
Native review artifacts validated; OPENCLAW_TESTBOX=1 scripts/pr prepare-run 102467 passed at the same SHA.

The focused test ran locally because fork-contributor code is not eligible for credential-hydrated Testbox execution before landing review. Exact-head fork CI supplies the broad hosted proof. No remaining proof gap.

steipete · 2026-07-09T07:03:23Z

Merged via squash.

Prepared head SHA: 7e4ace03e4b8884a38ccec7372915948b765ab9e
Landed commit: 4fa3118049c1592c3a057b25ac4857fa08cfdc38

…runcation (openclaw#102467) * fix(tool-policy-audit): use truncateUtf16Safe for audit field truncation Replace naive .slice(0, MAX) with truncateUtf16Safe() to prevent surrogate pair splitting in tool policy audit log output. Co-Authored-By: Claude <[email protected]> * fix(native-hook-relay): use truncateUtf16Safe for hook display text truncation Replace naive .slice(0, N) truncation with truncateUtf16Safe() in: - truncateText() helper (covers 4 call sites for tool display text) - Inline hook result truncation (...[truncated] suffix) Prevents surrogate pair splitting in native hook relay display output (tool names, commands, descriptions shown to users). Co-Authored-By: Claude <[email protected]> * test(agents): cover hook relay UTF-16 boundaries --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>

lsr911 and others added 2 commits July 9, 2026 14:30

fix(tool-policy-audit): use truncateUtf16Safe for audit field truncation

7b33e2e

Replace naive .slice(0, MAX) with truncateUtf16Safe() to prevent surrogate pair splitting in tool policy audit log output. Co-Authored-By: Claude <[email protected]>

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

test(agents): cover hook relay UTF-16 boundaries

7e4ace0

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

steipete merged commit 4fa3118 into openclaw:main Jul 9, 2026
115 of 118 checks passed

steipete mentioned this pull request Jul 9, 2026

fix(agents): keep truncateText and truncateTextToBudget UTF-16 safe #102482

Closed

lsr911 mentioned this pull request Jul 9, 2026

fix(codex): use truncateUtf16Safe for tool transcript output truncation #102522

Merged

clawsweeper Bot mentioned this pull request Jul 9, 2026

fix(active-memory): use truncateUtf16Safe for log value truncation #102551

Merged

This was referenced Jul 9, 2026

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

Closed

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

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(native-hook-relay): use truncateUtf16Safe for hook display text truncation#102467

fix(native-hook-relay): use truncateUtf16Safe for hook display text truncation#102467
steipete merged 3 commits into
openclaw:mainfrom
lsr911:fix/utf16-hook-relay

lsr911 commented Jul 9, 2026 •

edited

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

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

Uh oh!

What Problem This Solves

Why This Change Was Made

Evidence

Files Changed

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

Loading

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading