-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Fix UTF-8 encoding in ExecaTerminalProcess #3989
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix UTF-8 encoding in ExecaTerminalProcess #3989
Conversation
72b6a44 to
e976075
Compare
e976075 to
34ee039
Compare
|
Hey @mr-ryan-james, this makes sense, I'll try to repro the issue and see if this is a correct fix. |
|
Hey @mr-ryan-james. Just tested this and can confirm the fix works as intended. I created a test script that simulates the encoding issue, without the UTF-8 env vars, Ruby defaults to US-ASCII which would break CocoaPods. With your fix setting LANG/LC_ALL to en_US.UTF-8, everything works. I think it might be worth adding a unit test for this. I noticed ExecaTerminalProcess doesn't have test coverage yet. Could help catch any future regressions. What do you think? |
Thanks for taking the time to test thsi! I will start iterating a bit on tesing my functionality. |
- Set LANG and LC_ALL environment variables to en_US.UTF-8 - Resolves Encoding::CompatibilityError in Ruby/CocoaPods commands - Ensures consistent UTF-8 encoding across all terminal sessions This change addresses the terminal encoding issue where commands like 'pod install' would fail due to incompatible character encoding. The fix ensures all integrated terminals are initialized with proper UTF-8 locale settings.
- Tests verify LANG and LC_ALL are set to en_US.UTF-8 - Tests ensure existing environment variables are preserved - Tests confirm UTF-8 settings override conflicting locale values - Addresses PR feedback requesting test coverage for encoding fix - All 7 tests passing with proper mocking of execa and ps-tree
34ee039 to
ac674a1
Compare
|
Hey @mr-ryan-james, Thanks for adding the tests so quickly! Everything looks good to me now. |
Problem
I discovered an Encoding::CompatibilityError when running commands like
pod install(CocoaPods/Ruby) within Roo Code's integrated terminal when the "Disable terminal shell integration" setting is enabled. The root cause was identified as the ExecaTerminalProcess not being configured with UTF-8 encoding, leading to character encoding compatibility issues in Ruby scripts.The error message specifically suggested setting
export LANG=en_US.UTF-8.Root Cause Analysis
When users enable the "Disable terminal shell integration" setting, Roo Code switches from VSCode's built-in terminal to the ExecaTerminal implementation. This fallback path uses Node.js
execalibrary viaExecaTerminalProcess, which was not inheriting proper UTF-8 locale settings.Code Path When "Disable Terminal Shell Integration" is Enabled:
executeCommandTool.ts:149→provider = "execa"TerminalRegistry.ts:133→new ExecaTerminal()ExecaTerminal.ts:21→new ExecaTerminalProcess()ExecaTerminalProcess.ts→ 🎯 Where UTF-8 fix is appliedSolution
This PR resolves the UTF-8 encoding issue by ensuring the ExecaTerminalProcess sets proper UTF-8 locale environment variables when creating subprocesses.
Changes Made:
Updated ExecaTerminalProcess Environment Configuration (
src/integrations/terminal/ExecaTerminalProcess.ts):LANG: "en_US.UTF-8"to the subprocess environment variablesLC_ALL: "en_US.UTF-8"to the subprocess environment variablesScope and Impact
✅ What this fixes:
✅ What this doesn't affect:
Testing
The fix has been tested and resolves the
Encoding::CompatibilityErrorthat was preventing Ruby/CocoaPods commands from executing successfully when the "Disable terminal shell integration" setting is enabled.Alignment with Project Goals
This change aligns with the Reliability First roadmap goal by:
Type of Change
Technical Details
This fix specifically targets the ExecaTerminal code path which is only used when:
execainstead of VSCode terminal APIExecaTerminalProcesssubprocess creationChecklist
Fixes #4033