Skip to content

fix #84079: normalize SendMessage/content/text aliases to message before send validation#84102

Merged
steipete merged 1 commit into
openclaw:mainfrom
zhangguiping-xydt:feat/issue-84079
May 22, 2026
Merged

fix #84079: normalize SendMessage/content/text aliases to message before send validation#84102
steipete merged 1 commit into
openclaw:mainfrom
zhangguiping-xydt:feat/issue-84079

Conversation

@zhangguiping-xydt

@zhangguiping-xydt zhangguiping-xydt commented May 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #84079

Issue

message tool rejects model-generated SendMessage instead of normalizing it to message

Changes

  • Normalize common text-body aliases (SendMessage, content, text) to the canonical message field before send validation.
  • Sanitize formatted reasoning content before alias text is delivered.
  • Share the formatted-reasoning sanitizer between the agent tool and outbound action runner.
  • Add regression coverage for alias normalization, sanitizer behavior, and missing-message rejection.

Real behavior proof

Behavior addressed: PR #84102 fixes the outbound message send path so model-produced body aliases (SendMessage, content, text) are normalized to message before 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:

npx vitest run src/infra/outbound/message-action-runner.send-validation.test.ts --reporter=verbose
node scripts/check-changed.mjs

Evidence after fix:

Focused send-path regression output from head 3df704863afcab8d516aae6556192d8ab91ecbd7:

[message-tool] normalized alias "SendMessage" to "message" for send action
[message-tool] normalized alias "content" to "message" for send action
[message-tool] normalized alias "text" to "message" for send action
[message-tool] normalized alias "SendMessage" to "message" for send action
[message-tool] normalized alias "SendMessage" to "message" for send action

✓ runMessageAction send validation > requires message when no media hint is provided
✓ runMessageAction send validation > allows send when only presentation payloads are provided
✓ runMessageAction send validation > allows send when only generic presentation blocks are provided
✓ runMessageAction send validation > uses the current internal UI source as the message-tool-only send sink
✓ runMessageAction send validation > does not infer an internal UI sink outside message-tool-only source delivery
✓ runMessageAction send validation > keeps explicit message routes on the normal outbound path
✓ runMessageAction send validation > rejects send actions that include structured/string/snake_case/negative poll params
✓ message body alias normalization > normalizes 'SendMessage' alias to message for send
✓ message body alias normalization > normalizes 'content' alias to message for send
✓ message body alias normalization > normalizes 'text' alias to message for send
✓ message body alias normalization > does not overwrite an explicit message with an alias
✓ message body alias normalization > emits a diagnostic warning when normalizing an alias
✓ message body alias normalization > sanitizes SendMessage alias 'reasoning tag' before delivery
✓ message body alias normalization > sanitizes SendMessage alias 'formatted reasoning prefix' before delivery
✓ message body alias normalization > still rejects send with no message and no alias

Test Files  1 passed (1)
Tests       18 passed (18)

Changed-check output from the same head:

[check:changed] lanes=all
[check:changed] src/agents/tools/message-tool.ts: core production
[check:changed] src/infra/outbound/message-action-runner.send-validation.test.ts: core test
[check:changed] src/infra/outbound/message-action-runner.ts: core production
[check:changed] src/shared/text/formatted-reasoning-message.ts: core production

[check:changed] conflict markers
[check:changed] changelog attributions
[check:changed] guarded extension wildcard re-exports
No guarded extension wildcard re-exports found.
[check:changed] plugin-sdk wildcard re-exports
No plugin-sdk wildcard re-exports found in extension API barrels.
[check:changed] duplicate scan target coverage
[dup:check] target coverage ok
[check:changed] dependency pin guard
PASS direct dependency pin guard: checked 432 directly declared dependency specs across 131 tracked package manifests; 0 violations.
[check:changed] package patch guard
PASS package patch guard: no new pnpm patches; 2 legacy patches allowlisted.
[check:changed] runtime sidecar loader guard
runtime-sidecar-loaders: local runtime sidecar loaders look OK.
[check:changed] typecheck all
[check:changed] lint
[oxlint:core] finished
[oxlint:extensions] finished
[oxlint:scripts] finished
[check:changed] runtime import cycles
Import cycle check: 0 runtime value cycle(s).
Exit code: 0

Observed result after fix: The focused outbound tests exercise the send validation path and confirm SendMessage, content, and text aliases are normalized before validation; explicit message is not overwritten; formatted reasoning content is stripped before delivery; and a send with no message and no alias still rejects with message 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.

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 19, 2026
@clawsweeper

clawsweeper Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

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
  • 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.

Summary
The branch normalizes SendMessage, content, and text send-body aliases to message, shares formatted-reasoning stripping with the outbound runner, and adds focused send-validation regression tests plus a changelog entry.

Reproducibility: yes. source-reproducible: current main only reads message before required send validation, and the linked report includes logs where SendMessage is rejected with message required. I did not run a live Discord/Pi replay in this read-only review.

PR rating
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦞 diamond lobster
Summary: The patch is focused and reviewed cleanly, but stale/non-current real behavior proof keeps the PR below merge-ready evidence quality.

Rank-up moves:

  • Update the PR body with redacted current-head real send proof showing SendMessage, content, or text delivering as message and hidden reasoning text not being sent.
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.

Real behavior proof
Needs stronger real behavior proof before merge: The PR body proof is stale relative to the latest head and is test/check output; prior direct-send proof comments are for older heads, so current-head real behavior proof is still needed before merge. 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 send-path proof would materially confirm that aliased message bodies deliver and hidden reasoning text is stripped outside mocks. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify that a message tool send using SendMessage, content, and text aliases delivers visible text and strips hidden reasoning text.

Risk before merge

  • The current PR head lacks redacted real send-path proof: older direct-send comments were for prior heads, and the PR body now shows test/check output from an older SHA.
  • Because this changes central send validation, a bad alias normalization path could deliver text that would previously be rejected, so maintainers should see current-head delivery proof before merge.

Maintainer options:

  1. Require current-head send proof (recommended)
    Ask for a redacted direct OpenClaw send or live transport run from head 3f96ffacd2eeee5d8b74f98607435f79c76dd15e showing alias delivery and reasoning stripping.
  2. Accept unit-only confidence
    Maintainers could merge on the focused tests and code review alone, but that accepts the remaining real message-delivery proof gap.
  3. Pause if proof cannot be refreshed
    If no current-head proof can be supplied after the force-push, keep the linked issue open and pause or close this branch rather than merging stale evidence.

Next step before merge
No automated repair is indicated; the branch needs current-head real behavior proof or an explicit maintainer decision to accept the remaining message-delivery proof gap.

Security
Cleared: The diff changes send-argument normalization, a shared text sanitizer helper, tests, and changelog only; I found no dependency, workflow, permission, credential, or package-execution regression.

Review details

Best 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 message before required send validation, and the linked report includes logs where SendMessage is rejected with message required. I did not run a live Discord/Pi replay in this read-only review.

Is this the best way to solve the issue?

Yes for the code direction: normalizing aliases centrally in buildSendPayloadParts fixes the model-emitted argument shape before validation without provider-specific prompt workarounds. The remaining blocker is current-head real behavior proof, not a different implementation path.

Label justifications:

  • P1: The linked bug drops user-visible Discord progress/final replies for Anthropic/Pi-backed agent runs, which is a broken channel workflow affecting real users.
  • merge-risk: 🚨 message-delivery: The PR changes the central send validation path, so merge safety depends on proving aliased message bodies are delivered and hidden reasoning is not sent.
  • rating: 🦪 silver shellfish: Current PR rating is 🦪 silver shellfish because proof is 🦪 silver shellfish, patch quality is 🦞 diamond lobster, and The patch is focused and reviewed cleanly, but stale/non-current real behavior proof keeps the PR below merge-ready evidence quality.
  • 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 proof is stale relative to the latest head and is test/check output; prior direct-send proof comments are for older heads, so current-head real behavior proof is still needed before merge. 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.

Acceptance criteria:

  • Redacted current-head direct OpenClaw send or live transport proof for an aliased body key delivering as message and reasoning text being stripped.
  • Focused regression test for src/infra/outbound/message-action-runner.send-validation.test.ts.
  • Changed-file gate equivalent to node scripts/check-changed.mjs or the repo-approved changed-check lane.

What I checked:

  • Current-main failure path: On current main, buildSendPayloadParts reads only the canonical message field with required validation when no media/presentation/interactive payload exists, matching the linked report's SendMessage -> message required failure mode. (src/infra/outbound/message-action-runner.ts:763, 57178b188bd6)
  • PR implementation path: The live PR patch adds alias normalization before send validation in buildSendPayloadParts, preserving explicit message and warning when an alias is used. (src/infra/outbound/message-action-runner.ts:752, 3f96ffacd2ee)
  • Sanitizer reuse: The patch extracts the existing formatted-reasoning stripping helper from the agent message tool into src/shared/text/formatted-reasoning-message.ts and imports it in the outbound runner. (src/shared/text/formatted-reasoning-message.ts:1, 3f96ffacd2ee)
  • Regression coverage: The patch adds tests for SendMessage, content, and text alias normalization, explicit-message precedence, diagnostic warning, reasoning stripping, and continued no-body rejection. (src/infra/outbound/message-action-runner.send-validation.test.ts:281, 3f96ffacd2ee)
  • Proof is stale for latest head: The live PR head is 3f96ffacd2eeee5d8b74f98607435f79c76dd15e, while the PR body's proof names 3df704863afcab8d516aae6556192d8ab91ecbd7 and contains test/check output rather than current-head real send proof. (3f96ffacd2ee)
  • Prior direct-send proof was for older heads: Contributor comments report dryRun:false direct-send proof for older heads d783d289... and 2b864874..., but no comment or PR-body section proves the latest 3f96ff... head after the final force-push.

Likely related people:

  • steipete: Current-main blame for the central outbound send builder and existing formatted-reasoning helper points to Peter Steinberger, and recent full-history commits touch outbound routing and generic delivery/security seams. (role: recent area contributor; confidence: high; commits: 48bf0374c806, 2e3ef1b9e180, 856592cf001b; files: src/infra/outbound/message-action-runner.ts, src/agents/tools/message-tool.ts)
  • gumadeiras: Gustavo Madeira Santana appears repeatedly in message action discovery, dispatch, plugin SDK message-tool, and outbound fallback history around this surface. (role: feature-history contributor; confidence: medium; commits: 3ca8ad38459c, 951f3f992b68, a32c7e16d204; files: src/agents/tools/message-tool.ts, src/infra/outbound/message-action-runner.ts)
  • vincentkoc: Vincent Koc contributed message action capability and shared interactive schema work that defines adjacent channel/message-tool contracts. (role: adjacent architecture contributor; confidence: medium; commits: 92bea9704e17, c1846000ddbc, 74e7b8d47b18; files: src/agents/tools/message-tool.ts, src/infra/outbound/message-action-runner.ts)
  • martingarramon: Reviewed this PR discussion after the branch was narrowed and explicitly said buildSendPayloadParts looked correct. (role: reviewer; confidence: medium; files: src/infra/outbound/message-action-runner.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 57178b188bd6.

@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. labels May 19, 2026

@martingarramon martingarramon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (SendMessagecontenttext) 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.

@openclaw-barnacle openclaw-barnacle Bot removed the app: web-ui App: web-ui label May 19, 2026
@zhangguiping-xydt

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated this PR with a clean branch on current upstream/main and added real behavior proof to the PR body. Summary:

  • Started OpenClaw from this PR worktree at d783d2891dabf5ac0480d5ba050a0d266c7fbe83 using isolated OPENCLAW_HOME / state dir on port 19102; gateway reached [gateway] ready and ss -ltnp confirmed openclaw listening on 127.0.0.1:19102.
  • Ran a dryRun:false send-path proof for SendMessage, content, and text; each alias normalized to message, reached runMessageAction -> executeSendAction -> sendMessage -> sendDurableMessageBatch -> outbound adapter sendText, and returned message ids proof-1, proof-2, proof-3.
  • Confirmed missing body still rejects with message required.
  • pnpm test src/infra/outbound/message-action-runner.send-validation.test.ts -- --reporter=verbose: 16 passed.
  • pnpm check:changed: passed.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 19, 2026
@clawsweeper

clawsweeper Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 19, 2026
@zhangguiping-xydt

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Rebased this PR onto the latest upstream/main and refreshed the real behavior proof for new head 2b86487467c57e6cd9ffd1c96dea5f762b5b180a.

  • OpenClaw started from this PR worktree with isolated OPENCLAW_HOME / state dir on port 19102; gateway reached [gateway] ready, and ss -ltnp showed openclaw listening on 127.0.0.1:19102.
  • SendMessage, content, and text were each run through runMessageAction({ action: "send", dryRun: false }) into the core outbound direct send path and adapter sendText, returning proof-1, proof-2, and proof-3.
  • Empty send with no message and no alias still rejects with message required.
  • pnpm test src/infra/outbound/message-action-runner.send-validation.test.ts -- --reporter=verbose: 16 passed.
  • pnpm check:changed: passed.

@clawsweeper

clawsweeper Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 19, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 19, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 19, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 20, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels May 20, 2026
@clawsweeper

clawsweeper Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 20, 2026
@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. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels May 21, 2026
@clawsweeper clawsweeper Bot removed the merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. label May 21, 2026
@openclaw-barnacle openclaw-barnacle Bot added triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. and removed proof: supplied External PR includes structured after-fix real behavior proof. labels May 22, 2026
@zhangguiping-xydt

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 22, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. label May 22, 2026
@steipete steipete force-pushed the feat/issue-84079 branch from 3df7048 to 3f96ffa Compare May 22, 2026 19:18
@steipete steipete merged commit 6354569 into openclaw:main May 22, 2026
98 checks passed
SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 24, 2026
SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 24, 2026
SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 24, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 24, 2026
galiniliev pushed a commit to galiniliev/openclaw that referenced this pull request May 25, 2026
SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 26, 2026
SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 26, 2026
SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 26, 2026
jameslcowan pushed a commit to jameslcowan/openclaw that referenced this pull request Jun 2, 2026
SYU8384 pushed a commit to SYU8384/openclaw that referenced this pull request Jun 3, 2026
sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P1 High-priority user-facing bug, regression, or broken workflow. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: M 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.

[Bug]: message tool rejects model-generated SendMessage instead of normalizing it to message

3 participants