fix #84079: normalize SendMessage/content/text aliases to message before send validation#84102
Conversation
|
Codex review: needs real behavior proof before merge. Latest ClawSweeper review: 2026-05-22 19:27 UTC / May 22, 2026, 3:27 PM ET. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: yes. source-reproducible: current main only reads PR rating Rank-up moves:
What the crustacean ranks mean
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. Real behavior proof Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge Security Review detailsBest possible solution: Land the centralized alias normalization after current-head real OpenClaw send proof shows aliased body keys deliver and formatted reasoning text is stripped. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: current main only reads Is this the best way to solve the issue? Yes for the code direction: normalizing aliases centrally in Label justifications:
Acceptance criteria:
What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 57178b188bd6. |
martingarramon
left a comment
There was a problem hiding this comment.
The alias normalization is placed at the right shared send boundary. Two things to address before merge:
CI failure — ## Real behavior proof heading required
The PR body has ## Verification Evidence (REAL) but scripts/github/real-behavior-proof-check.mjs requires a heading matching ## Real behavior proof exactly. Rename the section header; no content changes needed.
Unrelated UI changes
The PR includes changes to ui/src/styles/components.css and ui/src/ui/presenter.ts that are not part of the alias normalization fix. Remove these unrelated UI changes from this PR. If the i18n removal and CSS adjustments are intentional, they need their own PR with proof.
Specifically: presenter.ts:25 replaces t("common.na") with hardcoded "n/a" and changes toLocaleDateString(undefined, ...) to toLocaleDateString("en", ...). Removing the t() call hardcodes the string and loses translation support; the locale override changes date rendering for non-English users.
Core fix: shared send-path coverage confirmed
buildSendPayloadParts is the shared entry point for both handleSendAction and handleInternalSourceReplySendAction, so the alias normalization covers both send callers. The alias priority order (SendMessage → content → text) matches observed model behavior in the issue logs. Test coverage is comprehensive: all three aliases, no-op when message already present, warning assertion, rejection when all missing.
5eb819f to
d783d28
Compare
|
@clawsweeper re-review Updated this PR with a clean branch on current
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
d783d28 to
2b86487
Compare
|
@clawsweeper re-review Rebased this PR onto the latest
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
2b86487 to
daab797
Compare
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
3c897b6 to
3df7048
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
3df7048 to
3f96ffa
Compare
Summary
Fixes #84079
Issue
messagetool rejects model-generatedSendMessageinstead of normalizing it tomessageChanges
SendMessage,content,text) to the canonicalmessagefield before send validation.Real behavior proof
Behavior addressed: PR #84102 fixes the outbound message send path so model-produced body aliases (
SendMessage,content,text) are normalized tomessagebefore validation and dispatch. It also prevents hidden formatted reasoning text from being sent when it appears inside an alias payload.Real environment tested: Linux x86_64, Node v22.22.0, pnpm 11.1.0, PR head
3df704863afcab8d516aae6556192d8ab91ecbd7. Private machine/user/path details are redacted.Exact steps or command run after this patch:
Evidence after fix:
Focused send-path regression output from head
3df704863afcab8d516aae6556192d8ab91ecbd7:Changed-check output from the same head:
Observed result after fix: The focused outbound tests exercise the send validation path and confirm
SendMessage,content, andtextaliases are normalized before validation; explicitmessageis not overwritten; formatted reasoning content is stripped before delivery; and a send with nomessageand no alias still rejects withmessage required. The changed-file check completed typecheck, lint, import-cycle, dependency, package-patch, and project guard lanes successfully on the same PR head.What was not tested: No live external provider account was used. The proof avoids sending real messages to Slack, Feishu, or other external services.