Repository navigation
fix(chat): multi-line composer messages sent with stray carriage returns - #1682
Conversation
The multiline composer (Enter sends, Shift+Enter newline) stores WinUI TextBox line breaks as carriage returns. SendAsync forwarded that raw format, so multi-line messages left the tray as lone CR bytes. Normalize CR and CRLF to LF at send time, keeping other Unicode line separators in user content untouched. Add a controller unit test and a real-process keyboard proof that Enter sends and Shift+Enter inserts a newline without sending.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
larroy
added
the
status: 🚢 actively landing
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed October 8, 2026, 1:58 PM ET / 17:58 UTC. ClawSweeper reviewWhat this changesThe branch converts carriage-return and Windows-style line breaks to line feeds when sending composer messages and adds controller and native keyboard tests. Example: The user sends a draft containing three lines.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review This PR addresses a defect still present on current main. The focused normalization is correct; exact outgoing line endings have unit coverage, while the reported native-app run proves adjacent keyboard behavior. Priority: P2 Before mergeNone. FindingsNone. Tests
Agent review detailsHow this fits togetherThe native chat composer holds the user's draft and submits it through a shared controller. The chat provider queues that text and the gateway client sends it in a chat request. flowchart LR
A[Native chat input] --> B[Composer draft]
B --> C[Send controller]
C --> D[Normalize carriage returns]
D --> E[Chat provider and queue]
E --> F[Gateway chat request]
Technical reviewBest possible solution: Keep normalization at the composer submission boundary, preserving native draft text and existing send, queue and attachment ownership. Do we have a high-confidence way to reproduce the issue? Yes, source establishes that a draft containing internal carriage returns reaches the outgoing message unchanged on main. No failing runtime run was performed during this read-only review. Is this the best way to solve the issue? Yes, converting CRLF and lone CR at submission is a narrow repair in the existing owner and avoids rewriting the controlled draft or caret. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 037c17dcb581. Provenance checked
TestingProof path: unit tests only. Added test files: 2. SecurityNone. EvidenceWhat I checked:
Likely related people:
Review metrics
LabelsLabel changes:
Label justifications:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment |
|
Global triage: NEEDS_HUMAN_TEST. Take confidence 55%; recommendation confidence 97%; effort small; risk medium. Reviewed exact head Maintainer or proof-pool owner: run every declared Windows proof pool at this exact head and replace blocked or historical evidence with current-head results. This is a triage classification, not a merge action. No branch changes were made. |
|
I tested myself by hand as well. |
Integrate the multiline composer normalization after the queued-draft preservation fix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a7c0b5c5-d1fc-468d-a9b1-e58c402f13e8
Keep the line-ending regression at the send boundary and rely on the existing opt-in NativeKeyboard coverage for foreground key injection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a7c0b5c5-d1fc-468d-a9b1-e58c402f13e8
shanselman
removed
the
status: 🚢 actively landing
3 participants
What Problem This Solves
Fixes: multi-line messages can leave the native composer with carriage returns and reach the Gateway without standard line-feed separators.
User Impact
User impact: line breaks typed in the native chat composer are sent as standard
\nseparators. Single-line messages and draft behavior remain unchanged.Why This Change Was Made
WinUI TextBox drafts can contain lone
\rcharacters. Normalize CRLF and lone CR once at the send boundary, preserving the native draft and every other Unicode character.The maintainer update integrates current
mainafter #1667 and removes a redundant foregroundSendKeystest that was not behind the repository's required opt-inNativeKeyboardlane. The exact outgoing message remains covered at the runtime-port boundary, while current main retains the separate gated native keyboard proof.Evidence
SendAsync_TextBoxLineBreaks_AreSentAsLineFeedssupplies a draft containing both lone CR and CRLF and verifies that the runtime port receives exactly:Two independent reviews found no production defect in the final two-file PR delta. The final integration with latest main and the clean-worktree build fix passed the complete local closeout.
Change Type
Scope
winnodeRequired proof pools
none: the changed contract is the exact string passed from the composer controller to the runtime port. Foreground key injection is unchanged and remains covered by the existing opt-inNativeKeyboardlane.Validation
Validation tree: PR head
2750135c3ef72cbe8bdc4cce465f42d786449e95, latest main998e6b17f30ef545f0cc11f4392bad26216bc460, and #1727's clean-build fix. The two PR-changed files are byte-identical to the published PR head..\build.ps1dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csprojdotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csprojThe test commands omitted
--no-restorebecause the proof used a new worktree with no test assets.Real Behavior Proof
2750135c3ef72cbe8bdc4cce465f42d786449e95; full validation treefc73630fc13a0ef73a8e2e9233ffe2ebf3c4090f.SendAsync_TextBoxLineBreaks_AreSentAsLineFeeds.Yes/No/N/A): N/A, no visual output changes.Security Impact
Yes/No): NoYes/No): NoYes/No): NoYes/No): NoYes/No): NoYes, explain the risk and mitigation: N/A.Compatibility and Migration
Yes/No): YesYes/No): NoYes/No): NoReview Conversations