Skip to content

fix(agents): keep prompt template descriptions UTF-16 safe#101660

Closed
ly85206559 wants to merge 1 commit into
openclaw:mainfrom
ly85206559:codex/prompt-template-description-utf16
Closed

fix(agents): keep prompt template descriptions UTF-16 safe#101660
ly85206559 wants to merge 1 commit into
openclaw:mainfrom
ly85206559:codex/prompt-template-description-utf16

Conversation

@ly85206559

@ly85206559 ly85206559 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes prompt template fallback descriptions being truncated with raw UTF-16 slicing. When a markdown template has no frontmatter description, OpenClaw derives the description from the first non-empty body line; the previous .slice(0, 60) path could cut an emoji or other surrogate pair in half.

Why This Change Was Made

Recent prompt, title, preview, and channel display truncation fixes use the shared truncateUtf16Safe helper for user-visible bounded text. This applies that existing helper to prompt template fallback descriptions while preserving the existing 60-character limit and ... marker behavior.

User Impact

Prompt template lists keep well-formed descriptions when template text contains emoji or non-BMP characters near the truncation boundary.

Evidence

  • node scripts/run-vitest.mjs src/agents/sessions/prompt-templates.test.ts
    • src/agents/sessions/prompt-templates.test.ts passed: 1 test passed.
  • git diff --check
  • python .agents\skills\autoreview\scripts\autoreview --mode local --no-web-search
    • autoreview clean: no accepted/actionable findings reported
  • Base: latest origin/main at 60f0749b7f

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S 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 7, 2026, 9:39 AM ET / 13:39 UTC.

Summary
The branch imports truncateUtf16Safe for prompt-template fallback descriptions and adds a focused temp-file Vitest case for an emoji at the 60-code-unit boundary.

PR surface: Source +1, Tests +56. Total +57 across 2 files.

Reproducibility: yes. from source inspection. Current main and the latest release both raw-slice the first non-empty prompt-template body line at 60 UTF-16 code units, and the added test gives a focused reproduction path, though I did not run tests in this read-only review.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/agents/sessions/prompt-templates.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦪 silver shellfish
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:

  • [P2] Add redacted real behavior proof for the prompt-template fallback description path, such as terminal output from loading or listing a template whose first body line has an emoji at the truncation boundary.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Proof is still limited to unit/static evidence; before merge, add redacted terminal output, logs, or a screenshot showing a real prompt-template fallback description load/list path, then update the PR body for re-review.

Risk before merge

  • [P2] The only unresolved merge blocker is proof quality: the contributor has not shown the prompt-template fallback description path in a real run, and the overlapping consolidation is not yet a safe replacement because it has its own blocking finding.

Maintainer options:

  1. Decide the mitigation before merge
    Land this focused helper swap with regression coverage after redacted real prompt-template loading or listing proof is added, or let a proven broader consolidation merge first and then close this PR as superseded.
  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 job is needed because there are no code findings; the remaining action is contributor-supplied real behavior proof or a maintainer-chosen proven consolidation path.

Security
Cleared: The diff only imports an existing internal normalization helper and adds a temp-file unit test; it does not touch dependencies, workflows, lockfiles, credentials, permissions, or code-execution surfaces.

Review details

Best possible solution:

Land this focused helper swap with regression coverage after redacted real prompt-template loading or listing proof is added, or let a proven broader consolidation merge first and then close this PR as superseded.

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

Yes, from source inspection. Current main and the latest release both raw-slice the first non-empty prompt-template body line at 60 UTF-16 code units, and the added test gives a focused reproduction path, though I did not run tests in this read-only review.

Is this the best way to solve the issue?

Yes for the prompt-template surface. Reusing the existing truncateUtf16Safe helper at the single raw-slice site is the narrowest maintainable fix; the broader consolidation is not a safe substitute yet because it has an unresolved review finding.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P3: This is a small, low-risk polish bug in prompt-template description rendering, not an urgent runtime, data, security, or channel-delivery failure.
  • 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: Proof is still limited to unit/static evidence; before merge, add redacted terminal output, logs, or a screenshot showing a real prompt-template fallback description load/list path, then update the PR body for re-review.
Evidence reviewed

PR surface:

Source +1, Tests +56. Total +57 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 2 1 +1
Tests 1 56 0 +56
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 58 1 +57

What I checked:

Likely related people:

  • zhangguiping-xydt: Live PR metadata for the merge that last rewrote this prompt-template loader identifies this account as the author, and blame points the current raw slice line to that merge commit. (role: introduced current behavior; confidence: high; commits: f14eb97ae7c5; files: src/agents/sessions/prompt-templates.ts, src/agents/sessions/resource-loader.ts, packages/normalization-core/src/utf16-slice.ts)
  • vincentkoc: The same merge commit lists this account as a co-author, and the PR metadata shows participation in the merged prompt-template/resource-loader change that owns the current line history. (role: co-author and adjacent owner; confidence: medium; commits: f14eb97ae7c5; files: src/agents/sessions/prompt-templates.ts, src/agents/sessions/resource-loader.ts, packages/normalization-core/src/utf16-slice.ts)
  • steipete: Recent merged work on the same UTF-16-safe truncation pattern and the overlapping open consolidation make this account relevant for routing the broader cleanup cluster, even though the current prompt-template line history is elsewhere. (role: recent bounded-text contributor; confidence: medium; commits: a9582a1bb62a; files: src/agents/command/attempt-execution.ts, src/agents/openai-transport-stream.ts, src/commands/onboard-skills.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.
Review history (1 earlier review cycle)
  • reviewed 2026-07-07T13:02:54.579Z sha 844d818 :: needs real behavior proof before merge. :: none

@clawsweeper 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. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jul 7, 2026
@steipete steipete mentioned this pull request Jul 7, 2026
Merged
@ly85206559
ly85206559 marked this pull request as ready for review July 7, 2026 13:21
@steipete

steipete commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Closed as superseded by #101685, landed in 176fee5d071dd03f6cbbc54b029f23380b04e911. The replacement carries this useful UTF-16 boundary fix on current main, uses the existing shared helper, expands caller-level coverage, and preserves your commit credit in the consolidated commit. Thank you, @ly85206559, for finding and fixing this.

@steipete steipete 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

agents Agent runtime and tooling P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: S 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

@ly85206559 @steipete