fix(codex): app inventory error diagnostics stay UTF-16 safe by zhangguiping-xydt · Pull Request #102414 · openclaw/openclaw

zhangguiping-xydt · 2026-07-09T04:25:34Z

What Problem This Solves

Codex app inventory refresh diagnostics could serialize a long top-level error message with a dangling UTF-16 surrogate when an app/list failure crossed the 500-code-unit limit. Nested error-data strings already used safe truncation, but the top-level message did not.

Why This Change Was Made

Route both top-level messages and nested error-data strings through one truncateSerializedErrorText owner.
Reuse the existing truncateUtf16Safe primitive and preserve the existing 500-unit limit, ellipsis, HTML omission, secret redaction, cache behavior, and protocol.
Add an exact regression where an emoji straddles the top-level message boundary.

The upstream Codex contract was checked directly: app/list is registered as an unserialized request in codex-rs/app-server-protocol/src/protocol/common.rs:732-735; its response is AppsListResponse in codex-rs/app-server-protocol/src/protocol/v2/apps.rs:159-168; and concurrent connector failures become JSON-RPC internal-error strings in codex-rs/app-server/src/request_processors/apps_processor.rs:164-279. OpenClaw therefore owns sanitizing those error strings before retaining and logging diagnostics.

User Impact

Codex app inventory refresh diagnostics remain valid UTF-16 when long upstream errors contain emoji or other astral characters at the truncation boundary. Normal errors and app inventory behavior are unchanged.

Evidence

Exact head: 87ea557c48f0611b7047f85b1359cbd5802be513

Blacksmith Testbox run 28998304437: app-inventory-cache.test.ts, 13 passed.
Targeted oxfmt and git diff --check: passed.
Fresh full-diff autoreview: clean, confidence 0.98.
Direct Codex source inspection also covered the app-server test request helper at codex-rs/app-server/tests/common/test_app_server.rs:815-819.

The new regression serializes ${"x".repeat(499)}🚀tail and asserts ${"x".repeat(499)}..., proving the truncator backs up before the split surrogate instead of retaining a lone high surrogate.

Real behavior proof

Field	Value
Behavior addressed	UTF-16-unsafe top-level `app/list` error diagnostic truncation
Environment	Blacksmith Testbox, Linux x64, exact PR head
Production path	Codex app inventory refresh catch path and exported error serializer
Result	13 focused tests passed; top-level and nested error strings share one safe policy
Not tested	Live Codex connector failure against a credentialed app directory; the upstream protocol/source contract and OpenClaw production serializer are exercised directly

Risk checklist

No config, environment, default, SDK, protocol, auth, or provider change.
No new dependency.
Existing redaction and size limits remain unchanged.
No changelog entry required for internal diagnostic serialization.
Allow edits by maintainers.

clawsweeper · 2026-07-09T04:26:57Z

Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 2:24 AM ET / 06:24 UTC.

Summary
The branch routes Codex app inventory top-level and nested serialized error text through a shared UTF-16-safe truncation helper and adds a regression for an emoji at the 500-code-unit boundary.

PR surface: Source +1, Tests +6. Total +7 across 2 files.

Reproducibility: yes. from source. Current main and the latest release use raw .slice at the 500-code-unit top-level diagnostic boundary, and the PR regression covers the non-BMP split case.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: extensions/codex/src/app-server/app-inventory-cache.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #102414
Summary: This PR is the narrow canonical fix for the remaining top-level Codex app inventory diagnostic UTF-16 truncation gap; earlier UTF-16 work fixed adjacent bounded-text paths but current main still has this sanitizer slice.

Members:

partial_overlap: fix: prevent garbled emoji at remaining text limits #101685 - Merged broad UTF-16 bounded-text work updated the same app inventory cache file for adjacent nested error-data handling but current main still retains the top-level raw slice.
superseded: fix(codex): keep app-inventory cache/runtime label truncation UTF-16 safe #101634 - Closed earlier Codex UTF-16 truncation PR was superseded by the merged consolidation, but that consolidation did not resolve this exact top-level message path.
adjacent_distinct: fix(codex): cache plugin inventory to prevent repeated disk I/O (#99071) #99177 - Open same-area Codex inventory PR addresses plugin inventory caching and disk I/O, not error diagnostic UTF-16 truncation.
adjacent_distinct: fix(codex): transfer typed generated images #102054 - Open Codex generated-image transfer PR touches app inventory behavior in a different way and does not modify this serializer truncation path.

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit ✨ media proof bonus
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.

Rank-up moves:

none.

Risk before merge

[P1] The branch is behind current main and some routine CI jobs were still pending when checked, so landing should rely on a refreshed merge ref or exact-head merge-queue checks.

Maintainer options:

Decide the mitigation before merge
Land the narrow helper reuse after normal exact-head or merge-queue checks pass, preserving the existing 500-unit limit, redaction behavior, cache semantics, and Codex app/list protocol.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P2] No repair lane is needed; the PR already contains the focused code change and regression coverage, leaving ordinary maintainer review plus exact-head CI or merge-queue handling.

Security
Cleared: The diff only reuses an existing local text helper and adds a focused regression test; it does not change dependencies, workflows, lockfiles, secrets, permissions, or executable supply-chain surfaces.

Review details

Best possible solution:

Land the narrow helper reuse after normal exact-head or merge-queue checks pass, preserving the existing 500-unit limit, redaction behavior, cache semantics, and Codex app/list protocol.

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

Yes, from source. Current main and the latest release use raw .slice at the 500-code-unit top-level diagnostic boundary, and the PR regression covers the non-BMP split case.

Is this the best way to solve the issue?

Yes. Reusing the existing UTF-16-safe helper in the local serializer is narrower than changing Codex protocol handling or adding another one-off truncation path.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This fixes a concrete Codex diagnostic text-corruption bug with limited blast radius and no config, protocol, auth, or migration surface.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The PR body links a Blacksmith Testbox run for the focused production-serializer regression and the repository proof check passed; for this internal diagnostic serializer, linked terminal/action proof is adequate.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body links a Blacksmith Testbox run for the focused production-serializer regression and the repository proof check passed; for this internal diagnostic serializer, linked terminal/action proof is adequate.

Evidence reviewed

PR surface:

Source +1, Tests +6. Total +7 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	8	7	+1
Tests	1	6	0	+6
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	14	7	+7

What I checked:

Repository policy read: The root and scoped extension policies were read and applied; the Codex dependency-inspection and bundled-plugin boundary rules are relevant because this PR touches the Codex plugin app-server path. (AGENTS.md:1, 583ca34bf88d)
Current main still has the bug: Current main sanitizes top-level app inventory error messages with redacted.slice(0, MAX_SERIALIZED_ERROR_MESSAGE_LENGTH), which can split a surrogate pair at the 500-code-unit boundary. (extensions/codex/src/app-server/app-inventory-cache.ts:391, 583ca34bf88d)
Latest release has the same top-level slice: The latest release tag still contains the raw top-level diagnostic slice, so the issue is not already shipped fixed. (extensions/codex/src/app-server/app-inventory-cache.ts:375, e085fa1a3ffd)
PR head centralizes safe truncation: At the PR head, both nested error data and sanitized top-level messages use truncateSerializedErrorText, which delegates to truncateUtf16Safe at the existing 500-unit limit. (extensions/codex/src/app-server/app-inventory-cache.ts:341, 87ea557c48f0)
Regression covers the boundary: The added regression constructs 499 ASCII code units followed by a rocket emoji and asserts the serialized message backs up to the safe prefix plus ellipsis. (extensions/codex/src/app-server/app-inventory-cache.test.ts:220, 87ea557c48f0)
Helper contract supports the fix: truncateUtf16Safe clamps the limit and calls sliceUtf16Safe, whose end-boundary logic backs off when the slice would end on a high surrogate followed by a low surrogate. (packages/normalization-core/src/utf16-slice.ts:33, 583ca34bf88d)

Likely related people:

kevinslin: GitHub path history shows multiple merged Codex native plugin/app inventory cache changes in this file before the UTF-16 follow-up work. (role: feature owner; confidence: high; commits: a1ac559ed7e6, f169e0aafde8, 389c355bcf3f; files: extensions/codex/src/app-server/app-inventory-cache.ts)
steipete: Authored and merged the broad UTF-16 bounded-text consolidation that touched this same Codex app inventory file, and authored the PR head refactor commit that unifies the serializer truncation path. (role: recent adjacent owner; confidence: high; commits: 176fee5d071d, 87ea557c48f0; files: extensions/codex/src/app-server/app-inventory-cache.ts, extensions/codex/src/app-server/run-attempt.ts)
lsr911: Authored the earlier superseded Codex UTF-16 truncation PR and was credited in the merged broad UTF-16 consolidation that partially overlapped this remaining gap. (role: adjacent contributor; confidence: medium; commits: adadfa04599f, 176fee5d071d; files: extensions/codex/src/app-server/app-inventory-cache.ts, extensions/codex/src/app-server/run-attempt.ts)
vincentkoc: The checked-out grafted main history attributes the current serializer and shared UTF-16 helper lines to a Vincent Koc release-refresh commit, so this is a weak routing hint for the current checkout state rather than sole ownership. (role: current-main blame signal; confidence: low; commits: 686b6b51daa0; files: extensions/codex/src/app-server/app-inventory-cache.ts, packages/normalization-core/src/utf16-slice.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-09T04:30:19.955Z sha 5bee7e6 :: needs maintainer review before merge. :: none
reviewed 2026-07-09T04:37:21.016Z sha 5bee7e6 :: needs maintainer review before merge. :: none

steipete · 2026-07-09T06:31:03Z

Land-ready verification at exact head 87ea557c48f0611b7047f85b1359cbd5802be513:

Consolidated top-level and nested Codex app inventory error strings behind one 500-unit UTF-16-safe serializer policy.
Direct upstream Codex contract audit: app/list registration (codex-rs/app-server-protocol/src/protocol/common.rs:732-735), response type (codex-rs/app-server-protocol/src/protocol/v2/apps.rs:159-168), concurrent failure-to-JSON-RPC paths (codex-rs/app-server/src/request_processors/apps_processor.rs:164-279), and request helper (codex-rs/app-server/tests/common/test_app_server.rs:815-819).
Blacksmith Testbox 28998304437: 13/13 focused cache/serializer tests passed.
Exact-head CI 28998384101: passed.
Targeted formatting and diff checks passed; fresh full-diff autoreview clean at 0.98 confidence.
Native OPENCLAW_TESTBOX=1 scripts/pr prepare-run 102414: passed with validated review artifacts.

No changelog entry is required because this only corrects internal diagnostic serialization.

steipete · 2026-07-09T06:31:23Z

Merged via squash.

Prepared head SHA: 87ea557c48f0611b7047f85b1359cbd5802be513
Landed commit: 4adfc30b90e8e70830a0024d3c9e0c218db36b97

…w#102414) * fix(codex): keep app inventory error messages UTF-16 safe * refactor(codex): unify app inventory error truncation --------- Co-authored-by: Peter Steinberger <[email protected]>

fix(codex): keep app inventory error messages UTF-16 safe

5bee7e6

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

refactor(codex): unify app inventory error truncation

87ea557

steipete merged commit 4adfc30 into openclaw:main Jul 9, 2026
103 checks passed

wangyan2026 mentioned this pull request Jul 9, 2026

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

Merged

This was referenced Jul 9, 2026

fix(text): keep five src-side text truncations UTF-16 safe #102630

Closed

fix(chat,cli): keep session snippet and CLI output digest truncation UTF-16 safe #102747

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(codex): app inventory error diagnostics stay UTF-16 safe#102414

fix(codex): app inventory error diagnostics stay UTF-16 safe#102414
steipete merged 2 commits into
openclaw:mainfrom
zhangguiping-xydt:fix/problem-codex-inventory-serialized-error-slice

zhangguiping-xydt commented Jul 9, 2026 •

edited by steipete

Loading

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 • edited by steipete 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

Real behavior proof

Risk checklist

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

zhangguiping-xydt commented Jul 9, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading