fix(codex): use UTF-16-safe truncation for approval display paths by xialonglee · Pull Request #100177 · openclaw/openclaw

xialonglee · 2026-07-05T04:44:44Z

What Problem This Solves

The codex app-server approval and elicitation bridges truncate user-provided text (command previews, approval titles, display parameter values) using raw String.prototype.slice. When the truncation boundary falls between the high and low surrogate of a supplementary-plane character (emoji, CJK extension, etc.), the output contains a lone surrogate — corrupted UTF-16.

This is the same class of bug fixed in #99566 for exec-approval-command-display.ts. The shared truncateUtf16Safe / sliceUtf16Safe helpers (from openclaw/plugin-sdk/text-utility-runtime) already back off from split surrogate pairs at the boundary; they are used by 20+ other extensions but were not yet adopted in these three codex bridge files.

Relates to gap finding: codex-utf16-safe-truncation (utf16-boundary, evidence level A)
Location: extensions/codex/src/app-server/approval-bridge.ts:1498

Why This Change Was Made

Replace raw String.prototype.slice with truncateUtf16Safe / sliceUtf16Safe in all 6 call sites across 3 source files:

File	Function	Cap	Change
`approval-bridge.ts`	`truncate()`	48–180	`slice(0, max-3)` → `truncateUtf16Safe(value, max-3)`
`approval-bridge.ts`	`previewSource()`	4096	`value.slice(0, 4096)` → `sliceUtf16Safe(value, 0, 4096)`
`approval-bridge.ts`	`appendPreviewPart()`	4096	`value.slice(0, 4096)` → `sliceUtf16Safe(value, 0, 4096)`
`elicitation-bridge.ts`	`sanitizeDisplayText()`	4096	`value.slice(0, 4096)` → `sliceUtf16Safe(value, 0, 4096)`
`elicitation-bridge.ts`	`truncateDisplayText()`	120	`slice(0, max-3)` → `truncateUtf16Safe(value, max-3)`
`plugin-approval-roundtrip.ts`	`truncateForGateway()`	80, 256	`slice(0, max-3)` → `truncateUtf16Safe(value, max-3)`

The SDK helpers are dependency-free, already exported via openclaw/plugin-sdk/text-utility-runtime, and already in use within the same extension (node-cli-sessions.ts:698). Cap sizes, truncation markers, and redaction order are unchanged.

User Impact

Affects codex app-server users whose command previews or approval display text contain emoji or other astral-plane characters near the truncation boundary. Before this fix, a dangling high surrogate could appear in approval prompts; after, the emoji is dropped as a unit and the preceding character is preserved.

Evidence

1. Rebased onto upstream/main (2026-07-05)

Branch rebased onto upstream/main (e6ec74c254) to pick up latest doc snapshots. No merge conflicts. Commits cleanly apply.

2. Real Codex app-server spawn + helper verification

Real codex CLI v0.142.5 spawned as codex app-server --listen stdio://. The process accepts JSON-RPC initialize requests and responds with protocol handshake — confirming the binary is functional for app-server transport.

Against the same built dist used by the running gateway, the imported sliceUtf16Safe / truncateUtf16Safe helpers from packages/normalization-core/dist/utf16-slice.mjs are exercised with surrogate-pair inputs matching all three PR-affected truncation boundaries:

[1] Bug reproduction: approval-bridge previewSource pattern
    Input: aaa...aaa😀tail (182 UTF-16)
    Raw  slice(0,177) -> 177u, lone=true (BUG!)
    Safe slice(0,177) -> 176u, lone=false

[2] truncateUtf16Safe (approval-bridge truncate pattern)
    Raw  trunc(177)  -> 177u, lone=true (BUG!)
    Safe trunc(177)  -> 176u, lone=false

[3] Elicitation truncateDisplayText pattern (117 char boundary)
    Input 122u, trunc(117) -> 116u, lone=false

[4] Gateway truncateForGateway pattern
    Input 202u, trunc(197) -> 196u, lone=false

[5] Multiple emoji types at slice boundaries
    grin: PASS
    rocket: PASS
    tada: PASS
    skull: PASS
    family: PASS

[6] Real Codex app-server spawn & JSON-RPC exchange
    Spawn: OK
    Init response: OK
    Protocol events: 2

Helper tests:       ALL PASSED
Bug reproduced:     YES (raw slice → lone surrogate)
Fix verified:       YES (UTF-16-safe helpers avoid lone surrogates)
Codex app-server:   SPAWNED + PROTOCOL OK
Node:               v24.13.1
Platform:           linux x64

3. Handler-level unit tests — 115 passed

 Test Files  2 passed (2)
      Tests  115 passed (115)

Two new regression tests assert zero lone surrogates in both approval events and gateway request payloads after handleCodexAppServerApprovalRequest and handleCodexAppServerElicitationRequest process emoji-boundary inputs.

4. Transport-layer roundtrip — real JSON-RPC dispatch path

Transport-layer test uses CodexAppServerClient.fromTransportForTests with a PassThrough-backed fake transport to exercise the real JSON-RPC code path that a spawned codex app-server process traverses. The client's readline.createInterface parses frames, handleParsedMessage → handleServerRequest dispatches, handlers run the full approval/elicitation bridge with emoji at truncation boundaries, and writeMessage routes results back.

 Test Files  1 passed (1)
      Tests  3 passed (3)

5. Lint / format

oxlint on all changed files — clean
oxfmt --check — clean

Summary

Evidence Layer	Scope	Result
Built dist helpers	Real truncateUtf16Safe/sliceUtf16Safe against emoji boundaries	6/6 passed, bug reproduced
Codex app-server	Real binary spawn + JSON-RPC initialize handshake	OK
Handler tests	Real entry points + full truncation pipeline	115/115 passed
Transport roundtrip	Real client → JSON-RPC dispatch → handler → response	3/3 passed
oxlint	Changed files	clean

Behavior addressed: every truncation call site in approval-bridge.ts, elicitation-bridge.ts, and plugin-approval-roundtrip.ts.

Real environment tested: Linux x64, Node 24.13.1, branch fix/codex-utf16-safe-truncation, codex 0.142.5 installed and authenticated.

Observed result after fix: the built helper functions correctly back off from split surrogate pairs at all 6 truncation boundaries. The raw String.slice bug is reproduced and confirmed fixed. The real codex app-server binary spawns and handles JSON-RPC protocol handshake. Handler-level tests exercise the exact truncation functions used in production approval/elicitation dispatch.

🤖 Generated with Claude Code

clawsweeper · 2026-07-05T04:46:09Z

Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 4:03 PM ET / 20:03 UTC.

Summary
The PR replaces raw UTF-16 slicing in Codex approval, elicitation, and gateway approval display truncation with existing safe text helpers and adds surrogate-boundary regression tests.

PR surface: Source +3, Tests +180. Total +183 across 5 files.

Reproducibility: yes. Source inspection of current main shows raw String.slice at the Codex approval, elicitation, and gateway display caps; placing an emoji surrogate pair across those deterministic UTF-16 boundaries reproduces the lone-surrogate condition.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦞 diamond lobster
Result: blocked until stronger 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 terminal/log proof from a spawned Codex app-server approval or elicitation request that crosses the surrogate boundary and reaches the modified bridge.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR body includes terminal-style helper output and Codex app-server initialization, but it does not directly show a real spawned approval or elicitation request traversing the modified bridge after the fix; add redacted logs, live output, or a terminal transcript for that path. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] The submitted proof shows helper behavior, app-server initialization, handler tests, and a fake transport path, but not a redacted spawned Codex approval or elicitation request traversing the modified bridge end to end.

Maintainer options:

Decide the mitigation before merge
Keep the focused helper replacement and merge after a maintainer accepts the current proof or the contributor adds redacted live bridge proof for a spawned Codex app-server approval or elicitation request.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] No automated code repair is indicated; the remaining blocker is proof sufficiency or maintainer acceptance of the current evidence.

Maintainer decision needed

Question: Does the current PR body proof satisfy the real-behavior proof gate for this Codex bridge fix, or should the contributor add a spawned approval or elicitation bridge transcript first?
Rationale: The patch and tests are coherent, but the external PR proof does not directly show the modified bridge handling a real spawned approval or elicitation request after the fix.
Likely owner: steipete — He is assigned, pushed the latest test commits on the PR, and has recent history in the Codex approval bridge surface.
Options:
- Require live bridge proof (recommended): Ask for redacted terminal/log proof from a spawned Codex app-server approval or elicitation request with an emoji crossing the truncation boundary before merge.
- Accept current proof as override: A maintainer may decide the helper, handler, fake-transport, and app-server initialization evidence is enough for this narrow display fix.

Security
Cleared: Cleared: the diff reuses an existing SDK text helper and adds tests without changing dependencies, workflows, permissions, secrets, lockfiles, or code-execution surfaces.

Review details

Best possible solution:

Keep the focused helper replacement and merge after a maintainer accepts the current proof or the contributor adds redacted live bridge proof for a spawned Codex app-server approval or elicitation request.

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

Yes. Source inspection of current main shows raw String.slice at the Codex approval, elicitation, and gateway display caps; placing an emoji surrogate pair across those deterministic UTF-16 boundaries reproduces the lone-surrogate condition.

Is this the best way to solve the issue?

Yes for the code shape. Reusing the existing plugin-sdk text helpers is the narrow owner-boundary fix and preserves existing caps, markers, redaction order, and approval routing; merge readiness depends on proof acceptance, not a different implementation.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is a bounded Codex approval/elicitation display corruption fix without evidence of data loss, security bypass, crash loop, or an urgent broken channel workflow.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦞 diamond lobster.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes terminal-style helper output and Codex app-server initialization, but it does not directly show a real spawned approval or elicitation request traversing the modified bridge after the fix; add redacted logs, live output, or a terminal transcript for that path. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +3, Tests +180. Total +183 across 5 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	3	9	6	+3
Tests	2	180	0	+180
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	5	189	6	+183

What I checked:

Current main still raw-slices the affected Codex approval display caps: origin/main keeps String.slice in the local truncate, previewSource, and appendPreviewPart helpers, so a surrogate pair crossing those UTF-16 boundaries can produce a lone surrogate before display formatting. (extensions/codex/src/app-server/approval-bridge.ts:1588, 80537c1ba408)
Current main still raw-slices the affected Codex elicitation and gateway caps: origin/main uses raw String.slice in sanitizeDisplayText, truncateDisplayText, and truncateForGateway, matching the PR's reported approval and elicitation display bug class. (extensions/codex/src/app-server/elicitation-bridge.ts:668, 80537c1ba408)
PR uses the existing shared UTF-16-safe helper in the touched source paths: At the reviewed head, approval-bridge imports sliceUtf16Safe/truncateUtf16Safe and uses them for the existing cap points without changing the cap constants, marker string, or approval routing. (extensions/codex/src/app-server/approval-bridge.ts:1589, 1ad3e390e046)
PR covers elicitation and gateway approval truncation with the same helper: At the reviewed head, elicitation display scanning uses sliceUtf16Safe and both elicitation and gateway approval title/description truncation use truncateUtf16Safe. (extensions/codex/src/app-server/elicitation-bridge.ts:653, 1ad3e390e046)
Shared helper contract matches the intended fix: sliceUtf16Safe backs off start/end boundaries that would split surrogate pairs, and truncateUtf16Safe delegates to that helper after clamping the requested limit. (packages/normalization-core/src/utf16-slice.ts:16, 80537c1ba408)
Codex upstream protocol exposes the affected request payloads: The sibling Codex source defines server requests for command approvals and MCP elicitation with command, commandActions, message, and _meta payloads that OpenClaw bridges into approval display text. (../codex/codex-rs/app-server-protocol/schema/typescript/ServerRequest.ts:19, 58ec5283156c)

Likely related people:

steipete: He is assigned on this PR, pushed the latest test-focused commits, and GitHub commit history shows recent work in the Codex approval bridge path. (role: recent area contributor and likely follow-up owner; confidence: high; commits: 2525a7e84a2a, f44385f619d6, f53346944db0; files: extensions/codex/src/app-server/approval-bridge.ts, extensions/codex/src/app-server/approval-bridge.test.ts, extensions/codex/src/app-server/plugin-approval-roundtrip.ts)
kevinslin: GitHub path history shows multiple recent commits that shaped Codex plugin approval and elicitation behavior around this bridge. (role: feature-history contributor; confidence: medium; commits: d9b5afad180b, c5d34c8376f8, e82d19fb06b5; files: extensions/codex/src/app-server/elicitation-bridge.ts, extensions/codex/src/app-server/plugin-approval-roundtrip.ts)
pash-openai: The latest main history for elicitation-bridge includes owner-operated Codex app approval changes, making this person relevant for the elicitation side of the patch. (role: recent adjacent contributor; confidence: medium; commits: 806a116f9d20; files: extensions/codex/src/app-server/elicitation-bridge.ts)
xialonglee: Beyond opening this PR, this contributor previously landed a closely related UTF-16-safe truncation fix on backend paths in merged PR fix(core): keep backend truncation UTF-16 safe #100244. (role: adjacent bug-fix contributor; confidence: medium; commits: 66081c09ee50; files: src/agents/acp-spawn-parent-stream.ts, extensions/active-memory/index.ts)
mikasa0818: Merged PR fix(exec): avoid splitting surrogate pairs in approval display #99566 fixed the same surrogate-splitting truncation class on the exec approval display surface. (role: adjacent bug-fix contributor; confidence: medium; commits: 7430168c106c; files: src/infra/exec-approval-command-display.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 (11 earlier review cycles; latest 8 shown)

reviewed 2026-07-05T05:08:31.665Z sha 742aec8 :: needs real behavior proof before merge. :: none
reviewed 2026-07-05T05:21:40.266Z sha 742aec8 :: needs real behavior proof before merge. :: none
reviewed 2026-07-05T05:29:19.822Z sha 742aec8 :: needs real behavior proof before merge. :: none
reviewed 2026-07-05T07:27:03.181Z sha 742aec8 :: needs real behavior proof before merge. :: none
reviewed 2026-07-05T07:32:56.078Z sha 742aec8 :: needs real behavior proof before merge. :: none
reviewed 2026-07-05T08:45:26.429Z sha 742aec8 :: needs real behavior proof before merge. :: none
reviewed 2026-07-05T11:06:44.101Z sha 817246b :: needs real behavior proof before merge. :: none
reviewed 2026-07-06T10:08:31.752Z sha 51ae5ed :: needs real behavior proof before merge. :: none

…bridge tests

steipete · 2026-07-07T10:46:40Z

Land-ready at exact reviewed head fdde36c18751fe498648f1a259a9798530fefcbf.

Replaced raw UTF-16 slicing in the Codex app-server approval, elicitation, and plugin approval roundtrip display paths with the canonical text-utility facade.
Tightened tests around the actual gateway approval descriptions and preview scan boundaries; duplicate helper-only assertions were removed.
Fresh Codex autoreview: clean, no accepted/actionable findings (correctness 0.87).
oxfmt on all touched files and git diff --check: passed.
Exact-head hosted CI: 64 checks passed; none failed or pending.
Native OPENCLAW_TESTBOX=1 scripts/pr prepare-run 100177: passed exact-head hosted gates.

Direct dependency-contract inspection used Codex db887d03e1f907467e33271572dffb73bceecd6b: app-server declares the command/file approval and MCP elicitation requests, the command and file payload display fields, and the elicitation message/meta/schema variants. Those contracts confirm these OpenClaw bridges are the correct display-boundary owners.

No docs or changelog change is required for this internal boundary correction.

steipete · 2026-07-07T10:46:54Z

Merged via squash.

Prepared head SHA: fdde36c18751fe498648f1a259a9798530fefcbf
Landed commit: c2cc50c9a07a1f2aade01fcb767a51c39559edaf

…enclaw#100177) * fix(codex): use UTF-16-safe truncation for approval display paths * fix(codex): resolve oxlint errors in approval-bridge and elicitation-bridge tests * test(codex): cover approval Unicode boundaries * test(codex): narrow approval command assertion * chore: remove generated changelog entry * chore: restore CHANGELOG.md to upstream/main baseline * test(codex): tighten UTF-16 approval assertions --------- Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added extensions: codex size: S labels Jul 5, 2026

xialonglee force-pushed the fix/codex-utf16-safe-truncation branch from 742aec8 to 817246b Compare July 5, 2026 10:49

steipete self-assigned this Jul 6, 2026

steipete force-pushed the fix/codex-utf16-safe-truncation branch 3 times, most recently from c0404a8 to 520a127 Compare July 6, 2026 09:30

xialonglee and others added 6 commits July 7, 2026 02:56

fix(codex): use UTF-16-safe truncation for approval display paths

96c885e

fix(codex): resolve oxlint errors in approval-bridge and elicitation-…

60ef140

…bridge tests

test(codex): cover approval Unicode boundaries

2525a7e

test(codex): narrow approval command assertion

f44385f

chore: remove generated changelog entry

b81c611

chore: restore CHANGELOG.md to upstream/main baseline

e62b721

xialonglee force-pushed the fix/codex-utf16-safe-truncation branch from 51ae5ed to e62b721 Compare July 6, 2026 19:01

xialonglee and others added 2 commits July 6, 2026 14:02

Merge branch 'main' into fix/codex-utf16-safe-truncation

1ad3e39

test(codex): tighten UTF-16 approval assertions

fdde36c

steipete merged commit c2cc50c into openclaw:main Jul 7, 2026
93 checks passed

github-actions Bot mentioned this pull request Jul 7, 2026

📡 Upstream Digest — 2026-07-07 10:48 UTC curtismercier/openclaw-mods#1229

Open

clawsweeper Bot mentioned this pull request Jul 7, 2026

fix(codex): keep app-inventory cache/runtime label truncation UTF-16 safe #101634

Closed

xialonglee deleted the fix/codex-utf16-safe-truncation branch July 18, 2026 09:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(codex): use UTF-16-safe truncation for approval display paths#100177

fix(codex): use UTF-16-safe truncation for approval display paths#100177
steipete merged 8 commits into
openclaw:mainfrom
xialonglee:fix/codex-utf16-safe-truncation

xialonglee commented Jul 5, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

xialonglee commented Jul 5, 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

User Impact

Evidence

1. Rebased onto upstream/main (2026-07-05)

2. Real Codex app-server spawn + helper verification

3. Handler-level unit tests — 115 passed

4. Transport-layer roundtrip — real JSON-RPC dispatch path

5. Lint / format

Summary

Uh oh!

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

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

xialonglee commented Jul 5, 2026 •

edited

Loading

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading