Skip to content

fix: render text formatting on the RPC send bridge path#143

Open
omarshahine wants to merge 2 commits into
openclaw:mainfrom
omarshahine:fix/rpc-send-text-formatting
Open

fix: render text formatting on the RPC send bridge path#143
omarshahine wants to merge 2 commits into
openclaw:mainfrom
omarshahine:fix/rpc-send-text-formatting

Conversation

@omarshahine

Copy link
Copy Markdown
Contributor

Problem

handleSend (RPC method send) forwards text and file to the IMCore bridge but never the attributed-text format ranges. So direct/handle sends come through plain even on macOS 15+ — only send-rich (CLI) and the RPC sendRich handler honor formatting.

This bites the OpenClaw gateway: its message tool routes plain handle targets (to: +1…) through the send method and puts format ranges in a formatting param. imsg's send ignores it, so **bold**/*italic* notifications silently lose styling. Replies render correctly only because the gateway routes those through sendRich.

Fix

Thread the format ranges through handleSendsendViaBridge → the bridge sendMessage action, mirroring what sendRich already does. Accept formatting (the key OpenClaw's message tool emits) alongside text_formatting/textFormatting for parity with handleSendRich.

  • Bridge transport only; AppleScript sends stay plain (attributed text needs the private API).
  • No behavior change when no formatting is supplied.

Tests

  • rpcSendThreadsTextFormattingToBridge — asserts ranges from the formatting key reach invokeBridge as textFormatting.
  • rpcSendWithoutFormattingOmitsTextFormatting — asserts no textFormatting key when none is supplied.

All RPCServerBridge tests pass (7/7, no regressions).

End-to-end proof (real device, macOS 15+)

On the unfixed 0.11.1 binary, RPC send + a formatting range → plain text. With this change (release build), the same call — method send, bare formatting key, transport: bridge — renders bold on the recipient. send-rich already worked; this brings the plain send path to parity.

handleSend forwarded text and file to the IMCore bridge but never the
attributed-text format ranges, so direct/handle sends came through plain
even on macOS 15+ — only `send-rich` (and the RPC `sendRich` handler)
honored formatting. The OpenClaw gateway's `message` tool uses the plain
`send` method for handle targets, so its bold/italic notifications
silently lost styling.

Thread the format ranges through handleSend -> sendViaBridge -> the
bridge sendMessage action, accepting `formatting` (the key OpenClaw
emits) alongside `text_formatting`/`textFormatting` for parity with
`sendRich`. Bridge transport only; AppleScript sends stay plain.

Adds tests covering the threaded ranges and the no-formatting case.
@clawsweeper

clawsweeper Bot commented Jun 21, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed June 21, 2026, 5:33 PM ET / 21:33 UTC.

Summary
The branch forwards RPC send formatting params through handleSend and sendViaBridge to the bridge sendMessage action, adds two RPC bridge regression tests, and adds a 0.11.2 changelog entry.

Reproducibility: yes. from source inspection: current main's handleSend never reads formatting params and sendViaBridge sends only chatGuid/message, while the bridge already consumes textFormatting. I did not run a live Messages-device reproduction.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
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:

  • [P1] Add redacted screenshot/video, terminal output, copied live output, or logs showing RPC send with formatting over the bridge renders formatted text; update the PR body so ClawSweeper can re-review, or ask a maintainer to comment @clawsweeper re-review.
  • Remove the CHANGELOG.md entry unless a maintainer explicitly wants release notes carried in this branch.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body describes real-device verification, but it does not include inspectable redacted proof such as a screenshot, recording, terminal output, copied live output, or logs. 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.

Mantis proof suggestion
A real Messages proof would materially help confirm the bridge renders formatting in the recipient UI. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify RPC send with formatting key over bridge renders bold text in Messages on macOS 15+.

Risk before merge

  • [P1] The real-device claim is not inspectable, so maintainers cannot independently verify the macOS 15+ bridge formatting path before merge.
  • [P1] The branch adds a release-owned CHANGELOG.md entry; maintainers need it removed or explicitly accepted as release-owned context.

Maintainer options:

  1. Decide the mitigation before merge
    Merge the RPC bridge forwarding and regression tests after inspectable real behavior proof is attached and release notes stay in the maintainer-owned process.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Keep this PR in maintainer review; the remaining blocker is contributor-supplied real behavior proof plus the small release-owned changelog cleanup, not an automated repair lane.

Security
Cleared: The diff forwards an already-supported formatting payload through an existing bridge call and adds tests; no new dependency, secret, permission, or supply-chain concern was found.

Review findings

  • [P3] Drop the release-owned changelog entry — CHANGELOG.md:5-6
Review details

Best possible solution:

Merge the RPC bridge forwarding and regression tests after inspectable real behavior proof is attached and release notes stay in the maintainer-owned process.

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

Yes, from source inspection: current main's handleSend never reads formatting params and sendViaBridge sends only chatGuid/message, while the bridge already consumes textFormatting. I did not run a live Messages-device reproduction.

Is this the best way to solve the issue?

Yes, forwarding the existing formatting payload into the existing bridge textFormatting contract is the narrow maintainable fix, while leaving AppleScript sends plain preserves the current limitation. The changelog entry should be removed or explicitly carried by maintainers.

Full review comments:

  • [P3] Drop the release-owned changelog entry — CHANGELOG.md:5-6
    CHANGELOG.md is release-owned in this workflow, so this contributor PR should keep release-note context in the PR body or commit message unless a maintainer explicitly wants the changelog line carried here.
    Confidence: 0.91

Overall correctness: patch is correct
Overall confidence: 0.85

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This fixes a bounded RPC bridge formatting bug for direct handle sends without changing plain-send delivery semantics.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish 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: The PR body describes real-device verification, but it does not include inspectable redacted proof such as a screenshot, recording, terminal output, copied live output, or logs. 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

What I checked:

  • Repository policy read: Read the full 31-line AGENTS.md; its focused-changes and regression-test guidance applies to this RPC bridge fix review. (AGENTS.md:1, e4a14f4056fb)
  • Current main drops RPC send formatting: On current main, handleSend calls sendViaBridge with only chatGUID, text, and file, so formatting/textFormatting params are never read on the plain RPC send path. (Sources/imsg/RPCServer+Handlers.swift:262, e4a14f4056fb)
  • Current bridge send helper stays plain: sendViaBridge currently invokes .sendMessage with only chatGuid and message for text sends, confirming the bridge never receives text formatting from send. (Sources/imsg/RPCServer+Handlers.swift:536, e4a14f4056fb)
  • Existing bridge contract supports formatting: handleSendRich already forwards text_formatting/textFormatting, and the injected bridge helper consumes textFormatting to build a formatted attributed body. (Sources/imsg/RPCServer+BridgeMessageHandlers.swift:27, e4a14f4056fb)
  • PR forwards formatting and covers regression cases: The PR adds text_formatting/textFormatting/formatting forwarding into bridge textFormatting and tests both the formatted and absent-formatting cases. (Sources/imsg/RPCServer+Handlers.swift:167, 282fc8fcc8a4)
  • Proof is claimed but not inspectable: The PR body describes real-device before/after verification, but gh pr view shows no screenshot, recording, terminal output, copied live output, redacted log, or linked artifact for reviewers to inspect. (282fc8fcc8a4)

Likely related people:

  • omarshahine: Prior merged bridge work introduced the private bridge surface and text-formatting support that this PR extends from send.rich to plain RPC send. (role: bridge text-formatting feature contributor; confidence: high; commits: c56c24d488ef, 0ece0cc4d0e6, 574bb2cfa353; files: Sources/IMsgHelper/IMsgInjected.m, Sources/imsg/Commands/BridgeMessagingCommands.swift, Sources/imsg/RPCServer+BridgeMessageHandlers.swift)
  • steipete: Current available blame for handleSend/sendViaBridge is on the v0.11.1 release commit, and prior commits exposed bridge message RPC methods and adjusted bridge IPC/review feedback. (role: recent RPC and bridge area contributor; confidence: medium; commits: f97a116dc8df, b71eb8c0f6f1, caac981480df; files: Sources/imsg/RPCServer+Handlers.swift, Sources/imsg/RPCServer+BridgeMessageHandlers.swift, Sources/imsg/Commands/BridgeMessagingCommands.swift)
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 rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. labels Jun 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant