Skip to content

fix(infra): keep restart log tails UTF-16 safe#103757

Merged
steipete merged 1 commit into
openclaw:mainfrom
zhangguiping-xydt:fix/problem-restart-log-tail-utf16
Jul 10, 2026
Merged

fix(infra): keep restart log tails UTF-16 safe#103757
steipete merged 1 commit into
openclaw:mainfrom
zhangguiping-xydt:fix/problem-restart-log-tail-utf16

Conversation

@zhangguiping-xydt

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where long update or restart log tails could contain a broken character when truncation landed between the two UTF-16 code units of an emoji or another supplementary-plane character.

The malformed tail was persisted in the restart sentinel and could surface as a replacement character when operators inspected update diagnostics.

Why This Change Was Made

This is a root-cause fix at the shared restart-log truncation boundary. It uses OpenClaw's existing surrogate-safe slicing helper while preserving the current tail budget, ellipsis, and start-index behavior.

The change does not alter restart sentinel storage, update/restart control flow, config, schema, or cleanup behavior. Open PR #101687 changes cleanup error logging in the same module but does not touch the log-tail truncation path.

User Impact

Operators can rely on truncated restart and update diagnostics to remain valid UTF-16, including when an emoji crosses the exact tail boundary. In that boundary case, OpenClaw drops the split surrogate pair instead of persisting one invalid half; all other log-tail behavior remains unchanged.

Evidence

The focused regression reproduced the current failure before the production change:

expected '…�tail' to be '…tail'
Expected: "…tail"
Received: "…�tail"

After the fix, the complete restart sentinel test file passed:

Test Files  1 passed (1)
Tests       28 passed (28)

Local E2E live proof used a real Node subprocess whose 8,007-code-unit output placed an emoji across the 8,000-unit update-log tail boundary. The production update step captured the output, the restart sentinel persisted it in SQLite, and a real OpenClaw gateway started with the same isolated state and consumed the sentinel:

prepared real update-step output: length=8000 loneSurrogate=false
[gateway] http server listening (9 plugins; 16.0s)
gateway ready and restart sentinel consumed

Observed before gateway startup: the persisted tail was exactly 8,000 code units, began with the truncation ellipsis, retained the final tail, and contained no lone surrogate. Observed after startup: the gateway had consumed and cleared the sentinel.

Additional validation:

  • node_modules/.bin/oxfmt --check src/infra/restart-sentinel.ts src/infra/restart-sentinel.test.ts
  • node scripts/run-oxlint.mjs src/infra/restart-sentinel.ts src/infra/restart-sentinel.test.ts
  • git diff --check

Not tested: a destructive package-manager self-update was not performed. The live proof exercised the real update subprocess boundary, SQLite restart sentinel persistence, and real gateway startup/consumption with isolated local state.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels Jul 10, 2026
@clawsweeper

clawsweeper Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 10, 2026, 5:27 PM ET / 21:27 UTC.

Summary
The PR makes restart/update log-tail truncation surrogate-safe by using the shared UTF-16 slicing helper and adds a split-surrogate boundary regression test.

PR surface: Source +1, Tests +4. Total +5 across 2 files.

Reproducibility: yes. On current main, trimLogTail("prefix🤖tail", 5) starts at the emoji's low surrogate and deterministically produces a malformed tail.

Review metrics: none identified.

Root-cause cluster
Relationship: canonical
Canonical: #103757
Summary: This PR is the sole identified implementation candidate for the restart/update log-tail UTF-16 boundary defect.

Members:

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

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Next step before merge

  • No automated repair is needed; the remaining action is ordinary exact-head maintainer review and landing.

Security
Cleared: The source-and-test-only patch reuses an existing workspace export and introduces no dependency, workflow, permission, secret, or downloaded-code change.

Review details

Best possible solution:

Land the existing-helper reuse at the single shared log-tail boundary with the focused split-surrogate regression test, preserving current storage, budgets, and update/restart control flow.

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

Yes. On current main, trimLogTail("prefix🤖tail", 5) starts at the emoji's low surrogate and deterministically produces a malformed tail.

Is this the best way to solve the issue?

Yes. Reusing the existing tested UTF-16 helper at the canonical shared truncation boundary is narrower and less drift-prone than repairing each update caller separately.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: The PR fixes malformed operator diagnostics in a narrow Unicode boundary case without affecting runtime availability, config, or stored schema.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR provides after-fix live output from a real update subprocess through SQLite sentinel persistence and real gateway startup and consumption, including an explicit lone-surrogate check.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR provides after-fix live output from a real update subprocess through SQLite sentinel persistence and real gateway startup and consumption, including an explicit lone-surrogate check.
Evidence reviewed

PR surface:

Source +1, Tests +4. Total +5 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 2 1 +1
Tests 1 4 0 +4
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 6 1 +5

What I checked:

  • Current-main defect: Current main trims persisted restart/update diagnostics with a raw UTF-16 code-unit slice, which can begin on the low half of a surrogate pair. (src/infra/restart-sentinel.ts:375, d92d5774f5c6)
  • Patch behavior: The proposed head replaces only that raw slice with sliceUtf16Safe while preserving the existing start index, ellipsis, and untrimmed fast path. (src/infra/restart-sentinel.ts:373, 18c567b3eb50)
  • Helper contract: The shared helper advances a start index past a low surrogate when it would otherwise split a valid pair, then delegates to normal string slicing. (packages/normalization-core/src/utf16-slice.ts:16, d92d5774f5c6)
  • Regression coverage: The new test asserts that a five-code-unit tail boundary crossing the robot emoji produces …tail rather than a dangling surrogate. (src/infra/restart-sentinel.test.ts:344, 18c567b3eb50)
  • Shared owner boundary: Update-runner subprocess results and package-update advisory output both flow through trimLogTail, so the patch fixes the common persistence boundary rather than individual callers. (src/infra/update-runner.ts:22, d92d5774f5c6)
  • Exact-head validation: The contributor supplied real subprocess-to-SQLite-to-gateway proof, and GitHub reports the current head checks passing or intentionally skipped with no observed failure. (18c567b3eb50)

Likely related people:

  • steipete: Recent GitHub path history shows repeated maintenance of src/infra/update-runner.ts and introduction or maintenance of the normalization-core package used by this fix. (role: recent adjacent owner; confidence: high; commits: c7909ead5cd6, 00d8d7ead059, b22c36f1125c; files: src/infra/update-runner.ts, packages/normalization-core/src/utf16-slice.ts)
  • vincentkoc: Commit 514b336 moved restart sentinels to SQLite and is central to the persisted diagnostic path exercised by the PR. (role: restart-sentinel storage refactor author; confidence: high; commits: 514b3365b54c, a4e2113e1b0f; files: src/infra/restart-sentinel.ts)
  • dwc1997: Commit f92ec2d added focused unit coverage for the shared surrogate-safe slicing contract reused here. (role: UTF-16 helper test contributor; confidence: medium; commits: f92ec2d4e88b; files: packages/normalization-core/src/utf16-slice.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.
Review history (2 earlier review cycles)
  • reviewed 2026-07-10T15:30:27.740Z sha c599c80 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-10T21:18:13.826Z sha 18c567b :: needs maintainer review before merge. :: none

@steipete
steipete force-pushed the fix/problem-restart-log-tail-utf16 branch from c599c80 to 18c567b Compare July 10, 2026 21:10
@steipete

Copy link
Copy Markdown
Contributor

Land-ready review complete at exact head 18c567b3eb50fc43516ee1e3a26d68ee28f30ac2.

  • Best-fix review: trimLogTail is the shared restart/update log-tail boundary, and reusing sliceUtf16Safe there fixes all callers without duplicate policy. No findings or nits remain.
  • Hosted exact-head CI: all relevant checks passed; no failures or pending jobs.
  • Isolated fork-safe proof: AWS Crabbox cbx_f81035be7222, run run_d303c41fc926, public network with no Tailscale or hydrated credentials. pnpm test src/infra/restart-sentinel.test.ts packages/normalization-core/src/utf16-slice.test.ts passed 2 files / 46 tests.
  • Fresh exact-head autoreview: clean, confidence 0.99.
  • Native prepare-run: passed with hosted exact-head gates; prepared SHA equals the PR head.

No known proof gaps.

@steipete
steipete merged commit 6f6c802 into openclaw:main Jul 10, 2026
97 checks passed
@steipete

Copy link
Copy Markdown
Contributor

Merged via squash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants