fix(shell-output): 🐛 Strip ANSI escape sequences from tool output

[jorben]

Summary

• Extract a shared Rust sanitizer for terminal-style ANSI, OSC, and DCS output sequences.
• Sanitize shell tool stdout/stderr before truncation so tool cards no longer show raw escape-code mojibake.
• Reuse the same sanitizer for terminal output snapshots while keeping the live PTY stream raw for xterm rendering.

Test Plan

• cargo fmt --manifest-path src-tauri/Cargo.toml --check
• cargo test --manifest-path src-tauri/Cargo.toml output_sanitizer -- --nocapture
• cargo test --manifest-path src-tauri/Cargo.toml truncation -- --nocapture

🤖 Generated with TiyCode

[github-actions]

AI Code Review Summary

PR: #111 (fix(shell-output): 🐛 Strip ANSI escape sequences from tool output)
Preferred language: English

Overall Assessment

Detected 1 actionable findings, prioritize CRITICAL/HIGH before merge.

Major Findings by Severity

• LOW (1)
  • src-tauri/src/core/executors/output_sanitizer.rs:77 - Carriage return handling drops character instead of simulating overwrite

Actionable Suggestions

• Consider handling \r by clearing the current line buffer instead of just dropping it to avoid duplicating progress bar outputs.
• Consider checking for newlines before applying output.pop() for backspace characters to prevent crossing line boundaries.
• Verify that all callers of truncate_tail_bytes expect sanitized text output, as its behavior has changed to strip control characters.

Potential Risks

• Agent may receive duplicated text if processes use carriage returns to overwrite lines.
• Backspaces at the start of a line may incorrectly delete the preceding newline.
• Binary data passed to truncate_tail_bytes will have control characters stripped.

Test Suggestions

• Add a test case for carriage return overwriting (e.g., hello\rJ).
• Add a test case for backspace at the start of a line (e.g., hello\n\u{8}Z).
• Add a test case for truncate_tail_bytes with binary/control characters outside of ANSI sequences.

File-Level Coverage Notes

• src-tauri/src/core/executors/output_sanitizer.rs: ok (Good extraction of the sanitizer into a shared module with comprehensive test coverage for standard ANSI sequences and edge cases like truncated sequences.)
• src-tauri/src/core/terminal_manager.rs: ok (Clean refactoring to use the shared sanitizer. Correctly preserves raw output for the live UI while sanitizing agent snapshots.)
• src-tauri/src/core/executors/truncation.rs: ok (Good integration of sanitization before truncation, ensuring ANSI codes don't inflate byte/line counts. Tests added are appropriate and cover the new behavior.)
• src-tauri/src/core/executors/mod.rs: ok (Trivial and correct module declaration for the new output_sanitizer.)

Inline Downgraded Items (processed but not inline)

• None

Coverage Status

• Target files: 4
• Covered files: 4
• Uncovered files: 0
• No-patch/binary covered as file-level: 0
• Findings with unknown confidence (N/A): 0

Uncovered list:

• None

No-patch covered list:

• None

Runtime/Budget

• Rounds used: 1/4
• Planned batches: 1
• Executed batches: 1
• Sub-agent runs: 1
• Planner calls: 1
• Reviewer calls: 1
• Model calls: 2/64
• Structured-output summary-only degradation: NO

[github-actions]

Automated PR review completed.

• Findings kept: 4
• Findings with unknown confidence: 0
• Inline comments attempted: 4
• Target files: 4
• Covered files: 4
• Uncovered files: 0
  See the summary comment for detailed analysis and coverage details.

[github-actions]

Automated PR review completed.

• Findings kept: 0
• Findings with unknown confidence: 0
• Inline comments attempted: 1
• Target files: 4
• Covered files: 4
• Uncovered files: 0
  See the summary comment for detailed analysis and coverage details.

[github-actions]

Automated review completed for this PR diff. No concrete inline issue was selected after aggregation.

[github-actions]

Automated PR review completed.

• Findings kept: 1
• Findings with unknown confidence: 0
• Inline comments attempted: 1
• Target files: 4
• Covered files: 4
• Uncovered files: 0
  See the summary comment for detailed analysis and coverage details.

[github-actions]

[LOW] Carriage return handling drops character instead of simulating overwrite

The sanitizer simply drops the \r character. In terminal semantics, \r moves the cursor to the beginning of the line, and subsequent characters overwrite the existing line. Dropping it can result in duplicated text for outputs with progress bars or carriage returns.

Suggestion: Consider maintaining line-level context to handle carriage returns by truncating the current line back to the beginning, or at least document this limitation.

Risk: Agent may receive duplicated text if a process uses \r to overwrite the current line (e.g., progress bars).

Confidence: 0.85

[From SubAgent: general]