Skip to content

fix(parallel-mcp): use truncateUtf16Safe for error message truncation#102615

Closed
lzyyzznl wants to merge 1 commit into
openclaw:mainfrom
lzyyzznl:fix/truncateUtf16Safe-parallel-mcp
Closed

fix(parallel-mcp): use truncateUtf16Safe for error message truncation#102615
lzyyzznl wants to merge 1 commit into
openclaw:mainfrom
lzyyzznl:fix/truncateUtf16Safe-parallel-mcp

Conversation

@lzyyzznl

@lzyyzznl lzyyzznl commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

extensions/parallel/src/parallel-mcp-search.runtime.ts uses raw .slice(0, N) for string truncation at three JSON-RPC error payload truncation sites JSON.stringify(...).slice(0, 500). When the text contains multi-byte Unicode characters such as emoji, a code-unit slice can split a UTF-16 surrogate pair, producing invalid Unicode.

Why This Change Was Made

Replacing these .slice(0, N) calls with truncateUtf16Safe ensures the truncated output is always valid Unicode while keeping identical behavior for ASCII-only content.

User Impact

Truncated text in the affected code path remains valid Unicode at the existing size limits. No behavioral, API, or performance changes for ASCII-only input.

Evidence

  • Mechanical one-to-one replacement: .slice(0, N) -> truncateUtf16Safe(str, N)
  • truncateUtf16Safe is already used extensively in the codebase following the same pattern
  • No runtime behavior change for the common case (valid Unicode or ASCII input)

Generated with Claude Code

@lzyyzznl
fix(parallel-mcp): use truncateUtf16Safe for error message truncation
8cb5d47 
Three .slice(0, 500) truncation sites in the Parallel MCP search
extension may cut UTF-16 surrogate pairs in half when the error
payload contains multi-byte characters such as emoji. Replace all
with truncateUtf16Safe to keep the truncated output valid Unicode.
@openclaw-barnacle openclaw-barnacle Bot added extensions: parallel size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 9, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 9, 2026
Merged
@clawsweeper

clawsweeper Bot commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 6:23 AM ET / 10:23 UTC.

Summary
The branch imports truncateUtf16Safe into extensions/parallel/src/parallel-mcp-search.runtime.ts and uses it for three JSON-stringified MCP error message truncations.

PR surface: Source +1. Total +1 across 1 file.

Reproducibility: yes. source-level reproduction is high confidence: a JSON-RPC error or result whose JSON string places an emoji surrogate pair across code-unit 500 can be cut by the current raw slice. I did not execute a live Parallel MCP error-path repro in this read-only review.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: extensions/parallel/src/parallel-mcp-search.runtime.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until 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:

Proof guidance:

  • [P1] Needs real behavior proof before merge: Missing: the PR body has rationale but no redacted terminal output, live output, logs, or recording showing the after-fix Parallel MCP error path; update the PR body with proof after redacting private details to trigger review, or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] No real after-fix Parallel MCP error-path proof is present, so the contributor proof gate remains unsatisfied.
  • [P1] fix(parallel): use truncateUtf16Safe for MCP error JSON truncation #102592 proposes the same runtime fix with focused tests, so maintainers should avoid landing both duplicate branches.
  • [P1] If maintainers choose this branch instead of the related tested branch, equivalent surrogate-boundary regression coverage should be added before merge.

Maintainer options:

  1. Decide the mitigation before merge
    Land one duplicate branch with equivalent regression tests and redacted real behavior proof for the Parallel MCP error truncation path, then close or supersede the other branch.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Human/contributor action remains: add real behavior proof and choose one duplicate branch; no safe ClawSweeper code repair is indicated because source review found no patch defect.

Maintainer decision needed

  • Question: Which duplicate Parallel MCP UTF-16 truncation branch should remain the landing candidate once real behavior proof is supplied?
  • Rationale: Both open PRs address the same three error-message sites; the related branch has focused tests, but neither branch currently has sufficient real error-path proof, so automation should not close or merge either one yet.
  • Likely owner: steipete — Recent adjacent UTF-16 cleanup and merge history make this the strongest available routing candidate for duplicate selection and proof expectations.
  • Options:
    • Prefer the tested related branch (recommended): Use fix(parallel): use truncateUtf16Safe for MCP error JSON truncation #102592 as the likely landing candidate after it receives sufficient real behavior proof, because it includes the same runtime fix plus regression tests.
    • Keep this minimal branch: Land this PR only if it adds equivalent regression coverage and sufficient redacted real behavior proof first.
    • Pause both duplicates: Leave both PRs open until a maintainer or contributor provides direct proof for the changed Parallel MCP error path.

Security
Cleared: The diff only reuses an existing text utility in a plugin runtime and does not touch dependencies, workflows, lockfiles, secrets, auth, permissions, or network policy.

Review details

Best possible solution:

Land one duplicate branch with equivalent regression tests and redacted real behavior proof for the Parallel MCP error truncation path, then close or supersede the other branch.

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

Yes, source-level reproduction is high confidence: a JSON-RPC error or result whose JSON string places an emoji surrogate pair across code-unit 500 can be cut by the current raw slice. I did not execute a live Parallel MCP error-path repro in this read-only review.

Is this the best way to solve the issue?

Yes for the code shape: reusing the existing public plugin-SDK UTF-16 helper at the three local error boundaries is the narrow maintainable fix. This branch is less complete than the related duplicate until it adds proof and equivalent regression coverage.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P3: This is a small provider-plugin error-message formatting fix with low runtime blast radius and no blocked user workflow.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Missing: the PR body has rationale but no redacted terminal output, live output, logs, or recording showing the after-fix Parallel MCP error path; update the PR body with proof after redacting private details to trigger review, or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +1. Total +1 across 1 file.

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

What I checked:

Likely related people:

  • NormallyGaussian: Authored merged PR feat(parallel): add free Parallel Search MCP as the zero-config default web_search provider #90849, which added the free Parallel Search MCP provider, runtime, tests, and normalization files involved in this fix. (role: feature owner; confidence: high; commits: 983b65b0e011; files: extensions/parallel/src/parallel-mcp-search.runtime.ts, extensions/parallel/src/parallel-mcp-search.runtime.test.ts, extensions/parallel/src/parallel-search-normalize.ts)
  • steipete: Authored and merged recent UTF-16 bounded-text cleanup in fix: preserve emoji at remaining bounded-text edges #101711, including adjacent Parallel normalization use of truncateUtf16Safe. (role: recent adjacent contributor and merger; confidence: medium; commits: 87fe266c5e4d; files: extensions/parallel/src/parallel-search-normalize.ts)
  • obviyus: The current checkout's blame for the raw-slice lines points to commit 4cbe49ba881ac59f132b146ef153fadb8a521a4c, whose associated merged PR was authored by this account, but the PR was broad and not clearly the feature origin. (role: recent current-main line carrier; confidence: low; commits: 4cbe49ba881a; files: extensions/parallel/src/parallel-mcp-search.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.
Review history (1 earlier review cycle)
  • reviewed 2026-07-09T09:42:42.462Z sha 8cb5d47 :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jul 9, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 9, 2026
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Thanks for the fix. This is an exact duplicate of #102592: all three extractMcpToolPayload error serializations were improved, boundary-tested, live-tested against Parallel MCP, and landed in c87b9a7cee909a7ac276b26e0f74f0fde79ce9a9.

I’m closing this duplicate so there is one canonical implementation and proof trail. Please reopen only if there is a distinct Parallel MCP truncation path not covered by #102592.

@steipete steipete closed this Jul 9, 2026
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

extensions: parallel P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lzyyzznl @steipete