Skip to content

fix(agents): keep cleanup timeout details UTF-16 safe#102565

Merged
steipete merged 1 commit into
openclaw:mainfrom
zhangguiping-xydt:fix/problem-cleanup-timeout-utf16-safe
Jul 9, 2026
Merged

fix(agents): keep cleanup timeout details UTF-16 safe#102565
steipete merged 1 commit into
openclaw:mainfrom
zhangguiping-xydt:fix/problem-cleanup-timeout-utf16-safe

Conversation

@zhangguiping-xydt

@zhangguiping-xydt zhangguiping-xydt commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where agent cleanup timeout warnings could emit malformed diagnostic text when cleanup timeout details were truncated at a UTF-16 surrogate boundary.

Why This Change Was Made

Cleanup timeout diagnostics now use the shared UTF-16-safe truncation helper before appending the truncation suffix. This keeps the existing 512-character diagnostic bound and does not change cleanup timeout scheduling, timeout values, or late-rejection handling.

User Impact

Operators and developers can rely on cleanup timeout warning details to remain readable even when oversized diagnostics contain emoji or other surrogate-pair characters at the truncation boundary.

Evidence

  • Validation final-cleanup-timeout-utf16-unit (pass, exit_code=0):

     RUN  v4.1.9 [local path redacted]

 Test Files  1 passed (1)
      Tests  13 passed (13)
   Duration  1.15s (transform 581ms, setup 571ms, import 57ms, tests 175ms, environment 0ms)

  • Validation final-cleanup-timeout-utf16-runtime (pass, exit_code=0):

    agent cleanup timed out: runId=runtime-run sessionId=runtime-session step=agent-trajectory-flush timeoutMs=1 details=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...[truncated]

  • Validation final-diff-check (pass, exit_code=0):

    git diff --check origin/main...HEAD

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S labels Jul 9, 2026
@clawsweeper

clawsweeper Bot commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 5:05 AM ET / 09:05 UTC.

Summary
The PR changes agent cleanup-timeout diagnostic truncation to use the shared UTF-16-safe helper and adds regression coverage for surrogate-boundary detail strings.

PR surface: Source +1, Tests +25. Total +26 across 2 files.

Reproducibility: yes. source inspection gives a high-confidence reproduction path by returning timeout details where a surrogate pair starts at prefixLength - 1, causing current main's raw slice to leave a dangling high surrogate before the suffix. I did not execute the repro locally because this review was read-only.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • [P2] Refresh the branch against current main and let exact-head CI rerun before merge.

Risk before merge

  • [P1] The PR is marked behind current main, so the merge should use a refreshed branch or exact-head CI after GitHub computes the final merge result.

Maintainer options:

  1. Decide the mitigation before merge
    Land the narrow helper-based fix after refreshing exact-head checks, keeping cleanup timing and the exported agent cleanup API unchanged.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] No repair job is needed because the patch is narrow and no review findings were found; maintainer action is normal branch refresh and merge review.

Security
Cleared: The diff only uses an existing internal helper and adds tests; it changes no dependencies, workflows, lockfiles, secrets, permissions, or execution sources.

Review details

Best possible solution:

Land the narrow helper-based fix after refreshing exact-head checks, keeping cleanup timing and the exported agent cleanup API unchanged.

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

Yes: source inspection gives a high-confidence reproduction path by returning timeout details where a surrogate pair starts at prefixLength - 1, causing current main's raw slice to leave a dangling high surrogate before the suffix. I did not execute the repro locally because this review was read-only.

Is this the best way to solve the issue?

Yes. Applying the existing shared UTF-16-safe helper at the single cleanup diagnostic truncation boundary is narrower and more maintainable than adding a local surrogate check or broader logging refactor.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P3: This is a narrow cleanup-diagnostic readability bug with limited blast radius and no change to runtime scheduling or user data.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied live runtime warning output plus unit validation that directly exercises the terminal/log diagnostic change.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied live runtime warning output plus unit validation that directly exercises the terminal/log diagnostic change.

Label justifications:

  • P3: This is a narrow cleanup-diagnostic readability bug with limited blast radius and no change to runtime scheduling or user data.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied live runtime warning output plus unit validation that directly exercises the terminal/log diagnostic change.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied live runtime warning output plus unit validation that directly exercises the terminal/log diagnostic change.
Evidence reviewed

PR surface:

Source +1, Tests +25. Total +26 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 2 1 +1
Tests 1 47 22 +25
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 49 23 +26

What I checked:

Likely related people:

  • steipete: Git blame and git show tie the current cleanup timeout truncation path, its tests, and the shared UTF-16 helper to the same recent commit authored by Peter Steinberger. (role: introduced behavior and adjacent owner; confidence: high; commits: 769878303744; files: src/agents/run-cleanup-timeout.ts, src/agents/run-cleanup-timeout.test.ts, src/agents/embedded-agent-runner/run/attempt-trajectory-flush-cleanup.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 proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jul 9, 2026
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Land-ready review completed at exact head 1ce00492887a46b8e90557aa86f181d92d52a538.

The cleanup-timeout owner now uses the shared UTF-16-safe prefix primitive while preserving its existing 512-code-unit bound and suffix. The regression exercises the exported timeout path at a split-surrogate boundary and verifies the emitted warning exactly.

Evidence:

  • node scripts/run-vitest.mjs src/agents/run-cleanup-timeout.test.ts — 1 file passed, 13 tests passed.
  • git diff --check — passed.
  • fresh autoreview — clean, 0.99 confidence.
  • native scripts/pr review-validate-artifacts 102565 — passed.
  • native OPENCLAW_TESTBOX=1 scripts/pr prepare-run 102565 — exact-head hosted CI/Testbox gates passed.

Known proof gaps: none for this internal diagnostic boundary.

@steipete
steipete merged commit 4ad94fe into openclaw:main Jul 9, 2026
143 of 152 checks passed
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

@wangmiao0668000666 wangmiao0668000666 mentioned this pull request Jul 9, 2026
Closed
9 tasks
@github-actions github-actions Bot mentioned this pull request Jul 9, 2026
Open
@wangmiao0668000666 wangmiao0668000666 mentioned this pull request Jul 9, 2026
Closed
9 tasks
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 10, 2026
@zhangguiping-xydt
fix(agents): keep cleanup timeout details UTF-16 safe (openclaw#102565)
d7108d1
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. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zhangguiping-xydt @steipete