fix(codex): avoid broken emoji in dynamic tool timeout logs by zhangguiping-xydt · Pull Request #102509 · openclaw/openclaw

zhangguiping-xydt · 2026-07-09T07:31:33Z

What Problem This Solves

Fixes an issue where Codex dynamic tool timeout logs could contain malformed emoji or other split surrogate-pair text when a long dynamic tool log field crosses the truncation boundary.

Why This Change Was Made

Codex dynamic tool timeout diagnostics now use the existing UTF-16-safe truncation helper for normalized log fields while preserving the current 160-character cap and ... marker. This is limited to timeout diagnostic/log formatting and does not change tool execution, timeout behavior, protocol payloads, or persisted data.

User Impact

Operators and developers inspecting Codex dynamic tool timeout output can now expect valid UTF-16 snippets for long tool/process fields, including emoji-containing user or tool text, instead of broken surrogate characters at the truncation edge.

Evidence

Pre-submit freshness and competition check:

Refreshed origin/main before implementation; extensions/codex/src/app-server/dynamic-tool-execution.ts still used raw normalized.slice(0, LOG_FIELD_MAX_LENGTH - 3).
Enumerated current open PR file lists before implementation; no open PR touched extensions/codex/src/app-server/dynamic-tool-execution.ts.

Issue-specific runtime proof:

Directly executed the Codex app-server dynamic tool timeout handler with long emoji-containing process action/session fields and observed UTF-16-safe truncated fields.

{
  "success": false,
  "processAction": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...",
  "processSessionId": "ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss...",
  "containsSplitSurrogate": false
}

Regression test:

node scripts/run-vitest.mjs run --config test/vitest/vitest.extension-codex-app-server-tools.config.ts codex/src/app-server/dynamic-tool-execution.test.ts

Test Files  1 passed (1)
Tests       26 passed (26)

Additional checks:

pnpm exec oxfmt --check --threads=1 extensions/codex/src/app-server/dynamic-tool-execution.ts extensions/codex/src/app-server/dynamic-tool-execution.test.ts

All matched files use the correct format.

pnpm exec oxlint extensions/codex/src/app-server/dynamic-tool-execution.ts extensions/codex/src/app-server/dynamic-tool-execution.test.ts

Validation result: pass, exit_code=0

git diff --check

Validation result: pass, exit_code=0

clawsweeper · 2026-07-09T07:38:31Z

Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 3:49 AM ET / 07:49 UTC.

Summary
This PR changes Codex dynamic tool timeout log formatting to use truncateUtf16Safe for normalized tool/process fields and adds a regression test for emoji at the truncation boundary.

PR surface: Source +1, Tests +39. Total +40 across 2 files.

Reproducibility: yes. source-level reproduction is high confidence: current main uses raw UTF-16 slice at the timeout log truncation boundary, so an emoji beginning at that boundary can produce a lone surrogate. I did not execute the runtime path locally in this read-only review, but the PR body includes direct handler output showing containsSplitSurrogate: false after the change.

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:

Resolve or rerun the failing check-test-types job once logs are available.

Risk before merge

[P1] At review time, check-test-types was failed and some CI shards were still in progress; because logs were unavailable while the run was active, this needs ordinary CI follow-up before merge.

Maintainer options:

Decide the mitigation before merge
Land this narrow helper reuse once required CI is green or the failing type-check job is understood, keeping the fix limited to timeout diagnostics.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No ClawSweeper repair lane is needed because source review found no patch defect; the remaining action is ordinary CI completion or failing-check follow-up before maintainer merge.

Security
Cleared: The diff only reuses an existing text helper and adds a focused test, with no dependency, workflow, lockfile, secret, or supply-chain surface change.

Review details

Best possible solution:

Land this narrow helper reuse once required CI is green or the failing type-check job is understood, keeping the fix limited to timeout diagnostics.

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

Yes, source-level reproduction is high confidence: current main uses raw UTF-16 slice at the timeout log truncation boundary, so an emoji beginning at that boundary can produce a lone surrogate. I did not execute the runtime path locally in this read-only review, but the PR body includes direct handler output showing containsSplitSurrogate: false after the change.

Is this the best way to solve the issue?

Yes, this is the best code shape for the reported bug: normalize once at normalizeLogField and reuse the existing plugin SDK UTF-16-safe truncation helper. Duplicating surrogate checks locally would add a second text-boundary implementation without improving ownership.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P3: This is a small diagnostic-log formatting bug fix with low runtime blast radius and no user workflow blockage.
add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from directly executing the dynamic tool timeout handler with emoji-containing fields and observing containsSplitSurrogate: false, which is sufficient real behavior proof for this log-formatting 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 (terminal): The PR body includes after-fix terminal output from directly executing the dynamic tool timeout handler with emoji-containing fields and observing containsSplitSurrogate: false, which is sufficient real behavior proof for this log-formatting change.

Label justifications:

P3: This is a small diagnostic-log formatting bug fix with low runtime blast radius and no user workflow blockage.
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 (terminal): The PR body includes after-fix terminal output from directly executing the dynamic tool timeout handler with emoji-containing fields and observing containsSplitSurrogate: false, which is sufficient real behavior proof for this log-formatting change.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from directly executing the dynamic tool timeout handler with emoji-containing fields and observing containsSplitSurrogate: false, which is sufficient real behavior proof for this log-formatting change.

Evidence reviewed

PR surface:

Source +1, Tests +39. Total +40 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	2	1	+1
Tests	1	39	0	+39
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	41	1	+40

What I checked:

Repository policy read: Root and scoped extension policy were read fully enough to apply the Codex dependency/source gate and bundled-plugin boundary rules; the new import stays on the public plugin SDK runtime surface. (AGENTS.md:1, 51bb5f684778)
PR diff is narrow: The PR imports truncateUtf16Safe, replaces the raw slice truncation in normalizeLogField, and adds one focused fake-timer regression test for timeout log fields containing emoji. (extensions/codex/src/app-server/dynamic-tool-execution.ts:62, f01b40314dd7)
Current main has the reported edge: Current main truncates normalized timeout log fields with normalized.slice(0, LOG_FIELD_MAX_LENGTH - 3), which can leave a dangling surrogate when the boundary splits an emoji. (extensions/codex/src/app-server/dynamic-tool-execution.ts:63, 51bb5f684778)
Timeout diagnostic path checked: formatDynamicToolTimeoutDetails uses normalizeLogField for both generic tool names and process action/sessionId, and handleDynamicToolCallWithTimeout logs those fields only when the per-call watchdog fires. (extensions/codex/src/app-server/dynamic-tool-execution.ts:81, 51bb5f684778)
Caller paths checked: Both the main run-attempt path and side-question path resolve dynamic tool timeouts and call handleDynamicToolCallWithTimeout, so this local formatter is the shared timeout-output boundary. (extensions/codex/src/app-server/run-attempt.ts:2442, 51bb5f684778)
Helper contract checked: truncateUtf16Safe floors the limit, returns the input under the limit, and otherwise delegates to sliceUtf16Safe, which avoids returning dangling surrogate halves at either edge. (packages/normalization-core/src/utf16-slice.ts:43, 51bb5f684778)

Likely related people:

steipete: GitHub history shows steipete introduced the split Codex app-server dynamic-tool execution module and appears repeatedly in recent Codex app-server/runtime history. (role: introduced behavior and recent area contributor; confidence: high; commits: a4c2e7f5cf1b, 7139f473332f, f53346944db0; files: extensions/codex/src/app-server/dynamic-tool-execution.ts, extensions/codex/src/app-server/run-attempt.ts, extensions/codex/src/app-server/side-question.ts)
cxbAsDev: The merged timeoutSeconds fix recently changed the same dynamic-tool timeout resolver and tests, making this account a strong routing candidate for timeout-path review context. (role: recent adjacent contributor of dynamic-tool timeout behavior; confidence: medium; commits: 68487e4c1e34; files: extensions/codex/src/app-server/dynamic-tool-execution.ts, extensions/codex/src/app-server/dynamic-tool-execution.test.ts)
obviyus: Recent history on src/plugin-sdk/text-utility-runtime.ts involved the public plugin SDK utility barrel that this PR imports from, though the helper itself predates this PR. (role: adjacent SDK surface contributor; confidence: medium; commits: d498b1cce49c, 210877a73e25; files: src/plugin-sdk/text-utility-runtime.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-09T08:26:04Z

Land-ready maintainer verification at exact head dd6de265b6292010a7f2c96387481adfce457a37:

The timeout path now truncates dynamic-tool log fields through the public plugin-SDK UTF-16 helper while preserving the existing 160-code-unit limit and ellipsis.
Focused dynamic-tool execution suite: 26 passed.
Fresh autoreview: clean (0.99).
Exact-head hosted code CI/Testbox gates: passed. The unrelated label maintenance job does not exercise this code.
Direct Codex contract inspection: codex-rs/app-server-protocol/src/protocol/common.rs:1486-1490, codex-rs/app-server-protocol/src/protocol/v2/item.rs:1541-1569, codex-rs/app-server/src/bespoke_event_handling.rs:825-863, and codex-rs/app-server/src/dynamic_tools.rs:16-89 confirm JSON dynamic-tool arguments flow through item/tool/call without a narrower character contract.

The timer-driven regression proves process action/session metadata and returned timeout text contain no dangling surrogate.

steipete · 2026-07-09T08:26:27Z

Merged via squash.

Prepared head SHA: dd6de265b6292010a7f2c96387481adfce457a37
Landed commit: b23fdcfac953d80b2e7d29bcf311e7f64b45ab7b

…#102509) * fix(codex): keep dynamic tool log fields UTF-16 safe * test(codex): narrow dynamic tool response item --------- Co-authored-by: Peter Steinberger <[email protected]>

fix(codex): keep dynamic tool log fields UTF-16 safe

7ac059d

openclaw-barnacle Bot added extensions: codex size: XS labels Jul 9, 2026

Merge branch 'main' into fix/problem-codex-dynamic-tool-log-utf16

f01b403

test(codex): narrow dynamic tool response item

dd6de26

steipete merged commit b23fdcf into openclaw:main Jul 9, 2026
90 of 91 checks passed

lsr911 mentioned this pull request Jul 9, 2026

fix(codex): use truncateUtf16Safe for log field truncation #102553

Closed

This was referenced Jul 9, 2026

fix(browser): keep UTF-8 stderr tails valid #102543

Merged

fix(memory-host-sdk): use truncateUtf16Safe for qmd stderr truncation #102589

Closed

fix(codex-supervisor): use truncateUtf16Safe for stderr tail truncation #102590

Merged

wangyan2026 mentioned this pull request Jul 9, 2026

fix(crestodian): keep prompt history UTF-16 safe #102627

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(codex): avoid broken emoji in dynamic tool timeout logs#102509

fix(codex): avoid broken emoji in dynamic tool timeout logs#102509
steipete merged 3 commits into
openclaw:mainfrom
zhangguiping-xydt:fix/problem-codex-dynamic-tool-log-utf16

zhangguiping-xydt commented Jul 9, 2026

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

zhangguiping-xydt commented Jul 9, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

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

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading