Skip to content

fix: sanitize control characters in terminal output#2007

Open
tengfeiwang1 wants to merge 2 commits intoQwenLM:mainfrom
tengfeiwang1:main
Open

fix: sanitize control characters in terminal output#2007
tengfeiwang1 wants to merge 2 commits intoQwenLM:mainfrom
tengfeiwang1:main

Conversation

@tengfeiwang1
Copy link
Copy Markdown

Summary

Fixes #1950 - Terminal output with \r\n or standalone \r causes interface misalignment in Ink-based UI.

Problem

When tool output contains Windows-style line endings (\r\n) or standalone carriage returns (\r), the terminal interface becomes misaligned because the carriage return moves the cursor to the beginning of the line without advancing, causing subsequent content to overwrite previous content.

Solution

Following Claude Code's approach to terminal output handling, this PR adds a sanitization layer that:

  1. Normalizes \r\n to \n (Windows line endings)
  2. Converts standalone \r to \n (progress bar scenarios)
  3. Removes other problematic control characters

Files Changed

  • New: packages/core/src/utils/controlCharSanitizer.ts - Core sanitization utility
  • New: packages/core/src/utils/controlCharSanitizer.test.ts - 12 unit tests
  • Modified: packages/core/src/tools/shell.ts - Apply sanitization at tool layer
  • Modified: packages/cli/src/ui/hooks/shellCommandProcessor.ts - Apply sanitization for real-time output
  • Modified: packages/core/src/utils/terminalSerializer.ts - Apply sanitization at serialization layer

Testing

  • ✅ 12/12 unit tests for controlCharSanitizer
  • ✅ 53/53 tests for shell.test.ts
  • ✅ 3686/3688 total tests passing
  • ✅ No regression issues

Checklist

  • Code follows project style guidelines
  • Tests added and passing
  • No regression issues
  • Commit message follows conventional commits

- Add controlCharSanitizer utility to handle \r, \r\n, and other control chars
- Apply sanitization at shellCommandProcessor, shell tool, and terminalSerializer
- Prevents rendering issues in Ink-based UI caused by carriage returns
- Add comprehensive unit tests (12 test cases)

Fixes QwenLM#1950
Copy link
Copy Markdown
Collaborator

@DennisYu07 DennisYu07 left a comment

Choose a reason for hiding this comment

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

LGTM!

@tanzhenxin
Copy link
Copy Markdown
Collaborator

@DennisYu07 Pls help reproduce this issue and attach screenshots before and after the change.
This code has some really bad coding style just by a glance, we need pay more attention to this PR before approving.

@DennisYu07 DennisYu07 added 0.13.0 and removed 0.13.0 labels Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

工具的输出有\r\n的时候会导致界面错位

3 participants