feat(blox-cli): strip ANSI escape sequences from stderr before error classification#416
feat(blox-cli): strip ANSI escape sequences from stderr before error classification#416
Conversation
…classification Add strip_ansi_escape_sequences() to sanitize stderr output before passing it to is_auth_error() and before including it in BloxError::CommandFailed. This prevents ANSI color/style codes (CSI and OSC sequences) emitted by sq from interfering with string-matching in error classification. This aligns with the sanitization that was previously implemented in the nexus repo's local copy of blox-cli. Includes unit tests covering CSI sequences, OSC sequences (both BEL and ST terminated), plain text passthrough, empty input, and mixed sequences.
The doc comment for is_auth_error was incorrectly merged into the strip_ansi_escape_sequences doc block. Move it back to is_auth_error. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bee9f104b1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let stderr = strip_ansi_escape_sequences(&String::from_utf8_lossy(&stderr)); | ||
| if is_auth_error(&stderr) { | ||
| return Err(BloxError::NotAuthenticated); | ||
| } | ||
| return Err(BloxError::CommandFailed(stderr.into_owned())); | ||
| return Err(BloxError::CommandFailed(stderr)); |
There was a problem hiding this comment.
Preserve original stderr for non-auth failures
If sq formats a non-auth error with OSC sequences (for example an OSC-8 hyperlink), this now strips that data before returning CommandFailed, so callers no longer receive the exact CLI message. That matters in the start_workspace path, which explicitly preserves the original stderr so users can see the onboarding URL/action (apps/staged/src-tauri/src/branches.rs:1251-1258). A safer change is to strip a temporary copy only for is_auth_error, while leaving the original stderr in CommandFailed.
Useful? React with 👍 / 👎.
Summary
strip_ansi_escape_sequenceshelper that removes CSI and OSC sequences from stringsis_auth_errorclassification so terminal colour/style codes fromsqdon't break string matchingis_auth_errorTest plan
🤖 Generated with Claude Code